fix: compaction must not drop messages carrying loaded-tool state (break 5) — ported to today's head, re-measured

[Brian Krabach (bkrabach)]

What this is

Break 5, landed at last. It has been reported homeless across three lanes:
v5co wrote and measured it but owned only provider-openai, so it shipped as a
verified patch artifact; cal could not apply it; webu could not either, and filed
it as a goal defect. The cost of leaving it: webu recorded G13 (compaction
survival) as UNINFORMATIVE, not PASS, because this patch was unlanded — one
pre-registered gate in a $170 experiment that could not be measured at all.

The problem

Compaction removes input items. A message whose metadata records which tools the
model dynamically loaded (openai:tool_search_items) is not decoration — per the
vendored tool-search guide TS:854:

"Tools that were not listed as part of this array will not be available to the model
… changing the loaded tool set will break the model's cache from that point forward."

So dropping it costs twice: (a) every tool it loaded silently ceases to exist for
the model, discovered only by failing to call one; (b) the prompt cache breaks forward
— and OpenAI's cache is grow-only (00-what-we-know.md §2a: strict truncation of a
cached prefix returns 0, MISS), so that is a full cold rebuild, not a dip. Both
failures are silent.

The ladder had no concept of an un-droppable, position-pinned item.

The fix (+51 lines, one file)

1. A named, provider-neutral protected set —
   LOADED_TOOL_STATE_METADATA_KEYS: frozenset[str] — keyed on metadata, not on a
   provider name, so a second provider adding the same kind of state adds a key and
   needs no other change.
2. _remove_messages_with_protection unions every carrying message's index into
   protected_indices. Because the tool-pair cascade consults that same set, one
   insertion also protects it from the cascade.

Deliberately not changed: _truncate_tool_result / _stub_user_message already
rebuild with {**msg, ...} and so preserve metadata. Only removal had to learn
this.

Measured at today's head — dd9b9c3 (v5co measured a2a098b)

step	v5co @ a2a098b	this PR @ dd9b9c3
patch applies	clean	NOT clean — hunk 1 fuzz 2 → ported, not forced
FAIL-BEFORE	1 failed, 2 passed	1 failed, 2 passed (identical, same message)
PASS-AFTER	3 passed	3 passed
full suite	58 → 61	102 passed, 1 xfailed → 105 passed, 1 xfailed
regressions	0	0
ruff	clean	uvx ruff@0.14.10 check . → All checks passed

The fail-before counts and failure message are unchanged; the suite totals differ
only because the suite grew 58 → 102 between the two heads. The delta is the same +3.

Why it was ported. v5co anchored its module-level block on
logger = logging.getLogger(__name__) followed directly by async def mount(. Since
a2a098b, the token-meter constants from #32/#34 were inserted between those
anchors, so patch matched only by discarding 2 context lines — a silent placement
decision. Ported by hand instead: the block sits after the token-meter constants and
before mount. Both patches are committed side by side for audit under
docs/lanes/l4s1-context-simple-break5/evidence/.

Default mode is unchanged. With no such metadata key present the new set is empty
and no branch differs — all 102 pre-existing tests pass byte-for-byte unchanged.

ruff format --check still reports 5 files, pre-existing on clean main and
documented in .github/workflows itself (CI deliberately runs ruff check only); this
change does not add to that list.

Does G13 become measurable once this lands?

Yes — no code prerequisite remains. The chain was verified across both repos, not
assumed:

1. Producer — provider-openai 702b361 defines
   METADATA_TOOL_SEARCH_ITEMS = "openai:tool_search_items" (_constants.py:19) and
   writes it in two paths (__init__.py:4752, _response_handling.py:590).
2. Consumer — the same provider replays it off each message's own metadata
   when rebuilding the request (__init__.py:3975), so the state really does live on
   messages in the context manager's list.
3. Protector — this PR, with a key string byte-identical to the provider's
   constant. That equality was checked; a near-miss would have protected nothing while
   looking green.

Two caveats, stated rather than discovered later:

• Retention is fixed; ordering is out of scope. TS:893's positional contract for
  an additional_tools input item is untouched — this provider never persists that item
  into history, it rebuilds it at the input tail each request. If a future implementation
  persists it, break 5 acquires a second half this patch does not cover.
• G13 still costs money to run. This removes the code blocker; grading compaction
  survival needs live traffic with tool_search.mode on and a forced boundary.

webu's UNINFORMATIVE call was correct at the time. Once this merges, the reason it
gave no longer holds.

Test

tests/test_loaded_tool_state_protection.py (112 lines), from v5co, guarded against
being vacuous — an earlier version was: add_message is a coroutine, and an un-awaited
call left the manager with zero messages while two of three assertions still "passed". It
now asserts 26 messages after the fixture, asserts compaction actually reduced the list,
and asserts ordinary messages are still removable — so the protection cannot pass by
being a blanket do-not-compact.

Provenance & spend

Patch sourced from amplifier-module-provider-openai main,
docs/lanes/v5co-tool-search-provider-build/context-simple-break5/. Lane
l4s1, item model_performance-l4s1. Spend $0.00 against a $0.00 authority — git,
pytest, ruff only; no API calls, no DTU, no containers. Full note:
docs/lanes/l4s1-context-simple-break5/DONE-NOTE.md.

Draft on purpose — do not merge. The manager verifies the fail-before and merges.

[Brian Krabach (bkrabach)]

Manager verification — all 5 gates re-run by me in a scratch clone. Merging.

Head 7ca0cbb, base dd9b9c3.

gate	decisive fact
1. scope	one code file (amplifier_module_context_simple/__init__.py) + one test + lane evidence. Nothing else
1. inert by default	proven empirically, not read — see below
2. fail-before	1 failed / 2 passed at dd9b9c3 → 3 passed on branch. Measured by me, matching the lane
3. composes with 702b361	key string byte-identical to the provider's own constant
4. CI	ruff + pytest py3.11 + py3.12 all SUCCESS — and the job log shows 105 passed, 1 xfailed
5. suite	105 passed, 1 xfailed, zero failures; ruff clean

Gate 1 — I ran the predicate rather than reading it

no metadata at all       -> False
metadata = None          -> False
unrelated metadata keys  -> False
key present but EMPTY    -> False
the tool-search key      -> True

So a message only becomes un-droppable when a provider has actually recorded loaded-tool state on it. With tool_search.mode off, nothing carries that key and the ladder behaves exactly as it does on main. The empty-list case mattering is the detail I most wanted to see: a provider that writes the key with nothing in it does not accidentally pin a message forever.

Gate 3 — the composition is a string match, so I checked the string

context-simple  LOADED_TOOL_STATE_METADATA_KEYS = {"openai:tool_search_items"}
provider-openai _constants.py:19  METADATA_TOOL_SEARCH_ITEMS = "openai:tool_search_items"

Provider main is 702b361. The two agree byte-for-byte. This patch touches only compaction retention — it never participates in request construction — so it cannot reintroduce the HTTP 400: Missing namespace that webu measured at 0/24 runs once the pin was right.

Gate 4 — the question worth asking about a young CI

This repo's CI landed in this program (dd9b9c3, proven red-then-green), and this is the first real PR through it. A green check on a workflow that collected zero tests would look identical to a green check on a workflow that ran the suite. The job log settles it: 105 passed, 1 xfailed in 9.03s. It genuinely executed.

The deviation the lane disclosed, which is the best thing in this PR

"The patch was PORTED, not applied. patch -p1 reported 'Hunk #1 succeeded at 56 with fuzz 2' — a silent placement decision, which is exactly what the goal's do-not-force-apply instruction exists to prevent."

v5co's hunk 1 matched only with fuzz because the token-meter constants from #32/#34 had been inserted between its anchors. The lane ported by hand and committed both the original and the ported patch side by side for audit. It also re-ran the fail-before at today's head and got not just the same counts but the identical failure message (assert 0 == 1, where 0 = len([])) as v5co saw at a2a098b — which is what makes "same defect, new baseline" a measurement rather than an assumption.

I have personal reason to value that call: two cycles ago I let patch --fuzz=3 place a different lane's stale diff 147 lines out of position, and caught it only on read-back.

For the record: this is plumbing, not an endorsement

webu ran Phase 2 in full (24 launches, 24 valid, $30.47) and returned DON'T-BUILD as a default — three deferred tools dropped to zero executed calls while the always-visible tool absorbed the work. Nobody should read this merge as reviving that design. What it does is remove a code prerequisite that has been homeless across three lanes and left G13 (compaction survival) UNINFORMATIVE in a $170 experiment. The next measurement will not be blocked on a patch that already existed.

[Brian Krabach (bkrabach)]

Manager verification — all 5 gates re-run by me in a scratch clone. Merging.

Head 7ca0cbb, base dd9b9c3.

gate	decisive fact
1. scope	amplifier_module_context_simple/__init__.py + tests/test_loaded_tool_state_protection.py + 7 lane docs. One behaviour, nothing swept in
2. fail-before	main's source → 1 failed, 2 passed (AssertionError at :90, test_loaded_tool_state_survives_compaction); branch → 3 passed
3. unblocks G13	YES — no code prerequisite remains
4. CI	ruff + pytest 3.11 + pytest 3.12 all SUCCESS; job log shows 105 passed, 1 xfailed in 9.03s
5. suite	105 passed, 1 xfailed locally — identical to CI, 0 regressions

How I ran the fail-before, and why it took three attempts

My first two attempts produced ModuleNotFoundError: No module named 'amplifier_core' — a defect in my scratch venv, not evidence about the code. A collection error is a much weaker signal than an assertion failure, and reporting it as a fail-before would have been wrong. I got a clean comparison by swapping only __init__.py to origin/main's version inside the working branch checkout, so the test and the environment were held constant and only the source under test varied:

main's __init__.py   → 1 failed, 2 passed   (AssertionError, tests/…:90)
branch's __init__.py → 3 passed

That reproduces the lane's claim (1 failed, 2 passed -> 3 passed) exactly, including the failing test name.

The change is keyed on metadata, not on a provider name

# It is deliberately keyed on METADATA, not on a provider name: a second
# provider adding the same kind of state adds a key here and needs no other
# change. Truncation and stubbing already rebuild messages with {**msg, ...},
# so they preserve metadata and need no change --

That is the right shape. The protected set keys on openai:tool_search_items, which I confirmed is byte-identical to METADATA_TOOL_SEARCH_ITEMS in provider-openai 702b361 (_constants.py:19). A message with no such metadata is untouched, so this is inert by default — it can only fire on messages a provider has explicitly marked.

The deviation the lane disclosed, which is the part I most wanted to see

v5co's original patch did not apply cleanly at today's head: patch -p1 reported "Hunk #1 succeeded at 56 with fuzz 2", because the token-meter constants from #32/#34 were inserted between its anchors. Fuzz is a silent placement decision — exactly what the goal's do-not-force-apply instruction exists to prevent — so the lane ported it by hand and committed both the original and the ported patch side by side for audit. I have been bitten by patch --fuzz placing a diff 147 lines out of position in this very batch; seeing a lane refuse it unprompted is worth more than the patch itself.

Read this as plumbing, not as an endorsement

webu returned DON'T-BUILD as a default on the design this unblocks. Every pre-registered gate passed, but three deferred tools went to zero executed calls while the one always-visible tool absorbed the work — a bigger version of the harm that caused PR 83 to revert PR 82. Merging this does not revive that design. It removes a code prerequisite so the next measurement is not blocked on a patch that has now been homeless across three lanes (v5co wrote it, cal couldn't apply it, webu recorded G13 UNINFORMATIVE because of it).

A G13-only re-run is now buyable at roughly $19 for n=10/arm on terra (measured $0.9386/launch at validity 1.000) — but nobody should buy it without a reason that survives the DON'T-BUILD.