Skip to content

fix(review): enforce blocker-first PR verdicts - #1781

Merged
JSONbored merged 2 commits into
mainfrom
codex/superagent-review-findings
Jun 29, 2026
Merged

fix(review): enforce blocker-first PR verdicts#1781
JSONbored merged 2 commits into
mainfrom
codex/superagent-review-findings

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Keep blockers authoritative: blockers, red CI, base conflicts, and unresolved review threads stay close/reject paths instead of being downgraded to manual review.
  • Treat guardrail and size holds as manual review only for otherwise-ready PRs.
  • Block on visible failed CI checks/statuses and unresolved GitHub review threads, including scanner threads with P0-P3 findings.
  • Elect same-linked-issue duplicate winners by first observed linked-issue claim time instead of PR number.
  • Make the maintainer Reviews & PRs Grafana dashboard honor the selected time range.

What changed

  • Added review-thread blocker ingestion through the PR review path and webhook triggers for review comments/threads.
  • Persisted linked_issue_claimed_at for PRs and threaded claim-aware duplicate winner logic through gate, comments, and planner paths.
  • Removed green-CI/manual-review downgrades for configured AI/gate blockers.
  • Updated the default large-PR manual-review line threshold to 1000 LOC.
  • Added focused regression coverage for blocker-first verdicts, review threads, claim-time duplicate election, and dashboard-safe paths.

Why

Blocked PRs were sometimes rendered as manual review when a guardrail/size hold was also present. Same-linked-issue duplicate handling also used PR creation order, which let older PRs that edited in a claim later suppress the real first claimant.

Validation

  • npm run validate — 5,166 tests passed, 4 skipped; coverage completed successfully.
  • npm run db:migrations:check — migrations contiguous through 0084.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • git diff --check
  • Parsed grafana/dashboards/maintainer-reviews.json with Node.

@dosubot dosubot Bot added the size:XL label Jun 29, 2026
@loopover-orb

loopover-orb Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-06-30 01:31:02 UTC

28 files · 1 AI reviewer · 1 blocker · readiness 86/100 · CI failing · unknown

🛑 Suggested Action - Manual Review

  • AI reviewers agree on a likely critical defect: src/github/backfill.ts:2250 only fetches reviewThreads(first: 50), so a PR with more than 50 unresolved review threads can hide a blocking unresolved thread beyond the first page and pass the new review-thread gate. — Resolve the flagged defect, or override if the AI reviewers are mistaken, then re-run the gate.

Review summary
This change makes the gate treat configured blockers, failed checks/statuses, unresolved review threads, and claim-time duplicate election as authoritative instead of downgrading them to manual review. The blocker-first direction is coherent, and the migration/repository plumbing generally threads the new linked-issue claim timestamp through the hot paths. The main correctness risk is that the new review-thread and claim-time logic are lossy in reachable cases, so the gate can still make the wrong decision for large review-thread sets or PRs whose linked-issue set changes over time.

Blockers

  • src/github/backfill.ts:2250 only fetches reviewThreads(first: 50), so a PR with more than 50 unresolved review threads can hide a blocking unresolved thread beyond the first page and pass the new review-thread gate.
  • src/db/repositories.ts:313 resets linkedIssueClaimedAt whenever the serialized linked issue set changes, so a PR that first claimed issue A and later adds issue B loses its original claim time for issue A and can lose duplicate-winner election incorrectly.
Nits — 5 non-blocking
  • nit: src/github/backfill.ts:2267 fetches only comments(first: 20), so scanner metadata or the first non-self author can be missed on long review threads even when the thread itself is fetched.
  • nit: src/review/review-thread-findings.ts:47 carries blocker.url through the parsed blocker but reviewThreadBlockerFinding never includes it in the finding detail/action, which makes the generated blocker less actionable than the source data allows.
  • nit: src/db/repositories.ts:313 compares linkedIssuesJson byte-for-byte, so a harmless ordering change in extraction would reset the claim timestamp unless linked issue extraction is guaranteed to be stable.
  • Paginate GraphQL reviewThreads until hasNextPage is false in src/github/backfill.ts, and add a regression with the blocking thread on page 2.
  • Preserve the earliest observed claim for any still-present linked issue in src/db/repositories.ts, or persist claim time per linked issue if multi-issue PRs need deterministic duplicate election.

Why this is blocked

  • src/github/backfill.ts:2250 only fetches reviewThreads(first: 50), so a PR with more than 50 unresolved review threads can hide a blocking unresolved thread beyond the first page and pass the new review-thread gate.

