Skip to content

fix(console): never crash on MarkupError — chokepoint escape fallback#197

Merged
jacsamell merged 1 commit into
mainfrom
fix-console-never-crashes
May 21, 2026
Merged

fix(console): never crash on MarkupError — chokepoint escape fallback#197
jacsamell merged 1 commit into
mainfrom
fix-console-never-crashes

Conversation

@jacsamell
Copy link
Copy Markdown
Contributor

@jacsamell jacsamell commented May 21, 2026

Summary

  • Subclass Rich's Console so it never raises MarkupError. If markup parsing fails, re-render with markup=False and dynamic content pre-escaped.
  • Applied to cube.core.output.console/console_err and BaseThinkingLayout.console — the two consoles that carry judge/agent generated text.
  • Per-site escape() calls remain valid but are no longer load-bearing.

Why

Three panel runs on aetheron-connect-v2#1361 crashed mid-render with MarkupError, hitting different judges each time (judge_3, then judge_4). Per-site escapes (#193, partially #196) couldn't keep up: the last 2 weeks shipped 4 new high-surface prints (#172, #180, #191, #196) that interpolate judge/Codex/CodeRabbit content. Whack-a-mole was losing.

Architecturally: there's now exactly one place that decides what happens when markup parsing fails. Future print sites are safe-by-default; the worst case is a degraded render (literal [bold] text on the fallback path) instead of a silent panel death.

Test plan

  • Manual: console.print('Issue: [bug] in [/path/to/file.ts]') no longer crashes
  • Manual: console.print(f'[bold cyan]Decision:[/bold cyan] {dynamic}') with bracket-heavy dynamic content renders (cosmetic loss on fallback, but no crash)
  • Re-run cube prv against pr-1361 — should reach the GitHub post step regardless of judge content

🤖 Generated with Claude Code

Problem

Three-panel console rendering crashed with MarkupError when judge/agent-generated text contained unescaped bracket characters, particularly in file paths. Isolated per-site escape() fixes didn't scale as new print sites were added.

Solution

Introduced _SafeConsole, a Rich Console subclass that acts as a chokepoint for all console output. When markup parsing fails, it gracefully degrades by re-rendering with markup=False and escaped dynamic content, eliminating crashes whilst preserving normal styled output when markup is valid.

Changes

  • python/cube/core/output.py: Created _SafeConsole class with overridden print() method that catches MarkupError and falls back to safe rendering. Replaced console and console_err instances with _SafeConsole().
  • python/cube/core/base_layout.py: Updated BaseThinkingLayout to use _SafeConsole instead of raw Rich Console.

Impact

  • Eliminates crashes from bracket-heavy content whilst maintaining visual styling for valid markup
  • Per-site escape() calls become optional rather than required
  • Single protective layer prevents regressions as new console output sites are added

Review Change Stack

…fallback

Rich treats every `[anything]` in printed strings as a markup tag. Judge
output, file paths, dedupe agent text, and CodeRabbit feedback all carry
literal brackets (`[/path/foo.ts]`, `[bug]`, `[ ]`) that look like closing
tags to the parser. Any interpolation site that forgot `escape()` was a
latent panel killer — MarkupError bubbled mid-render, the background task
exited 0, and the GitHub review never got posted.

Per-site escapes (PR #193, partially #196) didn't scale: in the last 2
weeks we added 4 new high-surface print sites (#172 file routing, #180
reply-chain rendering, #191 git-delta injection, #196 missing-judge
list) and the crash kept jumping to whichever one had unescaped content.
Three panel runs on pr-1361 crashed on judge_3 then judge_4 from
different render paths.

Architectural fix: subclass Console once, catch MarkupError, re-render
with markup disabled and dynamic content pre-escaped. One chokepoint.
All existing styled cube output keeps working — only crash-prone inputs
trip the escaped fallback (literal brackets, no tag styling for THAT
print). Future print sites are safe-by-default — no more whack-a-mole.

Applied to:
- `cube.core.output.console` / `console_err` (covers ~95% of cube prints)
- `cube.core.base_layout.BaseLayout.console` (Live render path)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 38e70a30-b1ac-4f0b-8cc8-f943ae1adca5

📥 Commits

Reviewing files that changed from the base of the PR and between ede05ad and 571ccd8.

📒 Files selected for processing (2)
  • python/cube/core/base_layout.py
  • python/cube/core/output.py
📜 Recent review details
🔇 Additional comments (3)
python/cube/core/output.py (1)

4-36: LGTM!

python/cube/core/base_layout.py (2)

33-34: LGTM!


70-70: ⚡ Quick win

Provide the missing review comment text: the message doesn’t include the contents from the <review_comment>...</review_comment> tags needed to rewrite it. Paste the original review comment (and any verification results you have) so I can update it.


Walkthrough

A new _SafeConsole class wraps Rich's console printing to catch markup parsing failures. When MarkupError occurs, the console re-renders arguments with escaped strings and markup disabled. Global console instances in output.py and BaseThinkingLayout are updated to use this crash-proof wrapper.

Changes

SafeConsole Crash-Handling Integration

Layer / File(s) Summary
SafeConsole implementation and module-level console instances
python/cube/core/output.py
Introduces _SafeConsole subclass that intercepts MarkupError in print() and re-renders with escaped strings and markup=False disabled. Replaces global console and console_err instances to use the new wrapper.
BaseThinkingLayout adoption of SafeConsole
python/cube/core/base_layout.py
Updates module imports to use _SafeConsole from output module and modifies BaseThinkingLayout.__init__ to initialise self.console with _SafeConsole() instead of Console().

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit guards the console output,
Catching errors that markup might wrought,
With escape strings and graceful retreat,
Rich printing becomes safe, complete! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: implementing a safeguard to prevent crashes from Rich MarkupError by using an escape fallback mechanism in the console.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • PR-1361: Request failed with status code 401

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

@jacsamell jacsamell merged commit 2585f93 into main May 21, 2026
1 of 2 checks passed
jacsamell added a commit that referenced this pull request May 21, 2026
Five sites that printed dev-debug per panel run, all of which expanded
the crash surface for free (each was a new bracket-bearing dynamic
interpolation site):

- peer_review.py:565 — "Looking for judge_X: <path> (exists: ...)" probe
  per judge. Pure dev debug.
- peer_review.py:567 — per-judge "Decision file not found" warning.
  Same info is now in the panel summary's missing-judges callout, with
  one consolidated line instead of N warnings.
- peer_review.py:611-613 — split "Fetching existing comments" /
  "Found N existing comments" collapsed to one line with the count.
- peer_review.py:498-500, 824-826 — "Resuming N judges" header + a
  "→ Judge Label" line per judge. Collapsed into one comma-joined line.
- judge_panel.py:708-714 — separate "PR lens breakdown" + per-judge
  "Skipping X — no files in lens" lines consolidated into one line.

What stays on the panel render after this: phase headers, one progress
line per judge, the final summary. What used to leak — decision-path
probes, per-comment-fetch debug, per-judge label dumps — is gone.

Pairs with PR #197 (MarkupError chokepoint) but reduces actual surface
area instead of just making it crash-proof. Each line removed is one
fewer dynamic-interpolation site that can carry untrusted brackets.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@jacsamell jacsamell deleted the fix-console-never-crashes branch May 21, 2026 08:48
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