Skip to content

Commit 4826a73

Browse files
committed
fix(s08): make prepare() idempotent after request reinsertion
Copilot review on #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.
1 parent 47e78d0 commit 4826a73

2 files changed

Lines changed: 51 additions & 1 deletion

File tree

‎s08_context_compact/code.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,8 @@ def prepare(self, messages: list, active_request: str) -> list:
513513
self.is_archive_marker(message) for message in messages
514514
) and not any(
515515
isinstance(message.get("content"), str)
516-
and message["content"] == active_request
516+
and (message["content"] == active_request
517+
or message["content"].startswith("Current user request:"))
517518
for message in messages
518519
if message.get("role") == "user"
519520
):
@@ -523,6 +524,10 @@ def prepare(self, messages: list, active_request: str) -> list:
523524
# reactive_compact so the model never loses the active task.
524525
messages.insert(0, {"role": "user", "content":
525526
f"Current user request:\n{active_request}"})
527+
# The extra message can push the list back over the snip limit;
528+
# archive again now so the next prepare() call in the agent loop
529+
# stays stable instead of re-archiving on every iteration.
530+
messages = self.snip_compact(messages)
526531
if self.estimate_chars(messages) > self.CONTEXT_CHAR_LIMIT:
527532
target = int(self.CONTEXT_CHAR_LIMIT * 0.8)
528533
messages = self.micro_compact(messages, target)

‎tests/test_s08_context_compact.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,51 @@ def test_prepare_reinserts_active_request_archived_by_snip_compact(
174174
assert request in rendered
175175

176176

177+
def test_prepare_is_idempotent_when_still_over_message_limit(
178+
tmp_path, monkeypatch):
179+
"""agent_loop calls prepare() every iteration; the reinsertion must not
180+
stack up duplicate request messages or re-archive on every pass."""
181+
lesson = load_lesson(monkeypatch, tmp_path)
182+
request = "compare s08 and s09 context handling"
183+
messages = [
184+
{"role": "user", "content": "earlier question one"},
185+
{"role": "assistant", "content": [{"type": "text", "text": "answer one"}]},
186+
{"role": "user", "content": "earlier question two"},
187+
{"role": "assistant", "content": [{"type": "text", "text": "answer two"}]},
188+
{"role": "user", "content": request},
189+
{"role": "assistant", "content": [{"type": "text", "text": "starting"}]},
190+
]
191+
for index in range(26):
192+
tool_id = f"tool-{index}"
193+
messages.extend([
194+
{"role": "assistant", "content": [
195+
{"type": "tool_use", "id": tool_id, "name": "bash", "input": {}}
196+
]},
197+
{"role": "user", "content": [
198+
{"type": "tool_result", "tool_use_id": tool_id, "content": f"out-{index}"}
199+
]},
200+
])
201+
202+
compactor = lesson["COMPACTOR"]
203+
prepared = compactor.prepare(list(messages), request)
204+
assert len(prepared) <= 50 # prepare() leaves the list within the snip limit
205+
assert prepared[0]["content"] == f"Current user request:\n{request}"
206+
transcripts_before = list(
207+
(tmp_path / ".transcripts").glob("transcript_*.jsonl"))
208+
209+
prepared_again = compactor.prepare(list(prepared), request)
210+
211+
assert prepared_again == prepared
212+
request_messages = [
213+
message for message in prepared_again
214+
if isinstance(message.get("content"), str)
215+
and message["content"].startswith("Current user request:")
216+
]
217+
assert len(request_messages) == 1
218+
assert len(list((tmp_path / ".transcripts").glob("transcript_*.jsonl"))) \
219+
== len(transcripts_before)
220+
221+
177222
def test_prepare_does_not_duplicate_active_request_visible_in_tail(
178223
tmp_path, monkeypatch):
179224
lesson = load_lesson(monkeypatch, tmp_path)

0 commit comments

Comments
 (0)