From 90843b3f50895b8e75665d5959867aba30284884 Mon Sep 17 00:00:00 2001 From: zhanghanduo Date: Sat, 5 Sep 2026 09:56:56 +0800 Subject: [PATCH] feat(context): let the host word a Tier 1 card's recovery footer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Tier 1 mini card REPLACES the whole tool message. The loop's own truncation footer (``_spill_footer``) appends to a surviving body and can therefore afford to be the only place that explains recovery — a card cannot, because after it runs there is nothing beside the handle at all. On a Tier1-only turn (Tier 2 fires only when Tier 1 did not free enough) the model sees a bare ``[Full text] `` and is never told the handle is recoverable, let alone how. The default cannot fix that itself. This module knows neither what a host calls its recovery tool nor whether that tool is bound for the agent whose history it is compacting, and a footer naming a tool the agent cannot call is worse than no footer — precisely why ``_spill_footer`` gates its prose on ``recover_result`` being in the tool map. So the handle-only rendering stays the default, and the decision moves to the party that has the tool map: ``recovery_footer``. ``default_recovery_footer`` is the existing wording, so no consumer changes behaviour by upgrading; both its docstring and the CHANGELOG carry the one constraint a host has to respect. Write the prose as prose. The callable form ``recover_result(spill_id="...")`` reads to the model as source code and it answers in kind: on a live run a model reproduced such a footer inside a ```bash block instead of emitting a tool call, and ``LeakedToolCallRetryObserver`` fired twice. That failure is why the wording was flattened to ``[Full text]`` in the first place — the fix is prose naming the tool, not a return to the callable form. Tests pin the three things a host renderer must not be able to break: the default names no tool, a host footer replaces the last line and leaves the card's arg/URL lines (and so its token budget) untouched, and the footer is never invoked when nothing spilled. A fourth pins idempotency against a footer LONGER than the default, since that is the case where a second pass re-carding the message would compound. MINOR per docs/versioning.md: new capability, additive keyword argument. Generated with [Claude Code](https://claude.ai/code) via [Very Happy](https://github.com/Mereithhh/very-happy) Co-Authored-By: Claude --- CHANGELOG.md | 40 +++++++++++ agent_core/runtime/loop/compact.py | 32 ++++++++- agent_core/runtime/loop/tiered_compact.py | 8 +++ pyproject.toml | 2 +- tests/test_keep_last_n_compactor.py | 83 +++++++++++++++++++++++ tests/test_tiered_compact.py | 54 +++++++++++++++ uv.lock | 2 +- 7 files changed, 218 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27688bf..2eaf36a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,46 @@ the GitHub Release body, so a release with no entry here fails. Versioning follows [docs/versioning.md](docs/versioning.md). +## [0.8.0] - 2026-09-05 + +### Added + +- `KeepLastNToolResultsCompactor` takes a `recovery_footer` callable that renders + the last line of a Tier 1 mini card — the handle that fetches the discarded body + back. The default, `default_recovery_footer`, is the existing + `[Full text] `, so no consumer changes behaviour by upgrading. + + Rationale: a Tier 1 card *replaces* the whole tool message, so unlike the + loop's own truncation footer (`_spill_footer`, which appends to a surviving + body) there is nothing left beside the handle to tell the model what it is for. + On a Tier1-only turn — Tier 2 fires only when Tier 1 did not free enough — the + model therefore sees bare handles and is never told they are recoverable at all. + + The default cannot fix that itself: this module knows neither what a host calls + its recovery tool nor whether that tool is bound for the agent whose history it + is compacting, and a footer naming a tool the agent cannot call is worse than no + footer (the same reason `_spill_footer` gates its prose on the tool map). A host + that does know both should pass `recovery_footer`. + + Write host prose as prose. `recover_result(spill_id="...")` renders to the model + as source code and it answers in kind: on a live run a model reproduced such a + footer inside a ```bash block instead of emitting a tool call, and + `LeakedToolCallRetryObserver` fired twice. Name the tool and its argument in + words. + + Idempotency is unaffected — the already-carded check keys on + `OMITTED_TOOL_RESULT_PLACEHOLDER`, not on the footer's wording — and the footer + is called only when a body actually spilled, so a host renderer is never asked + to point at nothing. + +- `TieredCompactor` takes and forwards the same `recovery_footer` to the Tier 1 + compactor it builds internally. Tier 1 is a winning candidate in its own right, + so its card reaches the model from the tiered path exactly as from the + standalone one. Without the forward, a workflow whose primary path is tiered and + whose fallback is standalone would render the handle-only default on every + production turn and the host's prose only in the fallback — the harder of the + two failures to notice. + ## [0.7.0] - 2026-09-04 ### Changed diff --git a/agent_core/runtime/loop/compact.py b/agent_core/runtime/loop/compact.py index 3a46e72..4f1c6cb 100644 --- a/agent_core/runtime/loop/compact.py +++ b/agent_core/runtime/loop/compact.py @@ -30,6 +30,7 @@ "StringSliceCompactor", "compact_messages", "compress_tool_results", + "default_recovery_footer", "estimate_tokens", "partition_for_compaction", "tool_names_by_call_id", @@ -77,6 +78,28 @@ URL_RE = re.compile(r'https?://[^\s\)>"\'<]+') _TOOL_RESULT_COMPACT_MAX_CHARS = 1_200 + +def default_recovery_footer(spill_path: str) -> str: + """Render the last line of a mini card: the handle that fetches the body back. + + Deliberately says nothing about HOW to fetch it. A host's recovery tool is + the host's own — it may be named anything, take different arguments, or not + be bound for this agent at all — and a footer naming a tool the agent cannot + call is worse than no footer (see ``_spill_footer``, which gates its own + prose on the tool being in the tool map for exactly that reason). This + default therefore carries only the handle, which is correct everywhere. + + Hosts that DO bind a recovery tool, and know it is bound for this agent, + should pass ``recovery_footer`` to say so in prose: a card is the only thing + left on the message, so unlike ``_spill_footer``'s site there is no + surviving instruction next to it telling the model what the handle is for. + Write that prose as prose. ``recover_result(spill_id="...")`` renders to the + model as source code, and it responds in kind — on a live run it reproduced + such a footer inside a ```bash block instead of emitting a tool call, and + ``LeakedToolCallRetryObserver`` fired twice. + """ + return f"[Full text] {spill_path}" + # Header of the spill recovery index. This is presentation only — the text the # MODEL reads above the paths — since the index is identified by # ``Message.spill_refs``. The two remaining substring checks against it @@ -574,6 +597,11 @@ class KeepLastNToolResultsCompactor: ``keep_tool_result == -1`` disables filtering entirely. + ``recovery_footer`` renders that last line. The default carries the handle + and nothing else, because this module cannot know what a host's recovery tool + is called or whether it is bound; a host that does know should pass prose + naming it. See :func:`default_recovery_footer`. + Caveat: only ``ToolMessage`` content is redacted. Workflows that inject large content as ``HumanMessage`` (e.g. an observer that splices fan-in reports between turns) bypass this compactor; pair @@ -586,6 +614,7 @@ def __init__( keep_tool_result: int, protect_tool_names: frozenset[str] = frozenset(), spill: Callable[[str, str], str | None] | None = None, + recovery_footer: Callable[[str], str] = default_recovery_footer, ) -> None: if keep_tool_result < -1: raise ValueError(f"keep_tool_result must be >= -1 (got {keep_tool_result})") @@ -597,6 +626,7 @@ def __init__( # age only, so existing callers are unaffected. self._protect = frozenset(protect_tool_names) self._spill = spill + self._recovery_footer = recovery_footer def compact( self, @@ -655,7 +685,7 @@ def compact( if card: placeholder += "\n" + card if spill_path: - placeholder += f"\n[Full text] {spill_path}" + placeholder += "\n" + self._recovery_footer(spill_path) # Without a pointer, replacing the body DESTROYS it, so a card that # is not even shorter is a pure loss and we keep the body. With one # we always replace, even when the card is longer: the pointer only diff --git a/agent_core/runtime/loop/tiered_compact.py b/agent_core/runtime/loop/tiered_compact.py index 4090e4f..d9f0ce2 100644 --- a/agent_core/runtime/loop/tiered_compact.py +++ b/agent_core/runtime/loop/tiered_compact.py @@ -18,6 +18,7 @@ INPUT_ESTIMATE_KEY, KeepLastNToolResultsCompactor, compress_tool_results, + default_recovery_footer, estimate_tokens, tool_names_by_call_id, ) @@ -215,6 +216,7 @@ def __init__( summary_retries: int = 2, summary_retry_timeout_s: float | None = None, prompt_builder: SummaryPromptBuilder = compaction_prompt, + recovery_footer: Callable[[str], str] = default_recovery_footer, manifest_max_paths: int | None = _SPILL_MANIFEST_MAX_PATHS, manifest_max_chars: int | None = _SPILL_MANIFEST_MAX_CHARS, ) -> None: @@ -233,10 +235,16 @@ def __init__( spill_callback = spill or ( spill_store.spill_compacted_body if spill_store is not None else None ) + # Forwarded, not defaulted here: Tier 1's card is the ONLY thing left on + # a message it rewrites, and Tier 1 is a winning candidate in its own + # right (``best_label == "tier1"``), so a host that worded the footer for + # the standalone compactor must get the same wording through this one or + # the same run renders two different footers. self._tier1 = KeepLastNToolResultsCompactor( keep_tool_result=keep_tool_result, protect_tool_names=protect_tool_names, spill=spill_callback, + recovery_footer=recovery_footer, ) # Tier 1 is not the only candidate that can win, and the others rewrite # the SAME protected results — which Tier 1 also keeps out of its spill diff --git a/pyproject.toml b/pyproject.toml index e4f5918..1895479 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "apodex-agent-core" -version = "0.7.0" +version = "0.8.0" description = "Shared, product-neutral runtime primitives for Apodex agents" readme = "README.md" license = "Apache-2.0" diff --git a/tests/test_keep_last_n_compactor.py b/tests/test_keep_last_n_compactor.py index e513bf4..d31973c 100644 --- a/tests/test_keep_last_n_compactor.py +++ b/tests/test_keep_last_n_compactor.py @@ -18,6 +18,7 @@ OMITTED_TOOL_RESULT_PLACEHOLDER, KeepLastNToolResultsCompactor, _args_preview, + default_recovery_footer, ) @@ -218,3 +219,85 @@ def test_loop_cap_handle_is_used_when_no_spill_ref_is_pinned(): ) tool_out = next(m for m in out if m.get("role") == "tool") assert tool_out["spill_refs"] == ["/spill/loop-cap"] + + +# --- the recovery footer hook ---------------------------------------------- + + +def test_the_default_footer_names_no_tool(): + """A card is the whole message, so a wrong tool name here has no antidote. + + This module cannot know what a host calls its recovery tool, or whether that + tool is bound for the agent whose history this is. Carrying only the handle + is the one rendering that is correct in every host, which is why it is the + default rather than a guess at the common case. + """ + body = "see https://example.com/a " + "x" * 2_000 + content = _blanked( + _one_call("web_search", '{"query": "x"}', body), + spill=lambda _n, _c: "/spill/xyz", + ) + assert content.splitlines()[-1] == default_recovery_footer("/spill/xyz") + assert "recover" not in content.splitlines()[-1].lower() + + +def test_a_host_footer_replaces_the_last_line_and_nothing_else(): + body = "see https://example.com/a " + "x" * 2_000 + messages = _one_call("web_search", '{"query": "x"}', body) + default = _blanked(messages, spill=lambda _n, _c: "/spill/xyz") + hosted = _blanked( + messages, + spill=lambda _n, _c: "/spill/xyz", + recovery_footer=lambda ref: f"[Saved. Fetch it with fetch_body id {ref}.]", + ) + + assert hosted.splitlines()[-1] == "[Saved. Fetch it with fetch_body id /spill/xyz.]" + # The card above the footer — call line and source URLs — is untouched, so a + # host swapping the footer cannot silently change the card's token budget. + assert hosted.splitlines()[:-1] == default.splitlines()[:-1] + + +def test_a_host_footer_still_reaches_the_model_only_when_a_body_spilled(): + """No handle, no footer — a host renderer must not invent one. + + The card is emitted for unspilled bodies too (that is the whole point of the + args + URLs lines). Calling the footer there would have it render a pointer + to nothing. + """ + calls: list[str] = [] + + def footer(ref: str) -> str: + calls.append(ref) + return f"[Saved: {ref}]" + + content = _blanked( + _one_call("web_search", '{"query": "x"}', "see https://example.com/a " + "x" * 2_000), + recovery_footer=footer, + ) + assert calls == [] + assert "[Saved:" not in content + + +def test_a_host_footer_survives_a_second_pass_unnested(): + """Idempotency is anchored on the placeholder, not the footer's wording. + + A host footer is free to be longer than the default, which is exactly the + case where a second pass re-carding the message would compound. The + already-placeheld check has to catch it regardless of what the last line says. + """ + body = "see https://example.com/a " + "x" * 2_000 + messages = _one_call("web_search", '{"query": "x"}', body) + compactor = KeepLastNToolResultsCompactor( + keep_tool_result=0, + spill=lambda _n, _c: "/spill/xyz", + recovery_footer=lambda ref: ( + "[Full text saved. Recovery id: " + ref + " — use the recover_result tool " + "(a tool call, not a shell command) with that spill id.]" + ), + ) + once = compactor.compact(messages, 0) + twice = compactor.compact(once, 0) + first = next(m["content"] for m in once if m.get("role") == "tool") + second = next(m["content"] for m in twice if m.get("role") == "tool") + assert first == second + assert second.count("Recovery id:") == 1 diff --git a/tests/test_tiered_compact.py b/tests/test_tiered_compact.py index 4948c4e..72abb33 100644 --- a/tests/test_tiered_compact.py +++ b/tests/test_tiered_compact.py @@ -15,6 +15,7 @@ OMITTED_TOOL_RESULT_PLACEHOLDER, SPILL_MANIFEST_HEADER, KeepLastNToolResultsCompactor, + default_recovery_footer, estimate_tokens, ) from agent_core.runtime.loop.tiered_compact import ( @@ -268,3 +269,56 @@ def test_overlong_handle_removes_an_old_index_without_inserting_an_empty_one(): for m in twice ) assert not any(SPILL_MANIFEST_HEADER in str(m.get("content")) for m in twice) + + +def test_tiered_forwards_the_recovery_footer_to_its_tier1(): + """A host that worded the footer must not get two wordings in one run. + + Tier 1 is a winning candidate in its own right, so its card reaches the model + from inside ``TieredCompactor`` exactly as it does from the standalone + compactor. If this constructor swallowed ``recovery_footer``, a workflow whose + primary path is tiered and whose fallback path is standalone would render the + handle-only default on every production turn and the host's prose only in the + fallback — the harder failure to notice of the two. + """ + async def run(): + footer = lambda ref: f"[Saved. Ask fetch_body for {ref}.]" # noqa: E731 + tiered = TieredCompactor( + keep_tool_result=1, + summary_llm=_FakeLLM(), + relief_target=10**9, + spill=lambda _n, _c: "/spill/xyz", + recovery_footer=footer, + ) + out = await tiered.compact(_msgs(), 1) + carded = [ + m.get("content") or "" + for m in out + if (m.get("content") or "").startswith(OMITTED_TOOL_RESULT_PLACEHOLDER) + ] + assert carded, "expected Tier 1 to card at least one tool body" + for content in carded: + assert content.splitlines()[-1] == "[Saved. Ask fetch_body for /spill/xyz.]" + + asyncio.run(run()) + + +def test_tiered_default_footer_is_unchanged(): + async def run(): + tiered = TieredCompactor( + keep_tool_result=1, + summary_llm=_FakeLLM(), + relief_target=10**9, + spill=lambda _n, _c: "/spill/xyz", + ) + out = await tiered.compact(_msgs(), 1) + carded = [ + m.get("content") or "" + for m in out + if (m.get("content") or "").startswith(OMITTED_TOOL_RESULT_PLACEHOLDER) + ] + assert carded + for content in carded: + assert content.splitlines()[-1] == default_recovery_footer("/spill/xyz") + + asyncio.run(run()) diff --git a/uv.lock b/uv.lock index d7829c1..24876fa 100644 --- a/uv.lock +++ b/uv.lock @@ -50,7 +50,7 @@ wheels = [ [[package]] name = "apodex-agent-core" -version = "0.7.0" +version = "0.8.0" source = { editable = "." } dependencies = [ { name = "anthropic", extra = ["bedrock"] },