Skip to content

fix(context-compressor): stop priming models to abbreviate their own tool-call arguments - #83843

Closed
djbclark wants to merge 1 commit into
NousResearch:mainfrom
djbclark:fix/compressor-truncation-marker-priming
Closed

djbclark wants to merge 1 commit into
NousResearch:mainfrom
djbclark:fix/compressor-truncation-marker-priming

Conversation

@djbclark

@djbclark djbclark commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Root-cause fix for #83714.

During context compression, _truncate_tool_call_args_json() shrinks long string values inside past assistant tool_calls[].function.arguments — content that is later replayed to the model as its own prior output. The old suffix was a bare ...[truncated], which is easy for a model to imitate in a new tool call (and then write to disk).

This is the same marker family that previously caused #11762 (invalid JSON after raw-string slicing). That fix kept JSON valid but left the imitable marker text.

Change

Replace the bare marker with a deliberately non-prose-shaped disclaimer:

⟪HERMES-CONTEXT-COMPRESSION: {omitted} of {total} chars omitted here
by Hermes's context compressor. This is NOT part of the original tool
call and must never be reproduced in new output — always write full,
untruncated content.⟫

Other ...[truncated] sites in the compressor (summarizer input, fallback recap, user-only paths) are left alone — they are not replayed as assistant tool-call args.

Suggested landing order

  1. This PR (fix(context-compressor): stop priming models to abbreviate their own tool-call arguments #83843) — stop minting the bad primer
  2. fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752 — fail closed on the write path if any residual imitation still happens (tool-result markers, already-compacted history, etc.)

Testing

  • Updated existing TestTruncateToolCallArgsJson expectations for the new marker
  • Added TestTruncationMarkerNotImitable coverage
  • pytest tests/agent/test_context_compressor.py tests/test_trajectory_compressor.py -q — green on the PR branch

Related

Happy to take review feedback or split further if useful.

@djbclark
djbclark requested a review from a team August 11, 2026 10:58
… into replayed tool_calls

Root cause for NousResearch#83714 (write_file/patch_tool writing literal
"...[truncated]" into files, PR NousResearch#83752's guard is the safety net, not
the fix): _truncate_tool_call_args_json() in the compression pass
shrinks long string values inside a PAST assistant message's
tool_calls[].function.arguments — the exact field that represents the
model's own prior generated output, replayed back to it verbatim on
every subsequent turn. The old marker, a bare "...[truncated]" suffix,
is indistinguishable from something the model itself could have
written (it's exactly the kind of terse ellipsis abbreviation models
already produce). A model conditioned on seeing itself "get away with"
that pattern in its own history imitates it in a new tool call,
writing the literal marker instead of real content.

This is the second bug from the same root text. The first (NousResearch#11762,
MiniMax 400s from unterminated JSON) was fixed by shrinking inside the
parsed structure so the JSON stays valid, but kept the same visible
marker text — fixing the syntax problem while leaving the imitation
problem untouched.

Fix: replace the marker with one deliberately NOT shaped like prose a
model would write — distinctive non-ASCII delimiters, an explicit "not
part of the original tool call" disclaimer, and a per-instance
char-count that won't match the next omission point even if copied
verbatim. The shrunk value stays a plain string (not a nested object)
so the NousResearch#11762 valid-JSON/matching-shape contract is unchanged — only
the marker text changed.

Checked context_compressor.py's other "...[truncated]" call sites
(_serialize_for_summary, _compact_fallback_turn, the user-message-only
one near _ACTIVE_TASK_MAX_CHARS) — none of them write into a value
that gets replayed as the main model's own assistant/tool_calls
history, so they don't share this priming risk and were left as-is.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@djbclark

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main (dropped unrelated fork-main commit that bloated the diff). Now MERGEABLE, 2 files / +114/−4. Local pytest: 163 passed on compressor + trajectory tests. Complementary symptom guard: #83752. Issue: #83714.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint area/compression Context compression and continuation sessions sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 11, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: #83735 fixes the same model-visible context-compression marker by removing it entirely; this PR retains a deliberately non-prose marker. Maintainer choice is needed.

