Skip to content

refactor(chat): stop post-turn reconciliation; trust the anchor invariant#32024

Merged
dvargasfuertes merged 1 commit into
mainfrom
apollo/stop-reconcile-on-message-complete
May 25, 2026
Merged

refactor(chat): stop post-turn reconciliation; trust the anchor invariant#32024
dvargasfuertes merged 1 commit into
mainfrom
apollo/stop-reconcile-on-message-complete

Conversation

@vellum-apollo-bot
Copy link
Copy Markdown
Contributor

PR 2b.3 of the chat-state-slimming workstream. The endgame piece.

What

With the anchor invariant landed in 2b.1 (#32012) — assistant emits messageId = state.lastAssistantMessageId, client preserves the anchor via first-id-wins lock in appendTextDelta and role-based finalize in finalizeMessageComplete — the post-turn reconciliation loop that fired on every message_complete and activity_state idle is now redundant. The live client state is already correct; the refetch existed only to paper over id drift.

Removes startReconciliationLoop calls from:

  • handleAssistantActivityState (idle phase)
  • handleMessageComplete

Drops the now-unused epoch parameter from both handler signatures.

What stays

Reconciliation still fires legitimately on:

  • Load / switchchat-page.tsx top-level wiring
  • SSE reopen / post-watchdog reconnectuse-event-stream.ts:361,408
  • POST resolve confirmationuse-send-message.ts:429

The dispatcher still uses epoch for stale-epoch guards on the dispatch side, and cancelReconciliation calls remain (in handleAssistantTextDelta, handleGenerationHandoff, tool-call-handlers) since those still-spawned loops can be cancelled by text deltas and generation handoff.

Diff

 apps/web/src/domains/chat/hooks/use-stream-event-handler.ts            |  4 ++--
 apps/web/src/domains/chat/utils/stream-handlers/message-handlers.test.ts | 14 ++++----------
 apps/web/src/domains/chat/utils/stream-handlers/message-handlers.ts    |  4 ----
 3 files changed, 6 insertions(+), 16 deletions(-)

Tests

Updated 6 tests in message-handlers.test.ts to drop the epoch arg and flip the startReconciliationLoop assertion from toHaveBeenCalledWith(1) to not.toHaveBeenCalled() on the idle + message_complete paths. Renamed the affected tests to make the no-reconcile expectation explicit.

  • Lint clean
  • tsc clean (same pre-existing baseline drift in unrelated use-assistant-lifecycle.ts / ai-page.tsx / billing files as main)
  • bun test src/domains/chat matches main exactly: 1331 pass / 29 fail / 3 errors — zero new failures introduced. (The 29/3 baseline is cross-suite mock isolation flakiness; affected tests pass in isolation.)

Closes the chat-state-slimming endgame for the streaming path. Next up: PR 2c (client rewrites reconcile to pure id-match + deletes stableId).

…iant

PR 2b.3 of the chat-state-slimming workstream. With the anchor invariant
landed in 2b.1 (#32012) — assistant emits messageId = state.lastAssistantMessageId,
client preserves the anchor via first-id-wins lock in appendTextDelta and
role-based finalize in finalizeMessageComplete — the post-turn reconciliation
loop that fired on every message_complete and activity_state idle is now
redundant. The live client state is already correct; the refetch existed
only to paper over id drift.

Removes startReconciliationLoop calls from:
- handleAssistantActivityState (idle phase)
- handleMessageComplete

Drops the now-unused epoch parameter from both handler signatures.

The dispatcher still uses epoch for stale-epoch guards on the dispatch
side, and reconciliation still fires on load / switch / SSE reopen /
POST-resolve confirmation. cancelReconciliation calls remain since
those still-spawned loops can be cancelled by text deltas and
generation handoff.
@dvargasfuertes dvargasfuertes merged commit 871b0d6 into main May 25, 2026
7 checks passed
@dvargasfuertes dvargasfuertes deleted the apollo/stop-reconcile-on-message-complete branch May 25, 2026 19:31
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

ctx.setMessages(finalizeOnIdle);
const turnPhaseBefore = ctx.getTurnState().phase;
ctx.turnActions.completeTurn();
if (convId) {

P1 Badge Re-start reconciliation on terminal stream events

Removing reconciliation starts from both handleAssistantActivityState(...idle) and handleMessageComplete leaves normal SSE-driven turns with no post-turn reconcile trigger. In useSendMessage, startReconciliationLoop runs only on the poll fallback path (when there is no matching active stream), so the common "active stream" path now never enters the loop after terminal events. That regresses the silent-stall rescue path (onPollReconciled) and can leave stale streaming/tool-call state or missed final server-side message fields whenever terminal SSE delivery is imperfect but the connection stays open.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

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