Repository navigation
fix(outcome): keep a pi send's branch when the session comes back to it (#1879) - #2146
Merged
Merged
Conversation
…it (#1879) A pi send is bound to the branch name read at its first poll. That name was a hash of the forks seen in the window, so when the session went to another branch and /tree came back to the send's branch, the returning entry looked like a fresh fork, the branch got a new name, and the send stayed accepted until unknown. The command transcript cache now remembers the node each of the last eight branches was read at. A path that runs on from such a node keeps that branch's name when every node written under it since is on the path; anything written under it another way still names the branch afresh. The cut-window rule is unchanged and now has a test for its fork guard; the docs say which known limits show a wrong result.
…nwhile Review follow-up. A send whose own result was written on the branch the session visited kept waiting on its old branch after /tree came back, and took a later identical send's record there. A remembered branch now resumes only when nothing written off the path since its read is a record a result could be (PI_EVIDENCE_TYPES, the types transcriptEvidence reads); otherwise it is named afresh as before, and the first send ends as unknown.
Review follow-up: transcriptEvidence now reads a pi, omp, gjc or OmO send's result only from the record types the branch check refuses to resume over, so the two cannot drift apart, and its doc comment is back on the function.
…t only its id Review follow-up. An entry written later with the id of a remembered branch's last entry, once the first had slid out of the window, resumed that branch, so an older send bound to it could take a newer send's record. The remembered node now carries its byte offset, and only that entry resumes the branch. The docs also state an existing limit the review surfaced: a pi-family send the agent dropped without a record takes a later equal send's record on the same branch, as on main.
|
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 6 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 |
Owner
Author
|
Merging afc4746: omo-native-code-reviewer and Codex both pass this head. The new branch-return tests fail on main, and every new guard has a test that fails without it. The required checks are green, and CodeRabbit did not review this head. The LOW notes go to a follow-up issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 (
observeTranscriptpinstranscriptStoreto 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/treecame back, the returning entry looked like a fresh fork, the branch got a new name, and the send stayedaccepteduntilunknown, even though itsmodel_changewas 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: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 fromPI_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)/treecomes back (Command outcomes: leftovers of #1487 and #1544 (low items, full-window reads, other agents' process reads) #1879): fails on main (staysaccepted), passes.forks.every((fork) => fork === head)makes it fail.bun run check fast: all steps ok, unit 7878 pass / 0 fail.Reviews
Left on #1879
omoSessionForPanestill ask herdr themselves.