Skip to content

fix(sidecar): keep the exception type and innermost frame per traceback - #1950

Merged
seonghobae merged 3 commits into
mainfrom
fix/sidecar-sanitizer-traceback-type-20260906
Sep 6, 2026
Merged

fix(sidecar): keep the exception type and innermost frame per traceback#1950
seonghobae merged 3 commits into
mainfrom
fix/sidecar-sanitizer-traceback-type-20260906

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Problem

.github#1812's strix run 33993155419 (new sidecar, pin 464da471) ended after 9014 s on the gateway's HTTP 500 {'error': {'code': 'internal_error'}} — contextual-orchestrator's generic except Exception: traceback.print_exc(); _send_error(500, "internal_error", …) path, which prints one traceback per unhandled exception. The sidecar stderr artifact holds 83 request_failed status=500 code=internal_error lines and exactly one sidecar emitted an unexpected exception line: scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py drops every traceback line and prints that sentence once per stream, so neither the exception type, nor where it escaped, nor how many times, survives into strix-reports / noema-sidecar-evidence. The observability lane (#1943/#1944/#1945) made provider walks readable; unhandled exceptions stayed opaque. Diagnosis on #1812: #1812 (comment)

Related: the escaping exception itself is fixed at its canonical owner in contextual-orchestrator#1082 (a bare TimeoutError on the tools/passthrough path is recorded on the breaker and classified to 502 provider_connection_error, still without replay). Acceptance chain: after this PR, the first strix failure artifact should carry unexpected_exception type=TimeoutError frame=contextual_orchestrator/orchestrator.py:…:_send_raw; once the sidecar pin passes #1082, that line disappears in favour of 502 provider_connection_error plus a circuit_failure event. Note the sidecar pin moved to 414f2297 (contextual-orchestrator#1081) in .github@efb89269 — the orchestrator templates this sanitizer admits were re-checked against that revision (see the PR thread).

Change

  • Each traceback is reduced to one line: unexpected_exception type=<ExceptionType> frame=contextual_orchestrator/<module>.py:<line>:<function>. A traceback opens at its Traceback header and closes at the column-0 terminal Type: message line, at the next header or allowlisted line, or at end of stream (type=unknown). Indented frame/source lines are consumed (not counted as omitted); only a contextual_orchestrator frame's package-relative path, line and function are kept (innermost wins). The two chain sentences are consumed, so a chained exception yields cause then effect.
  • Never re-emitted: the exception message (can carry provider bodies or credentials), source echoes, non-package frames. The type is bounded by regex ([A-Za-z_]\w{0,63} dotted up to 9 segments), the function by [A-Za-z0-9_<>]{1,80}.
  • Consumers: only the sanitizer's own test parsed the old sentence (grep over scripts/, workflows, docs, tests).

Tests

  • test_sidecar_stream_sanitizer_keeps_exception_type_and_innermost_frame: a real traceback rendered by traceback.format_exc() from code compiled under a …/contextual_orchestrator/server.py filename (do_POST → _serve → KeyError) → type=KeyError frame=contextual_orchestrator/server.py:2:_serve; the secret in the payload and the test file's own frame are absent.
  • …_keeps_dotted_exception_types_and_chains: package-defined ProviderResponseError raised from a KeyError → two lines, cause then effect, dotted type kept, message (sk-leak) absent.
  • …_closes_a_truncated_traceback_at_end_of_stream: sidecar dies mid-traceback → type=unknown frame=contextual_orchestrator/orchestrator.py:7824:_invoke.
  • …_does_not_treat_free_text_as_an_exception: a column-0 free-text line closes the traceback and is counted as omitted.
  • Existing …_summarizes_unstructured_and_traceback_lines expectation updated (two headers → two unknown lines; the indented pseudo-frame no longer counts as omitted).

Evidence

Gate on the head with GITHUB_ACTIONS=true: see the PR's first comment (full suite / coverage / interrogate figures). Negative control: the four new tests fail on main's sanitizer.

Developer experience: a Noema/Strix failure whose sidecar stderr shows unexpected_exception type=… frame=… names the escaping exception and the orchestrator frame directly in the artifact; omitted_unstructured_lines no longer absorbs traceback bodies.

User experience: none (CI evidence only).

Acceptance: the next noema-sidecar-evidence / strix-reports produced after this lands, for a run that hits gateway 500 internal_error, carries one unexpected_exception type=<T> frame=contextual_orchestrator/… line per 500 instead of a single sidecar emitted an unexpected exception.

🤖 Generated with Claude Code

The sidecar stream sanitizer collapsed every Python traceback into a single
"sidecar emitted an unexpected exception" line, printed once per stream. On
.github#1812's strix run 33993155419 the gateway answered 83 requests with
500 internal_error -- contextual-orchestrator's generic
`except Exception: traceback.print_exc(); _send_error(500, ...)` path -- and no
artifact could say which exception escaped, where, or how many times.

Each traceback now yields one bounded line,
`unexpected_exception type=<Type> frame=contextual_orchestrator/<module>.py:<line>:<fn>`:
opened at the header, closed at the column-0 terminal line (or the next
header, an allowlisted line, or end of stream -> type=unknown). Indented
frame/source lines are consumed rather than counted as omitted; only a
contextual_orchestrator frame's package path, line and function are kept, and
the exception message is never re-emitted. Chain sentences are consumed so a
chained exception yields cause then effect.

Tests render real tracebacks with traceback.format_exc() from code compiled
under a contextual_orchestrator/ filename (plain, dotted package type with a
`from` chain, truncated at end of stream, free-text column-0 line), and the
existing traceback expectation is updated. Only the sanitizer's own test
consumed the old sentence.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Gate on a9125755 (GITHUB_ACTIONS=true, local): tests/ 2908 passed, 1 skipped, 21 subtests; coverage report --fail-under=100 → 100% (sanitize_contextual_orchestrator_sidecar_stream.py 98 statements / 46 branches, 0 missed; TOTAL 13181/5326, 0 missed); interrogate → 100%.

Negative control: with origin/main's sanitizer and this head's tests, the four new tests plus the updated traceback expectation fail (5 failed / 5 passed of the sanitizer selection); restored head verified by symbol presence.

Design notes: blank lines are skipped outright (Python pads chain sentences with them; they carry nothing); a column-0 line inside a traceback that is neither a terminal Type[: msg] nor allowlisted closes the traceback with type=unknown and is counted as omitted, so free text can never be misread as an exception type; the terminal regex admits only a dotted identifier (≤9 segments, ≤64 chars each) followed by : or end of line.

@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 27 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: d436cccb-a3f4-4f90-8900-c274c31ae63b

📥 Commits

Reviewing files that changed from the base of the PR and between 46f5761 and e18d846.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py
  • tests/test_contextual_orchestrator_review_runtime_preflight.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

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

@seonghobae

Copy link
Copy Markdown
Contributor Author

Sidecar pin 414f2297 re-check (the templates #1945's allowlist is pinned to): all ten contextual_orchestrator/orchestrator.py logger templates the sanitizer admits (provider_attempt, provider_attempt_failed, provider_backoff, provider_exhausted, provider_rejected_permanent, provider_no_retry_budget, circuit_failure, circuit_opened, circuit_reset, circuit_cleared) are byte-identical between 2e414d15 and 414f2297, and that revision adds no new provider_*/circuit_*/preflight_* template. So the allowlist keeps matching after .github@efb89269; what changes is only the attempt denominator on the no-tools path (attempt=n/2 instead of n/3, contextual-orchestrator#1081), which the attempt=\d+/\d+ pattern already admits. Rule going forward: every sidecar pin bump should re-run this template diff (a one-liner over the two orchestrator.py blobs), because an allowlist fails silently into omitted_unstructured_lines.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Re-merged main@efb89269 as 71562e01 (the PR went dirty after the sidecar pin bump landed). Only CHANGELOG.md conflicted — both entries prepend at the top; resolved with this PR's section above the pin-advance section. No overlap with the sanitizer or its tests (efb89269 touches the sidecar script, ADR-0003 and the sidecar contract test). Gate on 71562e01 with GITHUB_ACTIONS=true: 2908 passed / 1 skipped / 21 subtests, coverage 100% (13181 statements, 5326 branches, 0 missed), interrogate 100%.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Re-merged main@46f5761a (#1947) as e18d846e — automatic merge, no conflicts: #1947 widens _PREFLIGHT_ROUTE_REJECTED to preflight_route_(rejected|deferred) inside sanitize_line, this PR adds the three traceback patterns, _traceback_summary and the main() state machine — disjoint regions; the merged file carries both (checked by symbol presence, not by trusting the merge). Gate on e18d846e with GITHUB_ACTIONS=true: 2913 passed / 1 skipped / 21 subtests, coverage 100% (13181 statements, 5326 branches, 0 missed), interrogate 100%. Negative control against main's sanitizer: 5 failed / 6 passed of the sanitizer selection (the four new tests plus the updated traceback expectation fail; #1947's deferred test passes on both).

@seonghobae

Copy link
Copy Markdown
Contributor Author

Independent head-tree verification and merge under the chicken-and-egg clause (host 1, 2026-09-06 14:2x KST; merger ≠ author). Requested by lane peer 1 after host 2 did not respond to two merge requests; verified from a fresh checkout of the pushed head, not from the author's report.

  • Head: e18d846e (ls-remote = checkout), base main@46f5761a, 0 behind / 3 ahead. Three-dot delta vs main: 3 files, +208/−8 — scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py (+77/−6: three new bounded regexes _TRACEBACK_FRAME, _TRACEBACK_TERMINAL, _TRACEBACK_CHAIN_LINES, helper _traceback_summary, and the traceback state machine in main()), tests/test_contextual_orchestrator_review_runtime_preflight.py (+136/−2), CHANGELOG.md (+3, own entry above the pin and #1947 entries).
  • Overlap with #1947 (merged 14:05 KST): none — #1947's preflight_route_(rejected|deferred) regex at line 19 is untouched; no duplicate definitions in the merged sanitizer (def names unique); only the type identifier and the package-relative frame are ever emitted, never the message.
  • My gate on this exact head: 2913 passed, 1 skipped, 21 subtests; coverage 100 % (13181 statements, 0 missed; sanitizer 98/98); interrogate 100 % — identical to the author's numbers.
  • Negative control, reproduced independently: this head's test file against main's sanitizer (in a worktree whose sanitizer equals main's) → 5 failed / 6 passed (…summarizes_unstructured_and_traceback_lines, …keeps_exception_type_and_innermost_frame, …keeps_dotted_exception_types_and_chains, …closes_a_truncated_traceback_at_end_of_stream, …does_not_treat_free_text_as_an_exception); the six that pass include #1947's deferred-line test. Identical to the author's count.
  • Checks on e18d846e: all queued at merge time (the head was pushed minutes ago). The clause applies for the same reason as #1947: this file is the review sidecar's own stream sanitizer, the required review contexts run the base branch's scripts and their verdict dispatch is blocked by the owner-gated #1929 variable, and the current provider capacity fails sidecars at preflight regardless of the tree.
  • Acceptance chain after merge: the next failing Strix artifact must carry unexpected_exception type=TimeoutError frame=contextual_orchestrator/orchestrator.py:<line>:_send_raw (the #1812 class); once the sidecar pin passes contextual-orchestrator#1082, that line must be gone and the same situation must read as a provider_connection_error 502 with a circuit_failure line.
  • Merge method: merge commit.

@seonghobae
seonghobae merged commit ff9848a into main Sep 6, 2026
3 of 15 checks passed
@seonghobae
seonghobae deleted the fix/sidecar-sanitizer-traceback-type-20260906 branch September 6, 2026 05:10
seonghobae pushed a commit that referenced this pull request Sep 6, 2026
Brings the branch current with protected main (24 commits, including the
contextual-orchestrator pin advance in efb8926 and the sidecar preflight
changes in #1947/#1949/#1950) so the gap-baseline entry this PR carries
can be extended with the retry-stacking root cause and its first post-pin
production measurement.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
seonghobae pushed a commit that referenced this pull request Sep 6, 2026
Brings the branch current with protected main (24 commits since 6f8c51d).
main did not touch .github/workflows/opencode-review-dispatch.yml in that
range, so this branch's REVIEW_DISPATCH_BLOB_SHA pin (0a39def) still
matches the merged workflow; the only textual merge was CHANGELOG.md, which
auto-merged. The merged main carries the contextual-orchestrator pin
advance (efb8926) and the sidecar preflight repairs (#1947, #1949,
#1950), so this head's required reviews run against the repaired gateway
instead of the retry-stacking pin that failed the previous head's
noema-review (502 after 2343 s) and strix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
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