Skip to content

fix(chat): keep a tool you opened in place while GJC streams, and through its rewrites - #1937

Merged
devswha merged 10 commits into
mainfrom
fix/gjc-tool-streaming-viewport
Oct 25, 2026
Merged

devswha merged 10 commits into
mainfrom
fix/gjc-tool-streaming-viewport

Conversation

@devswha

@devswha devswha commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

What it does for the user

While a GJC turn streams tool calls into Chat, a tool you opened now stays open and stays where it is on screen:

  • Opening is reading. Opening a tool row, a folded work block or a tool's whole output stops following the end, just as scrolling up does. New records arrive below without moving what you are reading. Jump to latest (or New messages) brings you back to the end, and following resumes from there. A reader who opens nothing keeps following exactly as before.
  • GJC rewrites that keep the turns no longer reset the chat. gjc writes its whole session file again and renames it over the old one: on its first save, and when it trims old tool outputs (it does the same when it compacts). The server reads the new file as another history (history_id is the file's identity), so the chat started over: every opened detail folded, a reader's place was lost, and at phone width the end was no longer followed. Now a new history ID whose newest page still opens with every turn already shown, in the same places, keeps the rows, what was opened and where you read. Each shown turn's role and start time, each part's kind, and each tool's call id and input must be unchanged (gjc's tool parts now carry their toolCall id as call_id for this); what a tool answered may be trimmed. Anything else still starts over as before: a compaction that dropped turns or moved where the newest page starts, a new session, a cleared context, a rewind (even one that keeps a turn's tools' names), a moved branch, or a reader holding older pages.
  • A todo update you opened stays open. Since feat(chat): fold contiguous todo snapshots without losing attempts #1846, contiguous todo updates fold into one Plan row. An update you had opened turned into a closed Plan row as soon as the next update arrived, so what you were reading folded away. The Plan row now opens when you had opened one of its updates, with that update still open inside it. A Plan row nobody opened still starts closed.

Why

#1388: "the viewport alternates between the user message and the bottom" and "tool details that the reader expanded collapse when new records arrive". Two causes, both reproduced on current main with GJC-shaped records:

  1. ChatTimeline remembered a click on a disclosure only until the next rows change (manualLayout cleared on every rows update). Then ChatView's layout effect and the list's size callback followed the end again, dragging an opened running tool's header up 383–455 px on every record.
  2. gjc 0.18's session manager rewrites its file through a temp file + rename (rewriteEntries after compaction eviction and tool-output pruning; I read this from the installed gjc 0.18.12 binary). A rename is a new inode, so transcriptStream gives a new history_id and ChatView's read dropped the row slots, the row memories and the reader's intent.

Since #1831, a message sent from this browser's box follows the chat back to the end unless the reader navigated after it went out. Opening a tool now counts as that navigation too, as scrolling up does, so a send on its way no longer pulls the reader off a tool they opened. A gjc rewrite that continues the chat is not navigation (the reader's place is kept); any other new history still is.

