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

Merged

salmaelsoly merged 3 commits from development_partial_job_success into development 2026-04-30 07:54:18 +00:00

Conversation 0 Commits 3 Files changed 5 +220 -14

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

Member

Copy link

Closes #39.

Summary

Stops reporting deck-generation jobs as failed when most slides succeeded and the PDF was produced. The Rhai child script now exits 0 on partial success; the server merges a per-run manifest into the job-status response so callers can tell partial-success apart from clean success and from catastrophic failure.

Behavior matrix

Run outcome	phase	exit_code	partial_success
All slides + PDF succeed	succeeded	0	false
Some slides fail; PDF still built (or some slides built)	succeeded	0	true
All slides fail AND PDF fails	failed	non-zero	false

Changes

• generate_deck.rhai — throws only on catastrophic failure (generated == 0 && !pdf_created). Per-slide errors are always printed for diagnosability.
• hero_slides_lib::deck::deck_generate — populates a per-slide outcome list and writes <deck>/output/.last_generate.json (best-effort; never propagates IO errors).
• hero_slides_lib::types — adds SlideOutcome and GenerateResult.per_slide with #[serde(default)] so older deserializations still work.
• hero_slides_server::generate_job::GenerateJob — gains optional manifest_path. status() reads the manifest on terminal phase and merges total / generated / skipped / failed / has_pdf / errors / per_slide / partial_success into the response. Other submitters (slide async, pdf async, agent jobs) keep manifest_path: None and produce the unchanged base shape.
• dashboard.js — pollGenerateProgress adds a partial-success branch (warning toast, error log lines, 4s modal hold) BEFORE the clean-success branch. Failure path also iterates status.errors.

Test plan

• Smoke: deck where one slide fails (e.g. corrupt the slide file). Expect: phase succeeded, exit_code 0, partial_success: true, failed: 1, has_pdf: true (or false if PDF blocked), warning toast in the dashboard.
• Smoke: deck where ALL slides fail. Expect: phase failed, non-zero exit, partial_success: false, error toast.
• Smoke: clean deck. Expect: phase succeeded, partial_success: false, success toast.
• Confirm cargo test --workspace is green (97 passing, 1 ignored).
• Confirm legacy callers reading only phase / done / exit_code still work (other job types unchanged).

Backward compatibility

All new status fields are additive. GenerateResult gains a field with #[serde(default)]. Older callers that don't parse the new fields keep working. Slide-async and pdf-async job statuses are unchanged.

Notes

• Empty deck (total == 0) hits the catastrophic branch (generated == 0 && !pdf_created). That seems right — generating an empty deck is a no-op error — but flagging in case reviewers prefer otherwise.
• Pre-existing clippy warnings in crates/hero_slides_lib/src/slide_ops.rs:219 and :448 (unrelated unnecessary_sort_by) were not touched. They're already on development and pass CI there.
• Static assets: dashboard.js is a static file. Users may need a hard refresh after deploy to pick up the partial-state branch.

Follow-up (not this PR)

