fix(review): stop mis-closing registry edits and AI-guessed CI diagnoses - #7670
Merged
Conversation
…submission
The registry surface lane's duplicate check compared an appended entry's
identity key against the raw base surfaces[] array, so editing an
already-registered entry in place (same url, changed fields -- e.g.
tightening probe.expect after live-verification) collided with its own
prior self and closed as a resubmitted duplicate, regardless of whether
the edit was correct. Every legitimate "fix an existing surface" PR hit
this. survivingExistingEntries now only counts a base entry as still
"existing" when it survives byte-identical into head, so an edit is
validated as content instead of auto-flagged as a duplicate.
Also fixes a compounding issue in the unified comment renderer: every
gate blocker finding from this lane shares the same constant title
("Registry surface review"), and only the title was ever rendered under
"Why this is blocked" -- the actual, verdict-specific reason in
finding.detail/publicText was silently dropped from the panel for every
close.
…d CI check failed
Confirmed against a real closed PR that the AI reviewer, given only a
failing check's name (no error text, no file content), was writing a
confidently-hedged content-level diagnosis ("this likely means a field
mismatch... which isn't visible in this diff") and getting that guess
quoted verbatim into the auto-close message -- directly against its own
existing "do not assert a defect on code you cannot see" instruction.
GROUNDING_GUIDANCE now explicitly names and forbids this exact hedge
pattern, and a failing check with no detail is marked "(no detail
provided)" in-line so the model can't miss it.
Also adds a new, genuinely verifiable fact for the same case: how many
commits the repository's current default branch has landed since this
PR's own base commit, via GitHub's compare API. Unlike the
mergeable_state "behind" signal the review pipeline already uses
elsewhere, this works regardless of a repo's branch-protection
configuration, so the reviewer can correctly attribute an unexplained CI
failure to a stale base instead of guessing a content defect.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7670 +/- ##
==========================================
- Coverage 88.51% 88.51% -0.01%
==========================================
Files 724 724
Lines 76020 76045 +25
Branches 22629 22635 +6
==========================================
+ Hits 67291 67308 +17
- Misses 7681 7688 +7
- Partials 1048 1049 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
30 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
…eable_state blind spot it depends on (#7686) The base-branch staleness fact shipped in #7670 anchored its compare-API read on a PR's own base.sha -- but GitHub keeps that field pointed at the live tip of the target branch as it moves, so comparing it against the current default branch would read ~0 regardless of how stale a PR's actual code is, making the fact effectively dead. Anchors on the PR's HEAD instead, which the compare API resolves via a true git merge-base, independent of that metadata timing. Also adds gate.staleBaseAheadByThreshold: when a repo opts in, the pre-review readiness gate (prReadyForReview) forces an update_branch once the default branch has advanced at least that many commits beyond a PR's head -- the same action the existing BEHIND-base path takes, but triggered by the same compare-API read rather than GitHub's own mergeable_state, which only ever reports "behind" when a repo's branch protection requires branches to be up to date before merging. A repo without that setting can have a PR genuinely dozens of commits behind and never see it auto-rebased before review otherwise.
21 tasks
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two review-accuracy bugs in the registry/CI-review pipeline, both confirmed against real closed PRs (read-only investigation, no action taken on those PRs themselves):
surfaces[]array, so any in-place edit of an already-registered surface (same url, changed fields) collided with its own prior self and closed as a "duplicate resubmission" -- regardless of whether the edit was correct. Every legitimate content-fix PR to a registry file was hitting this. Fixed by only counting a base entry as still "existing" when it survives byte-identical into head. Also fixed a compounding bug where every such finding's real reason (detail/publicText) was silently dropped from the "Why this is blocked" panel in favor of a constant, uninformative title.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally (unsharded) -- 19618 passed, 0 failed; every line/branch touched by this diff is 100% covered (verified directly against lcov.info, not just the console table).npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderate-- 0 vulnerabilitiesnpm run test:ci(all ~35 steps) run clean end to end, in isolation.Safety
UI Evidencesection -- N/A, no UI changes.Notes
Backend-only, no UI/API/MCP surface touched. Both bugs were diagnosed by tracing this repo's own source against the bot's real comments on live PRs (read-only
ghcalls), not by guessing from the bug report alone.