Skip to content

feat(review): post a quiet "review skipped (reason)" status when auto-review eligibility fails#3694

Merged
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
bohdansolovie:feat/review-auto-review-skip-status-2067
Jul 6, 2026
Merged

feat(review): post a quiet "review skipped (reason)" status when auto-review eligibility fails#3694
loopover-orb[bot] merged 2 commits into
JSONbored:mainfrom
bohdansolovie:feat/review-auto-review-skip-status-2067

Conversation

@bohdansolovie

Copy link
Copy Markdown
Contributor

Summary

  • Add AUTO_REVIEW_SKIP_SUMMARY / resolveAutoReviewSkipSummary mapping each auto-review skip reason to a public-safe one-line sentence (mirrors settings-preview SKIP_SUMMARY).
  • Extend auditPullRequestAutoReviewSkip to record the summary in audit metadata and product-usage telemetry.
  • When auto-review eligibility skips AI review, finalize the Orb review check as skipped with the human-readable summary instead of publishing a gate failure verdict.

Fixes #2067

Test plan

  • npx tsc -p tsconfig.json --noEmit
  • focus-manifest tests for every known skip-reason summary + unknown fallback
  • auto-review-wiring audit metadata includes summary
  • queue.test.ts integration: no AI call, gate check conclusion skipped, audit summary present

Made with Cursor

Map each auto-review skip reason to a public-safe summary, record it in
audit/product-usage metadata, and finalize the Orb review check as skipped
with that summary instead of a gate failure when AI review is bypassed.

Co-authored-by: Cursor <cursoragent@cursor.com>
@bohdansolovie
bohdansolovie requested a review from JSONbored as a code owner July 6, 2026 04:51
…s test

Co-authored-by: Cursor <cursoragent@cursor.com>
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 6, 2026
@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-06 04:58:44 UTC

5 files · 1 AI reviewer · no blockers · readiness 80/100 · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
This PR adds a public-safe skip-reason summary map and threads it through the audit/telemetry event and, more importantly, changes the Orb gate check-run to publish a 'skipped' conclusion with a human-readable summary instead of a gate-failure verdict when auto-review eligibility fails. The core logic change in maybePublishPrPublicSurface (routing to a new createOrUpdateSkippedGateCheckRun path when autoReviewSkipReason is set) matches the PR's stated intent, and the accompanying tests (queue.test.ts, focus-manifest.test.ts, auto-review-wiring.test.ts) exercise the real path — draft PR, no AI call, check-run conclusion 'skipped', audit metadata summary populated — rather than a fabricated scenario. The main gap I can't verify from the diff alone is the signature/definition of the new createOrUpdateSkippedGateCheckRun helper, since it isn't shown being added or already defined in the truncated file content.

Nits — 5 non-blocking
  • src/queue/processors.ts:6321-6327 — the new `recordGithubProductUsage` call in `auditPullRequestAutoReviewSkip` isn't wrapped in `.catch()` like the adjacent `recordAuditEvent` call; if telemetry write fails it could throw unhandled where the rest of the function fails soft.
  • src/queue/processors.ts (maybePublishPrPublicSurface) — `createOrUpdateSkippedGateCheckRun` isn't visible in this diff; worth confirming its parameter order (env, installationId, repoFullName, advisory, summary, mode, {checkRunId}) matches its actual declaration.
  • The external secret scanner flag on test/unit/queue.test.ts:3274 (`token: "fake-installation-token"`) is a test fixture, not a real credential — false positive, no action needed.
  • src/signals/focus-manifest.ts — `resolveAutoReviewSkipSummary`'s fallback returns the raw `skipReason` string verbatim for anything not in the map; since callers pass internal skip-reason tokens, a future unmapped reason added elsewhere without updating `AUTO_REVIEW_SKIP_SUMMARY` would leak the raw internal string into the public check-run summary.
  • Add a `.catch(() => undefined)` to the `recordGithubProductUsage` call in auditPullRequestAutoReviewSkip for consistency with the audit-event error handling right above it.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2067
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 331 registered-repo PR(s), 195 merged, 8 issue(s).
Contributor context ✅ Confirmed Gittensor contributor bohdansolovie; Gittensor profile; 331 PR(s), 8 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: bohdansolovie
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, C++, JavaScript
  • Official Gittensor activity: 331 PR(s), 8 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Await review-lane availability.
  • Refresh registry data or choose a registered active repo.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.35%. Comparing base (7a6ebab) to head (a8129a9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3694   +/-   ##
=======================================
  Coverage   93.35%   93.35%           
=======================================
  Files         315      315           
  Lines       32191    32197    +6     
  Branches    11809    11811    +2     
=======================================
+ Hits        30053    30059    +6     
  Misses       1507     1507           
  Partials      631      631           
Files with missing lines Coverage Δ
src/queue/processors.ts 94.07% <100.00%> (+<0.01%) ⬆️
src/signals/focus-manifest.ts 99.09% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb 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.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 24e9b4e into JSONbored:main Jul 6, 2026
8 checks passed
JSONbored added a commit that referenced this pull request Jul 6, 2026
#3694 (merged independently, unaware of this fix) made the gate check-run
publish a quiet "skipped (reason)" status instead of the evaluated gate
conclusion whenever auto-review is skipped -- including for an ignored
author, since evaluateAutoReviewSkipReason checks the same
review.auto_review.ignore_authors config this PR's own eligibility check
does. That silently reintroduced the exact bypass this PR exists to close:
an ignored author's PR would show "skipped" on its gate check-run instead of
a real hard-rule failure (e.g. no linked issue), even though the gate
evaluation itself was computed correctly.

Only take the "skipped status" shortcut when publicSurfaceSkipped is false.
publicSurfaceSkipped is already the flag this PR's ignored-author path (and
decidePublicSurface's own skip conditions) sets specifically so the
deterministic gate keeps reporting truthfully even when public review output
is suppressed -- reusing it here keeps the two mechanisms consistent instead
of introducing new reason-string matching. A benign content-based skip
(draft, WIP title, too-large, docs-only, base-branch, auto-pause) never sets
publicSurfaceSkipped, so #3694's quiet status is unaffected for those cases.
JSONbored added a commit that referenced this pull request Jul 6, 2026
* fix(review): keep gate evaluation for ignored authors

* fix(review): keep the real gate verdict for ignored authors past #3694

#3694 (merged independently, unaware of this fix) made the gate check-run
publish a quiet "skipped (reason)" status instead of the evaluated gate
conclusion whenever auto-review is skipped -- including for an ignored
author, since evaluateAutoReviewSkipReason checks the same
review.auto_review.ignore_authors config this PR's own eligibility check
does. That silently reintroduced the exact bypass this PR exists to close:
an ignored author's PR would show "skipped" on its gate check-run instead of
a real hard-rule failure (e.g. no linked issue), even though the gate
evaluation itself was computed correctly.

Only take the "skipped status" shortcut when publicSurfaceSkipped is false.
publicSurfaceSkipped is already the flag this PR's ignored-author path (and
decidePublicSurface's own skip conditions) sets specifically so the
deterministic gate keeps reporting truthfully even when public review output
is suppressed -- reusing it here keeps the two mechanisms consistent instead
of introducing new reason-string matching. A benign content-based skip
(draft, WIP title, too-large, docs-only, base-branch, auto-pause) never sets
publicSurfaceSkipped, so #3694's quiet status is unaffected for those cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): post a quiet "review skipped (reason)" status when auto-review eligibility fails

1 participant