fix(file-tools): abort write/patch when AI truncation placeholders detected

[ygd58]

Context

Ports #64233 forward onto current main per @teknium1's review.

Problem

AI models sometimes emit placeholder strings like // ... unchanged ... or /* ... full function ... */ when they omit sections they consider unmodified. Writing these produces corrupt files (issue #20805).

Fix

_check_truncation_signatures() applied at three backend-aware pipeline points: write_file_tool (unconditional), patch_replace new_string (compared against old_string), patch_v4a patch content (only the ADDED content).

Per review: the V4A check previously scanned the complete serialized patch text, which would false-positive on a legitimate edit that removes, or merely anchors context on, a pre-existing placeholder-like literal. Added _extract_v4a_added_content(), which parses the patch via tools.patch_parser.parse_v4a_patch() and joins only + hunk lines and full Add-file content, falling back to raw-text scanning if the patch fails to parse.

Verification

Added direct patch_tool coverage for both changed branches (replace and V4A) plus the V4A false-positive regression cases (placeholder on a removed line, on a context line, and in a brand-new Add-File). 16/16 new tests pass; 64/64 in the full tests/tools/test_file_tools.py file.

[teknium1]

Thanks for carrying the guard forward and addressing the earlier V4A false-positive review. Current main still forwards raw content through write_file_tool (tools/file_tools.py:1600,1621) and patch_tool (tools/file_tools.py:1766,1770), so the premise remains valid. The V4A extraction matches the parser’s application model (tools/patch_parser.py:477-484), which writes Add File content from + lines only.

Problems

• tools/file_tools.py:1602 treats an occurrence in original as permission for every same-signature occurrence in content. If old_string already contains one literal // ... unchanged ..., a new_string that retains it and adds a second omitted section passes the guard and reaches patch_replace at tools/file_tools.py:1862.

Suggested changes

• Compare per-signature occurrence counts after case normalization, rejecting when proposed content adds an occurrence beyond original.
• Add a replace-path regression for one existing occurrence plus one introduced duplicate, asserting the backend is not called.

Automated hermes-sweeper review.

[teknium1]

This boolean membership check allows a newly introduced duplicate placeholder whenever original already contains the same literal once. Compare normalized occurrence counts (and reject an increase) so retaining one pre-existing literal remains allowed but adding a second omitted section is blocked.

[ygd58]

Fixed -- now compares per-signature occurrence COUNTS (case-normalized) between content and original, rejecting when content's count exceeds original's, instead of a boolean membership check. A retained pre-existing occurrence still passes (count unchanged); adding any occurrence beyond what original had is now blocked regardless of how many already existed.

Added the requested replace-path regression (one existing occurrence plus one introduced duplicate, asserting the backend is not called), plus direct unit tests for the count-exceeds and same-count (moved, not added) cases. 16/16 pass in TestTruncationSignatureGuard; 67/67 in the full file.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address the truncation-overwrite portion of #20849 by adding guards against AI omission placeholders; their diffs touch file-edit tooling and tests, not the issue’s separate handoff/context-retrieval, destructive-action, or memory concerns. #20857 is the initial guard, #64233 extends it across backend-aware edit paths, and #68512 corrects the V4A behavior and occurrence-count bypass with broader regression coverage.

Related pull requests

• fix(file-tools): abort write/patch when AI truncation placeholders detected #20857 [closed] duplicate — (+50/-0) — n/a: Adds placeholder detection for writes and replacement strings, but uses host-side pathlib, does not inspect V4A patch payloads, and adds no tests. Despite its keep_open verdict, it remains relevant as the closed original implementation superseded by fix(file-tools): abort write/patch when AI truncation placeholders detected #68512, whose diff provides backend-aware V4A handling and regression coverage.
• fix(file-tools): abort write/patch when AI truncation placeholders detected #64233 [closed] duplicate — (+116/-0) — n/a: Extends the guard to backend-aware write, replace, and V4A paths with initial tests, but scans the entire serialized V4A patch and can reject placeholders appearing only on removed or context lines. Despite its keep_open review, it was closed in favor of fix(file-tools): abort write/patch when AI truncation placeholders detected #68512, whose diff parses V4A operations and checks only added content while testing those false-positive cases.
• fix(file-tools): abort write/patch when AI truncation placeholders detected #68512 best fix — (+364/-0) — n/a: The consolidated successor checks writes, count increases in replacement strings, and only added V4A hunk or Add-file content, with direct tests for each path and the reported false-positive regressions. Its current diff explicitly implements the count-based comparison and duplicate-occurrence regression requested by the visible keep_open review, preserving a concrete salvage path for this portion of [Bug/Architecture] Severe context loss, truncation-overwrites, and memory limitations during complex coding workflow #20849.

Duplicates

#20857, #64233, and #68512 implement the same truncation-placeholder guard; #64233 supersedes the initial shape of #20857, and #68512 is the corrected forward port that supersedes both closed PRs.

Suggested consolidation

Keep #68512 open with a salvage path: retain its count-based replacement guard, parsed V4A added-content validation, and direct write/replace/V4A regression tests, then obtain maintainer confirmation that the keep_open review’s occurrence-count blocker is resolved at the current head. Keep #20857 and #64233 closed as superseded duplicates of #68512, and track the other architectural causes in #20849 separately.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I20849(["issue #20849 (open)"])
    subgraph Dup20857 ["PRs duplicating each other"]
        P20857["PR #20857 (closed)"]
        P64233["PR #64233 (closed)"]
        P68512["PR #68512 (open)"]
    end
    P68512 -->|best fix| I20849
    class I20849 open
    class P20857 closed
    class P64233 closed
    class P68512 open
    class P68512 best
    class P68512 target
    click I20849 "https://github.com/NousResearch/hermes-agent/issues/20849"
    click P20857 "https://github.com/NousResearch/hermes-agent/pull/20857"
    click P64233 "https://github.com/NousResearch/hermes-agent/pull/64233"
    click P68512 "https://github.com/NousResearch/hermes-agent/pull/68512"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 30 kB of PR diffs, 7 kB of issue/PR text, 7 kB of discussion (9 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[djbclark]

Thanks for the earlier work here — especially the count-based original-vs-content comparison and the AI "unchanged"/"rest of file" signature list. Those pieces are folded into the combined stack for #83714 (branch djbclark:fix/truncation-combined-83714) together with the compressor root-cause fix. This PR is still CONFLICTING on main; happy for maintainers to close as superseded by the combined PR / #83752+#83843 once landed, with credit retained via the design borrow.

[djbclark]

Quick housekeeping note from the #83714 thread (no action needed from you unless you want it):

We hit a related failure mode where models wrote a literal ...[truncated] into files. Root cause investigation landed on context-compressor priming (#83843), with a write-path guard in #83752.

Your PR was genuinely useful prior art — especially:

• count-based original-vs-content comparison (membership-only is not enough)
• the broader “unchanged / rest of file” signature list
• V4A added-content-only scanning

#83752 covers the same three pipeline points on current main (and is MERGEABLE). #68512 is still CONFLICTING. From an operator perspective, the cleanest outcome for maintainers is probably:

• land fix(context-compressor): stop priming models to abbreviate their own tool-call arguments #83843 + fix(tools): refuse patch/write_file when content contains a truncation placeholder #83752 for [Bug]: patch tool truncates new_string content with literal '...[truncated]' text #83714, and/or
• rebase/fold any missing pieces from this PR into that guard if you still want the broader signature set

Either way, thank you — the design points above influenced how we described residual risk and follow-ups. No expectation that you chase the rebase unless you want to.