Design: T3 Code took the other side for a reader at the very bottom (pingdotgg/t3code#17954 keeps a reader who toggles a tool there pinned to the bottom). Here the issue asks for the opened row to stay put while a turn streams, and Ovis already treats scrolling up as "stop following until I come back". Opening a detail joins that rule instead of adding a new one, so nothing new is drawn: the existing Jump to latest and New messages controls carry it. DESIGN.md's chat follow paragraph says so.

The server's history identity is unchanged. HTTP contract: GET /api/pane/conversation now gives a gjc tool part its own call_id, the optional field shared/protocol.ts already declares for Claude and Codex; omp and pi still carry none (server/api.contract.test.ts). CodeRabbit is rate-limited here, so a code-review agent's review stands in, and this needs a real review before merging. A byte-prefix check on the server would miss gjc's rewrites, because pruning changes earlier records. The continuity check runs only in the chat that already holds the turns.

How it was verified

  • New scripts/chat-gjc-streaming-browser-qa.ts (added to the browser lane). It feeds GJC-shaped message/toolCall/toolResult records one at a time through the real transcriptPage("gjc-transcript") parser and the real ChatView → ChatTimeline → LegendList path in Chromium, at 1440 and 390 px. Three readers are covered: one who opened a running tool, one following the end, and one scrolled up. Each runs with plain appends and with a gjc-style temp+rename rewrite. It samples every frame from each refresh until the view settles:
    • the opened tool's header and the scrolled reader's row stay within 1 px in every frame
    • the opened tool stays aria-expanded="true"
    • a follower ends at the end after every record
    • Jump to latest resumes following
  • On main (d3eefef, before this branch was rebased) it fails 62 assertions:
    • header displacement −383 to −455 px per record
    • aria-expanded="false" after a rewrite
    • the scrolled reader's row lost after a rewrite
    • the 390 px follower 179–205 px short of the end
    • in a chat that still fit the screen when the tool was opened, the opened tool dragged up to 1502 px once later records overflow it
  • On this branch every scenario holds at 0 px (20 scenarios, rebased on ddee8be). Current main (ddee8be) still fails 62 assertions.
  • Its todo scenario (added with the rebase) opens a gjc todo_write update and appends the next one, plainly and through a rewrite: the Plan row's head stays where the opened update's head was in every frame, it is open, and the update read stays open in it. Run against the commit before that fix, the script fails only those 16 todo assertions.
  • server/conversation.test.ts runs a gjc rewrite through the real parser: the same calls written again continue, and a rewind that reads the same file again under another call id, later, does not.
  • src/lib/chatRows.test.ts covers continuesHistory: continuation, a dropped/moved/retimed turn, a page that starts later (dropped first turns), a rewind that puts another call of the same tool in a shown place (other input or call id), another source, scrollback, empty and untimed pages.
  • Its jump scenario opens a tool and takes Jump to latest at once; closes and reopens it and jumps again; closes it while following, scrolls away and jumps back; and does that again but scrolls back to the end by hand. After each Jump the last row grows with no record between, and the end is followed. The script counts an answer as served only once the chat has read its body and React has committed after it.
  • scripts/chat-virtual-browser-qa.ts passes at 1440 and 390 px, dark and light (paging, pagination, virtualization, selection and disclosure restoration). The other chat browser scripts run in this PR's browser lane.
  • scripts/todo-snapshots-demo-regression.ts passes, and so do chat-local-send-follow-demo-regression.ts (fix(chat): follow your own accepted send back to the end unless you navigate #1831), chat-browser-qa.ts and chat-tool-paths-demo-regression.ts on the rebased branch.
  • bun run check fast passes: typecheck, build, generated files, workflow syntax, 6772 unit tests.
  • Reviewed by a code-review agent (no HIGH; its one real gap, a chat too short to scroll re-arming follow, is fixed in the second commit) and by Codex Sol 6.1 (no HIGH; three MEDIUM findings on the history continuation are fixed in the third commit: it applies to gjc only and needs each shown part's kind and tool to match, an older-page read in flight is dropped, and the kept reading place moves to the new history ID). A Codex re-review of that commit found no remaining HIGH or MEDIUM issue.
  • After the rebase onto current main, Codex Sol reviewed the rebase delta. The two conflict resolutions with fix(chat): follow your own accepted send back to the end unless you navigate #1831 were clean. It found one MEDIUM, the feat(chat): fold contiguous todo snapshots without losing attempts #1846 todo row folding an update the reader had opened, which the last commit fixes. Its re-review found one more MEDIUM (a remounted Plan row lost the openness it had inherited), fixed in the same commit; its 13-case repro passes.

Not covered: native iOS/Safari, a gjc compaction that drops turns (it starts over, as before), real gjc runs (the records are synthetic and the rewrite is simulated as gjc does it), and a reader who has loaded older pages when gjc rewrites. That reader still starts over, because those pages were read from the old file.

Fixes #1388

mp added 5 commits October 25, 2026 22:46
…ough its rewrites

Opening a tool, a folded block or a whole output is now reading intent, as
scrolling up is: following stops until the reader goes back to the end. Before,
the next record cleared the click's guard and every follow path pulled the view
to the end, dragging an opened running tool 380-460 px up per record.

gjc writes its whole session anew and renames it over the old file when it
compacts or trims tool outputs. The new inode gave a new history_id, and the
chat reset: every open detail folded, a reader's row was lost and a phone-width
follower stopped short of the end. A new ID whose newest page still opens with
every shown turn (same role and start time), with no older pages held, now
keeps the rows and the reader's state.

scripts/chat-gjc-streaming-browser-qa.ts feeds GJC-shaped records one at a time
through the real transcript parser and ChatView, appended and rewritten, at
390 and 1440 px, and checks every frame.

Fixes #1388
…croll

measureEnd turns following back on whenever the content fits the view, so a
reader who opened a tool early in a session lost the reading intent at once,
and the records that later outgrew the screen dragged the opened row again.
Opening something now holds through a short chat until the reader goes back
to the end.

The GJC streaming QA adds that reader (a chat with no earlier turns), and the
gamma call carries real narration so the chat outgrows the screen midway.
…flight older reads

Review follow-ups for the history continuation:
- Only gjc-transcript answers can continue, and every shown part must keep its
  kind and tool name: a pi branch move can keep a turn's role and start while
  replacing its tools, and positional disclosures would carry over.
- A continuation drops an older-page read still in flight; its answer names the
  old file and used to reset the chat after the continuation.
- The place kept for the pane moves to the new history ID, so a pane switch
  after a rewrite still restores it.
…d that a gjc rewrite keeps the chat

Opening a tool, a folded block or a whole output now joins scrolling up: following stops, and a local send already on its way no longer takes the reader to the end. A gjc rewrite whose newest page still opens with every shown turn keeps the rows, what was opened and the reader's place.
… it into a group

Since contiguous todo updates fold into one Plan row (#1846), a todo call the reader had opened became a closed group as soon as the next update arrived, and what they were reading folded away, also across a gjc rewrite the chat continues. The group now opens when the reader had opened any of its updates; a group nobody opened still starts closed.
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between ddee8be and 858a85f.

Files selected for processing: 14
  • DESIGN.md
  • changelog.d/chat-opened-tool-stays.md
  • scripts/chat-gjc-streaming-browser-qa.ts
  • scripts/chat-virtual-fixture.tsx
  • scripts/ci-browser.sh
  • server/conversation.test.ts
  • server/conversation.ts
  • server/transcript-records.ts
  • shared/protocol.ts
  • src/components/ChatTimeline.tsx
  • src/components/ChatView.tsx
  • src/lib/chatRows.test.ts
  • src/lib/chatRows.ts
  • tsconfig.tools.json

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Chat no longer follows the latest message when you scroll up or open a tool, folded block, or full output. New messages are indicated while you’re away, and you can return to the latest content with Jump.
    • Opened content stays open and in place as the conversation updates, including todo updates grouped in the Plan row.
    • Rewritten session files preserve your open content and reading position when the displayed conversation remains unchanged. New or altered history resets them.

GJC transcript parsing now retains tool-call IDs for history-continuity checks. Opening chat content changes follow behavior, and confirmed history continuations preserve reader position and row state. Browser QA covers transcript appends and rewrites at desktop and mobile widths.

Changes: Chat streaming behavior

Layer / File(s) Summary
GJC tool identity and history continuity
shared/protocol.ts, server/transcript-records.ts, server/conversation.ts, src/lib/chatRows.ts, src/lib/chatRows.test.ts, server/conversation.test.ts
GJC parsing includes tool-call IDs. continuesHistory checks whether shown GJC turns and tool parts match the next conversation; tests cover accepted and rejected continuations.
Opened content and reader position
src/components/ChatTimeline.tsx, src/components/ChatView.tsx, DESIGN.md, changelog.d/chat-opened-tool-stays.md
Opening content stops following and holds reader position. Confirmed history continuations preserve rows and position. Todo groups retain their open state across remounts.
Streaming browser validation
scripts/chat-gjc-streaming-browser-qa.ts, scripts/chat-virtual-fixture.tsx, scripts/ci-browser.sh, tsconfig.tools.json
Browser QA exercises transcript appends and rewrites at desktop and mobile widths. The fixture exposes answer metadata, and browser CI runs the new script.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

  • devswha/herdr-web-ui issue 1601: The change preserves an opened todo group when a new update joins the row; the issue’s proposed grouping and status model remain outside this change.
  • devswha/herdr-web-ui issue 1053: The change preserves opened todo-group state as updates arrive, but does not implement the full tracking issue.

Suggested reviewers: yoonwoo-ha

Merge Risk: Minimal · up to 858a8

The chat streaming changes are mergeable after normal checks; no concrete blocking issue was established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 11 files. (3 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main change: a tool the reader opened stays in place while GJC streams, including through GJC's file rewrites. It matches the raw summary's chat follow and history-continuation cha…
Description check Passed The description follows the template. It has a Change section that describes the problem (#1388), the cause, and the resulting behavior, and it links the issue. It has a Validation section that lists …
Linked Issues check Passed [Issue #1388] The PR adds reader-navigation handling for opened tools, folded blocks, and outputs. It preserves expanded row state and viewport position during streaming. continuesHistory preserves …
Out of Scope Changes check Passed The changes stay within [Issue #1388]. Source changes implement chat follow behavior, disclosure-state retention, GJC history continuity, todo grouping behavior, and the required tool-call identity. T…
Full details: Title check

Explanation

The title names the main change: a tool the reader opened stays in place while GJC streams, including through GJC's file rewrites. It matches the raw summary's chat follow and history-continuation changes, and the objectives. It is specific and is not vague. The title is long (86 characters, 15 words) but the scope is clear.

Full details: Description check

Explanation

The description follows the template. It has a Change section that describes the problem (#1388), the cause, and the resulting behavior, and it links the issue. It has a Validation section that lists the browser QA scripts, unit tests, and the check run, and it covers the HTTP contract change with contract-test coverage (server/api.contract.test.ts). The description is long and detailed, but it is complete.

Full details: Linked Issues check

Explanation

[Issue #1388] The PR adds reader-navigation handling for opened tools, folded blocks, and outputs. It preserves expanded row state and viewport position during streaming. continuesHistory preserves state only for compatible GJC rewrites and rejects dropped, moved, retimed, rewound, or other-source histories. The parser carries GJC tool call IDs for this check. Todo grouping preserves an opened update inside the Plan row. The reported browser QA covers append, rewrite, grouping, pagination, virtualization, desktop, and mobile cases. The reported unit, parser, browser, and fast-check suites pass.

Full details: Out of Scope Changes check

Explanation

The changes stay within [Issue #1388]. Source changes implement chat follow behavior, disclosure-state retention, GJC history continuity, todo grouping behavior, and the required tool-call identity. Tests, fixtures, CI wiring, documentation, and the changelog directly verify or document these behaviors. No unrelated product behavior is demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 11 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

CodeRabbit hit its review limit on this PR ("Review limit reached"), so no CodeRabbit review ran. The reviews that did run, and how each finding was resolved:

  • Code-review agent (omo-native-code-reviewer), on the original fix before the rebase: no HIGH. Its one real gap was a chat too short to scroll re-arming follow after a tool was opened. Fixed in 71da058, and covered by the QA script's short scenario.

  • Codex Sol 6.1 (xhigh), on the original fix: no HIGH, three MEDIUM, all fixed in 7d60886:

    • continuation is gjc-only and needs every shown part's kind and tool to match;
    • an older-page read still in flight is dropped;
    • the kept reading place moves to the new history ID.

    A delta re-review found nothing left.

  • Codex Sol 6.1 (xhigh), on the rebase onto current main:

The PR touches no auth, takeover, queued input, HTTP/WS contract or workflow.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Merge pass review of 6930a39: Codex (gpt-6.1-sol, xhigh) asks for changes. The code review is still running. 132 focused unit tests pass.

MAJOR: a rewind can be mistaken for the same history (src/lib/chatRows.ts:75)

  • The continuation guard compares tool names but not their inputs.
  • Repro with the real parser: replace a later read(beta.ts) with read(gamma.ts) at a new timestamp. Both calls merge into an assistant turn that keeps its original start time, so continuesHistory returns true.
  • The kept positional tool:1 state then opens the replacement tool as if the reader had opened it.
  • Needed: compare a stable tool identity or its input fields, not just the name, and add this counterexample as a test.

MINOR: Jump to latest leaves the layout guard on (src/components/ChatTimeline.tsx:85)

  • Opening a tool sets manualLayout, which clears only when the rows change. Jump to latest brings following back without clearing it.
  • So until the rows are replaced, onItemSizeChanged stays suppressed. A pending whole-output response or a late image can grow below the viewport after Jump and not be followed.
  • This reproduced at the component level.

MINOR: the browser QA wait doesn't prove the response was drawn (scripts/chat-gjc-streaming-browser-qa.ts:101)

  • qaBytes advances when fetch returns the headers, which is before the body is parsed and React commits. Ten stable frames can therefore validate the old DOM.
  • An isolated reproduction of that helper returned after 160 ms without a render.
  • Needed: wait for a committed response or version marker before counting frames.
  • No fixed sleeps, but this synchronization lets the test pass when it shouldn't.

The merge pass will recheck only the new commits once these are fixed.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Merge pass, the second review: the code review (omo-native-code-reviewer) passed.

  • Browser script: the new script passes all 20 scenarios with 0 px movement. With the fix reverted, it fails 89 assertions.
  • Follower: a reader who has not opened anything behaves as before.

Codex's MAJOR above still blocks the merge. With the real parser, continuesHistory takes a rewind that keeps the tool names but changes their inputs for a continuation of the same history. One MEDIUM to handle in the same fix:

M1 (src/lib/chatRows.ts:67-77):

  • The gap: continuesHistory lines turns up by position from the first one. A rewrite that also moves where the newest page starts is therefore treated as a new history, and the chat starts over, as on main. Two such rewrites: a compaction that drops the first turns of a short session, or a rewrite that lands past the 50-prompt page start (server/conversation.ts:387-391).
  • Why it matters: this fails safe, with no stale rows. But DESIGN.md:1858-1860 and the changelog say a compaction keeps the chat, and the 7-prompt QA fixture never tests that.
  • Fix: either match by the first shown turn, comparing tool inputs as Codex asks, or narrow the claim to trimmed outputs.

mp added 2 commits October 25, 2026 23:12
… call and its input

A rewind that kept a turn's start and its tools' names but replaced a call (read beta.ts became read gamma.ts) passed for the same history, and the kept positional state opened the replacement as if the reader had opened it. Each shown tool's call id and input must now match too. A rewrite that moves where the newest page starts, such as a compaction that drops turns, still starts the chat over, so DESIGN.md and the changelog now claim only rewrites that keep the shown turns (trimmed tool outputs).
Opening a tool held the list's layout guard until the rows changed, so after Jump to latest a whole output or a late image growing below was not followed. Opening already stops following, so it now drops the guard, including one an earlier close left; a click a following reader makes still sets it. The GJC streaming QA gains a jump scenario (open, Jump, close and reopen, Jump, then a row grows), and counts an answer as served only once the chat has shown that very answer (its metadata comes back through onMetadata) and React has committed after it, not when its headers arrive.
@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Fixed in f88fa1b and eea0701, pushed together.

MAJOR: a rewind taken for the same history (src/lib/chatRows.ts)

  • A continuation now also needs each shown tool's call id and input to match, not only its name.
  • src/lib/chatRows.test.ts adds the counterexample: the same tool in a shown place with another input (read beta.ts → read gamma.ts) or another call id. That test fails with the old comparison.

M1: compaction (continuesHistory is positional)

  • I took the second option and narrowed the claim. DESIGN.md, the changelog fragment and the PR body now promise only rewrites that keep the shown turns in place: gjc's first save and trimmed tool outputs.
  • A compaction that drops turns or moves where the newest page starts still starts over, as on main. No stale rows are kept.
  • chatRows.test.ts also covers a page that now starts after the first shown turn.

MINOR: Jump to latest left the layout guard on (src/components/ChatTimeline.tsx)

  • Opening already stops following, so an opening click no longer sets manualLayout, and it drops a hold an earlier close left. Any other click a following reader makes still sets it, as before.
  • The QA script's new jump scenario covers this: open a tool, Jump, close it, reopen it, Jump, then the last row grows 240 px with no record. The end is followed.
  • On the previous heads the scenario fails: bottom 240 at 1440 and 390 px.

MINOR: the browser QA wait (scripts/chat-gjc-streaming-browser-qa.ts)

  • An answer now counts as served only once the chat has shown that very answer and React has committed after it.
  • The fixture passes onMetadata and reports each shown answer's metadata object. The script maps every parsed body's metadata to its transcript size.
  • Neither headers nor a read the chat dropped advance the wait.

Codex Sol on these commits, before the push. Two LOWs were fixed in eea0701: the hold left by a close-then-reopen, and a dropped read advancing the wait. One MEDIUM I left out of this PR:

  • gjc tool parts carry no call_id. server/api.contract.test.ts pins that the omp/pi/gjc parser exposes none, so adding it is an HTTP contract change that needs its own review.
  • What remains is a rewind that replays an identical call: same tool, same input, same turn start, same place. The row then shows the same call.
  • Codex's re-check agreed there is no concrete harm. Its final verdict: no HIGH, MEDIUM or LOW findings.

Verified on eea0701:

  • GJC streaming QA: 22/22 scenarios, 0 px in every frame.
  • chat-virtual-browser-qa and chat-browser-qa pass.
  • bun run check fast passes.

Verified on 8e1bdda08 (the same fixes, before the two LOWs): chat-local-send-follow-demo-regression, todo-snapshots-demo-regression and chat-selection-copy-regression pass. The browser lane reruns them on this head.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Merge pass re-review of eea0701 (Codex, changed range 6930a39..eea0701). Changes still requested. Two findings are only partly fixed. The browser QA wait (MINOR) is fixed, and the M1 docs are corrected. The focused tests pass: 11 in chatRows and 97 related.

1. MAJOR, partly fixed: a tool with the same input but a different call (src/lib/chatRows.ts:77)

  • The read(beta.ts) → read(gamma.ts) counterexample now correctly returns false.
  • But the GJС parser never sets call_id (server/transcript-records.ts:370). So the new check compares undefined === undefined.
  • Replacing call c2 with c3 at a later timestamp, with the same input, still returns true. The replacement tool then inherits the open state of the tool that was at its position.
  • Needed: compare something GJС actually records for each call. Either have the parser carry the call's id, or include the tool's own timestamp in the comparison. Add the c2 → c3 same-input case as a test.

2. MINOR, partly fixed: close a tool, then Jump (src/components/ChatTimeline.tsx:94)

  • Open a tool, Jump, then let new output arrive: that path now follows correctly.
  • Still failing: close a tool while following, scroll away, then Jump, with no rows replaced in between. Closing sets manualLayout again, and Jump leaves it set.
  • A component reproduction confirmed that later onItemSizeChanged calls request no scrolling.
  • Needed: clear the guard on Jump whatever set it.

The merge pass will recheck only the new commits.

mp added 2 commits October 25, 2026 23:31
gjc's tool parts carried no call id, so continuesHistory compared undefined with undefined: a rewind that called the same tool with the same input again, under another id and later, in a turn that kept its start, passed for the same history and the replacement inherited the opened state at its place. The parser now gives gjc tool parts their toolCall id (call_id; omp and pi still carry none, as api.contract.test.ts pins). server/conversation.test.ts runs both cases through the real parser: the same calls written again continue, the c2 -> c3 rewind does not.
…atever set it

A tool closed while following set the timeline's layout hold; scrolling away and coming back to the end, with Jump to latest or by hand, left it set, so what grew next was not followed until the rows changed. ChatView now owns the hold and drops it whenever following resumes: Jump to latest, New messages, an own send, or the reader scrolling back to the end. The GJC streaming QA's jump scenario adds both: close while following, scroll away, then Jump or scroll back, then a row grows.
@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Both partly-fixed findings are now fixed in e9f5365 and 858a85f, pushed together.

1. MAJOR: same input, different call (e9f5365)

  • I took the first option: the parser now gives gjc tool parts their toolCall id as call_id (server/transcript-records.ts, option callIds, set only for gjc-transcript in server/conversation.ts). continuesHistory now compares real ids, so the c2 → c3 rewind returns false.
  • server/conversation.test.ts runs both cases through the real parser. The same calls written again continue; c2 → c3, later, with the same input, in a turn that keeps its start, does not. With the option off, that test fails.
  • HTTP contract: this adds an optional field to gjc tool parts in GET /api/pane/conversation. shared/protocol.ts already declares the field; its doc now names gjc. omp and pi still carry none, so server/api.contract.test.ts still passes (163 tests). The PR body says this needs a real review.
  • One visible side effect: a gjc todo group's id now comes from its first call's id (workBlocks.ts todoGroups), so a group someone opened before the upgrade shows closed once.

2. MINOR: close a tool, scroll away, then Jump (858a85f)

  • ChatView now owns the layout hold, and ChatTimeline takes it as a ref. It is dropped whenever following resumes: Jump to latest, New messages, an own send, or the reader scrolling back to the end by hand.
  • The QA script's jump scenario adds both sequences: close while following, scroll away, then Jump or scroll back, then a row grows with no record in between. In both, the end is followed.
  • With the new script, the previous head eea0701 fails exactly those assertions (bottom 240 at 1440 and 390 px).

Stand-in for CodeRabbit. CodeRabbit is rate-limited on this PR, so a code-review agent (omo-native-code-reviewer) reviewed e9f5365 and the Jump commit before it was amended. It approved: no HIGH or MEDIUM. Its two LOWs:

  • L1: scrolling back to the end by hand left the hold set. Fixed in 858a85f, with a QA step.
  • L2: no HTTP-level test pins the gjc call_id. Not added: the parser-level test above covers it, and the contract test's fixture has no gjc pane.

Verified on 32c52fbfb (before the L1 amend):

  • GJC streaming QA 22/22, 0 px in every frame.
  • api.contract.test.ts, chat-virtual-browser-qa, chat-local-send-follow-demo-regression, chat-browser-qa, todo-snapshots-demo-regression, chat-selection-copy-regression and bun run check fast pass.

Verified on 858a85f:

  • GJC streaming QA 22/22, 0 px in every frame, including the jump scenario's Jump and by-hand returns.
  • With the same script, 32c52fbfb fails only the by-hand step (bottom 240 at both widths).
  • chat-virtual-browser-qa, chat-local-send-follow-demo-regression, chat-browser-qa and bun run check fast (6774 unit tests) pass.

…leaves on its own

Opening a run is reading there now (#1388), so after the snapshots open the Plan rows the chat offers Jump to latest while already at its end, and takes it back as soon as a layout change scrolls there. The helper had resolved the button, saw it detach before its click, and timed out after 30 s in CI's evidence pass (390 px, before the narration-boundary shot). It now presses the button only if it is there at that moment, and still requires it gone afterwards.
@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Merge pass: pulled from the merge queue, because CI at 858a85f fails a browser regression this change probably caused.

  • The failure: scripts/todo-snapshots-demo-regression.ts:173 fails twice in a row with TimeoutError: click: Timeout 30000ms exceeded. The step is toEnd(), which clicks .chat-virtual-jump and then waits for the button to be detached.
  • Why this PR: main's CI passed this script on e430be7. This PR changes when following stops and resumes (opening a tool stops following; Jump to latest clears the layout hold). So Jump to latest may now be covered by a card, may never detach, or may not bring following back in that script's flow.
  • Separate flake: terminal-arrows-clicks-regression failed once and passed on its retry, so it is unrelated.
  • Next step: please run bun scripts/todo-snapshots-demo-regression.ts locally on this branch and fix the cause. After the fix, the merge pass will recheck only the new commit and put the PR back in the queue.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Fixed in de1105a (test-only, scripts/todo-snapshots-demo-regression.ts toEnd()); Jump to latest itself works as designed.

Cause. CI runs this script with UI_EVIDENCE_DIR set, and only then does shoot(end=true) call toEnd(). My earlier local runs did not set it, so they never reached that step. With it set, the failure reproduces locally at 858a85f, eea0701 and 756d8dd, and does not on main. A frame trace at 390 px light, just before the narration-boundary shot (one frame per animation frame), showed:

  • Frame 0: the "New messages" button is shown with the view already at its end (0 px left). Opening the Plan rows is reading intent in this PR (Chat: GJC tool streaming moves the viewport and loses expanded tool details #1388), so the reader counts as away and the button is offered.
  • Frame 1: content grows 237 px above, the scroll position follows it (still 0 px), and the resulting scroll event re-arms following ("reached the end scrolling down"). measureEnd then clears away/new messages and the button is removed. It is never added back in the 3 s that follow.

Playwright had already resolved the button, saw it detach before the click, and kept retrying an element that no longer existed until the 30 s timeout. On main, opening kept following, so toEnd() found no button and skipped the click. That is why main passes.

So the button was not covered, it does detach, and following does come back: it came back on its own before the click. The helper now clicks the button through the DOM only if it is there at that instant (the query and the click happen in the same step, so no resolve-then-detach race remains). Either way it still requires .chat-virtual-jump to be gone within 10 s, meaning following is armed and nothing floats over the last rows.

Verified at de1105a, all through dori heavy:

  • todo-snapshots with UI_EVIDENCE_DIR: PASS, 20/20 scenarios plus shots, twice. Without it: PASS.
  • gjc streaming QA: PASS.
  • chat-virtual: PASS 3/3. One earlier run failed scrollbar-unseen-release at load average 26; the product code is identical to 858a85f, where it passed locally and in CI.
  • bun run check fast: PASS (unit 6774).

Codex (gpt-6.1-sol, xhigh) on the delta: "No HIGH or MEDIUM findings." It confirmed the pill's appearance and removal match the follow logic. It noted one LOW: an absent pill does not prove exact end geometry (the 48 px follow allowance). That weakness predates this commit and is how the helper worked on main, so I left it unchanged.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

CI is green at de1105a: Browser regression (other scripts) now passes, along with ui-regression, Fast checks, Integration and Native Windows install. Ready for the merge pass to recheck the new commit.

@devswha

devswha commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Merge pass review-work report. Overall: PASSED. This is an ordinary client change, gated by Codex Sol over three rounds, an omo code reviewer and the merge pass's own checks.

Lane Head Verdict
Code review (omo-native-code-reviewer) 6930a39 PASS. The new browser script passes 20/20 scenarios with 0 px movement, and fails 89 assertions with the fix reverted
Codex round 1 6930a39 CHANGES: a rewind was taken for a continuation (same tool names, different inputs); Jump left the layout guard set; the QA wait didn't prove a render
Codex round 2 eea0701 CHANGES: GJС had no call_id, so a different call with the same input still continued; closing a tool, scrolling away and then Jump left the guard set
Codex round 3 858a85f APPROVE

Final head:

de1105a (test only) fixes the CI failure at 858a85f: todo-snapshots-demo-regression.ts:173 timed out clicking Jump to latest. With this PR, a reader who opened a run stays where they were reading, so the button can show while the chat is already at its end, then leave by itself once a layout change scrolls there. The code review records this as intended (L2). The script now presses the button only if it is there at that moment, then waits for it to leave. The PR's code is unchanged.

CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it.

@devswha
devswha merged commit f6db8ee into main Oct 25, 2026
19 checks passed
@devswha
devswha deleted the fix/gjc-tool-streaming-viewport branch October 25, 2026 17:37
devswha added a commit that referenced this pull request Oct 28, 2026
## What it does for the user

Chat now keeps following new output to the end when part of the
transcript folds on its own. Before
this, a turn settling at the end (its work folding together with a block
above it), or a block above
the view folding while the view was still gliding to the end, could stop
following mid-transcript and
show **New messages**, with no scroll by the reader.

Fixes #2042. Reported by @odfalik.

## Cause

`onScroll` in `src/components/ChatView.tsx` treats an upward move with
no reader input that lands past
the 48 px layout allowance as the reader leaving the end: it sets
`pagingAway` and drops the list's
pending scroll to the end. LegendList (`maintainVisibleContentPosition`)
and the browser make such
moves themselves when the transcript shrinks. The list holds the rows on
screen where they were, and
the browser keeps the view inside a shorter transcript. For one shrink
both can move the view up, so
it lands hundreds of pixels from the end, or past the end of a follow
glide that was still on its way.

## Change

- Without reader input, an upward move during which the transcript got
shorter (the last row moved
up in it, or its height dropped since the previous scroll event) now
counts as the layout's: following
goes on, and a view left more than 48 px short of the end is put back
there.
- That pin-back is skipped while `pagingAway` is set and while a
following reader's own click is laid
out where they are (ChatTimeline `manualLayout`), as the timeline's own
follow paths already do.
- Over a transcript that did not shrink, nothing changes: a no-input
scroll (a screen reader's, a
Firefox scrollbar's) leaves the end once the view is past 48 px from it,
however small its steps
(#1902). Reader input (wheel, keys, touch, a scrollbar drag) is
unchanged.
- The new scroll baselines reset with the chat's other scroll state on a
pane change or a history
  replacement.

Known trade-off: a no-input reader scroll that lands in the same scroll
event as a fold is put back at
the end once; the next move leaves as before. During a turn that keeps
folding this can happen more
than once. The page has no signal that tells such a scroll from the
layout's own move in that event.

Design precedent: none of the coding-agent apps exposes how it tells its
own re-layout from a reader's
scroll; this follows the rule #1734 set and #1902 kept, that only input
that scrolls the transcript up
is the reader's intent.

## How it was verified

- `scripts/chat-virtual-browser-qa.ts` (real `ChatView` in Chromium,
1440 and 390 px, light and dark,
  already in CI's browser lane) gains:
- `fold-together`: at the end, the last row and a row above fold in one
layout (a turn settling).
Following goes on to the end and the next output is followed. **Fails on
main**
    ("fold-together: the next output is followed"), passes here.
- `fold-above` / `grow-above`: a row above folds or grows while the view
glides to the end after the
last row grew. These pass here; on main they fail only when the fold
lands in the same frame as the
    glide, so they do not fail on every main run.
- `wheel-up`: a real upward wheel (`page.mouse.wheel`) still stops
following ("Jump to latest", then
    **New messages** with the reader left away from the end).
- Two 30 px upward steps without input from the end stop following past
the allowance. This catches
    the first commit's regression found in review.
  - Full script: 60 PASS over all four width/theme combinations, exit 0.
- `scripts/chat-local-send-follow-demo-regression.ts` (#1831),
`scripts/chat-gjc-streaming-browser-qa.ts`
(#1388/#1937) and `scripts/composer-fit-demo-regression.ts` (#1431):
pass on the final commit.
- `bun run check fast`: green on the final commit.
- Reviews: Codex (gpt-6.1-sol, fast, xhigh) and the omo code reviewer.
Both blocked the first commit on
one finding: the 48 px allowance applied per scroll event, so small
no-input steps were pinned back
(a #1902 regression). The second commit measures it from the end again.
The omo reviewer's other
findings were fixed: the `manualLayout` guard and the baseline resets.
Codex re-reviewed the fix:
  APPROVE.

Not reproduced on a real Firefox or with a screen reader. Once, during
QA, a real wheel landing early in
a glide did not stop following; it did not reproduce in six later runs
(three on main, three here) and
this change does not touch moves made with reader input.
devswha added a commit that referenced this pull request Oct 28, 2026
… each update's own input and output (#1601) (#2161)

## Change

Fixes the two gaps #1601 still had after #1846 folded contiguous todo
updates into one Plan row.

- **Focus survives the fold.** When the agent's second todo update
turned the single row the reader was on into a Plan row, the focused
button left the page and the keyboard focus fell to `<body>` (expansion
already carried over since #1937). A row head now records in its
timeline row's memory that it holds the focus (`useRowFocus`,
`src/lib/chatRowState.tsx`). The head drawn in its place in the same
render takes the focus back: the same update inside the opened run, or
the run's own head when it is folded. Focus the reader moved elsewhere
is never taken, a head that leaves with no replacement ends its record
(a row scrolled back or reopened later never grabs the focus), and a
fold while the reader is in another app keeps it too.
- **Each attempt shows its own call.** An update opened inside the Plan
row showed the parsed list again, which the run already shows above it.
It now shows the input the agent sent (JSON indented, under "Input") and
the output it got back, including "Show the whole output" for a cut one.

Design: the Codex and Claude apps keep one row per action under the turn
and keep the reader's place while a turn streams. This follows that, as
#1846's DESIGN.md entry does; the entry now records both rules. "Input"
is translated into Korean, Japanese and Chinese. A `changelog.d/`
fragment is included.

Closes #1601.

## Validation

- Regressions that fail before and pass after:
- `scripts/chat-gjc-streaming-browser-qa.ts` (browser lane): its todo
scenario now asserts the focus is on the opened update inside the Plan
row after the next update arrives. On main all four variants (1440/390 ×
append/rewrite) failed with the focus on BODY; on this branch all pass.
- `src/components/ChatTurn.test.ts`: an opened two-update run shows each
update's raw input and output.
- `scripts/todo-snapshots-demo-regression.ts`: 21/21 pass (1440/390,
light/dark, Korean), including "open, focused run through append and
poll".
- `bun run check fast`: green on b4d51fd (unit tests 7886 pass, 0
fail). On the final head 12c0c34 (one line added to `useRowFocus`),
typecheck, build and generated files pass; the unit tests had one
timeout in an untouched file, `scripts/ci-browser.test.ts` "creates
evidence under .ci when CI has no CHECK_DIR" (5029 ms on a 5 s limit at
host load ~66). Rerun on its own, it passes in 3.4 s (7/7).
- Not covered by an automated test: the background-window case (headless
Chromium always reports the page focused) and StrictMode's rehearsed
unmount (the browser fixture renders without StrictMode). Both were
reviewed at source level and confirmed by Codex's last round.
- Reviews: omo-native-code-reviewer PASS (its MEDIUM, a window blur
ending the record, fixed; the over-long DESIGN.md line fixed; other LOWs
out of scope). Codex Sol 6.1 Fast xhigh: round 1 MEDIUM (a stale record
after a head left with no replacement) fixed; round 2 MEDIUMs
(StrictMode rehearsal, focus events deferred while the window is in the
background) fixed; the confirmation round on the final head PASS with no
findings.
devswha added a commit that referenced this pull request Oct 28, 2026
## What

OMO and GJC transcripts use omp's record format and fold, but were still
left out of the live incremental parser: every poll of a growing OMO/GJC
chat parsed the whole running task again. This routes `omo-transcript`
and `gjc-transcript` through `incrementalLiveTurn`, as Codex, OMP and
Claude already are, with the same committed-span SHA-256 check,
page/generation invalidation and detached snapshots.

Closes #1643. Based on #1634 by @Yoonwoo-Ha (closed while the refactor
landed), ported to current main.

- `ompReading()` gives the omp fold its options for both the one-shot
and the live parse, so they cannot drift: since #1634, GJC tool parts
carry their call ids (#1937 matches gjc's rewrites by them), and that
option must hold on the live path too.
- **Second commit:** OMO/GJC live entries get a cache budget of their
own (8 entries / 32 MiB of source bytes, LRU, the same size as the
existing one). With one shared budget, OMO/GJC entries pushed OMP,
Claude and Codex chats read in turn out of the cache: two 13 MiB OMP
chats plus one 11 MiB OMO chat made every OMP poll about 5x slower than
main, Claude chats up to 9x. With the split, Codex/OMP/Claude keep
exactly main's cache behaviour.
- The fixes #1643 asked for: a `JSON.parse` count pins the speed-up (4
parses per appended record instead of 205 after 101 records); the
discarded OMO title preview names its own task, so a leak would show;
the stale comments are corrected. Plus tests for gaps a mutation run
found: omo/gjc name no pictures and only gjc carries call ids, on both
the live and the settled path; a growing in-place rewrite drops a title
only the old bytes gave; and both budgets evict least-recently-read
entries within themselves.

## Measured (copies of real transcripts, median per poll of a growing
live task)

| Chat | main | this PR |
|---|---|---|
| OMO, ~2 MiB live task | 7.4 ms | 2.9 ms |
| OMO, ~11 MiB | 36 ms | 12 ms |
| OMO, ~15 MiB | 52 ms | 16 ms |
| GJC, ~2 MiB | 10.8 ms | 2.4 ms |
| GJC, ~11 MiB | 64 ms | 12 ms |

Quiet-host figures; under load the ratio held at 0.15-0.21 of main. A
cold first read and a live task over the 16 MiB window are unchanged.

## Verification

- **Differential, warm read vs a cold read of the same bytes**
(independent Bun Worker readers): over 85k reads replaying 23 OMO, 22
GJC and 6 OMP real transcripts (copies; cut lines and UTF-8 characters,
valid tails without a newline, same-size edits, truncation, gjc-style
tmp+rename rewrites, context clears, held pages); about 140k-180k
synthetic fuzz reads; and 60k reads under eviction pressure in both
budgets. Everything that differed from a cold read is a class that
already differs the same way on main. The one exception is listed below.
- **Cache behaviour**: over thousands of Codex/OMP/Claude polls (mixes
with OMO/GJC chats, either arrival order, chat switching, paused chats),
the `JSON.parse` count and cache membership matched main on every poll.
`liveTurnCacheUsage()` stayed within budget on every read, and no
OMO/GJC read ever evicted a Codex/OMP/Claude entry.
- **Contract tests** in an isolated herdr session: omo, gjc,
gjc-native-live (with the installed gjc binary), omo-detached, omp,
conversation-push and claude-session: 50 passed, 0 failed.
- `bun run check fast`: 8345 passed, 0 failed.

## Known limits

- Three or more OMO/GJC chats with live tasks of about 11 MiB read at
the same time exceed their own 32 MiB budget and evict each other. Each
miss costs about 15% more than main's reparse (the hash and the cloned
snapshot). This is the thrash #1643 lists as a follow-up, and main has
the same for Codex/OMP/Claude. A time-based admission rule was tried and
dropped: it brought arrival-order effects, a chat-switching penalty,
response-cache hits that never refresh its clock, and a wall-clock
dependence.
- The live cache's bound becomes 16 entries / 64 MiB of source bytes. An
entry keeps normalized turns: 1-2 MiB of heap per 11 MiB tool-heavy
task, up to about its source size for tasks of thinking and text.
- One cosmetic difference, accepted, and one OMP already has on main:
after the settled-page cache evicts a page while the live entry
survives, the live OMO/GJC fold can keep a task title (or a sticky
context window) that a cold read no longer finds.
- pi and Command Code transcripts keep reparsing their live task;
unchanged.

`server/` is a remote bundle input, so the release that ships this
raises `REMOTE_BUNDLE_VERSION`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Yoonwoo-Ha <274636136+Yoonwoo-Ha@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

Chat: GJC tool streaming moves the viewport and loses expanded tool details

1 participant