fix(chat): resolve the optimistic steer bubble against the steer receipt - #7658
Conversation
UX Review (Fable 5, fork) — ✅ PASSUX-level review of The base code confirms the pieces: the badge is One residual gap sits on a changed line: UX-Verdict: PASS Removes a badge that lied about injection and a duplicate rendering; every receipt shape now resolves to the honest state, and no user text is lost. Watch
[UX-REVIEWED] bfb24a2 |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of All claims in the description check out against the base tree: the three receipt shapes exist at Design-Verdict: PASS Reads the receipt the fire-and-forget path discarded, reusing the codebase's existing optimistic-reconciliation pattern — root-cause fix, correctly guarded against late/racing receipts. Suggestions
[DESIGN-REVIEWED] bfb24a2 |
First Principles Review (Fable 5, fork) — ✅ PASSPremise-level review of All evidence checked against the trusted base. Composing the review. First-Principles-Verdict: PASS Retracting an optimistic badge the server contradicted is a real, verified defect; every item in the diff serves that one retraction. What this change shipsIntent: stop the steer bubble asserting "steered into the running turn" when the server actually queued the text or started a new turn — a FIX.
Verified against base: the three receipt shapes exist ( [FIRST-PRINCIPLES-REVIEWED] bfb24a2 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsBoth No findings. [OPUS-REVIEWED] bfb24a2 |
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
9a969c5 to
b86da28
Compare
b86da28 to
590bca1
Compare
590bca1 to
08fee21
Compare
08fee21 to
2fc48aa
Compare
e263ecf to
5e4195c
Compare
5e4195c to
964d95a
Compare
A steer the server queued instead of injecting kept its "Steered into the running turn" badge beside the queue card for the same text; before/after capture committed as screenshot evidence.
964d95a to
bfb24a2
Compare
bolichen97
left a comment
There was a problem hiding this comment.
Approving on the strength of a full readiness audit of every open PR against main, not a
line-by-line reading of this diff — recording that plainly so the next reader knows what this
stamp does and does not cover.
Verified against this exact head SHA:
readiness: passedpresent, andPR Readiness— the one required status context onmain
(rulesetprotected-branches) — issuccesson this head.- No check run on this head is
failure,cancelled,timed_outor still in flight. Skipped
jobs are path-filtered conditionals, none of them required. mergeable: true, and the head is not far enough behindmainfor its green CI to describe a
base that no longer exists.- No surviving reviewer
CHANGES_REQUESTED: any such review is on an older commit and therefore
already dismissed bydismiss_stale_reviews_on_push. - Every issue comment, inline review comment and review thread was read and classified. Nothing
left is an unresolved human change request — the remainder is bot review-lane output, resolved
or outdated threads, explicitly non-blocking suggestions, and author status notes.
Auto-merge (squash) is armed, so this lands once every other ruleset requirement is met.
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
Problem / Motivation
Pressing Enter mid-turn renders the message twice, and one of the two copies
claims something that did not happen.
steer()optimistically appends the bubble withmeta.steer, which draws the"Steered into the running turn" badge. The mutation was explicitly
fire-and-forget (
// fire-and-forget: no onSuccess), so nothing ever comparedthat claim against the server's answer — and the backend does not always inject.
Two accepted receipt shapes contradict the badge:
{ok, queued}— the busy branch fell through to the queue, because the steerRPC was unavailable or the turn's teardown requeued the text. The backend
broadcasts
queue_push, so the same message is also drawn as an interactivequeue card above the composer. The user sees their text twice: once as a card
waiting for the next turn, once as a badge asserting it was injected into the
current one.
{ok, slot, mid}— the POST racedchat_done, missed the busy branch entirelyand started a new turn. The text ran, but it was not steered into anything.
The queued case has a second, more confusing symptom: switching away from the
session and back makes the badge disappear. That is the correct state showing
through — the transcript refetch has no steer row to rebuild, because the text is
in the queue and was never injected. The badge was the lie, and its vanishing
looked like a second, separate bug.
Why it matters
The badge is load-bearing for a decision the user makes constantly: whether their
correction landed in time to change the turn in flight, or is parked behind it.
Getting that wrong in the optimistic direction is the harmful direction — the
user believes the agent has been redirected and stops watching, while the text is
actually sitting in the queue and the turn continues on its original course.
The duplicate rendering compounds it: cancelling the queue card removes the card
and leaves the badge behind, so the transcript then shows an injection for a
message the user explicitly cancelled.
What changed (motivation → approach → change)
The receipt already carries everything needed to tell the three outcomes apart;
it was simply discarded.
confirmedDeliveredinutils/sendDelivery.tshadalready reasoned this through for the ordinary send path — "when it does queue,
it broadcasts
queue_push, and that card is the server-owned representation ofthe message" — but the steer path had no equivalent, and unlike a plain bubble a
steer bubble carries an affirmative claim rather than merely a pending one.
So the fix reads the answer and resolves the bubble against it, one remedy per
shape:
steered: true— the claim is true, nothing to do.queued: true— the queue card owns the text, so the bubble is removed.Demoting it instead would leave a duplicate, and the queue drain later appends
the real row on top of that.
meta.steeris dropped, demoting it to a plain user message.Both mutating modes are gated on the bubble still being
optimistic: once asteer_pushecho orconfirmOptimisticSendhas cleared that flag the server ownsthe row, and a late receipt must not delete or rewrite it. The slot travels in the
mutation variables rather than being read off
activeSlotinside the callback,because the POST outlives the render that started it and the user can switch
sessions while it is in flight — the same reason the steer echo and
confirmOptimisticSendare slot-addressed.An unreadable body (
okabsent) is deliberately left alone rather than treated asa failure: rewriting a rendered row on an answer we could not read would assert an
outcome that was never measured.
Tests
ChatPage.steerQueuedReceipt.test.tsxdrives the real path — mount mid-turn,type, press Enter (Steer is the default busy action), mock the receipt — and
asserts store state rather than the badge, since the flag is the input the badge
is derived from:
queuedreceipt leaves no user row for the steered text (the queue card ownsit),
meta.steercleared,steeredreceipt leaves the row and its flag untouched.Verified as a negative control by reverting only the two source files and re-running:
the first two fail for exactly the intended reason (
expected [ { role: 'user' } ] to have a length of +0 but got 1, andexpected true to be falsy) while the thirdstill passes — so the tests discriminate rather than failing wholesale.
chatSlice.resolveOptimisticSteer.test.tscovers the guards a single send cannotreach: a row the server already claimed is untouched, a non-matching
sendIdis ano-op, a bubble living in
slotMessagesafter a session switch still resolves, anunsafe slot key is refused, and demotion preserves the row's
sendId(thereconciliation key for any later echo).
Full website suite: 1736 files / 27311 tests pass.
tsc --noEmitclean;eslintreports 0 errors and the same 8 pre-existing warnings as the base commit.
Manual verification
The frames below were captured in a real browser (Playwright-driven Chromium,
980px viewport at 2x) rather than asserted in jsdom, so the duplicate is visible
rather than inferred.
Screenshots / video
Both frames render the real
UserMessageandQueueStackcomponents againststore state produced by the real reducers: the optimistic steer bubble
(
appendMessagewithmeta.steer), plus the queue card the backend'squeue_pushbroadcast creates (appendQueuedMessage). The only differencebetween them is the receipt dispatch —
beforeomits it, which is exactly whatmaindoes since the mutation was fire-and-forget;afterdispatchesresolveOptimisticSteerwith thequeuedoutcome the server actually returned.Before — the same message twice: the badge asserting it was injected into the
running turn, and the queue card saying it is waiting for the next one.
After — the queue card alone, which is where the text actually is.
A DOM probe run on each frame in the same pass, so the images are not the only
evidence:
beforereportsbadge: truewith one[data-role="user"]row and thetranscript reading
"Steered into the running turn / <text>";afterreportsbadge: false, zero user rows, and an empty transcript. The queue card is presentin both.
Related Issues
None — reported directly from a live session, with the duplicate render and the
badge disappearing on a session switch observed as two symptoms of this one cause.
Pattern harvest
Rule candidate: review-prompt
Pattern: an optimistic UI element that asserts a specific outcome (not merely
"pending") while its request is fire-and-forget — the assertion can never be
retracted when the server reports a different outcome, and the contradicting
server-owned representation renders alongside it.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)