feat: unify rust terminal panels - #198
Merged
Merged
Conversation
Contributor
|
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 |
Contributor
|
This pull request description looks incomplete. Please update the missing sections below before review. Missing items:
|
📝 WalkthroughWalkthroughAdds a new aish-ui TUI framework, wires it into the workspace, persists session snapshots and context, implements an interactive /resume command (CLI + shell), and migrates selection dialogs to the new panel system. ChangesSession Resumption Feature
Sequence DiagramssequenceDiagram
participant User
participant CLI as aish-cli
participant Shell as AishShell
participant Handler as AiHandler
participant Store as SessionStore
User->>CLI: aish resume <session_id>
CLI->>Shell: AishShell::resume(config, session_id)
Shell->>Store: get_session(session_id)
Store-->>Shell: SessionRecord with state
Shell->>Handler: restore_session_context_snapshot
Handler-->>Shell: restored context
Shell->>Shell: validate cwd & update config
Shell->>Shell: restart_pty (may suppress notice)
Shell->>Store: update_session_state
Shell-->>CLI: REPL ready
CLI-->>User: interactive shell resumed
sequenceDiagram
participant App as AishShell
participant Handler as AiHandler
participant Store as SessionStore
App->>Handler: export_session_context_snapshot
Handler-->>App: Vec<SessionContextMessage>
App->>App: session_state_snapshot()
App->>Store: update_session_state(uuid, snapshot)
Store->>Store: serialize & UPDATE sessions.state
Note over App,Store: Called after AI response, command execution, and shutdown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/aish-session/src/store.rs (1)
220-237:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t return failure after a successful history insert.
add_history_entryinserts the row first, then fails iftouch_sessionfails (Line 236). That produces partial success and can trigger duplicate history entries if callers retry on error.💡 Proposed fix (make touch best-effort)
- self.touch_session(&entry.session_uuid, entry.created_at)?; + if let Err(err) = self.touch_session(&entry.session_uuid, entry.created_at) { + tracing::warn!( + session_uuid = %entry.session_uuid, + error = %err, + "history row inserted but failed to update session timestamp" + ); + }🤖 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-session/src/store.rs` around lines 220 - 237, The insert currently succeeds but any error from touch_session(&entry.session_uuid, entry.created_at) is propagated causing callers to see failure after a successful INSERT; change add_history_entry so the execute(...) error still returns on failure but make the call to touch_session best‑effort: call touch_session in a non-fatal way (e.g., match or if let Err(e) = self.touch_session(...){ log a warning with the error } ) and do not map/return that error to the caller so duplicate retries won’t reinsert history rows.crates/aish-shell/src/app.rs (1)
2216-2233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake PTY restart failures observable to callers.
This helper only logs and flips
self.state.should_exit. The resume path still persists the restored snapshot and prints the localized success message afterward, so a failed PTY restart is surfaced as a successful resume.🤖 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/app.rs` around lines 2216 - 2233, The helper restart_pty_with_notice currently swallows PTY startup errors (only logs and sets self.state.should_exit) so callers (e.g., the resume path) still continue and print a success; change restart_pty_with_notice to return a Result<(), E> or a bool success indicator instead of void (e.g., fn restart_pty_with_notice(&mut self, show_notice: bool) -> Result<(), anyhow::Error> or -> bool), propagate the error from aish_pty::PersistentPty::start instead of only printing it, and update all callers to check the return value and abort the resume/persist path (and avoid printing the localized success message) when restart fails; keep existing lock_pty(), error message text, and state mutation as needed but ensure the failure is observable to callers.
🤖 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-i18n/locales/de-DE.yaml`:
- Around line 226-242: The resume localization block is at the wrong YAML depth;
move the entire resume mapping so it is nested under the existing session key
(so runtime lookups like shell.session.resume.* resolve), and indent all child
keys of resume by two additional spaces (e.g., the block starting with resume:
and its children such as exit_command, usage, session_store_unavailable, etc.
should be placed under session: and properly indented).
In `@crates/aish-i18n/locales/en-US.yaml`:
- Around line 427-443: The resume localization block is incorrectly placed at
top-level `shell.resume` instead of under `shell.session`, causing lookup
misses; move the entire `resume:` mapping (all keys like exit_command, usage,
session_store_unavailable, etc.) so it is nested as `session.resume:` under the
existing `shell.session` mapping (i.e., place the `resume` block inside the
`session` key) to match runtime key lookups (`shell.session.resume.*`).
In `@crates/aish-i18n/locales/es-ES.yaml`:
- Around line 226-242: The translations for the resume flow are currently under
the top-level shell.resume block but must live under shell.session.resume;
move/indent the entire resume mapping (keys like exit_command, usage,
session_store_unavailable, no_sessions, recent_header, list_hint, list_failed,
not_found, cwd_missing, resumed, selector_title, search_placeholder, no_matches,
current_marker, selector_footer) so they are nested as shell -> session ->
resume (i.e., shell.session.resume.*) to match the code path that looks up
shell.session.resume.
In `@crates/aish-i18n/locales/fr-FR.yaml`:
- Around line 226-242: The resume block is misindented so keys like
"resume.exit_command" currently live under the wrong hierarchy; move the entire
"resume:" block (all child keys: exit_command, usage, session_store_unavailable,
no_sessions, recent_header, list_hint, list_failed, not_found, cwd_missing,
resumed, selector_title, search_placeholder, no_matches, current_marker,
selector_footer) two spaces deeper so they are nested under "session" (i.e.,
become "session.resume.*") to restore correct lookups like
"shell.session.resume.*".
In `@crates/aish-i18n/locales/ja-JP.yaml`:
- Around line 226-241: The `resume` block is nested at same level as `session`
so translations resolve as shell.resume.* instead of shell.session.resume.*;
move the entire `resume:` mapping (including keys like exit_command, usage,
session_store_unavailable, no_sessions, recent_header, list_hint, list_failed,
not_found, cwd_missing, resumed, selector_title, search_placeholder, no_matches,
current_marker, selector_footer) so it becomes a child under the existing
`session:` mapping (adjust indentation accordingly) and remove the old top-level
`resume:` entry so lookups resolve to shell.session.resume.*.
In `@crates/aish-i18n/locales/zh-CN.yaml`:
- Around line 427-442: The locale entries for "resume" are currently nested
under shell.resume.* instead of the expected shell.session.resume.* path; move
the entire "resume" block so it's nested under the existing "session" mapping
(i.e., rename/move the block to shell.session.resume) and fix indentation so
keys like exit_command, usage, session_store_unavailable, etc. live under
session.resume to match the lookup path used by the code.
In `@crates/aish-session/src/models.rs`:
- Around line 35-37: The current state_snapshot method silently returns Default
on JSON parse failure; change its signature from pub fn state_snapshot(&self) ->
SessionStateSnapshot to pub fn state_snapshot(&self) ->
Result<SessionStateSnapshot, serde_json::Error> and replace the
unwrap_or_default call with serde_json::from_value(self.state.clone()),
propagating the error to callers (update any call sites to handle the Result),
so parse failures are surfaced instead of being silently dropped.
In `@crates/aish-session/src/store.rs`:
- Around line 133-143: The two DELETEs in delete_session are not wrapped in a
transaction, leaving history deletion committed if the sessions delete fails;
wrap both execute calls in a single DB transaction (use self.conn.transaction()
-> tx) and perform the two deletes on the transaction (tx.execute(...)) and then
tx.commit(), mapping any transaction/execute errors to AishError::Session so
failures roll back and the operation is atomic; update references inside
delete_session accordingly (transaction, tx.execute, tx.commit).
In `@crates/aish-shell/src/app.rs`:
- Around line 1047-1056: The new transient session created by Self::new is left
in the SessionStore if resume_session_with_options fails; update pub fn resume
so that after creating shell and capturing transient_session_uuid, you call
resume_session_with_options in a match/if let Err(e) branch that, on error,
attempts to delete the transient session via
shell.session_store.delete_session(&transient_session_uuid) (if Some) and then
returns the original Err; keep the existing successful-path logic that compares
shell.session_uuid to transient_session_uuid and deletes the transient only when
a switch occurred.
In `@crates/aish-shell/src/resume_selector.rs`:
- Around line 95-105: The relative_time function returns hardcoded English
strings; update relative_time(DateTime<Utc>) to use the project's localization
utilities instead of literal strings: replace "now", "{}m ago", "{}h ago", and
"{}d ago" with localized message lookups (e.g., a t()/gettext-like function or
the existing i18n API used elsewhere in the UI) and pass the numeric values as
formatting parameters so translations can reorder or change wording; locate and
call the same localization helper used by the resume UI so relative_time
integrates with the app's locale.
In `@crates/aish-shell/src/tui.rs`:
- Around line 132-157: The custom-input label is hard-coded as
CUSTOM_INPUT_LABEL and must be localized: replace the const with a function
(e.g., fn custom_input_label() -> String) that returns the translated string via
your i18n API (call the project/local crate translate function, e.g.
aish_i18n::t("...") or equivalent), and update run_panel_selection to call
ChoicePanel::with_custom_label using that localized value (e.g., panel =
panel.with_custom_label(&custom_input_label()) or pass the String if
with_custom_label accepts one); ensure the new function is used wherever
CUSTOM_INPUT_LABEL was referenced so the label is routed through i18n.
In `@crates/aish-ui/src/runtime.rs`:
- Around line 87-90: In TerminalGuard::enter, if terminal::enable_raw_mode()
succeeds but execute!(io::stdout(), cursor::Hide) fails you must disable raw
mode before returning the error to avoid leaving the terminal in raw state;
modify enter() so after calling terminal::enable_raw_mode() you attempt the
execute! call and on any error call terminal::disable_raw_mode() (or otherwise
rollback) before propagating the error, ensuring the function still returns the
original io::Error from execute! while guaranteeing raw mode is turned off.
---
Outside diff comments:
In `@crates/aish-session/src/store.rs`:
- Around line 220-237: The insert currently succeeds but any error from
touch_session(&entry.session_uuid, entry.created_at) is propagated causing
callers to see failure after a successful INSERT; change add_history_entry so
the execute(...) error still returns on failure but make the call to
touch_session best‑effort: call touch_session in a non-fatal way (e.g., match or
if let Err(e) = self.touch_session(...){ log a warning with the error } ) and do
not map/return that error to the caller so duplicate retries won’t reinsert
history rows.
In `@crates/aish-shell/src/app.rs`:
- Around line 2216-2233: The helper restart_pty_with_notice currently swallows
PTY startup errors (only logs and sets self.state.should_exit) so callers (e.g.,
the resume path) still continue and print a success; change
restart_pty_with_notice to return a Result<(), E> or a bool success indicator
instead of void (e.g., fn restart_pty_with_notice(&mut self, show_notice: bool)
-> Result<(), anyhow::Error> or -> bool), propagate the error from
aish_pty::PersistentPty::start instead of only printing it, and update all
callers to check the return value and abort the resume/persist path (and avoid
printing the localized success message) when restart fails; keep existing
lock_pty(), error message text, and state mutation as needed but ensure the
failure is observable to callers.
🪄 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: 61edb815-f7e0-45e9-9194-602128874a7e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
Cargo.tomlcrates/aish-cli/src/main.rscrates/aish-i18n/locales/de-DE.yamlcrates/aish-i18n/locales/en-US.yamlcrates/aish-i18n/locales/es-ES.yamlcrates/aish-i18n/locales/fr-FR.yamlcrates/aish-i18n/locales/ja-JP.yamlcrates/aish-i18n/locales/zh-CN.yamlcrates/aish-session/Cargo.tomlcrates/aish-session/src/lib.rscrates/aish-session/src/models.rscrates/aish-session/src/store.rscrates/aish-shell/Cargo.tomlcrates/aish-shell/src/ai_handler.rscrates/aish-shell/src/app.rscrates/aish-shell/src/input.rscrates/aish-shell/src/lib.rscrates/aish-shell/src/resume_selector.rscrates/aish-shell/src/tui.rscrates/aish-shell/tests/shell_integration_test.rscrates/aish-tools/Cargo.tomlcrates/aish-tools/src/ask_user.rscrates/aish-ui/Cargo.tomlcrates/aish-ui/src/choice.rscrates/aish-ui/src/lib.rscrates/aish-ui/src/runtime.rscrates/aish-ui/src/select.rs
Comment on lines
+226
to
242
| resume: | ||
| exit_command: "Fortsetzen mit: aish resume {session_id}" | ||
| usage: "Verwendung: /resume [session_id]" | ||
| session_store_unavailable: "Sitzungsspeicher ist nicht verfügbar." | ||
| no_sessions: "Keine früheren Sitzungen gefunden." | ||
| recent_header: "Letzte Sitzungen (max. {limit}):" | ||
| list_hint: "Fortsetzen mit: /resume <session_id>" | ||
| list_failed: "Sitzungen konnten nicht aufgelistet werden: {error}" | ||
| not_found: "Sitzung nicht gefunden: {session_id}" | ||
| cwd_missing: "Das gespeicherte Arbeitsverzeichnis existiert nicht mehr; aktuelles Verzeichnis wird beibehalten: {cwd}" | ||
| resumed: "Sitzung fortgesetzt: {session_id}" | ||
| selector_title: "Sitzung fortsetzen" | ||
| search_placeholder: "Suchen..." | ||
| no_matches: "Keine passenden Sitzungen." | ||
| current_marker: "(aktuell)" | ||
| selector_footer: "Tippen zum Suchen · ↑/↓ zum Auswählen · Enter zum Fortsetzen · Esc zum Abbrechen" | ||
| ask_user: |
There was a problem hiding this comment.
resume localization block is at the wrong YAML depth.
At Line 226, resume is not nested under session, so runtime lookups for shell.session.resume.* will not resolve.
🔧 Proposed fix
- resume:
+ resume:(And indent all child keys under resume by two additional spaces.)
🤖 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-i18n/locales/de-DE.yaml` around lines 226 - 242, The resume
localization block is at the wrong YAML depth; move the entire resume mapping so
it is nested under the existing session key (so runtime lookups like
shell.session.resume.* resolve), and indent all child keys of resume by two
additional spaces (e.g., the block starting with resume: and its children such
as exit_command, usage, session_store_unavailable, etc. should be placed under
session: and properly indented).
Comment on lines
+427
to
+443
| resume: | ||
| exit_command: "Resume with: aish resume {session_id}" | ||
| usage: "Usage: /resume [session_id]" | ||
| session_store_unavailable: "Session store is not available." | ||
| no_sessions: "No previous sessions found." | ||
| recent_header: "Recent sessions ({limit} max):" | ||
| list_hint: "Resume with: /resume <session_id>" | ||
| list_failed: "Failed to list sessions: {error}" | ||
| not_found: "Session not found: {session_id}" | ||
| cwd_missing: "Saved cwd no longer exists, staying in current directory: {cwd}" | ||
| resumed: "Resumed session: {session_id}" | ||
| selector_title: "Resume session" | ||
| search_placeholder: "Search..." | ||
| no_matches: "No matching sessions." | ||
| current_marker: "(current)" | ||
| selector_footer: "Type to search · ↑/↓ to select · Enter to resume · Esc to cancel" | ||
|
|
There was a problem hiding this comment.
Nest resume under shell.session to match runtime key lookups.
Line 427 is currently aligned with session, so these keys become shell.resume.* instead of shell.session.resume.*, causing lookup misses for resume UI text.
🔧 Proposed fix
- resume:
- exit_command: "Resume with: aish resume {session_id}"
- usage: "Usage: /resume [session_id]"
- session_store_unavailable: "Session store is not available."
- no_sessions: "No previous sessions found."
- recent_header: "Recent sessions ({limit} max):"
- list_hint: "Resume with: /resume <session_id>"
- list_failed: "Failed to list sessions: {error}"
- not_found: "Session not found: {session_id}"
- cwd_missing: "Saved cwd no longer exists, staying in current directory: {cwd}"
- resumed: "Resumed session: {session_id}"
- selector_title: "Resume session"
- search_placeholder: "Search..."
- no_matches: "No matching sessions."
- current_marker: "(current)"
- selector_footer: "Type to search · ↑/↓ to select · Enter to resume · Esc to cancel"
+ resume:
+ exit_command: "Resume with: aish resume {session_id}"
+ usage: "Usage: /resume [session_id]"
+ session_store_unavailable: "Session store is not available."
+ no_sessions: "No previous sessions found."
+ recent_header: "Recent sessions ({limit} max):"
+ list_hint: "Resume with: /resume <session_id>"
+ list_failed: "Failed to list sessions: {error}"
+ not_found: "Session not found: {session_id}"
+ cwd_missing: "Saved cwd no longer exists, staying in current directory: {cwd}"
+ resumed: "Resumed session: {session_id}"
+ selector_title: "Resume session"
+ search_placeholder: "Search..."
+ no_matches: "No matching sessions."
+ current_marker: "(current)"
+ selector_footer: "Type to search · ↑/↓ to select · Enter to resume · Esc to cancel"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resume: | |
| exit_command: "Resume with: aish resume {session_id}" | |
| usage: "Usage: /resume [session_id]" | |
| session_store_unavailable: "Session store is not available." | |
| no_sessions: "No previous sessions found." | |
| recent_header: "Recent sessions ({limit} max):" | |
| list_hint: "Resume with: /resume <session_id>" | |
| list_failed: "Failed to list sessions: {error}" | |
| not_found: "Session not found: {session_id}" | |
| cwd_missing: "Saved cwd no longer exists, staying in current directory: {cwd}" | |
| resumed: "Resumed session: {session_id}" | |
| selector_title: "Resume session" | |
| search_placeholder: "Search..." | |
| no_matches: "No matching sessions." | |
| current_marker: "(current)" | |
| selector_footer: "Type to search · ↑/↓ to select · Enter to resume · Esc to cancel" | |
| resume: | |
| exit_command: "Resume with: aish resume {session_id}" | |
| usage: "Usage: /resume [session_id]" | |
| session_store_unavailable: "Session store is not available." | |
| no_sessions: "No previous sessions found." | |
| recent_header: "Recent sessions ({limit} max):" | |
| list_hint: "Resume with: /resume <session_id>" | |
| list_failed: "Failed to list sessions: {error}" | |
| not_found: "Session not found: {session_id}" | |
| cwd_missing: "Saved cwd no longer exists, staying in current directory: {cwd}" | |
| resumed: "Resumed session: {session_id}" | |
| selector_title: "Resume session" | |
| search_placeholder: "Search..." | |
| no_matches: "No matching sessions." | |
| current_marker: "(current)" | |
| selector_footer: "Type to search · ↑/↓ to select · Enter to resume · Esc to cancel" |
🤖 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-i18n/locales/en-US.yaml` around lines 427 - 443, The resume
localization block is incorrectly placed at top-level `shell.resume` instead of
under `shell.session`, causing lookup misses; move the entire `resume:` mapping
(all keys like exit_command, usage, session_store_unavailable, etc.) so it is
nested as `session.resume:` under the existing `shell.session` mapping (i.e.,
place the `resume` block inside the `session` key) to match runtime key lookups
(`shell.session.resume.*`).
Comment on lines
+226
to
242
| resume: | ||
| exit_command: "Reanudar con: aish resume {session_id}" | ||
| usage: "Uso: /resume [session_id]" | ||
| session_store_unavailable: "El almacén de sesiones no está disponible." | ||
| no_sessions: "No se encontraron sesiones anteriores." | ||
| recent_header: "Sesiones recientes ({limit} máx.):" | ||
| list_hint: "Reanudar con: /resume <session_id>" | ||
| list_failed: "No se pudieron listar las sesiones: {error}" | ||
| not_found: "Sesión no encontrada: {session_id}" | ||
| cwd_missing: "El directorio de trabajo guardado ya no existe; se conserva el directorio actual: {cwd}" | ||
| resumed: "Sesión reanudada: {session_id}" | ||
| selector_title: "Reanudar sesión" | ||
| search_placeholder: "Buscar..." | ||
| no_matches: "No hay sesiones coincidentes." | ||
| current_marker: "(actual)" | ||
| selector_footer: "Escribe para buscar · ↑/↓ para seleccionar · Enter para reanudar · Esc para cancelar" | ||
| ask_user: |
There was a problem hiding this comment.
Place resume under shell.session (not shell).
Line 226 currently defines shell.resume.*; resume flow expects shell.session.resume.*, so these translations won’t be found.
🔧 Proposed fix
- resume:
+ resume:(Indent the nested resume entries accordingly.)
🤖 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-i18n/locales/es-ES.yaml` around lines 226 - 242, The translations
for the resume flow are currently under the top-level shell.resume block but
must live under shell.session.resume; move/indent the entire resume mapping
(keys like exit_command, usage, session_store_unavailable, no_sessions,
recent_header, list_hint, list_failed, not_found, cwd_missing, resumed,
selector_title, search_placeholder, no_matches, current_marker, selector_footer)
so they are nested as shell -> session -> resume (i.e., shell.session.resume.*)
to match the code path that looks up shell.session.resume.
Comment on lines
+226
to
242
| resume: | ||
| exit_command: "Reprendre avec : aish resume {session_id}" | ||
| usage: "Utilisation : /resume [session_id]" | ||
| session_store_unavailable: "Le stockage des sessions n'est pas disponible." | ||
| no_sessions: "Aucune session precedente trouvee." | ||
| recent_header: "Sessions recentes ({limit} max) :" | ||
| list_hint: "Reprendre avec : /resume <session_id>" | ||
| list_failed: "Impossible de lister les sessions : {error}" | ||
| not_found: "Session introuvable : {session_id}" | ||
| cwd_missing: "Le repertoire de travail enregistre n'existe plus ; conservation du repertoire courant : {cwd}" | ||
| resumed: "Session reprise : {session_id}" | ||
| selector_title: "Reprendre une session" | ||
| search_placeholder: "Rechercher..." | ||
| no_matches: "Aucune session correspondante." | ||
| current_marker: "(actuelle)" | ||
| selector_footer: "Tapez pour rechercher · ↑/↓ pour selectionner · Enter pour reprendre · Esc pour annuler" | ||
| ask_user: |
There was a problem hiding this comment.
Fix YAML hierarchy for resume locale keys.
Line 226 should be nested under session; current indentation creates shell.resume.* keys and breaks shell.session.resume.* lookups.
🔧 Proposed fix
- resume:
+ resume:(Plus two-space indentation shift for all resume child keys.)
🤖 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-i18n/locales/fr-FR.yaml` around lines 226 - 242, The resume block
is misindented so keys like "resume.exit_command" currently live under the wrong
hierarchy; move the entire "resume:" block (all child keys: exit_command, usage,
session_store_unavailable, no_sessions, recent_header, list_hint, list_failed,
not_found, cwd_missing, resumed, selector_title, search_placeholder, no_matches,
current_marker, selector_footer) two spaces deeper so they are nested under
"session" (i.e., become "session.resume.*") to restore correct lookups like
"shell.session.resume.*".
Comment on lines
+226
to
+241
| resume: | ||
| exit_command: "復元するには: aish resume {session_id}" | ||
| usage: "使い方: /resume [session_id]" | ||
| session_store_unavailable: "セッションストアを利用できません。" | ||
| no_sessions: "以前のセッションは見つかりませんでした。" | ||
| recent_header: "最近のセッション（最大 {limit} 件）:" | ||
| list_hint: "復元するには: /resume <session_id>" | ||
| list_failed: "セッション一覧の取得に失敗しました: {error}" | ||
| not_found: "セッションが見つかりません: {session_id}" | ||
| cwd_missing: "保存された作業ディレクトリは存在しません。現在のディレクトリに留まります: {cwd}" | ||
| resumed: "セッションを復元しました: {session_id}" | ||
| selector_title: "セッションを復元" | ||
| search_placeholder: "検索..." | ||
| no_matches: "一致するセッションはありません。" | ||
| current_marker: "(現在)" | ||
| selector_footer: "入力して検索 · ↑/↓ で選択 · Enter で復元 · Esc でキャンセル" |
There was a problem hiding this comment.
resume keys are nested at the wrong path.
resume is currently a sibling of session, so these strings resolve as shell.resume.* instead of shell.session.resume.*. This will break resume-flow localization lookups.
Proposed fix
- resume:
- exit_command: "復元するには: aish resume {session_id}"
- usage: "使い方: /resume [session_id]"
- session_store_unavailable: "セッションストアを利用できません。"
- no_sessions: "以前のセッションは見つかりませんでした。"
- recent_header: "最近のセッション（最大 {limit} 件）:"
- list_hint: "復元するには: /resume <session_id>"
- list_failed: "セッション一覧の取得に失敗しました: {error}"
- not_found: "セッションが見つかりません: {session_id}"
- cwd_missing: "保存された作業ディレクトリは存在しません。現在のディレクトリに留まります: {cwd}"
- resumed: "セッションを復元しました: {session_id}"
- selector_title: "セッションを復元"
- search_placeholder: "検索..."
- no_matches: "一致するセッションはありません。"
- current_marker: "(現在)"
- selector_footer: "入力して検索 · ↑/↓ で選択 · Enter で復元 · Esc でキャンセル"
+ resume:
+ exit_command: "復元するには: aish resume {session_id}"
+ usage: "使い方: /resume [session_id]"
+ session_store_unavailable: "セッションストアを利用できません。"
+ no_sessions: "以前のセッションは見つかりませんでした。"
+ recent_header: "最近のセッション（最大 {limit} 件）:"
+ list_hint: "復元するには: /resume <session_id>"
+ list_failed: "セッション一覧の取得に失敗しました: {error}"
+ not_found: "セッションが見つかりません: {session_id}"
+ cwd_missing: "保存された作業ディレクトリは存在しません。現在のディレクトリに留まります: {cwd}"
+ resumed: "セッションを復元しました: {session_id}"
+ selector_title: "セッションを復元"
+ search_placeholder: "検索..."
+ no_matches: "一致するセッションはありません。"
+ current_marker: "(現在)"
+ selector_footer: "入力して検索 · ↑/↓ で選択 · Enter で復元 · Esc でキャンセル"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resume: | |
| exit_command: "復元するには: aish resume {session_id}" | |
| usage: "使い方: /resume [session_id]" | |
| session_store_unavailable: "セッションストアを利用できません。" | |
| no_sessions: "以前のセッションは見つかりませんでした。" | |
| recent_header: "最近のセッション（最大 {limit} 件）:" | |
| list_hint: "復元するには: /resume <session_id>" | |
| list_failed: "セッション一覧の取得に失敗しました: {error}" | |
| not_found: "セッションが見つかりません: {session_id}" | |
| cwd_missing: "保存された作業ディレクトリは存在しません。現在のディレクトリに留まります: {cwd}" | |
| resumed: "セッションを復元しました: {session_id}" | |
| selector_title: "セッションを復元" | |
| search_placeholder: "検索..." | |
| no_matches: "一致するセッションはありません。" | |
| current_marker: "(現在)" | |
| selector_footer: "入力して検索 · ↑/↓ で選択 · Enter で復元 · Esc でキャンセル" | |
| resume: | |
| exit_command: "復元するには: aish resume {session_id}" | |
| usage: "使い方: /resume [session_id]" | |
| session_store_unavailable: "セッションストアを利用できません。" | |
| no_sessions: "以前のセッションは見つかりませんでした。" | |
| recent_header: "最近のセッション（最大 {limit} 件）:" | |
| list_hint: "復元するには: /resume <session_id>" | |
| list_failed: "セッション一覧の取得に失敗しました: {error}" | |
| not_found: "セッションが見つかりません: {session_id}" | |
| cwd_missing: "保存された作業ディレクトリは存在しません。現在のディレクトリに留まります: {cwd}" | |
| resumed: "セッションを復元しました: {session_id}" | |
| selector_title: "セッションを復元" | |
| search_placeholder: "検索..." | |
| no_matches: "一致するセッションはありません。" | |
| current_marker: "(現在)" | |
| selector_footer: "入力して検索 · ↑/↓ で選択 · Enter で復元 · Esc でキャンセル" |
🤖 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-i18n/locales/ja-JP.yaml` around lines 226 - 241, The `resume`
block is nested at same level as `session` so translations resolve as
shell.resume.* instead of shell.session.resume.*; move the entire `resume:`
mapping (including keys like exit_command, usage, session_store_unavailable,
no_sessions, recent_header, list_hint, list_failed, not_found, cwd_missing,
resumed, selector_title, search_placeholder, no_matches, current_marker,
selector_footer) so it becomes a child under the existing `session:` mapping
(adjust indentation accordingly) and remove the old top-level `resume:` entry so
lookups resolve to shell.session.resume.*.
Comment on lines
+95
to
+105
| fn relative_time(updated_at: DateTime<Utc>) -> String { | ||
| let elapsed = Utc::now().signed_duration_since(updated_at); | ||
| if elapsed.num_seconds() < 60 { | ||
| "now".to_string() | ||
| } else if elapsed.num_minutes() < 60 { | ||
| format!("{}m ago", elapsed.num_minutes()) | ||
| } else if elapsed.num_hours() < 24 { | ||
| format!("{}h ago", elapsed.num_hours()) | ||
| } else { | ||
| format!("{}d ago", elapsed.num_days()) | ||
| } |
There was a problem hiding this comment.
Localize the relative-time strings.
This feeds directly into the selector detail line, but "now", "m ago", "h ago", and "d ago" are hardcoded English. The rest of the resume UI is localized, so non-English shells will still show mixed-language results here.
🤖 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/resume_selector.rs` around lines 95 - 105, The
relative_time function returns hardcoded English strings; update
relative_time(DateTime<Utc>) to use the project's localization utilities instead
of literal strings: replace "now", "{}m ago", "{}h ago", and "{}d ago" with
localized message lookups (e.g., a t()/gettext-like function or the existing
i18n API used elsewhere in the UI) and pass the numeric values as formatting
parameters so translations can reorder or change wording; locate and call the
same localization helper used by the resume UI so relative_time integrates with
the app's locale.
Comment on lines
132
to
+157
| /// Label used for the custom-input entry in the select list. | ||
| const CUSTOM_INPUT_LABEL: &str = "(type custom answer)"; | ||
|
|
||
| fn run_inquire_selection( | ||
| fn run_panel_selection( | ||
| title: &str, | ||
| question: &str, | ||
| options: &[DialogOption], | ||
| allow_custom: bool, | ||
| allow_cancel: bool, | ||
| ) -> Result<DialogResult, inquire::InquireError> { | ||
| use inquire::Select; | ||
|
|
||
| // Build display items. Each item is either a real option or the custom slot. | ||
| #[derive(Clone)] | ||
| enum Item { | ||
| Real(usize), // index into `options` | ||
| Custom, // custom input slot | ||
| } | ||
|
|
||
| let mut items: Vec<(String, Item)> = options | ||
| ) -> Result<DialogResult, aish_ui::PanelError> { | ||
| let items: Vec<SearchSelectItem> = options | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(i, opt)| (opt.display_label(), Item::Real(i))) | ||
| .map(|opt| { | ||
| let mut item = SearchSelectItem::new(opt.value.clone(), opt.label.clone()); | ||
| if let Some(description) = &opt.description { | ||
| item = item.with_detail(description.clone()); | ||
| } | ||
| item.with_search_text(opt.display_label()) | ||
| }) | ||
| .collect(); | ||
|
|
||
| let mut panel = ChoicePanel::new(title, question, items) | ||
| .with_allow_cancel(allow_cancel) | ||
| .with_footer(ChoicePanel::default_footer(allow_cancel)); | ||
| if allow_custom { | ||
| items.push((CUSTOM_INPUT_LABEL.to_string(), Item::Custom)); | ||
| panel = panel.with_custom_label(CUSTOM_INPUT_LABEL); |
There was a problem hiding this comment.
Route the custom-input label through i18n.
CUSTOM_INPUT_LABEL is now rendered inside the shared panel whenever allow_custom is enabled, so localized shells will show an English-only affordance in that flow.
🤖 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/tui.rs` around lines 132 - 157, The custom-input label
is hard-coded as CUSTOM_INPUT_LABEL and must be localized: replace the const
with a function (e.g., fn custom_input_label() -> String) that returns the
translated string via your i18n API (call the project/local crate translate
function, e.g. aish_i18n::t("...") or equivalent), and update
run_panel_selection to call ChoicePanel::with_custom_label using that localized
value (e.g., panel = panel.with_custom_label(&custom_input_label()) or pass the
String if with_custom_label accepts one); ensure the new function is used
wherever CUSTOM_INPUT_LABEL was referenced so the label is routed through i18n.
There was a problem hiding this comment.
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 `@crates/aish-shell/src/app.rs`:
- Around line 4792-4795: The function collapse_terminal_paragraph_gap currently
unconditionally writes ANSI escape sequences to stdout; guard this so it only
emits those when stdout is an interactive TTY. Modify
collapse_terminal_paragraph_gap to check terminal interactivity (e.g.
atty::is(atty::Stream::Stdout) or an equivalent isatty call) and only call
print! and flush when that check is true; otherwise do nothing. Ensure the
needed atty import is added where collapse_terminal_paragraph_gap is defined.
In `@crates/aish-ui/src/select.rs`:
- Around line 297-301: Backspace handler resets self.selected to 0 even when
self.query is empty; change the logic in the KeyCode::Backspace branch so you
only mutate self.query and reset self.selected when there was actually a
character to remove (e.g., check self.query.is_empty() or use the return value
of self.query.pop()); if pop() returns None (noop) leave self.selected unchanged
and just return PanelEvent::Continue. Ensure you update the KeyCode::Backspace
branch in select.rs accordingly.
🪄 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: 4098598e-c9bd-449a-9605-6f3e1740776b
📒 Files selected for processing (3)
crates/aish-shell/src/app.rscrates/aish-shell/src/resume_selector.rscrates/aish-ui/src/select.rs
Comment on lines
+4792
to
+4795
| fn collapse_terminal_paragraph_gap() { | ||
| print!("\x1b[1A\r\x1b[K"); | ||
| let _ = std::io::stdout().flush(); | ||
| } |
There was a problem hiding this comment.
Gate ANSI gap-collapsing to interactive terminals only.
Line 4792 always writes raw escape sequences; when stdout is redirected, those bytes end up in files/pipes.
Suggested fix
-use std::io::{self, Write};
+use std::io::{self, IsTerminal, Write};
...
fn collapse_terminal_paragraph_gap() {
+ if !std::io::stdout().is_terminal() {
+ return;
+ }
print!("\x1b[1A\r\x1b[K");
let _ = std::io::stdout().flush();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn collapse_terminal_paragraph_gap() { | |
| print!("\x1b[1A\r\x1b[K"); | |
| let _ = std::io::stdout().flush(); | |
| } | |
| fn collapse_terminal_paragraph_gap() { | |
| if !std::io::stdout().is_terminal() { | |
| return; | |
| } | |
| print!("\x1b[1A\r\x1b[K"); | |
| let _ = std::io::stdout().flush(); | |
| } |
🤖 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/app.rs` around lines 4792 - 4795, The function
collapse_terminal_paragraph_gap currently unconditionally writes ANSI escape
sequences to stdout; guard this so it only emits those when stdout is an
interactive TTY. Modify collapse_terminal_paragraph_gap to check terminal
interactivity (e.g. atty::is(atty::Stream::Stdout) or an equivalent isatty call)
and only call print! and flush when that check is true; otherwise do nothing.
Ensure the needed atty import is added where collapse_terminal_paragraph_gap is
defined.
Comment on lines
+297
to
+301
| KeyCode::Backspace => { | ||
| self.query.pop(); | ||
| self.selected = 0; | ||
| PanelEvent::Continue | ||
| } |
There was a problem hiding this comment.
Keep cursor stable on noop Backspace.
On Line 297, Backspace resets selection to the first row even when query is already empty, which causes unexpected jump-to-top while navigating.
Suggested fix
KeyCode::Backspace => {
- self.query.pop();
- self.selected = 0;
+ if self.query.pop().is_some() {
+ self.selected = 0;
+ self.clamp_selection();
+ }
PanelEvent::Continue
}🤖 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-ui/src/select.rs` around lines 297 - 301, Backspace handler
resets self.selected to 0 even when self.query is empty; change the logic in the
KeyCode::Backspace branch so you only mutate self.query and reset self.selected
when there was actually a character to remove (e.g., check self.query.is_empty()
or use the return value of self.query.pop()); if pop() returns None (noop) leave
self.selected unchanged and just return PanelEvent::Continue. Ensure you update
the KeyCode::Backspace branch in select.rs accordingly.
4 tasks
This was referenced Jul 17, 2026
This was referenced Jul 28, 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.
Background
The Rust implementation had terminal selection flows implemented in multiple places. This PR introduces a shared Ratatui/Crossterm panel layer so
/resume, local selection dialogs, andask_userchoices can reuse the same terminal lifecycle and interaction primitives.Changes
aish-uicrate with a reusablePanelRuntime,SearchSelectPanel, andChoicePanel./resumesession selection to the shared searchable panel.ask_userchoice/text prompts to the dedicated choice panel.Validation
cargo fmt -p aish-ui -p aish-shell -p aish-toolscargo test -p aish-uicargo test -p aish-shell tuicargo test -p aish-shell resume_selectorcargo build -p aish-cliNote: validation currently surfaces an existing
aish-ptyunused variable warning forup_moves.Risk
Medium. This touches shared terminal UI paths and raw-mode lifecycle, but the runtime is isolated in
aish-uiand the migrated call sites preserve their fallback stdin behavior.ask_usercancellation semantics remain a follow-up because cancellation is currently handled as a normal successful tool result by the outer LLM loop.Summary by CodeRabbit
New Features
Improvements
Tests