Automated code review: CONFIRMED verdicts sit on unverified premises (13 PRs, 69 CONFIRMED, several refutable in one command)
Summary
Ran the new automated code review across 13 PRs in a single day on a public repo
(SecondMouseAU/OCCTSwift, a Swift wrapper over the
OpenCASCADE CAD kernel). Output volume is high and quality is inconsistent in a specific,
reproducible way: the verdict labels do not track how well a claim survives checking, and the
highest-confidence verdicts sat on the least-verified premises.
Every claim below is a link to a public PR comment where I adjudicated the finding with a measurement.
Volume, measured
| | |
|---|---|
| PRs reviewed | 13 |
| Review bodies posted | 15 |
| Total review body text | 64,035 characters |
| CONFIRMED verdicts | 69 |
| PLAUSIBLE verdicts | 16 |
| Ratio | 4.3 : 1 |
A 4.3:1 confident-to-hedged ratio is the core problem. It is not that the reviews find nothing real
(several findings were excellent, listed at the bottom). It is that CONFIRMED is applied so freely
that it carries no information, so every finding has to be independently verified anyway, which is
the cost the tool exists to remove.
---
Category 1: CONFIRMED verdicts that a single command refutes
These are the most damaging, because acting on them changes correct code into incorrect code.
1.1 API semantics inferred from a name, not read
PR #730, finding 1.
Verdict: CONFIRMED. Claim: a new if (extrema.IsParallel()) return 0; guard "discards the case
of finite parallel segments with non-overlapping ranges", called "a silent correctness regression".
Measured against the actual library:
parallel, ranges DISJOINT IsParallel=0 NbExtrema=1 LowerDistance=11.180340
Points(1) OK: (10,0,0)-(20,5,0)IsParallel() returns false for that geometry, so the guard never fires and the correct
nearest-endpoint answer is returned. The control case in the same program (overlapping ranges)
SIGSEGVs, which is the bug the PR fixes. The two cases are distinguished by IsParallel() itself,
correctly.
The reasoning error: IsParallel() was read as "the tangent directions are parallel", which that
geometry satisfies. It does not mean that. The header was available in the repo and would have
settled it. A code review tool inferring a predicate's contract from its identifier, then issuing
a CONFIRMED correctness-regression verdict on the inference, is the worst case for this feature.
1.2 A cross-reference asserted without reading the referent
PR #729, finding 2.
Verdict: CONFIRMED. Claim: "#512 is the established canonical tracker for 'carried patch not yet
in the rebuilt/pinned xcframework'", and my checklist should link it rather than re-derive.
Issue #512's actual title: *"Rebuild OCCT.xcframework with patch 0017 (#484 null-context guard)
and ship a patch release". Its body opens "Follow-up to #484 / PR #511"*. It is scoped to one
specific patch, and that patch already shipped in the currently pinned artifact. The checklist in
question is about two different patches it does not cover. Following the advice would send a
release engineer to an issue about work completed two rebuilds ago.
1.3 A "Critical, will break the build" verdict, refuted by one merge-base call
PR #734. Presented as
Critical, the single finding in the review. Three factual claims, all wrong:
- "This PR's diff was computed against base commit
4f05207, which predates PR #720."
git merge-base returns 49b5e3b, which is #720's own tip and contains the code in question.
- *"This PR's diff never touches
buildGraph()at all, and never deletesOCCTFaceGetAreaCentroid,
despite the PR description's explicit claims."* It does both. grep -c AreaCentroid returns 0 in
both named files, and the call site is visibly the 4-argument form.
- *"A merge of this PR onto the current branch tip will [...] leave the 10-arg call site calling a
4-arg function — a straight compile failure."* I merged it: zero conflicts, and
Build complete! (25.30s).
The underlying worry is legitimate in general (a deletion authored against an older base can fail to
apply). It does not occur here for a reason one command establishes. The review also asserted the PR
description was lying about its own contents, which it was not.
1.4 A mechanism asserted, measured not to occur
PR #733, findings 2 and 5.
Both CONFIRMED. Claim: a new 1e-4 Z-tolerance filter would drop walls when a boolean operation
runs with a fuzzy tolerance at or above that value, silently losing pockets and flipping anisOpen flag.
Swept the boolean fuzzy value from 0 to 1e-2, a hundred times the tolerance:
| fuzzy | worst wall drift | pockets | isOpen |
|---|---|---|---|
| 0 | 1.0e-07 | 1 | false |
| 1e-5 | 1.0e-07 | 1 | false |
| 1e-4 | 1.0e-07 | 1 | false |
| 1e-3 | 1.0e-07 | 1 | false |
| 1e-2 | 1.0e-07 | 1 | false |
The drift never moves off 1.0e-07, three orders of magnitude inside the tolerance. OCCT's fuzzy
value widens the tolerance used during intersection; it does not translate the resulting faces. The
finding assumed a relationship between two numbers because they are both tolerances.
*(Credit where due: probing finding 5 did surface a real, unrelated, pre-existing bug, now filed as
#735. Pointing at the right area for the wrong reason still has value. But it was labelled CONFIRMED
on a mechanism that does not occur.)*
---
Category 2: CONFIRMED applied to things that are not defects
PR #728, finding 3.
Verdict: CONFIRMED. The finding is that the PR's own CHANGELOG "discloses that the fix's core
scenario [...] was never exercised end-to-end before this PR was opened".
The PR disclosed a limitation. That is not a defect; it is the property that makes a PR reviewable.
The author had been explicitly instructed to separate what was verified from what was not, and did.
Issuing a CONFIRMED defect verdict against honest disclosure trains exactly the wrong behaviour: the
same PR would have scored better by saying nothing.
It was also stale within the hour. A real workflow_dispatch run had been triggered against
that branch before the review was written and was executing as it was published. It later completed
green end to end.
---
Category 3: Quantitative claims not sanity-checked
PR #728, finding 4.
Verdict: CONFIRMED. Claim: splitting one CI job into two "raises the worst-case critical path to
~150 minutes" and "doubles per-run runner setup overhead".
- The 150 minutes is the sum of the two jobs' timeout ceilings (120 + 30), not their runtimes.
The measured path is ~79 + ~10 = ~89 minutes, against the single job's previous 90.
- "Doubles runner setup overhead" is a second checkout and toolchain selection: roughly **one
minute** against a 79-minute build.
- Most importantly, the comparison omits that the baseline never completes. The job being
replaced timed out on 100% of runs by construction. A cost regression argued against a job that
always fails needs to say so.
---
Category 4: Reviews that contradict each other, or the tree
4.1 A review retracted by its own second pass
PR #717 received two reviews two and a half hours apart (05:44:54Z and 08:17:02Z). The second opens:
⚠️ This corrects the review posted above. That review's content did not match the findings verified by this run's actual review pipeline — only 2 of its 7 points overlap with what's below; the rest [...] were not part of the verified output and should not be treated as this run's findings.
Five of seven findings in the first review were disavowed by the tool itself. By then an agent had
already implemented two of the disavowed ones. That work was harmless, but it was work done against
findings the pipeline says it never made. Both reviews remain posted with no ordering cue other
than timestamps, so a reader arriving at the PR sees 14 findings, 13 marked CONFIRMED, of which 5
are formally retracted.
4.2 A headline finding already fixed before the review was written
PR #711, round 2. The most severe finding was that a static checker was blind to a particular C++
declaration syntax. The commit teaching it that syntax is timestamped 04:25:16 UTC. The review
asserting the blindness was submitted at 05:47:31 UTC, 82 minutes later. The reviewed state had
not existed for over an hour.
---
Category 5: Severity inversion
Repeatedly, the most actionable finding in a review carried the lowest confidence label.
rated PLAUSIBLE: "treating 'land #728' as closing the CI gap could leave a release manager
believing the gap is closed when it may not be." This was correct, was the only finding of the four
worth acting on, and I amended the document because of it. It sat below three CONFIRMED findings,
one of which (1.2 above) was factually wrong.
rated cleanup: collapsing a "curves are parallel" result into the same empty array as "no
extremal relationship" discards a signal the codebase's own lower-level API exposes. That is a real
API design defect, rated below the false CONFIRMED regression in the same review.
---
Category 6: Output scaled to nothing in particular
PR #729 changed one documentation file. It received 4 findings, two of which were duplication
complaints about prose. PR #734's review was 2,103 characters containing exactly one finding, marked
Critical, which was wrong in all three of its claims.
Meanwhile PR #716, a large change across solver bounds-checking, received 10 findings that were
all accurate, so the tool is clearly capable. The volume does not appear to track risk, size,
or its own confidence.
---
What this costs
The failure mode is not "noisy reviews are annoying". It is that a CONFIRMED verdict that is wrong
about a correctness issue is an instruction to break working code.
Concretely, today I ran five separate ground-truth probes (compiling C++ against the pinned
library, sweeping tolerances, doing trial merges and builds) purely to adjudicate findings that
turned out to be wrong. That is more expensive than reviewing the PRs myself would have been. Two of
those probes needed 10+ minute builds.
Had I not verified them:
- #730/1 would have removed a correct guard and reintroduced an uncatchable SIGSEGV.
- #734's Critical would have blocked a correct PR, or triggered a pointless rebase.
- #729/2 would have pointed release engineering at a closed, unrelated issue.
- #733/2 would have loosened a tolerance with three orders of magnitude of margin, on the theory
that a different tolerance in a different subsystem might interact with it.
---
Suggested changes, in priority order
- Make
CONFIRMEDmean something operational. Reserve it for findings where the pipeline
executed something that would have failed had the claim been false: ran the test, compiled the
merge, read the header, opened the referenced issue. Everything else is PLAUSIBLE. If that
collapses most findings to PLAUSIBLE, that is the honest picture and is still useful.
- Read the referent before asserting a cross-reference. If a finding cites an issue, a PR, a
commit, or a symbol, fetch it. Three of today's wrong findings (1.1, 1.2, 1.3) are all this same
error against three different kinds of referent.
- Never infer an API contract from an identifier. The header, the source, or the docs were
available in-repo in every case here.
- Do not issue defect verdicts against disclosure. A PR saying "I could not verify X" is the
behaviour to reinforce.
- Re-read the head SHA at emit time, and drop findings whose subject has changed. An 82-minute-
stale review is worse than no review because it looks current.
- When a pipeline supersedes its own output, retract rather than append. Post the corrected
review and mark the superseded one as withdrawn, or edit it in place. Leaving both live means
consumers act on retracted findings, which happened here.
- Sanity-check numbers against a measured baseline, and state the baseline. Summing timeout
ceilings and calling it a critical path is the kind of error a single sentence of grounding
prevents.
- Scale output to the diff. A one-file documentation change should not produce four findings
including two prose-duplication complaints.
---
To be fair: what worked
This is not a request to turn the feature off. Several findings today were genuinely excellent and I
acted on all of them:
caught a cache-poisoning bug I had missed and that would have been permanent and silent: an
if: always() on a CI cache-save step meant a cancelled or failed build would cache a partial
build artifact under an immutable key, so every later run would restore a broken kernel and fail
in a way indistinguishable from a real regression, with no path to green by re-running. The finding
was correct and, if anything, understated. Best single finding of the day, in any tool.
found that a parameterised test fixture never built the geometry its name claimed: the drill was
anchored at a fixed offset while the plate was centred, so a "through-hole" test cut a blind pocket
at every thickness. This also caught out my own verification of the same fixture. Excellent.
- PR #716's 10 findings were all accurate on a substantial change.
- PR #730's review correctly picked up a measured false positive from the PR comment thread and
omitted it from its findings rather than restating it. That is exactly right, and shows the
pipeline can incorporate prior adjudication when it looks.
The capability is clearly there. The problem is calibration and verification discipline, not
capability.
---
Environment
- Claude Code, review run via the new workflow on 2026-08-06.
- Repository: public, Swift + Objective-C++ + Python, ~5,400 tests, wraps a large C++ CAD kernel.
- 13 PRs, all targeting a long-lived release branch rather than the default branch.