From eb7355a961402c6228ae3c208658306df576c6a1 Mon Sep 17 00:00:00 2001 From: Dhirender Choudhary Date: Fri, 14 Aug 2026 05:00:23 +0530 Subject: [PATCH] fix: address round-4 review feedback on semantic memory --- docs/configuration/preferences.md | 2 + docs/features/semantic-memory.md | 73 ++++- source/acp/acp-agent.ts | 4 +- source/app/components/settings-selector.tsx | 66 +++- source/app/utils/app-util.ts | 4 + source/commands/memory.spec.tsx | 188 ++++++++++- source/commands/memory.ts | 166 ---------- source/commands/memory.tsx | 318 +++++++++++++++++++ source/config/preferences.spec.ts | 135 ++++++++ source/config/preferences.ts | 79 ++++- source/hooks/chat-handler/useChatHandler.tsx | 9 +- source/memory/project-context.spec.ts | 21 ++ source/memory/project-context.ts | 28 +- source/memory/proposal-store.ts | 58 ++++ source/memory/summarizer-service.spec.ts | 154 +++++++++ source/memory/summarizer-service.ts | 294 ++++++++++++++--- source/plain/shell.ts | 11 +- source/types/config.ts | 4 + 18 files changed, 1359 insertions(+), 255 deletions(-) delete mode 100644 source/commands/memory.ts create mode 100644 source/commands/memory.tsx create mode 100644 source/memory/proposal-store.ts diff --git a/docs/configuration/preferences.md b/docs/configuration/preferences.md index b91f527ac..1e5ed017e 100644 --- a/docs/configuration/preferences.md +++ b/docs/configuration/preferences.md @@ -31,6 +31,8 @@ Preferences follow the same location hierarchy as configuration files: | `trustedDirectories` | Directories you've approved through the first-run security disclaimer | | `lastUpdateCheck` | Timestamp of the last update check (used to avoid checking too frequently) | | `semanticMemoryEnabled` | Enables semantic memory across sessions. Set to `false` or use `/settings` → **Advanced** → **Semantic Memory** to keep agents stateless. | +| `semanticMemoryTokenBudget` | Approximate token ceiling for the recalled `## Project Context` block. Default `240`, clamped to 40-4000. Adjustable from `/settings` → **Advanced**. | +| `semanticMemoryLimit` | Maximum memories considered for a single prompt. Default `8`, clamped to 1-50. Adjustable from `/settings` → **Advanced**. | ### Paste Configuration diff --git a/docs/features/semantic-memory.md b/docs/features/semantic-memory.md index d9cca0641..2f798ca90 100644 --- a/docs/features/semantic-memory.md +++ b/docs/features/semantic-memory.md @@ -6,30 +6,35 @@ sidebar_order: 13 # Semantic Memory -Semantic memory lets you save durable facts about a project — architectural decisions, conventions, known issues, rejected approaches — so you don't have to re-explain them every session. Relevant memories are automatically recalled and injected into the system prompt as project context. +Semantic memory lets you save durable facts about a project - architectural decisions, conventions, known issues, rejected approaches - so you don't have to re-explain them every session. Relevant memories are automatically recalled and injected into the system prompt as project context. Memory creation is always manual and explicit. Nothing is ever saved automatically after a session; you decide what's worth remembering. ## Commands -- `/remember [--category ] ` — Save a memory directly -- `/memory list` — List all saved memories with their IDs and categories -- `/memory delete ` — Delete a specific memory -- `/memory clear` — Delete all memories for the current project -- `/memory propose` — Scan the current conversation for durable-sounding facts and print them as numbered proposals for review -- `/memory accept ` — Save proposal `n` from the most recent `/memory propose` output +- `/remember [--category ] ` - Save a memory directly. `-c` is a short form of `--category`. +- `/memory list` - List all saved memories with their short IDs and categories. `/memory ls` is an alias, and a bare `/memory` with no subcommand does the same thing. +- `/memory delete ` - Delete a specific memory. `/memory rm` is an alias. +- `/memory clear` - Delete all memories for the current project. +- `/memory propose` - Scan the recent conversation for durable-sounding facts and print them as numbered proposals for review. +- `/memory accept ` - Save proposal `n` from the most recent `/memory propose` output. ### Example ``` /remember The auth module uses Clerk and avoids middleware in the edge runtime. -/remember --category codingStyle Use camelCase for all variable names. +/remember -c codingStyle Use camelCase for all variable names. /memory list +/memory delete 18d51c0d /memory propose /memory accept 2 ``` +### Memory IDs + +`/memory list` prints an 8-character short ID for each memory, which is what you pass to `/memory delete`. The full UUID still works, as does any unambiguous prefix of either. If a prefix matches more than one memory, the command reports the ambiguity and deletes nothing rather than guessing. + ## Categories Memories are grouped into: `architecture`, `bugFix`, `refactor`, `todo`, `codingStyle`, or `project` (the default, for anything that doesn't match a more specific category). `/remember` infers a category automatically from the content unless you pass `--category`. @@ -38,26 +43,62 @@ Memories are grouped into: `architecture`, `bugFix`, `refactor`, `todo`, `coding When you send a message, Nanocoder ranks saved memories by relevance to that message (keyword overlap, with common words filtered out) and injects the most relevant ones into the system prompt under a `## Project Context` heading, up to a token budget. Low-relevance memories are dropped rather than injected as noise. -Recall works the same way across every interface: the interactive TUI, `nanocoder run` / `--plain`, and `--acp`. Each surface shows a `Recalling N project memories...` notice when memories are injected. +Retrieval is keyword-based, not a true embeddings/vector search. The "semantic" in the name refers to the kind of facts stored (durable project knowledge), not the matching technique. + +### Where recall is active + +Recall runs on the interactive TUI, on `nanocoder run` / `--plain`, and on `--acp`. Each of those shows a `Recalling N project memories...` notice when memories are injected. + +Recall does **not** currently run for subagent runs or for daemon-triggered skill runs. This is a deliberate limitation rather than an oversight: those runs are non-interactive, so nobody is present to notice a bad memory steering the run, and the failure mode is silent. Wiring recall into them is tracked as follow-up work, not shipped behaviour. + +### Tuning the budget + +Two settings bound how much of the context window project context may consume. Both are adjustable from `/settings` -> **Advanced**, which cycles through common presets, or by editing `nanocoder-preferences.json` directly for any value in range. + +| Preference key | Default | Range | Meaning | +|---|---|---|---| +| `semanticMemoryEnabled` | `true` | boolean | Master switch for recall and writes | +| `semanticMemoryTokenBudget` | `240` | 40 - 4000 | Approximate token ceiling for the injected block | +| `semanticMemoryLimit` | `8` | 1 - 50 | Maximum memories considered for one prompt | -Retrieval is keyword-based, not a true embeddings/vector search — the "semantic" in the name refers to the kind of facts stored (durable project knowledge), not the matching technique. +Values outside the supported range are clamped rather than rejected. On a small local model the 240-token default is a meaningful slice of the window, so lowering it is often the right call. ## Proposals -`/memory propose` looks back through the conversation for lines that read like durable facts (matched against the category keywords above) and prints them with their source (`explicit-user` or `conversation-inferred`) and a short evidence snippet. Nothing is saved until you run `/memory accept `. +`/memory propose` looks back through the recent conversation for lines that read like durable facts (matched against the category keywords above) and prints them with their source (`explicit-user` or `conversation-inferred`) and a short evidence snippet. Nothing is saved until you run `/memory accept `. -Proposals inferred purely from assistant text carry an `Inferred from conversation, no explicit user statement.` warning. If the assistant appears to be capitulating to pushback rather than stating a fact (opens with "you're right", "fair enough", etc., in response to a non-technical user message), the proposal is also flagged `Possible assistant position reversal.` — this catches the case where a model agreeing with a user's stylistic preference gets summarized into a "project convention" that was never actually decided. If you explicitly restate the same line yourself, the reversal warning is cleared, since your own statement is what actually resolves the ambiguity. +The scan covers the last 40 messages and prints at most 20 proposals, so a long session doesn't produce a list too large to review. Proposals without warnings are listed first. The printed numbering is fixed for as long as that list stands: accepting one proposal does not renumber the others, and accepting the same number twice is refused rather than repeated. Running `/clear` discards the list, since its evidence refers to a conversation you can no longer see. + +### Warnings + +Proposals inferred purely from assistant text carry an `Inferred from conversation, no explicit user statement.` warning. + +A proposal is additionally flagged `Possible assistant position reversal.` when the assistant turn looks like a concession to pressure rather than to evidence. That means the turn was preceded by a user message carrying no code, file path or error output, and the turn either contradicts an earlier assistant turn on the same subject or opens with an agreement phrase. Tool-call turns between the two are stepped over, so the check still works in a normal agentic session where the assistant reads files between turns. + +This catches the case where a model agreeing with a user's stylistic preference gets summarized into a "project convention" that was never actually decided. If you explicitly restate the same line yourself, the reversal warning is cleared, since your own statement is what actually resolves the ambiguity. + +The check is a heuristic tuned to over-flag rather than miss: it only adds a warning to a proposal you are already reviewing by hand, so a spurious warning costs you a moment's attention while a missed one costs you a false project convention. ## Turning It Off -Semantic memory is on by default. Toggle it from `/settings` → **Semantic Memory**. Turning it off disables both recall (memories are no longer injected into prompts) and writes (`/remember` and `/memory accept` are refused while it's off). +Semantic memory is on by default. Toggle it from `/settings` -> **Advanced** -> **Semantic Memory**. Turning it off disables both recall (memories are no longer injected into prompts) and writes (`/remember` and `/memory accept` are refused while it's off). ## Storage and Scope -Memories are stored per-repository in a local JSON file under your app data directory, keyed by a hash of the repository's `git remote origin.url` (or its absolute path, for non-git directories). This means: +Memories are stored per-repository in a local JSON file under the Nanocoder data directory: + +| Platform | Path | +|---|---| +| macOS | `~/Library/Application Support/nanocoder/memory/` | +| Linux | `~/.local/share/nanocoder/memory/` (or `$XDG_DATA_HOME/nanocoder/memory/`) | +| Windows | `%APPDATA%\nanocoder\memory\` | + +Setting `NANOCODER_DATA_DIR` overrides all of these. + +The filename is a hash of the repository's `git remote origin.url`, or of its absolute path for non-git directories. This means: - All branches, worktrees, and local clones that share the same `origin` remote share one memory pool. - Forks with a different `origin` get their own, separate pool. -- This scope isn't currently configurable — if you work across branches with genuinely divergent conventions in the same repository, they'll share memories. +- This scope isn't currently configurable. If you work across branches with genuinely divergent conventions in the same repository, they'll share memories. -Files are written atomically (temp file + rename) with restrictive permissions (`0600` on the file, `0700` on the directory), and nothing ever leaves your machine. +Files are written atomically (temp file + rename) with restrictive permissions (`0600` on the file, `0700` on the directory), and nothing ever leaves your machine. Memory content is fenced when injected into the system prompt, with the fence widened as needed so content containing backticks cannot break out of it. diff --git a/source/acp/acp-agent.ts b/source/acp/acp-agent.ts index 6d005308d..403ec49c8 100644 --- a/source/acp/acp-agent.ts +++ b/source/acp/acp-agent.ts @@ -34,7 +34,7 @@ import type {AcpInitContext} from '@/acp/acp-types'; import {appendToolDefinitionsToPrompt} from '@/ai-sdk-client/tools/system-prompt-assembler'; import {getAppConfig} from '@/config/index'; import { - getSemanticMemoryEnabled, + getProjectContextPreferences, loadPreferences, updateLastUsed, } from '@/config/preferences'; @@ -160,7 +160,7 @@ export class AcpAgent implements Agent { session.baseSystemMessage.content, userText, new SemanticMemoryManager({cwd: session.cwd}), - {semanticMemoryEnabled: getSemanticMemoryEnabled()}, + getProjectContextPreferences(), ); session.systemMessage = { role: 'system', diff --git a/source/app/components/settings-selector.tsx b/source/app/components/settings-selector.tsx index 0a4649e41..4e4b15317 100644 --- a/source/app/components/settings-selector.tsx +++ b/source/app/components/settings-selector.tsx @@ -12,8 +12,8 @@ import { getNotificationsPreference, getPasteThreshold, getPrivacyPreference, + getProjectContextPreferences, getReasoningExpanded, - getSemanticMemoryEnabled, updateCompactToolDisplay, updateNanocoderShape, updateNotificationsPreference, @@ -22,6 +22,8 @@ import { updateReasoningExpanded, updateSelectedTheme, updateSemanticMemoryEnabled, + updateSemanticMemoryLimit, + updateSemanticMemoryTokenBudget, } from '@/config/preferences'; import {getThemeColors, themes} from '@/config/themes'; import {useResponsiveTerminal} from '@/hooks/useTerminalWidth'; @@ -1045,6 +1047,18 @@ export function SettingsSelector({onCancel}: SettingsSelectorProps) { } } +/** Presets cycled by the Advanced panel. Any value in range can still be set + * directly in nanocoder-preferences.json; these are just the common choices. */ +const TOKEN_BUDGET_PRESETS = [120, 240, 480, 960]; +const MEMORY_LIMIT_PRESETS = [3, 5, 8, 12]; + +/** Next preset after `current`, wrapping. Falls to the first when `current` + * is a hand-edited value that isn't in the list. */ +function cyclePreset(presets: number[], current: number): number { + const index = presets.indexOf(current); + return presets[(index + 1) % presets.length] ?? presets[0] ?? current; +} + // Advanced settings panel function SettingsAdvancedPanel({ onBack, @@ -1056,8 +1070,15 @@ function SettingsAdvancedPanel({ const {boxWidth, isNarrow} = useResponsiveTerminal(); const {colors} = useTheme(); + const initialContextPreferences = getProjectContextPreferences(); const [semanticMemoryEnabled, setSemanticMemoryEnabled] = useState( - getSemanticMemoryEnabled(), + initialContextPreferences.semanticMemoryEnabled, + ); + const [tokenBudget, setTokenBudget] = useState( + initialContextPreferences.tokenBudget, + ); + const [memoryLimit, setMemoryLimit] = useState( + initialContextPreferences.memoryLimit, ); useInput((_, key) => { @@ -1075,13 +1096,38 @@ function SettingsAdvancedPanel({ label: `Semantic Memory: ${semanticMemoryEnabled ? 'ON' : 'OFF'}`, value: 'semantic-memory', }, + { + label: `Memory Token Budget: ${tokenBudget}`, + value: 'semantic-memory-token-budget', + }, + { + label: `Memories Per Prompt: ${memoryLimit}`, + value: 'semantic-memory-limit', + }, ]; - }, [semanticMemoryEnabled]); - - const handleSelect = () => { - const next = !semanticMemoryEnabled; - setSemanticMemoryEnabled(next); - updateSemanticMemoryEnabled(next); + }, [semanticMemoryEnabled, tokenBudget, memoryLimit]); + + const handleSelect = (item: {value: string}) => { + switch (item.value) { + case 'semantic-memory': { + const next = !semanticMemoryEnabled; + setSemanticMemoryEnabled(next); + updateSemanticMemoryEnabled(next); + break; + } + case 'semantic-memory-token-budget': { + const next = cyclePreset(TOKEN_BUDGET_PRESETS, tokenBudget); + setTokenBudget(next); + updateSemanticMemoryTokenBudget(next); + break; + } + case 'semantic-memory-limit': { + const next = cyclePreset(MEMORY_LIMIT_PRESETS, memoryLimit); + setMemoryLimit(next); + updateSemanticMemoryLimit(next); + break; + } + } }; const title = isNarrow ? 'Advanced' : 'Advanced Settings'; @@ -1107,7 +1153,9 @@ function SettingsAdvancedPanel({ Semantic Memory recalls saved project context and injects it into - future prompts. Turn it off for stateless agent behavior. + future prompts. Turn it off for stateless agent behavior. The budget + and per-prompt count bound how much of the context window it may + consume - lower them on small local models. diff --git a/source/app/utils/app-util.ts b/source/app/utils/app-util.ts index bb3dfc9ea..cdbd35429 100644 --- a/source/app/utils/app-util.ts +++ b/source/app/utils/app-util.ts @@ -5,6 +5,7 @@ import {CodexLogin} from '@/commands/codex-login'; import {CopilotLogin} from '@/commands/copilot-login'; import BashProgress from '@/components/bash-progress'; import {DELAY_COMMAND_COMPLETE_MS, MAX_SESSION_NAME_LENGTH} from '@/constants'; +import {sharedProposalStore} from '@/memory/proposal-store'; import {CheckpointManager} from '@/services/checkpoint-manager'; import {generateKey} from '@/session/key-generator'; import {executeBashCommand, formatBashResultForLLM} from '@/tools/execute-bash'; @@ -274,6 +275,9 @@ async function handleSpecialCommand( case SPECIAL_COMMANDS.CLEAR: await onClearMessages(); await clearAllTasks(); + // Proposals reference a conversation that no longer exists; accepting + // one after /clear would save evidence the user can no longer see. + sharedProposalStore.clear(); onAddToChatQueue(successMsg('Chat and tasks cleared.', 'clear-success')); setTimeout(() => onCommandComplete?.(), DELAY_COMMAND_COMPLETE_MS); return true; diff --git a/source/commands/memory.spec.tsx b/source/commands/memory.spec.tsx index 342532997..57ee9719c 100644 --- a/source/commands/memory.spec.tsx +++ b/source/commands/memory.spec.tsx @@ -1,5 +1,6 @@ import test from 'ava'; import React from 'react'; +import {ProposalStore} from '@/memory/proposal-store'; import type {SemanticMemory} from '@/memory/semantic-memory-manager'; import type {MemoryProposal} from '@/memory/summarizer-service'; import {renderWithTheme} from '@/test-utils/render-with-theme'; @@ -87,8 +88,9 @@ test('memory command lists saved memories', async t => { const {lastFrame} = renderWithTheme(result as React.ReactElement); const output = lastFrame() ?? ''; - t.true(output.includes('memory-1')); - t.true(output.includes('[architecture]')); + // Listed by short id rather than the raw UUID. + t.true(output.includes('memory1')); + t.true(output.includes('architecture')); t.true(output.includes('Auth uses Clerk.')); }); @@ -107,7 +109,7 @@ test('memory command deletes a memory', async t => { const result = await command.handler(['delete', 'memory-1'], [], testMetadata); const {lastFrame} = renderWithTheme(result as React.ReactElement); - t.true((lastFrame() ?? '').includes('Deleted memory: memory-1')); + t.true((lastFrame() ?? '').includes('Deleted memory: memory1')); t.deepEqual(manager.memories, []); }); @@ -263,3 +265,183 @@ test('lazy registry exposes /memory', t => { t.truthy(memory); t.is(memory?.description, 'Manage project memories'); }); + +// --- Accept indexing: the round-3 review's merge blocker. Accepting a proposal +// must not renumber the list the user is still reading off screen. --- + +function proposal(content: string, category = 'architecture'): MemoryProposal { + return { + content, + category, + sourceType: 'explicit-user', + evidence: {userMessages: [content], assistantMessages: []}, + warnings: [], + }; +} + +const FOUR_PROPOSALS = [ + proposal('Proposal one.'), + proposal('Proposal two.'), + proposal('Proposal three.'), + proposal('Proposal four.'), +]; + +test('memory accept keeps indices stable across successive accepts', async t => { + const summarizerService = new FakeSummarizerService(FOUR_PROPOSALS); + const command = createMemoryCommand({ + memoryManager: new FakeMemoryManager(), + summarizerService, + }); + + await command.handler( + ['propose'], + [{role: 'user', content: 'seed'}], + testMetadata, + ); + await command.handler(['accept', '2'], [], testMetadata); + await command.handler(['accept', '3'], [], testMetadata); + + // Before the fix the second accept saved "Proposal four." because the list + // was re-indexed after the first accept. + t.deepEqual(summarizerService.accepted, [ + {content: 'Proposal two.', category: 'architecture'}, + {content: 'Proposal three.', category: 'architecture'}, + ]); +}); + +test('memory accept refuses to save the same proposal twice', async t => { + const summarizerService = new FakeSummarizerService(FOUR_PROPOSALS); + const command = createMemoryCommand({ + memoryManager: new FakeMemoryManager(), + summarizerService, + }); + + await command.handler( + ['propose'], + [{role: 'user', content: 'seed'}], + testMetadata, + ); + await command.handler(['accept', '2'], [], testMetadata); + const result = await command.handler(['accept', '2'], [], testMetadata); + const {lastFrame} = renderWithTheme(result as React.ReactElement); + + t.true((lastFrame() ?? '').includes('Proposal 2 was already saved.')); + t.is(summarizerService.accepted.length, 1); +}); + +test('memory accept is reset when the proposal store is cleared', async t => { + const store = new ProposalStore(); + const summarizerService = new FakeSummarizerService(FOUR_PROPOSALS); + const command = createMemoryCommand({ + memoryManager: new FakeMemoryManager(), + summarizerService, + proposalStore: store, + }); + + await command.handler( + ['propose'], + [{role: 'user', content: 'seed'}], + testMetadata, + ); + // What /clear does. + store.clear(); + + const result = await command.handler(['accept', '1'], [], testMetadata); + const {lastFrame} = renderWithTheme(result as React.ReactElement); + + t.true( + (lastFrame() ?? '').includes('No proposals to accept. Run /memory propose first.'), + ); + t.deepEqual(summarizerService.accepted, []); +}); + +// --- Short ids --- + +const UUID_A = '18d51c0d-becb-4efc-8d0d-b8c1f3b61802'; +const UUID_B = '18d51c0d-0000-4efc-8d0d-b8c1f3b61802'; +const UUID_C = 'ff000000-1111-4efc-8d0d-b8c1f3b61802'; + +function storedMemory(id: string, content: string): SemanticMemory { + return { + id, + content, + category: 'architecture', + timestamp: '2026-07-21T00:00:00.000Z', + }; +} + +test('memory list shows a short id instead of the raw UUID', async t => { + const manager = new FakeMemoryManager(); + manager.memories = [storedMemory(UUID_A, 'Auth uses Clerk.')]; + const command = createMemoryCommand({memoryManager: manager}); + + const result = await command.handler(['list'], [], testMetadata); + const {lastFrame} = renderWithTheme(result as React.ReactElement); + const output = lastFrame() ?? ''; + + t.true(output.includes('18d51c0d')); + t.false(output.includes(UUID_A)); +}); + +test('memory delete accepts a short id', async t => { + const manager = new FakeMemoryManager(); + manager.memories = [storedMemory(UUID_C, 'Auth uses Clerk.')]; + const command = createMemoryCommand({memoryManager: manager}); + + const result = await command.handler(['delete', 'ff000000'], [], testMetadata); + const {lastFrame} = renderWithTheme(result as React.ReactElement); + + t.true((lastFrame() ?? '').includes('Deleted memory: ff000000')); + t.deepEqual(manager.memories, []); +}); + +test('memory delete still accepts a full UUID', async t => { + const manager = new FakeMemoryManager(); + manager.memories = [storedMemory(UUID_C, 'Auth uses Clerk.')]; + const command = createMemoryCommand({memoryManager: manager}); + + await command.handler(['delete', UUID_C], [], testMetadata); + + t.deepEqual(manager.memories, []); +}); + +test('memory delete reports an ambiguous short id instead of guessing', async t => { + const manager = new FakeMemoryManager(); + manager.memories = [ + storedMemory(UUID_A, 'Auth uses Clerk.'), + storedMemory(UUID_B, 'Storage uses SQLite.'), + ]; + const command = createMemoryCommand({memoryManager: manager}); + + const result = await command.handler(['delete', '18d51c0d'], [], testMetadata); + const {lastFrame} = renderWithTheme(result as React.ReactElement); + + t.true((lastFrame() ?? '').includes('Ambiguous memory id')); + t.is(manager.memories.length, 2); +}); + +test('bare /memory defaults to list', async t => { + const manager = new FakeMemoryManager(); + const command = createMemoryCommand({memoryManager: manager}); + + const result = await command.handler([], [], testMetadata); + const {lastFrame} = renderWithTheme(result as React.ReactElement); + + t.true((lastFrame() ?? '').includes('No project memories saved.')); +}); + +test('memory ls and rm aliases behave like list and delete', async t => { + const manager = new FakeMemoryManager(); + manager.memories = [storedMemory(UUID_C, 'Auth uses Clerk.')]; + const command = createMemoryCommand({memoryManager: manager}); + + const listed = await command.handler(['ls'], [], testMetadata); + t.true( + (renderWithTheme(listed as React.ReactElement).lastFrame() ?? '').includes( + 'Auth uses Clerk.', + ), + ); + + await command.handler(['rm', 'ff000000'], [], testMetadata); + t.deepEqual(manager.memories, []); +}); diff --git a/source/commands/memory.ts b/source/commands/memory.ts deleted file mode 100644 index 8e77e0cca..000000000 --- a/source/commands/memory.ts +++ /dev/null @@ -1,166 +0,0 @@ -import {SemanticMemoryManager} from '@/memory/semantic-memory-manager'; -import type {MemoryProposal} from '@/memory/summarizer-service'; -import {SummarizerService} from '@/memory/summarizer-service'; -import type {Command} from '@/types/commands'; -import {formatError} from '@/utils/error-formatter'; -import {errorMsg, infoMsg, successMsg} from '@/utils/message-factory'; - -interface MemoryCommandOptions { - memoryManager?: Pick< - SemanticMemoryManager, - 'listMemories' | 'deleteMemory' | 'clearMemories' - >; - summarizerService?: Pick< - SummarizerService, - 'proposeMemoriesFromMessages' | 'acceptProposal' - >; -} - -const USAGE = - 'Usage: /memory list | /memory delete | /memory clear | /memory propose | /memory accept '; - -export function createMemoryCommand( - options: MemoryCommandOptions = {}, -): Command { - const memoryManager = options.memoryManager ?? new SemanticMemoryManager(); - const summarizerService = - options.summarizerService ?? new SummarizerService(); - let lastProposals: MemoryProposal[] = []; - - return { - name: 'memory', - description: 'Manage project memories', - handler: async (args, messages) => { - const subcommand = args[0]?.toLowerCase() ?? 'list'; - - try { - if (subcommand === 'list' || subcommand === 'ls') { - const memories = await memoryManager.listMemories(); - if (memories.length === 0) { - return infoMsg('No project memories saved.', 'memory-list'); - } - - return infoMsg( - memories - .map( - memory => `${memory.id} [${memory.category}] ${memory.content}`, - ) - .join('\n'), - 'memory-list', - ); - } - - if (subcommand === 'delete' || subcommand === 'rm') { - const id = args[1]?.trim(); - if (!id) return errorMsg(USAGE, 'memory-error'); - - const deleted = await memoryManager.deleteMemory(id); - if (!deleted) { - return errorMsg(`Memory not found: ${id}`, 'memory-error'); - } - - return successMsg(`Deleted memory: ${id}`, 'memory-deleted'); - } - - if (subcommand === 'clear') { - await memoryManager.clearMemories(); - return successMsg('Cleared project memories.', 'memory-cleared'); - } - - if (subcommand === 'propose') { - const proposals = - summarizerService.proposeMemoriesFromMessages(messages); - if (proposals.length === 0) { - lastProposals = []; - return infoMsg( - 'No durable memory proposals found.', - 'memory-propose', - ); - } - - proposals.sort((a, b) => { - const aWarns = a.warnings.length; - const bWarns = b.warnings.length; - if (aWarns === 0 && bWarns > 0) return -1; - if (aWarns > 0 && bWarns === 0) return 1; - return 0; - }); - lastProposals = proposals; - - const lines: string[] = []; - for (let i = 0; i < proposals.length; i++) { - const proposal = proposals[i]!; - const hasWarnings = proposal.warnings.length > 0; - const header = `─────────────────────────────────\nProposal ${i + 1} of ${proposals.length}${hasWarnings ? ' ⚠ Review carefully' : ''}\n─────────────────────────────────`; - lines.push(header); - lines.push(`[${proposal.category}] ${proposal.content}\n`); - lines.push(`Source: ${proposal.sourceType}`); - - const evidenceLines: string[] = []; - for (const m of proposal.evidence.userMessages) { - evidenceLines.push(`User: "${m}"`); - } - for (const m of proposal.evidence.assistantMessages) { - evidenceLines.push(`Assistant: "${m}"`); - } - - if (evidenceLines.length > 0) { - lines.push(`Evidence: ${evidenceLines[0]}`); - for (let j = 1; j < evidenceLines.length; j++) { - lines.push(` ${evidenceLines[j]}`); - } - } - - for (const w of proposal.warnings) { - lines.push(`⚠ ${w}`); - } - lines.push(''); - } - - lines.push( - `─────────────────────────────────\nRun /memory accept <1-${proposals.length}> to save one of the above.`, - ); - - return infoMsg(lines.join('\n'), 'memory-propose'); - } - - if (subcommand === 'accept') { - if (lastProposals.length === 0) { - return errorMsg( - 'No proposals to accept. Run /memory propose first.', - 'memory-error', - ); - } - - const index = Number.parseInt(args[1] ?? '', 10); - const proposal = Number.isInteger(index) - ? lastProposals[index - 1] - : undefined; - if (!proposal) { - return errorMsg( - `Usage: /memory accept <1-${lastProposals.length}>`, - 'memory-error', - ); - } - - const memory = await summarizerService.acceptProposal(proposal); - lastProposals = lastProposals.filter((_, i) => i !== index - 1); - - return successMsg( - `Saved ${memory.category} memory: ${memory.content}`, - 'memory-accept', - ); - } - - return errorMsg(USAGE, 'memory-error'); - } catch (error) { - return errorMsg( - `Failed to manage memory: ${formatError(error)}`, - 'memory-error', - ); - } - }, - }; -} - -export const memoryCommand: Command = createMemoryCommand(); diff --git a/source/commands/memory.tsx b/source/commands/memory.tsx new file mode 100644 index 000000000..abdfdc02b --- /dev/null +++ b/source/commands/memory.tsx @@ -0,0 +1,318 @@ +import {Box, Text} from 'ink'; +import {TitledBoxWithPreferences} from '@/components/ui/titled-box'; +import {useTerminalWidth} from '@/hooks/useTerminalWidth'; +import {useTheme} from '@/hooks/useTheme'; +import {ProposalStore, sharedProposalStore} from '@/memory/proposal-store'; +import type {SemanticMemory} from '@/memory/semantic-memory-manager'; +import {SemanticMemoryManager} from '@/memory/semantic-memory-manager'; +import type {MemoryProposal} from '@/memory/summarizer-service'; +import {SummarizerService} from '@/memory/summarizer-service'; +import type {Command} from '@/types/commands'; +import {formatError} from '@/utils/error-formatter'; +import {errorMsg, infoMsg, successMsg} from '@/utils/message-factory'; + +interface MemoryCommandOptions { + memoryManager?: Pick< + SemanticMemoryManager, + 'listMemories' | 'deleteMemory' | 'clearMemories' + >; + summarizerService?: Pick< + SummarizerService, + 'proposeMemoriesFromMessages' | 'acceptProposal' + >; + proposalStore?: ProposalStore; +} + +const USAGE = + 'Usage: /memory list | /memory delete | /memory clear | /memory propose | /memory accept '; + +/** Length of the display id. Long enough to stay unique in a realistic pool, + * short enough to retype without copying out of wrapped terminal output. */ +const SHORT_ID_LENGTH = 8; + +export function shortMemoryId(id: string): string { + return id.replaceAll('-', '').slice(0, SHORT_ID_LENGTH); +} + +type IdLookup = + | {kind: 'found'; memory: SemanticMemory} + | {kind: 'missing'} + | {kind: 'ambiguous'; matches: SemanticMemory[]}; + +/** Accepts a short id, a full UUID, or any unambiguous prefix of either. */ +export function resolveMemoryId( + memories: SemanticMemory[], + input: string, +): IdLookup { + const needle = input.trim().toLowerCase(); + if (!needle) return {kind: 'missing'}; + + const exact = memories.find(memory => memory.id.toLowerCase() === needle); + if (exact) return {kind: 'found', memory: exact}; + + const matches = memories.filter(memory => { + const compact = memory.id.replaceAll('-', '').toLowerCase(); + return ( + compact.startsWith(needle.replaceAll('-', '')) || + memory.id.toLowerCase().startsWith(needle) + ); + }); + + if (matches.length === 0) return {kind: 'missing'}; + if (matches.length > 1) return {kind: 'ambiguous', matches}; + return {kind: 'found', memory: matches[0] as SemanticMemory}; +} + +function MemoryList({memories}: {memories: SemanticMemory[]}) { + const {colors} = useTheme(); + const width = useTerminalWidth(); + + return ( + + {memories.map((memory, index) => ( + + + + {shortMemoryId(memory.id)} + + · {memory.category} + + + {memory.content} + + + ))} + + + + {memories.length} memor{memories.length === 1 ? 'y' : 'ies'} · delete + one with /memory delete <id> + + + + ); +} + +function ProposalEvidence({proposal}: {proposal: MemoryProposal}) { + const {colors} = useTheme(); + const rows = [ + ...proposal.evidence.userMessages.map(text => ({label: 'User', text})), + ...proposal.evidence.assistantMessages.map(text => ({ + label: 'Assistant', + text, + })), + ]; + + if (rows.length === 0) return null; + + return ( + + {rows.map(row => ( + + {row.label}: "{row.text}" + + ))} + + ); +} + +function MemoryProposals({proposals}: {proposals: readonly MemoryProposal[]}) { + const {colors} = useTheme(); + const width = useTerminalWidth(); + + return ( + + {proposals.map((proposal, index) => { + const hasWarnings = proposal.warnings.length > 0; + return ( + + + + {index + 1}. + + [{proposal.category}] + {proposal.sourceType} + {hasWarnings && ( + · review carefully + )} + + + {proposal.content} + + + {proposal.warnings.map(warning => ( + + ⚠ {warning} + + ))} + + ); + })} + + + + Save one with /memory accept <1-{proposals.length}> + + + + ); +} + +export function createMemoryCommand( + options: MemoryCommandOptions = {}, +): Command { + const memoryManager = options.memoryManager ?? new SemanticMemoryManager(); + const summarizerService = + options.summarizerService ?? new SummarizerService(); + // Defaults to a private store; only the exported singleton binds the shared + // one, so tests and any ad-hoc instance can't clobber each other's state. + const proposalStore = options.proposalStore ?? new ProposalStore(); + + return { + name: 'memory', + description: 'Manage project memories', + handler: async (args, messages) => { + const subcommand = args[0]?.toLowerCase() ?? 'list'; + + try { + if (subcommand === 'list' || subcommand === 'ls') { + const memories = await memoryManager.listMemories(); + if (memories.length === 0) { + return infoMsg('No project memories saved.', 'memory-list'); + } + + return ; + } + + if (subcommand === 'delete' || subcommand === 'rm') { + const id = args[1]?.trim(); + if (!id) return errorMsg(USAGE, 'memory-error'); + + const lookup = resolveMemoryId( + await memoryManager.listMemories(), + id, + ); + if (lookup.kind === 'missing') { + return errorMsg(`Memory not found: ${id}`, 'memory-error'); + } + if (lookup.kind === 'ambiguous') { + const ids = lookup.matches + .map(memory => shortMemoryId(memory.id)) + .join(', '); + return errorMsg( + `Ambiguous memory id "${id}" matches: ${ids}`, + 'memory-error', + ); + } + + const deleted = await memoryManager.deleteMemory(lookup.memory.id); + if (!deleted) { + return errorMsg(`Memory not found: ${id}`, 'memory-error'); + } + + return successMsg( + `Deleted memory: ${shortMemoryId(lookup.memory.id)}`, + 'memory-deleted', + ); + } + + if (subcommand === 'clear') { + await memoryManager.clearMemories(); + proposalStore.clear(); + return successMsg('Cleared project memories.', 'memory-cleared'); + } + + if (subcommand === 'propose') { + const proposals = + summarizerService.proposeMemoriesFromMessages(messages); + if (proposals.length === 0) { + proposalStore.clear(); + return infoMsg( + 'No durable memory proposals found.', + 'memory-propose', + ); + } + + // Warning-free proposals first, so the safest choices carry the + // lowest numbers. Order is fixed here and never changes again - + // `/memory accept` indexes into exactly this list. + proposals.sort( + (a, b) => + (a.warnings.length === 0 ? 0 : 1) - + (b.warnings.length === 0 ? 0 : 1), + ); + proposalStore.set(proposals); + + return ; + } + + if (subcommand === 'accept') { + if (proposalStore.size === 0) { + return errorMsg( + 'No proposals to accept. Run /memory propose first.', + 'memory-error', + ); + } + + const index = Number.parseInt(args[1] ?? '', 10); + const proposal = proposalStore.at(index); + if (!proposal) { + return errorMsg( + `Usage: /memory accept <1-${proposalStore.size}>`, + 'memory-error', + ); + } + if (proposalStore.isAccepted(index)) { + return errorMsg( + `Proposal ${index} was already saved.`, + 'memory-error', + ); + } + + const memory = await summarizerService.acceptProposal(proposal); + proposalStore.markAccepted(index); + + return successMsg( + `Saved ${memory.category} memory: ${memory.content}`, + 'memory-accept', + ); + } + + return errorMsg(USAGE, 'memory-error'); + } catch (error) { + return errorMsg( + `Failed to manage memory: ${formatError(error)}`, + 'memory-error', + ); + } + }, + }; +} + +export const memoryCommand: Command = createMemoryCommand({ + proposalStore: sharedProposalStore, +}); diff --git a/source/config/preferences.spec.ts b/source/config/preferences.spec.ts index 705ca4bee..6527d11c5 100644 --- a/source/config/preferences.spec.ts +++ b/source/config/preferences.spec.ts @@ -2,16 +2,26 @@ import {existsSync, mkdirSync, readFileSync, rmSync, writeFileSync} from 'node:f import {tmpdir} from 'node:os'; import {join} from 'node:path'; import test from 'ava'; +import { + DEFAULT_MEMORY_LIMIT, + DEFAULT_TOKEN_BUDGET, + MAX_MEMORY_LIMIT, + MAX_TOKEN_BUDGET, + MIN_MEMORY_LIMIT, + MIN_TOKEN_BUDGET, +} from '@/memory/project-context'; import { getCompactToolDisplay, getLastUsedModel, getNanocoderShape, getNotificationsPreference, getPasteThreshold, + getProjectContextPreferences, getReasoningExpanded, getSemanticMemoryEnabled, loadPreferences, resetPreferencesCache, + resolveProjectContextPreferences, savePreferences, updateCompactToolDisplay, updateLastUsed, @@ -20,6 +30,8 @@ import { updatePasteThreshold, updateReasoningExpanded, updateSemanticMemoryEnabled, + updateSemanticMemoryLimit, + updateSemanticMemoryTokenBudget, getPrivacyPreference, updatePrivacyPreference, } from './preferences'; @@ -1615,3 +1627,126 @@ test.serial('updateSemanticMemoryEnabled saves the preference correctly', t => { } } }); + +// ============================================================================ +// Project Context Preferences Tests (token budget + memory limit, round-4 review) +// ============================================================================ + +test('resolveProjectContextPreferences falls back to the shipped defaults', t => { + t.deepEqual(resolveProjectContextPreferences({} as UserPreferences), { + semanticMemoryEnabled: true, + memoryLimit: DEFAULT_MEMORY_LIMIT, + tokenBudget: DEFAULT_TOKEN_BUDGET, + }); +}); + +test('resolveProjectContextPreferences honours configured values', t => { + t.deepEqual( + resolveProjectContextPreferences({ + semanticMemoryEnabled: false, + semanticMemoryLimit: 3, + semanticMemoryTokenBudget: 120, + } as UserPreferences), + {semanticMemoryEnabled: false, memoryLimit: 3, tokenBudget: 120}, + ); +}); + +test('resolveProjectContextPreferences clamps out-of-range values', t => { + const tooLow = resolveProjectContextPreferences({ + semanticMemoryLimit: 0, + semanticMemoryTokenBudget: 1, + } as UserPreferences); + t.is(tooLow.memoryLimit, MIN_MEMORY_LIMIT); + t.is(tooLow.tokenBudget, MIN_TOKEN_BUDGET); + + const tooHigh = resolveProjectContextPreferences({ + semanticMemoryLimit: 10_000, + semanticMemoryTokenBudget: 10_000, + } as UserPreferences); + t.is(tooHigh.memoryLimit, MAX_MEMORY_LIMIT); + t.is(tooHigh.tokenBudget, MAX_TOKEN_BUDGET); +}); + +test.serial('getProjectContextPreferences reads token budget and memory limit from disk', t => { + const preferencesPath = getTestPreferencesPath(); + const data: UserPreferences = { + semanticMemoryEnabled: true, + semanticMemoryLimit: 12, + semanticMemoryTokenBudget: 480, + }; + writeFileSync(preferencesPath, JSON.stringify(data, null, 2), 'utf-8'); + + try { + t.deepEqual(getProjectContextPreferences(), { + semanticMemoryEnabled: true, + memoryLimit: 12, + tokenBudget: 480, + }); + } finally { + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + } +}); + +test.serial('updateSemanticMemoryLimit saves a clamped value', t => { + const preferencesPath = getTestPreferencesPath(); + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + + try { + updateSemanticMemoryLimit(500); + + const content = readFileSync(preferencesPath, 'utf-8'); + const parsed = JSON.parse(content) as UserPreferences; + + t.is(parsed.semanticMemoryLimit, MAX_MEMORY_LIMIT); + } finally { + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + } +}); + +test.serial('updateSemanticMemoryTokenBudget saves a clamped value', t => { + const preferencesPath = getTestPreferencesPath(); + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + + try { + updateSemanticMemoryTokenBudget(1); + + const content = readFileSync(preferencesPath, 'utf-8'); + const parsed = JSON.parse(content) as UserPreferences; + + t.is(parsed.semanticMemoryTokenBudget, MIN_TOKEN_BUDGET); + } finally { + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + } +}); + +test.serial('full workflow: update and retrieve project context preferences', t => { + const preferencesPath = getTestPreferencesPath(); + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + + try { + updateSemanticMemoryLimit(5); + updateSemanticMemoryTokenBudget(960); + + t.deepEqual(getProjectContextPreferences(), { + semanticMemoryEnabled: true, + memoryLimit: 5, + tokenBudget: 960, + }); + } finally { + if (existsSync(preferencesPath)) { + rmSync(preferencesPath, {force: true}); + } + } +}); diff --git a/source/config/preferences.ts b/source/config/preferences.ts index 20d23e402..34763955c 100644 --- a/source/config/preferences.ts +++ b/source/config/preferences.ts @@ -1,6 +1,15 @@ import {readFileSync, writeFileSync} from 'fs'; import type {TitleShape} from '@/components/ui/styled-title'; import {getClosestConfigFile} from '@/config/index'; +import { + DEFAULT_MEMORY_LIMIT, + DEFAULT_TOKEN_BUDGET, + MAX_MEMORY_LIMIT, + MAX_TOKEN_BUDGET, + MIN_MEMORY_LIMIT, + MIN_TOKEN_BUDGET, + type ProjectContextOptions, +} from '@/memory/project-context'; import type {TuneConfig} from '@/types/config'; import type {UserPreferences} from '@/types/index'; import type {NanocoderShape, ThemePreset} from '@/types/ui'; @@ -193,12 +202,52 @@ export function updatePrivacyPreference(value: boolean): void { savePreferences(preferences); } +function clamp(value: number, min: number, max: number): number { + return Math.min(max, Math.max(min, Math.round(value))); +} + +/** + * Resolve the project-context knobs from an already-loaded preferences object. + * + * The single place the semantic-memory defaults live. Callers that inject + * `loadPreferences` (the plain shell) pass their own object in; everything else + * goes through {@link getProjectContextPreferences}. + */ +export function resolveProjectContextPreferences( + preferences: UserPreferences, +): Required< + Pick< + ProjectContextOptions, + 'semanticMemoryEnabled' | 'memoryLimit' | 'tokenBudget' + > +> { + return { + semanticMemoryEnabled: preferences.semanticMemoryEnabled ?? true, + memoryLimit: clamp( + preferences.semanticMemoryLimit ?? DEFAULT_MEMORY_LIMIT, + MIN_MEMORY_LIMIT, + MAX_MEMORY_LIMIT, + ), + tokenBudget: clamp( + preferences.semanticMemoryTokenBudget ?? DEFAULT_TOKEN_BUDGET, + MIN_TOKEN_BUDGET, + MAX_TOKEN_BUDGET, + ), + }; +} + +/** Project-context knobs for the current user. */ +export function getProjectContextPreferences(): ReturnType< + typeof resolveProjectContextPreferences +> { + return resolveProjectContextPreferences(loadPreferences()); +} + /** * Get the semantic memory preference from preferences */ export function getSemanticMemoryEnabled(): boolean { - const preferences = loadPreferences(); - return preferences.semanticMemoryEnabled ?? true; + return getProjectContextPreferences().semanticMemoryEnabled; } /** @@ -209,3 +258,29 @@ export function updateSemanticMemoryEnabled(value: boolean): void { preferences.semanticMemoryEnabled = value; savePreferences(preferences); } + +/** + * Save how many memories may be recalled into a single prompt. + */ +export function updateSemanticMemoryLimit(value: number): void { + const preferences = loadPreferences(); + preferences.semanticMemoryLimit = clamp( + value, + MIN_MEMORY_LIMIT, + MAX_MEMORY_LIMIT, + ); + savePreferences(preferences); +} + +/** + * Save the token budget project context may consume in the system prompt. + */ +export function updateSemanticMemoryTokenBudget(value: number): void { + const preferences = loadPreferences(); + preferences.semanticMemoryTokenBudget = clamp( + value, + MIN_TOKEN_BUDGET, + MAX_TOKEN_BUDGET, + ); + savePreferences(preferences); +} diff --git a/source/hooks/chat-handler/useChatHandler.tsx b/source/hooks/chat-handler/useChatHandler.tsx index a380d452a..956e9950a 100644 --- a/source/hooks/chat-handler/useChatHandler.tsx +++ b/source/hooks/chat-handler/useChatHandler.tsx @@ -3,7 +3,7 @@ import {appendToolDefinitionsToPrompt} from '@/ai-sdk-client/tools/system-prompt import {ConversationStateManager} from '@/app/utils/conversation-state'; import UserMessage from '@/components/user-message'; import {getAppConfig} from '@/config/index'; -import {getSemanticMemoryEnabled} from '@/config/preferences'; +import {getProjectContextPreferences} from '@/config/preferences'; import {CommandIntegration} from '@/custom-commands/command-integration'; import {appendRelevantProjectContextWithCount} from '@/memory/project-context'; import {SemanticMemoryManager} from '@/memory/semantic-memory-manager'; @@ -372,10 +372,9 @@ export function useChatHandler({ systemPrompt, message, projectMemoryFinder, - { - ...projectContextOptions, - semanticMemoryEnabled: getSemanticMemoryEnabled(), - }, + // Preferences supply the defaults; an explicit prop still wins so + // callers (and tests) can override per session. + {...getProjectContextPreferences(), ...projectContextOptions}, ); systemPrompt = projectContext.systemPrompt; setLastBuiltPrompt(systemPrompt); diff --git a/source/memory/project-context.spec.ts b/source/memory/project-context.spec.ts index f62d2ab92..d7d2f7b3d 100644 --- a/source/memory/project-context.spec.ts +++ b/source/memory/project-context.spec.ts @@ -143,3 +143,24 @@ test('appendRelevantProjectContext returns original prompt when lookup fails', a t.is(prompt, 'base prompt'); }); + +test('formatProjectContext widens the fence so memory content cannot escape it', t => { + const output = formatProjectContext([ + memory('Use ``` fenced blocks ``` carefully.'), + ]); + + t.is( + output, + '## Project Context\n\n````\n- Use ``` fenced blocks ``` carefully.\n````', + ); + // The payload stays strictly inside the fence. + const [, body] = output.split('````'); + t.true(body?.includes('fenced blocks') ?? false); +}); + +test('formatProjectContext keeps the standard fence when content has no backticks', t => { + t.is( + formatProjectContext([memory('Auth uses Clerk.')]), + '## Project Context\n\n```\n- Auth uses Clerk.\n```', + ); +}); diff --git a/source/memory/project-context.ts b/source/memory/project-context.ts index 452051085..00cf3a9de 100644 --- a/source/memory/project-context.ts +++ b/source/memory/project-context.ts @@ -14,13 +14,32 @@ export interface ProjectContextResult { memoryCount: number; } -const DEFAULT_MEMORY_LIMIT = 8; -const DEFAULT_TOKEN_BUDGET = 240; +export const DEFAULT_MEMORY_LIMIT = 8; +export const DEFAULT_TOKEN_BUDGET = 240; + +/** Bounds for the user-configurable values, applied when preferences are read. */ +export const MIN_MEMORY_LIMIT = 1; +export const MAX_MEMORY_LIMIT = 50; +export const MIN_TOKEN_BUDGET = 40; +export const MAX_TOKEN_BUDGET = 4000; function estimateTokens(value: string): number { return Math.ceil(value.length / 4); } +/** + * Picks a fence longer than the longest backtick run in the body, the way + * Markdown itself does. Memory content is interpolated verbatim, so a fixed + * three-backtick fence could be escaped by a memory containing backticks. + */ +function fenceFor(body: string): string { + let longest = 0; + for (const match of body.matchAll(/`+/gu)) { + longest = Math.max(longest, match[0].length); + } + return '`'.repeat(Math.max(3, longest + 1)); +} + export function formatProjectContext( memories: SemanticMemory[], options: ProjectContextOptions = {}, @@ -50,8 +69,11 @@ function formatProjectContextWithCount( if (bullets.length === 0) return {content: '', memoryCount: 0}; + const body = bullets.join('\n'); + const fence = fenceFor(body); + return { - content: `## Project Context\n\n\`\`\`\n${bullets.join('\n')}\n\`\`\``, + content: `## Project Context\n\n${fence}\n${body}\n${fence}`, memoryCount: bullets.length, }; } diff --git a/source/memory/proposal-store.ts b/source/memory/proposal-store.ts new file mode 100644 index 000000000..819983045 --- /dev/null +++ b/source/memory/proposal-store.ts @@ -0,0 +1,58 @@ +import type {MemoryProposal} from './summarizer-service'; + +/** + * Holds the proposal list printed by the last `/memory propose`. + * + * The list is never mutated once printed. `/memory accept ` addresses it by + * the same 1-based index the user is reading off screen, so accepted entries are + * tracked in a separate set rather than removed - dropping an entry would shift + * every later number against the printout and silently save the wrong memory. + */ +export class ProposalStore { + private proposals: MemoryProposal[] = []; + private readonly accepted = new Set(); + + set(proposals: MemoryProposal[]): void { + this.proposals = proposals; + this.accepted.clear(); + } + + list(): readonly MemoryProposal[] { + return this.proposals; + } + + get size(): number { + return this.proposals.length; + } + + /** `index` is 1-based, matching the printed list. */ + at(index: number): MemoryProposal | undefined { + if ( + !Number.isInteger(index) || + index < 1 || + index > this.proposals.length + ) { + return undefined; + } + return this.proposals[index - 1]; + } + + isAccepted(index: number): boolean { + return this.accepted.has(index); + } + + markAccepted(index: number): void { + this.accepted.add(index); + } + + clear(): void { + this.proposals = []; + this.accepted.clear(); + } +} + +/** + * Shared store backing the lazily-loaded `/memory` command. Cleared by `/clear` + * so a proposal derived from a discarded conversation can't still be accepted. + */ +export const sharedProposalStore = new ProposalStore(); diff --git a/source/memory/summarizer-service.spec.ts b/source/memory/summarizer-service.spec.ts index 6c19e8dfd..f70ab479a 100644 --- a/source/memory/summarizer-service.spec.ts +++ b/source/memory/summarizer-service.spec.ts @@ -2,10 +2,14 @@ import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; import test from 'ava'; +import type {Message} from '@/types/core'; import {SemanticMemoryManager} from './semantic-memory-manager.js'; import { inferMemoryCategory, + MAX_PROPOSALS, + MAX_SCANNED_MESSAGES, type MemoryProposal, + REVERSAL_WARNING, SummarizerService, toCamelCaseCategory, } from './summarizer-service.js'; @@ -386,3 +390,153 @@ test('SummarizerService acceptProposal saves a proposal without re-deriving its t.is(memory.sourceSessionId, 'session-1'); t.deepEqual(await manager.listMemories(), [memory]); }); + +// --- Reversal detector: the four variants the round-3 review found defeated, +// plus the contradiction case the original report actually asked for. --- + +const CONCESSION = + "You're right. The provider config should load lazily, not eagerly."; +const USER_PREFERENCE_ONLY = + 'Honestly, lazy provider config just feels cleaner to me.'; + +function reversalWarnings(messages: Message[], content: string): string[] { + const proposal = new SummarizerService() + .proposeMemoriesFromMessages(messages) + .find(p => p.content === content); + if (!proposal) throw new Error(`no proposal produced for: ${content}`); + return proposal.warnings; +} + +test('reversal is flagged on a clean two-turn concession', t => { + t.true( + reversalWarnings( + [ + {role: 'user', content: USER_PREFERENCE_ONLY}, + {role: 'assistant', content: CONCESSION}, + ], + CONCESSION, + ).includes(REVERSAL_WARNING), + ); +}); + +test('reversal is still flagged when the user message contains a bare slash', t => { + // "auth/login" is prose, not a file path; it must not count as evidence. + t.true( + reversalWarnings( + [ + { + role: 'user', + content: + 'Honestly, for auth/login lazy provider config just feels cleaner to me.', + }, + {role: 'assistant', content: CONCESSION}, + ], + CONCESSION, + ).includes(REVERSAL_WARNING), + ); +}); + +test('reversal is still flagged across intervening tool-call turns', t => { + t.true( + reversalWarnings( + [ + {role: 'user', content: USER_PREFERENCE_ONLY}, + { + role: 'assistant', + content: '', + tool_calls: [ + {id: 't1', function: {name: 'read_file', arguments: {}}}, + ], + }, + { + role: 'tool', + content: 'file contents', + tool_call_id: 't1', + name: 'read_file', + }, + {role: 'assistant', content: CONCESSION}, + ], + CONCESSION, + ).includes(REVERSAL_WARNING), + ); +}); + +test('reversal is flagged for agreement openers outside the original six phrases', t => { + const conceded = + 'Agreed on reflection. The provider config should load lazily, not eagerly.'; + t.true( + reversalWarnings( + [ + {role: 'user', content: USER_PREFERENCE_ONLY}, + {role: 'assistant', content: conceded}, + ], + conceded, + ).includes(REVERSAL_WARNING), + ); +}); + +test('reversal is flagged when a turn contradicts an earlier assistant turn without any opener', t => { + const reversed = + 'The provider config should load lazily, not eagerly, for this project.'; + t.true( + reversalWarnings( + [ + {role: 'user', content: 'How should provider config load?'}, + { + role: 'assistant', + content: 'The provider config should load eagerly, not lazily.', + }, + {role: 'user', content: USER_PREFERENCE_ONLY}, + {role: 'assistant', content: reversed}, + ], + reversed, + ).includes(REVERSAL_WARNING), + ); +}); + +test('reversal is not flagged when a substantive assistant reply sits in between', t => { + const conceded = "You're right. Use eager provider config."; + t.false( + reversalWarnings( + [ + {role: 'user', content: 'Honestly lazy just feels cleaner.'}, + {role: 'assistant', content: 'Here is a summary of the current setup.'}, + {role: 'assistant', content: conceded}, + ], + conceded, + ).includes(REVERSAL_WARNING), + ); +}); + +test('reversal is not flagged for a routine factual assistant turn', t => { + const fact = 'The storage schema keeps one provider row per workspace.'; + t.false( + reversalWarnings( + [ + {role: 'user', content: 'How is the storage laid out here?'}, + {role: 'assistant', content: fact}, + ], + fact, + ).includes(REVERSAL_WARNING), + ); +}); + +test('SummarizerService bounds the scan window and total proposal count', t => { + const service = new SummarizerService(); + const messages: Message[] = []; + // Well past both limits, and old enough that the earliest fall outside the window. + const total = MAX_SCANNED_MESSAGES + 20; + for (let i = 0; i < total; i++) { + messages.push({ + role: 'user', + content: `Fix the provider retry storage schema bug number ${i}.`, + }); + } + + const proposals = service.proposeMemoriesFromMessages(messages); + + t.is(proposals.length, MAX_PROPOSALS); + // The window keeps the newest turns, so the oldest message is not proposed. + t.false(proposals.some(p => p.content.endsWith('number 0.'))); + t.true(proposals.some(p => p.content.endsWith(`number ${total - 1}.`))); +}); diff --git a/source/memory/summarizer-service.ts b/source/memory/summarizer-service.ts index 255ec7b77..4db8ce716 100644 --- a/source/memory/summarizer-service.ts +++ b/source/memory/summarizer-service.ts @@ -27,6 +27,18 @@ export interface MemoryProposal { const MAX_CANDIDATES_PER_MESSAGE = 3; const MAX_EVIDENCE_LENGTH = 160; +/** + * How far back `/memory propose` scans. Proposals are reviewed by eye against a + * printed, numbered list, so an unbounded scan over a long session produces a + * list nobody reads. Both limits keep the newest turns. + */ +export const MAX_SCANNED_MESSAGES = 40; +export const MAX_PROPOSALS = 20; + +export const REVERSAL_WARNING = 'Possible assistant position reversal.'; +const INFERRED_WARNING = + 'Inferred from conversation, no explicit user statement.'; + function truncateEvidence(content: string): string { const collapsed = content.replaceAll(/\s+/gu, ' ').trim(); if (collapsed.length <= MAX_EVIDENCE_LENGTH) return collapsed; @@ -58,6 +70,173 @@ const CATEGORY_RULES: Array<{category: string; pattern: RegExp}> = [ }, ]; +/** + * Openers a model reaches for when conceding. Deliberately broader than a + * handful of stock phrases: a concession phrased "Agreed on reflection" is the + * same event as one phrased "You're right", and only one of them was previously + * detectable. + */ +const AGREEMENT_OPENER_PATTERN = + /^\s*[^a-z0-9]*(you(?:'|’)?re\s+(?:right|correct)|you\s+are\s+(?:right|correct)|good\s+point|fair\s+(?:enough|point)|that\s+makes\s+sense|agreed|i\s+agree|on\s+reflection|point\s+taken|my\s+mistake|i\s+was\s+wrong|apologies|sorry,\s+you)/i; + +/** + * Signals that a user turn carried real evidence rather than bare preference. + * + * A path needs a genuine path shape - a leading `/`, `./` or `~/`, two or more + * segments, or a known file extension. A single bare slash does not count, so + * ordinary prose like "auth/login" no longer suppresses detection. + */ +const CODE_FENCE_PATTERN = /```|~~~/; +const INLINE_CODE_PATTERN = /`[^`]+`/; +const PATH_PATTERN = + /(?:^|[\s('"])(?:~\/|\.{1,2}\/|\/)[\w.-]+|[\w.-]+\/[\w.-]+\/[\w.-]+|\b[\w-]+\.(?:ts|tsx|js|jsx|mjs|cjs|json|ya?ml|md|py|rb|go|rs|java|html|css|scss|toml|sh|sql)\b/; +const ERROR_OUTPUT_PATTERN = + /\b(error[:\s]|exception|traceback|stack\s?trace|failed\s+with|exit\s+code|ENOENT|undefined is not|cannot read)\b/i; +const CODE_IDENTIFIER_PATTERN = + /\b[A-Za-z_$][\w$]*\([^)]*\)|\b[A-Za-z_$][\w$]*\.[A-Za-z_$][\w$]+\b|\b[a-z]+[A-Z][\w$]*\b/; + +function hasTechnicalEvidence(content: string): boolean { + return ( + CODE_FENCE_PATTERN.test(content) || + INLINE_CODE_PATTERN.test(content) || + PATH_PATTERN.test(content) || + ERROR_OUTPUT_PATTERN.test(content) || + CODE_IDENTIFIER_PATTERN.test(content) + ); +} + +/** An assistant turn that only carries tool calls is plumbing, not a reply. */ +function isToolCallTurn(message: Message): boolean { + return ( + (message.tool_calls?.length ?? 0) > 0 && message.content.trim().length === 0 + ); +} + +const NEGATION_PATTERN = + /\b(not|never|no|avoid|isn'?t|aren'?t|won'?t|shouldn'?t|doesn'?t|don'?t|instead\s+of|rather\s+than|no\s+longer)\b/i; + +/** + * Opposed term pairs used to spot a stance flip between two assistant turns. + * Each entry is one axis; which side a turn *asserts* is decided by whether the + * term is negated, so "lazily, not eagerly" asserts lazy rather than both. + */ +const OPPOSED_TERM_PAIRS: Array<[RegExp, RegExp]> = [ + [/\blazil?y?\b|\blazy\b/i, /\beager(?:ly)?\b/i], + [/\bsynchronous(?:ly)?\b|\bsync\b/i, /\basynchronous(?:ly)?\b|\basync\b/i], + [/\benabled?\b/i, /\bdisabled?\b/i], + [/\bincluded?\b/i, /\bexcluded?\b/i], + [/\badded?\b/i, /\bremoved?\b/i], + [/\bmutable\b/i, /\bimmutable\b/i], + [/\bexplicit(?:ly)?\b/i, /\bimplicit(?:ly)?\b/i], + [/\bstatic(?:ally)?\b/i, /\bdynamic(?:ally)?\b/i], + [/\bbefore\b/i, /\bafter\b/i], + [/\bsingle\b/i, /\bmultiple\b/i], + [/\bshould\b/i, /\bshould\s?n[o']?t\b/i], +]; + +const NEGATION_LOOKBEHIND = 24; + +/** True when `pattern` matches `text` at a position not preceded by a negation. */ +function assertsTerm(text: string, pattern: RegExp): boolean { + const global = new RegExp( + pattern.source, + `${pattern.flags.replace('g', '')}g`, + ); + for (const match of text.matchAll(global)) { + const start = match.index ?? 0; + const preceding = text.slice( + Math.max(0, start - NEGATION_LOOKBEHIND), + start, + ); + if (!NEGATION_PATTERN.test(preceding)) return true; + } + return false; +} + +function contentTerms(value: string): Set { + return new Set( + value + .toLowerCase() + .split(/[^a-z0-9]+/u) + .filter(part => part.length > 2 && !TOPIC_STOPWORDS.has(part)), + ); +} + +const TOPIC_STOPWORDS = new Set([ + 'the', + 'and', + 'but', + 'for', + 'not', + 'you', + 'are', + 'was', + 'were', + 'this', + 'that', + 'with', + 'have', + 'has', + 'had', + 'will', + 'would', + 'should', + 'could', + 'can', + 'its', + 'your', + 'our', + 'their', + 'them', + 'they', + 'from', + 'into', + 'been', + 'right', + 'correct', + 'agreed', + 'agree', + 'point', + 'sense', + 'makes', + 'reflection', +]); + +const MIN_SHARED_TOPIC_TERMS = 2; + +/** + * True when `later` reverses a stance `earlier` took on the same subject. + * + * Requires topical overlap first, then either a flip along one of the opposed + * term axes or a negation asymmetry on a shared term. This is a heuristic + * feeding a *warning* on a proposal the user is already reviewing by hand, so it + * is tuned to tolerate false positives rather than miss real concessions. + */ +function isContradiction(earlier: string, later: string): boolean { + const earlierTerms = contentTerms(earlier); + const laterTerms = contentTerms(later); + const shared = [...laterTerms].filter(term => earlierTerms.has(term)); + if (shared.length < MIN_SHARED_TOPIC_TERMS) return false; + + for (const [sideA, sideB] of OPPOSED_TERM_PAIRS) { + const earlierA = assertsTerm(earlier, sideA); + const earlierB = assertsTerm(earlier, sideB); + const laterA = assertsTerm(later, sideA); + const laterB = assertsTerm(later, sideB); + if ( + (earlierA && !earlierB && laterB && !laterA) || + (earlierB && !earlierA && laterA && !laterB) + ) { + return true; + } + } + + return shared.some(term => { + const pattern = new RegExp(`\\b${term}\\b`, 'i'); + return assertsTerm(earlier, pattern) !== assertsTerm(later, pattern); + }); +} + export class SummarizerService { constructor( private readonly memoryManager = new SemanticMemoryManager(), @@ -115,7 +294,11 @@ export class SummarizerService { } >(); - for (let i = 0; i < messages.length; i++) { + // Only the most recent turns are scanned; older ones would swell the + // printed list past what anyone reviews by eye. + const firstScanned = Math.max(0, messages.length - MAX_SCANNED_MESSAGES); + + for (let i = firstScanned; i < messages.length; i++) { const message = messages[i]; if (!message || (message.role !== 'user' && message.role !== 'assistant')) continue; @@ -130,56 +313,51 @@ export class SummarizerService { if (category === 'project' && message.role === 'assistant') continue; const key = candidate.toLowerCase(); - if (!proposals.has(key)) { - proposals.set(key, { + let entry = proposals.get(key); + if (!entry) { + entry = { content: candidate, category, sourceRole: message.role, userTurns: [], assistantTurns: [], warnings: [], - }); + }; + proposals.set(key, entry); } - const entry = proposals.get(key)!; const snippet = truncateEvidence(message.content); if (message.role === 'user') { entry.userTurns.push(snippet); entry.sourceRole = 'user'; entry.warnings = entry.warnings.filter( - warning => warning !== 'Possible assistant position reversal.', + warning => warning !== REVERSAL_WARNING, ); } else { entry.assistantTurns.push(snippet); } - if (message.role === 'assistant' && entry.sourceRole !== 'user') { - if (this.isAssistantReversal(messages, i)) { - if ( - !entry.warnings.includes('Possible assistant position reversal.') - ) { - entry.warnings.push('Possible assistant position reversal.'); - } - } + if ( + message.role === 'assistant' && + entry.sourceRole !== 'user' && + !entry.warnings.includes(REVERSAL_WARNING) && + this.isAssistantReversal(messages, i) + ) { + entry.warnings.push(REVERSAL_WARNING); } } } - return [...proposals.values()].map(entry => { + return [...proposals.values()].slice(-MAX_PROPOSALS).map(entry => { const sourceType: MemorySourceType = entry.sourceRole === 'user' ? 'explicit-user' : 'conversation-inferred'; const warnings = [...entry.warnings]; - if (sourceType === 'conversation-inferred') { - if ( - !warnings.includes( - 'Inferred from conversation, no explicit user statement.', - ) - ) { - warnings.push( - 'Inferred from conversation, no explicit user statement.', - ); - } + if ( + sourceType === 'conversation-inferred' && + !warnings.includes(INFERRED_WARNING) + ) { + warnings.push(INFERRED_WARNING); } return { @@ -195,6 +373,20 @@ export class SummarizerService { }); } + /** + * Flags an assistant turn that reads as a concession to social pressure + * rather than to evidence. + * + * Shape, following the original report: + * 1. the turn is preceded by a user turn carrying no code, path or error + * output - i.e. pushback with no new information, and + * 2. the turn either contradicts an earlier assistant turn on the same + * subject, or opens with an agreement phrase. + * + * Tool-call turns and tool results are stepped over in (1); in a real + * agentic session the assistant reads files between almost every pair of + * user turns, and bailing on those made the check near-unreachable. + */ private isAssistantReversal( messages: Message[], assistantIndex: number, @@ -202,37 +394,51 @@ export class SummarizerService { const assistantMsg = messages[assistantIndex]; if (!assistantMsg) return false; - const agreementOpenerPattern = - /^\s*[^a-z0-9]*(you're right|good point|fair enough|that makes sense|you're correct|fair point)/i; - if (!agreementOpenerPattern.test(assistantMsg.content)) { - return false; - } - - let userMsg: Message | undefined; + let userIndex = -1; for (let i = assistantIndex - 1; i >= 0; i--) { const m = messages[i]; if (!m) continue; - if (m.role === 'user') { - userMsg = m; - break; - } + if (m.role === 'tool') continue; if (m.role === 'assistant') { + if (isToolCallTurn(m)) continue; return false; } + if (m.role === 'user') { + userIndex = i; + break; + } } + const userMsg = userIndex >= 0 ? messages[userIndex] : undefined; if (!userMsg) return false; + if (hasTechnicalEvidence(userMsg.content)) return false; - const userContent = userMsg.content; - const hasCodeBlock = /```/.test(userContent); - const hasFilePath = /[/]|\.(ts|js|jsx|tsx|py|json|md|html|css)\b/i.test( - userContent, - ); - const hasErrorOutput = /\b(error:|exception|traceback|stack trace)\b/i.test( - userContent, + if (AGREEMENT_OPENER_PATTERN.test(assistantMsg.content)) return true; + + return this.contradictsEarlierAssistantTurn( + messages, + assistantIndex, + userIndex, ); + } + + /** True when any assistant turn before `userIndex` took the opposite stance. */ + private contradictsEarlierAssistantTurn( + messages: Message[], + assistantIndex: number, + userIndex: number, + ): boolean { + const later = messages[assistantIndex]; + if (!later) return false; + + for (let i = userIndex - 1; i >= 0; i--) { + const earlier = messages[i]; + if (!earlier || earlier.role !== 'assistant') continue; + if (isToolCallTurn(earlier)) continue; + if (isContradiction(earlier.content, later.content)) return true; + } - return !(hasCodeBlock || hasFilePath || hasErrorOutput); + return false; } } diff --git a/source/plain/shell.ts b/source/plain/shell.ts index 5dbee6617..d28255300 100644 --- a/source/plain/shell.ts +++ b/source/plain/shell.ts @@ -1,7 +1,11 @@ import path from 'node:path'; import {appendToolDefinitionsToPrompt} from '@/ai-sdk-client/tools/system-prompt-assembler'; import {getAppConfig} from '@/config/index'; -import {loadPreferences, savePreferences} from '@/config/preferences'; +import { + loadPreferences, + resolveProjectContextPreferences, + savePreferences, +} from '@/config/preferences'; import {resolveTune} from '@/config/tune'; import {appendRelevantProjectContextWithCount} from '@/memory/project-context'; import {SemanticMemoryManager} from '@/memory/semantic-memory-manager'; @@ -167,10 +171,7 @@ export async function runPlainShell( toolPrompt, prompt, new SemanticMemoryManager(), - { - semanticMemoryEnabled: - deps.loadPreferences().semanticMemoryEnabled ?? true, - }, + resolveProjectContextPreferences(deps.loadPreferences()), ); const systemContent = projectContext.systemPrompt; if (projectContext.memoryCount > 0) { diff --git a/source/types/config.ts b/source/types/config.ts index 393649b33..2ccea6455 100644 --- a/source/types/config.ts +++ b/source/types/config.ts @@ -409,4 +409,8 @@ export interface UserPreferences { enablePromptScrubbing?: boolean; /** Whether semantic memory is active. Default true to preserve existing behavior. */ semanticMemoryEnabled?: boolean; + /** Max memories recalled into one prompt. Defaults and bounds live in project-context.ts. */ + semanticMemoryLimit?: number; + /** Approximate token ceiling for the injected Project Context block. */ + semanticMemoryTokenBudget?: number; }