Skip to content

v3

  • v3
  • 7a87480
  • Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
  • Choose a tag to compare

  • v3
  • 7a87480
  • Choose a tag to compare

  • Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
@narthur narthur tagged this 31 Aug 23:59
* fix: restrict PR comment comparisons to the base/head URL intersection

The pass/fail gate (#60) already restricts per-code comparisons to URLs
seen in both the base and head runs, but updateComment.ts still built
its summary and table from raw, unrestricted totals. Whenever the
scanned URL set drifted between runs, the comment could disagree with
the gate's verdict.

Extract the shared rule into getComparableCounts.ts (getUrlIntersection
+ getComparableCounts) and use it from both main.ts's gate and
updateComment.ts's summary/table, so the two can no longer diverge.
Also surface the added/removed URL counts in the comment so a reader
can see why a count moved.

Fixes #61

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Va54uerKUhz7jk5AhaL5zV

* fix(review): cycle 1 — dedupe URL-intersection computation, fix empty-baseline drift message, add missing coverage

- Thread the pass/fail gate's precomputed UrlIntersection into
  updateComment() instead of recomputing it there from `urls`, and
  compute each code's ComparableCounts once in updateComment() and
  reuse it for both the Summary total and the per-code table, instead
  of deriving it twice (structural review + intent-reconciliation
  findings).
- Fix a real bug: addUrlDrift (new in this branch) fired on a
  genuinely empty baseline (base run found zero issues, not "no base
  artifact"), misreporting every current URL as newly added. Guarded
  it the same way addSummary already guards that case (test-coverage
  finding, scored 85/100 -- verified reachable).
- Soften addSummary's comment: it sums per-code counts across all
  codes, so an aggregate "unchanged" total does not by itself
  guarantee every individual code passed the gate (only the per-code
  table does) -- the prior wording overclaimed this (comments-review
  finding, 78/100).
- Restore a dropped rationale sentence in getComparableCounts.ts's
  JSDoc that explained the zero-base-count carve-out's backward-compat
  intent (comments-review finding, 48/100 -- applied anyway, zero
  risk).
- Fix a stale "-- see below" log reference in main.ts left over from
  the extraction (comments-review finding, 65/100).
- Delete src/lib/compareIssues.ts and its spec: orphaned by this
  branch's refactor (its output was already unused by its only
  caller even before this PR) and now imported nowhere in src/
  (git-history + structural-review findings, 76/100 and 82/100).
- Add src/lib/getComparableCounts.spec.ts (no dedicated spec existed
  for this new shared module) and two updateComment.spec.ts cases:
  the zero-base-count carve-out rendered through the comment, and the
  empty-baseline drift-message fix above (test-coverage findings).

Auto-applied low-risk (Step 8a): all of the above are comment/doc
wording, a same-file duplicate-computation dedupe, an additive test,
or deletion of code with zero verified callers -- no behavior change
to the pass/fail gate itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Va54uerKUhz7jk5AhaL5zV

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assets 2
Loading