• A dashboard improvement to render the per-slide failure list inline in the deck view rather than only in the toast/log (BUG #11 from the recent dashboard review). Already filed as #37.

Closes #39. ## Summary Stops reporting deck-generation jobs as `failed` when most slides succeeded and the PDF was produced. The Rhai child script now exits 0 on partial success; the server merges a per-run manifest into the job-status response so callers can tell partial-success apart from clean success and from catastrophic failure. ## Behavior matrix | Run outcome | `phase` | `exit_code` | `partial_success` | |---|---|---|---| | All slides + PDF succeed | `succeeded` | 0 | `false` | | Some slides fail; PDF still built (or some slides built) | `succeeded` | 0 | `true` | | All slides fail AND PDF fails | `failed` | non-zero | `false` | ## Changes - **`generate_deck.rhai`** — throws only on catastrophic failure (`generated == 0 && !pdf_created`). Per-slide errors are always printed for diagnosability. - **`hero_slides_lib::deck::deck_generate`** — populates a per-slide outcome list and writes `<deck>/output/.last_generate.json` (best-effort; never propagates IO errors). - **`hero_slides_lib::types`** — adds `SlideOutcome` and `GenerateResult.per_slide` with `#[serde(default)]` so older deserializations still work. - **`hero_slides_server::generate_job::GenerateJob`** — gains optional `manifest_path`. `status()` reads the manifest on terminal phase and merges `total / generated / skipped / failed / has_pdf / errors / per_slide / partial_success` into the response. Other submitters (slide async, pdf async, agent jobs) keep `manifest_path: None` and produce the unchanged base shape. - **`dashboard.js`** — `pollGenerateProgress` adds a partial-success branch (warning toast, error log lines, 4s modal hold) BEFORE the clean-success branch. Failure path also iterates `status.errors`. ## Test plan - [x] Smoke: deck where one slide fails (e.g. corrupt the slide file). Expect: phase `succeeded`, `exit_code` 0, `partial_success: true`, `failed: 1`, `has_pdf: true` (or `false` if PDF blocked), warning toast in the dashboard. - [x] Smoke: deck where ALL slides fail. Expect: phase `failed`, non-zero exit, `partial_success: false`, error toast. - [x] Smoke: clean deck. Expect: phase `succeeded`, `partial_success: false`, success toast. - [x] Confirm `cargo test --workspace` is green (97 passing, 1 ignored). - [x] Confirm legacy callers reading only `phase` / `done` / `exit_code` still work (other job types unchanged). ## Backward compatibility All new status fields are additive. `GenerateResult` gains a field with `#[serde(default)]`. Older callers that don't parse the new fields keep working. Slide-async and pdf-async job statuses are unchanged. ## Notes - **Empty deck** (`total == 0`) hits the catastrophic branch (`generated == 0 && !pdf_created`). That seems right — generating an empty deck is a no-op error — but flagging in case reviewers prefer otherwise. - **Pre-existing clippy warnings** in `crates/hero_slides_lib/src/slide_ops.rs:219` and `:448` (unrelated `unnecessary_sort_by`) were not touched. They're already on `development` and pass CI there. - **Static assets**: `dashboard.js` is a static file. Users may need a hard refresh after deploy to pick up the partial-state branch. ## Follow-up (not this PR) - A dashboard improvement to render the per-slide failure list inline in the deck view rather than only in the toast/log (BUG #11 from the recent dashboard review). Already filed as #37.

salmaelsoly added 1 commit 2026-04-29 13:10:20 +00:00

fix(generate): treat partial deck-gen as success and surface per-slide outcomes ... 0055daa591

- generate_deck.rhai throws only on catastrophic failure (0 slides + no PDF).
  Partial failures now exit 0; hero_proc reports phase=succeeded as it should.
- deck_generate writes <deck>/output/.last_generate.json with per-slide outcomes,
  counts, and pdf_created so the server can read run results back after the
  child exits.
- GenerateJob gains an optional manifest_path; status() reads the manifest on
  terminal phase and merges total/generated/skipped/failed/has_pdf/errors
  (capped 50) /per_slide/partial_success into the response. Slide-async and
  pdf-async submitters keep manifest_path=None (no shape drift).
- dashboard.js renders the partial state distinctly: warning toast, per-slide
  error lines from status.errors, 4s modal hold. Failure path also dumps
  status.errors so users see which slide blew up.
- types.rs gains SlideOutcome { name, ok, error } and GenerateResult.per_slide
  with #[serde(default)] for back-compat.

#39

salmaelsoly added 1 commit 2026-04-30 06:10:21 +00:00

fix(generate): no-op deck-gen runs should not be reported as failed ... c1b8b3fbf3

The catastrophic predicate `generated == 0 && !pdf_created` fired when a deck
was re-generated with everything already up-to-date — `generated=0` because
nothing new was produced, `pdf_created=false` because the PDF rebuild was
skipped. The script then threw and hero_proc recorded `phase: failed` for a
benign no-op.

Tighten the predicate to `failed > 0 && generated == 0 && !pdf_created`, so
catastrophic only fires when at least one slide was *attempted* and none
succeeded. No-op runs (everything skipped) return cleanly.

Found during smoke test of #39.

#39

salmaelsoly added 1 commit 2026-04-30 07:43:53 +00:00

fix(generate): write manifest on no-op and empty-deck early-return paths 1317257869

salmaelsoly merged commit 641176179a into development 2026-04-30 07:54:18 +00:00

salmaelsoly referenced this pull request from a commit 2026-04-30 07:54:19 +00:00

Merge pull request 'fix(generate): treat partial deck-gen as success and surface per-slide outcomes' (#40) from development_partial_job_success into development

salmaelsoly deleted branch development_partial_job_success 2026-04-30 07:54:23 +00:00

salmaelsoly referenced this pull request 2026-04-30 12:15:10 +00:00

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

salmaelsoly referenced this pull request 2026-05-03 05:48:14 +00:00

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

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!40

No description provided.