Skip to content

feat(chat): in-chat session management + chat UX bug fixes#8

Open
kyluke wants to merge 2 commits into
mainfrom
chat-session-ux
Open

feat(chat): in-chat session management + chat UX bug fixes#8
kyluke wants to merge 2 commits into
mainfrom
chat-session-ux

Conversation

@kyluke

@kyluke kyluke commented Jun 25, 2026

Copy link
Copy Markdown
Member

Summary

A deep-dive overhaul of the interactive termai chat experience, anchored on session management and backed by an exhaustive bug hunt of the chat/session subsystem.

The headline fix: ad-hoc sessions actually save now

A chat started without --session was a temporary session whose flag was never cleared and whose row was never inserted — so /save, per-turn autosave, and exit-autosave all printed "💾 saved" while writing zero rows. Rebuilt around a promote_session path (INSERT row → flip temporary → flush messages).

New in-chat session management

  • /sessions (/ls, /list) — list saved sessions with message counts + active marker
  • /new [name] (/n) — swap history out to a blank slate (auto-saves the outgoing one)
  • /load <name> (/switch, /open) — swap another session's history in, mid-chat
  • /rename <name> — rename the active session
  • /save [name] — fixed: promotes/renames with a name-collision guard
  • Session-aware prompt (refactor-plan* ❯, * = unsaved) + welcome banner showing session/model/message-count
  • Auto-save-never-lose-data on /new / /load / /exit

Correctness bugs fixed along the way

  • Message duplication — assigned IDs are written back, so named sessions stop re-inserting their whole history every turn (was quadratic + scrambled reload)
  • Redacted text persisted — now unredacts before saving (resumed sessions show real content, not <REDACTED>)
  • Context bloat — file context is injected transiently into the wire payload, never baked into stored messages
  • termai chat "question" now runs the question; --system-prompt is now seeded
  • Ctrl+C cancels a hung generation; empty-Enter no longer disarms "press again to exit"
  • /clear deletes stored rows; /retry no longer deletes the user turn or double-injects context
  • /streaming toggles and /settings reports real state; /tools refuses non-OpenAI; honest /branch
  • History file moved to ~/.config/termai/; messages ORDER BY rowid; removed a debug println! leak and a no-op with_env_fallbacks; unknown slash commands report a hint

Test plan

  • cargo build (enforced warnings = "deny" gate) — clean
  • ✅ 246 unit tests + 5 new service-layer tests (persistence/dedup/load/rename) + 5 new E2E tests driving the real REPL over stdin
  • ✅ All existing integration suites pass

Decisions baked in

  • In-chat /model & /provider intentionally still write the global config (unchanged)
  • Unsaved conversations auto-save (never lose data)
  • No schema migration needed; all new service fns live in sessions_service.rs

Out of scope (deferred)

unicode-width box alignment, wiring --smart-context into chat, theme persistence. Pre-existing repo-wide cargo clippy failures and the cargo fmt issue in src/commands/config.rs were left untouched.

🤖 Generated with Claude Code

Rebuild the chat session experience around a correct persistence model
and add in-chat session management.

Persistence model:
- promote_session: temporary (no --session) chats now actually save. The
  former /save / autosave / exit-autosave silently wrote zero rows because
  the temporary flag was never cleared and the row was never inserted.
- session_add_messages takes &mut Session and writes assigned message ids
  back, fixing quadratic message duplication on named sessions.
- Persist unredacted content (unredact before save); messages order by rowid.

New in-chat commands: /sessions (/ls,/list), /new (/n), /load (/switch,/open),
/rename, fixed /save (promote + collision guard) and /clear (deletes stored
rows). Session-aware prompt + welcome banner; auto-save-never-lose-data on
/new, /load and /exit.

