Skip to content

Stop reporting "no matching diff" for code the model was never shown#81

Merged
mk7luke merged 1 commit into
mainfrom
fix/noisy-no-matching-diff-warnings
Jul 19, 2026
Merged

Stop reporting "no matching diff" for code the model was never shown#81
mk7luke merged 1 commit into
mainfrom
fix/noisy-no-matching-diff-warnings

Conversation

@mk7luke

@mk7luke mk7luke commented Jul 19, 2026

Copy link
Copy Markdown
Owner

The bug

Description-drift compares the PR description against the diff. It was being handed a partial diff while asked to reason about a description covering the whole PR, so any claim whose supporting code wasn't in the prompt got reported as unsupported.

On mk7luke/agent-tui#1, all six PR-level findings named files that were in the PR the entire time — .cargo/config.toml, build.rs, bin/protoc, hooks.rs, external_auth.rs, Cargo.toml.

The reason they read as confident Major findings rather than hedged ones is that the model's reasoning was correct. Given the diff it saw, the change really wasn't there. The confidence: high guidance already in the prompt couldn't help, because the diff plainly lacked the code. This was never fixable by prompt tuning.

Three ways the diff arrived incomplete

1. The incremental trim (src/reviewer.ts)

context.files was mutated down to files changed since the last review, and drift consumed that same object 400 lines later. Right for line-level review — re-flagging untouched files is pointless, and the inline findings on #1 were correct. Wrong for prose describing ten commits.

Extracted partitionFilesForReview(), which returns allFiles alongside filesToReview. Worsened with every push, since more files fall into filesSkippedSimilar.

2. Unpaginated fetch (src/github.ts)

pulls.listFiles used per_page: 100 with no pagination. PRs over 100 files silently lost the remainder — and the dropped files reached neither ignoredFiles nor cappedFiles, so nothing downstream could even report they were missed. Now paginated, bounded at GitHub's own 3000-file listFiles ceiling so a runaway PR costs a known number of calls.

3. The size budget (src/ai/prompt.ts)

Drift now computes its own budget. context.diffBudget is calculated after the trim, so restoring the full file set while reusing it would have sent an unbounded prompt — which throws, and the existing catch swallows drift silently. That would have traded loud false positives for quiet disappearance.

buildChatPrompt was ignoring diffBudget entirely, so without fixing that the new budget would have been a no-op. Other chat callers set no budget and are unaffected.

Beyond the reported case

Drift is now told which PR files it cannot see (config-ignored, past the cap, budget-dropped) and instructed not to report them. CHAT_SYSTEM no longer claims "full context of the PR" it may not have.

This matters independently: the same false positive could fire on a full review of a large PR. Fixing only the incremental trim would have left that alive.

Verification

  • tsc clean; 378 tests passing across 26 files; lint unchanged from baseline (12 warnings, 0 errors — all pre-existing).
  • 9 new tests, mutation-verified: reverting allFiles to the trimmed set, dropping the scope note, and restoring the single-page fetch each fail them.
  • One drift test survives mutation by design — it documents the passthrough contract; the real guard is the partition test.

Not addressed

getRelatedPRs (src/github.ts:~1320) also calls listFiles unpaginated at per_page: 50. Left alone — it's a best-effort overlap heuristic, not review input, so an incomplete list degrades a suggestion rather than producing a false finding. Worth a follow-up.

🤖 Generated with Claude Code


Summary

Fixed false-positive description-drift findings caused by incomplete diffs by paginating file fetches, preserving the full file set for whole-PR reasoning, and telling the model which files it cannot see.

Changes

File Changes
src/ai/prompt.ts Softened CHAT_SYSTEM's full-context claim and made buildChatPrompt honor diffBudget with truncation/omission labels.
src/drift.ts Added unavailableFiles option and a scope note instructing the model not to report drift for files whose diffs were not shown.
src/github.ts Paginated pulls.listFiles with a 3000-file ceiling so large PRs no longer silently drop files.
src/reviewer.ts Extracted partitionFilesForReview returning allFiles alongside the incremental slice, and ran drift against the full set with its own budget and unavailable-file list.
tests/unit/drift-scope.test.ts Added regression tests for full-file drift scope, unavailable-file notes, incremental partition invariants, and filename deduplication.
tests/unit/pr-files-pagination.test.ts Added tests for multi-page listFiles fetch, single-page efficiency, 3000-file ceiling, and file-cap application over the full paginated set.

Description-drift compared a whole-PR description against a partial diff,
so it reported claims as unsupported whenever their code was simply absent
from the prompt. On mk7luke/agent-tui#1 all six PR-level findings named
files that were in the PR the whole time.

The findings read as confident rather than hedged because the reasoning was
sound — only the evidence was truncated. No prompt tuning could fix that.

Three independent ways the diff arrived incomplete:

1. Incremental trim. reviewer.ts mutated context.files down to files changed
   since the last review, and drift consumed the same object 400 lines later.
   Correct for line-level review; wrong for prose describing every commit.
   Extracted partitionFilesForReview() so the full set survives the trim.

2. Unpaginated fetch. pulls.listFiles used per_page:100 with no pagination,
   so PRs over 100 files silently lost the rest. Those files reached neither
   ignoredFiles nor cappedFiles, so nothing downstream could report the loss.
   Now paginated, bounded at GitHub's own 3000-file listFiles ceiling.

3. Size budget. Drift now carries its own budget — context.diffBudget is
   computed after the trim, so reusing it would have sent an unbounded prompt
   that throws and gets swallowed by the existing catch. buildChatPrompt was
   ignoring diffBudget entirely, which would have made that budget a no-op.

Drift is also told which PR files it cannot see (ignored, capped, budget-
dropped) and instructed not to report them, and CHAT_SYSTEM no longer claims
full PR context it may not have. Both matter beyond the incremental case:
the same false positive could fire on a full review of a large PR.

Tests mutation-verified — reverting each fix fails them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@diffsentry

diffsentry Bot commented Jul 19, 2026

Copy link
Copy Markdown

DiffSentry has completed the review — Looks good!

@diffsentry

diffsentry Bot commented Jul 19, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Fixed false-positive description-drift findings caused by incomplete diffs by paginating file fetches, preserving the full file set for whole-PR reasoning, and telling the model which files it cannot see.

Changes

Cohort / File(s) Summary
Incremental File Partition
src/reviewer.ts
Extracted partitionFilesForReview so line-level review stays incremental while drift receives the full PR file set with a dedicated diff budget and unavailable-file list.
Drift Scope Guarding
src/drift.ts, src/ai/prompt.ts
Told the model which PR files it cannot see and stopped claiming full PR context; chat prompts now respect diffBudget with truncation notes.
GitHub File Pagination
src/github.ts
Paginated pulls.listFiles up to GitHub's 3000-file ceiling so files beyond the first page are no longer silently lost.
Regression Tests
tests/unit/drift-scope.test.ts, tests/unit/pr-files-pagination.test.ts
Added unit coverage for drift scope invariants, unavailable-file prompting, and listFiles pagination/cap behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Reviewer
    participant Partition as partitionFilesForReview
    participant Drift as detectDescriptionDrift
    participant AI
    Reviewer->>Partition: files, mode, priorFileShas
    Partition-->>Reviewer: allFiles, filesToReview, skipped
    Note over Reviewer: context.files trimmed for line-level review
    Reviewer->>Reviewer: applyDiffBudget on allFiles
    Reviewer->>Drift: full files, driftBudget, unavailableFiles
    Drift->>AI: chat prompt plus scope note
    AI-->>Drift: drift findings JSON
    Drift-->>Reviewer: DriftFinding list
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Suggested Labels

bug, enhancement

Risk Assessment

Score: 5/100 — 🟢 Low

Factor Weight Detail
Moderate change +5 409 lines changed across 6 files

Test Coverage Signal

🟢 140 prod / 243 test lines added.

Source files changed Test files changed Source lines + Test lines +
4 2 140 243

Linked Issues

  • #1 — Add CodeRabbit-parity features 🔴
🚥 Pre-merge checks | ✅ 5 | ❌ 0
✅ Passed checks (5 passed)
Check name Status Explanation
PR Title ✅ Passed Title meets all requirements: starts with imperative verb "Stop", is 68 chars (under 72), has no trailing period, and no Conventional Commits prefix.
PR Description ✅ Passed The description clearly explains WHAT changed (partition full vs incremental file sets for drift, paginate listFiles, honor diffBudget in chat prompts, and tell drift about unavailable files) and WHY (partial diffs caused confident false-positive "no matching diff" findings). It also links the related case mk7luke/agent-tui#1.
Schema bump ✅ Passed src/storage/db.ts was not modified in this PR
Provider parity ✅ Passed src/ai/anthropic.ts was not modified in this PR; no request/response contract changes to propagate to openai.ts or openai-compatible.ts.
Pattern test coverage ✅ Passed Neither src/safety-scanner.ts nor src/pattern-checks.ts were modified in this PR; no new rules to cover.

✏️ Tip: You can configure your own custom pre-merge checks in your .diffsentry.yaml.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Generate docstrings (beta)
  • Push docstring commit to this branch
🧹 Simplify (beta)
  • Push simplification commit to this branch
🪄 Autofix unresolved comments (beta)
  • Push autofix commit to this branch

Comment @diffsentry help to get the list of available commands and usage tips.

@diffsentry diffsentry 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.

Actionable comments posted: 0

This PR addresses the root cause of the noisy description-drift false positives in the right places: prompt scope, PR file retrieval, and reviewer orchestration. The changes are coherent and well covered by regression tests, and I did not find any correctness or contract regressions in the modified paths. The new tests also directly guard the previously broken behaviors, including incremental trimming and unpaginated listFiles fetches.


ℹ️ Review info
⚙️ Run configuration

Configuration used: .diffsentry.yaml

Review profile: ASSERTIVE

Run ID: 9b03e59c-aae8-4965-bbae-8757407db764

📥 Commits

Reviewing files at bd3aebe (base SHA unavailable).

📒 Files selected for processing (6)
  • src/ai/prompt.ts
  • src/drift.ts
  • src/github.ts
  • src/reviewer.ts
  • tests/unit/drift-scope.test.ts
  • tests/unit/pr-files-pagination.test.ts

ℹ️ The primary review model was unavailable; this review was generated by the configured backup provider.

@diffsentry

diffsentry Bot commented Jul 19, 2026

Copy link
Copy Markdown

📌 Status — last updated bd3aebe

🟢 Approved

Risk score 5/100 (Low) ▁
Unresolved threads 0
Failing checks 0
Pending checks 1
Files reviewed 6
Updated 2026-07-19 03:28Z

Live-updated by DiffSentry on every push. Use @diffsentry ship for a verdict, @diffsentry timeline for full history.

@mk7luke

mk7luke commented Jul 19, 2026

Copy link
Copy Markdown
Owner Author

@diffsentry release-notes

@diffsentry

diffsentry Bot commented Jul 19, 2026

Copy link
Copy Markdown

📣 Release Notes

✨ What's new

  • Description-vs-diff checks now look at the full pull request, not just the latest push — so claims about earlier commits are no longer flagged as “unsupported”
  • When some files can’t be shown (ignored, over the size limit, or past the file cap), the checker is told to skip them instead of treating them as missing

🛠 Improvements

  • Pull requests with more than 100 files are fully loaded instead of silently dropping the rest
  • Chat replies about a PR are clearer when the diff view is partial, and won’t assume missing code means the change doesn’t exist

🐛 Fixes

  • Fixed confident false alarms where description drift reported “no matching diff” for files that were in the PR the whole time

Marketing-speak version of this PR. Re-run with @diffsentry release-notes.

@mk7luke
mk7luke merged commit becf728 into main Jul 19, 2026
4 checks passed
@mk7luke
mk7luke deleted the fix/noisy-no-matching-diff-warnings branch July 19, 2026 03:32
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.

1 participant