feat(tui+lsp): streaming polish, tool card renderers, and LSP implementation guard - #159
Merged
Merged
Conversation
…tationProvider When a language server does not advertise implementationProvider in its ServerCapabilities, calling go_to_implementation now returns a structured error message (operation, server name, reason) instead of a raw exception. Also advertises the `implementation` client capability in the LSP initialize handshake so servers like Pyright enable the provider automatically.
Contributor
|
Warning Review limit reached
More reviews will be available in 35 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughThe PR updates LSP capability handling and result metadata, adds several shell tool renderers and registry wiring, refactors streaming live-view behavior around reason-based flushing and DiffLive, and renames blackbox/reference wording and paths across docs, configs, and tests. ChangesLSP Capability and Shell Tool Rendering
Streaming Live-View Overhaul
Terminology Cleanup, Scan Paths, and Guardrails
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Content blocks now promote to scrollback exactly once with a paint-before-print step in Rich Live mode, interrupted open ```report``` fences surface a short note instead of raw JSON, and paced transitions use bounded reveal so large backlogs no longer dump in full before tool cards. - src/pythinker_code/ui/shell/prompt.py: 78 lines - src/pythinker_code/ui/shell/visualize/_blocks.py: 369 lines - src/pythinker_code/ui/shell/visualize/_interactive.py: 148 lines - src/pythinker_code/ui/shell/visualize/_live_view.py: 98 lines - tests/ui_and_conv/test_stream_pacing.py: 170 lines - tests/ui_and_conv/test_streaming_content_block.py: 653 lines - tests/ui_and_conv/test_visualize_running_prompt.py: 166 lines - tests/utils/test_broadcast_queue.py: 12 lines - tasks/streaming-render-rootcause.md: rootcause report (new) - CHANGELOG.md: Unreleased entry
elkaix
changed the title
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
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/utils/rich/syntax.py (1)
280-293:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify the accepted-theme contract.
available_code_themes()does not include custom.tmThemenames; those are only added bylist_picker_code_themes(). The docstring currently implies otherwise.Suggested wording
- """Accepted ``code_theme`` values: bundled Pythinker names, sentinels, custom, Pygments.""" + """Accepted ``code_theme`` values: bundled Pythinker names, sentinels, and Pygments styles."""🤖 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/utils/rich/syntax.py` around lines 280 - 293, The docstring for the available_code_themes() function is misleading because it implies that custom themes are included in the returned list, but the actual implementation only returns bundled Pythinker theme names, Catppuccin and Pythinker ANSI sentinels, and Pygments styles. Update the docstring to accurately document what theme sources are actually included and clarify that custom .tmTheme names are NOT included in this function but are only added separately by the list_picker_code_themes() function.
🤖 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/tools/lsp/tool.py`:
- Around line 102-106: The truthiness check for implementationProvider at line
105 incorrectly rejects empty dictionaries because empty dicts evaluate as
falsy. Replace the `not server.capabilities.implementationProvider` check with
explicit comparisons that properly handle all LSP spec cases: check if
implementationProvider is explicitly False or is None, rather than relying on
truthiness evaluation. This ensures that empty dicts (which indicate supported
with default options) are correctly accepted while unsupported (False) and
unadvertised (None) cases are properly rejected.
In `@src/pythinker_code/ui/shell/tool_renderers/lsp.py`:
- Around line 63-70: The final return statement in the _operation_detail
function directly accesses ctx.args.get("operation") without checking if
ctx.args is None first, which causes an AttributeError in the result-only render
path. Guard ctx.args against None before attempting to call the get method on it
in the return statement, ensuring safe fallback behavior to "result". Use the
same guarded access pattern applied elsewhere in this file for similar attribute
access scenarios.
In `@src/pythinker_code/ui/shell/tool_renderers/worktree.py`:
- Around line 44-69: Both the _render_enter_result and _render_exit_result
functions must check the result.is_error property before rendering success
header messages to prevent failed tool calls from appearing as successful. Add a
check at the beginning of each function that returns early (with None or an
appropriate error rendering) if result.is_error is True, ensuring that error
states are honored before any success-oriented messages like "Switched to
worktree" or "Kept/Removed worktree" are rendered to the user.
In `@src/pythinker_code/ui/shell/visualize/_blocks.py`:
- Around line 255-276: The current code skips bare fence openers (fences without
language info) because of the "if not info: continue" check, preventing proper
tracking of unclosed plain fences. Instead of immediately returning when a
language-tagged fence is found, track fence pairs by storing the first opening
fence's details (open_match, open_marker, open_info, open_is_report) and look
for matching closing fences where the marker matches and info is empty. After
the loop, process the tracked opening fence if it was found and is not a report
fence, extracting the language from open_info and building the preview
placeholder using open_match's position instead of the current immediate return
logic.
In `@src/pythinker_code/ui/shell/visualize/_diff_live.py`:
- Around line 147-152: The _stop_interactive() method in the DiffLive class
calls self.console.clear_live() unconditionally before checking if the instance
is nested. When this instance is nested (self._nested is True), it should not
own the console._live and therefore should not clear it, as this tears down the
parent live renderer. Move the self.console.clear_live() call to only execute
when self._nested is False by either placing it after the nested guard check or
wrapping it with a conditional that skips it when nested.
In `@src/pythinker_code/ui/shell/visualize/_interactive.py`:
- Around line 356-367: The TurnEnd message handling block sets _force_refresh to
True and calls _flush_prompt_refresh(), but this method does not actually
invalidate the prompt state unless _dirty or _need_recompose is already True.
For contentless or tool-only turns, the prompt stays stale after TurnEnd. To fix
this, when handling the TurnEnd message and setting _force_refresh = True before
the _flush_prompt_refresh() call, also set either _dirty or _need_recompose to
True so that the refresh actually gets processed and the prompt state is
properly invalidated.
In `@src/pythinker_code/ui/shell/visualize/_live_view.py`:
- Around line 353-376: The `_extend_wire_batch()` method creates a new
`wire.receive()` task during batch collection, but if that task raises
`QueueShutDown`, the exception bypasses dispatch of messages already accumulated
in the `messages` list, potentially dropping critical final messages like
`TurnEnd` or `ToolResult`. Catch the `QueueShutDown` exception within
`_extend_wire_batch()` and either return a flag indicating the wire is closed or
handle it such that the caller can dispatch the accumulated `messages` before
running shutdown cleanup, ensuring no buffered messages are lost. Apply the same
fix to the similar code pattern around line 518-520.
In `@tests/tools/test_lsp_tool.py`:
- Around line 490-507: The test_go_to_implementation_unsupported_server function
currently only validates the error message content but lacks verification that
the textDocument/implementation request was never sent to the LSP server. Add a
log assertion or message capture mechanism to confirm that the guard
short-circuits before dispatching the request when the implementationProvider
capability is missing, ensuring the test covers the observable effect of the
guard preventing the request rather than just message matching.
In `@tests/ui_and_conv/test_stream_pacing.py`:
- Around line 146-165: Remove the monkeypatching of the private
_find_committed_boundary function and the assertion on the calls counter in the
test_reveal_tick_skips_markdown_boundary_scan_without_newline function. Instead
of verifying internal call counts, assert on externally observable behavior such
as the revealed or committed state of the _ContentBlock after calling
reveal_tick multiple times, or verify the actual output/content that gets
emitted from the block.
In `@tests/ui_and_conv/test_tui_card_tool_renderers.py`:
- Around line 2185-2217: The test function
test_worktree_renderers_parse_tool_output_metadata currently only validates the
success paths for the EnterWorktree and ExitWorktree renderers. Add failure-path
test coverage by creating additional _render calls with is_error=True parameter
for both EnterWorktree and ExitWorktree (similar to the existing success cases).
For each error case, add assertions that verify error text is displayed in the
output and that the success-path labels (such as "Switched to worktree" for
EnterWorktree and "Kept worktree" for ExitWorktree) are NOT present, ensuring
the error paths are properly handled and displayed.
In `@tests/ui_and_conv/test_visualize_running_prompt.py`:
- Around line 897-942: The test
test_prompt_live_view_flushes_content_before_marking_turn_ended does not
actually verify the ordering constraint stated in its name. Currently it only
checks the final state (that view._turn_ended is True and
view._current_content_block is None), but does not prove that flush_content was
called while view._turn_ended was still False. Add a mechanism to track when
flush_content is called (using monkeypatch or similar) and capture the state of
view._turn_ended at that moment, then add an explicit assertion that verifies
flush_content ran before view._turn_ended became True. This will properly test
the ordering guarantee implied by the test's name.
---
Outside diff comments:
In `@src/pythinker_code/utils/rich/syntax.py`:
- Around line 280-293: The docstring for the available_code_themes() function is
misleading because it implies that custom themes are included in the returned
list, but the actual implementation only returns bundled Pythinker theme names,
Catppuccin and Pythinker ANSI sentinels, and Pygments styles. Update the
docstring to accurately document what theme sources are actually included and
clarify that custom .tmTheme names are NOT included in this function but are
only added separately by the list_picker_code_themes() function.
🪄 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: c4d12e7d-0138-4a42-8ebf-a8a889b2f1a0
⛔ Files ignored due to path filters (10)
docs/en/customization/architecture.mdis excluded by!docs/**docs/en/release-notes/changelog.mdis excluded by!docs/**docs/history/CHANGELOG-pre-0.8.0.mdis excluded by!docs/**tasks/agent-harness-adoption-plan.mdis excluded by!tasks/**tasks/design-adoption-blueprint.mdis excluded by!tasks/**tasks/lessons.mdis excluded by!tasks/**tasks/reference-port-status.mdis excluded by!tasks/**tasks/streaming-render-rootcause.mdis excluded by!tasks/**tasks/streaming-wire-bug-hunt-report.mdis excluded by!tasks/**tasks/todo.mdis excluded by!tasks/**
📒 Files selected for processing (58)
.gitignoreCHANGELOG.mdpackages/pythinker-review/README.mdpackages/pythinker-review/docs/code-reviewr-migration.mdpackages/pythinker-review/docs/reference-parity.mdpackages/pythinker-review/docs/security-scan-migration.mdpackages/pythinker-review/src/pythinker_review/engine/structured_diff.pypackages/pythinker-review/src/pythinker_review/security_intel/__init__.pypackages/pythinker-review/src/pythinker_review/security_scan/knowledge.pyplips/plip-10-lsp-system.mdpytest.inisecurity-scan-findings.jsonsrc/pythinker_code/agents/default/security_reviewer.yamlsrc/pythinker_code/llm.pysrc/pythinker_code/lsp/instance.pysrc/pythinker_code/plugin/marketplace.pysrc/pythinker_code/tools/lsp/tool.pysrc/pythinker_code/ui/shell/__init__.pysrc/pythinker_code/ui/shell/components/dynamic_border.pysrc/pythinker_code/ui/shell/components/render_utils.pysrc/pythinker_code/ui/shell/components/tool_execution.pysrc/pythinker_code/ui/shell/motion.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/slash.pysrc/pythinker_code/ui/shell/tool_renderers/__init__.pysrc/pythinker_code/ui/shell/tool_renderers/_file_diff.pysrc/pythinker_code/ui/shell/tool_renderers/_render_utils.pysrc/pythinker_code/ui/shell/tool_renderers/background.pysrc/pythinker_code/ui/shell/tool_renderers/grep.pysrc/pythinker_code/ui/shell/tool_renderers/lsp.pysrc/pythinker_code/ui/shell/tool_renderers/mcp_resource.pysrc/pythinker_code/ui/shell/tool_renderers/memory.pysrc/pythinker_code/ui/shell/tool_renderers/read.pysrc/pythinker_code/ui/shell/tool_renderers/read_media.pysrc/pythinker_code/ui/shell/tool_renderers/smart_search.pysrc/pythinker_code/ui/shell/tool_renderers/worktree.pysrc/pythinker_code/ui/shell/tool_renderers/write.pysrc/pythinker_code/ui/shell/update.pysrc/pythinker_code/ui/shell/visualize/_blocks.pysrc/pythinker_code/ui/shell/visualize/_diff_live.pysrc/pythinker_code/ui/shell/visualize/_interactive.pysrc/pythinker_code/ui/shell/visualize/_live_view.pysrc/pythinker_code/ui/theme/pythinker_themes.pysrc/pythinker_code/utils/rich/syntax.pytests/core/test_toolset_concurrency.pytests/test_ai_static_requirements.pytests/tools/test_lsp_tool.pytests/ui_and_conv/test_modal_lifecycle.pytests/ui_and_conv/test_pythinker_themes_port.pytests/ui_and_conv/test_shell_slash_commands.pytests/ui_and_conv/test_spinner_words.pytests/ui_and_conv/test_stream_pacing.pytests/ui_and_conv/test_streaming_content_block.pytests/ui_and_conv/test_tool_call_block.pytests/ui_and_conv/test_tool_search_suppression.pytests/ui_and_conv/test_tui_card_tool_renderers.pytests/ui_and_conv/test_visualize_running_prompt.pytests/utils/test_broadcast_queue.py
Route interactive-mode scrollback through run_in_terminal so prompt preamble is not fossilized into permanent transcript output. Render parent-bullet summaries with aligned field rows as structured blocks with preserved hierarchy and correct continuation wrap indent. Make ty blocking for pythinker-code checks and accept TodoWrite merge field.
Share the approval/pager PythinkerSyntax pipeline with Edit/Write tool cards while keeping the compact boxless layout. Add AGENTS.md guidance to prefer LSP when a language server is available.
Render tool-card diffs in a three-column grid so wrapped continuation rows stay under the code body and repeat +/- markers. Fix LSP implementationProvider empty-object support, worktree error surfacing, nested DiffLive teardown, wire shutdown batching, and unclosed fence preview suppression.
The fake LSP server only creates lsp.log when a loggable method arrives. When the implementationProvider guard fires first, no loggable methods are dispatched so the file is legitimately absent. Reading it unconditionally raised FileNotFoundError.
- update SetTodoList schema snapshot to include the new `merge` compatibility field (excluded from serialization but present in JSON schema) - rename and flip test_helper_leaves_bare_fence_line_untouched: bare ` ``` ` openers are now suppressed with fallback lang "code", matching the paired-fence tracking introduced alongside the CodeRabbit fix - patch DiffLive (not the removed Rich Live) in test_live_view_processes_external_approval_messages; drop the Rich-specific _live_render attribute and add refresh() no-op - add CHANGELOG unreleased entry for the smooth-streaming coalescing work
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pythinker_code/ui/shell/markdown/normalizers.py (1)
476-477:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude
-in the fast-path guard for report-block normalization.The parser now accepts parent bullets matching
[-•], but the early return still exits when•is absent. Hyphen-only inputs skip normalization entirely.Suggested fix
def normalize_space_aligned_report_blocks(markup: str) -> str: """Convert LLM space-column report rows into nested Markdown lists.""" - if "•" not in markup: + if "•" not in markup and "-" not in markup: return markupAlso applies to: 508-513
🤖 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/markdown/normalizers.py` around lines 476 - 477, The fast-path guard condition that checks if "•" is not present in markup needs to be updated to also check for the hyphen character "-" since the parser now accepts both as valid parent bullets matching the pattern [-•]. Update the condition in the early return at lines 476-477 to return only when both "•" and "-" are absent from the markup, ensuring that hyphen-only inputs do not skip normalization. Apply the same fix to the other location mentioned at lines 508-513.tests/ui_and_conv/test_btw.py (1)
882-904:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the new queued steer echo side effect.
This test now initializes
_pending_scrollback, but it still only verifies steering and the dedup counter. A regression that stops queuing the permanent Ctrl+S echo would pass. Add an assertion for one queued non-blank-row echo entry, or flush and assert the captured output. As per coding guidelines, tests should not cover only the happy path while ignoring changed edge/user-visible behavior.Proposed assertion
assert len(steered) == 1 assert view._pending_local_steer_count == 1 # counter incremented + assert len(view._pending_scrollback) == 1 + _, blank_row = view._pending_scrollback[0] + assert blank_row is False🤖 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/ui_and_conv/test_btw.py` around lines 882 - 904, The test_normal_text_via_ctrl_s_steers_normally test initializes _pending_scrollback as an empty list but never verifies that the handle_immediate_steer method queues an echo entry to it. After the existing assertions for steered and _pending_local_steer_count, add an assertion to verify that _pending_scrollback now contains exactly one entry representing the queued echo for the Ctrl+S command. This ensures the test covers the side effect of queuing the permanent echo and would catch regressions if that behavior stops working.Source: Coding guidelines
src/pythinker_code/ui/shell/visualize/_interactive.py (1)
298-302:⚠️ Potential issue | 🟠 MajorAdd non-terminal fallback to incremental scrollback emission.
_emit_incremental_content_commitsalways callsrun_in_terminal, which does not write to captured stdout in non-terminal/piped mode. Sincetake_committed_renderables()removes these items before printing, they are silently lost in piped sessions._flush_pending_scrollbackalready handles this with aconsole.is_terminalcheck (lines 328–331); apply the same pattern here.- await run_in_terminal(emit_committed) + if console.is_terminal: + await run_in_terminal(emit_committed) + else: + emit_committed()🤖 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/_interactive.py` around lines 298 - 302, The emit_committed function inside _emit_incremental_content_commits always uses run_in_terminal, which fails to write to captured stdout in piped/non-terminal mode, causing renderables to be silently lost. Add a console.is_terminal check before calling run_in_terminal (similar to the pattern already implemented in _flush_pending_scrollback at lines 328-331), and provide a fallback that directly emits the renderables when not in terminal mode, ensuring committed items are properly output in both terminal and piped sessions.
🤖 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/visualize/_interactive.py`:
- Around line 309-331: The _flush_pending_scrollback() method clears
_pending_scrollback before awaiting the run_in_terminal(emit) call, which can
cause concurrent flushes or cancellation to silently drop or reorder queued
transcript entries. Add a lock to serialize pending scrollback flushes and move
the clearing of _pending_scrollback to after the emit operation completes
successfully, rather than before. This ensures that the to_print list is only
populated and the queue is only cleared after the terminal handoff finishes,
preventing any concurrent calls from _status_refresh_loop() or visualize_loop()
from causing data loss.
In `@tests/ui_and_conv/test_tui_card_tool_renderers.py`:
- Around line 1083-1091: The test function
test_render_diff_without_path_does_not_construct_highlighter is patching
make_diff_highlighter in the wrong location. Currently it patches
pythinker_code.utils.rich.diff_render.make_diff_highlighter, but the render_diff
function imports and uses make_diff_highlighter from
pythinker_code.ui.shell.components.diff. Update the monkeypatch call to target
the make_diff_highlighter symbol where it is actually imported and used by
render_diff, which is in the pythinker_code.ui.shell.components.diff module, not
in the original source module.
---
Outside diff comments:
In `@src/pythinker_code/ui/shell/markdown/normalizers.py`:
- Around line 476-477: The fast-path guard condition that checks if "•" is not
present in markup needs to be updated to also check for the hyphen character "-"
since the parser now accepts both as valid parent bullets matching the pattern
[-•]. Update the condition in the early return at lines 476-477 to return only
when both "•" and "-" are absent from the markup, ensuring that hyphen-only
inputs do not skip normalization. Apply the same fix to the other location
mentioned at lines 508-513.
In `@src/pythinker_code/ui/shell/visualize/_interactive.py`:
- Around line 298-302: The emit_committed function inside
_emit_incremental_content_commits always uses run_in_terminal, which fails to
write to captured stdout in piped/non-terminal mode, causing renderables to be
silently lost. Add a console.is_terminal check before calling run_in_terminal
(similar to the pattern already implemented in _flush_pending_scrollback at
lines 328-331), and provide a fallback that directly emits the renderables when
not in terminal mode, ensuring committed items are properly output in both
terminal and piped sessions.
In `@tests/ui_and_conv/test_btw.py`:
- Around line 882-904: The test_normal_text_via_ctrl_s_steers_normally test
initializes _pending_scrollback as an empty list but never verifies that the
handle_immediate_steer method queues an echo entry to it. After the existing
assertions for steered and _pending_local_steer_count, add an assertion to
verify that _pending_scrollback now contains exactly one entry representing the
queued echo for the Ctrl+S command. This ensures the test covers the side effect
of queuing the permanent echo and would catch regressions if that behavior stops
working.
🪄 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: e6378ca1-aed1-4dfb-bbee-628d590c45f2
📒 Files selected for processing (32)
AGENTS.mdCHANGELOG.mdMakefilesrc/pythinker_code/tools/lsp/tool.pysrc/pythinker_code/tools/todo/__init__.pysrc/pythinker_code/ui/shell/components/diff.pysrc/pythinker_code/ui/shell/components/report.pysrc/pythinker_code/ui/shell/components/report_prose_blocks.pysrc/pythinker_code/ui/shell/markdown/normalizers.pysrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/tool_renderers/_file_diff.pysrc/pythinker_code/ui/shell/tool_renderers/edit.pysrc/pythinker_code/ui/shell/tool_renderers/lsp.pysrc/pythinker_code/ui/shell/tool_renderers/worktree.pysrc/pythinker_code/ui/shell/tool_renderers/write.pysrc/pythinker_code/ui/shell/visualize/_blocks.pysrc/pythinker_code/ui/shell/visualize/_diff_live.pysrc/pythinker_code/ui/shell/visualize/_interactive.pysrc/pythinker_code/ui/shell/visualize/_live_view.pysrc/pythinker_code/utils/rich/diff_render.pytests/tools/test_lsp_tool.pytests/tools/test_todo.pytests/tools/test_tool_schemas.pytests/ui_and_conv/test_btw.pytests/ui_and_conv/test_modal_lifecycle.pytests/ui_and_conv/test_output_guards.pytests/ui_and_conv/test_report_prose_blocks.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_streaming_phase0.pytests/ui_and_conv/test_visualize_running_prompt.py
💤 Files with no reviewable changes (1)
- src/pythinker_code/ui/shell/prompt.py
…port rendering Streaming smoothness (the agent-working "jump"): - Stop committing completed prose to scrollback mid-stream. Each mid-stream commit was a run_in_terminal prompt-app teardown — the visible jump. Prose now stays in the in-place live preview (tail-clamped by _compose_composing) and is flushed to scrollback exactly once at a tool transition or turn end, mirroring the reference in-place-then-flush architecture. - Remove the now-dead per-tick commit throttle (_maybe_emit_incremental_commits and the _INCREMENTAL_COMMIT_* constants); the paced-reveal cap stays. Finalize/clipping honesty: - Route scrollback handoffs through _run_scrollback_handoff and show a Finalizing state instead of a false idle prompt while scrollback is pending. - Surface an "earlier output hidden · Ctrl+O expand" marker when live rows are clamped, instead of silently dropping them. Agent tool / prompts: - Add a fail-loud "did you mean?" suggestion for hallucinated subagent type names (cross-harness aliases + fuzzy match); never substitute silently. - Document structured report findings blocks and dual terminal/saved report destinations; expand report and audit-markdown rendering. Tests updated to the new contracts; full check + tests + tests_e2e green.
- Collapse low/info severities in large compact reports with a saved-report pointer - Strip duplicate preamble/trailer prose and compact artifact footer paths - Refine agent prompt: report JSON or prose (not both), compact terminal footers - Fix prose-block inline-code wrapping and add handoff-trace diagnostics - Harden tests for worktree errors, turn-end flush ordering, and audit rendering
Anchor tool-output path collapsing on the .pythinker/sessions marker instead of the runtime share dir so compact_known_paths works across users and OSes.
…rden review nits - Update prompt phrase-pin test to the reworded findings-report contract in system.md (deliberate rewording in 6302b49; fix the test, not the prompt). - Patch make_diff_highlighter where render_diff resolves it (ui.shell.components.diff), so the no-highlighter-without-path test actually guards the contract. - Remove the orphaned _COMPACT_REPORT_* threshold constants left unused after the layout switch to literal thresholds; behavior unchanged. - Document the intentional best-effort OSError swallow in _handoff_trace.
This was referenced Jul 6, 2026
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
go_to_implementationreturns a structured error when the server does not advertiseimplementationProvider; the client advertisesimplementationcapability during initialize so servers like Pyright enable the provider automatically.```reportfences show a short note instead of raw JSON; paced transitions use bounded reveal; diff-based live refresh reduces flicker.Test plan
make check-pythinker-codemake test-pythinker-code(focused UI/streaming/LSP tests)Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores