Skip to content

fix(orb): score enforcement closes as their own class, not quality mispredictions - #8827

Merged
JSONbored merged 1 commit into
mainfrom
fix/policy-close-scoring
Jul 26, 2026
Merged

fix(orb): score enforcement closes as their own class, not quality mispredictions#8827
JSONbored merged 1 commit into
mainfrom
fix/policy-close-scoring

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

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.blockerClass was "none" and the recorded reason fell back to the gate's own conclusion, writing success on 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

  1. 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.
  2. orb-collector.ts — bucket that prefix as policy_action, checked before the substring rules so policy_close:linked-issue-hard-rule doesn't flatten into plain issue_policy.
  3. analytics.ts — carry gate_reasoncode_bucket through the confusion matrix (Cell + the matrix GROUP BY), and exclude policy_action cells from decisionAccuracy/mergePrecision/closePrecision. They still count toward decided and reversalRate, which measure activity and human overrides rather than gate correctness, and the volume is reported separately as policyActions so it never silently vanishes.

Backward compatible: rows exported before the bucket existed carry null and 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

  • TSC clean; 399 tests green across analytics, collector, public-stats, queue, queue-2 and mcp-fleet-analytics.
  • New tests assert the exact failure mode: an instance with 40 enforcement closes alongside 8 confirmed and 2 missed quality closes scores 8/10, not 48/50, with policyActions = 40 and decided = 50. Plus null-bucket back-compat and the bucket-precedence case.
  • Changed-line coverage: 0 uncovered statements/branches.

…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.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.29%. Comparing base (6c1870c) to head (ad10337).
✅ All tests successful. No failed tests found.

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     
Flag Coverage Δ
backend 92.96% <100.00%> (-2.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/orb/analytics.ts 100.00% <100.00%> (ø)
src/queue/processors.ts 95.70% <100.00%> (+<0.01%) ⬆️
src/selfhost/orb-collector.ts 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@JSONbored
JSONbored merged commit ee5e724 into main Jul 26, 2026
7 checks passed
@JSONbored
JSONbored deleted the fix/policy-close-scoring branch July 26, 2026 10:33
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 26, 2026
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.
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.

gate_decision can contradict the action taken — stale 'merge' verdicts recorded after CI-failure closes distort calibration

1 participant