Bug fixes: feed `chat "question"` initial input; seed --system-prompt;
Ctrl+C cancels in-flight generation; empty-Enter no longer disarms exit;
/retry keeps the user turn and stops double-injecting context; context is
injected transiently (no stored-message bloat); /streaming toggles and
/settings reports real state; /tools refuses non-OpenAI; honest /branch;
history file moved to ~/.config/termai; removed unredact debug println and
no-op with_env_fallbacks; unknown slash commands report a hint.

Tests: 5 service-layer tests for the persistence/dedup/load/rename fixes and
5 E2E tests driving the real REPL over stdin.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR overhauls the termai chat REPL to support in-chat session management (create/list/load/rename/save/clear) and fixes several persistence/UX correctness issues (temporary session saving, message deduplication, context/redaction persistence behavior, Ctrl+C cancellation, history file location).

Changes:

  • Added in-chat session workflow commands (/sessions, /new, /load, /rename) plus supporting service-layer APIs (promote_session, load_session, rename_session, etc.).
  • Fixed persistence correctness: incremental message flushing writes IDs back to in-memory messages (prevents reinserts/duplication) and avoids storing redacted/context-augmented content.
  • Improved chat UX/ops: cancellable generations via Ctrl+C, moved rustyline history to ~/.config/termai/, added banner/prompt session indicators, and added E2E coverage for session commands.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/chat_session_integration.rs New end-to-end REPL-driven tests for session commands and SQLite side-effects.
src/session/service/sessions_service.rs New session-management APIs (list/load/promote/rename), incremental persistence fix, and new unit tests.
src/session/repository/session_repository.rs Orders sessions by expires_at DESC for MRU-style listings.
src/session/repository/mod.rs Extends MessageRepository trait with delete_messages_for_session.
src/session/repository/message_repository.rs Adds deterministic message ordering (ORDER BY rowid) and implements delete_messages_for_session.
src/session/model/session.rs Removes debug print; leaves message unchanged when no redaction mapping exists.
src/commands/tests.rs Updates repository test mock to satisfy new MessageRepository method.
src/commands/chat.rs Seeds --system-prompt into the session and passes initial CLI input into the interactive loop.
src/commands/ask.rs Removes no-op env fallback call.
src/chat/state.rs Suppresses dead-code warning for toggle_tools.
src/chat/repl.rs Moves history file to ~/.config/termai/history with fallback to legacy in-CWD file.
src/chat/interactive.rs Major REPL behavior changes: new session commands, autosave, cancellable generation, prompt/banner updates, and persistence fixes.
src/chat/formatter.rs Adds streaming state getter + session banner and session list rendering helpers.
src/chat/commands.rs Adds new slash commands and “unknown slash command” classification path.
src/args/structs.rs Removes no-op with_env_fallbacks methods for ChatArgs and AskArgs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 98 to +102
let session = match session_repo.fetch_session_by_name(name) {
Err(_) => {
let id = generate_uuid_v4().to_string();
let now = Utc::now().naive_utc();
let expires_at: NaiveDateTime = now + Duration::hours(24);

match session_repo.remove_current_from_all() {
Ok(_) => {}
Err(err) => panic!(
"could not remove current from previous sessions: {:#?}",
err
),
}

match session_repo.add_session(&id, name, expires_at, true) {
Ok(_) => println!("New session '{}' expires at {}", name, expires_at),
Err(err) => panic!("Could not create a new session: {:#?}", err),
}
let expires_at = fresh_expiry();

Comment on lines +39 to +55
/// Return all saved sessions (most-recently-used first) with their messages
/// hydrated, so callers can render counts / active markers. This is the
/// data-returning sibling of [`fetch_all_sessions`].
pub fn list_sessions<SR: SessionRepository, MR: MessageRepository>(
session_repo: &SR,
message_repository: &MR,
) -> Result<Vec<Session>> {
let session_entities = session_repo
.fetch_all_sessions()
.map_err(|e| anyhow!("Failed to fetch sessions: {:?}", e))?;

let sessions = session_entities
.iter()
.map(|s| {
let session = Session::from(s);
session_with_messages(message_repository, &session)
})
@kyluke

kyluke commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

@claude can you please fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants