Repository navigation
Fix duplicate session history on failed-run recovery - #1475
dajiaohuang wants to merge 2 commits into
Conversation
…1310) _salvage_stream_to_session previously did pre_run_items + replay, which duplicated items when stream.to_input_list() already included the pre-cycle input (the seeded initial_input is in both). Detect the overlap and avoid duplication by checking if replay suffix matches pre_run_items.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| desired = list(pre_run_items) + replay | ||
| if len(desired) <= len(pre_run_items): | ||
| pre_len = len(pre_run_items) | ||
| if pre_len > 0 and len(replay) >= pre_len and replay[-pre_len:] == pre_run_items: |
There was a problem hiding this comment.
replay[-pre_len:] checks the end of the history, but the overlap described by this fix is at the beginning. After a seeded run produces output and then fails, pre_run_items = [initial_input] and replay = [initial_input, new_output]. The comparison fails, so the fallback saves [initial_input, initial_input, new_output]. The revived agent still receives duplicate input.
Compare the beginning of replay instead. Add a test where replay includes saved input followed by new output; the existing test supplies only new output.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/core/execution.py
Line: 175
Comment:
**Recovered history still repeats**
`replay[-pre_len:]` checks the end of the history, but the overlap described by this fix is at the beginning. After a seeded run produces output and then fails, `pre_run_items = [initial_input]` and `replay = [initial_input, new_output]`. The comparison fails, so the fallback saves `[initial_input, initial_input, new_output]`. The revived agent still receives duplicate input.
Compare the beginning of `replay` instead. Add a test where `replay` includes saved input followed by new output; the existing test supplies only new output.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
1 participant
Withdrawn in favor of the earlier recovery work in #1311, which addresses #1310. The underlying duplication error remains real; this closure does not mean it is fixed on main.
At head 78b537c this patch compares the beginning of the replay, not its suffix. A normal SDK full-history replay includes the saved input, so prepending that input duplicates history. However, this patch still replaces the current session from an earlier snapshot and does not reconcile late persisted messages or make the rewrite atomic relative to SDK writers. Its existing first fixture also treats replay as new-items-only, unlike the supported full-history contract.
#1311 already covers the same recovery error, full-history/idempotence cases, late messages, and atomic SQLite reconciliation. No acceptance or execution of its tests is claimed here. No new tests, builds, runtime, installation, or CI runs were performed during this reassessment; the prior single targeted test pass is historical component evidence only. Branch and commits are retained.