@djbclark

Copy link
Copy Markdown
Contributor Author

Review follow-up after local verification + cross-check of remaining markers

This is the right root-cause fix for the #83714 mechanism we confirmed (compressor rewriting past assistant tool_calls[].function.arguments with an imitable bare ...[truncated]). Local pytest on the rebased branch: 163 passed (test_context_compressor + test_trajectory_compressor). A few residual risks that are clearly worth calling out — none invalidate the approach.

1. This does not scrub already-compacted session history

Sessions that already have old ...[truncated] inside stored assistant tool_call args will keep priming until those turns age out or are re-compressed/rewritten. The code change only affects future shrinks.

Optional follow-up (not required for this PR): on load/replay, rewrite known old markers inside assistant tool_call argument strings to the new non-imitable form (or strip the suffix). That would heal long-lived gateway sessions without waiting for natural turnover.

2. Other model-visible ...[truncated] sources remain (by design here, but incomplete as sole fix)

Agree with leaving _serialize_for_summary / _compact_fallback_turn / user-only paths alone for the assistant-tool_call imitation path. Separately, though, ordinary tool outputs still inject the old family into context every turn, e.g.:

  • tools/file_operations.py line truncation → ... [truncated]
  • tools/todo_tool.py / tools/registry.py… [truncated]
  • tools/mcp_tool.py, tools/delegate_tool.py, etc.

Those are role=tool (not “I generated this tool call”), so lower imitation risk than assistant tool_call args — but they are still a near-identical token pattern in context. Combined with #83752 this is fine; without the write guard, residual priming is still plausible.

3. Keep #83752 in lockstep with the new marker text

#83752’s _find_truncation_placeholder today matches the old marker family only. After this lands, please either:

  • extend the write/patch guard to also refuse HERMES-CONTEXT-COMPRESSION / , or
  • share one constant/helper between compressor + file_tools so the guard always knows what the compressor emits.

Otherwise the safety net does not cover “model copied the new disclaimer marker.”

4. Minor test nit (non-blocking)

test_old_bare_marker_no_longer_produced asserts "...[truncated]" not in shrunk. That’s correct for pure y*600 payloads. If a future change ever head-preserves user content that legitimately contained that substring in the first head_chars, the assert would be a false failure. Prefer asserting the suffix/marker region (e.g. ends with / contains HERMES-CONTEXT-COMPRESSION and does not append the old bare marker) rather than “nowhere in string.”

5. Landing order

Recommend: this PR first (stop minting the bad primer), then #83752 (fail closed on any remaining imitation, including tool-output-shaped markers and any copy of the new marker once extended). Both still justified.

@djbclark

Copy link
Copy Markdown
Contributor Author

Combined with write-guard + shared marker module on djbclark:fix/truncation-combined-83714 (compressor still uses the non-imitable marker; constant now lives in tools/truncation_markers.py so the guard stays in lockstep).

@djbclark

Copy link
Copy Markdown
Contributor Author

Housekeeping: description rewritten for a cleaner review path.

Canonical pair for #83714: this PR (root cause) → then #83752 (write guard).

The combined stack PR #83858 is closed as redundant (local-deploy convenience only). Please ignore it.

Thank you for your time reviewing.

@yuzilongleif-collab

Copy link
Copy Markdown
Contributor

Independent verification on exact head 509c5d82647669d879ea486af74ab7b286cf90b2 (Linux x86_64, Python 3.11):

pytest -q tests/agent/test_context_compressor.py tests/test_trajectory_compressor.py163 passed.

Our real failure sequence was consistent with this PR's root-cause model: a past assistant tool_calls[].function.arguments value had been replayed with a bare ...[truncated] suffix, and a later file-tool call imitated that omission while remaining valid JSON. The new non-prose marker preserves the parsed-string/valid-JSON contract in the exact-head tests.

No additional blocker found in this verification. The companion write-path guard in #83752 remains valuable for residual/old markers.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(context-compressor): stop priming models to abbreviate their own tool-call arguments

