fix(truncation): combine compressor marker + write guard (#83714) - #83858
Closed
djbclark wants to merge 3 commits into
Closed
fix(truncation): combine compressor marker + write guard (#83714)#83858djbclark wants to merge 3 commits into
djbclark wants to merge 3 commits into
Conversation
… 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>
…n placeholder Fixes NousResearch#83714. write_file_tool and patch_tool (both replace and V4A modes) wrote the model's function-call arguments straight to disk with no check for AI truncation-placeholder markers like "...[truncated]". When a model (observed with deepseek-v4-pro via deepseek) abbreviated a long new_string instead of emitting it in full, the literal marker text landed in the file, corrupting it. No length-based truncation of `content`/`new_string`/`patch` exists anywhere between JSON-parsing the tool call and the write/patch calls, so this rules out Hermes truncating the parameter in transit. The likely mechanism: Hermes itself uses this exact marker to signal truncated tool *output* elsewhere (todo_tool.py, file_operations.py's per-line cap, mcp_tool.py), and the model is imitating a pattern from its own context. Adds `_find_truncation_placeholder()` and wires it into: - write_file_tool: scans `content` - patch_tool replace mode: scans `new_string`, skipping markers already present in `old_string` (avoids false positives on legitimate edits to text that mentions the marker) - patch_tool V4A mode: scans only ADDED content via `_extract_v4a_added_content()` (parses the patch and joins '+' hunk lines plus Add-File bodies), not the whole patch text, so a patch that removes or merely anchors context on a placeholder-like literal isn't flagged Falls back to raw-text scanning if the V4A patch fails to parse, so detection degrades gracefully instead of silently skipping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…unt-based guard - Add tools/truncation_markers.py as the single source for the HERMES-CONTEXT-COMPRESSION template and write-path detection - context_compressor imports the shared marker (no drift with the guard) - file_tools uses count-based comparison (NousResearch#68512) and also refuses the new compressor marker plus AI "unchanged"/"rest of file" stubs - Regression tests for count-based allowance and compression-marker reject Combines NousResearch#83843 + NousResearch#83752 + the clearly-better pieces of NousResearch#68512 for local deploy.
alt-glitch
added
type/bug
Author
|
Closing this as redundant noise, not as a rejection of the work. The canonical upstream path for #83714 is the two focused PRs already open:
This combined PR was only an operator convenience branch for a local install. It is not needed for review, and leaving three overlapping PRs open is harder on maintainers than two clean ones. Optional follow-ups (shared marker module, count-based comparison inspired by #68512) can land as small commits on #83752 / #83843 if wanted after those merge — no third PR required. Sorry for the extra surface area. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Combined local-deploy stack for #83714 that merges:
...[truncated]in compressed assistant tool_call argstools/truncation_markers.pyso compressor marker text and write-guard detection cannot drift; refuse⟪HERMES-CONTEXT-COMPRESSION…⟫on writeWhy a combined PR
The two independent PRs are still valid and MERGEABLE on their own. This PR is the integration branch we deployed locally (plus the shared-constant follow-up requested after review). Maintainers can land the split PRs or this combined one.
Tests
pytest tests/tools/test_file_tools.py tests/agent/test_context_compressor.py tests/test_trajectory_compressor.py -q→ 216 passed, 2 skipped, 2 pre-existing macOS
/tmpvs/private/tmpmock failures (unrelated).Relationship
Refs: #83714, #83752, #83843, #68512, #11762, #20805