refactor(tui): prompt deep-module PR 3 — canonical completion + async workspace index#220
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR unifies slash-command and file-mention completion through shared context parsing, adds slash and workspace completion modules, introduces asynchronous Git-aware workspace indexing, integrates the index into the prompt session, and expands related tests and release metadata. ChangesUnified completion context and async workspace indexing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Prompt as prompt.py
participant Completer as HostFileMentionCompleter
participant Index as WorkspaceIndex
participant Host as Host/Git
Prompt->>Completer: request file completions
Completer->>Index: request_refresh(fragment)
Index->>Host: discover workspace paths
Host-->>Index: return indexed entries
Index-->>Completer: publish WorkspaceSnapshot
Completer-->>Prompt: return ranked candidates
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
6d842fb to
b8d8ec1
Compare
3baca3f to
14d58a5
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 17 minutes. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 3687-3694: Guard the HostPath.cwd() call in _prompt_once with the
same OSError handling used by the constructor, preserving the existing
workspace-root comparison and workspace_index.set_root behavior when the lookup
succeeds.
- Around line 1784-1796: Guard the HostPath.cwd() call in the shell
initialization flow around _workspace_root and handle OSError using the file’s
established fallback behavior from _render_bottom_toolbar or
_build_statusline_context. Apply the same required protection to the related
_prompt_once occurrence, preserving workspace setup when the current directory
is available.
In `@src/pythinker_code/ui/shell/prompting/completion/context.py`:
- Around line 205-212: Collapse the redundant conditional in the surrounding
completion logic to a single `if slash.kind is CompletionKind.SLASH_ARGUMENT`
branch that returns `slash`. Remove the unused `argument_commands` and
`argument_index` checks from this branch, leaving any actual argument-completion
gating to `SlashCommandCompleter.get_completions`.
In `@src/pythinker_code/ui/shell/prompting/completion/workspace.py`:
- Around line 102-136: Reorder the logic in request_refresh so the current
snapshot freshness check for the new key runs before cancelling any in-flight
_refresh_task. Return without cancellation when the requested key is already
fresh; only cancel an existing task after determining that new refresh work must
be scheduled, while preserving the same-key deduplication behavior.
In `@tests/ui_and_conv/test_workspace_index.py`:
- Around line 137-139: Update _InspectableWorkspaceIndex.degraded and
WorkspaceIndex to expose degraded status through a public method or property, or
through snapshot()’s returned value, instead of reading the private _snapshots
dictionary. Ensure the test asserts the observable degraded behavior without
assuming a single populated snapshot key.
- Around line 24-38: Move the superset _Lifecycle test double, including its
created-task tracking and drain() method, from
tests/ui_and_conv/test_workspace_index.py lines 24-38 into shared test support
such as tests/ui_and_conv/conftest.py; update test_workspace_index.py to use
that shared definition. Remove the duplicate _Lifecycle from
tests/ui_and_conv/test_file_completer.py lines 27-36 and import the shared
_Lifecycle there.
🪄 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
Run ID: 92d8b6a8-54cb-4985-8da4-684f826d44b5
📒 Files selected for processing (11)
CHANGELOG.mdpyproject.tomlsrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/prompting/completion/__init__.pysrc/pythinker_code/ui/shell/prompting/completion/context.pysrc/pythinker_code/ui/shell/prompting/completion/slash.pysrc/pythinker_code/ui/shell/prompting/completion/workspace.pytests/ui_and_conv/test_file_completer.pytests/ui_and_conv/test_slash_completer.pytests/ui_and_conv/test_slash_highlight.pytests/ui_and_conv/test_workspace_index.py
✅ Action performedFull review finished. You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 17 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/prompting/completion/slash.py`:
- Around line 55-73: Update discard_slash_command to also handle SLASH_ARGUMENT
when argument_index is 0, preserving the existing slash-command buffer removal
behavior for both completion kinds. Continue returning False for other
completion contexts so the fallback behavior remains available where
appropriate.
In `@src/pythinker_code/ui/shell/prompting/completion/workspace.py`:
- Around line 486-522: Update the ranking logic in _rank to compare each
candidate basename with the basename portion of the typed fragment, not the full
fragment including directory prefixes. Preserve the existing startswith,
contains, and fallback category ordering and test_penalty behavior for both
scoped and unscoped completions.
- Around line 409-441: Update _run_git so process.stdout and process.stderr are
drained concurrently rather than sequentially, ensuring verbose Git output
cannot deadlock on a full pipe. Preserve the existing bounded-read behavior,
timeout and process-stop handling, return-code check, and stdout decoding.
In `@tests/ui_and_conv/test_slash_completer.py`:
- Around line 226-247: Correct
test_completion_context_reports_first_and_later_arguments so escaped_whitespace
validates the intended escape behavior instead of merely asserting a second
argument: either assert that the backslash-escaped space remains one token with
the appropriate argument index, or use ordinary whitespace for later-argument
coverage and add a separate explicit malformed/unsupported escape case. Keep the
existing first-argument and later-argument assertions aligned with the contract.
In `@tests/ui_and_conv/test_slash_highlight.py`:
- Around line 132-134: Update
test_mention_after_allowed_punctuation_is_highlighted and the shared mention
parser so trailing sentence punctuation is excluded from file-mention spans.
Ensure _mentions(_lex_line("read (`@src/main.py`), now")) returns only
"`@src/main.py`", with completion matching operating on the punctuation-free path.
🪄 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
Run ID: 056fa868-42a9-436f-8e9f-776fb959b26b
📒 Files selected for processing (11)
CHANGELOG.mdpyproject.tomlsrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/prompting/completion/__init__.pysrc/pythinker_code/ui/shell/prompting/completion/context.pysrc/pythinker_code/ui/shell/prompting/completion/slash.pysrc/pythinker_code/ui/shell/prompting/completion/workspace.pytests/ui_and_conv/test_file_completer.pytests/ui_and_conv/test_slash_completer.pytests/ui_and_conv/test_slash_highlight.pytests/ui_and_conv/test_workspace_index.py
- Escape now dismisses the slash-argument menu (cancel_completion fallback) instead of doing nothing when there is no draft command to strip. - Strip trailing prose punctuation from unquoted @-mention highlight spans. - Rank scoped @dir/name completions by the fragment's basename so prefix priority applies to path-scoped fragments. - Drain git stdout/stderr concurrently to avoid a pipe deadlock on verbose output. - Guard the mid-session HostPath.cwd() lookup so a removed working directory no longer crashes the prompt turn. - Collapse a dead SLASH_ARGUMENT branch in the completion parser. - De-duplicate the PromptLifecycle test double, make the degraded-state test helper multi-key-safe, and re-frame the backslash-whitespace parser test to document the real (non-escape-aware) contract.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/pythinker_code/ui/shell/prompt.py (2)
3683-3692: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOnly refresh an already-open file-completion menu.
This callback checks whether the buffer resembles an
@mention, but not whether completion is currently active. After Escape dismisses@...completion, a later snapshot publish callsstart_completion()and reopens the menu unexpectedly. Require an existingcomplete_statebefore restarting completion.🤖 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/prompt.py` around lines 3683 - 3692, Update _on_workspace_snapshot_published to return unless the buffer already has an active complete_state, in addition to the existing app-running and HostFileMentionCompleter checks. Only call start_completion and app.invalidate when an existing file-completion menu is still open.
1785-1791: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftClose the workspace index on synchronous context exit.
The closer is registered with
PromptLifecycle, but__exit__at Line 3394 only cancels the status task and statusline runner; it does not invoke lifecycle shutdown, unlike__aexit__. Awith CustomPromptSession()caller can therefore leave workspace discovery tasks or processes running. Add a synchronous cancellation/close path for this resource.As per coding guidelines, background workers, tasks, and processes must define lifecycle, cancellation, and shutdown.
🤖 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/prompt.py` around lines 1785 - 1791, Update the synchronous __exit__ path in CustomPromptSession to invoke the PromptLifecycle shutdown/close operation, ensuring the registered workspace index closer (self._workspace_index.aclose) runs during with-block exit. Preserve the existing status-task and statusline cleanup, and align the synchronous behavior with __aexit__ without changing asynchronous lifecycle handling.Source: Coding guidelines
tests/ui_and_conv/test_file_completer.py (1)
24-42: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClose the lifecycle in a
finallyblock.If refresh or completion collection raises,
await lifecycle.aclose()is skipped, leaving lifecycle-owned tasks alive and potentially contaminating later tests. Wrap the helper body intry/finallyand close the lifecycle unconditionally.🤖 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_file_completer.py` around lines 24 - 42, Update the _completion_results helper to wrap workspace setup, refresh, and completion collection in a try/finally block, moving lifecycle.aclose() into the finally section so it runs even when an earlier operation raises. Preserve the existing completion return behavior on success.src/pythinker_code/ui/shell/prompting/completion/workspace.py (3)
183-187: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not log absolute workspace paths or raw exception details.
These logs interpolate
rootandexc; other messages in this module also log paths, command arguments, and user-derived scopes. Workspace paths commonly contain usernames or project-sensitive names. Redact them and log stable identifiers plus categorized exception types instead.As per coding guidelines, logs must never leak PII, tokens, secrets, or unsafe diagnostic details.
🤖 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/prompting/completion/workspace.py` around lines 183 - 187, Update the warning log in the workspace refresh failure path to avoid interpolating the absolute root or raw exception. Replace them with a stable, redacted workspace identifier and a safe categorized exception type, and audit nearby logging in the same module for paths, command arguments, or user-derived scopes that require the same redaction.Source: Coding guidelines
95-100: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftExpose degraded and unavailable snapshot states.
snapshot()collapses “not indexed,” degraded/partial, and empty workspaces into the same empty tuple; task-scheduling failure also returns without publishing a status.HostFileMentionCompletertherefore presents incomplete results as ordinary “no matches.” Return or expose a status-bearing snapshot so callers can distinguish pending, failed, degraded, and successful results.As per coding guidelines, observable results must distinguish success, failure, degraded output, and no data.
Also applies to: 127-132
🤖 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/prompting/completion/workspace.py` around lines 95 - 100, Update Workspace.snapshot to return a status-bearing result that distinguishes pending, unavailable/failed, degraded, successful, and genuinely empty snapshots instead of collapsing them to an empty tuple. Preserve immutable entries for successful results, and publish an explicit failure/unavailable status when snapshot task scheduling fails. Update HostFileMentionCompleter and other snapshot consumers to handle these states without presenting incomplete or unavailable results as ordinary “no matches.”Source: Coding guidelines
158-166: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftReject Windows path escapes in workspace discovery.
_snapshot_key()only checks/-delimited.., so Windows fragments likea\\..\\..\\secret/xorC:\\secret/xcan still reachroot.joinpath(...)in_discover_bounded()and turn into an escaping or absolute path. Normalize both separators, reject drive-qualified/absolute scopes, and enforce containment before bounded traversal andgit ls-files. Add a Windows regression test.🤖 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/prompting/completion/workspace.py` around lines 158 - 166, Update _snapshot_key() to normalize backslashes as path separators, reject Windows drive-qualified and absolute scopes, and reject any normalized scope containing parent traversal. In _discover_bounded(), enforce that the resolved traversal path remains contained within the workspace root before performing bounded traversal or git ls-files. Add a regression test covering Windows-style traversal and drive-qualified fragments.
♻️ Duplicate comments (1)
src/pythinker_code/ui/shell/prompt.py (1)
1784-1791: 🩺 Stability & Availability | 🟠 MajorGuard all constructor
HostPath.cwd()calls.
HostPath.cwd()can still raise here, and the constructor already calls it unguarded at Line 1706 for history hashing. If the working directory disappears, construction fails before_prompt_once()can apply its new guard. Resolve the root once with the establishedCwdLostError/OSErrorhandling and reuse it for both history and workspace setup.🤖 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/prompt.py` around lines 1784 - 1791, Update the constructor’s existing HostPath.cwd() usage, including the history-hashing setup and the workspace initialization around _workspace_root, to resolve the current directory once inside the established CwdLostError/OSError handling. Reuse that resolved root for both history and WorkspaceIndex construction so no constructor path calls HostPath.cwd() unguarded.
🤖 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/prompting/completion/workspace.py`:
- Around line 417-420: Update the process stream-reading flow around
_read_bounded and asyncio.gather to create explicit stdout and stderr tasks, and
on either reader’s failure cancel and await both tasks before completing
cleanup, while preserving process termination. Add a regression test covering an
exception from one reader and verifying the sibling reader is cancelled and
awaited.
---
Outside diff comments:
In `@src/pythinker_code/ui/shell/prompt.py`:
- Around line 3683-3692: Update _on_workspace_snapshot_published to return
unless the buffer already has an active complete_state, in addition to the
existing app-running and HostFileMentionCompleter checks. Only call
start_completion and app.invalidate when an existing file-completion menu is
still open.
- Around line 1785-1791: Update the synchronous __exit__ path in
CustomPromptSession to invoke the PromptLifecycle shutdown/close operation,
ensuring the registered workspace index closer (self._workspace_index.aclose)
runs during with-block exit. Preserve the existing status-task and statusline
cleanup, and align the synchronous behavior with __aexit__ without changing
asynchronous lifecycle handling.
In `@src/pythinker_code/ui/shell/prompting/completion/workspace.py`:
- Around line 183-187: Update the warning log in the workspace refresh failure
path to avoid interpolating the absolute root or raw exception. Replace them
with a stable, redacted workspace identifier and a safe categorized exception
type, and audit nearby logging in the same module for paths, command arguments,
or user-derived scopes that require the same redaction.
- Around line 95-100: Update Workspace.snapshot to return a status-bearing
result that distinguishes pending, unavailable/failed, degraded, successful, and
genuinely empty snapshots instead of collapsing them to an empty tuple. Preserve
immutable entries for successful results, and publish an explicit
failure/unavailable status when snapshot task scheduling fails. Update
HostFileMentionCompleter and other snapshot consumers to handle these states
without presenting incomplete or unavailable results as ordinary “no matches.”
- Around line 158-166: Update _snapshot_key() to normalize backslashes as path
separators, reject Windows drive-qualified and absolute scopes, and reject any
normalized scope containing parent traversal. In _discover_bounded(), enforce
that the resolved traversal path remains contained within the workspace root
before performing bounded traversal or git ls-files. Add a regression test
covering Windows-style traversal and drive-qualified fragments.
In `@tests/ui_and_conv/test_file_completer.py`:
- Around line 24-42: Update the _completion_results helper to wrap workspace
setup, refresh, and completion collection in a try/finally block, moving
lifecycle.aclose() into the finally section so it runs even when an earlier
operation raises. Preserve the existing completion return behavior on success.
---
Duplicate comments:
In `@src/pythinker_code/ui/shell/prompt.py`:
- Around line 1784-1791: Update the constructor’s existing HostPath.cwd() usage,
including the history-hashing setup and the workspace initialization around
_workspace_root, to resolve the current directory once inside the established
CwdLostError/OSError handling. Reuse that resolved root for both history and
WorkspaceIndex construction so no constructor path calls HostPath.cwd()
unguarded.
🪄 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
Run ID: 7e603fa5-06ac-4a78-ae46-55d23960a736
📒 Files selected for processing (9)
CHANGELOG.mdsrc/pythinker_code/ui/shell/prompt.pysrc/pythinker_code/ui/shell/prompting/completion/context.pysrc/pythinker_code/ui/shell/prompting/completion/workspace.pytests/ui_and_conv/_prompt_lifecycle.pytests/ui_and_conv/test_file_completer.pytests/ui_and_conv/test_slash_completer.pytests/ui_and_conv/test_slash_highlight.pytests/ui_and_conv/test_workspace_index.py
The concurrent stdout/stderr drain in _run_git used asyncio.gather, which leaves the sibling _read_bounded coroutine running after the first reader raises — only the process was killed, so the other reader could keep draining a closing pipe. Move the concurrent read into _read_streams, which uses explicit tasks and cancels+awaits both readers on any failure (or cancellation) before propagating. Adds a regression test for a reader-exception path.
1 participant
Part 3 of the prompt deep-module refactor series (docs/superpowers/plans/2026-07-19-tui-prompt-deep-module-refactor.md, Tasks 6–7). Stacked on #219.
Task 6 — One canonical completion context
prompting/completion/context.py:CompletionKind+ frozenCompletionContextwithparse_completion_context()as the single slash/file token parser; root-dropdown vs mid-line ghost activation preserved as facets of one parse.prompting/completion/slash.py:SlashCommandCompleter,SlashCommandAutoSuggest, and lexer slash decisions moved behind the context; no caller re-parses slash text.@"path with spaces"now parse and complete, retaining quotes and escaping embedded quotes/backslashes; unquoted mentions keep whitespace termination.prompt.pystays the compatibility façade: all previously public/tested names re-exported (pinned bytest_prompt_public_contract.py).Task 7 — Async, cwd-aware workspace index
prompting/completion/workspace.py: generation-ownedWorkspaceIndex(set_root/snapshot/request_refresh/aclose) over thepythinker_host.Hostinterface.snapshot()andget_completions()perform no I/O (pinned by test); stale-generation/stale-root scan results can never publish; Git failure and bounded traversal publish explicit degraded snapshots; results truncate at 1,000 entries.HostFileMentionCompleterreads snapshots only;LocalFileMentionCompleterkept as a façade alias withshould_completeunchanged.HostPath.cwd()before each prompt read; all discovery tasks are owned byPromptLifecyclewith cancellation and awaited shutdown.on_publishcallback restarts the open file-mention completion menu when a fresh snapshot lands, so async discovery results appear without retyping (caught bytests/e2e/test_file_mention_e2e.py, which passes unmodified).Verification
make check-pythinker-code: All checks passed (ruff + format + pyright + ty).make test-pythinker-code: full unit suite +tests_e2egreen (final phase 65 passed, 4 skipped).tests/ui_and_conv3062 passed;tests/e2e/test_file_mention_e2e.py4 passed.Summary by CodeRabbit
New Features
@file mentions, including quoted paths (e.g., paths with spaces) and inline highlighting.@file mention completion backed by refreshed snapshots.Bug Fixes
@dir/name).Documentation
caf.Tests