fix(orb): score enforcement closes as their own class, not quality mispredictions - #8827
Merged
Conversation
…spredictions
A policy close (contributor cap, blacklist, copycat, review-nag,
screenshot-table, linked-issue hard rule) carries no gate blocker, so
blockerClass was 'none' and the recorded reason fell back to the gate's
own conclusion -- writing 'success' on a PR the bot deliberately closed.
Calibration then scored every one as a quality misprediction ('said
merge, ended closed') when the gate never made a quality claim at all.
On the live fleet all 210 rows in that class carried the bare summary
'success', indistinguishable from a real verdict.
Closes #8825.
An enforcement close is a deliberate decision, not an error, and can be
neither confirmed nor disconfirmed as a quality prediction -- so it is
excluded from precision/accuracy scoring entirely rather than counted
on either side, and reported separately as policyActions so the volume
stays visible.
- processors: name the closeKind in the recorded reason
(policy_close:<kind>) instead of falling back to the conclusion
- orb-collector: bucket that prefix as policy_action, checked before
the substring rules so linked-issue-hard-rule doesn't flatten into
issue_policy
- analytics: carry the bucket through the confusion matrix; exclude
policy_action cells from decisionAccuracy/precision while still
counting them in decided and reversalRate
Rows exported before the bucket existed carry null and keep scoring as
ordinary quality verdicts, so historical data is unchanged.
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 #8827 +/- ##
==========================================
- Coverage 93.83% 92.29% -1.55%
==========================================
Files 800 800
Lines 79844 79851 +7
Branches 24199 24203 +4
==========================================
- Hits 74921 73695 -1226
- Misses 3558 5092 +1534
+ Partials 1365 1064 -301
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This was referenced Jul 26, 2026
Merged
JSONbored
added a commit
that referenced
this pull request
Jul 26, 2026
…for close precision When the bot closes a PR we never observe whether merging would have been fine: close-precision is estimated only on closes humans happened to contest, a biased sample (selective labels, Lakkaraju et al. KDD 2017). ORB is deterministic, so every off-policy estimator is undefined -- propensities are 0/1, no overlap. Randomization is the only fix. Closes #8831. Part of epic #8828 (Phase 2 -- labels). A configurable fraction epsilon (gate.closeAuditHoldoutPct, 0-20%, manifest-only, default 0 = byte-identical) of would-AUTO-close PRs is HELD for human adjudication instead, with epsilon and the draw logged -- the propensity record that makes counterfactual evaluation well-defined for the first time. Invariants, each pinned by a test: - the draw consumes the FINAL post-breaker plan: it can divert the decided close to a human but never influence the decision - heuristic closes only: policy closes are enforcement (#8827), and staged closes already get a human - HARD ordering rule: the plan is only diverted when the propensity record wrote -- an unlogged hold would silently bias every estimator, so on a write failure the close proceeds (the instrument degrades, never the gate) - held PRs land in decision_audit_labels as stratum holdout_close with a NULL outcome (the PR is still open; the adjudication IS the label)
JSONbored
added a commit
that referenced
this pull request
Jul 26, 2026
…for close precision (#8841) * feat(orb): weekly stratified decision-audit sampling with a frozen adjudication rubric The gate's only confirmation signal is 'no human reversed it' -- a lower bound on error, not a label: humans reverse only what they notice, so a silently-wrong merge scores as correct and every accuracy figure inherits the bias. Closes #8830. Part of epic #8828 (Phase 2 -- labels). - decision_audit_labels (migration 0178): one label per PR ever, adjudications comparable only within a rubric version - pure stratified sampler: merges oversampled (the costly arm), a first-time-author stratum (weakest priors), deterministic under an injected rng, thin strata spill instead of shrinking the sample - weekly cron enqueue (Tuesday 08:00 UTC), flag-gated by LOOPOVER_DECISION_AUDIT (default OFF, byte-identical), self-host only; the dispatch re-checks the flag so a stale queued job after a flag flip does zero work - internal adjudication routes: list (filterable) + adjudicate; a second adjudication 409s -- labels are calibration data, rewrites must be deliberate via a rubric-version bump, never silent - docs/decision-audit-rubric.md v1: judge the decision at decision-time knowledge, not the outcome; 'uncertain' is a first-class label excluded from both sides and tracked as a rate * feat(orb): admit the holdout_close stratum in decision_audit_labels upfront The randomized close-holdout (#8831) lands its adjudications in the same label store; widening the CHECK now spares that PR a SQLite table rebuild for one enum value. * feat(orb): make decision_audit_labels.outcome nullable for holdout rows A holdout_close row (#8831) is created while the PR is still held -- no realized outcome exists yet; the human adjudication is its ground truth. The weekly sampler always supplies an outcome. * fix(orb): claim the adjudication atomically — the status predicate rides the UPDATE Superagent P1 on this PR, and it is right: select-then-update lets two concurrent adjudications both pass the pending check and both write, silently violating the one-adjudication-per-label guarantee. The predicate now rides the UPDATE (WHERE status = 'pending'); zero changes disambiguates 404 vs 409 with a read that no longer guards anything. Regression test races two concurrent adjudications: exactly one 200, one 409, one surviving label. Also binds LOOPOVER_NATIVE_SOURCE instead of a hardcoded source literal (branding-drift check). * fix(orb): count only DECIDED PRs in first-time-author detection Review blocker on this PR, and it is right: the author count read every cached pull_requests row, so a newcomer with several open-but-undecided PRs and exactly one decided PR was wrongly marked veteran -- starving the stratum whose whole purpose is weak-prior newcomers. The count now requires a realized pr_outcome per counted PR, with a regression test seeding open PRs that must not inflate the total. * feat(orb): randomized close-audit holdout — the selective-labels fix for close precision When the bot closes a PR we never observe whether merging would have been fine: close-precision is estimated only on closes humans happened to contest, a biased sample (selective labels, Lakkaraju et al. KDD 2017). ORB is deterministic, so every off-policy estimator is undefined -- propensities are 0/1, no overlap. Randomization is the only fix. Closes #8831. Part of epic #8828 (Phase 2 -- labels). A configurable fraction epsilon (gate.closeAuditHoldoutPct, 0-20%, manifest-only, default 0 = byte-identical) of would-AUTO-close PRs is HELD for human adjudication instead, with epsilon and the draw logged -- the propensity record that makes counterfactual evaluation well-defined for the first time. Invariants, each pinned by a test: - the draw consumes the FINAL post-breaker plan: it can divert the decided close to a human but never influence the decision - heuristic closes only: policy closes are enforcement (#8827), and staged closes already get a human - HARD ordering rule: the plan is only diverted when the propensity record wrote -- an unlogged hold would silently bias every estimator, so on a write failure the close proceeds (the instrument degrades, never the gate) - held PRs land in decision_audit_labels as stratum holdout_close with a NULL outcome (the PR is still open; the adjudication IS the label) * chore(orb): reflect closeAuditHoldoutPct in the settings OpenAPI schemas and preview The settings-parity gate requires every RepositorySettings field on RepositorySettingsSchema and the settings-preview shape; regenerated openapi.
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
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.
What
Scores enforcement closes as their own class instead of as quality mispredictions. Closes #8825 (the remaining half — the recording-order fix shipped in #8826).
Why
A policy close — contributor cap, blacklist, copycat, review-nag, screenshot-table, linked-issue hard rule — carries no gate blocker. So
disposition.blockerClasswas"none"and the recorded reason fell back to the gate's own conclusion, writingsuccesson a PR the bot had deliberately closed.Calibration then scored every one of those as "the gate said merge and it ended up closed" — a quality misprediction, when the gate never made a quality claim at all. Measured on the live fleet, all 210 rows in that class carried the bare summary
success, completely indistinguishable from a real verdict. That was the root reason the class couldn't simply be filtered out: the information wasn't in the ledger.An enforcement close is a deliberate decision, not an error. It can be neither confirmed nor disconfirmed as a prediction about code quality, so counting it on either side of accuracy is wrong.
How
processors.ts— name the close kind in the recorded reason (policy_close:<kind>) instead of falling back to the conclusion, so the action is identifiable in the ledger at all.orb-collector.ts— bucket that prefix aspolicy_action, checked before the substring rules sopolicy_close:linked-issue-hard-ruledoesn't flatten into plainissue_policy.analytics.ts— carrygate_reasoncode_bucketthrough the confusion matrix (Cell+ the matrixGROUP BY), and excludepolicy_actioncells fromdecisionAccuracy/mergePrecision/closePrecision. They still count towarddecidedandreversalRate, which measure activity and human overrides rather than gate correctness, and the volume is reported separately aspolicyActionsso it never silently vanishes.Backward compatible: rows exported before the bucket existed carry
nulland keep scoring as ordinary quality verdicts, so historical data is unchanged and no backfill is required.Effect on the published number
This removes a systematic downward bias. Combined with #8824 (reversals were under-counted → biased up) and #8826 (verdicts contradicting actions → biased down), all three known distortions in the accuracy metric are now addressed, and the figure becomes defensible in both directions for the first time.
Verification
policyActions = 40anddecided = 50. Plus null-bucket back-compat and the bucket-precedence case.