Skip to content

feat: cross-session memory (summarize + inject context)#113

Open
sogadaiki wants to merge 4 commits intoRichardAtCT:mainfrom
sogadaiki:feat/session-memory
Open

feat: cross-session memory (summarize + inject context)#113
sogadaiki wants to merge 4 commits intoRichardAtCT:mainfrom
sogadaiki:feat/session-memory

Conversation

@sogadaiki
Copy link

@sogadaiki sogadaiki commented Feb 26, 2026

Summary

Implements the cross-session memory feature proposed in #105.

When a user starts a new session via /new, the previous session's conversation is summarized by Claude and stored in SQLite. On the next new session, stored summaries are injected into the system prompt, giving Claude context about prior work.

Changes

  • Migration 5: session_memories table with user+project scoping
  • SessionMemoryModel + SessionMemoryRepository: CRUD for memory storage
  • SessionMemoryService: Summarization via Claude API + memory retrieval/formatting
  • System prompt injection: memory_context parameter flows through facade -> SDK
  • Feature flag: ENABLE_SESSION_MEMORY (default false, opt-in)
  • Background summarization: /new triggers async summarization (fire-and-forget)
  • Unit tests: Service tests (mocked) + repository tests (in-memory SQLite)

Configuration

ENABLE_SESSION_MEMORY=true           # Enable the feature
SESSION_MEMORY_MAX_COUNT=5           # Max memories per user+project
SESSION_MEMORY_MIN_MESSAGES=3        # Min messages before summarizing

Design Decisions

  • Default OFF: Summarization costs a small API call per /new, so opt-in
  • System prompt injection (not user prompt): Keeps memory as background context
  • Skip on resume: Memory injection only on genuinely new sessions
  • Scoped to user+project: No cross-user or cross-project leakage
  • 2000 char cap: Prevents system prompt bloat

Test plan

  • Enable ENABLE_SESSION_MEMORY=true, send several messages, run /new
  • Verify session summary is stored in session_memories table
  • Start a new conversation and verify Claude references prior context
  • Verify feature is completely inactive when flag is false
  • Verify short sessions (< 3 messages) are not summarized

Closes #105

sogadaiki and others added 4 commits February 26, 2026 19:26
AI秘書まいの人格をTelegram Botに統合し日本語対話を実現
- config/persona/mai.md: ペルソナ定義
- src/bot/i18n.py: 辞書ベース軽量i18n (ja/en)
- settings.py: persona/knowledge/effort/permission_mode設定追加
- sdk_integration.py: ペルソナ読み込み+SDKオプション
- orchestrator.py/auth.py/core.py: UIメッセージi18n化

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SDK stream callbackでThinkingBlockのみのcontentをstr()変換して表示していた問題を修正
- sdk_integration.py: ThinkingBlockをスキップ、fallbackでも表示可能ブロックのみ通す
- orchestrator.py: [ThinkingBlock(で始まるテキストを進捗表示から除外

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a user starts a new session via /new, the previous session's
conversation is summarized by Claude and stored in SQLite. On the next
new session, stored summaries are injected into the system prompt,
giving Claude context about prior work.

- Migration 5: session_memories table
- SessionMemoryModel + SessionMemoryRepository
- SessionMemoryService (summarize + retrieve)
- System prompt injection via memory_context parameter
- Feature flag: ENABLE_SESSION_MEMORY (default false)
- Background summarization on /new (fire-and-forget)
- Unit tests for service and repository

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add `id DESC` tiebreaker to session memory query for deterministic ordering
- Add `bot_language` to test mock settings (Pydantic spec compatibility)
- Pin `bot_language="en"` in orchestrator test fixture to avoid .env leakage

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@FridayOpenClawBot
Copy link

PR Review
Reviewed head: c5129ab04c8cbad43eca3e060ea93cf4dbbb51be

Summary

  • Implements cross-session memory: on /new, the previous session is summarized by Claude and stored in SQLite; on the next new session, stored summaries are injected into the system prompt
  • Feature-flagged via ENABLE_SESSION_MEMORY (default false); scoped to user+project; 2000 char cap on injected context
  • Includes DB migration 5, SessionMemoryModel/Repository/Service, and 613 lines of unit tests
  • Also includes unrelated changes: config/persona/mai.md, src/bot/i18n.py, and src/bot/middleware/auth.py (same pattern as PR feat: exponential backoff retry for transient SDK errors #117)

What looks good

  • Default-off feature flag is the right call given the per-/new API cost
  • SESSION_MEMORY_MIN_MESSAGES=3 guard prevents summarizing trivially short sessions
  • Scoping to user+project prevents cross-contamination; 2000 char cap is sensible
  • Strong test coverage: 295 lines for the service + 318 for the repository

Issues / questions

  1. [Important] src/claude/memory.pySessionMemoryService.summarize_session() calls the Claude API synchronously inside a fire-and-forget asyncio.create_task(). If summarization fails (network error, API rate limit), the exception is silently dropped. Consider logging the failure so operators know summarization is broken.
  2. [Important] PR body's test plan has unchecked boxes (manual tests not verified). Before merging, confirm: (a) summary is stored after /new, (b) context appears in the next session, (c) feature is fully inactive when flag is false.
  3. [Important] config/persona/mai.md contains {knowledge_paths_section} — an unfilled template placeholder. This will appear verbatim in the persona prompt if it's ever loaded. Should be removed or resolved.
  4. [Nit] src/storage/repositories.py — consider adding a created_at index on session_memories if you expect to query by recency at scale (the default ORDER BY created_at DESC LIMIT n will do a full scan otherwise).

Suggested tests

  • Test that a summarization failure (mocked exception) is logged and does not bubble up to the /new handler
  • Test that injected memory is truncated at 2000 chars even if stored summaries are longer

Verdict
⚠️ Merge after fixes — excellent feature with good test coverage, but the silent failure mode in the background summarization task and the unresolved persona placeholder should be addressed first.

Friday, AI assistant to @RichardAtCT

@FridayOpenClawBot
Copy link

PR Review
Reviewed head: c5129ab04c8cbad43eca3e060ea93cf4dbbb51be

Summary

  • Implements opt-in cross-session memory: when /new is called, the previous session is summarized by Claude and stored in SQLite; summaries are injected into the system prompt on subsequent new sessions
  • Adds i18n infrastructure (src/bot/i18n.py) with en/ja strings, used across error messages, auth flow, and bot commands
  • Bundles a Japanese-language persona config (config/persona/mai.md) alongside the memory feature

What looks good

  • Feature flag defaulting to false is the right call — summarization costs an extra API call per /new
  • 2000-char cap on injected context is a sensible guard against system prompt bloat
  • Fire-and-forget async summarization means /new stays fast for the user
  • Solid repository test coverage: ordering, limits, deactivation, inactive filtering

Issues / questions

  1. [Blocker] src/claude/memory.py _generate_summary — The summarization call uses execute_command with session_id=None and continue_session=False, meaning it runs a full Claude Code SDK invocation just to summarize a transcript. If the SDK spawns a subprocess/shell session for this, it may be slower, more expensive, or have side effects (e.g. spawning a working-directory context, tool use permissions). Is there a lighter path — e.g. a direct Anthropic API messages call without the SDK layer? If not, at minimum add a comment explaining why execute_command is intentional here.

  2. [Important] src/claude/memory.py _build_transcript — The transcript truncation logic checks total_len + len(line) > _MAX_TRANSCRIPT_CHARS but only after the message has been formatted. If the last message included in the transcript pushes exactly over the limit, it gets silently dropped rather than truncated. This means the transcript can end on an incomplete exchange — not a correctness bug but could confuse the summarizer. Consider appending a partial message or at least a # ... (truncated) marker.

  3. [Important] src/claude/memory.py summarize_session — If _generate_summary raises (API error, timeout, etc.), the exception propagates up. Since summarize_session is called fire-and-forget from the orchestrator, an unhandled exception here would silently swallow the error. The logger.info("Session memory saved") would never run either. Add a try/except inside summarize_session that logs the error and returns None gracefully.

  4. [Important] src/claude/sdk_integration.py _load_persona_prompt{knowledge_paths_section} is replaced via plain string .replace(). If the persona file contains any other {…} placeholders that aren't knowledge_paths_section, they will survive silently and potentially cause a KeyError if someone later calls .format() on the content. Either document that the file must only contain {knowledge_paths_section}, or switch to a format that's unambiguous (e.g. {{ escaping or a template library).

  5. [Nit] config/persona/mai.md — This persona is scoped to a specific user ("曽我大樹"). Shipping it in the main repo as a default config file feels like it exposes someone's personal setup. Is this intentional, or should it live in a .gitignored user config directory?

Suggested tests

  • Test summarize_session error path: mock _generate_summary to raise, assert it returns None and logs an error (once error handling is added)
  • Integration-style test for the fire-and-forget: confirm a crash in the background task doesn't propagate to the caller

Verdict
⚠️ Merge after fixes — the fire-and-forget error handling gap (item 3) is the most pressing; the summarization API path (item 1) is worth a discussion before merging.

Friday, AI assistant to @RichardAtCT

Copy link
Owner

@RichardAtCT RichardAtCT left a comment

Choose a reason for hiding this comment

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

Thanks for this feature! A few things need to be addressed:

  1. Merge conflicts — this PR has conflicts with main that need resolving
  2. CI failing — tests are not passing
  3. Unrelated files — please remove any files not directly related to the cross-session memory feature

Please rebase against current main, fix CI, and remove unrelated changes. Happy to re-review once cleaned up!

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.

feat: セッション間長期記憶(会話要約→次回注入)

3 participants