CI checks failing

  • codecov/patch — 70.79% of diff hit (target 97.00%)
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (size label size:XL; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 2 registered-repo PR(s), 2 merged, 263 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 2 PR(s), 263 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 2 PR(s), 263 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jun 29, 2026
@JSONbored
JSONbored merged commit 311b761 into main Jun 29, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jun 29, 2026
@JSONbored
JSONbored deleted the codex/superagent-review-findings branch June 29, 2026 23:03
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 70.79646% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.64%. Comparing base (5733910) to head (d151f53).
⚠️ Report is 10 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/review/review-thread-findings.ts 42.85% 12 Missing and 8 partials ⚠️
src/queue/processors.ts 55.55% 4 Missing and 4 partials ⚠️
src/github/backfill.ts 78.94% 0 Missing and 4 partials ⚠️
src/db/repositories.ts 90.00% 0 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (70.79%) is below the target coverage (97.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1781      +/-   ##
==========================================
- Coverage   95.74%   95.64%   -0.10%     
==========================================
  Files         211      214       +3     
  Lines       23030    23218     +188     
  Branches     8322     8390      +68     
==========================================
+ Hits        22050    22207     +157     
- Misses        401      417      +16     
- Partials      579      594      +15     
Files with missing lines Coverage Δ
src/db/schema.ts 68.32% <ø> (ø)
src/review/content-lane-wire.ts 97.72% <ø> (ø)
src/review/grounding-wire.ts 94.80% <ø> (ø)
src/review/guardrail-config.ts 100.00% <ø> (ø)
src/review/unified-comment.ts 99.43% <ø> (-0.01%) ⬇️
src/rules/advisory.ts 97.64% <100.00%> (-0.10%) ⬇️
src/rules/predicted-gate.ts 100.00% <ø> (ø)
src/settings/agent-actions.ts 92.52% <100.00%> (-0.27%) ⬇️
src/signals/duplicate-winner.ts 100.00% <100.00%> (ø)
src/signals/engine.ts 97.31% <100.00%> (+<0.01%) ⬆️
... and 4 more
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JSONbored added a commit that referenced this pull request Jul 10, 2026
…urable disposition (#4603)

isConfiguredGateBlocker voided aiReviewCloseConfidence entirely (commit 311b761, #1781), so a
sub-floor ai_consensus_defect/ai_review_split finding always one-shot-closed a contributor PR under
aiReviewGateMode: block regardless of the AI reviewers' own calibrated confidence, while five other
comments across the codebase still described the old (correct) "downgrades to a hold" behavior.

Adds aiReviewLowConfidenceDisposition ("one_shot" | "hold_for_review" | "advisory_only", default
hold_for_review) to govern a sub-floor finding: hold_for_review (shipped default) still blocks the
gate but routes the would-be close through the same held-for-manual-review mechanism the disposition
planner already uses for migrationCollisionHold/unlinkedIssueMatchHold; one_shot keeps today's
unconditional-close behavior as an opt-in; advisory_only drops a sub-floor finding to non-blocking.

Wires the new field through the full template this repo uses for a RepositorySettings field:
migration + Drizzle schema + resolver/parser (src/db/repositories.ts), OpenAPI schema + regenerated
apps/gittensory-ui/public/openapi.json, .gittensory.yml config-as-code override
(gate.aiReview.lowConfidenceDisposition), and the maintainer /ai-review + internal settings routes.
Mirrors the gate-decision change into the packages/gittensory-engine predictor twin so the MCP
predict_gate tool stays accurate. Fixes all 6 stale-comment locations the original audit found.
JSONbored added a commit that referenced this pull request Jul 10, 2026
…urable disposition (#4656)

* fix(review): resolve dead aiReviewCloseConfidence floor with a configurable disposition (#4603)

isConfiguredGateBlocker voided aiReviewCloseConfidence entirely (commit 311b761, #1781), so a
sub-floor ai_consensus_defect/ai_review_split finding always one-shot-closed a contributor PR under
aiReviewGateMode: block regardless of the AI reviewers' own calibrated confidence, while five other
comments across the codebase still described the old (correct) "downgrades to a hold" behavior.

Adds aiReviewLowConfidenceDisposition ("one_shot" | "hold_for_review" | "advisory_only", default
hold_for_review) to govern a sub-floor finding: hold_for_review (shipped default) still blocks the
gate but routes the would-be close through the same held-for-manual-review mechanism the disposition
planner already uses for migrationCollisionHold/unlinkedIssueMatchHold; one_shot keeps today's
unconditional-close behavior as an opt-in; advisory_only drops a sub-floor finding to non-blocking.

Wires the new field through the full template this repo uses for a RepositorySettings field:
migration + Drizzle schema + resolver/parser (src/db/repositories.ts), OpenAPI schema + regenerated
apps/gittensory-ui/public/openapi.json, .gittensory.yml config-as-code override
(gate.aiReview.lowConfidenceDisposition), and the maintainer /ai-review + internal settings routes.
Mirrors the gate-decision change into the packages/gittensory-engine predictor twin so the MCP
predict_gate tool stays accurate. Fixes all 6 stale-comment locations the original audit found.

* fix(review): restore accidentally-deleted accuracy-trend feature + close codecov/patch gap on #4603

Two independent problems found in PR #4656 (#4603) before merge, both fixed here:

1. The PR silently deleted src/services/public-accuracy-trend.ts (181 lines) and its test,
   un-exported the public-stats.ts helpers it depended on, and dropped the accuracyTrend field
   from the public GET /v1/public/stats response -- entirely unrelated to #4603's actual scope
   (a live, wired-in public homepage feature from epic #4445/#4447, undisclosed in the commit
   message or PR body). Restored all four pieces verbatim from main.

2. codecov/patch was failing (83.33% vs 99% target): the new isConfiguredGateBlocker branches
   for aiReviewLowConfidenceDisposition in packages/gittensory-engine/src/advisory/gate-advisory.ts
   had a dedicated node:test suite, but that suite runs against the compiled dist/ output in a
   separate test runner whose coverage never reaches the coverage/lcov.info file Codecov actually
   reads -- only the root vitest suite's coverage does, and it only touches this file through the
   handful of test/unit/*.ts files that import it directly by relative path (not via the npm
   package). Added the missing branch cases to the existing
   test/unit/predicted-gate-engine-branch-coverage.test.ts, which does import it that way.

   Also found src/api/routes.ts's `updated.aiReviewLowConfidenceDisposition ?? "hold_for_review"`
   fallback is unreachable in practice -- parseAiReviewLowConfidenceDisposition's return type is
   non-nullable and already falls back to the same literal itself -- so marked it
   /* v8 ignore next */ rather than writing a contrived test for a branch that can't actually run.

Part of #4603
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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant