Two follow-ups from the stage-353 / v0.51.60 Opus review of PR #2227 (compression-rotation data-loss fix for #2223). Neither blocked the release, but worth filing so they don't get lost.
1. Integration coverage gap on PR #2227
The 6 new tests in tests/test_issue2223_compression_no_rename.py are:
- 3 static
streaming_src grep checks (test_rename_call_removed, test_parent_session_id_stamped_on_continuation, test_old_session_preservation_logic_exists)
- 3 unit tests targeting
_merge_display_messages_after_agent_result() (a related but separate function)
There is no test that exercises the actual rotation block end-to-end — build a Session with N messages, stub agent.session_id = new_sid, invoke the rotation code, assert old_sid.json exists with N messages AND new_sid.json gets created on the subsequent s.save().
The grep tests catch removal-of-string but don't lock in behavior. Someone could re-introduce a different destructive op (e.g. os.unlink(old_path) or shutil.move) and all the grep checks would still pass.
Suggested fix: One integration test with tmp_path that:
- Creates a Session with N>1 messages and saves it as
old_sid.json in the tmp SESSION_DIR
- Sets up
s.session_id = old_sid then triggers the compression rotation block (or its extracted helper) with _agent_sid = new_sid
- Asserts
old_sid.json still exists on disk after the rotation with all N messages intact
- Asserts
new_sid.json is created and contains the post-compression state
Replace one of the static grep checks with this integration test. The other two grep checks still have value as "presence-of-fix" canaries.
2. Sidebar duplication after compression
After v0.51.60's fix, both old_sid.json and new_sid.json stay indexed in the sidebar. This matches /branch fork semantics (the old session becomes a permanent snapshot accessible via lineage traversal from the new one), but in a long-lived workspace with multiple compressions per session, the sidebar will accumulate snapshot entries.
_write_session_index fast path at api/models.py:158-160 keeps entries whose file exists on disk. The preservation block writes old_sid.json with skip_index=True, but the old entry was already in the index (it was the active session), so it stays.
Suggested fix path: Stamp s.archived = True on the preservation save so the snapshot stays out of the active sidebar but remains accessible via lineage traversal. The compression continuation new_sid would NOT inherit the archived flag (it's the new active session).
Alternative: add a new pre_compression_snapshot: True field on the snapshot and filter on it in the sidebar query, keeping archived semantics independent.
Worth discussing the right ownership model — for now this is just a future-proofing concern, not a user-reported problem.
Provenance
Two follow-ups from the stage-353 / v0.51.60 Opus review of PR #2227 (compression-rotation data-loss fix for #2223). Neither blocked the release, but worth filing so they don't get lost.
1. Integration coverage gap on PR #2227
The 6 new tests in
tests/test_issue2223_compression_no_rename.pyare:streaming_srcgrep checks (test_rename_call_removed,test_parent_session_id_stamped_on_continuation,test_old_session_preservation_logic_exists)_merge_display_messages_after_agent_result()(a related but separate function)There is no test that exercises the actual rotation block end-to-end — build a Session with N messages, stub
agent.session_id = new_sid, invoke the rotation code, assertold_sid.jsonexists with N messages ANDnew_sid.jsongets created on the subsequents.save().The grep tests catch removal-of-string but don't lock in behavior. Someone could re-introduce a different destructive op (e.g.
os.unlink(old_path)orshutil.move) and all the grep checks would still pass.Suggested fix: One integration test with
tmp_paththat:old_sid.jsonin the tmp SESSION_DIRs.session_id = old_sidthen triggers the compression rotation block (or its extracted helper) with_agent_sid = new_sidold_sid.jsonstill exists on disk after the rotation with all N messages intactnew_sid.jsonis created and contains the post-compression stateReplace one of the static grep checks with this integration test. The other two grep checks still have value as "presence-of-fix" canaries.
2. Sidebar duplication after compression
After v0.51.60's fix, both
old_sid.jsonandnew_sid.jsonstay indexed in the sidebar. This matches/branchfork semantics (the old session becomes a permanent snapshot accessible via lineage traversal from the new one), but in a long-lived workspace with multiple compressions per session, the sidebar will accumulate snapshot entries._write_session_indexfast path atapi/models.py:158-160keeps entries whose file exists on disk. The preservation block writesold_sid.jsonwithskip_index=True, but the old entry was already in the index (it was the active session), so it stays.Suggested fix path: Stamp
s.archived = Trueon the preservation save so the snapshot stays out of the active sidebar but remains accessible via lineage traversal. The compression continuationnew_sidwould NOT inherit the archived flag (it's the new active session).Alternative: add a new
pre_compression_snapshot: Truefield on the snapshot and filter on it in the sidebar query, keepingarchivedsemantics independent.Worth discussing the right ownership model — for now this is just a future-proofing concern, not a user-reported problem.
Provenance