Repository navigation
fix(chat): keep a folded run folded when the words before its tools join it - #2098
Conversation
…oin it A run's block and fold memory were keyed by its first part's index. When leading words joined the run (a later Codex record marks them commentary, or a blank commentary is filled in), the index moved and the folded block reopened. A run now keeps the key of its part that the row's memory holds a fold for. Fixes #2063
Keying a run by the remembered part it contained lost a merged block's fold once that part left the run (Codex review). Each run now keeps the name its parts' run had at the last draw (runNames); the row's memory records it at commit. A run only takes a fresh work:<start> name when it has none, and never one a run already shown holds.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 10 minutes. View limit details
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. Comment |
|
CI on 439678c: two failures, both wall-clock bounds in server tests this PR does not touch (it changes only client chat code and tests). I reran the failed jobs once.
Both browser regression jobs passed, and so did |
|
Rerun result: Integration tests passed (the OpenCode first-key bound was load). Fast checks did not run its tests: the job hit GitHub's 10-minute limit in its "Release guards" step (the first attempt took 3m15s in total), so I reran that one job again. |
|
Merging 439678c: omo-native-code-reviewer and Codex both pass this head, and the required checks are green (Fast checks passed on a rerun after its job was cancelled). CodeRabbit did not review this head. Codex's MEDIUM also happens on main, so it goes to a follow-up issue with the LOW notes. |
Change
Fixes #2063 (the MEDIUM follow-up from the merge-pass review of #2060).
A work block the reader folded opened again when the words before its tools joined its run: a
later Codex record marks unphased text
phase: "commentary", or a blank leading commentary getsits text.
Turn(src/components/ChatView.tsx) keyed each block, and its fold memory, by theindex of the run's first part (
work:<start>). When the words joined,startmoved, so theReact key and the memory key both changed and the block came back open.
Now a run keeps its name while its parts change.
runNames(src/lib/workBlocks.ts) gives eachrun the name its parts' run had the last time the turn was drawn. The earliest run claims a name
first, which keeps #2060's rule that the earlier half of a split keeps the name. Only a run with
no name gets
work:<start>, and never a name a shown run holds. The row's memory(
ChatRowMemory.runs, part index → name) records the names at commit, so they also survive therow scrolling out of the virtual list and back.
Design: no new UI. This extends the fold contract that #2060 set for runs that split to runs that
grow or shrink at either end (#2060's runs follow the Claude app's interleaved tool groups): a
block keeps the fold the reader gave it.
Not changed: the LOW items in #2063 (the stand-in's handover to the first real run, which has no
visible effect).
Tests
src/components/ChatTurn.test.ts, statefulreact-test-renderercases with a row memory:[text, tool]run, then redraws it with the textmarked commentary, and also with a blank commentary filled in. The block stays folded, and
stays folded again after a remount from the same memory. Fails on main.
[tool, answer, commentary]. The answer becomes commentary, so the runsmerge, and the reader folds the merged block. Then the last part becomes the answer, and the
block stays folded. Main passes it; the first commit of this PR failed it.
src/lib/workBlocks.test.ts:runNameskeeps a name while parts change, gives a split run'sfirst part the old name, and never hands a shown name to a new run.
scripts/chat-fallback-browser-qa.ts: the issue's reproduction in the real ChatView. On main'sChatView it fails at "words that join the run keep it folded" (
[true]); here the whole scriptpasses.
bun run check fast: green (7723 unit tests).Found along the way, also on main: a conversation without a
history_idkeeps its row slots byposition when its turns are replaced (
ChatView.tsxshowAnswer), so a row's memory goes to thenext turn shown at that position. The QA scenario places its turn on a row with no memory.
Reviews
runNames); the case is now the second ChatTurn test.