fix(chat): fold every sub-agent wave in a user turn, not just the first - #8323
fix(chat): fold every sub-agent wave in a user turn, not just the first#8323iamwhatever wants to merge 1 commit into
Conversation
Problem: a user turn that fans out more than once rendered every wave after the first in full -- the per-completion prose sat in the transcript beside a synthesis turn that restated it, which is exactly the duplication the fan-out fold was added to remove. Root cause: the region a synthesis row opens was marked `regionHasForeign = true` wholesale, to keep that row's own answer out of a later wave's fold. That protected the answer by disqualifying the whole region, so no wave after the first could ever fold, and the flag stayed sticky until the next user prompt. Change: carve out just the answer instead of the region. `settlePendingSynthesisAnswer` splits an un-flushed synthesis answer off at its turn boundary -- the `turn_stats` meta the runner stamps on a completed turn's last assistant row -- flushes it as its own visible turn, and re-anchors the region behind it, so the rows after it (the next wave's interim work) fold normally. It runs at BOTH flush sites that can hold a pending answer: the synthesis branch and the `subagent` turn-opener a queue-drained completion arrives as. Re-anchoring RE-DERIVES `regionHasForeign` from the batch it retains rather than clearing it, so a cron reply or note in wave 1 no longer disqualifies waves 2..N while one that landed after the answer -- and is therefore still inside the new region -- keeps it disqualified. With no locatable turn boundary (an errored turn, an older transcript, an answer still streaming) the region degrades to unfolded -- the previous behaviour -- rather than guessing where the answer ended. Display-only: the model still receives every result in full.
github-actions
Bot
added
the
readiness: checking
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS Extends the existing fan-out fold to later waves, keeping every synthesis answer visible; all uncertain paths degrade to the familiar unfolded rendering. No new strings, controls, or visual surfaces — the change removes the duplicated per-completion prose users saw beside each restating synthesis, and every ambiguous boundary (no [UX-REVIEWED] c7a6c9f |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Root-cause fix — replaces the region-wide poison flag with a per-answer carve-out, degrades unfolded on any doubt, each mechanism pinned by a failing-without-it test. Suggestions
[DESIGN-REVIEWED] c7a6c9f |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of The change checks out against the repository. Verified: First-Principles-Verdict: PASS A reported live-session defect, fixed at its named cause — the borrowed region-poisoning flag — with every behavior declared, tested, and degrading toward visible. What this change shipsIntent: stop later sub-agent waves in one chat turn from showing their interim prose twice, in full, beside the synthesis that restates it. FIX.
No undeclared items, no riders, no moves or relabels, no new public surface. Counted: [FIRST-PRINCIPLES-REVIEWED] c7a6c9f |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
github-actions
Bot
added
readiness: action required
1 participant
Problem / Motivation
A chat turn that fans out sub-agents more than once renders every wave after the
first in full: the per-completion prose the agent writes as each result lands
sits in the transcript immediately above a synthesis turn that restates it. The
user reads the same report twice, at full length.
Reported from a live session judging two large PR stacks — a collapsed
Worked through N stepstoggle, the2 agents finishedrun card, then afull-length verdict, then
Sub-agents finished · consolidated summary requestedfollowed by a synthesis saying the same thing again.
Why it matters
Folding that interim region is the whole remedy for post-fan-out duplication
(#7231). Wave 1 got it; waves 2..N never did — and a second wave inside one user
turn is the normal shape for the work this feature was built for, because the
gateway re-arms
_pending_synthesiswhenever a wave's last agent finishes,whichever turn spawned it. So the duplication the fold exists to remove came back
for the majority of real fan-out sessions, while looking fixed.
What changed (motivation → approach → change)
Symptom: only the first wave in a user turn folds.
Root cause: the region a synthesis row opens was disqualified wholesale with
regionHasForeign = true. That flag exists to keep unrelated content (a cronreply, a note) out of a fold, and it was borrowed here to protect one specific
row — the synthesis turn's own answer, which is real user-facing content no later
synthesis restates. Protecting one row by poisoning the whole region meant no
wave after the first could ever fold, and the flag stayed sticky until the next
user prompt.
Change: carve out just the answer instead of the region.
isTurnEndidentifies a completed turn's last assistant row by theturn_statsmeta the runner already stamps there (_attach_turn_stats,chat_runner.py). Nothing else in the display layer marks a turn boundary —chat_segmentis a live wire event, not a property of a persisted row — sothis is what a reload has to go on.
settlePendingSynthesisAnswer()splits an un-flushed synthesis answer off atthat boundary, flushes it as its own visible (non-interim) turn, and
re-anchors
regionStartbehind it. The rows after it belong to the next waveand fold normally.
branch, and the
subagentturn-opener — a completion row drained through theslot queue flushes the batch there, and skipping it let the next synthesis
mistake that wave's own reply for the answer boundary and leave the wave
unfolded.
regionHasForeignfrom the batch it retains ratherthan clearing it. Clearing is what lets a cron reply in wave 1 stop
disqualifying waves 2..N; re-deriving is what keeps a cron reply that landed
after the answer — and is therefore still inside the new region —
disqualifying it, instead of folding an unrelated prompt's reply behind the
fan-out toggle.
Every failure path still degrades toward unfolded: with no locatable turn
boundary (an errored turn, a pre-
turn_statstranscript, an answer stillstreaming) the region renders exactly as it did before this change rather than
guessing where the answer ended. Hiding an answer behind a toggle that promises a
repeat below is the one outcome that must stay impossible.
Display-only. No message content, no wire shape, and no backend behaviour
changes — the model still receives every result in full.
Tests
website/src/test/groupDisplayItems.test.ts, five new cases:it — two interim regions, both synthesis answers visible. This is the
reported bug; it fails on the pre-fix module.
subagentcompletion openerfolds — the second flush site. Removing the
settlePendingSynthesisAnswer()call there turns this red, so the call is not decorative.
the re-anchor's foreign reset. Dropping that reset turns this red.
fold — the other half of the same rule: re-derived, not cleared. This one
fails if the reset is unconditional.
degradation, so a future change cannot turn a missing
turn_statsinto ahidden answer.
The pre-existing case asserting a synthesis answer is never folded behind a later
wave's toggle still passes unchanged.
Manual verification
N/A —
groupDisplayItemsis a pure function over the message list and theregression is a grouping decision, which the unit tests assert directly. The
reported transcript shape is reproduced verbatim as a test fixture.
Screenshots / video
Why no screenshot: grouping logic only — the fold's rendered appearance
(
TurnBlock's interim branch, its toggle and collapsed sections) is untouched;this change only decides which rows enter that already-shipped fold.
Related Issues
no linked issue: reported directly in chat from a live session, not filed.
Pattern harvest
Pattern: a broad disqualifier flag reused to protect one narrow row, which then
suppresses the feature for every later region in the same scope — and, on the way
out, a reset of that flag that must be re-derived from the retained state rather
than cleared.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)