fix(tui): survive private-mode CSI sequences in adapter logs

[pbean]

Summary

Fixes #111 — bmad-loop tui crashed with TypeError: Screen.select_graphic_rendition() got an unexpected keyword argument 'private' while polling a gemini-adapter dev-session log.

Root cause

The gemini CLI's startup preamble includes XTMODKEYS CSI > 4 ; ? m (\x1b[>4;?m). We already strip private-marker SGR before feeding pyte (_PRIVATE_MARKER_SGR), but the regex only tolerated the marker byte in first position — a ? inside the params leaked through. pyte 0.8.2's parser dispatches any private-marked CSI with private=True, and select_graphic_rendition rejects the kwarg. The TypeError escaped LogView.read_new() into the poll worker, which has no try/except; textual workers default to exit_on_error=True, so the whole app died.

Not fixable by dependency bump: pyte's latest release is 0.8.2 (Nov 2023). Upstream fixed exactly this in master (selectel/pyte#202, Sep 2025 — if private: return) but never released it, and only patched the SGR handler — 22 of 27 CSI handlers still reject private= (verified \x1b[?1;1H crashes cursor_position the same way).

Fix (two layers, both in the data layer per the existing degrade convention)

1. Broaden _PRIVATE_MARKER_SGR to match a marker byte (< > = ?) anywhere in the param area of an m-final CSI. Covers gemini's >4;?m, vim9's ?4m, and the already-handled >4;2m. Legitimate SGR params are only [0-9;:], so genuine styling is untouched; h/l finals (DECSET/DECRST — ?25l, ?2004h, ?1049h) deliberately still flow to pyte. The held-back-tail logic (_INCOMPLETE_CSI_TAIL) already admits marker bytes anywhere, so split-across-reads filtering needed no change.
2. _TolerantByteStream — _send_to_parser drops any sequence pyte raises on instead of propagating. pyte re-initializes its parser before re-raising, so the stream stays usable for everything after; verified empirically that subsequent styled output renders correctly. This makes it impossible for any future/unknown escape sequence to kill the poll worker.

Tests

Six new regression tests in tests/test_tui_data.py, including the exact byte prefix from the issue's traceback and a stream-guard test using an unfilterable private CSI (\x1b[?1;1H). Verified the fix-reverted tree fails 4 of them with the exact TypeError from the issue. Full suite: 2050 passed; full trunk check clean.

Summary by CodeRabbit

• Bug Fixes

  • Fixed a TUI crash when processing private terminal control sequences in adapter logs.
  • Improved handling of terminal output from Gemini, Vim, and similar applications.
  • Ensured subsequent log rendering and styling continue after unsupported sequences are encountered.

• Tests

  • Added regression coverage for private control sequences, split reads, and terminal startup sequences.

• Documentation

  • Added an unreleased changelog entry describing the TUI stability fix.

[coderabbitai]

Walkthrough

The TUI now filters private CSI markers appearing anywhere in parameters and uses a tolerant pyte byte stream to suppress parser failures. Regression tests cover multiple marker formats, split reads, startup sequences, and continued rendering.

Changes

TUI CSI resilience

Layer / File(s)	Summary
CSI filtering and tolerant stream src/bmad_loop/tui/data.py	Private-marker CSI matching now covers markers within parameters, and LogView routes parsing through _TolerantByteStream.
Regression coverage and changelog tests/test_tui_data.py, CHANGELOG.md	Tests cover private CSI variants, split reads, startup input, unsupported sequences, and continued styling; the changelog records the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

I’m a rabbit with ears held high,
Filtering odd CSI as they fly.
Pyte may stumble, but logs carry on,
Through Gemini dawn to the next line of song.
Hop, hop—no TUI crash by dawn!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main fix: the TUI now survives private-mode CSI sequences in adapter logs.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/tui-pyte-private-csi-111

---

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.}

[pbean]

@CodeRabbit review
augment review

[augmentcode]

Looks like you're out of credits for this review. Head to your account settings to top up and keep reviews running. https://app.augmentcode.com/account/subscription

[coderabbitai]

@pbean, I'll review the changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

🧹 Nitpick comments (1)

src/bmad_loop/tui/data.py (1)

408-422: 🩺 Stability & Availability | 🔵 Trivial

Log dropped parser exceptions. _send_to_parser is a safe place to keep the TUI alive, but a debug log on the exception path would make malformed-sequence regressions much easier to diagnose.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/bmad_loop/tui/data.py` around lines 408 - 422, Add debug-level logging in
_TolerantByteStream._send_to_parser when super()._send_to_parser raises,
recording the exception and relevant parser/input context while preserving the
existing behavior of returning None so the TUI remains alive.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/bmad_loop/tui/data.py`:
- Around line 408-422: Add debug-level logging in
_TolerantByteStream._send_to_parser when super()._send_to_parser raises,
recording the exception and relevant parser/input context while preserving the
existing behavior of returning None so the TUI remains alive.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e8efe2f2-4e2c-4d3d-aff6-69b0631388a5

📥 Commits

Reviewing files that changed from the base of the PR and between b7afb1e and fd0bb2e.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/bmad_loop/tui/data.py
• tests/test_tui_data.py

[pbean]

@coderabbitai Declining the logging nitpick on _TolerantByteStream._send_to_parser, with reasoning:

• No logging subsystem exists in this codebase — src/bmad_loop/ has zero import logging/getLogger anywhere. An unconfigured logging.debug(...) call is a silent no-op (no handlers; DEBUG sits below the root default of WARNING) — dead code that looks like observability.
• The two real diagnostic surfaces are both unusable at this spot: textual.log is forbidden in data.py (the module must stay textual-free — enforced by test_data_imports_without_textual), and stderr output corrupts a live Textual display.
• The repo's convention for observation-path faults is deliberately silent degrade with an explanatory comment (_char_style → uncolored on ColorParseError, liveness → "unknown" on host errors); none of those precedents log or count either.
• The diagnostic intent is covered: the class docstring records the drop semantics and the upstream root cause (TypeError in select_graphic_rendition handler (argument private) from SGR sequence ESC[?4m selectel/pyte#202, fixed in master but unreleased), and six regression tests pin the behavior. A future malformed-sequence investigation reproduces via LogView against the captured log file directly — exactly how TUI crashes with TypeError: Screen.select_graphic_rendition() got an unexpected keyword argument 'private' when polling gemini adapter log #111 itself was diagnosed.

On the docstring-coverage pre-merge warning: the diff is mostly test functions, which this repo documents with comment narration rather than docstrings (consistent with every existing test in tests/test_tui_data.py); the only new non-test unit carries a docstring. Treating as advisory.

No code changes; resolving.

[pbean]

@coderabbitai resolve

[coderabbitai]

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

[coderabbitai]

@pbean, that's a well-reasoned rebuttal — thanks for the detailed breakdown. Agreed on all points:

• No logging subsystem in src/bmad_loop/ means an unconfigured logging.debug(...) call would be inert, so it's fair to treat it as dead-weight observability rather than an improvement.
• textual.log being off-limits in data.py (enforced by test_data_imports_without_textual) and stderr risk to a live Textual display both rule out the other obvious diagnostic surfaces.
• Falling back to the existing "silent degrade + explanatory comment" convention (as with _char_style/liveness) keeps _TolerantByteStream._send_to_parser consistent with the rest of the codebase, and the docstring + regression tests already pin the drop semantics and root cause (TypeError in select_graphic_rendition handler (argument private) from SGR sequence ESC[?4m selectel/pyte#202).
• The docstring-coverage advisory point is reasonable too, given the diff is mostly test functions following the existing comment-narration convention in tests/test_tui_data.py.

Withdrawing both the logging nitpick and the docstring-coverage advisory. Resolving as requested.

(⁠^⁠‸⁠^⁠) noted, thanks for walking through it

---

✏️ Learnings added

Learnt from: pbean
URL: https://github.com/bmad-code-org/bmad-loop/pull/114

Timestamp: 2026-07-10T06:06:55.373Z
Learning: Repository: bmad-code-org/bmad-loop. In `src/bmad_loop/tui/data.py`, there is no logging subsystem anywhere in `src/bmad_loop/` (no `import logging`/`getLogger`), so suggestions to add `logging.debug(...)` calls should be avoided since they'd be silent no-ops without configured handlers. Additionally, `textual.log` cannot be used in `data.py` because the module must remain textual-free (enforced by `test_data_imports_without_textual`), and stderr output is avoided there because it corrupts a live Textual display. The repo's convention for observation-path faults in this module is silent degrade with an explanatory comment (e.g., `_char_style` falls back to uncolored on `ColorParseError`; liveness checks fall back to `"unknown"` on host errors) rather than logging or counting failures. Test functions in `tests/test_tui_data.py` are documented via comment narration rather than docstrings, consistent with existing tests in that file.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.