fix(noema): fail closed on a transport error instead of crashing the required check

[seonghobae]

Summary

Live incident on ContextualWisdomLab/naruon#1486: the required noema-review check crashed with an unhandled urllib.error.HTTPError: HTTP Error 502: Bad Gateway:

File ".../scripts/ci/noema_review_gate.py", line 967, in call_llm
    with opener.open(request, timeout=NOEMA_LLM_TIMEOUT_SECONDS) as response:
...
urllib.error.HTTPError: HTTP Error 502: Bad Gateway

Root cause: call_llm's opener.open(request) call for the actual LLM completion sat outside the surrounding try/except — only the JSON-decode/validation step after a successful response was guarded. Any transport-level exception (here, a genuine 502 from the gateway) crashed the whole job with an unhandled traceback instead of getting the same one-time repair-retry the malformed-verdict path already has.

I checked this isn't already fixed by the recently-merged #1546 reconciliation either — its call_llm has the identical unguarded with opener.open(request) as response: line, no except around it. So this is a real, independent gap that survives both the old and new (unbounded) design, unrelated to the wall-clock-deadline direction #1438 was closed over.

Fix

Widened the try to also cover the request itself (opener.open() + response.read()), and added urllib.error.URLError (the parent of HTTPError) alongside RuntimeError to the existing repair-retry except clause. A transient transport failure now gets the same one-time retry a malformed verdict already gets, then fails closed with a clean RuntimeError on a second failure — never an unhandled traceback.

Test plan

• New regression tests in tests/test_noema_review_gate.py:
  • test_call_llm_repairs_once_after_a_transport_error_then_succeeds — a first-attempt 502 gets one retry, then a normal successful verdict.
  • test_call_llm_fails_closed_after_a_repeated_transport_error — two consecutive 502s produce one clean RuntimeError, never an unhandled traceback.
• Verified genuine RED first: both tests failed with the exact live HTTPError: HTTP Error 502: Bad Gateway reproduced uncaught against the pre-fix code. GREEN after the fix.
• Full suite: PYTHONPATH=. coverage run -m pytest tests -q → 2248 passed, 1 skipped, 21 subtests.
• python -m interrogate scripts/ci/noema_review_gate.py → 100%.
• Repo-wide coverage is 99% (11 statements / 7 branches, in pr_review_fix_scheduler.py / pr_review_merge_scheduler.py) both before and after this change — independently confirmed by running the full suite against bare main with this diff stashed out. Pre-existing, unrelated to these two changed files.

Scope note

This is deliberately narrow: it only closes an unguarded exception path. It does not touch the wall-clock-deadline design ContextualWisdomLab/.github#1438 was closed over, and does not conflict with #1546's unbounded-inference direction (checked directly — the same gap exists there too, so this fix applies equally regardless of which timeout policy wins).

Related

• ContextualWisdomLab/naruon#1486 — the PR whose CI run surfaced this incident.

---

Generated by Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 45 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 25f0ab32-426e-4cdd-a2f5-8d8783aad8f7

📥 Commits

Reviewing files that changed from the base of the PR and between 5686de4 and a74fd03.

📒 Files selected for processing (4)

• CHANGELOG.md
• docs/product-technical-gap-baseline.md
• scripts/ci/noema_review_gate.py
• tests/test_noema_review_gate.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

exact-head-path-policy failed at head dd82b866 with 3 parametrized failures in tests/test_opencode_required_verdict_regression.py::test_scheduler_wake_reuses_trusted_receipt_predicate (reviews2-1, reviews3-1, reviews4-1) — all assert result.returncode == 0 got 141 (SIGPIPE) from a subprocess.run(["bash", "-c", request_review_script()], ...) invocation.

This is unrelated to this PR's diff, which only touches scripts/ci/noema_review_gate.py, its test file, and CHANGELOG.md — it never touches test_opencode_required_verdict_regression.py or anything upstream of it. A full-suite run I did minutes earlier directly against bare origin/main (same base, before this PR's two-file diff) passed cleanly: 2246 passed, 1 skipped, 21 subtests passed, zero failures. That, plus the SIGPIPE exit code (a pipe-race signature, not a deterministic assertion failure), points to a flake rather than something this PR introduced.

Triggered one re-run to confirm (run_id: 33481716693, queued). Will report back if it fails again identically — if that happens I'll treat it as a real, if unrelated, bug worth root-causing rather than dismissing again.

---

Generated by Claude Code

[seonghobae]

@opencode-agent fix the unresolved exact-head Devin finding on the existing single-writer branch, then re-review the new head. Keep the transport boundary narrow: normalize open/read transport failures such as urllib.error.URLError, http.client.HTTPException (including IncompleteRead/RemoteDisconnected), and socket/OSError transport failures into the existing one-repair-then-clean-RuntimeError path without swallowing JSON/validator/programming errors. Add RED→GREEN regressions for truncated Response.read() succeeding after one retry and repeated truncated/read failure failing closed, plus at least one timeout/disconnect family if it exercises a distinct exception path. Update docs/product-technical-gap-baseline.md with the 2026-09-01 naruon#1486 transport-crash root cause/owner/status and keep the existing CHANGELOG entry accurate. Preserve #1546's unbounded inference semantics and exact-head revalidation; no fixed inference timeout, direct-provider fallback, or bypass.

