From 6cb5af17d07d80cb92058004914e49c962dfbb5d Mon Sep 17 00:00:00 2001 From: autodev-bot Date: Thu, 23 Jul 2026 18:16:20 +0800 Subject: [PATCH 1/2] fix(plugin): wire capture reflection to main llm (#2148) Capture batch reflection is a JSON-output task. When operators configure skillEvolver.enableThinking=true alongside llm.enableThinking=false (the typical Qwen3 setup), passing `deps.reflectLlm` into the capture runner made the batch reflection run on the thinking-enabled skill-evolver model, which emits blocks that break JSON parsing and produce `malformed JSON` errors during capture summarisation. Route the capture runner's reflectLlm slot to `deps.llm` so JSON tasks stay on the non-thinking main model, matching the l3Llm pattern from #1959. - deps.ts: captureRunner.reflectLlm = deps.llm - tests: regression guard in tests/unit/pipeline/capture-reflect-llm-wiring.test.ts Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/memos-local-plugin/core/pipeline/deps.ts | 13 +- .../capture-reflect-llm-wiring.test.ts | 190 ++++++++++++++++++ 2 files changed, 202 insertions(+), 1 deletion(-) create mode 100644 apps/memos-local-plugin/tests/unit/pipeline/capture-reflect-llm-wiring.test.ts diff --git a/apps/memos-local-plugin/core/pipeline/deps.ts b/apps/memos-local-plugin/core/pipeline/deps.ts index decf974d0..a65d1d1bd 100644 --- a/apps/memos-local-plugin/core/pipeline/deps.ts +++ b/apps/memos-local-plugin/core/pipeline/deps.ts @@ -213,7 +213,18 @@ export function buildPipelineSubscribers( episodesRepo: adaptEpisodesRepo(deps.repos.episodes), embedder: deps.embedder, llm: deps.llm, - reflectLlm: deps.reflectLlm, + // Issue #2148: capture batch reflection is a JSON-output task, so + // it must run on the main `llm` — NOT `deps.reflectLlm` (the + // skill-evolver model). When the operator configures + // `skillEvolver.enableThinking=true` alongside + // `llm.enableThinking=false` (typical Qwen3 setup), passing + // `reflectLlm` here makes the reflection call return prose with + // `...` blocks, which break JSON parsing and produce + // `malformed JSON` errors during summarisation. The dedicated + // `reflectLlm` client is still exposed on `PipelineDeps` for the + // reward-runner's evaluator metadata (see below) and the overview + // health endpoint — capture just picks the JSON-safe main llm. + reflectLlm: deps.llm, bus: buses.capture, cfg: algorithm.capture, now: deps.now, diff --git a/apps/memos-local-plugin/tests/unit/pipeline/capture-reflect-llm-wiring.test.ts b/apps/memos-local-plugin/tests/unit/pipeline/capture-reflect-llm-wiring.test.ts new file mode 100644 index 000000000..91229a364 --- /dev/null +++ b/apps/memos-local-plugin/tests/unit/pipeline/capture-reflect-llm-wiring.test.ts @@ -0,0 +1,190 @@ +/** + * Regression test for issue #2148 — + * `captureRunner` must receive the main `llm` for its batch-reflection + * pass, NOT `reflectLlm` (skill-evolver). + * + * Background: batch reflection is a JSON-output task. When the operator + * configures a stronger, thinking-enabled model under `skillEvolver.*` + * and leaves `llm.enableThinking=false`, wiring `reflectLlm` (skill- + * evolver) into the capture pipeline makes reflection produce + * `...` blocks that break JSON parsing. The skill-evolver + * model is intended for skill crystallization, not capture reflection. + * + * This test locks the wiring at the pipeline layer: no matter what the + * caller sets on `deps.reflectLlm`, `buildPipelineSubscribers` must pass + * `deps.llm` to `createCaptureRunner`'s `reflectLlm` slot. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { + LlmCallOptions, + LlmClient, + LlmClientStats, + LlmMessage, + LlmProviderName, +} from "../../../core/llm/types.js"; + +const captureRunnerCalls: Array<{ + llm: LlmClient | null; + reflectLlm: LlmClient | null; +}> = []; + +vi.mock("../../../core/capture/index.js", async () => { + const actual = await vi.importActual< + typeof import("../../../core/capture/index.js") + >("../../../core/capture/index.js"); + return { + ...actual, + createCaptureRunner: (deps: { + llm: LlmClient | null; + reflectLlm: LlmClient | null; + [k: string]: unknown; + }) => { + captureRunnerCalls.push({ llm: deps.llm, reflectLlm: deps.reflectLlm }); + return actual.createCaptureRunner( + deps as Parameters[0], + ); + }, + }; +}); + +import { + buildPipelineBuses, + buildPipelineSession, + buildPipelineSubscribers, + extractAlgorithmConfig, + type PipelineDeps, +} from "../../../core/pipeline/index.js"; +import { DEFAULT_CONFIG } from "../../../core/config/defaults.js"; +import { resolveHome } from "../../../core/config/paths.js"; +import { rootLogger } from "../../../core/logger/index.js"; +import { makeTmpDb, type TmpDbHandle } from "../../helpers/tmp-db.js"; +import { fakeEmbedder } from "../../helpers/fake-embedder.js"; + +function fakeLlmClient(name: string): LlmClient { + return { + provider: "local_only" as LlmProviderName, + model: name, + canStream: false, + async complete(_messages: LlmMessage[] | string, _opts?: LlmCallOptions) { + return { + text: "{}", + provider: "local_only" as LlmProviderName, + model: name, + servedBy: "local_only" as LlmProviderName, + durationMs: 0, + }; + }, + async completeJson() { + return { + value: {} as T, + raw: "{}", + provider: "local_only" as LlmProviderName, + model: name, + servedBy: "local_only" as LlmProviderName, + durationMs: 0, + }; + }, + async *stream() { + yield { delta: "", done: true }; + }, + stats(): LlmClientStats { + return { + requests: 0, + hostFallbacks: 0, + failures: 0, + retries: 0, + totalPromptTokens: 0, + totalCompletionTokens: 0, + lastOkAt: null, + lastError: null, + lastStatus: null, + }; + }, + resetStats() {}, + async close() {}, + }; +} + +let dbHandle: TmpDbHandle | null = null; + +function buildDepsWithDistinctLlms( + h: TmpDbHandle, + lightweight: boolean, +): PipelineDeps { + return { + agent: "openclaw", + home: resolveHome("openclaw", "/tmp/memos-issue-2148-test"), + config: { + ...DEFAULT_CONFIG, + algorithm: { + ...DEFAULT_CONFIG.algorithm, + lightweightMemory: { + ...DEFAULT_CONFIG.algorithm.lightweightMemory, + enabled: lightweight, + }, + }, + }, + db: h.db, + repos: h.repos, + llm: fakeLlmClient("main-llm"), + reflectLlm: fakeLlmClient("skill-evolver-llm"), + l3Llm: fakeLlmClient("l3-llm"), + embedder: fakeEmbedder({ dimensions: 384 }), + log: rootLogger.child({ channel: "test.issue-2148" }), + namespace: { agentKind: "openclaw", profileId: "main" }, + now: () => 1_700_000_000_000, + }; +} + +beforeEach(() => { + dbHandle = makeTmpDb(); + captureRunnerCalls.length = 0; +}); + +afterEach(() => { + dbHandle?.cleanup(); + dbHandle = null; +}); + +describe("pipeline/deps captureRunner wiring (issue #2148)", () => { + it("passes the main llm — not reflectLlm — as the capture runner's reflectLlm slot (normal mode)", () => { + const buses = buildPipelineBuses(); + const deps = buildDepsWithDistinctLlms(dbHandle!, false); + const algorithm = extractAlgorithmConfig(deps); + const session = buildPipelineSession(deps, buses.session); + buildPipelineSubscribers(deps, buses, algorithm, session); + + expect(captureRunnerCalls).toHaveLength(1); + const call = captureRunnerCalls[0]; + + // The main `llm` slot is unchanged: still the main model. + expect(call.llm?.model).toBe("main-llm"); + + // Regression guard: even though `deps.reflectLlm` is a distinct + // skill-evolver model (with e.g. enableThinking=true in real use), + // the capture pipeline must ignore it and use the main llm — batch + // reflection is a JSON-output task and cannot tolerate thinking + // tags. See issue #2148. + expect(call.reflectLlm?.model).toBe("main-llm"); + expect(call.reflectLlm?.model).not.toBe("skill-evolver-llm"); + }); + + it("passes the main llm as reflectLlm even in lightweight mode", () => { + // Lightweight mode still constructs the capture runner (it's what + // handles the lite/lightweight capture paths); the reflect pass is + // gated separately. The wiring guard must hold either way so a + // future flip of the flag can't reintroduce the bug. + const buses = buildPipelineBuses(); + const deps = buildDepsWithDistinctLlms(dbHandle!, true); + const algorithm = extractAlgorithmConfig(deps); + const session = buildPipelineSession(deps, buses.session); + buildPipelineSubscribers(deps, buses, algorithm, session); + + expect(captureRunnerCalls).toHaveLength(1); + const call = captureRunnerCalls[0]; + expect(call.llm?.model).toBe("main-llm"); + expect(call.reflectLlm?.model).toBe("main-llm"); + }); +}); From 5f1d0d3c1d9ee2276a9378564df02ab5abc5194c Mon Sep 17 00:00:00 2001 From: MemTensor Bot Date: Thu, 23 Jul 2026 18:40:52 +0800 Subject: [PATCH 2/2] docs(plugin): clarify reflectLlm exposure comment (#2148 OCR follow-up) Enumerate the two read-only consumers that still reach `deps.reflectLlm` (reward-runner evaluator metadata + Overview health card via `resolveSkillEvolver`) and note that no other subscriber is wired to it. Prevents future readers from misreading the previous prose as "skill / capture also use reflectLlm". --- apps/memos-local-plugin/core/pipeline/deps.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/memos-local-plugin/core/pipeline/deps.ts b/apps/memos-local-plugin/core/pipeline/deps.ts index a65d1d1bd..6bea606b0 100644 --- a/apps/memos-local-plugin/core/pipeline/deps.ts +++ b/apps/memos-local-plugin/core/pipeline/deps.ts @@ -221,9 +221,17 @@ export function buildPipelineSubscribers( // `reflectLlm` here makes the reflection call return prose with // `...` blocks, which break JSON parsing and produce // `malformed JSON` errors during summarisation. The dedicated - // `reflectLlm` client is still exposed on `PipelineDeps` for the - // reward-runner's evaluator metadata (see below) and the overview - // health endpoint — capture just picks the JSON-safe main llm. + // `reflectLlm` client is still exposed on `PipelineDeps` for two + // read-only, non-JSON consumers only: + // 1. the reward-runner's evaluator metadata (see below) — just + // provider/model strings, no LLM call runs on `reflectLlm` + // from here. + // 2. the Overview health card — reads `handle.reflectLlm.stats()` + // via `resolveSkillEvolver` in `memory-core.ts` to paint the + // skill-evolver slot. + // No other subscriber (capture / skill / l2 / l3 / feedback) is + // wired to `reflectLlm`; capture in particular picks the JSON-safe + // main llm here. reflectLlm: deps.llm, bus: buses.capture, cfg: algorithm.capture,