Skip to content

chach#42

Merged
yaacov merged 1 commit into
mainfrom
cache
Jun 8, 2026
Merged

chach#42
yaacov merged 1 commit into
mainfrom
cache

Conversation

@yaacov

@yaacov yaacov commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • Added --resume ID option to resume saved chat sessions; session IDs and copy-pastable resume commands are now printed on exit.
    • Increased conversation history capacity from 80,000 to 160,000 characters for larger context windows.
    • Improved session persistence: full chat history, including tool interactions, is restored on resume.
  • Bug Fixes

    • Fixed message duplication when resuming sessions.
  • Documentation

    • Updated README with resume session instructions.

Signed-off-by: yaacov <yzamir@redhat.com>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds session resumption to the MTV-agent TUI, allowing users to save and restore chat sessions via a --resume ID flag. The implementation increases the message history budget, improves history trimming with atomic tool-call grouping, adds server-side message snapshotting for clean persistence, and wires CLI/TUI support for resuming saved chats.

Changes

Session Resume and History Persistence

Layer / File(s) Summary
Configuration and documentation for new defaults
mtv_agent/server/config.py, README.md
Default max_history_chars is doubled from 80,000 to 160,000. README documents the --resume ID option for both mtv-agent run and mtv-tui, and describes session persistence: on exit, session ID and resume command are printed; chat history including tool interactions is cached and fully restored.
History budget and grouping-based trimming
mtv_agent/server/agent.py, mtv_agent/server/tools.py
agent.py increases default max_history_chars in both run_stream and _build_messages to 160,000. tools.py adds json import and replaces flat per-message trimming with grouping that treats assistant tool_calls plus following tool messages as atomic, non-splittable units; _truncate default is also increased to 160,000.
Server message snapshotting for persistence
mtv_agent/server/agent.py, mtv_agent/server/app.py
agent.py adds _prepare_snapshot helper to remove system prompt, strip metadata keys, and truncate oversized tool content, then emits both _messages_snapshot and content events after final LLM responses. app.py treats snapshot events as authoritative state replacement and avoids duplicating assistant messages when persisting history.
CLI and TUI resume implementation
mtv_agent/cli.py, mtv_agent/tui/app.py
cli.py adds --resume ID argument and passes it to MTVApp; after the app exits, prints resume instructions using the session ID prefix. tui/app.py accepts optional resume_id, calls _resume_chat on mount, filters assistant-only messages during replay (skipping tool-call turns), and the main CLI entry point integrates the --resume argument and session recovery flow.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🐰 With whiskers twitching bright,
Sessions saved, resuming right,
History grouped in atomic pairs,
Snapshots clean—the rabbit declares!
Chat flows on from where it starts,
No lost context in these parts! 🎉

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'chach' is vague and non-descriptive, providing no meaningful information about the changeset's purpose or scope. Use a clear, descriptive title that summarizes the main change, such as 'Add session resume support with chat history persistence' or 'Implement --resume CLI option for saved TUI sessions'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cache

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 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 `@mtv_agent/server/agent.py`:
- Around line 173-177: The dict comprehension that builds cleaned incorrectly
uses "or" and should use "and": locate the comprehension that assigns cleaned
from msg (symbols: cleaned, msg, _SNAPSHOT_STRIP_KEYS) and change the condition
from "if k not in _SNAPSHOT_STRIP_KEYS or v is not None" to "if k not in
_SNAPSHOT_STRIP_KEYS and v is not None" so keys listed in _SNAPSHOT_STRIP_KEYS
are always removed while keeping only non-None values for other keys.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09509645-5e1b-4c37-859b-4a20f73876a9

📥 Commits

Reviewing files that changed from the base of the PR and between 6820526 and cce6381.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • README.md
  • mtv_agent/cli.py
  • mtv_agent/server/agent.py
  • mtv_agent/server/app.py
  • mtv_agent/server/config.py
  • mtv_agent/server/tools.py
  • mtv_agent/tui/app.py

Comment thread mtv_agent/server/agent.py
Comment on lines +173 to +177
cleaned = {
k: v
for k, v in msg.items()
if k not in _SNAPSHOT_STRIP_KEYS or v is not None
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical logic error: or should be and in the dict comprehension.

The condition if k not in _SNAPSHOT_STRIP_KEYS or v is not None incorrectly keeps keys that should be stripped. For example, if "annotations" has a non-None value, the condition evaluates to False or True = True, so the key is kept—but annotations is in _SNAPSHOT_STRIP_KEYS and should always be removed. Similarly, keys not in strip_keys but with None values are incorrectly kept.

🐛 Proposed fix
         cleaned = {
             k: v
             for k, v in msg.items()
-            if k not in _SNAPSHOT_STRIP_KEYS or v is not None
+            if k not in _SNAPSHOT_STRIP_KEYS and v is not None
         }
📝 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
cleaned = {
k: v
for k, v in msg.items()
if k not in _SNAPSHOT_STRIP_KEYS or v is not None
}
cleaned = {
k: v
for k, v in msg.items()
if k not in _SNAPSHOT_STRIP_KEYS and v is not None
}
🤖 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 `@mtv_agent/server/agent.py` around lines 173 - 177, The dict comprehension
that builds cleaned incorrectly uses "or" and should use "and": locate the
comprehension that assigns cleaned from msg (symbols: cleaned, msg,
_SNAPSHOT_STRIP_KEYS) and change the condition from "if k not in
_SNAPSHOT_STRIP_KEYS or v is not None" to "if k not in _SNAPSHOT_STRIP_KEYS and
v is not None" so keys listed in _SNAPSHOT_STRIP_KEYS are always removed while
keeping only non-None values for other keys.

@yaacov yaacov merged commit edb480d into main Jun 8, 2026
4 checks passed
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.

1 participant