fix(web): stabilize fork action during streaming - #1399
Conversation
a242d73 to
502312c
Compare
There was a problem hiding this comment.
Findings
-
[Major] Keep the completed boundary stable while the new prompt is still queued —
SessionChatfilters uninvoked user rows out ofvisibleBlocksatweb/src/components/SessionChat.tsx:1195, but the hub setsthinking=trueas soon as the send is queued. Atweb/src/lib/assistant-runtime.ts:384,findLastIndexthen selects the previous completed turn's user as the active-turn start, so its assistant response is excluded and the Fork action moves backward or disappears until the CLI stampsinvokedAt.Suggested fix:
export function findLatestCompletedBoundaryId( blocks: readonly VisibleChatBlock[], isRunning: boolean, activeTurnStartedAt: number | null ): string | null { const assigned = assignThreadMessageIds(blocks) let limit = assigned.length if (isRunning) { const activeTurnStart = assigned.findLastIndex(({ block }) => ( visibleBlockRole(block) === 'user' && ( activeTurnStartedAt == null ? block.invokedAt != null : (block.invokedAt ?? block.createdAt) >= activeTurnStartedAt ) )) if (activeTurnStart >= 0) limit = activeTurnStart else if (activeTurnStartedAt == null) return null } // existing candidate scan } findLatestCompletedBoundaryId( visibleBlocks, props.session.thinking, props.session.activeTurnStartedAt )
Summary
- Review mode: initial
- One major boundary-selection gap remains during the queued-to-invoked handoff.
Testing
- Not run (automation). Add a regression case with completed
[u1, a1],isRunning=true, and an active-turn timestamp newer than both visible blocks; the expected boundary remainsagent-text:a1.
HAPI Bot
502312c to
ef25ea6
Compare
There was a problem hiding this comment.
Findings
-
[Major] Keep the boundary at the first user message of a steered turn - while Pi is streaming, an immediate follow-up is delivered as a steer and becomes another invoked user row without ending the main thinking state.
activeTurnStartedAtremains the original prompt timestamp (hub/src/sync/sessionCache.ts:492), butfindLastIndexselects the later steer, so assistant blocks already streamed within the same active turn are scanned as "completed" and the Fork action moves onto that transient card. Evidence:web/src/lib/assistant-runtime.ts:385.Suggested fix:
const activeTurnStart = activeTurnStartedAt === null ? assigned.findLastIndex(({ block }) => ( visibleBlockRole(block) === 'user' && block.invokedAt != null )) : assigned.findIndex(({ block }) => ( visibleBlockRole(block) === 'user' && (block.invokedAt ?? block.createdAt) >= activeTurnStartedAt ))
Summary
Review mode: follow-up after new commits
- The prior queued-before-invocation gap is addressed, but Pi steering can still advance the boundary into the active turn.
Testing
- Not run (automation). Repository execution is prohibited by this review workflow; GitHub
testandpr-reviewchecks were pending when reviewed. Add a regression with[completedUser, completedAssistant, activeUser, streamingAssistant, steerUser],isRunning=true, andactiveTurnStartedAtequal toactiveUserso the result remainscompletedAssistant.
HAPI Bot
ef25ea6 to
586ae5e
Compare
There was a problem hiding this comment.
Findings
-
[Major] Cut off active output even when the active user row is outside the tail window — a fresh tail request loads only 200 messages and the client later caps regular rows at 400 (
web/src/lib/message-window-store.ts:39,web/src/lib/message-window-store.ts:623). A long running turn can therefore omit its starting user row while retaining newer assistant output. When that happens,activeTurnStartis-1; because the explicit-marker branch leaveslimitunchanged, the scan promotes the first visible streaming assistant card to the current Fork boundary, recreating the movement this PR is intended to stop. Evidenceweb/src/lib/assistant-runtime.ts:393.Suggested fix:
if (activeTurnStart >= 0) { limit = activeTurnStart } else if (activeTurnStartedAt === null) { return null } else { const firstActiveBlock = assigned.findIndex(({ block }) => (block.invokedAt ?? block.createdAt) >= activeTurnStartedAt ) if (firstActiveBlock >= 0) limit = firstActiveBlock }
Summary
Review mode: follow-up after new commits
- The previous steering finding is resolved. One active-turn truncation path still exposes transient streaming output as the current fork boundary.
Testing
- Not run (automation; executing PR code is prohibited by this review workflow). GitHub
testandpr-reviewchecks were in progress when reviewed. Add a regression where the active user row is absent but a visible assistant block hascreatedAt >= activeTurnStartedAt; the result must remain the prior completed boundary, ornullwhen no prior boundary is visible.
HAPI Bot
586ae5e to
b37bc7d
Compare
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
- The previous Major finding at
web/src/lib/assistant-runtime.ts:393is resolved by the timestamp-based fallback cutoff when the active user row is outside the tail window. No additional correctness, security, regression, data-loss, performance, or maintainability issues found in the latest full diff. - Residual risk: browser-level rendering behavior for assistant-card joining during live Pi streaming is covered indirectly by helper tests, not an end-to-end streaming UI test.
Testing
- Not run locally (automation; executing PR code is prohibited by this review workflow). GitHub
testcheck passed. Focused coverage exists inweb/src/lib/assistant-runtime.test.ts:139for streaming reshapes, queued prompts, steering, tail truncation, completion, and missing markers.
HAPI Bot
Summary
Root cause
latestCompletedBoundaryIdwas derived from every currentvisibleBlockssnapshot. During Pi streaming, agent events can split assistant output into multiple temporary groups, so the candidate moved between message cards. The Fork button was mounted and unmounted as a result, shifting the timestamp beside it.Validation
bun run typecheckbun run test:web(241 files, 2157 tests)assistant-runtime.test.tsregression coverageCloses #1396