[seonghobae]

Pushed the requested fix on 0f40dac2:

• Transport boundary widened to (RuntimeError, urllib.error.URLError, http.client.HTTPException, OSError), covering IncompleteRead/RemoteDisconnected and raw socket/OSError transport failures alongside the existing URLError case, without touching the JSON/validator/programming-error RuntimeError paths (those still re-raise as-is on a second failure via isinstance(exc, RuntimeError)).
• Added RED→GREEN regressions for a truncated Response.read() succeeding after one retry and failing closed on repeated truncation (http.client.IncompleteRead), plus a distinct raw TimeoutError reaching opener.open() directly (never wrapped as URLError) for the requested timeout/disconnect family — each verified genuinely RED against the pre-fix boundary before being folded in.
• docs/product-technical-gap-baseline.md updated with the 2026-09-01 root cause/owner/status entry for this incident (both fix rounds), and the CHANGELOG entry extended to match.
• #1546's unbounded inference semantics, exact-head revalidation, and the absence of any fixed inference timeout/direct-provider fallback/bypass are all preserved — this change only widens which transport exceptions get the existing one-repair-then-fail-closed treatment.
• Full suite: 2252 passed, 1 skipped, 21 subtests. noema_review_gate.py at 100% line/branch coverage, 100% docstrings. Repo-wide 99% is the same pre-existing, unrelated gap already confirmed and documented on the prior commit.

Resolved the Devin/owner review thread accordingly. Requesting re-review on the new head.

---

Generated by Claude Code

[seonghobae]

@opencode-agent review new exact head 0f40dac2550daa74b03f4339ac68cf0c1a586800; predecessor dd82b866… evidence is stale. Verify the widened boundary catches only bounded transport/read families (URLError, HTTPException, OSError) while validator/JSON/programming errors retain their existing semantics, and confirm the new truncated-read and raw-timeout RED→GREEN pairs plus baseline entry. Treat #1567 as the independent merged-tree coverage prerequisite.

[seonghobae]

@cwl-noema-review independently review new exact head 0f40dac2550daa74b03f4339ac68cf0c1a586800. Confirm one transport failure gets exactly one repair retry, repeated transport failure becomes a bounded clean failure, and #1546's unbounded substantive inference/exact-head publication checks remain unchanged.

[devin-ai-integration]

Devin Review found 3 new potential issues.

[seonghobae]

Pushed the fix for Devin's fourth finding ("Empty transport errors retry forever") on a74fd031:

• Gating retry-vs-fail-closed on repair_error's truthiness conflated "is this the second attempt" with "does the caught exception have display text" — several transport exceptions (OSError(), TimeoutError(), a message-less http.client.HTTPException) stringify to '', so an empty-message failure on the first attempt could retry unboundedly.
• Added an explicit is_retry: bool = False parameter that tracks retry state independently of the exception's text, threaded through as is_retry=True on the recursive call.
• New RED→GREEN regression pair: test_call_llm_repairs_once_after_an_empty_message_transport_error_then_succeeds / test_call_llm_fails_closed_after_a_repeated_empty_message_transport_error (the latter bounds the fixture at 6 calls so a regression fails fast with a clear AssertionError instead of recursing to CPython's limit).
• Full suite: 2254 passed, 1 skipped, 21 subtests. noema_review_gate.py still at 100% line/branch coverage, 100% docstrings. Repo-wide 99% remains the same pre-existing gap tracked by #1567.
• docs/product-technical-gap-baseline.md and CHANGELOG.md updated with this fourth fix round.

Resolved the corresponding review thread. The two remaining Devin comments on this PR are informational (kind: analysis, not bug) and need no action. Requesting re-review on the new head.

---

Generated by Claude Code

[seonghobae]

@opencode-agent review

Re-review exact current head a74fd03102fd5850a591747f3b193029cf71f45d after the transport-boundary repairs. Validate one-repair/fail-closed behavior for HTTPError/URLError, IncompleteRead/HTTPException, raw TimeoutError/OSError, and empty-message exceptions; confirm no unrelated validation/programming errors are swallowed and that current-main #1567's separate merged-tree coverage gap is not misclassified as this PR's source defect.

[seonghobae]

@opencode-agent review exact current head a74fd03102fd5850a591747f3b193029cf71f45d against protected main@5686de41660d51a7a7f22b8840dfa6ccfe5ff3f1. @cwl-noema-review independently review the same exact head. All current Devin transport/retry findings are resolved; do not reuse predecessor verdicts. Auto-merge is armed but may complete only after fresh exact-head required checks and qualifying review policy succeed.