Skip to content

feat(tui): TUI enhancements — adaptive theme, layout, and agent prompt overhaul - #102

Closed
elkaix wants to merge 18 commits into
mainfrom
feat/tui-enhancements
Closed

feat(tui): TUI enhancements — adaptive theme, layout, and agent prompt overhaul#102
elkaix wants to merge 18 commits into
mainfrom
feat/tui-enhancements

Conversation

@elkaix

@elkaix elkaix commented Jun 10, 2026

Copy link
Copy Markdown
Member

Summary

  • Adaptive theme foundation: background probe, color-depth detection, and blending for terminal-aware rendering
  • Layout & chrome refinements: reference-CLI layout, palette polish, renderer guards, and markdown fence table unwrapping
  • Visual consistency: unified todo-list renderer, white running-task titles, diff palette alignment, and transcript-row bullet fixes
  • Status line metadata: elapsed time, token count, and tokens-per-second on the background status line
  • Agent prompt overhaul: all default agents restructured with explicit Mission / Hard Constraints / Workflow / Output Contract sections
  • Background/subagent hardening: stale-record reconciliation, resume contract enforcement, and interactive-visualizer fixes
  • Config default: automatic turn recaps disabled by default

Test plan

  • Run full test suite: pytest
  • Verify TUI renders correctly across terminal types (256-color, truecolor, dark/light bg)
  • Confirm background status line shows elapsed/tokens/t-s metadata during active tasks
  • Check todo-list renders consistently in both modal and inline views
  • Validate agent prompts load and pass spec tests: pytest tests/core/test_agent_spec.py tests/core/test_default_agent.py
  • Run e2e PTY and modal tests: pytest tests/e2e/

Summary by CodeRabbit

  • New Features

    • Auto theme detection; thinking-effort shown as a single top-right input label; selected Qwen models treated as native-thinking (no effort dial).
  • Improvements

    • Major TUI polish: adaptive theming/layout, improved diff & markdown rendering (table unwrapping), syntax-highlighting size guards, refined shimmer/blink effects, and input top-border effort placement. Turn recaps default to off.
  • Stability & Security

    • Hardened background-agent resume/reconciliation, stricter path/SSRF/symlink handling, tightened file/permission safety, and capped retention of resolved approvals.
  • Tests

    • Broad new and updated tests across UI, tools, background, auth, import/export, and integration areas.

elkaix added 11 commits June 9, 2026 18:23
Turn recaps now default off; the agent only recaps when asked. Add a
direct toggle — /config recaps on|off (also /settings recaps ...) —
that persists to the config file and reloads the shell. The interactive
/settings panel keeps its existing turn-recaps item.

Part of the Codex TUI adoption backlog (item 0.1).
Port the Codex terminal-adaptation layer (codex-rs/tui) to Python:

- ui/color_utils.py: hex parse/format, linear RGB blend, BT.601 luma.
- ui/terminal_background.py: OSC 11 background probe (100ms timeout,
  per-process cache, PYTHINKER_NO_BG_PROBE opt-out) and theme = "auto"
  resolution with a dark fallback. /theme and the settings selector
  accept the new value.
- terminal_capabilities.color_depth(): three usable color tiers
  (truecolor/256/16 + none) honoring FORCE_COLOR levels and the
  Windows Terminal WT_SESSION truecolor promotion.
- get_diff_colors(): 16-color terminals now get plain green/red
  foreground diff styles instead of quantized hex background tints.

/theme now compares against the persisted setting rather than the
resolved active theme so users can pin dark/light while on auto.

Backlog items 1.1-1.4.
Adopt the Codex renderer-safety behaviors:

- Syntax-highlight size guard: code blocks beyond 512 KiB or 10k lines
  render as plain text with a 'highlighting skipped (N lines)' notice
  instead of paying an unbounded Pygments cost.
- Large diff guard: expanded diffs are capped at 400 rendered lines
  (head + tail with an explicit omitted-line count) so one huge edit
  cannot freeze or flood the terminal.
- Generic tool output switches from head-only to head-tail truncation,
  keeping the start (identifies the result) and the end (the actionable
  part) with an omitted-line notice.
- Fence unwrapping for tables: ```md/```markdown fences whose body
  contains a header+delimiter table pair now render as markdown instead
  of opaque code. Conservative Codex heuristics: other languages,
  untagged fences, md fences without tables, and unclosed fences pass
  through unchanged.

Backlog items 2.6, 3.1, 6.2, 7.5.
User-directed design wave on top of the Codex adoption Phase 1:

- Transcript marker is now ⏺ (U+23FA) on macOS/Linux — Windows keeps the
  text circle, ASCII mode keeps the star. Tool/assistant rows blink the
  marker while running (reduced-motion pins it static) and settle to the
  solid green marker when finished; thinking rows carry the marker too.
- Activity coral muted to a clay ramp (#C68D7E/#D8AC9E/#E9CDC2 dark,
  #B26A52/#9E563E/#82412D light). Shimmer simplified from wave+splash to a
  calm bidirectional sweep with settle beats; truecolor terminals get a
  continuous cosine-blended sheen via ui.color_utils.blend, lower tiers
  keep the discrete ramp.
- Activity/todo headers share one metadata design:
  'Verb… (12s, ↓ 2.4k tokens, 45 t/s)' — parenthesized, comma-separated,
  with a live tokens/sec readout on the working indicator and the pinned
  todo header (sliding-window rate over the turn's context tokens).
- Thinking-effort frame colors form a cold→hot gradient ending on dark red
  for xhigh (slate→blue→teal→amber→orange→red).
- Pinned todos: the active task title+box are coral; concurrent in-progress
  rows read light grey so the running task stays unmistakable.
- Diff word-level highlights drop reverse-video for the theme's add/del
  highlight backgrounds (GitHub-style emphasis, no glare).
- Turn recap is padded to the card inset instead of spanning edge-to-edge.

Hardening (findings from the in-app review validated and fixed):
- Terminal probe: catch select's ValueError (fd >= FD_SETSIZE), suppress
  tcsetattr restore failures, cap the OSC reply buffer at 4 KiB, and
  serialize the probe cache behind a lock.
- Generic tool-output head/tail truncation halves the char budget per side
  so combined output can never exceed the limit.
- /settings arg parsing splits the mode string once.
- Added ~~~md tilde-fence unwrap coverage.
Second design wave driven by side-by-side comparison with the reference
transcripts:

- Tool headers use the parenthesized single-line form — ⏺ Bash(cmd…),
  ⏺ Update(path) — ellipsizing at the terminal edge instead of wrapping.
- Result gutters no longer pad rows with trailing spaces to the terminal
  edge (copy-clean, ragged-right like the reference).
- Todo list matches the reference: coral ■ box with bold default-color
  (white) title for in-progress rows, green ✓ with struck muted titles
  for done rows; coral stays on the top activity line.
- Diff palette set to the specified values: row tints #052e05/#3a0808,
  sign/line-number accents #81C784/#E57373, content in the terminal's
  default (white) over the tint. Also fixes a span-layering bug where
  the row restyle buried word-level highlights.
- Dark text hierarchy: primary output #D4D4D4; UI chrome (gutters, line
  numbers, expand hints, toolbar metadata) #6F6F6F.
- Thinking-effort input bars dim their gradient color 30% toward the
  background pole so the frame hints without shouting.
- The 'N background agents' line is gone everywhere — the bottom toolbar
  owns that count; the verb spinner + todos remain. Background subagent
  status rows hang-indent under their label.
- Welcome banner gains a bold /init tip when the repo has no AGENTS.md
  or CLAUDE.md.
The pinned todo list is drawn by two code paths — the live view during a
foreground turn and the prompt-side background status between turns —
and they had drifted apart (◼/◻/✔ vs ■/□/✓ glyphs, hex off-white vs
terminal-default bold titles, bright pending rows, no strikethrough).
That made running tasks appear to change style mid-session.

Both paths now render identically: coral ■ with a bold default-color
(white) title for running rows, muted □ pending, green ✓ with struck
muted titles when done.
… consistency

Three fixes from live-session screenshots:

- Pinned todo rows no longer carry a muted base style: the bold
  running-task title sets no color of its own, so the muted base bled
  through and rendered it grey instead of the terminal-default white.
  The base style is gone; prefix/icon/title each carry their own style.
- The background working status line ('Ebbing…') now shows the same
  '(elapsed, ↓ Nk tokens, N t/s)' metadata as the live working
  indicator: elapsed since background work appeared and a 1.5s
  sliding-window token rate over the status snapshot, dropped first on
  narrow terminals and reset when background work drains.
- Word-level diff highlights are gated on line similarity (ratio >= 0.5):
  heavy single-line rewrites previously flooded the whole row with the
  brighter highlight tint, reading as a different palette from plain
  added/removed rows. Such rewrites now render as plain rows.
Completes the c8c0f05 commit: the prompt.py change was clobbered from
the index by a concurrent session's git operation before that commit
landed, leaving the renderer half-committed.

The background working status line ('Ebbing…') now carries the same
'(elapsed, ↓ Nk tokens, N t/s)' metadata as the live working indicator:
elapsed since background work first appeared and a 1.5s sliding-window
token rate over the status snapshot's context tokens. The metadata is
dropped first on narrow terminals and both trackers reset when
background work drains.
Notification, progress-note, question-answered, and suggestion rows in
the transcript still rendered with BulletColumns' default list bullet.
They now carry the record marker in the block's accent color (severity
for notifications, green for progress/answers, accent for suggestions).
Genuine list rows — /help sections and nested subagent detail lines —
deliberately keep the list dot.
Refactor all default agent YAML prompts with explicit Mission / Hard
Constraints / Workflow / Output Contract sections for consistency and
clarity. Update system.md overlay accordingly.

Harden background manager, subagent runner, and agent tool to align
with the Codex TUI adoption (Phase 1) gap map: stale-record
reconciliation, resume contract enforcement, and interactive-visualizer
fixes. Update all affected unit and e2e tests.
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

One cohesive PR: agent prompts rewritten; terminal/theme auto-detection and TUI renderer overhauled (markdown, diff, shimmer, markers, truncation); file/host/SSRF safety and background-agent resume reconciliation added; wire/session replay and compaction hardened; extensive tests updated.

Changes

Prompt and policy updates

Layer / File(s) Summary
Default agent contract restructuring
src/pythinker_code/agents/default/*.yaml, src/pythinker_code/agents/default/system.md
Default agent prompts rewritten into Mission/Hard Constraints/Workflow/Output Contract/Escalation sections; system.md adds Non‑Negotiables and Definition of Done checks.
Prompt snapshots and docs
tasks/todo.md, tests/core/test_agent_spec.py
Changelog and task notes updated; agent prompt snapshots refreshed in tests.

TUI, theme, and renderer

Layer / File(s) Summary
Terminal theme & color utilities
src/pythinker_code/ui/color_utils.py, src/pythinker_code/ui/terminal_background.py, src/pythinker_code/ui/terminal_capabilities.py, src/pythinker_code/ui/theme.py
Color math helpers, OSC11 background probe, color-depth detection, and theme="auto" resolution added.
Markdown / code-block / diff rendering
src/pythinker_code/ui/shell/components/markdown.py, src/pythinker_code/ui/shell/components/diff.py, src/pythinker_code/ui/shell/tool_renderers/_file_diff.py
Fenced-table unwrapping, code-highlighting size guard, word-level diff highlighting via theme tokens, and expanded-diff head/tail capping added.
Prompt, glyphs, motion & render utilities
src/pythinker_code/ui/shell/prompt.py, src/pythinker_code/ui/shell/glyphs.py, src/pythinker_code/ui/shell/motion.py, src/pythinker_code/ui/shell/components/render_utils.py
Effort label moved to right-flush top border; platform-aware transcript glyphs; shimmer reworked (smooth truecolor blending); trailing-space trimming wrapper added.

Background agents & orchestration

Layer / File(s) Summary
Background reconcile & resume
src/pythinker_code/background/manager.py, src/pythinker_code/subagents/runner.py, src/pythinker_code/tools/agent/__init__.py, tests/background/*, tests/tools/*
APIs to find agent task views and reconcile stale records; standardized busy-resume messages; resume reconciles stale background tasks before rejecting; tests validate settle/reject paths.

Host, file tools, and security

Layer / File(s) Summary
Host.realpath and HostPath changes
packages/pythinker-host/*
Added async realpath APIs for Local/SSH hosts and HostPath method.
File tools path validation
src/pythinker_code/tools/file/{read,write,replace}.py, src/pythinker_code/tools/file/grep_local.py
All validate symlink-resolved real paths for workspace containment; Grep/SmartSearch bound to runtime and always include internal line numbers with conditional strip for display.
SSRF & fetch hardening
src/pythinker_code/tools/web/fetch.py, src/pythinker_code/utils/aiohttp.py
Fail-closed global-routable IP check added and wired into aiohttp connector (_SSRFConnector) with tests.
Import/export & restore safety
src/pythinker_code/utils/export.py, src/pythinker_code/ui/shell/export_import.py, src/pythinker_code/file_restore.py, src/pythinker_code/vis/api/sessions.py
perform_import(force=False) now blocks sensitive files unless --force; restore IDs validated against pattern and workspace; zip member extraction tightened.

Runtime, web, wire, and locks

Layer / File(s) Summary
Wire/session & runner
src/pythinker_code/wire/file.py, src/pythinker_code/wire/server.py, src/pythinker_code/web/runner/process.py, src/pythinker_code/web/api/sessions.py
WireFile append serialized with asyncio.Lock; read loop sends PARSE_ERROR for oversized lines; add_websocket returns watermark to gate replay; runner/session remove_session added.
Soul compaction & approval retention
src/pythinker_code/soul/pythinkersoul.py, src/pythinker_code/soul/compaction_restore.py, src/pythinker_code/approval_runtime/runtime.py, src/pythinker_code/project_memory.py
Compaction restore guarded and pre-compaction history restored on failure; approval runtime caps terminal-record retention and evicts oldest entries; ProjectMemoryStore uses an asyncio.Lock to prevent same-loop deadlock.

Sanitization & telemetry

Layer / File(s) Summary
Sanitization & untrusted data
src/pythinker_code/ui/shell/tool_renderers/_render_utils.py, src/pythinker_code/ui/shell/visualize/_worklog.py, src/pythinker_code/tools/background/__init__.py, src/pythinker_code/utils/trust.py
Sanitize ANSI in titles/headers; TaskOutput formatting adjusted; expanded invisible char stripping.
Telemetry / Sentry
src/pythinker_code/telemetry/errors.py, src/pythinker_code/telemetry/sentry.py
Redact absolute paths and collapse $HOME to <home> in messages; disable local-var and source-context capture in Sentry init.

Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels
enhancement

✨ 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 feat/tui-enhancements

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
tests/core/test_agent_spec.py (1)

117-182: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Replace full prompt-body snapshots with invariant checks.

These blocks assert long prompt text verbatim, which makes the tests brittle to non-functional wording edits. Keep only contract-level invariants (required sections, key constraints, and tool policy assertions), and move away from full-body snapshots.

As per coding guidelines, “tests/**/*.py: Avoid brittle prompt tests that assert large prompt snapshots; prefer behavior, required sections, and exact small invariants.”

Also applies to: 254-299, 370-425, 496-515

🤖 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 `@tests/core/test_agent_spec.py` around lines 117 - 182, The test currently
asserts the entire prompt body verbatim (see
subagent_specs["coder"].system_prompt_args snapshot) which is brittle; update
the tests to instead assert a small set of contract-level invariants: check that
required section headers (e.g., "Mission", "Hard Constraints", "Workflow",
"Output Contract") appear, that key constraints like "Stay tightly scoped" and
"Never leave placeholders" are present, and that the artifact/structured-output
requirement (the <coding_artifact> JSON fields) and tool/policy directives are
enforced; replace the full-string snapshot assertions at the referenced blocks
(around subagent_specs["coder"].system_prompt_args and the other noted ranges)
with assertions using .contains() or regex matches for those specific phrases
and that the snapshot only stores a minimal list of expected invariants instead
of the whole prompt body.

Source: Coding guidelines

src/pythinker_code/ui/shell/visualize/_live_view.py (1)

868-895: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset token-rate samples on turn boundaries.

_turn_token_rate() at Line 654 accumulates a sliding window, but top-level turn transitions (Lines 868-895) never clear self._turn_token_samples. A new turn started quickly after the previous one can show stale t/s from the prior turn.

Suggested fix
@@
             case TurnBegin(user_input=user_input):
                 if self._active_turn_depth == 0:
                     self._turn_start_time = time.monotonic()
+                    self._turn_token_samples.clear()
@@
             case TurnEnd():
                 self._active_turn_depth = max(0, self._active_turn_depth - 1)
                 if self._active_turn_depth == 0:
                     self._turn_start_time = None
+                    self._turn_token_samples.clear()
                     self._pending_turn_recap = True
@@
         if is_interrupt:
             self._active_turn_depth = 0
             self._turn_start_time = None
+            self._turn_token_samples.clear()

Also applies to: 654-679, 217-220

🤖 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/ui/shell/visualize/_live_view.py` around lines 868 - 895,
The issue: _turn_token_rate() accumulates sliding-window samples in
self._turn_token_samples but top-level turn transitions (handled in the
TurnBegin and TurnEnd branches and in SteerInput/cleanup flows) never clear
them, causing stale token/sec values to carry into new turns; fix by
clearing/resetting self._turn_token_samples (and any related sampling state used
by _turn_token_rate) whenever a top-level turn starts or fully
ends—specifically, add code to clear self._turn_token_samples in the TurnBegin
branch when self._active_turn_depth == 0 (where _turn_start_time,
_recap_user_input, etc. are initialized) and in the TurnEnd branch when
self._active_turn_depth becomes 0 (where _turn_start_time is set to None and
_pending_turn_recap is set), and likewise ensure SteerInput/cleanup paths that
begin an interrupting flow also reset self._turn_token_samples so samples never
carry across independent turns.
🤖 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 891-893: The AGENTS.md/CLAUDE.md existence check currently uses
Path.cwd() (work_dir variable) but must use the active session directory; change
the detection to check self._runtime.session.work_dir (or the existing session
work_dir variable) instead of Path.cwd(), e.g., compute session_dir =
self._runtime.session.work_dir and test (session_dir / "AGENTS.md").exists() ||
(session_dir / "CLAUDE.md").exists(); preserve the OSError handling and ensure
any subsequent logic that references work_dir uses the session_dir value so the
/init tip reflects the correct session path.

In `@src/pythinker_code/ui/color_utils.py`:
- Line 42: The docstring in src/pythinker_code/ui/color_utils.py contains an en
dash in "0–255" which triggers Ruff RUF002; edit the module/function docstring
(the line reading 'ITU-R BT.601 perceived brightness in the 0–255 range.') and
replace the en dash (–) with an ASCII hyphen (-) so it reads "0-255".

In `@src/pythinker_code/ui/shell/slash.py`:
- Around line 1295-1297: The warning string built in the f-string that uses
_t_set.warning is ambiguous; update the copy to reference the correct CLI flag
name so it matches other messages (use --config-file rather than --config), e.g.
change the message that currently reads "restart without --config text" to
"restart without --config-file text" (locate the f-string around _t_set.warning
in src/pythinker_code/ui/shell/slash.py and replace the token accordingly).

In `@src/pythinker_code/ui/terminal_background.py`:
- Around line 72-112: The _probe_uncached probe currently swallows all
exceptions and returns None; change its exception handlers to capture the
exception (e.g., except (ValueError, OSError) as exc) and emit a debug-level log
before returning so failures are visible while behavior remains the same.
Specifically, in _probe_uncached add logging in the initial stdin/stdout TTY
check except block, the termios.tcgetattr except block, and the outer probe
except (OSError, ValueError) block; include contextual info such as the fd (if
available), timeout/_OSC11_QUERY, and the exception instance, using the module
logger (logging.getLogger(__name__)) or the existing logger, then continue
returning None. Ensure the finally block still suppresses restore errors but
consider logging suppressed termios.tcsetattr failures at debug level as well;
do not change return values or control flow and keep parsing delegated to
parse_osc11_response.

In `@tests/ui_and_conv/test_output_guards.py`:
- Around line 21-24: Annotate the autouse fixture _isolated_registry with an
explicit generator/iterator return type to satisfy ANN202; change its signature
to something like def _isolated_registry() -> typing.Generator[None, None, None]
(or typing.Iterator[None]) and add the corresponding import from typing at the
top if missing, keeping the body that calls clear_tool_renderers() unchanged.

In `@tests/ui_and_conv/test_streaming_content_block.py`:
- Line 265: The current assertion only checks for any parentheses in output;
strengthen it by asserting the compact-metadata pattern is present: use a regex
search on the test's output variable to ensure there is a parenthesized metadata
block containing the "activity" key (e.g., match a pattern like
"(...activity...)" or a parenthesis-enclosed comma-separated key:value list)
rather than just any "(" or ")". Update the assertion in
tests/ui_and_conv/test_streaming_content_block.py that references output to use
re.search with an appropriate pattern (and add an import for re if missing) so
the test fails unless properly formatted compact-metadata is present.

---

Outside diff comments:
In `@src/pythinker_code/ui/shell/visualize/_live_view.py`:
- Around line 868-895: The issue: _turn_token_rate() accumulates sliding-window
samples in self._turn_token_samples but top-level turn transitions (handled in
the TurnBegin and TurnEnd branches and in SteerInput/cleanup flows) never clear
them, causing stale token/sec values to carry into new turns; fix by
clearing/resetting self._turn_token_samples (and any related sampling state used
by _turn_token_rate) whenever a top-level turn starts or fully
ends—specifically, add code to clear self._turn_token_samples in the TurnBegin
branch when self._active_turn_depth == 0 (where _turn_start_time,
_recap_user_input, etc. are initialized) and in the TurnEnd branch when
self._active_turn_depth becomes 0 (where _turn_start_time is set to None and
_pending_turn_recap is set), and likewise ensure SteerInput/cleanup paths that
begin an interrupting flow also reset self._turn_token_samples so samples never
carry across independent turns.

In `@tests/core/test_agent_spec.py`:
- Around line 117-182: The test currently asserts the entire prompt body
verbatim (see subagent_specs["coder"].system_prompt_args snapshot) which is
brittle; update the tests to instead assert a small set of contract-level
invariants: check that required section headers (e.g., "Mission", "Hard
Constraints", "Workflow", "Output Contract") appear, that key constraints like
"Stay tightly scoped" and "Never leave placeholders" are present, and that the
artifact/structured-output requirement (the <coding_artifact> JSON fields) and
tool/policy directives are enforced; replace the full-string snapshot assertions
at the referenced blocks (around subagent_specs["coder"].system_prompt_args and
the other noted ranges) with assertions using .contains() or regex matches for
those specific phrases and that the snapshot only stores a minimal list of
expected invariants instead of the whole prompt body.
🪄 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: 66f4c405-8357-4477-b33b-4c7164afc523

📥 Commits

Reviewing files that changed from the base of the PR and between 00e7b67 and edf9080.

📒 Files selected for processing (73)
  • CHANGELOG.md
  • src/pythinker_code/agents/default/ask.yaml
  • src/pythinker_code/agents/default/code_reviewer.yaml
  • src/pythinker_code/agents/default/coder.yaml
  • src/pythinker_code/agents/default/debug.yaml
  • src/pythinker_code/agents/default/debugger.yaml
  • src/pythinker_code/agents/default/explore.yaml
  • src/pythinker_code/agents/default/implementer.yaml
  • src/pythinker_code/agents/default/judge.yaml
  • src/pythinker_code/agents/default/plan.yaml
  • src/pythinker_code/agents/default/planner.yaml
  • src/pythinker_code/agents/default/review.yaml
  • src/pythinker_code/agents/default/scout.yaml
  • src/pythinker_code/agents/default/security_reviewer.yaml
  • src/pythinker_code/agents/default/system.md
  • src/pythinker_code/agents/default/verifier.yaml
  • src/pythinker_code/app.py
  • src/pythinker_code/background/manager.py
  • src/pythinker_code/config.py
  • src/pythinker_code/subagents/runner.py
  • src/pythinker_code/tools/agent/__init__.py
  • src/pythinker_code/ui/color_utils.py
  • src/pythinker_code/ui/shell/__init__.py
  • src/pythinker_code/ui/shell/components/diff.py
  • src/pythinker_code/ui/shell/components/markdown.py
  • src/pythinker_code/ui/shell/components/render_utils.py
  • src/pythinker_code/ui/shell/components/tool_execution.py
  • src/pythinker_code/ui/shell/glyphs.py
  • src/pythinker_code/ui/shell/motion.py
  • src/pythinker_code/ui/shell/prompt.py
  • src/pythinker_code/ui/shell/render_constants.py
  • src/pythinker_code/ui/shell/selectors/settings.py
  • src/pythinker_code/ui/shell/slash.py
  • src/pythinker_code/ui/shell/tool_renderers/_file_diff.py
  • src/pythinker_code/ui/shell/tool_renderers/_render_utils.py
  • src/pythinker_code/ui/shell/tool_renderers/agent.py
  • src/pythinker_code/ui/shell/visualize/_blocks.py
  • src/pythinker_code/ui/shell/visualize/_interactive.py
  • src/pythinker_code/ui/shell/visualize/_live_view.py
  • src/pythinker_code/ui/terminal_background.py
  • src/pythinker_code/ui/terminal_capabilities.py
  • src/pythinker_code/ui/theme.py
  • src/pythinker_code/utils/rich/columns.py
  • tasks/todo.md
  • tests/background/test_manager.py
  • tests/core/test_agent_spec.py
  • tests/core/test_config.py
  • tests/core/test_default_agent.py
  • tests/e2e/test_shell_modal_e2e.py
  • tests/e2e/test_shell_pty_e2e.py
  • tests/tools/test_agent_tool.py
  • tests/ui/test_shell_markdown.py
  • tests/ui_and_conv/test_color_utils.py
  • tests/ui_and_conv/test_empty_think_part_indicator.py
  • tests/ui_and_conv/test_live_view_notifications.py
  • tests/ui_and_conv/test_live_view_todos.py
  • tests/ui_and_conv/test_markdown_guards.py
  • tests/ui_and_conv/test_modal_lifecycle.py
  • tests/ui_and_conv/test_output_guards.py
  • tests/ui_and_conv/test_prompt_tips.py
  • tests/ui_and_conv/test_settings_recaps_slash.py
  • tests/ui_and_conv/test_shell_motion.py
  • tests/ui_and_conv/test_shell_motion_shimmer.py
  • tests/ui_and_conv/test_streaming_content_block.py
  • tests/ui_and_conv/test_terminal_background.py
  • tests/ui_and_conv/test_terminal_capabilities.py
  • tests/ui_and_conv/test_theme.py
  • tests/ui_and_conv/test_thinking_cycle.py
  • tests/ui_and_conv/test_tui_blocks_integration.py
  • tests/ui_and_conv/test_tui_card_tool_renderers.py
  • tests/ui_and_conv/test_tui_components.py
  • tests/ui_and_conv/test_tui_theme_tokens.py
  • tests/ui_and_conv/test_visualize_running_prompt.py

Comment thread src/pythinker_code/app.py Outdated
Comment thread src/pythinker_code/ui/color_utils.py Outdated
Comment thread src/pythinker_code/ui/shell/slash.py
Comment thread src/pythinker_code/ui/terminal_background.py
Comment thread tests/ui_and_conv/test_output_guards.py Outdated
Comment thread tests/ui_and_conv/test_streaming_content_block.py Outdated
elkaix added 3 commits June 9, 2026 23:40
Implements the remediation plan for the validated audit findings
(1 Critical, 15 High, 29 Medium, 20 Low) across five phases, each fix
behind a TDD test and gated by `make check` plus a security review.

Phase 0 — shell permission classifier: close the read-only / auto-mode
  bypass cluster (interior &/|& separators, casefolded base commands,
  wrapper value-options, find/xargs/awk payloads, glued output
  redirection, unsafe `git -c`, uv sub-namespaces and `uv run`
  option-prefix bypass).
Phase 1 — confinement, egress, telemetry: symlink-resolve file
  read/write/edit and grep before workspace/sensitive checks; fail-closed
  SSRF with a connection-pinned resolver; bounded shell wait; Sentry
  path/home redaction; invisible-char and case-folded sensitive-file
  handling; sensitive-import gate; untrusted-output wrapping; subagent-id
  path validation.
Phase 2 — tool dispatch, lifecycle, context integrity: MCP-vs-builtin
  tool collisions; run_soul task-leak cleanup; restore-id/path traversal
  guards; compaction rollback; mid-tool-cancel turn balance; cyclic-extend
  detection.
Phase 3 — wire server, auth, web-server: wire read-loop hardening; OAuth
  refresh / device-id / 403 handling; provider base_url validation; replay
  watermark; session-leak cleanup; ZIP-import validation.
Phase 4 — UI/usage/CLI: ANSI sanitization at render boundaries (incl.
  generic tool arg-key names); usage-meter consumed-vs-remaining;
  reset-window loop guard; RunAgents approval fingerprint over child
  prompts; owner-only MCP config; live Typer help; /restore traversal and
  error handling; bounded approval-request store.

Review-found gaps were fixed with regression tests: uv-run option bypass,
grep symlink escape, SSH-key import gate, ANSI arg-key injection, and the
MCP-config / share-dir permission race. Also hardens auth JSON parsing.
- app.py: detect AGENTS.md/CLAUDE.md in the session work_dir (awaiting the
  async HostPath.exists) instead of process cwd, so the /init tip is correct
  when cwd differs from the session path.
- color_utils.py: replace EN DASH with ASCII hyphen in the luma docstring.
- slash.py: clarify the recaps config-flag guidance (--config vs --config-file).
- terminal_background.py: debug-log swallowed OSC11 probe failures instead of
  silently degrading, per the exception-handling rule.
- tests: annotate the autouse fixture return type; tighten the thinking-status
  metadata assertion to validate the token-count block, not just any paren.
Qwen3.x/3.7 are hybrid thinking models: reasoning is toggleable per request
via the standard Anthropic thinking block, which the OpenCode Go
@ai-sdk/anthropic route (and Alibaba Model Studio's Anthropic-compatible
endpoint) accepts as {"type": "enabled", "budget_tokens": N} /
{"type": "disabled"}.

Previously OpenCode Go Qwen models got no thinking capability, so their
effort was uncontrollable — while the Alibaba plan already exposed it. Give
the Qwen family the controllable 'thinking' capability so create_llm routes
the selected effort through with_thinking, which (for these non-Claude
models) emits the budget-based payload and clamps xhigh/max -> high,
minimal -> low. GLM/MiniMax remain always_thinking (effort can't be turned
off); Qwen can.

Adds a clamp regression case pinning the budget-safe mapping for qwen3.7-max
so the budgets[...] lookup can never KeyError.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🤖 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/cli/vis.py`:
- Around line 15-19: The parameter named ctx in the CLI command signature (the
typer.Context parameter in vis.py) is unused and triggers Ruff ARG001; rename it
to _ctx (or remove the parameter entirely if not required) in the function
signature where ctx: typer.Context is declared so the linter recognizes it as
intentionally unused; ensure any internal references (if any) are updated or
removed accordingly and keep the Annotated option for host unchanged.

In `@src/pythinker_code/file_restore.py`:
- Around line 83-97: The code currently decodes point.content_b64 with
base64.b64decode(point.content_b64 or "") which can silently write empty or
malformed data; update the restore flow in the block that follows
FileRestorePoint parsing (around restore_path, point, and before
point.path.write_bytes) to treat missing/empty or invalid base64 as a corrupt
restore point: first check that point.content_b64 is a non-empty string and if
not raise FileNotFoundError(f"Corrupt restore point: {restore_id}"), then
attempt to base64.b64decode(point.content_b64) inside a try/except that catches
base64/binascii decode errors (e.g., binascii.Error or ValueError) and re-raises
FileNotFoundError from the caught exception; only call
point.path.write_bytes(decoded_bytes) after a successful decode.

In `@src/pythinker_code/scratchpad.py`:
- Around line 438-439: The code currently deletes the lock file by calling
lock_file.unlink() inside a contextlib.suppress(OSError) after releasing the
file lock; remove that unlink so the lock file remains on disk and only the file
lock is released (e.g., via closing or releasing the flock handle). Update the
release logic in the function/method that manipulates the lock (references to
lock_file and contextlib.suppress) to stop unlinking and ensure the lock is
released correctly without deleting the *.scratchpad.lock inode.

In `@src/pythinker_code/soul/__init__.py`:
- Around line 240-244: The cleanup loop currently cancels then unconditionally
awaits each task (soul_task, cancel_event_task, notification_task), which can
re-raise a previously-failed exception and abort remaining shutdown steps
(wire.shutdown(), wire.join(), _current_wire.reset). Fix by skipping awaiting a
task that has already completed with an exception: after task.cancel(), check
task.done() and task.exception(), and only await the task (inside
contextlib.suppress(asyncio.CancelledError)) if it is not already done with a
non-None exception; this prevents re-raising past failures while still awaiting
normally-cancelled or successful tasks so shutdown (wire.shutdown(),
wire.join(), _current_wire.reset) always runs.

In `@src/pythinker_code/soul/permission.py`:
- Around line 214-217: The current sudo-unwrapping treats only short forms in
_SUDO_VALUE_OPTS as value-taking, allowing long options (e.g. --user) and
--opt=value to be misinterpreted as commands; update _SUDO_VALUE_OPTS to include
the long-form option names you support and change the unwrapping logic (the code
around the sudo unwrap block that iterates args at ~lines 551-567) to: treat any
token starting with "--" that exactly matches a long option in _SUDO_VALUE_OPTS
as consuming the next token, and treat tokens of the form "--opt=value" as also
consuming their value (i.e., do not treat the portion after '=' as a command);
also ensure the same pattern is applied consistently to the time-related logic
using _TIME_VALUE_OPTS.
- Around line 705-723: The function _uv_run_payload currently assumes the first
non-option token is "run" and misses cases with global options (e.g., "uv
--directory repo run ..."); change the logic to first scan args from the start
to skip any global options (respecting "--" and value-taking global options
using the same _UV_RUN_VALUE_OPTS set) until you encounter the literal "run"
token, return None if none found, then set rest = args[index_of_run + 1 :] and
continue the existing run-specific option skipping loop (using
_UV_RUN_VALUE_OPTS for run options) to extract and return the wrapped payload
tokens.

In `@src/pythinker_code/soul/pythinkersoul.py`:
- Around line 1668-1683: When cancellation happens in the try around
result.tool_results(), we must not overwrite already-streamed tool outputs;
change the except asyncio.CancelledError branch to collect the set of completed
results (the ToolResult instances that were returned before cancellation or that
were already emitted via on_tool_result) by inspecting whatever
partial/available results you can get from result (e.g., the portion of
result.tool_results() that succeeded or a record of IDs emitted by
on_tool_result), then synthesize ToolResult(tool_call_id=...,
return_value=ToolRuntimeError(...)) only for tool_call IDs present in
result.tool_calls but missing from the completed set, combine completed results
+ synthesized interrupted results and pass that combined list into
self._grow_context(result, interrupted_or_combined) via asyncio.shield, and
finally re-raise the CancelledError; reference symbols: result.tool_results(),
on_tool_result, ToolResult, ToolRuntimeError, self._grow_context,
result.tool_calls.

In `@src/pythinker_code/soul/slash.py`:
- Around line 325-327: The current parsing uses args.split() which breaks
quoted/escaped paths and mangles targets; replace that with a proper shell-style
parser (e.g., use shlex.split(args) or an argparse.ArgumentParser) to correctly
handle quoting/escaping and to parse the --force flag and the target path;
update the code in slash.py (replace args.split() usage where tokens =
args.split(), force = "--force" in tokens, target = sanitize_cli_path(...)) to
parse with shlex.split(args) or an ArgumentParser and derive force (bool) and
target (the remaining positional argument, sanitized with sanitize_cli_path),
and remove/replace the duplicated parsing logic in
src/pythinker_code/ui/shell/export_import.py by reusing the same parsing
approach or centralizing it into a shared helper (e.g., a parse_import_args
function) so both modules behave identically.

In `@src/pythinker_code/tools/web/fetch.py`:
- Around line 28-33: The current _ip_is_blocked function returns False on
ipaddress.ip_address(address) ValueError which allows unknown/invalid addresses
to proceed; update the except ValueError branch in _ip_is_blocked to return True
(block) instead, so any parse error results in a blocked outcome; locate the
function _ip_is_blocked and change the exception handler for
ipaddress.ip_address to return True while keeping the final return of (not
ip.is_global) or ip.is_multicast.

In `@src/pythinker_code/web/runner/process.py`:
- Around line 570-592: The async function add_websocket_and_begin_replay
performs a blocking filesystem stat via wire_file.stat().st_size; replace that
call with an async stat (e.g., anyio.Path or aiofiles) so the event loop isn't
blocked: convert wire_file to an async path (for example anyio.Path(wire_file)),
await its .stat() and read .st_size, keep the same OSError handling, and ensure
you import the chosen async filesystem API; update the watermark assignment
inside add_websocket_and_begin_replay accordingly so it awaits the async stat
rather than calling the blocking Path.stat().
- Around line 676-683: The current broad except Exception in the error recovery
block (inside the handler that checks isinstance(in_message,
JSONRPCPromptMessage) and manipulates self._in_flight_prompt_ids, was_busy, and
await self._emit_status) should be narrowed to only the intended error classes
to avoid masking bugs; change the handler to catch a tuple of expected errors
(e.g., ValueError, OSError, and json.decoder.JSONDecodeError) or split into
separate except blocks for validation errors versus I/O errors, keeping the same
cleanup logic (discarding in_message.id, checking was_busy and calling
_emit_status("idle", reason="prompt_error")) and still logging the exception
class and message via logger.error.

In `@tests/core/test_project_memory.py`:
- Around line 254-256: The local async helper slow_write is missing a return
type annotation; update its signature to include an explicit return type (e.g.,
async def slow_write(self, target, entries) -> None:) so the coroutine's return
type is declared, keeping the body calling await orig_write(self, target,
entries) unchanged and referencing the existing orig_write symbol.

In `@tests/e2e/test_cli_error_output.py`:
- Line 207: The subprocess.run call in tests/e2e/test_cli_error_output.py should
explicitly pass check=False to satisfy the linter and clarify intent; update the
return subprocess.run(...) invocation used in the test to include check=False
(since the test inspects result.returncode rather than raising exceptions) so
Ruff no longer warns about a missing check argument.

In `@tests/ui_and_conv/test_export_import.py`:
- Around line 1402-1411: Add assertions that pin the token-count side effect
when testing perform_import: in the non-force case (where you currently assert
isinstance(result, str) and ctx.append_message.assert_not_awaited()), also
assert ctx.update_token_count.assert_not_awaited() to ensure token counts aren’t
changed; in the force=True case (where you assert isinstance(result2, tuple) and
ctx.append_message.assert_awaited_once()), also assert
ctx.update_token_count.assert_awaited_once(). Apply the same pair of assertions
to the other identical test block mentioned (the second import test around the
later assertions) so both refused and forced import paths verify
update_token_count usage alongside append_message.
🪄 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: ee8e8376-0578-4f25-9dac-2ab27712d3ea

📥 Commits

Reviewing files that changed from the base of the PR and between edf9080 and 8ddebac.

📒 Files selected for processing (127)
  • packages/pythinker-host/src/pythinker_host/__init__.py
  • packages/pythinker-host/src/pythinker_host/local.py
  • packages/pythinker-host/src/pythinker_host/path.py
  • packages/pythinker-host/src/pythinker_host/ssh.py
  • packages/pythinker-review/tests/unit/test_security_intel.py
  • src/pythinker_code/__main__.py
  • src/pythinker_code/acp/host.py
  • src/pythinker_code/agentspec.py
  • src/pythinker_code/app.py
  • src/pythinker_code/approval_runtime/runtime.py
  • src/pythinker_code/auth/lm_studio.py
  • src/pythinker_code/auth/oauth.py
  • src/pythinker_code/auth/ollama.py
  • src/pythinker_code/auth/openai.py
  • src/pythinker_code/cli/export.py
  • src/pythinker_code/cli/mcp.py
  • src/pythinker_code/cli/vis.py
  • src/pythinker_code/cli/web.py
  • src/pythinker_code/file_restore.py
  • src/pythinker_code/hooks/engine.py
  • src/pythinker_code/memory/recall.py
  • src/pythinker_code/plugin/manager.py
  • src/pythinker_code/project_memory.py
  • src/pythinker_code/scratchpad.py
  • src/pythinker_code/session_fork.py
  • src/pythinker_code/share.py
  • src/pythinker_code/soul/__init__.py
  • src/pythinker_code/soul/compaction_restore.py
  • src/pythinker_code/soul/permission.py
  • src/pythinker_code/soul/pythinkersoul.py
  • src/pythinker_code/soul/slash.py
  • src/pythinker_code/soul/toolset.py
  • src/pythinker_code/subagents/discovery.py
  • src/pythinker_code/subagents/store.py
  • src/pythinker_code/telemetry/errors.py
  • src/pythinker_code/telemetry/sentry.py
  • src/pythinker_code/tools/agent/__init__.py
  • src/pythinker_code/tools/background/__init__.py
  • src/pythinker_code/tools/file/grep_local.py
  • src/pythinker_code/tools/file/read.py
  • src/pythinker_code/tools/file/replace.py
  • src/pythinker_code/tools/file/write.py
  • src/pythinker_code/tools/shell/__init__.py
  • src/pythinker_code/tools/web/fetch.py
  • src/pythinker_code/ui/color_utils.py
  • src/pythinker_code/ui/shell/components/bash_execution.py
  • src/pythinker_code/ui/shell/export_import.py
  • src/pythinker_code/ui/shell/slash.py
  • src/pythinker_code/ui/shell/tool_renderers/_render_utils.py
  • src/pythinker_code/ui/shell/usage.py
  • src/pythinker_code/ui/shell/usage_adapters/alibaba.py
  • src/pythinker_code/ui/shell/usage_adapters/minimax.py
  • src/pythinker_code/ui/shell/usage_adapters/openai_chatgpt.py
  • src/pythinker_code/ui/shell/visualize/_blocks.py
  • src/pythinker_code/ui/shell/visualize/_worklog.py
  • src/pythinker_code/ui/terminal_background.py
  • src/pythinker_code/utils/aiohttp.py
  • src/pythinker_code/utils/export.py
  • src/pythinker_code/utils/path.py
  • src/pythinker_code/utils/sensitive.py
  • src/pythinker_code/utils/trust.py
  • src/pythinker_code/vis/api/sessions.py
  • src/pythinker_code/web/api/config.py
  • src/pythinker_code/web/api/sessions.py
  • src/pythinker_code/web/runner/process.py
  • src/pythinker_code/wire/file.py
  • src/pythinker_code/wire/server.py
  • tests/auth/test_lm_studio_auth.py
  • tests/auth/test_oauth_device_id.py
  • tests/auth/test_oauth_refresh.py
  • tests/auth/test_ollama_auth.py
  • tests/auth/test_openai_auth.py
  • tests/conftest.py
  • tests/core/test_agent_spec.py
  • tests/core/test_approval_auto.py
  • tests/core/test_approval_runtime.py
  • tests/core/test_cli_reload.py
  • tests/core/test_compaction_restore.py
  • tests/core/test_default_agent.py
  • tests/core/test_export_cli.py
  • tests/core/test_file_restore_points.py
  • tests/core/test_mcp_docker_rm.py
  • tests/core/test_notifications.py
  • tests/core/test_permission_profiles.py
  • tests/core/test_project_memory.py
  • tests/core/test_pythinkersoul_turn_balance.py
  • tests/core/test_recall_provider.py
  • tests/core/test_scratchpad.py
  • tests/core/test_session_fork.py
  • tests/core/test_soul_import_command.py
  • tests/core/test_startup_imports.py
  • tests/core/test_subagent_discovery.py
  • tests/core/test_subagent_store.py
  • tests/core/test_toolset.py
  • tests/core/test_wire_file_compat.py
  • tests/core/test_wire_plan_mode.py
  • tests/core/test_wire_server_steer.py
  • tests/e2e/test_cli_error_output.py
  • tests/hooks/test_engine.py
  • tests/telemetry/test_sentry_filters.py
  • tests/tools/test_agent_tool.py
  • tests/tools/test_background_tools.py
  • tests/tools/test_fetch_url.py
  • tests/tools/test_grep.py
  • tests/tools/test_read_file.py
  • tests/tools/test_shell_bash.py
  • tests/tools/test_smart_search.py
  • tests/tools/test_str_replace_file.py
  • tests/tools/test_untrusted_wrapping.py
  • tests/tools/test_write_file.py
  • tests/ui/usage_adapters/test_alibaba_adapter.py
  • tests/ui/usage_adapters/test_minimax.py
  • tests/ui/usage_adapters/test_openai_chatgpt.py
  • tests/ui_and_conv/test_export_import.py
  • tests/ui_and_conv/test_output_guards.py
  • tests/ui_and_conv/test_shell_export_import_commands.py
  • tests/ui_and_conv/test_shell_switch_slash.py
  • tests/ui_and_conv/test_streaming_content_block.py
  • tests/ui_and_conv/test_tui_card_tool_renderers.py
  • tests/ui_and_conv/test_tui_components.py
  • tests/ui_and_conv/test_visualize_running_prompt.py
  • tests/ui_and_conv/test_worklog_render.py
  • tests/utils/test_sensitive.py
  • tests/utils/test_trust.py
  • tests/vis/test_app.py
  • tests/web/test_config_api_redaction.py
  • tests/web/test_session_error_recovery.py
💤 Files with no reviewable changes (2)
  • src/pythinker_code/plugin/manager.py
  • src/pythinker_code/main.py

Comment thread src/pythinker_code/cli/vis.py Outdated
Comment thread src/pythinker_code/file_restore.py
Comment thread src/pythinker_code/scratchpad.py Outdated
Comment thread src/pythinker_code/soul/__init__.py Outdated
Comment thread src/pythinker_code/soul/permission.py
Comment thread src/pythinker_code/web/runner/process.py
Comment thread src/pythinker_code/web/runner/process.py
Comment thread tests/core/test_project_memory.py Outdated
Comment thread tests/e2e/test_cli_error_output.py
Comment thread tests/ui_and_conv/test_export_import.py
elkaix added 2 commits June 10, 2026 00:12
Stop recoloring the whole input border by thinking effort. The border is now
one static frame grey at every level; the effort signal moves to a small
label flushed right on the input's top border — a level-colored dot
(off->max: slate->blue->teal->amber->orange->red) plus the muted level word.

The dot carries the cold->hot color at full strength (it's a single glyph),
while the word uses a muted class so it never competes with the typed text.
The label is hidden for native-thinking models (always_thinking with no user
dial) and non-thinking models, and the rule auto-shortens by the measured
label width so the top line never wraps.

_prompt_separator_style no longer borrows thinking_frame_style, so all input
separators (top border and footer) render the same static frame grey.
…king

Two related thinking-effort changes:

1. Remove the duplicated effort indicator from the footer (the
   'agent <model> • <effort>' / 'native reasoning' text under the input).
   Effort now shows in exactly one place — the top-border label added
   earlier. The footer mode line is just 'agent <model>', degrading to the
   bare mode on narrow terminals. Drops the now-orphaned
   _thinking_footer_label helper.

2. Mark all Qwen models (qwen3.7-max/plus, qwen3.6-plus/flash, qwen3 coder
   plus/flash on Alibaba; qwen* on OpenCode Go) as always_thinking instead
   of the controllable 'thinking' capability. Qwen now matches GLM/MiniMax:
   reasoning is native and always on, with no user effort dial and no
   top-border effort label. Reasoning still flows over the Anthropic
   thinking block both Anthropic-compatible routes accept.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/ui/shell/prompt.py`:
- Around line 2478-2484: The top-border assembly currently always appends the
label which can cause wrapping; change the return logic to check if label_width
+ gap <= len(rule) (using the existing gap, label_width, rule_width, label,
border_style, and rule variables) and only include the label when it fits;
otherwise return just the plain rule segment (border_style with "─" * max(0,
len(rule) - gap - label_width) or simply border_style with "─" * len(rule) as
the fallback) so narrow terminals won’t wrap the label.

In `@tests/ui_and_conv/test_prompt_tips.py`:
- Line 608: The test function
test_card_toolbar_separator_is_static_grey_regardless_of_effort currently types
the monkeypatch fixture as Any; change its parameter annotation to the concrete
pytest.MonkeyPatch type (i.e., monkeypatch: pytest.MonkeyPatch) and ensure
pytest is imported in the test module so the annotation resolves, replacing the
Any usage to satisfy the ANN401 lint rule.
🪄 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: d5547b6c-a99f-43e2-a62f-413e201c7b66

📥 Commits

Reviewing files that changed from the base of the PR and between 8ddebac and 2495020.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • packages/pythinker-core/tests/test_anthropic_thinking.py
  • src/pythinker_code/auth/opencode_go.py
  • src/pythinker_code/ui/shell/prompt.py
  • src/pythinker_code/ui/theme.py
  • tests/auth/test_opencode_go_auth.py
  • tests/ui_and_conv/test_prompt_tips.py

Comment thread src/pythinker_code/ui/shell/prompt.py
Comment thread tests/ui_and_conv/test_prompt_tips.py
Critical permission-classifier bypasses:
- sudo long value-options (`sudo --user alice rm -rf /`) were not consumed, so
  the wrapped destructive payload classified as the option's value. Add the long
  forms to _SUDO_VALUE_OPTS.
- uv global options before `run` (`uv --directory repo run rm -rf /`) hid the
  subcommand; the global flag's value was mistaken for it. Add
  _uv_strip_global_opts and apply it in both mutation and destructive paths.

Major:
- file_restore: treat missing (None) or malformed-base64 content as a corrupt
  restore point instead of silently writing an empty/garbage file.
- web/fetch: _ip_is_blocked now fails closed (blocks) on an unparseable address.
- scratchpad: stop unlinking the advisory lock file (split-inode race); keep it
  persistent and add *.scratchpad.lock to the written .gitignore patterns.
- soul shutdown: don't re-await an already-finished task in the cleanup loop —
  retrieve its exception without re-raising so the rest of shutdown still runs.
- pythinkersoul: on mid-tool interruption, keep the real results of calls that
  already completed (captured via on_tool_result) and only synthesize the
  interruption marker for still-pending calls.

Minor / nitpick:
- /import arg parsing now uses shlex via a shared parse_import_args helper
  (soul/slash + ui/shell/export_import), preserving quoted paths.
- web/runner: offload the blocking wire-file stat with asyncio.to_thread;
  document the intentional broad except at the per-message dispatch boundary.
- cli/vis: rename unused callback param to _ctx.
- tests: regression cases for both bypasses; strengthened import token-count
  assertions; lock-file persistence test; minor annotations.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/pythinker_code/soul/pythinkersoul.py (1)

1681-1695: ⚠️ Potential issue | 🟠 Major

Keep and drain the interrupted _grow_context task before re-raising CancelledError.

In the except asyncio.CancelledError for result.tool_results() (src/pythinker_code/soul/pythirersoul.py ~1681-1696), asyncio.shield(asyncio.create_task(...)) is used without storing the task reference; if cancellation hits again while awaiting the shielded wait, you can end up re-raising without having “drained” the same task like the normal path immediately below does (~1704-1710). Mirror that normal pattern (store grow_context_task and re-shield-await it on CancelledError) so the interrupted context write completes cleanly.

Suggested fix
-                await asyncio.shield(asyncio.create_task(self._grow_context(result, interrupted)))
+                grow_context_task = asyncio.create_task(
+                    self._grow_context(result, interrupted)
+                )
+                try:
+                    await asyncio.shield(grow_context_task)
+                except asyncio.CancelledError:
+                    await asyncio.shield(grow_context_task)
                 raise
🤖 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/pythinkersoul.py` around lines 1681 - 1695, The
current CancelledError handling creates a shielded task for
self._grow_context(...) but doesn't keep a reference, so if another cancellation
occurs while awaiting it the background grow_context write may never be drained;
change the pattern to store the task in a variable (e.g., grow_context_task =
asyncio.create_task(self._grow_context(result, interrupted))) and then await it
via await asyncio.shield(grow_context_task) inside the except
asyncio.CancelledError block (and re-shield-await again before re-raising) so
the interrupted context write started by _grow_context (and the
ToolResult/ToolRuntimeError entries built from result.tool_calls and
completed_tool_results) is always completed/drained before letting
CancelledError propagate.
tests/e2e/test_cli_error_output.py (1)

199-216: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Test helper uses a fixed path in $HOME instead of tmp_path.

_run_pythinker_main hardcodes Path.home() / ".pythinker-test-share" which leaves artifacts in the user's home directory and can cause flaky behavior under parallel test runs. Consider accepting a tmp_path fixture like _run_pythinker does.

Suggested fix
-def _run_pythinker_main(args: list[str]) -> subprocess.CompletedProcess[str]:
+def _run_pythinker_main(args: list[str], *, share_dir: Path) -> subprocess.CompletedProcess[str]:
     """Run via python -m pythinker_code (__main__.py) rather than pythinker_code.cli."""
     env = os.environ.copy()
-    env["PYTHINKER_SHARE_DIR"] = str(Path.home() / ".pythinker-test-share")
+    env["PYTHINKER_SHARE_DIR"] = str(share_dir)
     env["NO_COLOR"] = "1"
     ...

Then update the test:

-def test_root_help_lists_live_options() -> None:
+def test_root_help_lists_live_options(tmp_path: Path) -> None:
     ...
-    result = _run_pythinker_main(["--help"])
+    result = _run_pythinker_main(["--help"], share_dir=tmp_path / "share")
🤖 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 `@tests/e2e/test_cli_error_output.py` around lines 199 - 216, The helper
_run_pythinker_main currently hardcodes PYTHINKER_SHARE_DIR to Path.home() /
".pythinker-test-share"; change it to accept a tmp_path fixture (e.g., add
parameter tmp_path: Path) and set env["PYTHINKER_SHARE_DIR"] = str(tmp_path /
".pythinker-test-share") so tests use an isolated temporary directory; update
any calls to _run_pythinker_main (or prefer reusing the existing _run_pythinker
helper) in the test file to pass the tmp_path fixture and remove reliance on the
user HOME to avoid cross-test interference.
🤖 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.

Outside diff comments:
In `@src/pythinker_code/soul/pythinkersoul.py`:
- Around line 1681-1695: The current CancelledError handling creates a shielded
task for self._grow_context(...) but doesn't keep a reference, so if another
cancellation occurs while awaiting it the background grow_context write may
never be drained; change the pattern to store the task in a variable (e.g.,
grow_context_task = asyncio.create_task(self._grow_context(result,
interrupted))) and then await it via await asyncio.shield(grow_context_task)
inside the except asyncio.CancelledError block (and re-shield-await again before
re-raising) so the interrupted context write started by _grow_context (and the
ToolResult/ToolRuntimeError entries built from result.tool_calls and
completed_tool_results) is always completed/drained before letting
CancelledError propagate.

In `@tests/e2e/test_cli_error_output.py`:
- Around line 199-216: The helper _run_pythinker_main currently hardcodes
PYTHINKER_SHARE_DIR to Path.home() / ".pythinker-test-share"; change it to
accept a tmp_path fixture (e.g., add parameter tmp_path: Path) and set
env["PYTHINKER_SHARE_DIR"] = str(tmp_path / ".pythinker-test-share") so tests
use an isolated temporary directory; update any calls to _run_pythinker_main (or
prefer reusing the existing _run_pythinker helper) in the test file to pass the
tmp_path fixture and remove reliance on the user HOME to avoid cross-test
interference.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 454217a2-23d9-474c-8f24-605eb5b7a2bf

📥 Commits

Reviewing files that changed from the base of the PR and between a39d29b and 32108f0.

📒 Files selected for processing (16)
  • src/pythinker_code/cli/vis.py
  • src/pythinker_code/file_restore.py
  • src/pythinker_code/scratchpad.py
  • src/pythinker_code/soul/__init__.py
  • src/pythinker_code/soul/permission.py
  • src/pythinker_code/soul/pythinkersoul.py
  • src/pythinker_code/soul/slash.py
  • src/pythinker_code/tools/web/fetch.py
  • src/pythinker_code/ui/shell/export_import.py
  • src/pythinker_code/utils/export.py
  • src/pythinker_code/web/runner/process.py
  • tests/core/test_permission_profiles.py
  • tests/core/test_project_memory.py
  • tests/core/test_scratchpad.py
  • tests/e2e/test_cli_error_output.py
  • tests/ui_and_conv/test_export_import.py

@elkaix

elkaix commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

Superseded by #103 (branch renamed to feat/tui-enhancements-and-hardening, which contains this work plus the security-audit remediation, multi-instance hardening, and openai package refactor).

@elkaix elkaix closed this Jun 10, 2026
@elkaix
elkaix deleted the feat/tui-enhancements branch June 10, 2026 13:47
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