Good idea executed carefully — replacing the prose-shaped "...[truncated]" with a distinctive, self-disclaiming marker is a sensible anti-imitation measure, and the tests pin the properties well. One substantive observation and a couple of minors:

  1. Marker length vs. compression goal: the template is ~370+ chars. For a string just above head_chars (e.g. 201 chars with head_chars=200), the shrunk value (obj[:200] + marker) is roughly 3x the original — the "compression" inflates the payload and increases token cost for exactly the small-overflow cases. The existing assert len(shrunk) < len(original) passes only because its inputs are large; the small-boundary case is untested and behaves against intent. Consider truncating only when len(obj) - head_chars exceeds the marker length (or some minimum omission), otherwise leave the string untouched — the marker's purpose is anti-imitation, not to shrink small overflows.

  2. Minor: the {omitted:,}/{total:,} grouping is locale-independent (fixed ,), good. Just confirm no downstream consumer treats tool-call args as strictly-JSON-content (the marker is plain text inside a JSON string, so validity is preserved — the existing valid-JSON tests cover this).

  3. Minor: prior compressed history already containing the old "...[truncated]" marker is unaffected (only new truncations change), so fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752's write-guard remains load-bearing for old sessions. That pairing is good defense in depth — worth one sentence in the PR description (if not already there) so reviewers understand fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752 is not made redundant by this change.

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Landed on main as #116169 (rebase merge, 39abfdf4ec). Your commit is preserved verbatim — 262a6436fa, author Daniel JB Clark — with the follow-ups on top; the contributor map landed as 6339f7a2bb.

Point-by-point on this thread's open items:

  • Marker text (your design): landed as written — ⟪HERMES-CONTEXT-COMPRESSION: N of M chars omitted … must never be reproduced …⟫, exposed as _COMPRESSION_MARKER_PREFIX / _COMPRESSION_MARKER_TEMPLATE.
  • Your item 1 — already-compacted history: unchanged, as you said. This only affects future shrinks; old sessions age out or are re-shrunk. A write-path guard remains the safety net for markers already persisted (see item 3).
  • Your item 2 — other ...[truncated] sources: left alone, deliberately. The file_operations line clamp, todo/registry/mcp_tool/delegate_tool markers are role=tool text, never replayed as the model's own tool call, so they do not prime the [Bug]: patch tool truncates new_string content with literal '...[truncated]' text #83714 imitation path. The one site that did — _truncate_tool_call_args_json — is the only production writer of a marker into replayed assistant tool-call arguments.
  • Your item 3 — keep fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752 in lockstep: the new marker is in-tree now. fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752's _find_truncation_placeholder still matches only the old family, so it needs ⟪HERMES-CONTEXT-COMPRESSION added before it lands; I left a note there.
  • Your item 4 — test nit: fixed your way. The three assertions now check the marker region (parsed[...][200:].startswith(_COMPRESSION_MARKER_PREFIX)) plus the preserved head, instead of substring-anywhere.
  • @Enough1122's review, item 1 — small-overflow inflation: fixed. A leaf is now replaced only when the replacement is strictly shorter; a 628-char blob with a 201-char leaf used to grow to 863 chars and is now returned byte-identical. Pinned by a test.

Follow-ups on top of your commit (ours):

  • never re-shrink a marked leaf — the marker was rewritten on the next compaction with self-referential counts (2,800 of 3,000223 of 423), which destroyed the per-instance-count property your design relies on;
  • guard the marker by its whole-tail shape (startswith(prefix, head_chars) and endswith("⟫")) so the imitation shape itself — head + marker + new content — still shrinks;
  • return the caller's exact bytes when nothing was replaced — re-serialising compact wire JSON added separator spaces and was counted upstream as reclaimed pressure.

Deliberate trade-offs, documented in the function docstring: ~60 tokens per shrunk leaf against ~4 for the old marker, and leaves under the ~420-char break-even are now left intact instead of head-truncated.

Thanks — the diagnosis and the non-prose marker were the substance of this fix. Closing in favour of #116169; if you want any of the follow-ups shaped differently, say so and I will reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants