Skip to content

fix: silence duplicate error toasts in the digital-twin tabs (#2669)#2730

Merged
atomantic merged 3 commits into
mainfrom
claim/issue-2669
Jul 17, 2026
Merged

fix: silence duplicate error toasts in the digital-twin tabs (#2669)#2730
atomantic merged 3 commits into
mainfrom
claim/issue-2669

Conversation

@atomantic

@atomantic atomantic commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

The Digital twin batch of the silent: true sweep. Refs #2669.

Most of this batch was already swept by #2682 — the issue's comments say so, and that held up. I re-verified rather than trusting them (the thread documents these lists going stale in both directions), using a wrapper-aware scan: classify every api* wrapper as toasting vs silent-by-default, then match custom-catch callers that don't pass silent. That plus review found four genuine bugs:

  • OverviewTab — contradiction check. Renders the failure inline as contradictions.error, so request()'s default toast reported the same error a second time. Every sibling using the identical .catch(e => ({ error: e.message })) pattern (ConfidenceGauge, ListEnrichment, PersonalityMap, TestTab, EnrichTab, ImportTab) already passes silent — this was the outlier. Threaded a backward-compatible trailing options arg into detectDigitalTwinContradictions, the one affected wrapper.

  • TasteTab — section reset. Toasted 'Section reset' unconditionally, so a failed reset stacked a success toast on top of request()'s error. Gated on the outcome via .then(() => true).catch(() => false), the form already used by PersonasTab:144 / TimeCapsuleTab:68. (Truthiness would be wrong — the endpoint 204s, so success is null.)

  • TasteTab — "Go deeper" (found in review). Collapsed "request failed" and "nothing to ask" into the same null, so a failure got request()'s toast and a "try completing more identity documents" hint that misattributed the cause. Split with an undefined sentinel per CLAUDE.md's rule that failed-to-fetch must not collapse into legitimately-empty. No silent here — request() is the right single layer for the error, and the hint is now only reachable on the genuine 200-null path.

  • OverviewTab — settings save (found in review). An uncaught rejection left saving true, stranding the Save button permanently disabled on "Saving..." with no retry. A state wedge rather than a toast bug, and adjacent to the sweep rather than part of it — rolled in because it's three lines in a file this batch already audits.

Known gap, deliberately not fixed here → #2733

Review flagged that "Go deeper"'s sentinel doesn't cover a provider failure, and that's correct: callProviderAISimple returns { error } instead of throwing (aiProvider.js:161,167,176), the service turns that into return null (taste-questionnaire.js:934-937), and the route bare-res.json()s it — so a provider error is 200-null, indistinguishable client-side.

I did not fix it here because the obvious fix is unsafe on its own: making the service throw would give request()'s toast plus useAIStatusNotifications' toast (it always toasts ai:status phase error, even for silent ops), re-creating the double toast #2669 exists to kill. It needs a server-side signal plus a decision about which layer owns the message — a server-contract change outside this client-side batch. Filed as #2733 with the full trace and acceptance criteria; the caveat is recorded at the call site.

Verified correct and deliberately left alone

  • Toast-less swallowing catches (AccountsTab CRUD, PersonasTab, TimeCapsuleTab, TasteTab.handleGenerateSummary) — only a validation toast before the call or a toast.success on the happy path. These keep the single default layer; adding silent would remove their only failure feedback.
  • Already passing silentSoulWizard, NextActionBanner, AutobiographyTab (all 5), EnrichTab, ConfidenceGauge, ListEnrichment, PersonalityMap, ImportTab, ObservedTasteEvidence.
  • TestTab:156 — catch has no toast, with a comment saying so. Correct single layer.
  • The two grep-invisible classes prior batches flagged — no useAsyncAction / useLockToggle / useSseJobSlot consumers and no .then(onOk, onErr) sites in this directory.

Added client/src/services/apiDigitalTwin.test.js pinning that the wrapper forwards silent without clobbering method/body, and stays callable without options. This class of miss has recurred across batches, so it's worth a guard.

Test plan

  • cd client && npx vitest run404 files / 4389 tests, 0 failures.
  • New apiDigitalTwin.test.js covers silent-forwarding and the no-options back-compat path.
  • Reviewers: claude (2 rounds — both findings applied) and codex --base origin/main (clean, no findings).
  • Manual reasoning: a failed contradiction check shows only the inline red error; a failed Taste reset shows only request()'s error with no contradictory "Section reset"; a failed "Go deeper" no longer adds the misleading hint; a failed settings save re-enables the button.

Remaining

Batches still unswept on #2669 (counts unverified — re-scan first; every batch so far has found the advertised list stale):

  • Creative Director (~3)CreativeDirector.jsx:180, PlanTab.jsx:91/103/113, OverviewTab.jsx:148
  • CatalogCatalogCastPanel
  • Meatspace
  • Misc singlesGmailMboxImportPanel, LocalLlmTab, MessageDetail, GenomeTab, PipelineManuscriptEditor
  • ManuscriptCommentCard.jsx:247,251generatePipelineManuscriptFix still needs an options param

Already shipped, do not re-sweep: settings + JIRA (#2670), messages/calendar/sharing, pipeline stages/canvas (#2714), writers-room/universe/universeBuilder/media/hooks (#2682), apps/GitHub/misc (#2721), LoRAs/media (#2723), and digital-twin (this PR — now exhausted apart from #2733).

Most of this batch was already swept by #2682. A wrapper-aware rescan
(classify each api* wrapper as toasting vs silent-by-default, then match
custom-catch callers) found two genuine sites left:

- OverviewTab's contradiction check renders the failure inline as
  `contradictions.error`, so request()'s default toast reported the same
  error a second time. Threaded a backward-compatible `options` arg into
  `detectDigitalTwinContradictions` — every sibling using this same
  `.catch(e => ({ error: e.message }))` pattern already passes `silent`.
- TasteTab's section reset toasted "Section reset" unconditionally, so a
  failed reset stacked a success toast on top of request()'s error. Gated
  it on the outcome via the `.then(() => true).catch(() => false)` form
  already used by PersonasTab and TimeCapsuleTab (a truthiness check would
  misread the endpoint's 204/null success as a failure).

Everything else in the directory verified correct and left alone: the
toast-less swallowing catches keep the single default layer, and TestTab /
EnrichTab / SoulWizard / NextActionBanner / ObservedTasteEvidence already
pass `silent`. No useAsyncAction/useLockToggle consumers and no
`.then(ok, err)` sites exist here — the two grep-invisible classes prior
batches flagged.
…ents (#2669)

Review findings on the digital-twin batch.

TasteTab's handleGoDeeper collapsed "request failed" and "no question
available" into the same null. The endpoint 200s with a null body when it
has nothing to ask, but rejects on a real failure — so a provider error
got request()'s toast AND the "try completing more identity documents"
hint, which actively misattributes the cause. Split the two with an
`undefined` sentinel per the CLAUDE.md rule that failed-to-fetch must never
collapse into fetched-and-legitimately-empty. No `silent` needed: request()
is the right single layer for the error, and the hint is now only reachable
on the genuine 200-null path.

Also clear the spinner in OverviewTab's handleSaveSettings. The uncaught
rejection left `saving` true, stranding the Save button permanently
disabled on "Saving..." with no retry. Adjacent to the sweep rather than
part of it, but it is three lines in a file this batch already audits.
…fixes (#2669)

The provider-failure path arrives as 200-null (callProviderAISimple returns
{ error } instead of throwing, and the service returns null on it), so the
client sentinel cannot reach it. Narrow the changelog to the HTTP-level
failures it does fix and record the gap at the call site. Server-side
separation tracked in #2733.
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.

1 participant