fix(spec-builder): delete error in dialog, clear-filter exit (#7662)

[NicholasRBowers]

Problem / Motivation

Two UX gaps in the Spec Builder delete/filter flow, deferred out of PR #5111 by its UX Review lane:

1. A failed spec delete is invisible. The delete mutation's error handler wrote the raw error message to the page-top alert banner — which sits behind the confirm dialog's dimmed backdrop while focus is trapped inside the dialog. On a failed delete, the user only sees the Remove button revert from its busy state with no visible reason, inviting blind retries.
2. The empty-filter state is a dead end. The rail's "No specs match that filter" state offers no recovery affordance: the filter input has no clear control, so recovering means re-finding the field and manually emptying it.

Why it matters

A delete that fails silently reads as the app ignoring the click — the user retries into the same refusal (e.g. a mid-turn worker slot) with no way to learn why, and the actual reason is painted on a surface they physically cannot see. The filtered-to-nothing rail strands the user one unnecessary interaction away from their spec list. Both were flagged by the UX Review lane on #5111 and tracked in #7662.

What changed (motivation → approach → change)

1. Delete failure renders inside the dialog, where the user is looking. Dropped the onError → setErr raw pass-through on deleteMutation and rendered the failure in the confirm dialog through the shared ErrorNotice (per the errors-use-error-notice rule): a translated what-happened + try-again lead (couldn_t_remove_this_spec_try_again, new key in all 13 catalogs) with the reason detail beneath. No agent hand-off on this notice — the chat navigation unmounts the whole view, and SpecDetail can be holding stacked unsent review comments and an in-progress DocView draft; the call site carries the No hand-off: comment naming both. deleteMutation.reset() runs when the dialog opens, so a dismissed failure does not greet the next attempt as if it had already failed.
2. One-click exit from an empty filter. The filtered-to-nothing rail state now renders the shared FilteredEmpty component (components/ui.tsx, documented for exactly this case): it echoes the query back and offers the established Clear-filter action, which empties the input and restores the full list. No new i18n keys — FilteredEmpty carries its own components.ui wording; the rail's now-orphaned no_matching_specs key is removed from all 13 catalogs (the dead-key ratchet enforces this).

Tests

Red-before-green proven: all four fail against the pre-fix components and pass after.

• SpecBuilderSpecDetailCoverage.test.tsx — surfaces a refused delete inside the dialog and leaves the spec open: a 409 delete renders the translated lead + reason in the dialog's role="alert", setErr (the page-top banner) is not called, onDeleted not fired, dialog stays open.
• SpecBuilderSpecDetailCoverage.test.tsx — opens a fresh dialog without the previous attempt's failure: dismiss after a failure, reopen → no stale alert.
• SpecBuilderRailClearFilter.test.tsx — offers a Clear-filter action only when the filter matched nothing: unfiltered list shows no button; a no-match filter shows the empty state + button.
• SpecBuilderRailClearFilter.test.tsx — clears the filter and restores the full list on click: the input value is actually emptied and both groups return.

Full local gates: npx tsc -b, full npm run test website suite (29,175 tests green), i18n-check, eslint, isort/flake8/mypy.

Manual verification

Verified the rendered states myself via the asserted-state capture harness (scripted Playwright, gateway-free — capture/spec-builder-7662.tsx + scripts/capture-spec-builder-7662.mjs): every frame below asserts its state before the shot (translated lead present, reason present, zero setErr calls, dialog fits 390px, input emptied on clear).

Screenshots / video

Refused delete — inside the dialog (dark)	Empty-filter state with Clear filter

More: light theme, 390px viewport, cleared state

Related Issues

Closes #7662

Pattern harvest

Rule candidate: review-prompt
Pattern: "mutation error routed to a surface occluded by the modal that triggered it — error must render inside the focus-trapped layer"

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 1323e76bb447621ff2f6c44cf718c4b213e5fb13 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Both fixes check out cleanly against the shared components they claim to reuse: ErrorNotice (falsy-message renders nothing, opt-in agent hand-off matching the stated draft-loss rationale) and FilteredEmpty (documented for exactly this filtered-to-zero case). The capture harness and committed screenshots follow the repo's established convention, all 13 locales carry the key swap, and the deleteMutation.reset() on dialog open closes the stale-error loop end-to-end. No design-level issues found.

Design-Verdict: PASS

Two real dead-end UX states fixed at the right layer, by reusing the two shared components documented for exactly these cases.

[DESIGN-REVIEWED] 1323e76

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of 1323e76bb447621ff2f6c44cf718c4b213e5fb13 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Both fixes move feedback to where the user is actually looking — the dialog and the filtered rail — and the screenshots confirm clean rendering at all sizes.

Suggestions

• couldn_t_remove_this_spec_try_again: drop "— try again." from the title. The 01 screenshot shows the conflict this bakes in: the bold lead says retry while the appended backend message ("the worker slot is mid-turn — pause the build first") says a retry will fail until the user does something else. "Couldn't remove this spec." lets the specific remedy own the next step.

[UX-REVIEWED] 1323e76

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 1323e76bb447621ff2f6c44cf718c4b213e5fb13 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 1323e76

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 1323e76bb447621ff2f6c44cf718c4b213e5fb13: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 1323e76bb447621ff2f6c44cf718c4b213e5fb13 — this comment is updated in place on each push.

Review details

The change is clean: the removed i18n key has no call sites, FilteredEmpty receives valid props, the trigger condition is unchanged from the prior banner, and deleteMutation.reset() clears stale errors on reopen. Nothing survives falsification and I found no new grounded defect.

No findings.

[OPUS-REVIEWED] 1323e76

Verdict parsed from the review's SHA-scoped output markers for commit 1323e76bb447621ff2f6c44cf718c4b213e5fb13.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 1323e76bb447621ff2f6c44cf718c4b213e5fb13: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 1323e76bb447621ff2f6c44cf718c4b213e5fb13 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All facts are verified. The fix mechanisms are the documented ones (errors-use-error-notice rule, FilteredEmpty documented for exactly this case), the capture harness and screenshots follow heavy repo precedent (195 files in website/capture/, 925 in temp-screenshots/). The one real finding: the root cause — a mutation error routed to a page-top banner occluded by the modal that triggered it — has a confirmed unfixed sibling in the same app: SettingsModal.tsx:61 (and the read-failure at :46) routes to the same setErr banner behind that modal's own backdrop, and the modal stays open on error. The PR's own "Pattern harvest" names the general pattern while fixing one instance.

First-Principles-Verdict: CONCERNS

Both fixes land at mechanism level with derived causes, but the occluded-banner root cause survives one door over, in SettingsModal, unfixed.

What this change ships

Intent: make a failed spec delete visible and give the filtered-to-nothing spec list a one-click exit (#7662). This is a FIX (two of them).

1. A failed delete now shows its reason inside the confirm dialog — justified (spec-builder: surface delete failure inside the confirm dialog; add Clear-filter action to empty rail state #7662; errors-use-error-notice rule)
2. Reopening the delete dialog no longer shows the prior attempt's failure — justified
3. New "Couldn't remove this spec" message in all 13 languages — justified (i18n invariant)
4. The empty-filter rail now offers Clear filter — justified (shared FilteredEmpty, ui.tsx:326, documented for this case)
5. Empty-filter wording becomes the generic "No matches for …" echo — declared, rides with item 4
6. Old "no specs match" key removed from all 13 catalogs — justified (dead-key ratchet)
7. Committed capture harness (entry + Playwright script) — declared; convention, 195 sibling files in website/capture/
8. Five PNGs under temp-screenshots/ — declared; convention, 925 sibling files

Watch

• Point patch with one counted unfixed sibling. Grepped onError:.*setErr: 33 hits; exactly one shares the fixed root cause — spec-builder/components/SettingsModal.tsx:61 routes a failed settings save (and :46 a failed read) to the same page-top setErr banner behind that modal's own dimmed backdrop, and the modal only closes onSuccess, so a failed save reads as the Save button silently reverting — the identical defect. The description's own "Pattern harvest" states the general pattern ("error must render inside the focus-trapped layer") while the diff fixes one of the two instances in this app. The sibling fix is the same size as this one; fix it here or record it as deferred on spec-builder: surface delete failure inside the confirm dialog; add Clear-filter action to empty rail state #7662.

[FIRST-PRINCIPLES-REVIEWED] 1323e76

[NicholasRBowers]

Disposition: fixed (in f5fcbfebf6f370c8a4edecdc49689352582cc29b)

• The Clear-filter item re-implements FilteredEmpty (ui.tsx:326)

The Clear-filter affordance duplicates the shared FilteredEmpty component (ui.tsx:326, whose doc comment describes this exact case), and copies its translation into 13 new catalog lines.

Verified against the code: FilteredEmpty at website/src/components/ui.tsx:326 is documented for precisely this state, takes query/onClear, and renders the clear action with the existing components.ui.clear_filter key. The finding holds, and the demanded change is a subtraction — applied exactly as suggested:

• SpecRail.tsx now renders <FilteredEmpty query={filter} onClear={() => setFilter('')} /> in place of the hand-rolled Btn + FilterX block; the FilterX import is gone.
• The duplicate apps.specBuilder.components.specRail.clear_filter key is deleted from all 13 catalogs (one line each; the delete-error key from finding-free item 1 stays).
• Tests and the capture harness assert the shared component's rendering (filtered-empty test id); frames 04/05 re-captured from the asserted state and re-pinned to the new head.

On the Watch item (SettingsModal.tsx:61/:46 routing errors to the occluded page-top setErr): agreed it is the same harvested root cause one level up, and taking it as a follow-up rather than widening this PR — tracked in the issue's follow-up notes rather than expanding this diff, per the review's own "fair follow-up, not a demand" framing.

[NicholasRBowers]

Disposition: accepted-and-deferred → tracked in #8757

• Occluded-banner root cause survives one door over, in SettingsModal, unfixed

spec-builder/components/SettingsModal.tsx:61 routes a failed settings save (and :46 a failed read) to the same page-top setErr banner behind that modal's own dimmed backdrop, and the modal only closes onSuccess … The sibling fix is the same size as this one; fix it here or record it as deferred on #7662.

The finding holds — verified the same grep (onError:.*setErr under apps/spec-builder): SettingsModal is the one remaining instance of the harvested pattern in this app. Taking the review's second offered path (record as deferred) rather than widening this PR: #7662 names exactly the two gaps this PR fixes, and each new surface here re-arms every review lane on the whole diff. The deferral is tracked as #8757 — carries the deferred-finding label, an assignee, and Due: 2026-09-19 — scoped to mirror this PR's ErrorNotice-in-dialog fix (both :61 save and :46 read paths) with equivalent tests. Not a security/data-loss/corruption finding, so deferral is permitted.

[bolichen97]

Tech Lead review: approved.

Scope matches #7662 exactly — both acceptance criteria met, nothing extra. The delete fix is a subtraction plus the documented shared surface: onError -> setErr dropped, ErrorNotice rendered inside the confirm dialog with askAgent correctly left at its opt-out default (the hand-off would unmount SpecDetail and discard stacked unsent review comments plus the in-progress DocView draft, and the call site comments say so). deleteMutation.reset() on dialog open closes the stale-error path. ErrorNotice renders nothing on a falsy message, so the first open is clean without a guard. The rail fix replaces a hand-rolled empty state with the shared FilteredEmpty, whose doc comment names this exact case; the trigger condition is unchanged, and the orphaned no_matching_specs key is removed from all 13 catalogs with the one new key added to all 13.

All gates green on 1323e76b (the six cancelled check-runs are superseded duplicates, each with a later success of the same name). GPT 5.6 and Opus 4.8: no findings. Design and UX: PASS. First Principles CONCERNS is advisory and correctly dispositioned — the sibling occluded-banner instance at SettingsModal.tsx:46/:61 is deferred to #8757 with a label, assignee, and due date rather than widening this diff.

Non-blocking, author's call: the UX lane's suggestion to drop "— try again." from couldn_t_remove_this_spec_try_again is a fair point — the bold lead can contradict a backend reason that says a retry will fail until something else is done first. Not worth another full re-review cycle on its own.