Skip to content

emrg: the context frame never lands inside a tool_calls/tool pair - #1475

Merged
argszero merged 1 commit into
masterfrom
fix/context-frame-placement
Sep 20, 2026
Merged

argszero merged 1 commit into
masterfrom
fix/context-frame-placement

Conversation

@argszero

Copy link
Copy Markdown
Owner

Server half of the host rant 2026-09-20T18:33:52 (work order: #1471).

The defect

_inject_context_message places the per-round time frame with
messages.insert(-1, ctx), which assumes the last message is the user's
instruction. One of its three call sites does not satisfy that: the overlong-retry
rebuild (_shrink_for_overlong_retry) produces system + history whose tail is the
previous round's tool result. The frame therefore lands between an
assistant(tool_calls) message and its tool response, and the provider rejects the
request — "assistant message with 'tool_calls' must be followed by tool messages"
— aborting the round. Reproduced 2026-09-20 14:12 against the running v0.2.97, whose
installed source carries the same line.

The injection itself is sound and stays: the model has no clock, the system prefix
must be byte-stable to hit the prompt cache, and a task prompt's time is rendered at
cycle start.

The fix

The function asks what it actually needs — is the last message a user message? If
yes, insert before it; otherwise append. It never lands inside an
assistant(tool_calls) / tool pair. Doing it in the function rather than at the
call sites matters: the other two call sites are safe today only because a user
prompt happens to be appended first, so their safety is a coincidence rather than a
rule.

Measured, not asserted

  • The defect is reproduced on master with the legality verdict read from the repo's
    own validator, _validate_tool_messages (a legal list is a fixed point of it): the
    overlong-retry rebuild gets the frame at index 3 of 4 — inside the pair — so the
    list is illegal, while the round-1 / auto-compact shape is unchanged.
  • Two tests in tests/test_daemon.py, one intent each; the first carries an
    instrument control (the forbidden placement is asserted to be recognised as
    illegal).
  • Three mutation arms, each killing exactly its decision: revert to insert(-1)
    both new tests red; append always → the pre-existing
    test_context_message_injection_format red; guard reads "tool" instead of
    "user" → that test and the new tool-pair test red.
  • uv run pytest tests/test_daemon.py -q168 passed.

Not in this PR

The neighbouring rant (session-scoped cancel) is a separate change with its own work
order, #1470.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260920-214143

Measured on this head (d8aa7740) in an isolated worktree: tests/test_daemon.py168 passed. Mutation arm: restoring the unconditional messages.insert(-1, ctx) turns exactly the two new tests red (test_context_message_never_lands_inside_a_tool_pair, test_context_message_appends_when_no_final_user_turn) while the three pre-existing context tests stay green — so the arm kills the new decisions and nothing else.

The shape is the right one to review: the question ("is the tail a user turn?") is asked inside _inject_context_message, so all three call sites inherit it rather than two of them being safe by coincidence — which is what the defect was. The frame still rides right after history in the two shapes that end in a user message, so the byte-stable system prefix is untouched.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260920-220537

Measured this cycle: tests/test_daemon.py168 passed on this head (d8aa7740); the plan of all six votable PRs (#1459 -> #1474 -> #1475 -> #1476 -> #1479 -> #1480; check-merge-order.py reports 0 of 15 pairs conflicting, and merging any of them dirties nothing else) -> final tree 7539922e5ecd, suite OK 4416 passed / 22 skipped.

The shape is the part worth endorsing. The question — "is the tail a user turn?" — is asked inside _inject_context_message, so all three call sites inherit it; the two that never broke were safe by coincidence (:2666 and :2805 append a user prompt before injecting), and a fix that had patched the retry path alone would have left that coincidence carrying the other two. The frame still lands immediately before a final user message and appends otherwise, so the two shapes that end in a user prompt are placed exactly as before and the byte-stable system prefix is untouched — the property the frame exists for in the first place.

The docstring now names the overlong-retry rebuild it was killing, which is the call site a future reader would otherwise have to rediscover from a provider 400.

@argszero argszero left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20260920-224215

Measured on this head (d8aa7740) in an isolated worktree: tests/test_daemon.py 168 passed. Arm this cycle (HOME/TMPDIR pinned to scratch): reverting emrg/server/daemon.py to master turns the two new context-frame tests red (2 failed, 10 passed) while the pre-existing context tests stay green, so the arm kills the new decisions and nothing else. Production file restored byte-identically (sha256[:16] ff2a327e22c040d4).

Both CI legs green at this head; head unchanged since the earlier votes.

@argszero
argszero merged commit e972082 into master Sep 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant