Skip to content

Per-file diff fingerprint depends on the file's position in the patch (trailing-newline artifact), silently defeating --resume reuse #718

Description

@FeilixX

OpenCodeReview Version

open-code-review v1.8.8 (6dc3eb0) darwin/arm64, built at 2026-08-04T13:09:52Z

Operating System

macOS (Apple Silicon)

Installation Method

npm (global)

LLM Provider

Other OpenAI-compatible endpoint (DeepSeek) — the defect is provider-independent: the fingerprint is computed before any LLM call.

Bug Description

The review-item fingerprint is not invariant to the file's position within the patch, so --resume silently re-reviews files whose content did not change.

reviewItemFingerprint hashes the extracted per-file diff text (internal/agent/agent.go:733 at 6dc3eb0):

func reviewItemFingerprint(mode string, d model.Diff) string {
	sum := sha256.Sum256([]byte(mode + "\x00" + d.OldPath + "\x00" + d.NewPath + "\x00" + d.Diff))
	return fmt.Sprintf("%x", sum)
}

but d.Diff for the last file in a multi-file patch carries a trailing blank line that the same file does not carry when another file's section follows it. When the file set of the range changes (a new file enters the diff), a file whose own hunks are byte-identical can move from tail position to non-tail position (or vice versa), its d.Diff gains/loses that trailing line, and its fingerprint flips.

Consequence: on a resumed run, that file misses the fingerprint index and is re-reviewed at full LLM cost even though nothing in it changed. In iterating workflows (review after each push on a growing range) this happens on every run. Any future fingerprint-based dedup or caching inherits the same instability.

I diffed the extracted <current_file_diff> payloads for the same unchanged file across two runs (from the session JSONL): the only difference was one trailing empty line at the end of the block.

Steps to Reproduce

Any configured provider works. In an empty directory:

git init -q ocr-fp-repro && cd ocr-fp-repro
printf 'fn main() {\n    println!("hi");\n}\n' > main.rs
git add . && git commit -qm init && git tag base

# commit 1: modify main.rs — it will be the ONLY (thus last) file in the patch
printf 'fn helper() -> u32 { 41 }\n' >> main.rs
git add . && git commit -qm "change main"
ocr review --from base --to HEAD --audience agent        # run 1

# commit 2: add an unrelated file; main.rs is untouched but no longer last in the patch
printf 'pub fn double(x: u32) -> u32 { x * 2 }\n' > util.rs
git add . && git commit -qm "add util"
ocr review --from base --to HEAD --resume <RUN1_SESSION_ID> --audience agent   # run 2

# control — commit 3: add another file; main.rs stays in a non-tail position
printf 'pub fn triple(x: u32) -> u32 { x * 3 }\n' > zeta.rs
git add . && git commit -qm "add zeta"
ocr review --from base --to HEAD --resume <RUN2_SESSION_ID> --audience agent   # run 3

Session IDs are printed on failure hints and available via ocr session list; per-item fingerprints are visible in ~/.opencodereview/sessions/<repo>/<session>.jsonl (review_item_done / review_item_reused events) and in the --format json run manifest.

Expected Behavior

The fingerprint of a file whose own diff content is byte-identical between two runs should be identical, regardless of which other files entered or left the range. Run 2 should reuse main.rs; run 3 should reuse both main.rs and util.rs.

Logs / Error Output

Observed (run 2): main.rs is re-reviewed — run manifest shows "reused": [] — and its fingerprint changed although its hunks are identical:

run1  review_item_done  main.rs  fp=71ae2305bc49fd54f5dfc23c3dd260648faaa6c211aa1aa333f1a82f99c81b18
run2  review_item_done  main.rs  fp=587e13a2…   (same hunks; only a trailing blank line differs)

Unified diff of the two extracted per-file diff payloads for main.rs (run1 vs run2) — the entire difference:

@@ -6,4 +6,3 @@
      println!("{}", data);
  }
 +fn helper() -> u32 { ... }
-

Control (run 3, resumed from run 2): main.rs kept a stable non-tail position and WAS reused with zero LLM calls, confirming the mechanism; util.rs moved from tail to non-tail and was re-reviewed despite being unchanged:

run3  review_item_reused  main.rs  fp=587e13a2…  sourceSession=<run2>
run3  review_item_done    util.rs  fp=8e99a001…  (was 36a119eb… in run2; content unchanged)

Additional Context

Suggested fix: normalize trailing whitespace/newlines of d.Diff before hashing in reviewItemFingerprint (or make the patch splitter emit position-independent per-file text). Note the compatibility edge: changing the computation invalidates fingerprints stored in existing sessions, so it may be worth bumping a fingerprint/schema version in the run manifest so a resume across the boundary degrades to a clean re-review instead of silent mismatch.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions