Skip to content

Retain operation request errors in shared progress notices - #159

Merged
den-sq merged 1 commit into
codex/156-initialization-presentationfrom
codex/157-error-presentation
Oct 19, 2026
Merged

den-sq merged 1 commit into
codex/156-initialization-presentationfrom
codex/157-error-presentation

Conversation

@den-sq

@den-sq den-sq commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Task-start and previous-task deletion errors on slicing/backprojection now stay in the progress panel with a Dismiss button. Dismissal clears the request error at its existing owner, so it does not reappear on navigation or clear the reported task outcome. Both pages reuse the same notice presentation and styling.

Fixes #157. Stacked on #158 (codex/156-initialization-presentation); merge #158 first. This diff contains only the notice work. The presentation reference established there is extended with the notice rules used by the subsequent autoseg PRs.

Reviewer clarifications

  • Only /slice/, /backproject/, and previous-task deletion failures move into retained notices. Form/input validation, filesystem, and rendering alerts keep their existing locations and lifetimes.
  • Dismiss clears only the canonical request error. A new submission clears its earlier start/delete notices; successful requests already clear their own errors. Input editing and unrelated requests do not clear them.
  • Undismissed errors retain the existing ServerContext lifetime. Dismissed errors stay cleared on remount; there is no page-local dismissal flag or new persistence.
  • Task outcomes, steps, statistics messages, and the existing last-reported/communication distinction remain intact.

Shared-basis audit

  • ServerContext owns request results, errors, and pending state. RequestNotices reads that owner directly. The three mirrored states in useFetchListener and the page effects copying request errors to global toasts were removed. clearFetchError extends the existing update facility and preserves results/pending; it neither aborts requests nor modifies streams.
  • Previous-task deletion uses its actual task-specific URL as the existing request-map key, derived from the saved task response. This replaces the shared /delete/ error slot for these two consumers and prevents one workflow showing another's deletion error. HTTP operation and task identity are unchanged; the existing provider still owns request-snapshot lifetime.
  • Alert owns notice severity, layout, and optional dismissal. Existing taskStatusMessages remains the source of task/statistics wording. The same Alert styling also applies to existing global toasts, without changing their behavior or introducing another notice manager.
  • The Unify backend initialization presentation and define status vocabulary #158 status document remains authoritative for the display vocabulary; no backend states, readiness rules, observers, retries, or job lifecycle events were added.

Validation

  • 147 release tests passed; both TypeScript checks, changed-source ESLint, formatting, and diff checks passed.
  • Focused tests exercise the actual page submission/delete callbacks and notice rendering using the existing fixture.
  • Real Electron interaction check with ServerProvider passed: display, dismissal at the owner, preserved task result/outcome, remount, a new identical error, and success clearing. Normal/narrow layout review found no overflow or browser warnings.

@tavateva tavateva left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE at 28999440891a91900f3a3703bb14614b506d74b6. Reviewed as the diff on top of #158 (23de4d2).

This resolves all three points I raised on #157, and the body's "Reviewer clarifications" now state the rules the issue left open:

  • Which errors move. Only /slice/, /backproject/ and previous-task deletion failures become retained notices. Form/input, filesystem and rendering alerts keep their global toasts, and the page effects that copied request errors into toasts are removed, so nothing shows twice.
  • Clearing rule. A new submission clears its earlier start and delete errors; dismissal clears only that request's error; success clears as before. The rule is documented in status-presentation.md.
  • Navigation. Dismissal calls clearFetchError on the ServerContext owner. That is a field-level update, so it keeps results/pending and leaves streams alone, and a dismissed error can't reappear on remount. No page-local flag or persistence is added.

I checked the two non-obvious changes against the code:

  • Per-task delete key. performFetch('/delete/?task_id=…', {}, POST) builds the same request as before. getFullURL appends ? only for a non-empty query, and the /delete/ route takes task_id as a query parameter. The id is now encodeURIComponent-escaped, and each workflow gets its own request-map key, which removes the shared /delete/ error slot.
  • useFetchListener returning fetchStates.get(url) ?? emptyFetchResult(). This is behaviour-preserving. clearFetch writes an empty entry rather than deleting one, so the old mirrored state saw the same values, one render later. Dropping the mirror also removes that one-render lag.

The Alert restyle also applies to existing global toasts. That is declared in the body and only changes presentation: severity accent, role="alert" for errors and role="status" otherwise, wrapping and an optional Dismiss button.

Verification at the head:

  • npm ci --include=dev then npm run test:release: 147 passed, 0 failed, including the new submission/deletion notice tests driven through the real page callbacks.
  • npm run typecheck: clean.
  • ESLint on the changed sources: clean.
  • docs is green. Validate release inputs had every step completed successfully, but its job status had not yet flipped to complete when I posted. The full workflow set runs only against main, so it will run after retargeting.

Merge note. Because this is stacked, Fixes #157 won't link until the PR targets main. After #158 merges, retarget this PR to main before deleting codex/156-initialization-presentation, so it isn't closed with its base.

@den-sq
den-sq added this pull request to stack #160 October 19, 2026 16:21
@den-sq
den-sq merged commit 50cd59d into main Oct 19, 2026
10 checks passed
@den-sq
den-sq deleted the codex/157-error-presentation branch October 19, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify error and warning presentation on slicing and backprojection

2 participants