fix(file-tools): abort write/patch when AI truncation placeholders detected - #64233
fix(file-tools): abort write/patch when AI truncation placeholders detected#64233ygd58 wants to merge 1 commit into
Conversation
…tected AI models sometimes emit placeholder strings like '// ... unchanged ...' or '/* ... full function ... */' when they omit sections they consider unmodified. Writing these to disk produces corrupt files that cannot be compiled or run (issue NousResearch#20807). Fix: add _check_truncation_signatures() helper and apply it in the backend-aware edit pipeline at three points: 1. write_file_tool: check unconditionally before the write (no original to compare against; any placeholder in a write is wrong). 2. patch_replace (new_string): compare against old_string so a pre-existing placeholder in the region being replaced is not re-flagged. 3. patch_v4a (patch content): check unconditionally since V4A +/* ... full function ... */ is a common truncation pattern and there is no convenient old_string reference. All three checks go through the backend-aware _get_file_ops / ShellFileOperations pipeline, not host-side pathlib reads, so docker/modal/SSH environments see the same file as the eventual write. 5 tests in TestTruncationSignatureGuard: - _check_truncation_signatures detects placeholder - pre-existing placeholder in original is not re-flagged - clean content passes - write_file_tool blocks on placeholder - write_file_tool allows clean content Fixes NousResearch#20805
alt-glitch
added
type/bug
Duplicate of #20857 — both add the same |
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for extending the earlier guard to the V4A path; current main still forwards raw writes and V4A payloads at tools/file_tools.py:1723-1724 and tools/file_tools.py:1870-1873, so the underlying corruption risk is real.
Problems
- The new V4A check scans the complete serialized patch (
tools/file_tools.py:1894in this diff). V4A represents added, removed, and context lines distinctly (tools/patch_parser.py:186-197). This rejects a legitimate edit that removes or anchors on a pre-existing placeholder literal, contrary to the helper's pre-existing-content exception. TestTruncationSignatureGuardadds no directpatch_toolcoverage, so neither changed patch branch nor the V4A false-positive case is tested.
Suggested changes
- Parse the V4A patch and inspect only added lines / Add-file content before applying it.
- Add replace and V4A path tests, including a V4A patch with a placeholder on a removed or context line that remains allowed.
Automated hermes-sweeper review.
| return tool_error(_trunc_err) | ||
| result = file_ops.patch_replace(_replace_target, old_string, new_string, replace_all) | ||
| elif mode == "patch": | ||
| if not patch: |
There was a problem hiding this comment.
This scans the entire V4A payload, including - and context lines. The parser preserves those separately from + lines (tools/patch_parser.py:186-197), so a patch that removes or anchors on an existing literal placeholder is incorrectly rejected. Parse and validate only added content (including Add-file content) to preserve the stated pre-existing-signature exception.
teknium1
added
sweeper:risk-compatibility
|
Ported forward in #68512: V4A check now only inspects added ("+") hunk lines and Add-file content via patch_parser.parse_v4a_patch(), so removing or anchoring on a pre-existing placeholder-like literal no longer false-positives. Added direct patch_tool coverage for both changed branches plus the false-positive regression cases. Closing in favor of #68512. |
3 participants
Problem
AI models emit placeholder strings like // ... unchanged ... or /* ... full function ... */ when omitting sections. Writing these produces corrupt files (issue #20805).
Fix
_check_truncation_signatures() applied at 3 backend-aware pipeline points: write_file_tool (unconditional), patch_replace new_string (compared against old_string to allow pre-existing), patch_v4a patch content (unconditional). All checks go through _get_file_ops not host-side pathlib reads, so docker/modal/SSH see the same file.
Verification
5 tests: detects placeholder, allows pre-existing, clean content passes, write blocks on placeholder, write allows clean. 5/5 pass.
Fixes #20805