Skip to content

fix(backend,frontend): record WHY a review failed, not just that it failed - #9

Open
exos-marc wants to merge 1 commit into
mainfrom
fix/review-failure-reason-442
Open

fix(backend,frontend): record WHY a review failed, not just that it failed#9
exos-marc wants to merge 1 commit into
mainfrom
fix/review-failure-reason-442

Conversation

@exos-marc

Copy link
Copy Markdown
Contributor

Propagates the fix landed on the private origin as 87a11ff (issue #442 there).

The problem

Every pipeline failure was recorded as the constant "unhandled_exception", so the UI could only guess. A real production review failed on an HTTP 402 — the model account was out of credits — and the UI offered three wrong guesses (bad key / model unavailable / document too long). Diagnosing it required shell access to a production container, for a condition an admin can fix in two minutes if told.

The fix

  • Classify, don't guess. A mapper turns the caught exception into a reason token (model_account_out_of_credits, model_key_rejected, model_rate_limited, model_context_length_exceeded, model_unavailable), falling back to unhandled_exception.
  • Structured, not stringly-typed. ModelInvocationError carries the provider status as a .status_code attribute; classification reads that rather than regex-matching the message, which would rot the next time the copy changes.
  • The status number stops at the backend. Only the token crosses the API boundary; the frontend maps tokens to prose. No raw HTTP <n>, endpoint, or key material reaches user-facing copy.
  • Fail-closed preserved. The bare except Exception stays — classification happens inside it — so a review still can never wedge in PENDING/RUNNING. An unrecognised exception yields unhandled_exception and today's copy, so no path regresses.

Verification

  • tests/test_review_failure_reason_442.py (new, 16 tests) — asserts the mapping per status code and that no status number reaches the UI.
  • Red-first confirmed: against the pre-fix tree this file fails with 1 failure + 28 errors; every path yielded unhandled_exception.
  • bash scripts/check-frontend.sh → CHECK-FRONTEND: ALL GREEN
  • SKIP_INFRA=1 bash scripts/check.sh → CHECK: ALL GREEN
  • tests/lint-brand-free.py on this branch → BRAND-FREE LINT: PASS (incl. Check 4: zero exos-legal)

🤖 Generated with Claude Code

…ailed

Classify the pipeline's caught exception into a reason TOKEN instead of the
constant "unhandled_exception", and prefer reason-keyed copy over stage-keyed
copy in the UI.

- ModelInvocationError carries the provider status as a structured
  .status_code attribute; classification reads it rather than regex-matching
  the message string.
- The status NUMBER stops at the backend. Only the token crosses the API;
  the frontend maps the token to prose, so no raw HTTP status reaches
  user-facing copy.
- The bare except Exception stays (fail closed, never wedge PENDING/RUNNING);
  classification happens inside it. An unrecognised exception still yields
  unhandled_exception and today's copy.

Refs #442
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.

1 participant