OnIdle and OnEnding must extract memories from transcript#47
Merged
Conversation
added 8 commits
May 11, 2026 04:13
- Add ExtractionEngine and EmbeddingEngine fields to SessionHookConfig
- Add extractMemories helper to SessionHookCoordinator for memory extraction
- OnIdle: calls SupersedeBySource before extraction, extracts memories,
embeds content, and logs extraction via LogExtraction
- OnEnding: extracts memories, runs IntrospectTranscript for session
introspection, falls back to session_end event on introspection failure
- Add IntrospectTranscript function in introspect package for session-end
analysis with graceful LLM degradation
- Remove _ = transcript discard lines from OnIdle and OnEnding
- Update cmd/llmem/main.go to wire ExtractionEngine and EmbeddingEngine
- Add comprehensive tests for all new behavior
All checklist items from DESIGN_BRIEF addressed:
- SessionHookConfig.ExtractionEngine (nil = graceful no-op)
- SessionHookConfig.Embedding (nil = skip embedding)
- OnIdle uses SupersedeBySource (not IsExtracted) for dedup
- OnEnding runs IntrospectTranscript + event memory fallback
- IntrospectTranscript contract: never returns ("", nil)
- ExtractionEngine.Extract never returns error
- Memory creation validates keys with sensible defaults
- No new package-level mutable state
- All errors wrapped with domain prefix
- Graceful degradation throughout
…ded OllamaClient Issue 1 (reviewer): vecToBytes was triplicated across store/helpers.go, embed/embed.go, and session/session.go. Exported VecToBytes/BytesToVec from store package as the canonical implementations. Removed duplicates from embed and session packages. Updated all call sites to use store.VecToBytes and store.BytesToVec. Issue 2 (reviewer): IntrospectTranscript created its own OllamaClient with hardcoded defaults instead of reusing the session's configured connection. OnEnding passed empty model/baseURL, bypassing Ollama config. Fixed by: - Adding callModelWithClient() that accepts a pre-configured *ollama.OllamaClient - Modifying IntrospectTranscript to accept *ollama.OllamaClient instead of model/baseURL strings - Adding OllamaClient and IntrospectModel fields to SessionHookConfig - Storing OllamaClient in SessionHookCoordinator and passing it to IntrospectTranscript from OnEnding - When OllamaClient is nil, OnEnding skips introspection gracefully - Added openOllamaClient() helper in cmd/llmem/main.go - Updated all OnEnding tests to provide mock OllamaClient instances - Added new tests: TestIntrospectTranscript_WithOllamaClient, TestIntrospectTranscript_NilOllamaClient_DegradedContent
…aceful degradation Issue ll-m8knf-vup88: OnEnding guarded IntrospectTranscript with c.ollamaClient != nil, bypassing the function's own nil-client graceful degradation. When OllamaClient was nil, OnEnding produced zero introspection memories for sessions with valid transcripts. Fix: remove the nil guard so IntrospectTranscript is called whenever transcript is non-empty. The function already handles nil OllamaClient by producing a degraded self_assessment memory with a plain-text session summary (no LLM call attempted). Updated SessionHookConfig.OllamaClient doc comment to reflect the actual behavior. Updated TestSessionHookCoordinator_OnEnding_NilExtractionEngine to verify degraded self_assessment memories are produced. Added TestSessionHookCoordinator_OnEnding_NilOllamaClient_ProducesDegradedIntrospection to specifically validate the fix.
…essages - session_test.go:834: Fix stale comment from 'introspect is skipped gracefully' to 'IntrospectTranscript degrades gracefully' (issue ll-m8knf-tvok3) - cmd/llmem/main.go:152: Fix doc comment from 'skipping introspection' to 'falling back to degraded introspection' - cmd/llmem/main.go:156: Fix slog message from 'skipping introspection' to 'falling back to degraded introspection' IntrospectTranscript with nil OllamaClient produces a degraded plain-text memory (graceful degradation), not a skip. All documentation now accurately reflects this behavior.
…ding test, server leaks - Document context.Background() usage in IntrospectTranscript ms.Add() call with justification: session-ending context may have expired, must persist findings regardless. Addresses ll-m8knf-qz489 and ll-m8knf-y5k22. - Wire newTestEmbeddingEngine into TestSessionHookCoordinator_OnIdle_WithEmbedding_VerifiesMemoriesHaveEmbeddings, covering the previously-untested embedder != nil code path in extractMemories (session.go:677-683). Addresses ll-m8knf-45jp1. - Add t.Cleanup(server.Close) to newIntrospectTestServer helper, fixing httptest.Server leaks in OnEnding tests. Addresses ll-m8knf-9y5qj.
…spectTranscript - API.md: Document new SessionHookConfig fields (ExtractionEngine, Embedding, OllamaClient, IntrospectModel) - API.md: Document IntrospectTranscript function with nil-client graceful degradation - API.md: Update SessionHookCoordinator usage to show new extraction/introspection pipeline - API.md: Fix VecToBytes/BytesToVec description text (was lowercase, now exported) - CLI.md: Document OnIdle extraction pipeline and OnEnding introspection behavior - DREAM.md: Add IntrospectTranscript to introspect section with context.Background() note
9bde772 to
38a80c3
Compare
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
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.
Closes droplet ll-m8knf.