Skip to content

fix(compat): unify Read/Edit content hashing — non-UTF-8 files lock into a permanent stale-read loop (#177)#199

Merged
OGtwelve merged 1 commit into
mainfrom
fix/read-hash-content-177
Jul 14, 2026
Merged

fix(compat): unify Read/Edit content hashing — non-UTF-8 files lock into a permanent stale-read loop (#177)#199
OGtwelve merged 1 commit into
mainfrom
fix/read-hash-content-177

Conversation

@OGtwelve

@OGtwelve OGtwelve commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Closes #177. First of the XS P1 batch from the parity verification (#196).

Problem

Read fingerprinted raw file bytes (hashFile) while Edit/MultiEdit compare against hashContent(await readFile(safe, 'utf8')). For any file whose bytes are not UTF-8-roundtrippable (Latin-1 remnants, mixed encodings), the two hashes never agree: Edit rejects with "modified since you read it", re-reading records the same mismatching byte-hash, and the tool pair locks into a permanent stale loop that burns the whole step budget (probe-verified).

Fix

Read now fingerprints hashContent(await readFile(safe, 'utf8')) — the same normalization Edit compares — so a Read→Edit round-trip agrees on non-UTF-8 files, while a genuine external modification is still detected (different decoded content → different hash). hashFile stays in file-state.ts for callers that want byte identity.

Tests (edit-tools.test.ts, where the Read+Edit interaction lives)

  • A 0xE9-byte fixture survives Read→Edit (the edit lands, no stale rejection).
  • An external modification after Read on the same non-UTF-8 file is still rejected.
  • Green: typecheck (both) + test:node (compat 252) + biome.

Part of #196.

Summary by CodeRabbit

  • Bug Fixes
    • Improved file editing reliability for files containing non-UTF-8 characters.
    • Prevented false “modified since you read it” errors after reading such files.
    • Continued detecting genuine external changes before applying edits.
    • Added coverage to verify successful edits and accurate replacement counts.

…UTF-8 stale-read loop (#177)

Read recorded hashFile(path) (raw bytes) while Edit/MultiEdit compare
hashContent(readFile(utf8)). For any file whose bytes are not UTF-8-roundtrippable
(Latin-1 remnants, mixed encodings), the two hashes never agree: Edit rejects with
'modified since you read it', re-reading records the same mismatching byte-hash,
and the pair locks into a permanent stale loop that burns the whole step budget.

Read now fingerprints hashContent(readFile(utf8)) — the same normalization Edit
compares — so a Read->Edit round-trip agrees on non-UTF-8 files while a genuine
external modification is still detected. hashFile stays in file-state.ts for
callers wanting byte identity.

Tests (edit-tools.test.ts, where the Read+Edit interaction lives): a 0xE9-byte
fixture survives Read->Edit (lands, no stale rejection); an external modification
after Read on the same non-UTF-8 file is still rejected.

Part of #196.
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

readFileTool now fingerprints UTF-8-decoded content, and editFileTool tests verify both successful edits and detection of genuine external changes for non-UTF-8 files.

Changes

Read/Edit hashing

Layer / File(s) Summary
UTF-8 fingerprinting
packages/ai-claude-compat/src/fs-tools.ts
readFileTool now hashes the complete file after UTF-8 decoding, matching the content representation used by edit validation.
Non-UTF-8 edit validation
packages/ai-claude-compat/src/edit-tools.test.ts
Tests verify that edits succeed after reading non-UTF-8 content and that genuine external modifications are rejected.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: unifying Read/Edit hashing to fix the non-UTF-8 stale-read loop.
Linked Issues check ✅ Passed The changes align with #177: Read now hashes UTF-8-decoded content, and tests cover successful Read→Edit plus external modification rejection.
Out of Scope Changes check ✅ Passed The PR only adjusts Read hashing and adds focused tests for the non-UTF-8 compatibility fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/read-hash-content-177

Comment @coderabbitai help to get the list of available commands.

@OGtwelve

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/ai-claude-compat/src/fs-tools.ts (1)

85-85: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff

Full-file buffering for fingerprinting, even on windowed reads.

readFile(safe, 'utf8') now decodes and buffers the entire file into memory purely to compute the fingerprint, even when only a small window (offset/limit) was requested. For very large files this is more memory-intensive than a streaming hash would be (the old hashFile likely streamed). This is a reasonable tradeoff to guarantee hash parity with readForEdit, but worth keeping in mind if large-file reads become a hot path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/ai-claude-compat/src/fs-tools.ts` at line 85, Update the
fingerprinting in the file-read flow around init.fileState.record to avoid
decoding and buffering the entire file for windowed reads, while preserving hash
parity with readForEdit. Reuse the existing streaming hash behavior, such as
hashFile, where appropriate and keep the recorded fingerprint unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/ai-claude-compat/src/fs-tools.ts`:
- Line 85: Update the fingerprinting in the file-read flow around
init.fileState.record to avoid decoding and buffering the entire file for
windowed reads, while preserving hash parity with readForEdit. Reuse the
existing streaming hash behavior, such as hashFile, where appropriate and keep
the recorded fingerprint unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: edaf49c7-fa20-477c-833c-1cfa329c6c16

📥 Commits

Reviewing files that changed from the base of the PR and between da2719f and 404bdad.

📒 Files selected for processing (2)
  • packages/ai-claude-compat/src/edit-tools.test.ts
  • packages/ai-claude-compat/src/fs-tools.ts

@OGtwelve OGtwelve merged commit 8221395 into main Jul 14, 2026
4 checks passed
@OGtwelve OGtwelve deleted the fix/read-hash-content-177 branch July 14, 2026 05:49
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.

fix(compat): unify Read/Edit content hashing — non-UTF-8 files lock into a permanent stale-read loop

1 participant