Skip to content

orb(ledger): a hold records reason_code 'success', so the manual-review surface cannot be split by cause #9991

Description

@JSONbored

Prerequisite for #9729's clearance run, and a defect in its own right.

The ledger says a PR was held for reason "success"

Measured on the Orb: 1,679 of 2,429 verdicts are holds (69%), and the single largest bucket is reason_code = 'success'518 holds over 146 pull requests, 31% of all manual load.

That value is not informative, it is an artifact. deriveDecisionReasonCode is:

blockerClass !== "none" ? blockerClass : policyCloseKind != null ? `policy_close:${policyCloseKind}` : conclusion

For a hold with no blocker and no policy close, it falls through to the gate conclusion — which for these is success. So the record states that a pull request was held for the reason "success", which is not a reason at all. A reader of the public proof page's sample records sees the same thing.

Why it blocks #9729

#9729 must loosen only what clears a backtest, per path. success is not a path — it is at least seven of them. MERGE_HOLD_INPUTS (src/settings/pr-disposition.ts) enumerates every input that suppresses a would-merge:

guardrailHit, migrationCollisionHold, unlinkedIssueMatchHold, advisoryCheckHold, priorityEligibilityHold, screenshotEvidenceHold, unlinkedIssueMatchCloseWithoutCloseActing

derivePrDisposition already computes which of them fired — heldForManualReview is a .some(...) over exactly that set — and then discards the answer. A clearance run against a bucket conflating seven mechanisms would not mean anything.

The table's own comment anticipates this: "the value documents WHY the input holds; nothing reads it as text today, and it is the natural place for a ledger reason to come from when one is wanted." It is wanted now.

Do NOT fix this by changing reason_code

replayDecision recomputes reasonCode through the same function and reports a divergence when it does not match the recorded value (decision-replay.ts:168). Changing the derivation would make all 518 existing hold records replay as unreproducible — a false "this decision cannot be re-derived" on records that are fine. It would also change a value published in the proof page's sample records.

The reason belongs in a new nullable column, written alongside, outside record_json so record digests and the replay contract are untouched. Historical rows stay null, which is honest: we genuinely do not know why those 518 were held.

Acceptance

  • derivePrDisposition exposes which hold inputs fired, deterministically ordered.
  • A hold's cause is persisted per decision record, without altering reason_code, record_json, or any record digest.
  • Replay of an existing record still verifies.
  • The manual-review surface becomes splittable by cause, so orb(gate): next guardrail-loosening tranche — backtest-cleared paths only #9729 can pick candidates per path.

Metadata

Metadata

Assignees

Labels

maintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions