diff --git a/CHANGELOG.md b/CHANGELOG.md index 28c02b56..f1dc0115 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## Unreleased + +### Fix: cumulative turn counting for auto-capture smart extraction (#417, PR #518) + +**Bug**: With `extractMinMessages: 2` + `smartExtraction: true`, single-turn DM conversations always fell through to regex fallback, writing dirty data (`l0_abstract == text`, no LLM distillation). + +**Root causes**: +- `autoCaptureSeenTextCount` was overwritten per-event (always 1 for DM), never accumulating +- `buildAutoCaptureConversationKeyFromIngress` returned `null` for DM (no `conversationId`), so `pendingIngressTexts` was never written + +**Changes**: +- **Cumulative counting**: `autoCaptureSeenTextCount` now accumulates across events instead of overwriting per-event +- **DM key fallback**: `buildAutoCaptureConversationKeyFromIngress` falls back to `channelId` when `conversationId` is falsy, so DM sessions now correctly write to `pendingIngressTexts` and match the key extracted by `buildAutoCaptureConversationKeyFromSessionKey` +- **Smart extraction threshold**: now uses cumulative turn count (`currentCumulativeCount`) instead of per-event message count +- **`extractMinMessages` cap**: `Math.min(config.extractMinMessages ?? 4, 100)` prevents misconfiguration (e.g., setting 999999 would permanently disable smart extraction) +- **MAX_MESSAGE_LENGTH guard**: 5000 char limit per message in `pendingIngressTexts` rolling window prevents OOM from malformed input +- **Test**: added `runCumulativeTurnCountingScenario` in `test/smart-extractor-branches.mjs` verifying turn-1 skip and turn-2 trigger with `extractMinMessages=2` + +**⚠️ Breaking change**: `extractMinMessages` semantics changed from "per-event message count" to "cumulative conversation turns". Before: each `agent_end` needed ≥N messages. After: smart extraction triggers at conversation turn N. This is a bug fix since the old semantics were structurally broken for DM; users relying on the old behavior may need to adjust their `extractMinMessages` values. + +--- + ## 1.1.0-beta.2 (Smart Memory Beta + Access Reinforcement) This is a **beta** release published under the npm dist-tag **`beta`** (it does not affect the stable `latest` channel). diff --git a/index.ts b/index.ts index 6cb1e2d5..65b949b3 100644 --- a/index.ts +++ b/index.ts @@ -774,8 +774,10 @@ function buildAutoCaptureConversationKeyFromIngress( ): string | null { const channel = typeof channelId === "string" ? channelId.trim() : ""; const conversation = typeof conversationId === "string" ? conversationId.trim() : ""; - if (!channel || !conversation) return null; - return `${channel}:${conversation}`; + if (!channel) return null; + // DM: conversationId=undefined -> fallback to channelId (matches regex extract from sessionKey) + // Group: conversationId=exists -> returns channelId:conversationId (matches regex extract) + return conversation ? `${channel}:${conversation}` : channel; } /** @@ -2059,8 +2061,9 @@ const memoryLanceDBProPlugin = { ); const normalized = normalizeAutoCaptureText("user", event.content, shouldSkipReflectionMessage); if (conversationKey && normalized) { + const MAX_MESSAGE_LENGTH = 5000; const queue = autoCapturePendingIngressTexts.get(conversationKey) || []; - queue.push(normalized); + queue.push(normalized.slice(0, MAX_MESSAGE_LENGTH)); autoCapturePendingIngressTexts.set(conversationKey, queue.slice(-6)); pruneMapIfOver(autoCapturePendingIngressTexts, AUTO_CAPTURE_MAP_MAX_ENTRIES); } @@ -2724,36 +2727,37 @@ const memoryLanceDBProPlugin = { const pendingIngressTexts = conversationKey ? [...(autoCapturePendingIngressTexts.get(conversationKey) || [])] : []; - if (conversationKey) { - autoCapturePendingIngressTexts.delete(conversationKey); - } - const previousSeenCount = autoCaptureSeenTextCount.get(sessionKey) ?? 0; + // [Fix #2] Cumulative counting: accumulate across events, not per-event overwrite + const currentCumulativeCount = previousSeenCount + eligibleTexts.length; let newTexts = eligibleTexts; if (pendingIngressTexts.length > 0) { + // [Fix #3] Use pendingIngressTexts as-is (REPLACE, not APPEND). + // REPLACE is correct because: (1) Fix #2 cumulative count ensures enough turns + // accumulate; (2) Fix #4 (delete) restores original behavior where pending is + // event-scoped; (3) APPEND causes deduplication issues when the same text + // appears in both pendingIngressTexts and eligibleTexts (after prefix stripping). newTexts = pendingIngressTexts; } else if (previousSeenCount > 0 && eligibleTexts.length > previousSeenCount) { newTexts = eligibleTexts.slice(previousSeenCount); } - autoCaptureSeenTextCount.set(sessionKey, eligibleTexts.length); + autoCaptureSeenTextCount.set(sessionKey, currentCumulativeCount); pruneMapIfOver(autoCaptureSeenTextCount, AUTO_CAPTURE_MAP_MAX_ENTRIES); const priorRecentTexts = autoCaptureRecentTexts.get(sessionKey) || []; let texts = newTexts; - if ( - texts.length === 1 && - isExplicitRememberCommand(texts[0]) && - priorRecentTexts.length > 0 - ) { - texts = [...priorRecentTexts.slice(-1), ...texts]; - } + // [Fix #5 REMOVED] isExplicitRememberCommand guard: unreachable under REPLACE strategy. + // With REPLACE, texts = pendingIngressTexts (length typically > 1 in multi-turn), + // so texts.length === 1 guard can never trigger. + // This guard was designed for the old APPEND strategy and is obsolete. if (newTexts.length > 0) { const nextRecentTexts = [...priorRecentTexts, ...newTexts].slice(-6); autoCaptureRecentTexts.set(sessionKey, nextRecentTexts); pruneMapIfOver(autoCaptureRecentTexts, AUTO_CAPTURE_MAP_MAX_ENTRIES); } - const minMessages = config.extractMinMessages ?? 4; + // [Fix #6] Cap extractMinMessages to prevent misconfiguration + const minMessages = Math.min(config.extractMinMessages ?? 4, 100); if (skippedAutoCaptureTexts > 0) { api.logger.debug( `memory-lancedb-pro: auto-capture skipped ${skippedAutoCaptureTexts} injected/system text block(s) for agent ${agentId}`, @@ -2827,9 +2831,10 @@ const memoryLanceDBProPlugin = { ); return; } - if (cleanTexts.length >= minMessages) { + // [Fix #3 updated] Use cumulative count (turn count) for smart extraction threshold + if (currentCumulativeCount >= minMessages) { api.logger.debug( - `memory-lancedb-pro: auto-capture running smart extraction for agent ${agentId} (${cleanTexts.length} clean texts >= ${minMessages})`, + `memory-lancedb-pro: auto-capture running smart extraction for agent ${agentId} (cumulative=${currentCumulativeCount} >= minMessages=${minMessages})`, ); const conversationText = cleanTexts.join("\n"); const stats = await smartExtractor.extractAndPersist( @@ -2856,7 +2861,7 @@ const memoryLanceDBProPlugin = { ); } else { api.logger.debug( - `memory-lancedb-pro: auto-capture skipped smart extraction for agent ${agentId} (${cleanTexts.length} < ${minMessages})`, + `memory-lancedb-pro: auto-capture skipped smart extraction for agent ${agentId} (cumulative=${currentCumulativeCount} < minMessages=${minMessages})`, ); } } diff --git a/test/smart-extractor-branches.mjs b/test/smart-extractor-branches.mjs index 67830f68..62541fc5 100644 --- a/test/smart-extractor-branches.mjs +++ b/test/smart-extractor-branches.mjs @@ -64,7 +64,7 @@ function createEmbeddingServer() { }); } -function createMockApi(dbPath, embeddingBaseURL, llmBaseURL, logs) { +function createMockApi(dbPath, embeddingBaseURL, llmBaseURL, logs, pluginConfigOverrides = {}) { return { pluginConfig: { dbPath, @@ -72,6 +72,7 @@ function createMockApi(dbPath, embeddingBaseURL, llmBaseURL, logs) { autoRecall: false, smartExtraction: true, extractMinMessages: 2, + ...pluginConfigOverrides, embedding: { apiKey: "dummy", model: "qwen3-embedding-4b", @@ -1315,4 +1316,90 @@ assert.ok( ), ); +// ============================================================ +// Test: cumulative turn counting with extractMinMessages=2 +// Verifies issue #417 fix: 2 sequential agent_end events should +// trigger smart extraction on turn 2 (cumulative count >= 2). +// ============================================================ + +async function runCumulativeTurnCountingScenario() { + const workDir = mkdtempSync(path.join(tmpdir(), "memory-cumulative-turn-")); + const dbPath = path.join(workDir, "db"); + const logs = []; + const embeddingServer = createEmbeddingServer(); + + await new Promise((resolve) => embeddingServer.listen(0, "127.0.0.1", resolve)); + const embeddingPort = embeddingServer.address().port; + process.env.TEST_EMBEDDING_BASE_URL = `http://127.0.0.1:${embeddingPort}/v1`; + + try { + const api = createMockApi( + dbPath, + `http://127.0.0.1:${embeddingPort}/v1`, + "http://127.0.0.1:9", + logs, + // extractMinMessages=2 (the key setting for this test) + { extractMinMessages: 2, smartExtraction: true, captureAssistant: false }, + ); + plugin.register(api); + + const sessionKey = "agent:main:discord:dm:user123"; + const channelId = "discord"; + const conversationId = "dm:user123"; + + // Turn 1: message_received -> agent_end + await api.hooks.message_received( + { from: "user:user123", content: "我的名字是小明" }, + { channelId, conversationId, accountId: "default" }, + ); + await runAgentEndHook( + api, + { + success: true, + messages: [{ role: "user", content: "我的名字是小明" }], + }, + { agentId: "main", sessionKey }, + ); + + // Turn 2: message_received -> agent_end (this should trigger smart extraction) + await api.hooks.message_received( + { from: "user:user123", content: "我喜歡游泳" }, + { channelId, conversationId, accountId: "default" }, + ); + await runAgentEndHook( + api, + { + success: true, + messages: [{ role: "user", content: "我喜歡游泳" }], + }, + { agentId: "main", sessionKey }, + ); + + const smartExtractionTriggered = logs.some((entry) => + entry[1].includes("running smart extraction") && + entry[1].includes("cumulative=") + ); + const smartExtractionSkipped = logs.some((entry) => + entry[1].includes("skipped smart extraction") && + entry[1].includes("cumulative=1") + ); + + return { logs, smartExtractionTriggered, smartExtractionSkipped }; + } finally { + delete process.env.TEST_EMBEDDING_BASE_URL; + await new Promise((resolve) => embeddingServer.close(resolve)); + rmSync(workDir, { recursive: true, force: true }); + } +} + +const cumulativeResult = await runCumulativeTurnCountingScenario(); +// Turn 2 must trigger smart extraction (cumulative >= 2) +assert.ok(cumulativeResult.smartExtractionTriggered, + "Smart extraction should trigger on turn 2 with cumulative count >= 2. Logs: " + + cumulativeResult.logs.map((e) => e[1]).join(" | ")); +// Turn 1 must have been skipped (cumulative=1 < 2) +assert.ok(cumulativeResult.smartExtractionSkipped, + "Turn 1 should skip smart extraction (cumulative=1 < 2). Logs: " + + cumulativeResult.logs.map((e) => e[1]).join(" | ")); + console.log("OK: smart extractor branch regression test passed");