Repository navigation
fix(client): keep an evicted transcript evicted on later steps - #56751
shreyanshj10 wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The change looks sound. I ran the #56737 reproduction against both checkouts. On the base, an evicted session's transcript grows by one full assistant message per background step, and a later metadata update doesn't evict it again. With this change it stays empty, and after the session is reopened its history reloads and new steps stream normally. Your new test fails on the base and passes here. The client tests and the packages/tui session retention tests pass, and type checks are clean in both packages.
Public API: unchanged (the new tracking is internal to createData).
I also checked the edge cases: a step that starts while a reopened session's history is still loading is dropped, but the base already loses it in that case, so it isn't a regression. Every session in an evicted family gets marked, and sessions with a queued outgoing message are never evicted, so they never get the mark. No blocking concerns.
Related: #56736
Issue for this PR
Closes #56737
Type of change
What does this PR do?
Retention evicts a family's transcript when it falls out of the recent-three set, but a later
session.step.startedfor that family went straight intomessage.update, which lazily recreates the per-session message array. A background session that keeps running therefore rebuilds a full transcript behind the retention decision, one assistant message per step, and the next recompute does not evict it again because the family is already in the excluded map.session.step.startedis the only handler that appends an assistant row from an event; everything that streams into that row afterwards (text, reasoning, tools,step.ended) already drops its edit when the row is missing. So I record the sessions eviction dropped and have that one handler skip them until a transcript read puts the session back. A read is where the history comes from anyway, so nothing is lost: reopening the session hydrates it from the server as before, and the current session streams normally.The issue asks for the policy for an excluded but still-running family to be explicit, so to be clear about the one I picked: its heavy assistant transcript stays excluded until the session is read again, and the light marker rows are unchanged. I kept the markers because
session.model.selectedalready guards itself on a loaded transcript andtest/solid-eviction.test.tsexpects the others (agent-switched) to keep mirroring live state after an eviction. If you would rather exclude those too it is a one-line move of the same guard intomessage.update, plus that assertion.How did you verify your code works?
Added a case to
packages/tui/test/context/session-retention.test.tsthat views four root sessions with no kept tabs and a limit of 3, then delivers a complete step (session.step.started,session.text.started,session.text.delta) for the evicted family and updates its metadata: its transcript stays empty. Without the change it holds one assistant message with its text. The same test then reopens that session and sends another step, to confirm a current session still gets its message and text.In
packages/tui:bun test test/context/session-retention.test.ts. Inpackages/client:bun test test/solid-eviction.test.ts test/solid-data.test.ts test/solid-inbox-race.test.ts test/solid-compaction.test.ts test/solid-pending-status.test.ts. Plusbun run checkat the root.Checklist
Added by the opencode bot (AI-generated).
Related: #56736