Skip to content

perf(streaming): bound terminal session payloads - #6818

Open
starship-s wants to merge 1 commit into
nesquena:masterfrom
starship-s:perf/bound-terminal-session-payload
Open

starship-s wants to merge 1 commit into
nesquena:masterfrom
starship-s:perf/bound-terminal-session-payload

Conversation

@starship-s

@starship-s starship-s commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

  • Large conversations can freeze when a run settles because terminal SSE events embed the complete persisted transcript and the browser rebuilds the active pane from that payload.
  • The persisted session remains correct; the failure is in terminal transport and reconciliation, where an unbounded payload replaces an already loaded transcript window.
  • Terminal events now carry a bounded, redacted message window with explicit raw offsets and total counts. The browser reconciles that authoritative window with the transcript rows it already has.
  • The merge preserves expanded windows, pagination coordinates, tool metadata, compression/rotation ownership, scroll state, and the settled final answer. It fails closed on malformed or stale window metadata.
  • This avoids asynchronous refetch/reconciliation, timers, fuzzy content matching, new dependencies, and broader stream-lifecycle state.

Contract Routing

Task type: Runtime/streaming performance and settlement-correctness fix.

Touched areas: Native and gateway terminal SSE payloads, terminal session reconciliation, sidebar message counts, paginated tool-result bounding, and focused regression coverage.

Relevant public docs:

  • AGENTS.md
  • CONTRIBUTING.md
  • docs/CONTRACTS.md
  • docs/rfcs/stable-assistant-turn-anchors.md

Scope boundaries: Preserve existing settlement semantics and transcript ownership. This PR does not change persisted session data, public APIs, visual layout, or the accepted terminal-state contract.

What Changed

  • Bound ordinary terminal session snapshots to the last 30 renderable rows while preserving raw message offsets, total message counts, todo state, and relevant tool-call metadata.
  • Reuse the existing 4,096-character tool-result clipping behavior for limited terminal and paginated payloads.
  • Apply the same bounded payload shape to native completion, cancellation, error, gateway completion, gateway error, background wakeup, and ephemeral terminal paths.
  • Reconcile terminal windows in the browser without collapsing a larger loaded window. Adopt self-consistent disjoint windows for long tool-heavy turns and local-only transient rows, while rejecting malformed or genuinely stale snapshots.
  • Keep sidebar message counts authoritative during optimistic sends and terminal rotation.
  • Add focused backend and Node-driven frontend coverage for payload bounds, offsets, tool-call projection, malformed metadata, long turns, local-only rows, compression, rotation, cancellation, errors, background completion, scroll preservation, and tool metadata.

Why It Matters

  • Terminal settlement no longer serializes and sends an entire large transcript when the browser needs only a bounded authoritative window.
  • A synthetic 6,000-row transcript reduced from about 23.7 MB to 197.8 KB, a 99.17% reduction.
  • Users keep the transcript range they already loaded instead of being collapsed to a short tail after completion.
  • Long tool-heavy turns and transient local rows still retain the settled final answer and usage metadata.
  • Full transcript history remains available through the existing pagination and reload paths.

UI Evidence

Not applicable. This PR does not change visual layout or styling. The user-visible behavior is terminal settlement performance and transcript continuity, covered by executable payload and browser-state tests.

Verification

Targeted terminal and neighboring regression coverage:

./scripts/test.sh -q tests/test_terminal_session_tail_and_merge.py tests/test_issue856_background_completion_unread.py tests/test_auto_compression_terminal_failure.py tests/test_cancelled_turn_status.py tests/test_reasoning_chip_btw_fixes.py tests/test_live_to_final_anchor_visible_order.py

Result: 144 passed

All changed test files:

pytest -q -p no:randomly <changed test files>

Result: 379 passed

Additional frontend regression coverage:

Result: 2,758 passed

Syntax and hygiene:

node --check static/messages.js

node --check static/sessions.js

git diff --check

Result: all passed.

Synthetic payload checks:

  • 6,000-row transcript: about 23.7 MB before, 197.8 KB after.
  • Tool-heavy turns at 2, 29, 30, 31, and 60 iterations retained their settled final answer.
  • Expanded 1,000-row and paginated 90-row browser windows remained expanded after an optimistic send and terminal settlement.

Risks / Follow-ups

Model Used

  • OpenAI gpt-5.6-sol for investigation, specification, orchestration, and verification.
  • OpenAI Codex gpt-5.6-luna at maximum reasoning effort for implementation and adversarial review.
  • Anthropic claude-opus-5 through Claude Code at high effort for independent correctness review.

@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch from d717724 to 7cb4a47 Compare August 7, 2026 06:00
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 06:07
@starship-s
starship-s marked this pull request as draft August 7, 2026 06:07
@greptile-apps

greptile-apps Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR bounds terminal SSE session snapshots and synchronously reconciles their explicit transcript coordinates with the browser’s already-loaded window.

  • Limits terminal snapshots to a recent renderable-message window while preserving total counts, raw offsets, todos, and projected tool metadata.
  • Applies the bounded payload contract across native, gateway, cancellation, error, background, and ephemeral terminal paths.
  • Preserves expanded browser transcript windows and reconciles capped keyed and repeated unkeyed tool metadata.
  • Keeps sidebar message counts authoritative across optimistic sends and terminal settlement.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
api/streaming.py Introduces the shared bounded, redacted terminal-session projection and uses it across native terminal lifecycle events.
api/gateway_chat.py Aligns gateway completion and terminal-error events with the bounded terminal-session payload contract.
api/session_ops.py Centralizes clipping of large tool-result rows for limited session payloads.
api/routes.py Reuses the centralized limited-payload clipping helper for paginated session responses.
static/messages.js Validates and merges terminal transcript windows while preserving expanded history and capped tool-call metadata.
static/sessions.js Keeps authoritative sidebar message counts synchronized through optimistic and terminal transitions.
tests/test_terminal_session_tail_and_merge.py Exercises payload bounds, coordinate validation, expanded-window preservation, keyed replacement, and repeated unkeyed metadata reconciliation.

Sequence Diagram

sequenceDiagram
  participant Runtime
  participant SSE as Terminal SSE
  participant Browser
  Runtime->>Runtime: Persist complete session
  Runtime->>Runtime: Select bounded message tail
  Runtime->>Runtime: Project and cap tool metadata
  Runtime-->>SSE: Session window + offset + total count
  SSE-->>Browser: done / cancel / error
  Browser->>Browser: Validate coordinates and metadata
  Browser->>Browser: Merge authoritative tail with loaded prefix
  Browser->>Browser: Preserve omitted keyed and repeated unkeyed calls
  Browser->>Browser: Render settled transcript
Loading

Reviews (13): Last reviewed commit: "perf(streaming): bound terminal session ..." | Re-trigger Greptile

Comment thread static/messages.js Outdated
@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch from 7cb4a47 to 6a1dd7d Compare August 7, 2026 06:27
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 06:50
@starship-s
starship-s marked this pull request as draft August 7, 2026 06:50
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 07:36
@starship-s
starship-s marked this pull request as draft August 7, 2026 08:00
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 08:00
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks @starship-s — this is a genuinely valuable fix (unbounded terminal done payloads freezing large conversations on settle is a real problem), and the bulk of it is solid. I gated the current head 6a1dd7da (Codex adversarial reproduction + full suite, 13,954 passed). Codex verified the core is sound — 480 same-session boundary combinations merged without loss or duplication, redaction is safe, and both the native and gateway terminal producers use the bounded redacted helper (39 focused terminal/gateway/cancel/pagination/redaction tests pass).

Holding for one CORE fix:

CORE — auto-compression settlement collapses an expanded transcript to the bounded tail (static/messages.js:2029)

The reconciler treats every session-ID change as unrelated and skips the merge:

if (!currentSession || !Array.isArray(currentMessages) || !currentSid || incomingSid !== currentSid) {
    return { ...incoming, messages: incomingMessages };   // raw bounded window, no merge
}

But automatic compression changes the session ID and sets parent_session_id to the previous ID (api/streaming.py:9816-9869). So when compression settles, incomingSid !== currentSid is true, the merge is skipped, and the bounded 30-row tail replaces the whole loaded window. Verified reproduction: loaded rows 910–971 were dropped, leaving only the terminal window — a user who had scrolled back loses their expanded transcript on compression.

Fix: recognize incoming.parent_session_id === currentSid as the same coordinate lineage and run the normal validated merge for that case (not just exact-SID-match). Add a rotated done/apperror regression test asserting the expanded prefix, the new session ID, offsets, and tool indexes all survive a compression rotation.

Merge-order note (not a blocker, but important)

This PR moves _messages_for_limited_payload from api/routes.py to api/session_ops.py, and PR #6815 (currently in review) adds to that same function (bounding oversized user/assistant rows in the paginated payload). The two aren't independent — whichever lands second needs a rebase that preserves the other's changes to that helper. Recommend shipping this one solo once the CORE fix lands, and rebasing whichever of the two goes second.

Everything else checks out — this is one reconciliation edge away from being a strong fix. Ping me when it's re-pushed and I'll re-gate the exact head.

Gate: Codex (full 22-file diff audit + real-probe reproduction of the compression-rotation collapse; verified same-session merge, redaction, and gateway parity intact) + full suite (13,954 passed). Verdict: SHIP ONLY WITH FIXES.

@nesquena-hermes nesquena-hermes added changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address size:L Large PR (>10 files or >250 LOC) labels Aug 7, 2026
@starship-s
starship-s marked this pull request as draft August 7, 2026 17:38
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 17:45
@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch from 6a1dd7d to 354a97b Compare August 7, 2026 17:51
@starship-s

Copy link
Copy Markdown
Contributor Author

Fixed and re-pushed at 354a97b9. _applyEmbeddedTerminalSession now treats a direct compression continuation (incoming.parent_session_id === currentSid) as the same coordinate lineage, so the existing validated merge preserves the expanded prefix while adopting the continuation session ID, offsets/count, and translated tool metadata. Unrelated rotations still use the existing raw-adoption path.

Added regression coverage for rotated done and apperror settlement, including expanded-prefix preservation, continuation metadata, tool indexes, and unrelated/missing-parent rejection. All 23 checks are green on the exact head, and Greptile reports 5/5 with no blocking finding.

@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch from 354a97b to 7954613 Compare August 7, 2026 18:02
Comment thread static/messages.js Outdated
@starship-s
starship-s marked this pull request as draft August 7, 2026 18:20
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Thanks @starship-s — the compression-rotation fix is spot on. I re-gated the new head 79546138 (adversarial Codex + full suite, 13,954 passed), and the CORE window-collapse from the last round is fixed: messages.js now treats parent_session_id === currentSid as same-coordinate lineage, so a compression rotation goes through the validated windowed merge instead of collapsing to the bounded tail. Same-session merges, redaction, and native/gateway parity all still hold.

One more edge — a separate cap that silently drops data, this time on tool calls rather than messages:

SILENT — terminal settlement drops tool cards when a session has >300 tool calls (api/streaming.py:136)

The done-payload applies an independent tail slice:

raw['tool_calls'] = terminal_tool_calls[-300:]

so only the final 300 tool summaries are sent, and static/messages.js:2084-2091 then discards the already-loaded overlapping calls and installs the capped list. Reproduced with 301 calls: call-0 disappeared on settle. It's the same unbounded→bounded truncation this PR correctly solves for the message window, but applied to tool_calls without the lineage-aware merge — so the earliest tool cards/results are silently lost for any long-running session that accumulates more than 300 calls.

Exact fix: remove the independent [-300:] slice and emit all projected tool calls until there's a lossless truncation protocol for them (mirroring the offset/count windowed-merge you built for messages), and add a 301-call merge regression test to lock it in.

Everything else on this head is clean — this is the last thin edge on the same "bound it without losing already-loaded data" theme. Ping me on re-push and I'll re-run the adversarial gate on the exact head.

Gate: Codex (adversarial reproduction — compression-rotation collapse confirmed fixed; one SILENT >300-tool-call truncation reproduced with 301 calls, call-0 dropped) + full suite (13,954 passed). Verdict: SHIP ONLY WITH FIXES. Large/sensitive streaming PR — parks for a maintainer decision, and ships solo (it shares _messages_for_limited_payload with #6815, so whichever lands second needs a rebase).

@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch from 7954613 to 0edf6be Compare August 7, 2026 19:12
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 19:17
@starship-s

Copy link
Copy Markdown
Contributor Author

@nesquena-hermes Re-pushed the final tool-metadata cap fix at 0edf6be4. All 22 checks are green and the PR is ready for your exact-head re-gate.

Comment thread static/messages.js Outdated
@starship-s
starship-s marked this pull request as draft August 7, 2026 19:21
@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch from 0edf6be to c40c001 Compare August 7, 2026 19:27
@starship-s
starship-s marked this pull request as ready for review August 7, 2026 19:31
@starship-s

Copy link
Copy Markdown
Contributor Author

@nesquena-hermes Re-pushed the repeated-unkeyed tool metadata fix at c40c0019. All 22 checks are green and the PR is ready for exact-head re-gating.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Re-gated at c40c001 after the follow-up commits and re-read the complete changed backend/frontend files plus tests/test_terminal_session_tail_and_merge.py. The two substantive concerns from the earlier review are resolved in the current head. I do not see a remaining blocker in the bounded terminal-snapshot path.

The server now emits a coordinate-complete tail contract in api/streaming.py:103-139: the payload retains the last 30 renderable rows, applies a 300-raw-row backstop, reports the full raw message_count, and rebases retained tool-call indexes. The client validates that contract before mutating state in static/messages.js:2018-2053, then builds the union in the same coordinate space at static/messages.js:2054-2059. Direct continuations are accepted through parent_session_id without widening the relationship to arbitrary session IDs.

Code reference

The current merge gate is now explicit and fail-closed:

const sameCoordinateLineage=incomingSid===currentSid||incomingParentSid===currentSid;
if(!currentSession||!Array.isArray(currentMessages)||!currentSid||!sameCoordinateLineage){
  return {...incoming,messages:incomingMessages};
}
...
if(incomingEnd!==messageCount) return null;

The truncated tool-call case is also no longer shape-set deduplication. static/messages.js:2090-2122 counts repeated unkeyed calls by (assistant_msg_idx, name), preserving the unmatched current-prefix occurrences and replacing only the incoming suffix occurrences. That matches the regression at tests/test_terminal_session_tail_and_merge.py:164-193, where four same-shape calls settle to two preserved prefix calls plus two authoritative incoming calls. The continuation case at tests/test_terminal_session_tail_and_merge.py:277-310 also verifies both message union and rebased tool indexes.

Diagnosis / recommendation

The implementation now preserves the pagination coordinate contract while removing the ordinary post-turn full-session payload. Failure cases still return null to the existing restore path rather than guessing. I would keep the current 30-visible/300-raw constants centralized in api/streaming.py; if they later become configurable, the client should continue treating only the offset/count metadata as authoritative.

Verification step

CI is green on the current head. I did not execute code from the PR worktree. For final manual verification, the highest-value cases are a long session ending normally, cancelling while viewing a paginated tail, and an auto-compression continuation with repeated legacy tool calls that lack IDs. Those correspond directly to the three coordinate branches reviewed above.

@starship-s
starship-s force-pushed the perf/bound-terminal-session-payload branch 4 times, most recently from e56acd5 to 648896b Compare August 12, 2026 23:14
@starship-s

Copy link
Copy Markdown
Contributor Author

Updated this branch onto current master because the terminal settlement and live-stream lifecycle paths had diverged. The refresh preserves the bounded terminal payload behavior alongside the newer replay/session handling; CI and Greptile are green on the updated head.

Assisted-by: Hermes Agent:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-luna
Assisted-by: Claude Code:claude-opus-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes-requested Maintainer left detailed feedback requesting changes; PR is waiting on author to address size:L Large PR (>10 files or >250 LOC)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants