feat(shell): inline AI completion with ghost text - #338
Conversation
Copilot-style inline completion for AI-mode prompts (; or ； prefix): - New InlineCompletionConfig block in aish-config (enabled, debounce_ms, context_lines, max_tokens, min_input_chars, timeout_secs) - New inline_completion module in aish-shell: LLM-backed suffix prediction, gray ghost-text rendering via DECSC/DECRC, debounce + cancellation, faux provider for testing - PromptSpinner: bounces-dot animation replacing <aish> badge during LLM calls, with locale-aware CJK width calculation (unicode-width cjk feature) so lines_up stays correct on CJK terminals - AcceptInlineHintHandler (Right/Ctrl+F) and ClearGhostOnSubmitHandler wired into rustyline key bindings - LlmClient: model field made Mutex for runtime /model switching; new chat_completion_with_extras for provider-specific flags - Suffix width capping to prevent ghost wrapping; full suffix shown when even the first char cannot fit (wraps to next line rather than suppressing) - AutoSuggest::recent_n for context building; extract_ai_question / is_ai_prompt_line helpers in input module
|
Thanks for the pull request. A maintainer will review it when available. Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review. Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md |
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds inline AI ghost-text completion to the shell, with config and client support, locale-aware width handling, completion-provider logic, and readline/app integration. ChangesInline AI completion feature
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/aish-shell/src/ai_handler.rs (1)
1004-1021: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffBrute-force JSON scan now reused on inline-completion path.
This is now
pub(crate)for reuse by the inline completion provider (per stack layer 5), but the implementation is O(n·m) in the worst case — it iterates every{/}combination via nestedmatch_indices. For large or brace-heavy LLM responses on the debounced ghost-text path, this could add latency to an interactive typing flow.Consider bounding the scan (e.g., stop at the first successfully-balanced brace via a stack-based scan, or cap slice length) if reused in the completion hot path.
🤖 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 `@crates/aish-shell/src/ai_handler.rs` around lines 1004 - 1021, The reusable extract_json_object_from_text helper is now on an interactive inline-completion path, but its nested match_indices scan is O(n·m) and can add latency on large brace-heavy responses. Update extract_json_object_from_text to use a bounded, linear-time approach (for example, a stack-based brace scan that stops at the first balanced candidate, or a maximum slice length) so the inline completion provider can safely reuse it without worst-case slowdowns.crates/aish-llm/src/client.rs (1)
131-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood delegation pattern; consider adding a test for extras merging.
chat_completioncorrectly delegates tochat_completion_with_extraswith an empty map, preserving prior behavior. The extras-merging logic (lines 178-182) silently overwrites any conflicting top-level keys (e.g.model,messages,temperature) if present inextras— expected/intended for vendor overrides, but there's no test exercisingchat_completion_with_extraswith a non-emptyextrasmap to lock in this merge behavior.✅ Example test to add
#[test] fn test_extras_are_merged_into_body_shape() { // Build a body via chat_completion_with_extras with a custom "reasoning_effort" // extra and assert it appears in the serialized JSON (requires exposing a // body-builder helper or a lightweight mock transport). }🤖 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 `@crates/aish-llm/src/client.rs` around lines 131 - 182, The delegation in chat_completion is fine, but chat_completion_with_extras needs a test that locks in its extras merge behavior. Add coverage around the body-building path in chat_completion_with_extras to verify a non-empty extras map is merged into the request payload and that vendor-specific fields are preserved. Use the chat_completion_with_extras and chat_completion symbols to locate the logic, and assert the serialized JSON includes the injected extra key(s) without regressing the existing empty-map delegation from chat_completion.crates/aish-shell/src/inline_completion.rs (2)
331-335: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueLayer‑3 escape decoding is order‑dependent and can corrupt a literal
\n.Because
\\→\runs before\n→ newline, an input containing a literal backslash‑n (JSON\\n, i.e. two chars\+n) is first collapsed to\nand then turned into an actual newline. This only affects the regex last‑resort path, so impact is small, but the result is wrong for that edge case. Consider a single left‑to‑right scan that consumes one escape at a time.🤖 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 `@crates/aish-shell/src/inline_completion.rs` around lines 331 - 335, The Layer-3 escape decoding in the inline completion fallback is order-dependent and can turn a literal backslash-n into an actual newline. Update the unescaping logic in inline_completion.rs, specifically the code that builds the unescaped string from raw, so it decodes escapes in a single left-to-right pass instead of chaining replacements; this should preserve literal `\n` while still handling quotes, backslashes, newlines, and tabs correctly.
246-263: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRetry fires on every error, not just field‑rejection (HTTP 400).
The comment scopes the bare retry to gateways rejecting unknown fields, but the
Err(e)arm retries on any failure (timeouts, 5xx, network). The outer poll loop still bounds total time byhard_cap, so this is not a correctness/hang risk — but on transient failures it doubles upstream load for no benefit. IfAishErrorexposes a status/kind, consider gating the bare retry on the 4xx case only.🤖 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 `@crates/aish-shell/src/inline_completion.rs` around lines 246 - 263, The retry in inline_completion::InlineCompletion::request is currently triggered for every error from the extras-enabled request, not only for unknown-field rejections. Update the Err(e) branch to inspect the AishError coming back from request and only fall back to the bare request when it represents the expected HTTP 400/4xx field-rejection case; for all other failures, return the original error without retrying. Keep the existing tracing::debug! message for the gated fallback path.
🤖 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 `@crates/aish-config/src/model.rs`:
- Around line 251-254: The comment above the disable_thinking default in
model.rs is stale and contradicts the actual behavior and struct-level docs.
Update or remove that inline comment so it matches the real default of false,
and keep it consistent with the disable_thinking field in the model config and
the existing cfg.disable_thinking test assertion.
---
Nitpick comments:
In `@crates/aish-llm/src/client.rs`:
- Around line 131-182: The delegation in chat_completion is fine, but
chat_completion_with_extras needs a test that locks in its extras merge
behavior. Add coverage around the body-building path in
chat_completion_with_extras to verify a non-empty extras map is merged into the
request payload and that vendor-specific fields are preserved. Use the
chat_completion_with_extras and chat_completion symbols to locate the logic, and
assert the serialized JSON includes the injected extra key(s) without regressing
the existing empty-map delegation from chat_completion.
In `@crates/aish-shell/src/ai_handler.rs`:
- Around line 1004-1021: The reusable extract_json_object_from_text helper is
now on an interactive inline-completion path, but its nested match_indices scan
is O(n·m) and can add latency on large brace-heavy responses. Update
extract_json_object_from_text to use a bounded, linear-time approach (for
example, a stack-based brace scan that stops at the first balanced candidate, or
a maximum slice length) so the inline completion provider can safely reuse it
without worst-case slowdowns.
In `@crates/aish-shell/src/inline_completion.rs`:
- Around line 331-335: The Layer-3 escape decoding in the inline completion
fallback is order-dependent and can turn a literal backslash-n into an actual
newline. Update the unescaping logic in inline_completion.rs, specifically the
code that builds the unescaped string from raw, so it decodes escapes in a
single left-to-right pass instead of chaining replacements; this should preserve
literal `\n` while still handling quotes, backslashes, newlines, and tabs
correctly.
- Around line 246-263: The retry in inline_completion::InlineCompletion::request
is currently triggered for every error from the extras-enabled request, not only
for unknown-field rejections. Update the Err(e) branch to inspect the AishError
coming back from request and only fall back to the bare request when it
represents the expected HTTP 400/4xx field-rejection case; for all other
failures, return the original error without retrying. Keep the existing
tracing::debug! message for the gated fallback 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 211ad9ab-7968-4550-94a7-9c75c1a975a4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlcrates/aish-config/src/lib.rscrates/aish-config/src/model.rscrates/aish-llm/src/client.rscrates/aish-shell/Cargo.tomlcrates/aish-shell/src/ai_handler.rscrates/aish-shell/src/app.rscrates/aish-shell/src/autosuggest.rscrates/aish-shell/src/inline_completion.rscrates/aish-shell/src/input.rscrates/aish-shell/src/lib.rscrates/aish-shell/src/prompt.rscrates/aish-shell/src/readline.rs
…ding - model.rs: fix comment that said 'Default ON' when the value is false - inline_completion.rs: replace chained .replace() unescape with a single-pass scanner so literal \n (backslash+n) is not mistaken for a newline escape
1 participant
Summary
Copilot-style inline completion for AI-mode prompts (
;or；prefix). As the user types an AI question, a gray ghost-text suffix is predicted via LLM and rendered inline. Right Arrow or Ctrl+F accepts.Changes
InlineCompletionConfigblock (enabled, debounce_ms, context_lines, max_tokens, min_input_chars, timeout_secs)inline_completionmodule — LLM-backed suffix prediction, gray ghost-text rendering via DECSC/DECRC, debounce + cancellation, faux provider for testing<aish>badge during LLM calls, with locale-aware CJK width calculation (unicode-widthcjk feature) so cursor navigation stays correct on CJK terminalsMutexfor runtime/modelswitching; newchat_completion_with_extrasfor provider-specific flags (thinking suppression, JSON mode)Test plan
cargo test -p aish-shell --lib inline_completion— 42 unit tests passcargo test -p aish-shell --lib prompt::tests— 29 tests passcargo clippy -D warningscleancargo fmt --checkclean；现在开始分析, verify ghost text appears, Right accepts<aish>Summary by CodeRabbit