Repository navigation
fix(s08): keep the active user request visible after snip_compact - #580
wudilyy999 wants to merge 2 commits into
Conversation
snip_compact archives the middle section of the history. Once a task accumulates many tool calls, the current user request moves into that archived middle, and because the context then fits under the limit, compact_history never runs — so the model loses sight of the active request and can drift back to an older one (reproduced in shareAI-lab#577 by entering the three README example prompts in one conversation). prepare() now checks whether snip_compact archived messages and the active request is no longer present verbatim; only then is the request reinserted as a dedicated "Current user request" message, in the same format compact_history and reactive_compact already use. Histories without an archive marker and requests still visible in the kept head or tail are left untouched. Adds two regression tests: one drives the request into the archived middle behind earlier turns plus a tool-call burst, the other asserts no duplicate request message appears when nothing was archived. Fixes shareAI-lab#577
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The current reinsertion approach is not idempotent across repeated prepare() calls and can cause repeated request duplication and repeated re-snipping/transcript writes in the main agent loop.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
This PR fixes a compaction edge case in s08_context_compact where snip_compact() can archive the currently active user request during long tool-call bursts, causing the agent to “lose” the task it’s working on. It adds logic in prepare() to re-surface the active request when structural archiving occurred, plus regression tests.
Changes:
- Add a post-
snip_compact()check inprepare()to ensure the active request is still model-visible, reinserting it when it was archived. - Add two regression tests covering reinsertion when the request is archived and no duplication when the request is already visible.
| File | Description |
|---|---|
| s08_context_compact/code.py | Adds logic in prepare() to reintroduce the active request after structural snipping/archiving. |
| tests/test_s08_context_compact.py | Adds regression tests for the archived-request reinsertion behavior and non-duplication when nothing is archived. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if active_request and any( | ||
| self.is_archive_marker(message) for message in messages | ||
| ) and not any( | ||
| isinstance(message.get("content"), str) | ||
| and message["content"] == active_request | ||
| for message in messages | ||
| if message.get("role") == "user" | ||
| ): | ||
| # snip_compact archives the middle section, which may contain the | ||
| # current user request once a task accumulates many tool calls. | ||
| # Reinsert it in the same format used by compact_history / | ||
| # reactive_compact so the model never loses the active task. | ||
| messages.insert(0, {"role": "user", "content": | ||
| f"Current user request:\n{active_request}"}) |
There was a problem hiding this comment.
Fixed in 4826a73: the existence check now also recognizes the prefixed "Current user request:" message, so prepare() will not reinsert on subsequent agent-loop iterations. The extra message could also push the list back over snip_compact()'s 50-message limit, so prepare() re-runs snip_compact() right after reinserting; the returned list is already within the limit and the next iteration stays stable (no repeated transcript writes). Covered by the new idempotency regression test.
| prepared = lesson["COMPACTOR"].prepare(list(messages), request) | ||
| rendered = "\n".join( | ||
| message["content"] for message in prepared | ||
| if message.get("role") == "user" | ||
| and isinstance(message.get("content"), str) | ||
| ) | ||
| assert "Current user request" in rendered | ||
| assert request in rendered | ||
|
|
There was a problem hiding this comment.
Added in 4826a73: test_prepare_is_idempotent_when_still_over_message_limit calls prepare() twice, asserts the output is byte-identical, contains exactly one "Current user request:" message, and writes no additional transcript files.
Copilot review on shareAI-lab#580 flagged two issues: the reinsertion guard only matched the raw request string, so after the first pass the prefixed "Current user request" message no longer matched and prepare() reinserted on every agent-loop iteration; and the extra message kept the list over snip_compact()'s 50-message limit, causing repeated re-archiving and transcript writes. Recognize the prefixed insertion in the existence check, and re-run snip_compact() right after reinserting so prepare() returns a list that is already within the snip limit. Add a regression test that calls prepare() twice and asserts the output is stable, contains a single request message, and writes no extra transcripts.
Fixes #577
Problem
snip_compact()archives the middle section of the conversation history. Once a task accumulates many tool calls, the current user request moves into that archived middle — and because the context then fits under the limit,compact_history()never runs, so the active request is never reinserted. The model loses sight of the task it is currently working on and can drift back to an older request (reproduced in #577 by entering the three README example prompts in one CLI conversation: while processing Prompt 3, the agent returned to Prompt 1).Fix
prepare()now checks two conditions aftersnip_compact():Only when both hold is the request reinserted as a dedicated
Current user requestmessage at the head — the same framingcompact_history()andreactive_compact()already use. Histories without an archive marker, and requests still visible in the kept head or tail, are left untouched.Verification
tests/test_s08_context_compact.py:test_prepare_reinserts_active_request_archived_by_snip_compact— earlier turns occupy the head, a 26-call tool burst pushes the active request into the archived middle; asserts the request is reinserted and visible. Fails before the fix, passes after.test_prepare_does_not_duplicate_active_request_visible_in_tail— asserts no duplicate request message appears when nothing was archived.tests/test_s08_context_compact.py: 6/6 pass.Disclosure: prepared with AI assistance (Codex). I have read the change, run the tests, and stand behind it.