Skip to content

fix(tools): retain read-before-edit snapshots across turns#4475

Merged
Hmbown merged 2 commits into
Hmbown:mainfrom
Angel-Hair:fix/compaction-readfile-nodedup
Jul 17, 2026
Merged

fix(tools): retain read-before-edit snapshots across turns#4475
Hmbown merged 2 commits into
Hmbown:mainfrom
Angel-Hair:fix/compaction-readfile-nodedup

Conversation

@Angel-Hair

Copy link
Copy Markdown
Contributor

Summary

After compaction, edit_file rejects edits on previously-read files with "not been read in this session". The root cause is not in edit_file — it correctly requires a prior read_file invocation. The problem is in compaction: it deduplicates read_file results into <TOOL_RESULT_REF> references, telling the model "the content is still available", but the in-memory file_read_tracker is not updated. The model sees the content and assumes it can edit, but the tracker sees no record and rejects the edit. The two mechanisms give the model contradictory signals.

Root cause

compact_tool_result_for_wire classifies read_file as dedup-eligible and collapses identical large results into <TOOL_RESULT_REF>. The dedup logic saves tokens but skips the tool invocation — so note_file_read (which populates file_read_tracker) is never called. Meanwhile, edit_file's require_fresh_file_read checks file_read_tracker and rejects when no snapshot is found.

Fix

Exclude read_file from the dedup_eligible predicate so that after compaction, file contents are naturally absent from context. This makes the model's context accurately reflect what it can act on: file contents are gone, file_read_tracker is empty, everything is consistent. The model will naturally re-read files before editing.

Testing

  • cargo fmt -- --check
  • cargo clippy -p codewhale-tui (zero warnings)
  • cargo check -p codewhale-tui
  • cargo test -p codewhale-tui -- client::chat::stream_decoder (34/34 passed)

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes
  • Harvested/co-authored credit uses a GitHub numeric noreply address

@Angel-Hair
Angel-Hair requested a review from Hmbown as a code owner July 17, 2026 17:39
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @Angel-Hair for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

@Hmbown Hmbown added this to the v0.9.1 milestone Jul 17, 2026
…ections

After compaction, read_file results were deduplicated into TOOL_RESULT_REF
references. The model sees the content and assumes it can edit, but the
in-memory file_read_tracker has no record, causing edit_file to reject with
'not been read in this session'.

- Exclude read_file from the dedup_eligible predicate so that after
  compaction, file contents are naturally absent from context
- This makes file_read_tracker state consistent with what the model sees,
  causing the model to re-read files before editing (correct behavior)
@Angel-Hair
Angel-Hair force-pushed the fix/compaction-readfile-nodedup branch from 5b42e7f to e6795f7 Compare July 17, 2026 18:03
Maintainer amendment for PR Hmbown#4475: retain the contributor-reported edit_file failure, but fix its actual lifetime boundary. Tool-result dedup only serializes request history and never skips read_file execution, so preserve that cache-saving contract.

Share one FileReadTracker across transient turn contexts, while retaining stale-file rejection and the original contributor commit in history.

Signed-off-by: Hunter B <hmbown@gmail.com>
@Hmbown Hmbown changed the title fix: exclude read_file from compaction dedup to prevent edit_file rejections fix(tools): retain read-before-edit snapshots across turns Jul 17, 2026
@Hmbown

Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Maintainer amendment pushed at f6d6516d6 while preserving @Angel-Hair's original commit and authorship.

The reported failure is real, but wire compaction does not skip read_file execution: it only serializes already-executed tool results for a later model request. The actual mismatch was that Engine::build_tool_context created a fresh FileReadTracker for every turn even though the contract calls it session-scoped.

The amended PR now:

  • keeps one read tracker on the engine and shares it with each transient turn context;
  • preserves identical read_file result dedup and the existing retrieval contract;
  • preserves stale-file rejection after an observed file changes;
  • adds a cross-turn regression proving read -> later context -> edit authority works.

Local focused gates passed:

  • exact cross-turn tracker regression: 1 passed;
  • medium identical-result dedup regression: 1 passed;
  • large mutation-result inline/persistence regressions: 2 passed;
  • cargo fmt --all --check and git diff --check.

Fresh exact-head CI is now running. Thank you for surfacing this — the report led directly to the lifetime fix.

@Hmbown
Hmbown merged commit 89e85a9 into Hmbown:main Jul 17, 2026
14 checks passed
@Angel-Hair
Angel-Hair deleted the fix/compaction-readfile-nodedup branch July 17, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants