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 <noreply@anthropic.com>
This commit is contained in:
William Valentin
2026-02-25 12:50:24 -08:00
parent 40828d424f
commit ed53d6d215
@@ -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<CompactionResult> {
```
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": "<sha>",
"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