lhumina_code/hero_slides

Fork 0

Create Slide: slug double-prefixed by auto-numbering, then fails silently #37

New issue

Open

opened 2026-04-29 11:13:20 +00:00 by salmaelsoly · 3 comments

salmaelsoly commented

2026-04-29 11:13:20 +00:00

Member

Summary

The Create Slide flow double-prefixes the slug when the user already typed a numeric prefix, then the deck-generation job fails to find the file (it uses the raw slug while the file was saved with an extra prefix), and the failure surfaces as a silent "Pending" state with no error in the UI. Combined, this is a single user-facing bug with three contributing code points.

Reproduction

Open Create Slide on a deck that already has 5 slides.
Type slug 06_conclusion.
Click Create.

Expected: file saved as 06_conclusion.md; gen runs; thumbnail appears.

Actual:

File saved as 06_06_conclusion.md (double prefix).
Generation job for that slide fails with Missing required file: slide not found: 06_conclusion.md.
The slide's UI stays in "Pending" state with no error indicator. Only the Jobs tab reveals the failure.

A second contributing UX issue appears earlier in the flow: the Create Deck modal (and Create Slide modal) silently keeps itself open if the user submits an empty name, with no inline validation message.

Root causes

1. Server unconditionally prepends `NN_` to the slug (the actual file-save bug)

crates/hero_slides_lib/src/slide_ops.rs:226:

// Pseudocode of what happens
let filename = format!("{}_{}", zero_pad2(at), slug);

at is the new slide's position; slug is whatever the client passed. There's no "if slug already starts with \d{2}_, strip it" logic.

Client side, the Create Slide handler at crates/hero_slides_ui/static/js/dashboard.js:1876 and :2569 passes the user's slug verbatim to slide.insert.

2. Pending-state badge is rendered unconditionally

crates/hero_slides_ui/static/js/dashboard.js:2003-2010:

The "Pending" badge is shown for any slide whose generation result is missing. There's no check for the case where the generation job exists but failed because the filename didn't match — that should be rendered as an error badge with the actual error.

crates/hero_slides_ui/static/js/dashboard.js:128-131:

function showPrompt(...) {
    ...
    if (!val) return;   // silent: no error, modal stays open
    onConfirm(val);
}

The Create Slide variant at :1880 does toast but still doesn't dismiss the modal.

Suggested fix

Server: in slide_ops.rs::insert_slide, strip a leading \d{2}_ from slug before prepending. Or: detect the prefix and reject with a clear error pointing the user at the slugify rule.
Client (Pending badge): when the slide's expected filename doesn't match any on-disk file or when the most recent gen-job for that slide has phase failed, render an error badge with the failure reason instead of "Pending".
Client (empty name): in showPrompt, render an inline error inside the modal and keep focus in the input. Don't fall through to a no-op return.

Severity

High. The double-prefix silently corrupts the slide-file naming convention and the failure isn't surfaced anywhere the user looks first.

## Summary The Create Slide flow double-prefixes the slug when the user already typed a numeric prefix, then the deck-generation job fails to find the file (it uses the raw slug while the file was saved with an extra prefix), and the failure surfaces as a silent "Pending" state with no error in the UI. Combined, this is a single user-facing bug with three contributing code points. ## Reproduction 1. Open Create Slide on a deck that already has 5 slides. 2. Type slug `06_conclusion`. 3. Click Create. **Expected:** file saved as `06_conclusion.md`; gen runs; thumbnail appears. **Actual:** - File saved as `06_06_conclusion.md` (double prefix). - Generation job for that slide fails with `Missing required file: slide not found: 06_conclusion.md`. - The slide's UI stays in "Pending" state with no error indicator. Only the Jobs tab reveals the failure. A second contributing UX issue appears earlier in the flow: the Create Deck modal (and Create Slide modal) silently keeps itself open if the user submits an empty name, with no inline validation message. ## Root causes ### 1. Server unconditionally prepends `NN_` to the slug (the actual file-save bug) `crates/hero_slides_lib/src/slide_ops.rs:226`: ```rust // Pseudocode of what happens let filename = format!("{}_{}", zero_pad2(at), slug); ``` `at` is the new slide's position; `slug` is whatever the client passed. There's no "if slug already starts with `\d{2}_`, strip it" logic. Client side, the Create Slide handler at `crates/hero_slides_ui/static/js/dashboard.js:1876` and `:2569` passes the user's slug verbatim to `slide.insert`. ### 2. Pending-state badge is rendered unconditionally `crates/hero_slides_ui/static/js/dashboard.js:2003-2010`: The "Pending" badge is shown for any slide whose generation result is missing. There's no check for the case where the generation job exists but failed because the filename didn't match — that should be rendered as an error badge with the actual error. ### 3. Empty-name submission silently keeps modal open `crates/hero_slides_ui/static/js/dashboard.js:128-131`: ```js function showPrompt(...) { ... if (!val) return; // silent: no error, modal stays open onConfirm(val); } ``` The Create Slide variant at `:1880` does toast but still doesn't dismiss the modal. ## Suggested fix - **Server:** in `slide_ops.rs::insert_slide`, strip a leading `\d{2}_` from `slug` before prepending. Or: detect the prefix and reject with a clear error pointing the user at the slugify rule. - **Client (Pending badge):** when the slide's expected filename doesn't match any on-disk file or when the most recent gen-job for that slide has phase `failed`, render an error badge with the failure reason instead of "Pending". - **Client (empty name):** in `showPrompt`, render an inline error inside the modal and keep focus in the input. Don't fall through to a no-op `return`. ## Severity High. The double-prefix silently corrupts the slide-file naming convention and the failure isn't surfaced anywhere the user looks first.

salmaelsoly referenced this issue

2026-04-29 11:24:19 +00:00

deck.generateAsync: phase 'failed' on partial success #39

salmaelsoly referenced this issue

2026-04-29 13:10:20 +00:00

fix(generate): treat partial deck-gen as success and surface per-slide outcomes #40

salmaelsoly commented

2026-04-30 12:15:10 +00:00

Author

Member

Implementation Spec for Issue #37

Objective

Fix the "Create Slide" double-prefix bug at three layers: stop the server from prepending NN_ when the slug already carries one, surface failed-slide outcomes from deck.generateAsync in the dashboard with an explicit error badge, and replace the silent empty-name no-op in both showPrompt and the Create Slide modal with inline validation that keeps the modal open.

Scope decisions

Sub-bug 1 (server slug): strip-and-flag, not reject. insert_slide detects a leading ^\d{2}_ in slug and removes it before computing the final filename. The boolean is bubbled up so the client can show a non-blocking notice. Rejection was considered and ruled out — 06_conclusion is a plausible thing for a user to type from muscle memory; a hard error breaks existing UI flows that already lower-case + slugify on the client.
Carrier of the flag: add slug_normalized: bool to the slide.insert JSON-RPC response. Internally insert_slide returns (String, bool) and slide_insert returns (SlideFile, bool). Tests cover the stripping logic at slide_ops.rs level.
Sub-bug 2 (failed-slide badge): consume status.per_slide from deck.generateJobStatus (already shipped in PR #40). When the poller sees done, build a Map<slideName, errorMessage> of ok: false entries, store it as _failedSlides, then loadSlidesForDeck() triggers the badge re-render. slideBadge(s) reads _failedSlides and emits a red state-badge-error with a title= tooltip showing the error. The map is cleared at the start of every deck.generateAsync run, and individual entries are cleared when a slide later succeeds. No new RPC, no extra state on the slide objects.
Sub-bug 3 (empty input): showPrompt and doCreateSlide show inline error text via a sibling <div class="invalid-feedback d-block"> and add is-invalid to the input. They do not call onConfirm, do not hide the modal, and re-focus the input. Typing in the input clears the invalid state.
Out of scope: any redesign of the Create Slide modal, server-side reject mode, or making slide.insert accept arbitrary slug characters. Existing client-side slug normalization (replace(/[^a-z0-9_]/g, '_')) is left untouched — only the leading-numeric-prefix case is added.

Files to Modify/Create

crates/hero_slides_lib/src/slide_ops.rs — extract strip_leading_position_prefix(&str) -> (String, bool) helper; change insert_slide to return Result<(String, bool), HeroSlidesError>; apply the strip before constructing new_name; update duplicate_slide caller. Add unit tests.
crates/hero_slides_lib/src/deck.rs — slide_insert propagates the bool. Change return type to Result<(SlideFile, bool), HeroSlidesError>. Update internal callers/tests.
crates/hero_slides_server/src/rpc.rs — handle_slide_insert: destructure the tuple; include slug_normalized in the JSON response.
crates/hero_slides_ui/static/js/dashboard.js:
- showPrompt: clear-on-open invalid state, on empty input add is-invalid, set inline error text, focus input, return without hiding. Wire an input listener that clears the invalid state on the next keystroke.
- doCreateSlide: replace toast(...) + return on empty slug with the same inline-error treatment. After successful slide.insert, if result.slug_normalized === true, show an info toast.
- insertSlide (the in-place insert at a position): same response handling.
- Add _failedSlides = new Map() near _generatingSlides. Helpers markSlideFailed(name, err), clearSlideFailed(name), clearAllFailedSlides().
- slideBadge(s): when _failedSlides.has(s.name), return <span class="state-badge state-badge-error" title="..." >Error</span> (priority below the generating spinner so retry shows spinner).
- pollGenerateProgress: on status.done with Array.isArray(status.per_slide), mark/clear per slide before loadSlidesForDeck(). Call clearAllFailedSlides() at the start of generateDeck().
- Per-slide generate paths: on success, clearSlideFailed(name); on failure, markSlideFailed(name, msg).
crates/hero_slides_ui/templates/index.html — add <div class="invalid-feedback" id="prompt-modal-error"></div> under #prompt-modal-input; add <div class="invalid-feedback" id="create-slide-title-error"></div> under #create-slide-title.
crates/hero_slides_ui/static/css/dashboard.css — add .state-badge-error (red fill, white text). Match the existing badge palette (verify the file by greping for .state-badge-pending).

Implementation Plan

Step 1: Server — slug strip-and-flag

Files: crates/hero_slides_lib/src/slide_ops.rs, crates/hero_slides_lib/src/deck.rs, crates/hero_slides_server/src/rpc.rs.
Dependencies: none.
Tasks:

Add private helper fn strip_leading_position_prefix(slug: &str) -> (String, bool) near parse_slide_stem. Logic: if slug.len() >= 3, first two bytes are ASCII digits, third is _, and the rest is non-empty, return (rest.to_string(), true); else (slug.to_string(), false). ASCII-only — matches the existing zero_pad2 convention.
In insert_slide, call the helper on the input slug before the existing format!("{}_{}", zero_pad2(at), slug). Change return type to Result<(String, bool), HeroSlidesError>.
Update the in-file caller duplicate_slide to destructure: let (new_name, _) = insert_slide(dir, position + 1, &slug)?;.
Update slide_insert in deck.rs to call let (new_name, slug_normalized) = insert_slide(...)?; and return Ok((SlideFile { name: new_name, ... }, slug_normalized)). Change signature to Result<(SlideFile, bool), HeroSlidesError>.
Update internal callers/tests in deck.rs to destructure: let (s1, _) = slide_insert(...)?;.
Update handle_slide_insert in rpc.rs to destructure the tuple and emit json!({ "new_name": slide.name, "inserted": true, "slug_normalized": slug_normalized }).

Step 2: Server — unit tests for strip behavior

Files: crates/hero_slides_lib/src/slide_ops.rs.
Dependencies: Step 1.
Tasks:

Add #[test] fn test_strip_leading_position_prefix_basic() covering: "06_conclusion" -> ("conclusion", true), "99_x" -> ("x", true), "conclusion" -> ("conclusion", false), "6_conclusion" -> ("6_conclusion", false) (single digit unchanged), "ab_conclusion" -> false, "06" -> false (no body), "06_" -> false (empty body).
Add #[test] fn test_insert_slide_strips_double_prefix(): build a 5-slide deck, call insert_slide(&dir, 6, "06_conclusion"), assert returned (name, normalized) is ("06_conclusion", true) and the resulting filename is 06_conclusion.md. Add a sibling test for clean slug returning false.

Step 3: Dashboard — `showPrompt` inline validation

Files: crates/hero_slides_ui/templates/index.html, crates/hero_slides_ui/static/js/dashboard.js.
Dependencies: none.
Tasks:

Add <div class="invalid-feedback" id="prompt-modal-error"></div> after #prompt-modal-input.
In showPrompt: at modal-open, clear is-invalid on the input and reset the error div text. In the submit handler, replace if (!val) return; with: set input.classList.add('is-invalid'), set the error div text to "This field is required.", focus the input, return without hiding. Re-bind on the cloned OK button per existing pattern. Add an input listener (also re-bound per show) that strips is-invalid and clears the error text on the next keystroke. The Enter-to-submit path (existing keydown handler on the input) goes through the same submit function so it gets the same validation.

Files: crates/hero_slides_ui/templates/index.html, crates/hero_slides_ui/static/js/dashboard.js.
Dependencies: Step 1 (response contains slug_normalized).
Tasks:

Add <div class="invalid-feedback" id="create-slide-title-error"></div> directly under #create-slide-title.
In doCreateSlide: replace the existing if (!slug) { toast(...); titleEl.focus(); return; } block with: set titleEl.classList.add('is-invalid'), set #create-slide-title-error text to "Please enter a slide title/slug.", focus, return. Add an input listener (wire-once on modal show) to clear the invalid state. Successful creation also clears it.
After await rpc('slide.insert', ...) succeeds, if the response has slug_normalized: true, call toast('Slug normalized: removed leading number prefix', 'warn').
Apply the same response handling to insertSlide (the position-aware variant) — capture the result and conditionally toast.

Step 5: Dashboard — failed-slide badge

Files: crates/hero_slides_ui/static/js/dashboard.js, crates/hero_slides_ui/static/css/dashboard.css.
Dependencies: none (uses already-shipped PR #40 payload).
Tasks:

Near _generatingSlides, add const _failedSlides = new Map(); plus markSlideFailed(name, err), clearSlideFailed(name), clearAllFailedSlides() helpers.
In slideBadge(s), insert at the top: if _failedSlides.has(s.name), return <span class="state-badge state-badge-error" title="${escapeHtml(msg)}">Error</span>. Order: spinner (regenerating) > error > pending > generated. The spinner-wins ordering is enforced because per-slide generate paths call clearSlideFailed(name) before adding to _generatingSlides.
Add .state-badge-error CSS rule using the dashboard's red palette.
In generateDeck, call clearAllFailedSlides() immediately before rpc('deck.generateAsync', ...).
In pollGenerateProgress, inside both status.done branches (partial-success and plain succeeded), iterate status.per_slide (when present) and markSlideFailed / clearSlideFailed accordingly before loadSlidesForDeck().
In each per-slide generate path: on adding to _generatingSlides, also clearSlideFailed(name). On per-slide success (poller sees has_png: true), clearSlideFailed(name). On per-slide failure (caught error from poller), markSlideFailed(name, message).

Step 6: Manual smoke test pass

Files: none.
Dependencies: Steps 1–5.
Tasks:

Load a deck with 5+ slides; type 06_conclusion in Insert Slide; verify file is 06_conclusion.md, an info toast announces the normalization, and the slide renders.
Trigger an empty-name submit on Insert Slide modal — modal stays open, input shows red border with "This field is required."
Trigger Create Slide with empty title — same.
Generate a deck where one slide intentionally fails (e.g. via the pre-block trick from issue #39 testing); after the poller completes, the failed slide card shows the red Error badge with hover-tooltip; regenerate that single slide and verify the badge transitions to spinner → Generated.

Acceptance Criteria

User typing slug 06_conclusion on a 5-slide deck creates file 06_conclusion.md (no double-prefix). Server response includes slug_normalized: true; clean slugs return slug_normalized: false.
After deck-gen completes with a partial failure, the dashboard renders the failed slide with a red error badge and the failure message accessible via the badge title= tooltip. A successful regenerate of that slide clears the error.
Empty-name submission in showPrompt shows an inline error under the input, applies is-invalid, keeps focus in the input, and keeps the modal open. Typing in the input clears the invalid state.
Empty-slug submission in Create Slide modal shows the same inline error treatment without dismissing the modal.
cargo test -p hero_slides_lib passes with two new test cases for strip_leading_position_prefix and insert_slide double-prefix handling.
No regressions: existing slide_ops.rs and deck.rs tests still pass after the tuple-return signature change.

Risks / Notes

Changing the return type of insert_slide (from Result<String, _> to Result<(String, bool), _>) and slide_insert (from Result<SlideFile, _> to Result<(SlideFile, bool), _>) is a breaking API change. The lib appears to be consumed inside this workspace only (the server and duplicate_slide); blast radius is contained. Confirm by greping slide_insert / insert_slide outside slide_ops.rs and deck.rs — if any other crate uses them (e.g. CLI, agent), update those callers in the same change.
The _failedSlides map is in-memory and lost on page reload. Stale failures are not persisted, which is correct: an error badge is a freshness signal, not durable state. After reload the slide reverts to Pending until the next gen run.
The state-badge-error class must exist in the CSS or the badge renders unstyled. Verify the actual CSS file path; if .state-badge-pending lives in an inline <style> block, add the new rule there.
For showPrompt, the existing okBtn.replaceWith(okBtn.cloneNode(true)) pattern strips listeners on every show. The reworked listener wiring must remain compatible: re-bind both click and the input event after the clone.
This branch is rebased on development AFTER PR #41 merged, so all the dashboard helpers (appendSlideCard, renderSlideCardHtml, slides-empty-no-deck/no-slides, showPrompt spinner) are present. Step 3 modifies the spinner-equipped showPrompt, not the original.

## Implementation Spec for Issue #37 ### Objective Fix the "Create Slide" double-prefix bug at three layers: stop the server from prepending `NN_` when the slug already carries one, surface failed-slide outcomes from `deck.generateAsync` in the dashboard with an explicit error badge, and replace the silent empty-name no-op in both `showPrompt` and the Create Slide modal with inline validation that keeps the modal open. ### Scope decisions - **Sub-bug 1 (server slug):** strip-and-flag, not reject. `insert_slide` detects a leading `^\d{2}_` in `slug` and removes it before computing the final filename. The boolean is bubbled up so the client can show a non-blocking notice. Rejection was considered and ruled out — `06_conclusion` is a plausible thing for a user to type from muscle memory; a hard error breaks existing UI flows that already lower-case + slugify on the client. - **Carrier of the flag:** add `slug_normalized: bool` to the `slide.insert` JSON-RPC response. Internally `insert_slide` returns `(String, bool)` and `slide_insert` returns `(SlideFile, bool)`. Tests cover the stripping logic at `slide_ops.rs` level. - **Sub-bug 2 (failed-slide badge):** consume `status.per_slide` from `deck.generateJobStatus` (already shipped in PR #40). When the poller sees `done`, build a `Map<slideName, errorMessage>` of `ok: false` entries, store it as `_failedSlides`, then `loadSlidesForDeck()` triggers the badge re-render. `slideBadge(s)` reads `_failedSlides` and emits a red `state-badge-error` with a `title=` tooltip showing the error. The map is cleared at the start of every `deck.generateAsync` run, and individual entries are cleared when a slide later succeeds. No new RPC, no extra state on the slide objects. - **Sub-bug 3 (empty input):** `showPrompt` and `doCreateSlide` show inline error text via a sibling `<div class="invalid-feedback d-block">` and add `is-invalid` to the input. They do not call `onConfirm`, do not hide the modal, and re-focus the input. Typing in the input clears the invalid state. - **Out of scope:** any redesign of the Create Slide modal, server-side reject mode, or making `slide.insert` accept arbitrary slug characters. Existing client-side slug normalization (`replace(/[^a-z0-9_]/g, '_')`) is left untouched — only the leading-numeric-prefix case is added. ### Files to Modify/Create - `crates/hero_slides_lib/src/slide_ops.rs` — extract `strip_leading_position_prefix(&str) -> (String, bool)` helper; change `insert_slide` to return `Result<(String, bool), HeroSlidesError>`; apply the strip before constructing `new_name`; update `duplicate_slide` caller. Add unit tests. - `crates/hero_slides_lib/src/deck.rs` — `slide_insert` propagates the bool. Change return type to `Result<(SlideFile, bool), HeroSlidesError>`. Update internal callers/tests. - `crates/hero_slides_server/src/rpc.rs` — `handle_slide_insert`: destructure the tuple; include `slug_normalized` in the JSON response. - `crates/hero_slides_ui/static/js/dashboard.js`: - `showPrompt`: clear-on-open invalid state, on empty input add `is-invalid`, set inline error text, focus input, return without hiding. Wire an `input` listener that clears the invalid state on the next keystroke. - `doCreateSlide`: replace `toast(...) + return` on empty slug with the same inline-error treatment. After successful `slide.insert`, if `result.slug_normalized === true`, show an info toast. - `insertSlide` (the in-place insert at a position): same response handling. - Add `_failedSlides = new Map()` near `_generatingSlides`. Helpers `markSlideFailed(name, err)`, `clearSlideFailed(name)`, `clearAllFailedSlides()`. - `slideBadge(s)`: when `_failedSlides.has(s.name)`, return `<span class="state-badge state-badge-error" title="..." >Error</span>` (priority below the generating spinner so retry shows spinner). - `pollGenerateProgress`: on `status.done` with `Array.isArray(status.per_slide)`, mark/clear per slide before `loadSlidesForDeck()`. Call `clearAllFailedSlides()` at the start of `generateDeck()`. - Per-slide generate paths: on success, `clearSlideFailed(name)`; on failure, `markSlideFailed(name, msg)`. - `crates/hero_slides_ui/templates/index.html` — add `<div class="invalid-feedback" id="prompt-modal-error"></div>` under `#prompt-modal-input`; add `<div class="invalid-feedback" id="create-slide-title-error"></div>` under `#create-slide-title`. - `crates/hero_slides_ui/static/css/dashboard.css` — add `.state-badge-error` (red fill, white text). Match the existing badge palette (verify the file by greping for `.state-badge-pending`). ### Implementation Plan #### Step 1: Server — slug strip-and-flag Files: `crates/hero_slides_lib/src/slide_ops.rs`, `crates/hero_slides_lib/src/deck.rs`, `crates/hero_slides_server/src/rpc.rs`. Dependencies: none. Tasks: - Add private helper `fn strip_leading_position_prefix(slug: &str) -> (String, bool)` near `parse_slide_stem`. Logic: if `slug.len() >= 3`, first two bytes are ASCII digits, third is `_`, and the rest is non-empty, return `(rest.to_string(), true)`; else `(slug.to_string(), false)`. ASCII-only — matches the existing `zero_pad2` convention. - In `insert_slide`, call the helper on the input slug before the existing `format!("{}_{}", zero_pad2(at), slug)`. Change return type to `Result<(String, bool), HeroSlidesError>`. - Update the in-file caller `duplicate_slide` to destructure: `let (new_name, _) = insert_slide(dir, position + 1, &slug)?;`. - Update `slide_insert` in `deck.rs` to call `let (new_name, slug_normalized) = insert_slide(...)?;` and return `Ok((SlideFile { name: new_name, ... }, slug_normalized))`. Change signature to `Result<(SlideFile, bool), HeroSlidesError>`. - Update internal callers/tests in `deck.rs` to destructure: `let (s1, _) = slide_insert(...)?;`. - Update `handle_slide_insert` in `rpc.rs` to destructure the tuple and emit `json!({ "new_name": slide.name, "inserted": true, "slug_normalized": slug_normalized })`. #### Step 2: Server — unit tests for strip behavior Files: `crates/hero_slides_lib/src/slide_ops.rs`. Dependencies: Step 1. Tasks: - Add `#[test] fn test_strip_leading_position_prefix_basic()` covering: `"06_conclusion" -> ("conclusion", true)`, `"99_x" -> ("x", true)`, `"conclusion" -> ("conclusion", false)`, `"6_conclusion" -> ("6_conclusion", false)` (single digit unchanged), `"ab_conclusion" -> false`, `"06" -> false` (no body), `"06_" -> false` (empty body). - Add `#[test] fn test_insert_slide_strips_double_prefix()`: build a 5-slide deck, call `insert_slide(&dir, 6, "06_conclusion")`, assert returned `(name, normalized)` is `("06_conclusion", true)` and the resulting filename is `06_conclusion.md`. Add a sibling test for clean slug returning `false`. #### Step 3: Dashboard — `showPrompt` inline validation Files: `crates/hero_slides_ui/templates/index.html`, `crates/hero_slides_ui/static/js/dashboard.js`. Dependencies: none. Tasks: - Add `<div class="invalid-feedback" id="prompt-modal-error"></div>` after `#prompt-modal-input`. - In `showPrompt`: at modal-open, clear `is-invalid` on the input and reset the error div text. In the `submit` handler, replace `if (!val) return;` with: set `input.classList.add('is-invalid')`, set the error div text to "This field is required.", focus the input, return without hiding. Re-bind on the cloned OK button per existing pattern. Add an `input` listener (also re-bound per show) that strips `is-invalid` and clears the error text on the next keystroke. The Enter-to-submit path (existing keydown handler on the input) goes through the same `submit` function so it gets the same validation. #### Step 4: Dashboard — Create Slide modal inline validation + slug-normalized toast Files: `crates/hero_slides_ui/templates/index.html`, `crates/hero_slides_ui/static/js/dashboard.js`. Dependencies: Step 1 (response contains `slug_normalized`). Tasks: - Add `<div class="invalid-feedback" id="create-slide-title-error"></div>` directly under `#create-slide-title`. - In `doCreateSlide`: replace the existing `if (!slug) { toast(...); titleEl.focus(); return; }` block with: set `titleEl.classList.add('is-invalid')`, set `#create-slide-title-error` text to "Please enter a slide title/slug.", focus, return. Add an input listener (wire-once on modal show) to clear the invalid state. Successful creation also clears it. - After `await rpc('slide.insert', ...)` succeeds, if the response has `slug_normalized: true`, call `toast('Slug normalized: removed leading number prefix', 'warn')`. - Apply the same response handling to `insertSlide` (the position-aware variant) — capture the result and conditionally toast. #### Step 5: Dashboard — failed-slide badge Files: `crates/hero_slides_ui/static/js/dashboard.js`, `crates/hero_slides_ui/static/css/dashboard.css`. Dependencies: none (uses already-shipped PR #40 payload). Tasks: - Near `_generatingSlides`, add `const _failedSlides = new Map();` plus `markSlideFailed(name, err)`, `clearSlideFailed(name)`, `clearAllFailedSlides()` helpers. - In `slideBadge(s)`, insert at the top: if `_failedSlides.has(s.name)`, return `<span class="state-badge state-badge-error" title="${escapeHtml(msg)}">Error</span>`. Order: spinner (regenerating) > error > pending > generated. The spinner-wins ordering is enforced because per-slide generate paths call `clearSlideFailed(name)` before adding to `_generatingSlides`. - Add `.state-badge-error` CSS rule using the dashboard's red palette. - In `generateDeck`, call `clearAllFailedSlides()` immediately before `rpc('deck.generateAsync', ...)`. - In `pollGenerateProgress`, inside both `status.done` branches (partial-success and plain succeeded), iterate `status.per_slide` (when present) and `markSlideFailed` / `clearSlideFailed` accordingly *before* `loadSlidesForDeck()`. - In each per-slide generate path: on adding to `_generatingSlides`, also `clearSlideFailed(name)`. On per-slide success (poller sees `has_png: true`), `clearSlideFailed(name)`. On per-slide failure (caught error from poller), `markSlideFailed(name, message)`. #### Step 6: Manual smoke test pass Files: none. Dependencies: Steps 1–5. Tasks: - Load a deck with 5+ slides; type `06_conclusion` in Insert Slide; verify file is `06_conclusion.md`, an info toast announces the normalization, and the slide renders. - Trigger an empty-name submit on Insert Slide modal — modal stays open, input shows red border with "This field is required." - Trigger Create Slide with empty title — same. - Generate a deck where one slide intentionally fails (e.g. via the pre-block trick from issue #39 testing); after the poller completes, the failed slide card shows the red Error badge with hover-tooltip; regenerate that single slide and verify the badge transitions to spinner → Generated. ### Acceptance Criteria - [ ] User typing slug `06_conclusion` on a 5-slide deck creates file `06_conclusion.md` (no double-prefix). Server response includes `slug_normalized: true`; clean slugs return `slug_normalized: false`. - [ ] After deck-gen completes with a partial failure, the dashboard renders the failed slide with a red error badge and the failure message accessible via the badge `title=` tooltip. A successful regenerate of that slide clears the error. - [ ] Empty-name submission in `showPrompt` shows an inline error under the input, applies `is-invalid`, keeps focus in the input, and keeps the modal open. Typing in the input clears the invalid state. - [ ] Empty-slug submission in Create Slide modal shows the same inline error treatment without dismissing the modal. - [ ] `cargo test -p hero_slides_lib` passes with two new test cases for `strip_leading_position_prefix` and `insert_slide` double-prefix handling. - [ ] No regressions: existing `slide_ops.rs` and `deck.rs` tests still pass after the tuple-return signature change. ### Risks / Notes - Changing the return type of `insert_slide` (from `Result<String, _>` to `Result<(String, bool), _>`) and `slide_insert` (from `Result<SlideFile, _>` to `Result<(SlideFile, bool), _>`) is a breaking API change. The lib appears to be consumed inside this workspace only (the server and `duplicate_slide`); blast radius is contained. Confirm by greping `slide_insert` / `insert_slide` outside `slide_ops.rs` and `deck.rs` — if any other crate uses them (e.g. CLI, agent), update those callers in the same change. - The `_failedSlides` map is in-memory and lost on page reload. Stale failures are not persisted, which is correct: an error badge is a freshness signal, not durable state. After reload the slide reverts to Pending until the next gen run. - The `state-badge-error` class must exist in the CSS or the badge renders unstyled. Verify the actual CSS file path; if `.state-badge-pending` lives in an inline `<style>` block, add the new rule there. - For `showPrompt`, the existing `okBtn.replaceWith(okBtn.cloneNode(true))` pattern strips listeners on every show. The reworked listener wiring must remain compatible: re-bind both click and the input event after the clone. - This branch is rebased on `development` AFTER PR #41 merged, so all the dashboard helpers (`appendSlideCard`, `renderSlideCardHtml`, `slides-empty-no-deck/no-slides`, `showPrompt` spinner) are present. Step 3 modifies the spinner-equipped `showPrompt`, not the original.

salmaelsoly commented

2026-05-03 05:47:06 +00:00

Author

Member

Test Results

Check	Result
`cargo test --workspace`	105 passed / 0 failed / 1 ignored (the AI-gated `test_generate_single_slide_ai`). 7 new tests added in `slide_ops.rs` for `strip_leading_position_prefix` and `insert_slide` double-prefix handling.
`cargo fmt --check`	clean (after `cargo fmt`)
`cargo clippy --workspace --no-deps`	only the 2 pre-existing `unnecessary_sort_by` warnings in `slide_ops.rs:219` and `:448` (unchanged from `development`)
`bun build --no-bundle dashboard.js`	parses cleanly

New automated tests (Step 2)

test_strip_leading_position_prefix_strips_double_digit_prefix — 06_conclusion, 99_x, 00_intro all stripped, flag true.
test_strip_leading_position_prefix_leaves_clean_slug_untouched — conclusion, my_slide_name unchanged, flag false.
test_strip_leading_position_prefix_does_not_strip_single_digit — 6_conclusion unchanged.
test_strip_leading_position_prefix_does_not_strip_letters — ab_conclusion unchanged.
test_strip_leading_position_prefix_handles_no_body — 06_ and 06 unchanged (no body to keep).
test_insert_slide_strips_double_prefix — full insert_slide(&dir, 6, "06_conclusion") returns ("06_conclusion", true); file 06_conclusion.md exists.
test_insert_slide_clean_slug_returns_false — clean slug summary returns flag false.

Diffstat

 crates/hero_slides_lib/src/deck.rs           |  37 +++++----
 crates/hero_slides_lib/src/slide_ops.rs      | 111 ++++++++++++++++++++++++++-
 crates/hero_slides_rhai/src/slide_module.rs  |   6 +-
 crates/hero_slides_server/src/rpc.rs         |   3 +-
 crates/hero_slides_ui/static/js/dashboard.js | 107 ++++++++++++++++++++++++--
 crates/hero_slides_ui/templates/index.html   |   2 +
 6 files changed, 237 insertions(+), 29 deletions(-)

Manual smoke needed before merge

Type slug 06_conclusion on a 5-slide deck via Insert Slide. Expect: file 06_conclusion.md (no double prefix); a Slug normalized: removed leading number prefix toast.
Empty-name on showPrompt-driven modals (Create Deck, Rename, Duplicate, Insert Slide, Move Slide, Folder, Subfolder): inline error appears, modal stays open, typing clears the error.
Empty title on Create Slide modal: same.
Generate a deck where one slide fails (e.g. pre-block via mkdir -p output/02_x.png per #39 testing). Verify the failed slide shows a red Error badge with the failure message in the title= tooltip; regenerating that slide clears the badge.

## Test Results | Check | Result | |---|---| | `cargo test --workspace` | **105 passed / 0 failed / 1 ignored** (the AI-gated `test_generate_single_slide_ai`). 7 new tests added in `slide_ops.rs` for `strip_leading_position_prefix` and `insert_slide` double-prefix handling. | | `cargo fmt --check` | clean (after `cargo fmt`) | | `cargo clippy --workspace --no-deps` | only the 2 pre-existing `unnecessary_sort_by` warnings in `slide_ops.rs:219` and `:448` (unchanged from `development`) | | `bun build --no-bundle dashboard.js` | parses cleanly | ### New automated tests (Step 2) - `test_strip_leading_position_prefix_strips_double_digit_prefix` — `06_conclusion`, `99_x`, `00_intro` all stripped, flag `true`. - `test_strip_leading_position_prefix_leaves_clean_slug_untouched` — `conclusion`, `my_slide_name` unchanged, flag `false`. - `test_strip_leading_position_prefix_does_not_strip_single_digit` — `6_conclusion` unchanged. - `test_strip_leading_position_prefix_does_not_strip_letters` — `ab_conclusion` unchanged. - `test_strip_leading_position_prefix_handles_no_body` — `06_` and `06` unchanged (no body to keep). - `test_insert_slide_strips_double_prefix` — full `insert_slide(&dir, 6, "06_conclusion")` returns `("06_conclusion", true)`; file `06_conclusion.md` exists. - `test_insert_slide_clean_slug_returns_false` — clean slug `summary` returns flag `false`. ### Diffstat ``` crates/hero_slides_lib/src/deck.rs | 37 +++++---- crates/hero_slides_lib/src/slide_ops.rs | 111 ++++++++++++++++++++++++++- crates/hero_slides_rhai/src/slide_module.rs | 6 +- crates/hero_slides_server/src/rpc.rs | 3 +- crates/hero_slides_ui/static/js/dashboard.js | 107 ++++++++++++++++++++++++-- crates/hero_slides_ui/templates/index.html | 2 + 6 files changed, 237 insertions(+), 29 deletions(-) ``` ### Manual smoke needed before merge - Type slug `06_conclusion` on a 5-slide deck via Insert Slide. Expect: file `06_conclusion.md` (no double prefix); a `Slug normalized: removed leading number prefix` toast. - Empty-name on `showPrompt`-driven modals (Create Deck, Rename, Duplicate, Insert Slide, Move Slide, Folder, Subfolder): inline error appears, modal stays open, typing clears the error. - Empty title on Create Slide modal: same. - Generate a deck where one slide fails (e.g. pre-block via `mkdir -p output/02_x.png` per #39 testing). Verify the failed slide shows a red `Error` badge with the failure message in the `title=` tooltip; regenerating that slide clears the badge.

salmaelsoly commented

2026-05-03 05:48:14 +00:00

Author

Member

Implementation Summary

All 5 implementation steps complete on branch development_slug_double_prefix (rebased on latest development post-merge of PRs #40 and #41).

Files modified

File	Change
`crates/hero_slides_lib/src/slide_ops.rs`	New `strip_leading_position_prefix(&str) -> (String, bool)` helper. `insert_slide` now strips a leading `NN_` from the user's slug before prepending the position prefix, and returns `(String, bool)`. 7 new unit tests covering the strip logic and the full `insert_slide` flow.
`crates/hero_slides_lib/src/deck.rs`	`slide_insert` propagates the new `slug_normalized` bool: signature changed to `Result<(SlideFile, bool), HeroSlidesError>`. Internal callers (`slide_copy_to_deck` plus 4 in-file tests) updated to destructure.
`crates/hero_slides_rhai/src/slide_module.rs`	Rhai binding for `slide_insert` destructures the tuple and exposes `slug_normalized` in the returned `Map`.
`crates/hero_slides_server/src/rpc.rs`	`handle_slide_insert` includes `slug_normalized` in the JSON response alongside `new_name` and `inserted`.
`crates/hero_slides_ui/static/js/dashboard.js`	`showPrompt`: clones the input on each open (drops stale listeners), wires per-keystroke clear-invalid handler, replaces silent empty-input bail with inline `is-invalid` + error message; the existing PR #41 spinner phase is left intact. `doCreateSlide` and `insertSlide`: capture `slide.insert` response; show a warn toast `"Slug normalized: removed leading number prefix"` when `slug_normalized: true`. New `_failedSlides` Map and helpers; `slideBadge` renders a red `Error` badge with tooltip when an entry exists. `generateDeck` clears all failed entries before each run. `pollGenerateProgress` (both partial-success and plain branches) reads `status.per_slide` and marks/clears per slide. Per-slide generate paths (`generateSlide`, `pollSlideGenerateProgress`, `generateAllSlides`) clear on retry/success and mark on failure.
`crates/hero_slides_ui/templates/index.html`	Added `<div class="invalid-feedback" id="prompt-modal-error"></div>` under `#prompt-modal-input` and `<div class="invalid-feedback" id="create-slide-title-error"></div>` under `#create-slide-title`.

crates/hero_slides_ui/static/css/dashboard.css was not modified — .state-badge-error already exists at line 157 with a red palette matching the dashboard theme.

Behavior

Action	Before	After
Insert Slide with slug `06_conclusion` on a 5-slide deck	File saved as `06_06_conclusion.md`; later generation fails silently with a "Pending" badge	File saved as `06_conclusion.md`. Warn toast: "Slug normalized: removed leading number prefix". Generation works.
Empty-name submit on `showPrompt` (Create Deck, Rename, Duplicate, Insert Slide, Move Slide, Folder, Subfolder)	Modal stayed open silently with no feedback	Inline red error "This field is required." under the input. Modal stays open. Typing clears the error.
Empty-title submit on Create Slide modal	Toast appeared but modal stayed open	Inline red error "Please enter a slide title or slug." under the title input. Modal stays open.
Slide whose generation failed during deck-gen	Indistinguishable from "Pending" — only Jobs tab revealed it	Red `Error` badge with the failure message in `title=` tooltip. Regenerating clears it.

Acceptance criteria

User typing slug 06_conclusion on a 5-slide deck creates file 06_conclusion.md (no double-prefix). Server response includes slug_normalized: true; clean slugs return slug_normalized: false.
After deck-gen completes with a partial failure, the dashboard renders the failed slide with a red error badge and the failure message accessible via the badge title= tooltip. Regenerating clears the error.
Empty-name submission in showPrompt shows an inline error, applies is-invalid, keeps focus in the input, keeps the modal open. Typing clears the invalid state.
Empty-slug submission in Create Slide modal shows the same inline-error treatment without dismissing the modal.
cargo test -p hero_slides_lib passes with 7 new test cases.
No regressions: existing tests still pass after the tuple-return signature change. cargo test --workspace: 105 passed / 0 failed / 1 ignored.

Notes for review

The lib API change (tuple return on insert_slide and slide_insert) is a breaking change for any downstream consumer. In-tree callers updated: duplicate_slide, slide_copy_to_deck, handle_slide_insert, the Rhai binding, and 4 in-file tests. Outside this workspace none were found.
_failedSlides is in-memory and lost on page reload. After reload a slide reverts to "Pending" until the next generation pass — acceptable for an ephemeral status indicator.
The Rhai binding now also returns slug_normalized in the result Map. Existing Rhai scripts that read name, path, slug, number, hidden continue to work; the new key is additive.
The OK button in showPrompt lost its { once: true } flag. Without that change, a validation-failure submit would permanently disable OK on that modal open. The cloneNode-on-show in PR #41 already covers per-invocation listener cleanup, so removing the flag is safe.
.state-badge-error was already present in dashboard.css:157, so no CSS edits were needed.
Static assets are embedded via rust_embed. After deploy users may need a hard refresh, and the hero_slides_ui binary needs rebuilding.

## Implementation Summary All 5 implementation steps complete on branch `development_slug_double_prefix` (rebased on latest `development` post-merge of PRs #40 and #41). ### Files modified | File | Change | |---|---| | `crates/hero_slides_lib/src/slide_ops.rs` | New `strip_leading_position_prefix(&str) -> (String, bool)` helper. `insert_slide` now strips a leading `NN_` from the user's slug before prepending the position prefix, and returns `(String, bool)`. 7 new unit tests covering the strip logic and the full `insert_slide` flow. | | `crates/hero_slides_lib/src/deck.rs` | `slide_insert` propagates the new `slug_normalized` bool: signature changed to `Result<(SlideFile, bool), HeroSlidesError>`. Internal callers (`slide_copy_to_deck` plus 4 in-file tests) updated to destructure. | | `crates/hero_slides_rhai/src/slide_module.rs` | Rhai binding for `slide_insert` destructures the tuple and exposes `slug_normalized` in the returned `Map`. | | `crates/hero_slides_server/src/rpc.rs` | `handle_slide_insert` includes `slug_normalized` in the JSON response alongside `new_name` and `inserted`. | | `crates/hero_slides_ui/static/js/dashboard.js` | `showPrompt`: clones the input on each open (drops stale listeners), wires per-keystroke clear-invalid handler, replaces silent empty-input bail with inline `is-invalid` + error message; the existing PR #41 spinner phase is left intact. `doCreateSlide` and `insertSlide`: capture `slide.insert` response; show a warn toast `"Slug normalized: removed leading number prefix"` when `slug_normalized: true`. New `_failedSlides` Map and helpers; `slideBadge` renders a red `Error` badge with tooltip when an entry exists. `generateDeck` clears all failed entries before each run. `pollGenerateProgress` (both partial-success and plain branches) reads `status.per_slide` and marks/clears per slide. Per-slide generate paths (`generateSlide`, `pollSlideGenerateProgress`, `generateAllSlides`) clear on retry/success and mark on failure. | | `crates/hero_slides_ui/templates/index.html` | Added `<div class="invalid-feedback" id="prompt-modal-error"></div>` under `#prompt-modal-input` and `<div class="invalid-feedback" id="create-slide-title-error"></div>` under `#create-slide-title`. | `crates/hero_slides_ui/static/css/dashboard.css` was not modified — `.state-badge-error` already exists at line 157 with a red palette matching the dashboard theme. ### Behavior | Action | Before | After | |---|---|---| | Insert Slide with slug `06_conclusion` on a 5-slide deck | File saved as `06_06_conclusion.md`; later generation fails silently with a "Pending" badge | File saved as `06_conclusion.md`. Warn toast: "Slug normalized: removed leading number prefix". Generation works. | | Empty-name submit on `showPrompt` (Create Deck, Rename, Duplicate, Insert Slide, Move Slide, Folder, Subfolder) | Modal stayed open silently with no feedback | Inline red error "This field is required." under the input. Modal stays open. Typing clears the error. | | Empty-title submit on Create Slide modal | Toast appeared but modal stayed open | Inline red error "Please enter a slide title or slug." under the title input. Modal stays open. | | Slide whose generation failed during deck-gen | Indistinguishable from "Pending" — only Jobs tab revealed it | Red `Error` badge with the failure message in `title=` tooltip. Regenerating clears it. | ### Acceptance criteria - [x] User typing slug `06_conclusion` on a 5-slide deck creates file `06_conclusion.md` (no double-prefix). Server response includes `slug_normalized: true`; clean slugs return `slug_normalized: false`. - [x] After deck-gen completes with a partial failure, the dashboard renders the failed slide with a red error badge and the failure message accessible via the badge `title=` tooltip. Regenerating clears the error. - [x] Empty-name submission in `showPrompt` shows an inline error, applies `is-invalid`, keeps focus in the input, keeps the modal open. Typing clears the invalid state. - [x] Empty-slug submission in Create Slide modal shows the same inline-error treatment without dismissing the modal. - [x] `cargo test -p hero_slides_lib` passes with 7 new test cases. - [x] No regressions: existing tests still pass after the tuple-return signature change. `cargo test --workspace`: 105 passed / 0 failed / 1 ignored. ### Notes for review - The lib API change (tuple return on `insert_slide` and `slide_insert`) is a breaking change for any downstream consumer. In-tree callers updated: `duplicate_slide`, `slide_copy_to_deck`, `handle_slide_insert`, the Rhai binding, and 4 in-file tests. Outside this workspace none were found. - `_failedSlides` is in-memory and lost on page reload. After reload a slide reverts to "Pending" until the next generation pass — acceptable for an ephemeral status indicator. - The Rhai binding now also returns `slug_normalized` in the result Map. Existing Rhai scripts that read `name`, `path`, `slug`, `number`, `hidden` continue to work; the new key is additive. - The OK button in `showPrompt` lost its `{ once: true }` flag. Without that change, a validation-failure submit would permanently disable OK on that modal open. The cloneNode-on-show in PR #41 already covers per-invocation listener cleanup, so removing the flag is safe. - `.state-badge-error` was already present in `dashboard.css:157`, so no CSS edits were needed. - Static assets are embedded via `rust_embed`. After deploy users may need a hard refresh, and the `hero_slides_ui` binary needs rebuilding.

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

lhumina_code/hero_slides#37

No description provided.

Rows
Columns

Create Slide: slug double-prefixed by auto-numbering, then fails silently #37

Summary

Reproduction

Root causes

1. Server unconditionally prepends NN_ to the slug (the actual file-save bug)

2. Pending-state badge is rendered unconditionally

3. Empty-name submission silently keeps modal open

Suggested fix

Severity

Implementation Spec for Issue #37

Objective

Scope decisions

Files to Modify/Create

Implementation Plan

Step 1: Server — slug strip-and-flag

Step 2: Server — unit tests for strip behavior

Step 3: Dashboard — showPrompt inline validation

Step 4: Dashboard — Create Slide modal inline validation + slug-normalized toast

Step 5: Dashboard — failed-slide badge

Step 6: Manual smoke test pass

Acceptance Criteria

Risks / Notes

Test Results

New automated tests (Step 2)

Diffstat

Manual smoke needed before merge

Implementation Summary

Files modified

Behavior

Acceptance criteria

Notes for review

1. Server unconditionally prepends `NN_` to the slug (the actual file-save bug)

Step 3: Dashboard — `showPrompt` inline validation