Skip to content

Judge incremental reviews against the complete change - #216

Merged
morgaesis merged 4 commits into
mainfrom
fix/incremental-complete-change
Sep 30, 2026
Merged

morgaesis merged 4 commits into
mainfrom
fix/incremental-complete-change

Conversation

@morgaesis

Copy link
Copy Markdown
Contributor

An incremental re-review saw only the newly pushed commits, so a follow-up that cleaned up a reference to something an earlier commit removed (an alert selector, a rate-limit entry, a caller) read as lost coverage and was reported as a regression; incremental generator and scorer requests now also carry the complete pull-request change as uncitable context, bounded to 24 KiB with a per-file summary fallback, fetched by forge reviews and supplied locally with --pull-request-diff-file. An unresolved adjudication of a finding anchored on change metadata, such as a deleted file, no longer fails the whole review. A new incremental screen and cross-file clean and contrast cases cover dependent cleanups; with openai/gpt-5.6-luna on Azure EU the incremental clean cases were silent in 6 of 6 runs with both contrasts detected, against false positives in 2 of 3 runs without the complete change. The admission corpus and evaluator sources are unchanged, and the CLI gate, exact-head canonical review and exhaustive pre-push review passed.

Add two clean cases in which part of a change removes a target and
another part narrows a dependency on it: test edge hostnames removed
together with their Traefik routes and alert selectors, and a disabled
export feature removed with its route, caller, flag, rate-limit entry
and alert selectors. Each narrowed hunk looks like lost coverage in
isolation. Add two must-block contrasts whose narrowing also drops a
target that the change keeps.

The cases extend only the supplemental clean and causality banks. The
admission corpus and the attested evaluator sources are unchanged.
A finding anchored on a change-metadata or pull-request description line
took its source role from the adjudicator's copied evidence. Unresolved
results must carry empty evidence, so any fresh unresolved finding on a
deleted file failed scope validation for every candidate and ended the
review as invalid adjudication output.

The reviewed citation now fixes the metadata role, and its evidence
becomes the causal change. Confirmed results must still copy the anchored
metadata evidence.
An incremental re-review sent only the commits pushed since the last
review. A later commit that narrowed an alert selector, rate limit, or
caller for a target an earlier commit had removed looked like lost
coverage, and the review reported it as a regression.

Incremental generator and scorer requests now also carry the complete
pull-request change as uncitable context: the raw diff up to 24 KiB,
else a per-file summary with status and line counts. The context counts
toward request admission and is dropped only when it does not fit the
model budget. Forge reviews fetch the complete change themselves; a
failed fetch keeps the incremental review. Local reviews supply it with
--pull-request-diff-file, which requires --diff-file and --since-sha.
Findings still cite only the increment.

A supplemental incremental screen reproduces the isolated false positive
and pairs each clean case with a contrast that narrows a kept target.
@morgaesis
morgaesis merged commit b0c7371 into main Sep 30, 2026
8 checks passed
@morgaesis
morgaesis deleted the fix/incremental-complete-change branch September 30, 2026 23:01
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