Repository navigation
fix(outcome): one process read per Chat poll, the stale-session minute pinned, and pi sessions over 16 MiB - #1936
Conversation
While a command sent from Chat was inside its window, each /api/pane/prompt poll of a Claude pane asked herdr for pane.process_info four times and for session.snapshot twice: once for the binding check, then again in the transcript lookup (paneRunsOmo twice, claudeProcesses). The poll now reads the processes once and hands that answer and its snapshot down to readCommandTranscript, paneTranscript and the Claude, pi, devin and unlabelled lookups. Fixes #1544
… send under herdr's label Polls at 59 s and 61 s after a session change typed in the terminal now pin STALE_SESSION_LAG_MS on both sides (the old test jumped to 11 minutes, so a 5-minute bound also passed). A send whose agent process could not be matched adopts a session herdr reports under the label it gave the pane at the send, so an OmO send (herdr names the pane pi) is no longer hidden when its own session appears. docs/agent-commands.md now states which session a row belongs to and its known limits: the minute is counted when Chat next asks, a resumed session's terminal-typed change within it, pid reuse outside Linux, and repeat ownership trusting herdr's history rows. Fixes #1549
… of hashing changed polls Once a pi session outgrew the 16 MiB window, the first entry inside the window was counted as a fork, so almost every append that slid the window changed the transcript's identity and observeTranscript skipped every send bound to the old one: a /model sent to such a session stayed accepted and ended unknown. Whether that first entry forks cannot be told from inside the window, so it no longer names the branch. A changed poll still checks every byte the cached and new reads share, but compares them instead of hashing both sides with sha256, and reads into an unzeroed buffer: on a 33 MiB pi transcript a changed poll's median went from 34-49 ms to 10-20 ms. Fixes #1487
… after a failed process read Review follow-ups. Leaving the first entry of a cut pi window out of the branch still changed the identity once a real fork slid to the head of the window, and a branch switch whose first entry headed the window kept the old identity, so the old branch's send could be completed from the new one. An append now keeps the branch it read before while that branch's last entry is still on the active path, and names the branch afresh otherwise. A poll whose own read of the pane's processes failed passed that as no processes, so a claude attach client went unseen and the transcript lookup took the session herdr's hook last reported. A failed read is now no answer, and the lookup asks herdr itself as before.
… digests instead of the window Second review round. An append now keeps the branch read before only when the new path runs on from that branch's last entry with no fork after it, or that entry was cut away with the window's start and is the parent of its first entry. A fork made between two polls after the old last entry is named afresh, and an append large enough to cut the old last entry out of the window no longer loses the branch. The window now starts on a 64 KiB boundary and the cache keeps an xxHash3 digest of each chunk instead of the window's bytes. A changed poll still checks every byte the two reads share against those digests, the old read's last chunk as far as it went. With four 33 MiB pi transcripts cached, the memory held went from about 190 MiB to 65-95 MiB.
…, now that the poll's snapshot is shared The poll hands its own session.snapshot to the transcript lookup (#1544), so a settled send no longer saves a snapshot; the lookup is told by the pane.process_info read the poll makes only while a send can still change. Forcing the transcript open makes the test fail.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Session matching and visibility server/prompt.ts, server/outcome.test.ts, docs/agent-commands.md |
Sends capture the pane’s agent label. Unmatched sends can adopt a session whose agent matches the sent agent or captured label. Tests cover session changes and the adoption behavior; documentation describes matching limits. |
Reuse poll snapshot and process data server/prompt.ts, server/conversation.ts, server/outcome.test.ts, server/outcome-transcript.test.ts, server/dialogs/hermes-outcome.test.ts, changelog.d/command-outcome-followups.md |
The prompt poll passes its snapshot and process lookup through command outcome and transcript resolution. Tests check request counts and the process-read failure case. |
Bounded transcript windows and Pi branch identity server/conversation.ts, server/outcome-transcript-budget.test.ts |
Transcript caching checks overlapping chunks using digests. Pi transcript identity persists across window advances when the active branch continues; tests cover appended results, forks, and clipped leaves. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
devswha/ovis#1879— The PR changes transcript reads and process-data reuse, but the issue notes that some readers and full-window reads remain.devswha/herdr-web-ui#1544— The shared process and snapshot data address the issue’s poll request-count objective.devswha/herdr-web-ui#1549— The PR tests the stale-session timing boundary and adopts unmatched sends using the pane label.devswha/ovis#1793— The Pi branch identity changes support matching outcomes to the correct transcript branch, but the PR does not add a visible receipt state.
Merge Risk: 
Minimal · up to e7558
-
No merge-blocking regression was established; the change is ready for normal checks.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Description check
-
Explanation
The description explains the problem, resulting behavior, affected issues, validation results, test coverage, and the absence of contract or workflow changes. It uses equivalent headings instead of the template's exact "Change" and "Validation" headings, but the required information is present.
Full details: Linked Issues check
-
Explanation
The PR implements the coding requirements for all three active issues. For
#1544, prompt polling shares one process read and one session snapshot with transcript resolution, and tests assert the call counts. For#1549, tests pin the 59/61-second stale-session boundary and unmatched OmO adoption, and the documentation records poll-time and label limitations. For#1487, Pi tests cover sliding windows, branch changes, leaf cuts, and later forks. The implementation uses 64 KiB-aligned chunk digests while retaining overlap validation. The reported test coverage and fast check support the existing outcome behavior.
Full details: Out of Scope Changes check
-
Explanation
The changed source, tests, documentation, and changelog support
#1544,#1549, or#1487. The shared-read refactoring supports the call-count objective. The Pi cache changes and regression tests support the large-session objective. The documentation and changelog describe the implemented outcome behavior and limits. No unrelated HTTP, WebSocket, authentication, or workflow changes are identified.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 6 files. (2 skipped: 2 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.
Comment @coderabbitai help to get the list of available commands.
|
Merge pass review-work report. Overall: PASSED. This is an ordinary server behaviour change, so it was gated by Codex Sol, an omo code reviewer and the merge pass's own checks.
Pi branch carry-over is correct. It applies only to an append from a resolved branch, and only in one of these cases:
Overlap check:
Process read: one xxHash3 is Tests:
Overlaps: Follow-ups:
|
…it (#1879) (#2146) Part of #1879: fixes the pi branch return case from the issue's 2026-10-24 comment and the test gap and docs wording from the 2026-10-25 comment. The remaining items stay open on the issue (listed below). ## What was wrong A Chat send to pi is bound to the transcript identity of the branch it was sent on (`observeTranscript` pins `transcriptStore` to the first identity it sees). That branch name was a hash of the forks visible in the window. When the session went to another branch and `/tree` came back, the returning entry looked like a fresh fork, the branch got a new name, and the send stayed `accepted` until `unknown`, even though its `model_change` was right there. ## What changes - `readCommandTranscript` (`server/conversation.ts`) remembers, for the last eight branches, the entry each was last read at (id and byte offset; carried only across byte-verified appends). A path keeps a remembered branch's name (`resumedBranch`) when that entry is on the path and, of everything written since: - nothing off the path is under that entry (an answer that went another way from it), and - nothing off the path is a record a send's result could be (`PI_EVIDENCE_TYPES`: `model_change`, `thinking_level_change`, `compaction`), so a result written on the visited branch cannot leave its send waiting on the old branch for a later identical send's record. Otherwise the branch is named afresh as before. The cut-window rule is unchanged (`runsOnFromCut`). - `transcriptEvidence` (`server/outcome.ts`) reads pi-family results only from `PI_EVIDENCE_TYPES`, so the list and the evidence reader cannot drift apart. - `docs/agent-commands.md`: the pi branch rule, the known limit below, and "except the last two" in the session-binding limits. ## Tests (`server/outcome-transcript-budget.test.ts`) - completes a pi send bound to a branch after the session leaves it and `/tree` comes back (#1879): fails on main (stays `accepted`), passes. - keeps a pi branch when the session goes to another branch and back to its last entry between two polls: fails on main, passes. - does not credit a pi send with a later identical send's result when its own went to the visited branch: the first review's HIGH; fails on the first commit, passes. - does not take a later entry that reuses a remembered branch's last id: the second review's MEDIUM; fails before afc4746, passes. - names a pi branch afresh on coming back when something else was written under its last entry. - names a pi branch afresh when an append cuts its last entry away and forks after the window's first entry: the test gap from #1936's review; removing `forks.every((fork) => fork === head)` makes it fail. `bun run check fast`: all steps ok, unit 7878 pass / 0 fail. ## Reviews - omo-native-code-reviewer: round 1 HIGH (a send credited with a later identical send's result after its own went to the visited branch) fixed in 34643a4; round 2 APPROVE, its LOWs fixed in 3d02f36. - Codex Sol 6.1 Fast xhigh: MEDIUM (duplicate id resurrecting a branch after the window slid) fixed in afc4746. Its HIGH (a send the agent dropped takes a later identical send's record) is main's existing send-order claiming: the same sequence with no branch switch gives the same result on main. Documented as a known limit and left on #1879. - Codex final round on the delta 34643a4..afc4746: APPROVE, no findings. ## Left on #1879 - That known limit (needs send-level ambiguity for pi-family records, which have no echo to tell sends apart). - #1544 leftovers: the Codex, omp and gjc lookups and `omoSessionForPane` still ask herdr themselves. - #1487 MEDIUM 2: a changed poll still reads the whole window (dropping that guarantee is a decision). - #1487 LOW items (unchanged).
Fixes #1544
Fixes #1549
Fixes #1487
Follow-ups to the command outcomes (#1543, #1450) that Chat shows under a command it sent. Seven commits: one per issue, the changelog fragment, two rounds of review fixes, and a Hermes test brought up to date with the shared snapshot.
What changes for the user
Command outcomes: keep resolving on pi sessions over 16 MiB, cheaper changed polls (#1450 follow-ups) #1487: pi sessions over 16 MiB. A command sent from Chat to a pi session larger than the 16 MiB window now shows its result. Before, it stayed "Accepted" until "No result seen" whenever pi kept writing. The first entry in a cut window was counted as a branch point, so almost every append changed the transcript's identity, and each send bound to the old identity was skipped from then on. On an append, the branch read before is now kept while the new path runs on from its last entry with no fork after it. That last entry may sit on the path or just before the window's first entry. Any other append, such as a
/treeswitch or a fork made since, names the branch afresh, as before.Command outcomes: keep resolving on pi sessions over 16 MiB, cheaper changed polls (#1450 follow-ups) #1487: cheaper changed polls. A changed poll still checks every byte the cached and new reads share;
validates the full overlapping tail within budget when the window slidespins that check against in-place rewrites. Three things changed:With four 33 MiB pi transcripts cached, the memory held went from about 190 MiB to 65-95 MiB. On 33 MiB with 16 KiB appends, a changed poll's median went from 34-49 ms on origin/main to 10-20 ms (busy host, 3 runs each). Reading only the appended bytes would drop that pinned full-overlap check, so it is left for Command outcomes: leftovers of #1487 and #1544 (low items, full-window reads, other agents' process reads) #1879, with the rest of Command outcomes: keep resolving on pi sessions over 16 MiB, cheaper changed polls (#1450 follow-ups) #1487's low items.
Command outcomes: one process read per Chat poll, not four #1544: herdr calls per poll. While a command waits for its result, each
GET /api/pane/promptpoll of a Claude pane asked herdr forpane.process_infoup to four times and forsession.snapshottwice. The poll now reads the processes once and hands that answer and its snapshot to the transcript lookup (Claude, pi, devin, unlabelled panes, and the OmO check). A read that fails is no answer, so the lookup then asks herdr itself as before; aclaude attachclient still leads to its job's session. The Codex, omp, gjc and OmO store lookups still read on their own, noted in Command outcomes: leftovers of #1487 and #1544 (low items, full-window reads, other agents' process reads) #1879.Command outcomes: pin the stale-session bound, and adopt unmatched OmO sends #1549:
STALE_SESSION_LAG_MS. The old test jumped to 11 minutes, so a 5-minute bound also passed. A mutation to 300 s or 30 s fails one of them.pilabel.docs/agent-commands.mdnow states which session a row belongs to, plus four limits: the minute is counted when Chat next asks, a terminal-typed change in a resumed session within it, pid reuse outside Linux, and repeat ownership trusting herdr's history rows (the F4 items).How it was verified
Each new test fails before its fix and passes after:
pane.process_infocalls before);claude attachtest, each against the commit before its fix.With the chunk-digest check forced to always match, five rewrite tests fail, including the pinned one.
Real surface:
server/outcome.contract.test.tsstarts the server, uses the isolatedovis-testherdr and runs a stand-inclaudein a real pty. It sends/contextfrom Chat over the WebSocket and readsGET /api/pane/prompt. A copy of it counts herdr RPCs during the first poll after the send: origin/main made 3pane.process_info, 2session.snapshotand 1agent.getcalls; this branch makes 1, 1 and 1. The outcome isresult-observedin both.server/outcome*,prompt*,conversation*andpi*: 752 pass. The one exception isconversation.test.ts"Devin pane identity > falls back when the explicit store is inaccessible or malformed", which timed out at 5.9 s on this busy host. It runs 4.1 s alone on origin/main and does not touch this change.bun run check fastpasses.Reviews: omo-native-code-reviewer and Codex (gpt-6.1-sol, xhigh) reviewed this.
claudewhile its Claude child runs could adopt the child's session, which hides the row and never shows a wrong result.After the rebase onto main (fix(chat): follow claude attach panes with options, an id prefix or a job name #1897's
claude attachoptions and fix(pi): read the context window from the pane's own PI_CODING_AGENT_DIR #1899's pi agent directory meet this change inserver/conversation.ts): Codex reviewed the rebase delta and found nothing. Its 19 scratch tests confirm that an attach client found through the shared process read still leads to its job's session and that pi's store and agent directory resolve the same with and without the shared read. The live contract QA above was rerun and still counts 1pane.process_info, 1session.snapshotand 1agent.get,result-observed.bun run check fastpasses on the rebased head.No HTTP or WebSocket contract, auth or workflow changes.