feat: improve auto-mode and TUI rendering - #62
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughHardens unattended auto/yolo approvals and plan-mode gating; adds ChangesYOLO/Auto Mode Hardening and Safety
Code Theme Customization and Packaging
Paced Text Reveal and Smooth Streaming Animation
TUI Visual Updates and Styling
Documentation and Analysis
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ceb9f5e to
a7c5fe3
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pythinker_code/soul/agent.py (1)
282-301:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--no-yololeaks into persisted session state.
effective_yolocorrectly forces the runtime off for this run, but_on_approval_change()still writes that effective value back tosession.state.approval.yolo. Any later state change in the run (approve_for_session,/auto, safe-mode toggles, etc.) will permanently clear the previously persisted yolo flag, which contradicts the "for this run" contract.Possible fix
+ persisted_yolo = session.state.approval.yolo + # Merge invocation flags with persisted session state. ``--no-yolo`` is an explicit # force-off that beats the flag, config ``default_yolo``, and persisted state. effective_yolo = (yolo or session.state.approval.yolo) and not no_yolo @@ def _on_approval_change() -> None: - session.state.approval.yolo = approval_state.yolo + session.state.approval.yolo = persisted_yolo if no_yolo else approval_state.yolo session.state.approval.auto = approval_state.auto session.state.approval.auto_approve_actions = set(approval_state.auto_approve_actions) session.state.trust.safe_mode = approval_state.safe_mode session.save_state()🤖 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/pythinker_code/soul/agent.py` around lines 282 - 301, The runtime `--no-yolo` flag is being persisted back into session state; capture the original persisted yolo value (e.g., original_persisted_yolo = session.state.approval.yolo) before computing effective_yolo and then update `_on_approval_change()` so it only writes `session.state.approval.yolo = approval_state.yolo` when the invocation did not set `no_yolo` (i.e., if not no_yolo) — otherwise leave `session.state.approval.yolo` as the original persisted value; this prevents the temporary `--no-yolo` runtime override from being saved permanently.
🤖 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.
Inline comments:
In `@src/pythinker_code/app.py`:
- Around line 66-77: The banner in _resumed_unsupervised_notice incorrectly
asserts actions are auto-approved whenever auto is true; change the message
logic so it only says "actions auto-approved" for YOLO (yolo==True), and for
auto==True (but yolo==False) use wording that does not promise auto-approval
(e.g., "auto mode active — interactive approvals still required" or similar).
Update the string construction that uses yolo and auto so the text for the
"auto" case reflects that auto may not imply auto-approve.
In `@src/pythinker_code/utils/rich/syntax.py`:
- Around line 98-113: PythinkerSyntax currently ignores the process-wide theme
by hardcoding PYTHINKER_ANSI_THEME in its constructor; update
PythinkerSyntax.__init__ to default to get_active_code_theme() (or fall back to
PYTHINKER_ANSI_THEME only if get_active_code_theme() is empty) instead of using
the literal PYTHINKER_ANSI_THEME, so instances follow the process-wide state set
via set_active_code_theme/get_active_code_theme and the _active_code_theme
variable.
In `@tasks/yolo-auto-mode-analysis.md`:
- Around line 18-23: The fenced code block for is_auto_approve() in
tasks/yolo-auto-mode-analysis.md needs a language tag and blank lines: add a
blank line before the triple-backtick fence and specify the language (e.g.,
```python) and add a blank line after the closing fence; also add missing blank
lines above and below each subsection heading later in the file (around the
164-202 region) so every heading is separated by a blank line from surrounding
text to satisfy markdownlint rules.
- Around line 123-130: Update the contradictory status text in
tasks/yolo-auto-mode-analysis.md: reconcile Section 4b ("No production code
changed") with Section 7's handling of B3 (which currently first says B3 was
deferred and then lists B3a/B3b/B3c as implemented) so the document consistently
states whether B3 was implemented or deferred; specifically edit the entries for
B3 and the Section 4b line and the related block around "B3a/B3b/B3c" so they
match, and scan the nearby lines referenced (around the Section 7 discussion and
lines ~160-195) to remove the contradictory phrasing and ensure the status for
B1/B2/B3 and the note about "No production code changed" are accurate and
consistent.
In `@tests/ui_and_conv/test_shell_prompt_echo.py`:
- Around line 193-208: The test currently only checks that the user_message_bg
escape sequence appears somewhere; tighten it so the tint is verified on the
"apply" token itself by either (a) asserting the expected escape sequence occurs
immediately before the literal "apply" in capture.get() (e.g. check for
f"{expected}mapply" within the captured output) or (b) better, inspect Rich's
rendered segments/styles instead of raw escapes: render the object returned by
render_user_echo_text via Console.render/Console.render_lines or Console.capture
and iterate the resulting segments/text objects to assert that the Text segment
containing "apply" has the style produced by tui_rich_style("user_message_bg");
reference test_user_echo_wraps_message_in_tinted_block, render_user_echo_text,
tui_rich_style and expected to locate where to change the assertion.
---
Outside diff comments:
In `@src/pythinker_code/soul/agent.py`:
- Around line 282-301: The runtime `--no-yolo` flag is being persisted back into
session state; capture the original persisted yolo value (e.g.,
original_persisted_yolo = session.state.approval.yolo) before computing
effective_yolo and then update `_on_approval_change()` so it only writes
`session.state.approval.yolo = approval_state.yolo` when the invocation did not
set `no_yolo` (i.e., if not no_yolo) — otherwise leave
`session.state.approval.yolo` as the original persisted value; this prevents the
temporary `--no-yolo` runtime override from being saved permanently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad2472a8-d2cc-4b52-9730-133e68975dd9
📒 Files selected for processing (40)
CHANGELOG.mddocs/en/release-notes/changelog.mdpackages/linux-installer/pythinker.specpackages/windows-installer/pythinker.specsrc/pythinker_code/app.pysrc/pythinker_code/cli/__init__.pysrc/pythinker_code/config.pysrc/pythinker_code/soul/agent.pysrc/pythinker_code/soul/approval.pysrc/pythinker_code/soul/dynamic_injections/auto_mode.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/components/markdown.pysrc/pythinker_code/ui/shell/echo.pysrc/pythinker_code/ui/shell/motion.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/tool_renderers/ask_user.pysrc/pythinker_code/ui/shell/visualize/_blocks.pysrc/pythinker_code/ui/shell/visualize/_interactive.pysrc/pythinker_code/ui/shell/visualize/_live_view.pysrc/pythinker_code/ui/theme.pysrc/pythinker_code/utils/pyinstaller.pysrc/pythinker_code/utils/rich/markdown.pysrc/pythinker_code/utils/rich/syntax.pytasks/yolo-auto-mode-analysis.mdtests/core/test_approval_auto.pytests/core/test_auto_injection.pytests/core/test_config.pytests/core/test_plan_mode_auto_approval.pytests/core/test_resume_safety_notice.pytests/core/test_runtime_auto_state.pytests/ui_and_conv/test_code_theme_opt_in.pytests/ui_and_conv/test_live_view_todos.pytests/ui_and_conv/test_shell_motion_shimmer.pytests/ui_and_conv/test_shell_prompt_echo.pytests/ui_and_conv/test_stream_pacing.pytests/ui_and_conv/test_streaming_content_block.pytests/ui_and_conv/test_tui_card_tool_renderers.pytests/ui_and_conv/test_tui_theme_tokens.pytests/utils/test_pyinstaller_utils.py
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
tasks/yolo-auto-mode-analysis.md (2)
164-164:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd blank lines after subsection headings.
Headings at lines 164, 176, 182, 196, and 202 are missing blank lines below them, violating MD022.
📝 Proposed fix
Add a blank line after each of these headings:
- Line 164:
### B1 — destructive backstop now holds whenever unattended- Line 176:
### B2 — plan-mode checkpoint preserved under interactive yolo- Line 182:
### B3 — persisted-state footguns (all three implemented)- Line 196:
### B4 — resolved as correct-as-designed (no change)- Line 202:
### Tests (all RED→GREEN)Also applies to: 176-176, 182-182, 196-196, 202-202
🤖 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 `@tasks/yolo-auto-mode-analysis.md` at line 164, The markdown headings "### B1 — destructive backstop now holds whenever unattended", "### B2 — plan-mode checkpoint preserved under interactive yolo", "### B3 — persisted-state footguns (all three implemented)", "### B4 — resolved as correct-as-designed (no change)", and "### Tests (all RED→GREEN)" each lack a blank line below them; update the file by inserting a single blank line immediately after each of those heading lines so they conform to MD022 (add blank lines after the headings named above).
19-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the markdownlint violations.
The code fence is missing a language specifier and surrounding blank lines. Add a blank line before line 19, change the fence to
```python, and add a blank line after line 23.📝 Proposed fix
Key compound: `is_auto_approve()` (`approval.py:223-234`): + -``` +```python if yolo: return True # YOLO overrides everything below if safe_mode: return False # untrusted workspace blocks auto (but NOT yolo) return is_auto()
</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@tasks/yolo-auto-mode-analysis.mdaround lines 19 - 23, Add surrounding blank
lines and a language specifier to the fenced code block containing the snippet
with symbols yolo, safe_mode, and is_auto: insert one blank line before the
opening fence, change the fence to ```python, and insert one blank line after
the closing fence so the block is properly delimited and markdownlint-compliant.</details> </blockquote></details> <details> <summary>src/pythinker_code/utils/rich/syntax.py (1)</summary><blockquote> `116-120`: _⚠️ Potential issue_ | _🟠 Major_ | _⚡ Quick win_ **`PythinkerSyntax` ignores the configured theme.** `PythinkerSyntax.__init__` still hardcodes `PYTHINKER_ANSI_THEME` when no theme is provided, so direct `PythinkerSyntax(code, lexer)` calls won't pick up `config.tui.code_theme` set via `set_active_code_theme`. The process-wide state is wired but unused here. <details> <summary>🔧 Proposed fix</summary> ```diff class PythinkerSyntax(Syntax): def __init__(self, code: str, lexer: str, **kwargs: Any) -> None: if "theme" not in kwargs or kwargs["theme"] is None: - kwargs["theme"] = PYTHINKER_ANSI_THEME + kwargs["theme"] = resolve_code_theme(get_active_code_theme()) super().__init__(code, lexer, **kwargs) ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` 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/pythinker_code/utils/rich/syntax.py` around lines 116 - 120, PythinkerSyntax.__init__ currently hardcodes PYTHINKER_ANSI_THEME when no theme is passed, so it should instead read the process-wide active theme; change the "if 'theme' not in kwargs or kwargs['theme'] is None" branch to call the global/theme accessor (e.g. get_active_code_theme() or config.tui.code_theme) and assign that value to kwargs["theme"], only falling back to PYTHINKER_ANSI_THEME if the accessor returns None; ensure the code still forwards kwargs into super().__init__(code, lexer, **kwargs) so the selected theme is actually used. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@docs/en/release-notes/changelog.md:
- Around line 20-24: The changes were made to the auto-synced docs mirror
(docs/en/release-notes/changelog.md) instead of the canonical root changelog;
revert or remove your manual edits from docs/en/release-notes/changelog.md, add
the new release-note bullets to the root CHANGELOG.md (preserving existing
changelog format), then run the repository's changelog sync/regeneration script
to regenerate docs/en/release-notes/changelog.md and commit the updated root
CHANGELOG.md and regenerated docs file so the mirror remains authoritative.In
@src/pythinker_code/ui/shell/visualize/_blocks.py:
- Around line 451-458: The paced-preview branch currently renders raw preview
text with Text(preview), allowing ANSI/control sequences to leak; change the
paced-path to sanitize the preview before wrapping in Text by calling
sanitize_ansi(preview) (or the project's equivalent sanitizer) so the interim
tail is safe, while leaving the non-paced path using Markdown(preview) and
completed blocks committed via render_agent_body; update the code paths around
the _paced check in the block rendering logic (the Text/Markdown selection in
_blocks.py) to use sanitize_ansi for the paced branch.In
@tests/utils/test_pyinstaller_utils.py:
- Around line 247-294: Replace the hard-coded list of pygments.styles.* modules
in tests/utils/test_pyinstaller_utils.py with a dynamic discovery: import
pygments.styles and use pkgutil.iter_modules(pygments.styles.path) or
importlib.metadata to build the current set of "pygments.styles." modules
at runtime, then assert/compare only against the project-owned entries (keep the
snapshot for our own modules) or ensure our expected hiddenimports are a subset
of the discovered set; update the test logic that previously referenced the full
static list to generate discovered_styles = {"pygments.styles."+m.name for m in
iter_modules(...)} and only check our project-specific expected entries against
discovered_styles.
Duplicate comments:
In@src/pythinker_code/utils/rich/syntax.py:
- Around line 116-120: PythinkerSyntax.init currently hardcodes
PYTHINKER_ANSI_THEME when no theme is passed, so it should instead read the
process-wide active theme; change the "if 'theme' not in kwargs or
kwargs['theme'] is None" branch to call the global/theme accessor (e.g.
get_active_code_theme() or config.tui.code_theme) and assign that value to
kwargs["theme"], only falling back to PYTHINKER_ANSI_THEME if the accessor
returns None; ensure the code still forwards kwargs into super().init(code,
lexer, **kwargs) so the selected theme is actually used.In
@tasks/yolo-auto-mode-analysis.md:
- Line 164: The markdown headings "### B1 — destructive backstop now holds
whenever unattended", "### B2 — plan-mode checkpoint preserved under interactive
yolo", "### B3 — persisted-state footguns (all three implemented)", "### B4 —
resolved as correct-as-designed (no change)", and "### Tests (all RED→GREEN)"
each lack a blank line below them; update the file by inserting a single blank
line immediately after each of those heading lines so they conform to MD022 (add
blank lines after the headings named above).- Around line 19-23: Add surrounding blank lines and a language specifier to the
fenced code block containing the snippet with symbols yolo, safe_mode, and
is_auto: insert one blank line before the opening fence, change the fence toproperly delimited and markdownlint-compliant.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID:
567e38ef-ca9a-4d0e-8127-f182743af54f📒 Files selected for processing (40)
CHANGELOG.mddocs/en/release-notes/changelog.mdpackages/linux-installer/pythinker.specpackages/windows-installer/pythinker.specsrc/pythinker_code/app.pysrc/pythinker_code/cli/__init__.pysrc/pythinker_code/config.pysrc/pythinker_code/soul/agent.pysrc/pythinker_code/soul/approval.pysrc/pythinker_code/soul/dynamic_injections/auto_mode.pysrc/pythinker_code/soul/pythinkersoul.pysrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/components/markdown.pysrc/pythinker_code/ui/shell/echo.pysrc/pythinker_code/ui/shell/motion.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/tool_renderers/ask_user.pysrc/pythinker_code/ui/shell/visualize/_blocks.pysrc/pythinker_code/ui/shell/visualize/_interactive.pysrc/pythinker_code/ui/shell/visualize/_live_view.pysrc/pythinker_code/ui/theme.pysrc/pythinker_code/utils/pyinstaller.pysrc/pythinker_code/utils/rich/markdown.pysrc/pythinker_code/utils/rich/syntax.pytasks/yolo-auto-mode-analysis.mdtests/core/test_approval_auto.pytests/core/test_auto_injection.pytests/core/test_config.pytests/core/test_plan_mode_auto_approval.pytests/core/test_resume_safety_notice.pytests/core/test_runtime_auto_state.pytests/ui_and_conv/test_code_theme_opt_in.pytests/ui_and_conv/test_live_view_todos.pytests/ui_and_conv/test_shell_motion_shimmer.pytests/ui_and_conv/test_shell_prompt_echo.pytests/ui_and_conv/test_stream_pacing.pytests/ui_and_conv/test_streaming_content_block.pytests/ui_and_conv/test_tui_card_tool_renderers.pytests/ui_and_conv/test_tui_theme_tokens.pytests/utils/test_pyinstaller_utils.py
- Sanitize ANSI sequences in paced preview text (_blocks.py) to prevent control-sequence leaks when the markdown path is bypassed - Replace brittle Pygments style snapshot with dynamic pkgutil.iter_modules check; snapshot now covers only project-owned hiddenimports entries - Add blank lines around code fence and subsection headings in tasks/yolo-auto-mode-analysis.md to satisfy markdownlint (MD031/MD040/MD022) - Reconcile contradictory B3/section-4b status in the analysis doc
Summary
Verification
uv run ruff check src/pythinker_code/ui/shell/visualize/_blocks.py tests/ui_and_conv/test_streaming_content_block.py tests/ui_and_conv/test_stream_pacing.pyuv run ruff format --check src/pythinker_code/ui/shell/visualize/_blocks.py tests/ui_and_conv/test_streaming_content_block.py tests/ui_and_conv/test_stream_pacing.pyuv run pyright src/pythinker_code/ui/shell/visualize/_blocks.py tests/ui_and_conv/test_streaming_content_block.py tests/ui_and_conv/test_stream_pacing.pyuv run pytest tests/ui_and_conv/test_stream_pacing.py tests/ui_and_conv/test_streaming_content_block.py tests/ui_and_conv/test_md_stream_idempotency.py tests/ui/test_shell_markdown.pySummary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests