From ed53d6d2152ea31be46d2625352b0ddeb5c48eb6 Mon Sep 17 00:00:00 2001 From: William Valentin Date: Wed, 25 Feb 2026 12:50:24 -0800 Subject: [PATCH] docs(memory): revise pi personal assistant memory plan after code review Key changes from review: - Use separate _sessionContext field instead of mutating _systemPromptBase - Parameterize compaction prompt via buildCompactionPrompt() instead of duplicating - Fix flaky TTL=0 test to use hardcoded past expiry date - Route memory extraction to {userNamespace}/facts when namespace is set - Document future considerations (config refactor, concurrent writes, token counting) Co-Authored-By: Claude Opus 4.6 --- ...02-25-pi-personal-assistant-memory-plan.md | 239 ++++++++++++------ 1 file changed, 155 insertions(+), 84 deletions(-) diff --git a/docs/plans/2026-02-25-pi-personal-assistant-memory-plan.md b/docs/plans/2026-02-25-pi-personal-assistant-memory-plan.md index 46fe1c5..63c7cf3 100644 --- a/docs/plans/2026-02-25-pi-personal-assistant-memory-plan.md +++ b/docs/plans/2026-02-25-pi-personal-assistant-memory-plan.md @@ -4,30 +4,35 @@ **Goal:** Add two-tier personal-assistant memory: a unified `user/*` namespace shared across channels and a working-memory layer (`user/working`) that survives restarts and is injected at session start. -**Architecture:** `user/working` is written on every compaction (TTL-based flat file with header metadata). On the first message of a new session, `user/profile` and `user/working` are injected into the base system prompt once. All behavior is behind a `memory.user_namespace` config key; when unset the feature is entirely inert. +**Architecture:** `user/working` is written on every compaction (TTL-based flat file with header metadata). On the first message of a new session, `user/profile` and `user/working` are composed into the system prompt alongside the existing memory context. All behavior is behind a `memory.user_namespace` config key; when unset the feature is entirely inert. **Tech Stack:** TypeScript, Vitest, existing `MemoryStore` (`src/memory/store.ts`), `AgentOrchestrator` (`src/backends/native/orchestrator.ts`), Zod config schema +**Key design decisions (from review):** +- Session context is stored in a separate `_sessionContext` field — `_systemPromptBase` is never mutated +- Compaction prompt is parameterized via `buildCompactionPrompt()` rather than duplicating the constant +- Expired-memory test uses a hardcoded past date (not TTL=0) for determinism +- Memory extraction writes to `{userNamespace}/facts` when user namespace is set + --- ## Task 1: Config schema — new memory fields **Files:** -- Modify: `src/config/schema.ts:592-613` +- Modify: `src/config/schema.ts` -Add four new fields to `memorySchema`. They must come before `.default({})` at line 613. +Add four new fields to `memorySchema`. They must come before `.default({})`. -**Step 1: Write the failing type check** +**Step 1: Run baseline typecheck** -Run: ```bash pnpm typecheck ``` -Expected: passes (baseline). We'll check again after the edit. +Expected: passes (baseline). **Step 2: Add fields to memorySchema** -In `src/config/schema.ts`, inside `memorySchema` (after `qmd: qmdSchema,` on line 612, before `}).default({});`), add: +In `src/config/schema.ts`, inside `memorySchema` (after `qmd: qmdSchema,`, before `}).default({});`), add: ```typescript /** @@ -80,7 +85,7 @@ Expires: 2026-03-10T11:30:00Z Create `src/memory/workingMemory.test.ts`: ```typescript -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect } from 'vitest'; import { join } from 'path'; import { mkdtempSync, rmSync } from 'fs'; import { tmpdir } from 'os'; @@ -135,8 +140,15 @@ describe('readWorkingMemory', () => { it('returns null when expired', () => { const { store, dir } = makeStore(); - // Write with 0 TTL days (expires immediately) - writeWorkingMemory(store, 'user/working', 'stale content', 0, 1000); + // Write a file with a hardcoded past expiry date for deterministic testing + const expiredFile = [ + '# Working Memory', + 'Updated: 2025-01-01T00:00:00Z', + 'Expires: 2025-01-02T00:00:00Z', + '', + 'stale content', + ].join('\n'); + store.write('user/working', expiredFile, 'replace'); const result = readWorkingMemory(store, 'user/working'); expect(result).toBeNull(); rmSync(dir, { recursive: true }); @@ -201,6 +213,7 @@ export function writeWorkingMemory( /** * Read working memory. Returns null if the file is absent, malformed, or expired. + * MemoryStore.read() returns '' for missing files, so falsy check works. * Expiry is checked lazily here — no background cleanup needed. */ export function readWorkingMemory( @@ -271,31 +284,45 @@ git commit -m "feat(memory): add working memory read/write with TTL expiry" --- -## Task 3: PA-focused compaction prompt +## Task 3: Parameterized compaction prompt **Files:** - Modify: `src/backends/native/prompts.ts` -The existing `COMPACTION_SYSTEM_PROMPT` is a generic summarizer. We need a personal-assistant variant that captures what the user was working on, decisions made, expressed preferences, and open threads. +Instead of duplicating `COMPACTION_SYSTEM_PROMPT`, add a `buildCompactionPrompt()` function that shares rules and parameterizes the focus section. The existing `COMPACTION_SYSTEM_PROMPT` constant stays for backward compatibility. -**Step 1: Add the new prompt constant** +**Step 1: Add the parameterized prompt builder** In `src/backends/native/prompts.ts`, after `COMPACTION_SYSTEM_PROMPT`, add: ```typescript /** - * Personal-assistant variant of the compaction prompt. - * Used when `memory.user_namespace` is configured. - * Captures continuity context: what the user was working on, decisions, - * preferences, and open threads — not just a generic recap. + * Build a compaction system prompt. When `personalAssistant` is true, + * the prompt focuses on continuity context (what the user was working on, + * decisions, preferences, open threads) rather than generic summarization. + * + * The shared rules (preserve facts, 20% length, bullet points, no invention, + * skip transient content) are identical in both variants. */ -export const PA_COMPACTION_SYSTEM_PROMPT = `You are summarising a conversation for a personal assistant. Your summary will be injected at the start of the next session so the assistant can pick up exactly where things left off. - -Focus on: +export function buildCompactionPrompt(opts?: { personalAssistant?: boolean }): string { + const focus = opts?.personalAssistant + ? `Focus on: - What the user was working on and its current status (be specific: which files, commands, or steps were involved) - Decisions made and why (include rationale when stated) - Preferences or constraints the user expressed (tools, styles, approaches to avoid or prefer) -- Open threads, unresolved questions, or explicit follow-up items +- Open threads, unresolved questions, or explicit follow-up items` + : `Focus on: +- Key topics discussed and conclusions reached +- Important decisions, commitments, or action items +- Technical details, code changes, or configurations that were established`; + + const preamble = opts?.personalAssistant + ? 'You are summarising a conversation for a personal assistant. Your summary will be injected at the start of the next session so the assistant can pick up exactly where things left off.' + : 'You are a conversation summarizer. Create a concise summary of the conversation that captures all important information.'; + + return `${preamble} + +${focus} Rules: - Preserve key facts, file paths, error messages, and specific values verbatim. @@ -306,6 +333,7 @@ Rules: Output format: Return a markdown summary. No preamble — output only the summary.`; +} ``` **Step 2: Typecheck** @@ -319,7 +347,7 @@ Expected: no errors. ```bash git add src/backends/native/prompts.ts -git commit -m "feat(memory): add personal-assistant compaction prompt" +git commit -m "feat(memory): add parameterized compaction prompt builder" ``` --- @@ -329,13 +357,9 @@ git commit -m "feat(memory): add personal-assistant compaction prompt" **Files:** - Modify: `src/context/compaction.ts` -Add `summary` to `CompactionResult` so the orchestrator can write it to `user/working` without re-computing it. Also thread the PA prompt option. +Add `summary` to `CompactionResult` so the orchestrator can write it to `user/working` without re-computing it. Thread the PA prompt option. Also route memory extraction to `{userNamespace}/facts` when namespace is set. -**Step 1: Write the failing test** - -There is no direct unit test for `compactHistory()` currently (it requires a live orchestrator). We'll test via the orchestrator integration in Task 5. For now, just verify the type change works. - -**Step 2: Update `CompactionResult` interface** +**Step 1: Update `CompactionResult` interface** In `src/context/compaction.ts`, add `summary` to `CompactionResult`: @@ -354,9 +378,9 @@ export interface CompactionResult { } ``` -**Step 3: Add `usePersonalAssistantPrompt` option and populate `summary`** +**Step 2: Add `usePersonalAssistantPrompt` option and populate `summary`** -Update the `compactHistory` function signature to accept the option and return the summary: +Update the `compactHistory` function signature: ```typescript export async function compactHistory(opts: { @@ -365,18 +389,22 @@ export async function compactHistory(opts: { config: CompactionConfig; memoryStore?: MemoryStore; autoExtract?: boolean; - usePersonalAssistantPrompt?: boolean; // ← new + usePersonalAssistantPrompt?: boolean; + memoryExtractionNamespace?: string; }): Promise { ``` -In the body, replace the `COMPACTION_SYSTEM_PROMPT` reference in the `orchestrator.delegate()` call: +Update the imports to include `buildCompactionPrompt`: ```typescript -import { COMPACTION_SYSTEM_PROMPT, MEMORY_EXTRACTION_PROMPT, PA_COMPACTION_SYSTEM_PROMPT } from '../backends/native/prompts.js'; +import { COMPACTION_SYSTEM_PROMPT, MEMORY_EXTRACTION_PROMPT, buildCompactionPrompt } from '../backends/native/prompts.js'; +``` -// ...inside compactHistory, where orchestrator.delegate is called: +In the body, replace the hardcoded `COMPACTION_SYSTEM_PROMPT` in the `orchestrator.delegate()` call: + +```typescript const systemPrompt = opts.usePersonalAssistantPrompt - ? PA_COMPACTION_SYSTEM_PROMPT + ? buildCompactionPrompt({ personalAssistant: true }) : COMPACTION_SYSTEM_PROMPT; const result = await orchestrator.delegate({ @@ -388,7 +416,7 @@ const result = await orchestrator.delegate({ }); ``` -And populate `summary` in the return value: +Populate `summary` in the return value: ```typescript return { @@ -396,10 +424,31 @@ And populate `summary` in the return value: compactedCount: toSummarize.length, tokensBefore: estimateMessageTokens(messages), tokensAfter: estimateMessageTokens([...preservedMessages, summaryMessage, ...toKeep]), - summary: result.content, // ← new + summary: result.content, }; ``` +**Step 3: Route memory extraction to user namespace when set** + +In the memory extraction block (around line 133), change the write target: + +```typescript + if (opts.memoryStore && opts.autoExtract !== false) { + try { + // ...existing extraction delegate call... + + const extractedContent = extraction.content.trim(); + if (extractedContent.length > 0 && !extractedContent.toLowerCase().includes('no facts')) { + const extractionNs = opts.memoryExtractionNamespace ?? 'global'; + opts.memoryStore.write(extractionNs, extractedContent, 'append'); + console.log(`[Flynn:memory] Extracted ${extractedContent.length} chars of facts to ${extractionNs} memory`); + } + } catch (error) { + console.warn('[Flynn:memory] Failed to extract facts during compaction:', error); + } + } +``` + **Step 4: Typecheck** ```bash @@ -430,35 +479,21 @@ git commit -m "feat(memory): thread summary and PA prompt through compaction res This task has two parts: 1. Write working memory after every compaction (when `userNamespace` is set) -2. Inject `user/profile` + `user/working` into the base system prompt on first message +2. Inject `user/profile` + `user/working` into the system prompt on first message -**Step 1: Write the failing test — post-compaction working memory write** +**Critical design note:** `_systemPromptBase` must remain immutable (set once in constructor). Session context is stored in a separate `_sessionContext` field and composed into the system prompt in `_injectMemoryContext()`. -In `src/backends/native/orchestrator.test.ts` (or the closest existing test file — check with `ls src/backends/native/*.test.ts`), add a test. First, find the test file: +**Step 1: Explore existing test patterns** ```bash ls src/backends/native/*.test.ts ``` -If `orchestrator.test.ts` exists, add to it. The test should verify that after calling `compact()`, the working memory namespace contains the summary. Because this requires a functioning orchestrator setup, write an integration-style test using vitest mocks rather than a full live model: - -Locate the `describe('compact')` block in `src/backends/native/orchestrator.test.ts`. Add: - -```typescript -it('writes summary to user/working when userNamespace is set', async () => { - // Setup: orchestrator with a mock delegate that returns a fixed summary - // and a real MemoryStore in a temp dir - // (follow the existing orchestrator test setup patterns) - // After orchestrator.compact(), read store.read('user/working') - // Expect it to contain the summary content -}); -``` - -You'll need to examine the existing test setup patterns carefully before writing the full test body. See the file for the existing mock structure. +Examine the existing orchestrator test setup to understand mock patterns before writing the integration test. **Step 2: Add new fields to `OrchestratorConfig`** -In `src/backends/native/orchestrator.ts`, in `OrchestratorConfig` (after `attachmentCollector` on line ~160), add: +In `src/backends/native/orchestrator.ts`, in `OrchestratorConfig` (after `attachmentCollector`), add: ```typescript /** Shared identity namespace for cross-channel memory (e.g. 'user'). Absent = session-scoped. */ @@ -473,14 +508,14 @@ In `src/backends/native/orchestrator.ts`, in `OrchestratorConfig` (after `attach **Step 3: Store new config fields on the class** -In the `AgentOrchestrator` class, add private fields (near the other `_memory*` fields): +Add private fields (near the other `_memory*` fields): ```typescript private _userNamespace?: string; private _workingMemoryTtlDays: number; private _workingMemoryMaxTokens: number; private _proactiveSessionGreeting: boolean; -private _sessionContextInjected = false; +private _sessionContext: string | null = null; ``` In the constructor, after the existing memory field assignments: @@ -500,7 +535,7 @@ Import `writeWorkingMemory` at the top of orchestrator.ts: import { writeWorkingMemory } from '../../memory/workingMemory.js'; ``` -In the `compact()` method, after the `auditLogger?.sessionCompact(...)` call (around line 500), add: +In the `compact()` method, after the `auditLogger?.sessionCompact(...)` call, add: ```typescript // Write working memory when user namespace is configured @@ -517,7 +552,7 @@ In the `compact()` method, after the `auditLogger?.sessionCompact(...)` call (ar } ``` -Also, pass `usePersonalAssistantPrompt` to `compactHistory()` in `compact()`: +Also, pass `usePersonalAssistantPrompt` and `memoryExtractionNamespace` to `compactHistory()` in `compact()`: ```typescript const result = await compactHistory({ @@ -526,22 +561,24 @@ Also, pass `usePersonalAssistantPrompt` to `compactHistory()` in `compact()`: config, memoryStore: this._memoryStore, autoExtract: this._memoryAutoExtract, - usePersonalAssistantPrompt: Boolean(this._userNamespace), // ← new + usePersonalAssistantPrompt: Boolean(this._userNamespace), + memoryExtractionNamespace: this._userNamespace ? `${this._userNamespace}/facts` : undefined, }); ``` -**Step 5: Session-start injection** +**Step 5: Session-start injection — `_buildSessionContext()`** -Add `_injectSessionContext()` private method to `AgentOrchestrator` (after `_injectMemoryContext`): +Add a private method that builds the session context string (but does NOT mutate `_systemPromptBase`): ```typescript - private _injectSessionContext(): void { - if (this._sessionContextInjected) { - return; - } - this._sessionContextInjected = true; - - if (!this._memoryStore || !this._userNamespace) { + /** + * Build session context from user/profile and user/working memory. + * Called once on first process() call. Returns null if no context available. + * Does NOT mutate _systemPromptBase — the result is stored in _sessionContext + * and composed into the system prompt by _injectMemoryContext(). + */ + private _buildSessionContext(): void { + if (this._sessionContext !== null || !this._memoryStore || !this._userNamespace) { return; } @@ -560,15 +597,19 @@ Add `_injectSessionContext()` private method to `AgentOrchestrator` (after `_inj } if (sections.length === 0) { + // Set to empty string (not null) to indicate we've run but found nothing. + // Null means "not yet computed". + this._sessionContext = ''; return; } - this._systemPromptBase = `${this._systemPromptBase}\n\n${sections.join('\n\n')}`; + let ctx = sections.join('\n\n'); if (this._proactiveSessionGreeting) { - this._systemPromptBase += - '\n\n[If relevant, briefly acknowledge what the user was last working on before responding to their first message.]'; + ctx += '\n\n[If relevant, briefly acknowledge what the user was last working on before responding to their first message.]'; } + + this._sessionContext = ctx; } ``` @@ -578,17 +619,37 @@ Add the import at the top of orchestrator.ts: import { writeWorkingMemory, readWorkingMemory } from '../../memory/workingMemory.js'; ``` -**Step 6: Call `_injectSessionContext()` from `process()`** +**Step 6: Compose session context in `_injectMemoryContext()`** -In the `process()` method, find the first place where `_injectMemoryContext()` is called. Add `_injectSessionContext()` just before it: +In `_injectMemoryContext()`, change the base prompt computation to include session context: + +Where the method currently does: +```typescript +this._agent.setSystemPrompt(this._systemPromptBase); +// and later: +const enrichedPrompt = `${this._systemPromptBase}\n\n# Memory Context\n\n...`; +``` + +Change to compose session context into the effective base: +```typescript +const effectiveBase = this._sessionContext + ? `${this._systemPromptBase}\n\n${this._sessionContext}` + : this._systemPromptBase; +``` + +Then use `effectiveBase` everywhere `_systemPromptBase` was used in that method. + +**Step 7: Call `_buildSessionContext()` from `process()`** + +In the `process()` method, before `_injectMemoryContext()`: ```typescript // One-time session-start context injection (user/profile + user/working) - this._injectSessionContext(); + this._buildSessionContext(); this._injectMemoryContext(userMessage); ``` -Also reset `_sessionContextInjected` in `reset()`: +**Step 8: Reset session context in `reset()`** ```typescript reset(): void { @@ -597,28 +658,28 @@ Also reset `_sessionContextInjected` in `reset()`: this._lastContextAlertLevel = null; this._pendingContextAlert = undefined; this._lastCheckpointAt = 0; - this._sessionContextInjected = false; // ← add + this._sessionContext = null; // ← add: re-read on next process() } ``` -**Step 7: Typecheck** +**Step 9: Typecheck** ```bash pnpm typecheck ``` Expected: no errors. -**Step 8: Run full test suite** +**Step 10: Run full test suite** ```bash pnpm test:run ``` Expected: all pass. -**Step 9: Commit** +**Step 11: Commit** ```bash -git add src/backends/native/orchestrator.ts src/memory/workingMemory.ts +git add src/backends/native/orchestrator.ts git commit -m "feat(memory): inject session context and write working memory after compaction" ``` @@ -631,7 +692,7 @@ git commit -m "feat(memory): inject session context and write working memory aft **Step 1: Add the new fields after `memoryDailyLogMaxAssistantChars`** -In `src/daemon/routing.ts` around line 756, after: +In `src/daemon/routing.ts`, after: ```typescript memoryDailyLogMaxAssistantChars: deps.config.memory?.daily_log?.max_assistant_chars, ``` @@ -697,7 +758,7 @@ In `docs/plans/state.json`, add an entry to `completed`: "pi_personal_assistant_memory": { "status": "complete", "commit": "", - "summary": "Two-tier personal assistant memory: working memory (user/working, TTL-based) written on compaction, injected at session start; unified user/* namespace across channels; PA-focused compaction prompt; proactive session greeting option." + "summary": "Two-tier personal assistant memory: working memory (user/working, TTL-based) written on compaction, injected at session start; unified user/* namespace across channels; parameterized compaction prompt; memory extraction routed to user/facts; proactive session greeting option." } ``` @@ -717,10 +778,20 @@ git commit -m "docs(state): mark pi personal assistant memory as complete" - [ ] Daemon restart + new session → `user/working` content appears in the first-turn system prompt - [ ] Telegram and web UI sessions share the same `user/working` file - [ ] Expired working memory (TTL elapsed) is silently ignored +- [ ] `_systemPromptBase` is never mutated — session context composed via `_sessionContext` field +- [ ] Memory extraction writes to `{userNamespace}/facts` when namespace set, `global` otherwise - [ ] All existing tests pass --- +## Future considerations (not in scope) + +- **OrchestratorConfig refactor:** Group the 17+ flat `memory*` fields into a nested `MemoryConfig` interface. Mechanical but improves readability. Do this in a separate PR. +- **Concurrent compaction guard:** Multiple sessions could write `user/working` simultaneously. `writeFileSync` is atomic per-call so data won't corrupt, but the last-writer-wins. Consider a merge strategy if this becomes a problem. +- **Token counting:** Currently using `maxTokens * 4` char heuristic. Could reuse `estimateMessageTokens()` for consistency. + +--- + ## Reference: Config ```yaml