From 7d89c189f24262bb35b7b483546579cd914ddd5b Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 18 Jun 2026 21:25:01 -0300 Subject: [PATCH] test(chatcore): dedicated unit tests for 6 leaves + wire into stryker mutate (QG v2 Fase 9 T5 Fase 3) (#4218) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrated into release/v3.8.29 (QG v2 Fase 9 T5 Fase 3 — dedicated leaf unit tests + stryker batch h). --- .github/workflows/nightly-mutation.yml | 10 +- stryker.conf.json | 33 +++- tests/unit/chatcore-executor-helpers.test.ts | 171 ++++++++++++++++++ tests/unit/chatcore-headers.test.ts | 34 +++- tests/unit/chatcore-log-truncation.test.ts | 143 +++++++++++++-- tests/unit/chatcore-memory-extraction.test.ts | 112 +++++++++++- 6 files changed, 459 insertions(+), 44 deletions(-) create mode 100644 tests/unit/chatcore-executor-helpers.test.ts diff --git a/.github/workflows/nightly-mutation.yml b/.github/workflows/nightly-mutation.yml index ec78b64ce8..cb7f520f43 100644 --- a/.github/workflows/nightly-mutation.yml +++ b/.github/workflows/nightly-mutation.yml @@ -23,9 +23,11 @@ jobs: # combo/* leaves are back in `mutate` (covered by the 24 combo-*.test.ts), grouped into # 2 batches (d=heavy, e=light). After #4204 (D7b) merged, the reset-aware quota pair # quotaScoring/quotaStrategies was added as batch f. A covering-test audit then added the - # 6 chatCore/* leaves with direct unit coverage as batch g; the other 9 chatCore leaves - # remain a follow-up — see _mutate_godfiles_excluded_comment in stryker.conf.json. - # 7 PARALLEL batches, each overriding the mutate set via `--mutate` (Stryker 9 CLI: + # 6 chatCore/* leaves with direct unit coverage as batch g. A follow-up then wrote dedicated + # unit tests for 6 more leaves and added them as batch h; the last 3 chatCore leaves + # (memorySkillsInjection/semanticCache/telemetryHelpers) remain a follow-up — see + # _mutate_godfiles_excluded_comment in stryker.conf.json. + # 8 PARALLEL batches, each overriding the mutate set via `--mutate` (Stryker 9 CLI: # `-m, --mutate `; the conf's `mutate[]` remains the local-run default/union). # Each batch targets <180min; full coverage every night in parallel (~180min wall-clock). # The `incremental` cache (per-batch key) makes runs after the first cold one much cheaper, @@ -50,6 +52,8 @@ jobs: mutate: "open-sse/services/combo/quotaScoring.ts,open-sse/services/combo/quotaStrategies.ts" - name: g mutate: "open-sse/handlers/chatCore/comboContextCache.ts,open-sse/handlers/chatCore/idempotency.ts,open-sse/handlers/chatCore/passthroughHelpers.ts,open-sse/handlers/chatCore/responseHeaders.ts,open-sse/handlers/chatCore/sanitization.ts,open-sse/handlers/chatCore/upstreamTimeouts.ts" + - name: h + mutate: "open-sse/handlers/chatCore/headers.ts,open-sse/handlers/chatCore/logTruncation.ts,open-sse/handlers/chatCore/memoryExtraction.ts,open-sse/handlers/chatCore/nonStreamingSse.ts,open-sse/handlers/chatCore/passthroughToolNames.ts,open-sse/handlers/chatCore/executorHelpers.ts" timeout-minutes: 180 steps: - uses: actions/checkout@v6 diff --git a/stryker.conf.json b/stryker.conf.json index 684330e554..2faf253376 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -67,8 +67,14 @@ "tests/unit/chat-route-coverage.test.ts", "tests/unit/chat-route-edge-cases.test.ts", "tests/unit/chatcore-compression-integration.test.ts", + "tests/unit/chatcore-executor-helpers.test.ts", "tests/unit/chatcore-extracted-modules-3821.test.ts", + "tests/unit/chatcore-headers.test.ts", "tests/unit/chatcore-imports-cleanly.test.ts", + "tests/unit/chatcore-log-truncation.test.ts", + "tests/unit/chatcore-memory-extraction.test.ts", + "tests/unit/chatcore-non-streaming-sse.test.ts", + "tests/unit/chatcore-passthrough-tool-names.test.ts", "tests/unit/chatcore-sanitization.test.ts", "tests/unit/chatcore-strip-stale-headers.test.ts", "tests/unit/chatcore-translation-paths.test.ts", @@ -202,20 +208,21 @@ "the 24 combo-*.test.ts files already in tap.testFiles (quota by combo-prescreen/combo-config/", "combo-strategy-fallbacks). types.ts is omitted (pure type declarations produce 0 mutants).", "", - "chatCore/* leaves: a covering-test audit (do any tap.testFiles reference each leaf's exported", - "function names?) found 6 with direct unit coverage — comboContextCache/idempotency/", - "passthroughHelpers/responseHeaders/sanitization/upstreamTimeouts — ADDED here (batch g).", + "chatCore/* leaves: a covering-test audit found 6 with direct unit coverage (batch g:", + "comboContextCache/idempotency/passthroughHelpers/responseHeaders/sanitization/upstreamTimeouts).", + "A follow-up then added DEDICATED unit tests for 6 more leaves (tests/unit/chatcore-headers,", + "-log-truncation, -memory-extraction, -non-streaming-sse, -passthrough-tool-names,", + "-executor-helpers — wired into tap.testFiles above) and added those leaves as batch h", + "(headers/logTruncation/memoryExtraction/nonStreamingSse/passthroughToolNames/executorHelpers).", "", "STILL EXCLUDED (follow-ups, NOT in `mutate` yet):", " - combo.ts + chatCore.ts barrels: their handleComboChat/handleChatCore CORES were not", " split (out of scope — Fase 3 ChatCoreContext refactor). The barrels are now thin-ish", " but still large; keep excluded until the cores are split.", - " - the other 9 chatCore/* leaves (executorHelpers/headers/logTruncation/memoryExtraction/", - " memorySkillsInjection/nonStreamingSse/passthroughToolNames/semanticCache/telemetryHelpers):", - " no tap.testFiles reference their exported symbols by name — they are only exercised", - " TRANSITIVELY via handleChatCore, which the tap unit tests may not fully drive. Adding them", - " blindly would produce 'survived (no coverage)' noise. Add each once it gains a dedicated", - " unit test (or after a Stryker --dryRunOnly confirms the perTest map covers it).", + " - the last 3 chatCore/* leaves (memorySkillsInjection/semanticCache/telemetryHelpers): no", + " dedicated unit test yet (memorySkillsInjection has 6 imports, semanticCache 7, telemetryHelpers", + " uses fetch) — they need mocked deps. Add each once it gains a dedicated unit test (or after", + " a Stryker --dryRunOnly confirms the perTest map covers it).", "See project memory: Quality Gate v2 / Fase 9 (project-combo-split)." ], "mutate": [ @@ -240,7 +247,13 @@ "open-sse/handlers/chatCore/passthroughHelpers.ts", "open-sse/handlers/chatCore/responseHeaders.ts", "open-sse/handlers/chatCore/sanitization.ts", - "open-sse/handlers/chatCore/upstreamTimeouts.ts" + "open-sse/handlers/chatCore/upstreamTimeouts.ts", + "open-sse/handlers/chatCore/headers.ts", + "open-sse/handlers/chatCore/logTruncation.ts", + "open-sse/handlers/chatCore/memoryExtraction.ts", + "open-sse/handlers/chatCore/nonStreamingSse.ts", + "open-sse/handlers/chatCore/passthroughToolNames.ts", + "open-sse/handlers/chatCore/executorHelpers.ts" ], "_ignorePatterns_comment": [ "ignorePatterns = files NOT copied into the Stryker sandbox. It does NOT scope", diff --git a/tests/unit/chatcore-executor-helpers.test.ts b/tests/unit/chatcore-executor-helpers.test.ts new file mode 100644 index 0000000000..d25fb09f53 --- /dev/null +++ b/tests/unit/chatcore-executor-helpers.test.ts @@ -0,0 +1,171 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + resolveAccountSemaphoreKey, + resolveAccountSemaphoreAccountKey, + resolveAccountSemaphoreMaxConcurrency, + buildClaudePromptCacheLogMeta, +} from "../../open-sse/handlers/chatCore/executorHelpers.ts"; +import { FORMATS } from "../../open-sse/translator/formats.ts"; + +test("resolveAccountSemaphoreAccountKey prefers an explicit non-blank connectionId", () => { + assert.equal(resolveAccountSemaphoreAccountKey("conn-1", { id: "ignored" }), "conn-1"); +}); + +test("resolveAccountSemaphoreAccountKey falls back through credential candidates in order", () => { + // blank connectionId -> first non-blank candidate (connectionId field) wins + assert.equal( + resolveAccountSemaphoreAccountKey(" ", { connectionId: "cred-conn", id: "id-x" }), + "cred-conn" + ); + // candidate order: connectionId, id, email, name, displayName + assert.equal(resolveAccountSemaphoreAccountKey(null, { id: " id-x " }), "id-x"); + assert.equal(resolveAccountSemaphoreAccountKey(null, { email: "a@b.co" }), "a@b.co"); + assert.equal(resolveAccountSemaphoreAccountKey(null, { name: "the-name" }), "the-name"); + assert.equal(resolveAccountSemaphoreAccountKey(null, { displayName: "Disp" }), "Disp"); +}); + +test("resolveAccountSemaphoreAccountKey returns null when nothing usable is present", () => { + assert.equal(resolveAccountSemaphoreAccountKey(null, null), null); + assert.equal(resolveAccountSemaphoreAccountKey(undefined, undefined), null); + assert.equal(resolveAccountSemaphoreAccountKey("", {}), null); + // non-string / blank candidates are all rejected + assert.equal(resolveAccountSemaphoreAccountKey("", { id: 123, email: " " } as unknown as Record), null); +}); + +test("resolveAccountSemaphoreMaxConcurrency parses finite numbers and numeric strings", () => { + // number passthrough + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: 4 }), 4); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: 0 }), 0); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: -2 }), -2); + // numeric string is coerced + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: "8" }), 8); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: " 3.5 " }), 3.5); +}); + +test("resolveAccountSemaphoreMaxConcurrency rejects non-finite / non-numeric / missing values", () => { + // exercises the private toFiniteNumberOrNull null branches indirectly + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: Infinity }), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: NaN }), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: "abc" }), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: "" }), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: " " }), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency({ maxConcurrent: true } as unknown as Record), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency({}), null); + assert.equal(resolveAccountSemaphoreMaxConcurrency(null), null); +}); + +test("resolveAccountSemaphoreKey builds provider:accountKey when both resolve", () => { + assert.equal( + resolveAccountSemaphoreKey({ + provider: "openai", + model: "gpt-4o", + connectionId: "conn-9", + credentials: null, + }), + "openai:conn-9" + ); + // accountKey can come from credentials when connectionId is blank + assert.equal( + resolveAccountSemaphoreKey({ + provider: "anthropic", + model: "claude", + connectionId: null, + credentials: { email: "user@x.io" }, + }), + "anthropic:user@x.io" + ); +}); + +test("resolveAccountSemaphoreKey returns null without a provider or account key", () => { + // no account key resolvable + assert.equal( + resolveAccountSemaphoreKey({ provider: "openai", model: "m", connectionId: null, credentials: null }), + null + ); + // account key resolves but provider missing + assert.equal( + resolveAccountSemaphoreKey({ provider: null, model: "m", connectionId: "conn", credentials: null }), + null + ); + assert.equal( + resolveAccountSemaphoreKey({ provider: "", model: "m", connectionId: "conn", credentials: null }), + null + ); +}); + +test("buildClaudePromptCacheLogMeta returns null for non-Claude format or non-object body", () => { + assert.equal(buildClaudePromptCacheLogMeta(FORMATS.OPENAI, { system: [] }, null), null); + assert.equal(buildClaudePromptCacheLogMeta(FORMATS.CLAUDE, null, null), null); + assert.equal( + buildClaudePromptCacheLogMeta(FORMATS.CLAUDE, "x" as unknown as Record, null), + null + ); +}); + +test("buildClaudePromptCacheLogMeta returns null when there are no breakpoints and no beta header", () => { + const body = { + system: [{ text: "plain system, no cache_control" }], + tools: [{ name: "t1" }], + messages: [{ role: "user", content: [{ type: "text", text: "hi" }] }], + }; + assert.equal(buildClaudePromptCacheLogMeta(FORMATS.CLAUDE, body, null), null); +}); + +test("buildClaudePromptCacheLogMeta counts cache_control breakpoints across system/tools/messages", () => { + const body = { + system: [ + { text: "billing", cache_control: { type: "ephemeral", ttl: "5m" } }, + // billing header lines are skipped entirely + { text: "x-anthropic-billing-header: abc", cache_control: { type: "ephemeral" } }, + ], + tools: [{ name: "lookup", cache_control: { type: "ephemeral" } }], + messages: [ + { + role: "user", + content: [ + { type: "text", text: "q", cache_control: { type: "ephemeral" } }, + { type: "text", text: "no cache here" }, + ], + }, + ], + }; + const meta = buildClaudePromptCacheLogMeta(FORMATS.CLAUDE, body, null); + assert.ok(meta); + // 1 system (the billing-header one is skipped) + 1 tool + 1 message = 3 + assert.equal(meta.totalBreakpoints, 3); + assert.equal(meta.applied, true); + assert.equal(meta.systemBreakpoints.length, 1); + assert.equal(meta.systemBreakpoints[0].ttl, "5m"); + assert.equal(meta.systemBreakpoints[0].index, 0); + assert.equal(meta.toolBreakpoints.length, 1); + assert.equal(meta.toolBreakpoints[0].name, "lookup"); + assert.equal(meta.messageBreakpoints.length, 1); + assert.equal(meta.messageBreakpoints[0].role, "user"); + assert.equal(meta.messageBreakpoints[0].blockType, "text"); + // default type when cache_control.type missing/blank is "ephemeral" + assert.equal(meta.messageBreakpoints[0].type, "ephemeral"); +}); + +test("buildClaudePromptCacheLogMeta surfaces the Anthropic-Beta header even with zero breakpoints", () => { + const body = { system: [{ text: "plain" }] }; + // provider header present -> meta returned, applied=false (no breakpoints) + const fromProvider = buildClaudePromptCacheLogMeta(FORMATS.CLAUDE, body, { + "anthropic-beta": "prompt-caching-2024-07-31", + }); + assert.ok(fromProvider); + assert.equal(fromProvider.applied, false); + assert.equal(fromProvider.totalBreakpoints, 0); + assert.equal(fromProvider.anthropicBeta, "prompt-caching-2024-07-31"); + + // falls back to client headers when provider header absent + const fromClient = buildClaudePromptCacheLogMeta( + FORMATS.CLAUDE, + body, + null, + new Headers({ "Anthropic-Beta": "client-beta" }) + ); + assert.ok(fromClient); + assert.equal(fromClient.anthropicBeta, "client-beta"); +}); diff --git a/tests/unit/chatcore-headers.test.ts b/tests/unit/chatcore-headers.test.ts index 9858d749e3..290336703e 100644 --- a/tests/unit/chatcore-headers.test.ts +++ b/tests/unit/chatcore-headers.test.ts @@ -3,18 +3,46 @@ import assert from "node:assert/strict"; import { getHeaderValueCaseInsensitive } from "../../open-sse/handlers/chatCore/headers.ts"; -test("getHeaderValueCaseInsensitive reads Headers and plain objects, case-insensitively", () => { +test("getHeaderValueCaseInsensitive reads a Headers instance via .get()", () => { const h = new Headers({ "Content-Type": "text/event-stream" }); + // Headers.get is itself case-insensitive, so a lowercase lookup hits the value. assert.equal(getHeaderValueCaseInsensitive(h, "content-type"), "text/event-stream"); + assert.equal(getHeaderValueCaseInsensitive(h, "Content-Type"), "text/event-stream"); + // Missing header on a Headers instance returns null (Headers.get contract). + assert.equal(getHeaderValueCaseInsensitive(h, "x-missing"), null); +}); +test("getHeaderValueCaseInsensitive matches plain-object keys case-insensitively", () => { const obj = { Accept: "text/event-stream", "X-Foo": "bar" }; assert.equal(getHeaderValueCaseInsensitive(obj, "accept"), "text/event-stream"); + assert.equal(getHeaderValueCaseInsensitive(obj, "ACCEPT"), "text/event-stream"); assert.equal(getHeaderValueCaseInsensitive(obj, "x-foo"), "bar"); +}); - // plain-object values are trimmed +test("getHeaderValueCaseInsensitive trims plain-object string values", () => { assert.equal(getHeaderValueCaseInsensitive({ Accept: " v " }, "accept"), "v"); - // missing / non-object -> null +}); + +test("getHeaderValueCaseInsensitive ignores blank and non-string plain-object values", () => { + // whitespace-only value: value.trim() is falsy -> skipped -> null + assert.equal(getHeaderValueCaseInsensitive({ Accept: " " }, "accept"), null); + // empty string -> skipped -> null + assert.equal(getHeaderValueCaseInsensitive({ Accept: "" }, "accept"), null); + // non-string values are not strings -> skipped -> null + assert.equal(getHeaderValueCaseInsensitive({ "Content-Length": 42 }, "content-length"), null); + assert.equal(getHeaderValueCaseInsensitive({ Flag: true }, "flag"), null); +}); + +test("getHeaderValueCaseInsensitive returns null for missing key on plain object", () => { assert.equal(getHeaderValueCaseInsensitive({ Accept: "x" }, "missing"), null); +}); + +test("getHeaderValueCaseInsensitive returns null for null/undefined/non-object inputs", () => { assert.equal(getHeaderValueCaseInsensitive(null, "accept"), null); assert.equal(getHeaderValueCaseInsensitive(undefined, "accept"), null); + // a non-object (typeof !== "object") short-circuits to null + assert.equal( + getHeaderValueCaseInsensitive("text/event-stream" as unknown as Record, "accept"), + null + ); }); diff --git a/tests/unit/chatcore-log-truncation.test.ts b/tests/unit/chatcore-log-truncation.test.ts index 704b57e252..5f7e68d62a 100644 --- a/tests/unit/chatcore-log-truncation.test.ts +++ b/tests/unit/chatcore-log-truncation.test.ts @@ -3,44 +3,149 @@ import assert from "node:assert/strict"; import { capMemoryExtractionText, - truncateForLog, + truncateChatLogText, cloneBoundedChatLogPayload, + truncateForLog, MEMORY_EXTRACTION_TEXT_LIMIT, } from "../../open-sse/handlers/chatCore/logTruncation.ts"; +import { + getChatLogTextLimit, + getChatLogArrayTailItems, + getChatLogMaxDepth, + getChatLogMaxObjectKeys, +} from "../../src/lib/logEnv.ts"; -test("capMemoryExtractionText keeps short strings and tail-truncates long ones", () => { - assert.equal(capMemoryExtractionText("hello"), "hello"); - const long = "x".repeat(MEMORY_EXTRACTION_TEXT_LIMIT + 100); - const capped = capMemoryExtractionText(long); - assert.equal(capped.length, MEMORY_EXTRACTION_TEXT_LIMIT); - assert.ok(capped.endsWith("x")); +test("MEMORY_EXTRACTION_TEXT_LIMIT is the documented 64KB constant", () => { + assert.equal(MEMORY_EXTRACTION_TEXT_LIMIT, 64 * 1024); }); -test("truncateForLog summarizes oversized objects and passes through small ones", () => { +test("capMemoryExtractionText returns short strings unchanged", () => { + assert.equal(capMemoryExtractionText("hello"), "hello"); + // exactly at the limit is not truncated (<= limit) + const exact = "x".repeat(MEMORY_EXTRACTION_TEXT_LIMIT); + assert.equal(capMemoryExtractionText(exact), exact); +}); + +test("capMemoryExtractionText keeps the trailing window of over-limit strings", () => { + const long = "a".repeat(MEMORY_EXTRACTION_TEXT_LIMIT) + "TAIL"; + const capped = capMemoryExtractionText(long); + assert.equal(capped.length, MEMORY_EXTRACTION_TEXT_LIMIT); + // slice(-LIMIT) keeps the end, so the appended TAIL survives. + assert.ok(capped.endsWith("TAIL")); +}); + +test("truncateChatLogText passes through strings up to the configured limit", () => { + const limit = getChatLogTextLimit(); + const under = "y".repeat(limit); + assert.equal(truncateChatLogText(under), under); +}); + +test("truncateChatLogText builds head + marker + tail for over-limit strings", () => { + const limit = getChatLogTextLimit(); + const head = "H".repeat(Math.floor(limit / 2)); + const tail = "T".repeat(Math.ceil(limit / 2)); + // make the total strictly larger than the limit by inserting a middle chunk + const middle = "M".repeat(500); + const value = head + middle + tail; + const out = truncateChatLogText(value); + const expectedHead = value.slice(0, Math.floor(limit / 2)); + const expectedTail = value.slice(-Math.ceil(limit / 2)); + assert.equal( + out, + `${expectedHead}\n[...truncated ${value.length - limit} chars...]\n${expectedTail}` + ); + // sanity: the marker reports exactly the number of dropped chars + assert.ok(out.includes(`[...truncated ${value.length - limit} chars...]`)); +}); + +test("cloneBoundedChatLogPayload returns null/undefined/primitives as-is", () => { + assert.equal(cloneBoundedChatLogPayload(null), null); + assert.equal(cloneBoundedChatLogPayload(undefined), undefined); + assert.equal(cloneBoundedChatLogPayload(42), 42); + assert.equal(cloneBoundedChatLogPayload(true), true); +}); + +test("cloneBoundedChatLogPayload truncates long string leaves via truncateChatLogText", () => { + const limit = getChatLogTextLimit(); + const big = "z".repeat(limit + 1000); + const out = cloneBoundedChatLogPayload(big) as string; + assert.equal(out, truncateChatLogText(big)); + assert.ok(out.includes("[...truncated")); +}); + +test("cloneBoundedChatLogPayload tail-truncates long arrays with a marker entry", () => { + const maxTail = getChatLogArrayTailItems(); + const n = maxTail + 100; + const cloned = cloneBoundedChatLogPayload(new Array(n).fill("a")) as unknown[]; + // marker prepended + the retained tail items + assert.equal(cloned.length, maxTail + 1); + const marker = cloned[0] as Record; + assert.equal(marker._omniroute_truncated_array, true); + assert.equal(marker.originalLength, n); + assert.equal(marker.retainedTailItems, maxTail); +}); + +test("cloneBoundedChatLogPayload leaves short arrays unmarked", () => { + const cloned = cloneBoundedChatLogPayload(["a", "b", "c"]) as unknown[]; + assert.deepEqual(cloned, ["a", "b", "c"]); +}); + +test("cloneBoundedChatLogPayload caps object keys and records the dropped count", () => { + const maxKeys = getChatLogMaxObjectKeys(); + const obj: Record = {}; + for (let i = 0; i < maxKeys + 5; i += 1) obj[`k${i}`] = i; + const cloned = cloneBoundedChatLogPayload(obj) as Record; + // first maxKeys keys retained + the synthetic _omniroute_truncated_keys field + assert.equal(cloned._omniroute_truncated_keys, 5); + assert.equal(cloned.k0, 0); + assert.equal(cloned[`k${maxKeys - 1}`], maxKeys - 1); + // a key beyond the cap is dropped + assert.equal(cloned[`k${maxKeys}`], undefined); +}); + +test("cloneBoundedChatLogPayload stops at max depth with a [MaxDepth] sentinel", () => { + const depth = getChatLogMaxDepth(); + // build nesting that goes one level past the cap + let leaf: Record = { deep: "value" }; + for (let i = 0; i < depth + 1; i += 1) leaf = { nested: leaf }; + const cloned = cloneBoundedChatLogPayload(leaf) as Record; + // walk down `depth` levels; the level at >= maxDepth becomes the sentinel string + let cursor: unknown = cloned; + for (let i = 0; i < depth; i += 1) { + cursor = (cursor as Record).nested; + } + assert.equal(cursor, "[MaxDepth]"); +}); + +test("truncateForLog returns null/undefined and non-object primitives unchanged", () => { + assert.equal(truncateForLog(null), null); + assert.equal(truncateForLog(undefined), undefined); + assert.equal(truncateForLog("hi" as unknown), "hi" as unknown); +}); + +test("truncateForLog passes small objects through untouched", () => { const small = { model: "gpt-4o", messages: [{ role: "user", content: "hi" }] }; assert.equal(truncateForLog(small), small); +}); +test("truncateForLog summarizes oversized payloads instead of cloning", () => { const huge = { model: "gpt-4o", provider: "openai", stream: true, - // Use Array.from to create distinct object references so estimateSizeFast - // (which deduplicates via WeakSet) counts every message individually. + // distinct object references so estimateSizeFast (WeakSet-dedup) counts each one messages: Array.from({ length: 50000 }, () => ({ role: "user", content: "x".repeat(64) })), + contents: [{ a: 1 }], }; const summary = truncateForLog(huge) as Record; assert.equal(summary._truncated, true); + assert.equal(typeof summary._originalBytes, "number"); + assert.ok((summary._originalBytes as number) > 8 * 1024); assert.equal(summary.model, "gpt-4o"); assert.equal(summary.provider, "openai"); assert.equal(summary.messageCount, 50000); + assert.equal(summary.contentCount, 1); assert.equal(summary.stream, true); -}); - -test("cloneBoundedChatLogPayload truncates long tail arrays with a marker", () => { - const cloned = cloneBoundedChatLogPayload({ items: new Array(1000).fill("a") }) as { - items: unknown[]; - }; - const marker = cloned.items[0] as Record; - assert.equal(marker._omniroute_truncated_array, true); - assert.equal(marker.originalLength, 1000); + // the original (huge) is NOT returned — it is a fresh summary object + assert.notEqual(summary, huge); }); diff --git a/tests/unit/chatcore-memory-extraction.test.ts b/tests/unit/chatcore-memory-extraction.test.ts index 5eece2bf0c..bca5d15f6a 100644 --- a/tests/unit/chatcore-memory-extraction.test.ts +++ b/tests/unit/chatcore-memory-extraction.test.ts @@ -7,20 +7,49 @@ import { resolveMemoryOwnerId, } from "../../open-sse/handlers/chatCore/memoryExtraction.ts"; -test("extractMemoryTextFromResponse reads OpenAI, Claude-array and responses output_text", () => { +test("extractMemoryTextFromResponse reads OpenAI choices[0].message.content (trimmed)", () => { assert.equal( extractMemoryTextFromResponse({ choices: [{ message: { content: " hi " } }] }), "hi" ); - assert.equal( - extractMemoryTextFromResponse({ content: [{ type: "text", text: " a " }, { type: "image" }] }), - "a" - ); - assert.equal(extractMemoryTextFromResponse({ output_text: " out " }), "out"); - assert.equal(extractMemoryTextFromResponse(null), ""); }); -test("extractMemoryTextFromRequestBody returns the last user message text", () => { +test("extractMemoryTextFromResponse joins Claude content text blocks and skips non-text", () => { + assert.equal( + extractMemoryTextFromResponse({ + content: [ + { type: "text", text: " a " }, + { type: "image" }, + { type: "text", text: "b" }, + ], + }), + "a\nb" + ); +}); + +test("extractMemoryTextFromResponse falls back to Responses output_text", () => { + assert.equal(extractMemoryTextFromResponse({ output_text: " out " }), "out"); +}); + +test("extractMemoryTextFromResponse returns empty string for null/empty/no-text shapes", () => { + assert.equal(extractMemoryTextFromResponse(null), ""); + assert.equal(extractMemoryTextFromResponse(undefined), ""); + assert.equal(extractMemoryTextFromResponse({}), ""); + // content array with no text parts -> contentText is "" -> falls through to "" + assert.equal(extractMemoryTextFromResponse({ content: [{ type: "image" }] }), ""); +}); + +test("extractMemoryTextFromResponse prefers OpenAI content over output_text", () => { + assert.equal( + extractMemoryTextFromResponse({ + choices: [{ message: { content: "openai" } }], + output_text: "responses", + }), + "openai" + ); +}); + +test("extractMemoryTextFromRequestBody returns the LAST user message (string content)", () => { const body = { messages: [ { role: "user", content: "first" }, @@ -29,14 +58,79 @@ test("extractMemoryTextFromRequestBody returns the last user message text", () = ], }; assert.equal(extractMemoryTextFromRequestBody(body), "second"); +}); + +test("extractMemoryTextFromRequestBody joins array content parts of the last user message", () => { + const body = { + messages: [ + { + role: "user", + content: [ + { type: "input_text", text: " a " }, + { text: "b" }, + { type: "image_url" }, + ], + }, + ], + }; + assert.equal(extractMemoryTextFromRequestBody(body), "a\nb"); +}); + +test("extractMemoryTextFromRequestBody reads Responses-style input items", () => { const inputBody = { input: [{ role: "user", type: "message", content: [{ type: "input_text", text: "hey" }] }], }; assert.equal(extractMemoryTextFromRequestBody(inputBody), "hey"); }); -test("resolveMemoryOwnerId returns trimmed id or null", () => { +test("extractMemoryTextFromRequestBody reads a string-content input item", () => { + const inputBody = { + input: [{ role: "user", type: "message", content: " plain " }], + }; + assert.equal(extractMemoryTextFromRequestBody(inputBody), "plain"); +}); + +test("extractMemoryTextFromRequestBody scans input items from the end and returns the last user text", () => { + // The primary input scan walks from the end and returns immediately on the + // last item whose role is user (or unset) and type is message (or unset). + const inputBody = { + input: [ + { type: "reasoning", content: "thinking" }, // itemType !== "message" -> skipped + { role: "user", content: "alpha" }, // matches, but an earlier item from the end wins + { role: "user", content: "beta" }, // last matching item -> returned + ], + }; + assert.equal(extractMemoryTextFromRequestBody(inputBody), "beta"); +}); + +test("extractMemoryTextFromRequestBody skips assistant input items but accepts the prior user item", () => { + const inputBody = { + input: [ + { role: "user", type: "message", content: "the question" }, + { role: "assistant", type: "message", content: "the answer" }, // role !== user -> skipped + ], + }; + assert.equal(extractMemoryTextFromRequestBody(inputBody), "the question"); +}); + +test("extractMemoryTextFromRequestBody returns empty for null/empty/no-user bodies", () => { + assert.equal(extractMemoryTextFromRequestBody(null), ""); + assert.equal(extractMemoryTextFromRequestBody(undefined), ""); + assert.equal(extractMemoryTextFromRequestBody({}), ""); + // only an assistant message -> no user text + assert.equal( + extractMemoryTextFromRequestBody({ messages: [{ role: "assistant", content: "x" }] }), + "" + ); +}); + +test("resolveMemoryOwnerId returns the id when present, null otherwise", () => { assert.equal(resolveMemoryOwnerId({ id: "key_123" }), "key_123"); + // whitespace-only id is rejected assert.equal(resolveMemoryOwnerId({ id: " " }), null); + // non-string id is rejected + assert.equal(resolveMemoryOwnerId({ id: 7 } as unknown as Record), null); + // missing id / null info + assert.equal(resolveMemoryOwnerId({}), null); assert.equal(resolveMemoryOwnerId(null), null); });