Skip to content

fix(OMN-16998): classifier names the observed provider status class - #2287

Merged
jonahgabriel merged 1 commit into
devfrom
lakshman/omn-16998-terminal-failure-cause-classifier
Sep 4, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
lakshman/omn-16998-terminal-failure-cause-classifier

Conversation

@Patel230

@Patel230 Patel230 commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

What

resolve_terminal_failure_cause now reports the cause the provider's own status class implies. A live HTTP 401 classifies as auth_failed; it previously reported provider_quota_exhausted.

Ticket: https://linear.app/omninode/issue/OMN-16998

Why it matters beyond the label

terminal_failure_cause is the field beta criterion B7 ("zero over-quota refusals") is measured from. While a 401 classified as a quota event, auth failures were being counted as quota exhaustion — so the metric read as capacity pressure when the real cause was a credential. Audit G4/H15 found the live payload carrying terminal_failure_cause: provider_quota_exhausted while quality_gates_failed[0] was the verbatim HTTP 401 Authentication Failed.

A quota cause now requires a corroborating 429: status and body must agree within one reported failure before the classifier will name quota.

  • 20 unit tests pin each status-to-cause mapping. RED→GREEN proven: 12 of the 20 fail against the previous implementation.
  • Full governed pre-push suite on .201: 17952 passed, 214 skipped, 3 deselected, 1 xfailed, 0 failed in 7m32s, [prepush-smart-tests] impacted tests passed; allowing push.
  • No PREPUSH_* variables, no ENABLE_SMART_TESTS=off, no --no-verify, nothing hand-installed.

The core half of this change is already merged as omnibase_core#1632 (EnumTerminalFailureCause gains AUTH_FAILED and PROVIDER_ERROR, with the vocabulary asserted closed). This is the omnimarket consumer half.

Definition of done

# Item State
1 Classifier maps provider status to a distinct cause Done
2 A unit test pins each mapping Done — 20 cases
3 A live 401 produces auth_failed end to end Not in this PR — blocked upstream on OMN-15660
4 No quota cause without a 429 Done — TestQuotaCauseRequires429

DoD 3 is deliberately absent. A live-401 probe on the collaborator lane consumed a prior run's terminal event and reported success without calling an LLM; the OMN-17295 correlation-join guard caught it and refused the receipt. That is OMN-15660 (RuntimeLocal shares terminal consumer groups and never filters by correlation_id), which is unmerged. Per the operator ruling the probe is not to be retried until it lands, so this PR carries the classifier and its tests only.

Not included

No lock or pin changes. An earlier relock commit was dropped during rebase because dev already carries that work via #2274 — verified both declaration sites (pyproject.toml:71 and the resolver override at :638) already pin omnibase-infra>=0.38.16 / omnibase-core>=0.47.2, and uv.lock resolves to them.

Evidence-Ticket: OMN-16998
Evidence-Source: OCC#8153

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

Rule Description
ARCH-TOPIC-001 No hardcoded Kafka topic strings in handler code
ARCH-DI-001 No event_bus=None bypass
ARCH-DI-002 No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003 No reinvented DI containers
ARCH-TOPIC-002 contract.yaml topics follow `onex.{cmd

No architectural violations found.

Static architectural lint — no model inference (OMN-14176).

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 0
Models succeeded: qwen3-review,qwen3-review-b


Gate semantics

Verdict Meaning Blocks merge?
passed >=2 models succeeded, no critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded Fewer than 2 models succeeded (infra unavailable/timeout) Yes (OMN-15110)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Sep 4, 2026
…imarket#2287 (#8153)

* evidence: OCC companion pass 1 for OmniNode-ai/omnimarket#2287

* evidence: OCC companion self-bind for #8153

---------

Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
The fallback was a single regex alternating between 429 and quota wording, so
quota wording ALONE was sufficient to claim a capacity refusal — and with only
one enum member, an HTTP 401 fell through to that same quota label.

Status and body are now separate patterns that must corroborate within a single
reported failure: a 429 on one rung and quota wording on another are not one
event. Resolution order is typed failure_class, then observed status (401/403 to
auth_failed, 429 with quota body to provider_quota_exhausted), then
provider_error for any other observed failure, and None only when nothing was
observed at all.

One invariant overrides the typed path: the DoD says NO classifier path may emit
a quota cause without a 429, so an uncorroborated typed quota claim degrades to
provider_error. That narrows a documented contract and is asserted explicitly.

Held from PR until core 0.47.2 publishes and an infra release repins to it — the
new enum members are not visible to omnimarket before then.
@Patel230
Patel230 force-pushed the lakshman/omn-16998-terminal-failure-cause-classifier branch from 90cb8f8 to 3bc0b6e Compare September 4, 2026 02:30
@onexbot

onexbot Bot commented Sep 4, 2026

Copy link
Copy Markdown

No OCC evidence companion was minted for this PR.

this PR is already bound to OCC#8153; its evidence companion exists and nothing needs authoring

To clear this: Nothing to do — the companion already exists.

Reported by node_occ_companion_effect (decline code already_bound, OMN-16665). This decision was made against the PR's LIVE state at compute time, not at publish time — a green publisher job only means the command reached the broker.

@jonahgabriel
jonahgabriel merged commit 60718c2 into dev Sep 4, 2026
118 checks passed
@jonahgabriel
jonahgabriel deleted the lakshman/omn-16998-terminal-failure-cause-classifier branch September 4, 2026 03:01
@Patel230

Copy link
Copy Markdown
Contributor Author

Close-out for OMN-16998. Jonah merged this as 60718c2f on dev at 2026-09-04T03:01:56Z.

There were no review points. The Architectural Review and the Hostile Reviewer both passed with 0 findings. The companion bot only noted that OCC#8153 was already bound.

What landed: resolve_terminal_failure_cause now maps 401/403 to auth_failed, a 429 with a quota body to provider_quota_exhausted, and anything else to provider_error. It names quota only when the status and the body agree within one failure. The change includes 20 tests in tests/unit/models/delegation/wire/test_resolve_terminal_failure_cause.py. It ships in v0.4.17: that tag contains 60718c2f, and v0.4.15 does not.

How it was verified: CI run 33829777959 passed on head 3bc0b6ec. Of 121 checks, 118 passed, 2 were skipped, 1 was neutral and 0 failed. The governed pre-push on .201 passed 18012 tests with 0 failures on 3bc0b6ec. The 17952 in the description is from the earlier head 90cb8f8c.

Two corrections to the description:

  • The core enum is EnumDelegationTerminalFailureCause, not EnumTerminalFailureCause.
  • The Evidence-Source: line points at OCC#8153, a companion whose receipts pin the earlier head 90cb8f8c, not the merged head 3bc0b6ec. Nothing rejected that at merge. This case is recorded on OMN-15501.

DoD 3, which the description left out, was proven on 2026-09-23. Jonah's lane did a clean install from the package index and set an invalid GLM key. GLM returned a real 401, and the run reported terminal_failure_cause == "auth_failed" (correlation 9534364f). The details are on the ticket.

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants