feat(review): capture fired signals + reversal mapping for the slop and quality gate scores - #8239
Conversation
…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.
…oreSignals (JSONbored#8223) The pure gate path applies applyMergeReadinessGate(policy) before evaluating, so a repo with mergeReadinessGateMode: block and slopGateMode unset genuinely blocks on slop via the composite promotion -- but the writer read the RAW policy and skipped the write for exactly that case, silently dropping corpus evidence. Read every field from the promoted policy, mirroring recordConfiguredGateBlockerSignals, with a composite-promotion regression test.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8239 +/- ##
==========================================
- Coverage 92.11% 90.39% -1.72%
==========================================
Files 779 99 -680
Lines 78430 26104 -52326
Branches 23696 5127 -18569
==========================================
- Hits 72245 23598 -48647
+ Misses 5062 2233 -2829
+ Partials 1123 273 -850
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-23 14:12:28 UTC
Review summary Nits — 4 non-blocking
Decision 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.
|
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 besiderecordConfiguredGateBlockerSignals's existing call — with the same filter each pure evaluation applies, including the same policy transform: the function appliesapplyMergeReadinessGate(policy)first (exactly asevaluateGateCheckCoredoes beforebuildSlopGateBlockerever sees the policy), so a repo gating slop via the feat(github-app): merge-readiness aggregate gate #551 composite promotion withslopGateModeunset is captured too — this was the defect that closed the previous attempt at this issue (feat(review): capture fired signals + reversal mapping for the slop and quality gate scores #8235), resolved here with a dedicated composite-promotion regression test. Slop evaluates only in (effective)blockmode with a non-null risk, including the default-60 threshold; quality evaluates whenever its mode is notoffwith both a score and threshold present, pass AND fail alike — a threshold backtest needs both outcomes.targetKey = repo#pr.normalizeScore, divided by 100 to be confidence-equivalent forbuildConfidenceThresholdClassifierreplays — documented beside the writer) 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 posture for computed-score rules. Best-effort writes that can never affect the verdict.CONFIGURED_GATE_BLOCKER_SIGNAL_CODESvia the exportedGATE_SCORE_SIGNAL_CODESconstant, with the justification the issue requires recorded beside it: slop carries direct gate authority inblockmode (composite-promoted or direct); 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— 21 tests green, including the 7 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; the merge-readiness composite promotion case (the prior attempt's defect, now regression-pinned); every never-evaluated arm; the normalization bounds; the no-diff-content invariant; and the rejecting-SignalStore fail-open path covering both write arms) plus the full rootnpm run typecheck. Per-diff-line lcov intersection verified 100% line+branch onadvisory.ts's changed region.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
applyMergeReadinessGate(policy)'s output — the same promoted policyevaluateGateCheckCorehands its pure evaluations — and the new#551 paritytest locks the composite-promoted case in.