feat(review): capture fired signals + reversal mapping for the slop and quality gate scores - #8235
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…nd quality gate scores (JSONbored#8223) slopGateMinScore and qualityGateMinScore gate real verdicts but recorded no signal.rule_fired events, so no corpus could ever form for them and the knob registry could never govern their thresholds. Mirror JSONbored#8101/JSONbored#8104's capture wiring exactly: recordGateScoreSignals(env, policy, repo, pr) fires one rule per knob (slop_gate_score, quality_gate_score) at the env-bearing gate path, with the SAME filter each pure evaluation applies -- slop evaluates only in block mode with a non-null risk (buildSlopGateBlocker's own arm, including the default 60 threshold); quality evaluates whenever its mode is not off with both a score and a threshold present (buildQualityGateWarning's arm), pass AND fail alike, since a threshold backtest needs both outcomes. Metadata carries the score normalized to [0,1] (both are 0-100 integers per normalizeScore, divided by 100 to be confidence-equivalent for buildConfidenceThresholdClassifier replays -- documented beside the writer) plus the detection's own detail string as rawSignal, never diff content, per JSONbored#8130's raw-context posture for computed-score rules. Best-effort writes that can never affect the verdict. Reversal labeling: both ids join CONFIGURED_GATE_BLOCKER_SIGNAL_CODES via the new GATE_SCORE_SIGNAL_CODES constant, with the justification the issue requires recorded beside it -- slop carries direct gate authority in block mode; quality is advisory-only but registry-governable, and a reversal labels the overall bot outcome its score contributed to, exactly the corpus label the drift/loosening evaluators consume. Tests: both firing arms per knob (crossing and non-crossing outcomes, the default slop threshold), every never-evaluated arm (off/advisory-slop modes, null score/threshold), normalization bounds, the no-diff-content invariant, and the rejecting-SignalStore fail-open path.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8235 +/- ##
==========================================
- Coverage 92.10% 90.39% -1.71%
==========================================
Files 776 99 -677
Lines 78328 26103 -52225
Branches 23668 5127 -18541
==========================================
- Hits 72147 23597 -48550
+ Misses 5062 2233 -2829
+ Partials 1119 273 -846
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Caution 🛑 LoopOver review result - reject/close recommendedReview updated: 2026-07-23 13:56:04 UTC
Review summary Blockers
Nits — 5 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
LoopOver is closing this pull request on the maintainer's behalf (AI reviewers agree on a likely critical defect: src/rules/advisory.ts: `recordGateScoreSignals` reads `policy.slopGateMode`/`policy.slopGateMinScore` directly without first calling `applyMergeReadinessGate(policy)` (as `recordConfiguredGateBlockerSignals` does two functions above it), so when a repo sets `mergeReadinessGateMode: block` with `slopGateMode` left unset/advisory, the gate genuinely evaluates and can block on slop via the composite override (since `buildSlopGateBlocker` only ever reads `policy.slopGateMode` directly and has no knowledge of the composite itself), but this function's `slopMode === "block"` check sees the un-overridden mode and skips the write — breaking the PR's own stated invariant that this uses 'the same filter each pure evaluation applies' and silently dropping corpus evidence for exactly the case #8223 is meant to capture.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
slopGateMinScoreandqualityGateMinScoregate real verdicts today but recorded nosignal.rule_firedevents, so no corpus could ever form for them and the knob registry could never govern their thresholds.recordGateScoreSignals(env, policy, repoFullName, prNumber)fires one rule per knob (slop_gate_score,quality_gate_score) at the env-bearing gate path — wired immediately besiderecordConfiguredGateBlockerSignals's existing call — with the same filter each pure evaluation applies: slop evaluates only inblockmode with a non-null risk (mirroringbuildSlopGateBlocker's own arm, including the default-60 threshold); quality evaluates whenever its mode is notoffwith both a score and a threshold present (mirroringbuildQualityGateWarning's arm), pass AND fail alike — a threshold backtest needs both outcomes.targetKey = repo#pr.normalizeScore, divided by 100 to be confidence-equivalent forbuildConfidenceThresholdClassifierreplays — the normalization is documented beside the writer, per the issue) plus the detection's own detail string asrawSignal— never diff content, per calibration: capture bounded raw context for the remaining isConfiguredGateBlocker codes, excluding secret_leak #8130's raw-context audit posture for computed-score rules. Best-effort writes (.catch(() => undefined)) that can never affect the verdict.CONFIGURED_GATE_BLOCKER_SIGNAL_CODESvia a new exportedGATE_SCORE_SIGNAL_CODESconstant, with the justification the issue requires recorded beside it: slop carries direct gate authority inblockmode; quality is advisory-only but registry-governable, and a reversal labels the overall bot outcome its score contributed to — exactly the corpus label the drift/loosening evaluators consume. The fired side is unaffected (that list feeds only the reversal lookups).Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run typechecknpm run actionlintnpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.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=moderateIf any required check was skipped, explain why:
vitest run test/unit/configured-gate-blocker-signals.test.ts— 20 tests green, including the 6 new calibration: capture writers + corpus mappings for the slop and quality gate scores #8223 cases: both firing arms per knob with crossing and non-crossing outcomes plus the default slop threshold; every never-evaluated arm (off/advisory-slop modes, null score/threshold); the normalization bounds; the no-diff-content invariant; and the rejecting-SignalStore fail-open path) plus the full rootnpm run typecheck.actionlint/workers/mcp/ui checks are untouched surfaces; CI runs them all.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — backend capture wiring only (no UI, docs, or extension surface touched; no registry entries, no knob movement, per the issue's Boundaries).
Notes