From baa921d9bee0879bb086ae763a176fc7158b3feb Mon Sep 17 00:00:00 2001 From: AutoDev Bot Date: Thu, 17 Sep 2026 05:12:34 +0800 Subject: [PATCH 1/2] fix(health): fix model slot status lookup not bounded by top-N window (#2380) findLatestPersistedModelStatus() previously fetched the newest 500 api_logs rows and post-filtered in JS for role/provider/model. On busy installs the target slot (e.g. skillEvolver) is pushed >500 positions below the head by high-frequency llm/embedding writes, causing the lookup to silently return null and the Overview card to show 'Not called yet' even when historical rows exist. Fix: add findLatestModelStatus(role, provider, model) to the api_logs repo. It uses json_extract() to filter at SQL level with no window, returning the single newest matching row. Update findLatestPersistedModelStatus() to delegate to this method instead. Regression test seeds 600 llm rows after one skillEvolver row (putting the target 600 positions out of the old window) and asserts health() returns the correct lastOkAt timestamp. --- .../core/pipeline/memory-core.ts | 54 ++++----- .../core/storage/repos/api_logs.ts | 46 ++++++++ apps/memos-local-plugin/pnpm-lock.yaml | 13 ++- .../pipeline/health-model-display.test.ts | 103 ++++++++++++++++++ 4 files changed, 179 insertions(+), 37 deletions(-) diff --git a/apps/memos-local-plugin/core/pipeline/memory-core.ts b/apps/memos-local-plugin/core/pipeline/memory-core.ts index 01402a86b..d3fc1e864 100644 --- a/apps/memos-local-plugin/core/pipeline/memory-core.ts +++ b/apps/memos-local-plugin/core/pipeline/memory-core.ts @@ -5750,42 +5750,34 @@ function findLatestPersistedModelStatus( message?: string; } | null { try { - const rows = repos.apiLogs.list({ - toolName: "system_model_status", - limit: 500, - offset: 0, - }); - for (const row of rows) { - try { - const out = JSON.parse(row.outputJson) as { - role?: unknown; - status?: unknown; - provider?: unknown; - model?: unknown; - message?: unknown; - }; - if (out.role !== role) continue; - // Only apply status rows for the currently configured model. - // This prevents an old 404 for a typo'd model from keeping the - // card red after the operator fixes Settings and restarts. - if (String(out.provider ?? "") !== provider) continue; - if (String(out.model ?? "") !== model) continue; - if (out.status !== "ok" && out.status !== "fallback" && out.status !== "error") { - continue; - } - return { - status: out.status, - at: row.calledAt, - message: typeof out.message === "string" ? out.message : undefined, - }; - } catch { - // Malformed row — skip and keep walking. + // Use a targeted SQL query that filters role/provider/model via + // json_extract() so the result is never bounded by a top-N window. + // The previous approach (list 500 rows, post-filter) silently returned + // null whenever the target slot's row had been pushed >500 positions + // below the head by higher-frequency llm/embedding writes (#2380). + const row = repos.apiLogs.findLatestModelStatus({ role, provider, model }); + if (!row) return null; + try { + const out = JSON.parse(row.outputJson) as { + status?: unknown; + message?: unknown; + }; + if (out.status !== "ok" && out.status !== "fallback" && out.status !== "error") { + return null; } + return { + status: out.status, + at: row.calledAt, + message: typeof out.message === "string" ? out.message : undefined, + }; + } catch { + // Malformed row — treat as missing. + return null; } } catch { // Repo failure is non-fatal for health; leave in-memory stats. + return null; } - return null; } type RetrievalStatsLogPayload = { diff --git a/apps/memos-local-plugin/core/storage/repos/api_logs.ts b/apps/memos-local-plugin/core/storage/repos/api_logs.ts index 4d56a4842..961467b78 100644 --- a/apps/memos-local-plugin/core/storage/repos/api_logs.ts +++ b/apps/memos-local-plugin/core/storage/repos/api_logs.ts @@ -46,6 +46,12 @@ export interface ApiLogFilter { offset?: number; } +export interface ModelStatusFilter { + role: string; + provider: string; + model: string; +} + export function makeApiLogsRepo(db: StorageDb) { const insert = db.prepare< { @@ -87,6 +93,31 @@ export function makeApiLogsRepo(db: StorageDb) { LIMIT @limit OFFSET @offset`, ); + /** + * Find the single newest `system_model_status` row whose `output_json` + * encodes the given role/provider/model triple. Filtering is done in SQL + * via `json_extract()` so the query is not bounded by a top-N window — + * it scans the whole `api_logs` table but returns at most one row. + * + * This replaces the previous pattern of fetching the newest 500 rows and + * post-filtering in JS, which silently returned `null` whenever the target + * slot's most-recent row had been pushed further than 500 positions below + * the head by higher-frequency `llm`/`embedding` writes. + */ + const selectLatestModelStatus = db.prepare< + { role: string; provider: string; model: string }, + RawRow + >( + `SELECT id, tool_name, input_json, output_json, duration_ms, success, called_at + FROM api_logs + WHERE tool_name = 'system_model_status' + AND json_extract(output_json, '$.role') = @role + AND json_extract(output_json, '$.provider') = @provider + AND json_extract(output_json, '$.model') = @model + ORDER BY called_at DESC, id DESC + LIMIT 1`, + ); + const countByToolNames = (toolNames: readonly string[]): number => { const names = normalizeToolNames(toolNames); if (names.length === 0) return countAll.get({})?.n ?? 0; @@ -158,6 +189,21 @@ export function makeApiLogsRepo(db: StorageDb) { : selectAll.all({ limit, offset }); return rows.map(mapRow); }, + + /** + * Return the single newest `system_model_status` row whose + * `output_json` matches the given role/provider/model triple, or + * `null` if no such row exists. Filtering is done entirely in SQL so + * the result is not constrained by any top-N window. + */ + findLatestModelStatus(f: ModelStatusFilter): ApiLogRow | null { + const row = selectLatestModelStatus.get({ + role: f.role, + provider: f.provider, + model: f.model, + }); + return row ? mapRow(row) : null; + }, }; } diff --git a/apps/memos-local-plugin/pnpm-lock.yaml b/apps/memos-local-plugin/pnpm-lock.yaml index 8d693a97e..88296d805 100644 --- a/apps/memos-local-plugin/pnpm-lock.yaml +++ b/apps/memos-local-plugin/pnpm-lock.yaml @@ -18,8 +18,8 @@ importers: specifier: ^0.34.48 version: 0.34.49 better-sqlite3: - specifier: ^12.6.3 - version: 12.9.0 + specifier: ^12.10.0 + version: 12.11.1 preact: specifier: ^10.29.1 version: 10.29.1 @@ -900,9 +900,9 @@ packages: engines: {node: '>=6.0.0'} hasBin: true - better-sqlite3@12.9.0: - resolution: {integrity: sha512-wqUv4Gm3toFpHDQmaKD4QhZm3g1DjUBI0yzS4UBl6lElUmXFYdTQmmEDpAFa5o8FiFiymURypEnfVHzILKaxqQ==} - engines: {node: 20.x || 22.x || 23.x || 24.x || 25.x} + better-sqlite3@12.11.1: + resolution: {integrity: sha512-dq9AtApgg5PGFtBzPFSBl3HZQjHok5gaQCM6zh2Yk0aSmDCs1CbnVI8/HgASQkNKsWFpseIO9beg5xxpYhbIfA==} + engines: {node: 20.x || 22.x || 23.x || 24.x || 25.x || 26.x} bindings@1.5.0: resolution: {integrity: sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==} @@ -1401,6 +1401,7 @@ packages: uuid@10.0.0: resolution: {integrity: sha512-8XkAphELsDnEGrDxUOHB3RGvXz6TeuYSGEZBOjtTtPm2lwhGBjLgOzLHB63IUWfBpNucQjND6d3AOudO+H3RWQ==} + deprecated: uuid@10 and below is no longer supported. For ESM codebases, update to uuid@latest. For CommonJS codebases, use uuid@11 (but be aware this version will likely be deprecated in 2028). hasBin: true vite-node@2.1.9: @@ -2124,7 +2125,7 @@ snapshots: baseline-browser-mapping@2.10.20: {} - better-sqlite3@12.9.0: + better-sqlite3@12.11.1: dependencies: bindings: 1.5.0 prebuild-install: 7.1.3 diff --git a/apps/memos-local-plugin/tests/unit/pipeline/health-model-display.test.ts b/apps/memos-local-plugin/tests/unit/pipeline/health-model-display.test.ts index ad401d664..d76fb09f2 100644 --- a/apps/memos-local-plugin/tests/unit/pipeline/health-model-display.test.ts +++ b/apps/memos-local-plugin/tests/unit/pipeline/health-model-display.test.ts @@ -243,6 +243,109 @@ llm: expect(h.embedder?.provider).toBe("gemini"); }); + /** + * Regression test for issue #2380: + * findLatestPersistedModelStatus previously fetched the newest 500 + * api_logs rows and post-filtered in JS. On busy installs the target + * slot's row is pushed >500 positions below the head by high-frequency + * llm/embedding writes, so the lookup silently returned null and the + * Overview card read "Not called yet" even though rows existed. + * + * The fix uses json_extract() in SQL so the query is not bounded by any + * top-N window. + */ + it("finds skillEvolver status row even when >500 higher-frequency rows follow it (#2380)", async () => { + home = await makeTmpHome({ + agent: "openclaw", + configYaml: ` +version: 1 +llm: + provider: openai_compatible + endpoint: https://example.test/v1 + model: gpt-4o-mini + apiKey: sk-test +skillEvolver: + provider: anthropic + endpoint: https://example.test + model: claude-skill-evolver-test + apiKey: sk-test-skill +algorithm: + lightweightMemory: + enabled: false +`, + }); + + // Phase 1: run migrations so the DB file and schema exist, then shut down + // cleanly so we can seed rows via a second SQLite connection. + const seeder = await bootstrapMemoryCore({ + agent: "openclaw", + home: home.home, + config: home.config, + pkgVersion: "seed-2380", + }); + await seeder.init(); + await seeder.shutdown(); + + // Phase 2: seed rows directly — one skillEvolver "ok" row followed by + // 600 llm rows. After this the skillEvolver row sits 600 positions below + // the table head, well outside the old 500-row scan window. + const Sqlite = (await import("better-sqlite3")).default; + const seedDb = new Sqlite(home.home.dbFile); + const now = 1_700_000_000_000; + + const insertRow = seedDb.prepare( + `INSERT INTO api_logs (tool_name, input_json, output_json, duration_ms, success, called_at) + VALUES (?, ?, ?, ?, ?, ?)`, + ); + + insertRow.run( + "system_model_status", + "{}", + JSON.stringify({ + role: "skillEvolver", + provider: "anthropic", + model: "claude-skill-evolver-test", + status: "ok", + }), + 10, + 1, + now, + ); + + for (let i = 1; i <= 600; i++) { + insertRow.run( + "system_model_status", + "{}", + JSON.stringify({ + role: "llm", + provider: "openai_compatible", + model: "gpt-4o-mini", + status: "ok", + }), + 10, + 1, + now + i, + ); + } + + seedDb.close(); + + // Phase 3: boot the real core and verify health() surfaces the seeded row. + core = await bootstrapMemoryCore({ + agent: "openclaw", + home: home.home, + config: home.config, + pkgVersion: "check-2380", + }); + await core.init(); + + const h = await core.health(); + + // Before the fix this was null because the skillEvolver row was outside + // the 500-row scan window; with json_extract() filtering it is always found. + expect(h.skillEvolver?.lastOkAt).toBe(now); + }); + it("inherited skillEvolver reflects mid-flight llm changes (no restart yet)", async () => { home = await makeTmpHome({ agent: "openclaw", From 7ce4811ed3a34778a3d8139ab14c04ab870954e1 Mon Sep 17 00:00:00 2001 From: autodev Date: Thu, 17 Sep 2026 05:25:37 +0800 Subject: [PATCH 2/2] perf(storage): index-drive api_logs model-status lookup Follow-up to #2380 addressing OCR review feedback: the json_extract() predicates in findLatestModelStatus() had no supporting index, so the lookup was a full-table scan on every /health poll (api_logs holds up to ~10k rows on busy installs). Add migration 013 with a partial functional index that covers tool_name='system_model_status' and matches the query's filter + ORDER BY shape. SQLite's planner picks it up and turns the scan into an index seek. Migration 013 uses tableExists('api_logs') so the partial-schema test path in migrator.test.ts continues to pass. Regression-test the property that the /health lookup is index-driven (EXPLAIN QUERY PLAN sees `USING INDEX idx_api_logs_model_status`) so future changes can't quietly regress it. --- .../013-api-logs-model-status-index.sql | 23 +++++++++++ .../core/storage/migrator.ts | 6 +++ .../tests/unit/storage/migrator.test.ts | 40 +++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 apps/memos-local-plugin/core/storage/migrations/013-api-logs-model-status-index.sql diff --git a/apps/memos-local-plugin/core/storage/migrations/013-api-logs-model-status-index.sql b/apps/memos-local-plugin/core/storage/migrations/013-api-logs-model-status-index.sql new file mode 100644 index 000000000..7809b58aa --- /dev/null +++ b/apps/memos-local-plugin/core/storage/migrations/013-api-logs-model-status-index.sql @@ -0,0 +1,23 @@ +-- Speed up the health-check model-status lookup. +-- +-- After #2380 `findLatestModelStatus()` filters `api_logs` rows by +-- `json_extract(output_json, ...)` so the query is not bounded by any +-- top-N window. Without a functional index that is a full-table scan on +-- every /health poll, and `api_logs` grows to ~10k rows on busy installs. +-- +-- The partial functional index below covers exactly the rows the query +-- reads (`tool_name = 'system_model_status'`, a small fraction of the +-- table) and matches the query's filter + ORDER BY shape, so SQLite's +-- planner can turn the scan into an index seek. Functional indexes have +-- been supported since SQLite 3.9; the plugin ships with better-sqlite3 +-- v12 which bundles a recent build (>= 3.45). + +CREATE INDEX IF NOT EXISTS idx_api_logs_model_status + ON api_logs ( + json_extract(output_json, '$.role'), + json_extract(output_json, '$.provider'), + json_extract(output_json, '$.model'), + called_at DESC, + id DESC + ) + WHERE tool_name = 'system_model_status'; diff --git a/apps/memos-local-plugin/core/storage/migrator.ts b/apps/memos-local-plugin/core/storage/migrator.ts index 47e42807c..a0da698e5 100644 --- a/apps/memos-local-plugin/core/storage/migrator.ts +++ b/apps/memos-local-plugin/core/storage/migrator.ts @@ -202,6 +202,12 @@ function applyMigration(db: StorageDb, file: MigrationFile): void { } return; } + if (file.version === 13 && file.name === "api-logs-model-status-index") { + if (tableExists(db, "api_logs")) { + db.exec(fs.readFileSync(file.fullPath, "utf8")); + } + return; + } db.exec(fs.readFileSync(file.fullPath, "utf8")); } diff --git a/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts b/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts index c0e0eb215..4cccb9e18 100644 --- a/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts +++ b/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts @@ -205,4 +205,44 @@ describe("storage/migrator", () => { db.close(); } }); + + it("keeps the model-status lookup index-driven so /health polls stay O(log n) (#2381)", () => { + // Regression test for the partial functional index added to keep + // `findLatestModelStatus()` from doing a full table scan on every + // /health poll. `api_logs` grows to ~10k rows on busy installs. + const { dbPath, cleanup } = tmpDb(); + cleanups.push(cleanup); + const db = openDb({ filepath: dbPath, agent: "openclaw" }); + try { + runMigrations(db); + + const indexRow = db + .prepare( + `SELECT name FROM sqlite_master WHERE type='index' AND name='idx_api_logs_model_status'`, + ) + .get({}); + expect(indexRow?.name).toBe("idx_api_logs_model_status"); + + const plan = db + .prepare<{ role: string; provider: string; model: string }, { detail: string }>( + `EXPLAIN QUERY PLAN + SELECT id, tool_name, input_json, output_json, duration_ms, success, called_at + FROM api_logs + WHERE tool_name = 'system_model_status' + AND json_extract(output_json, '$.role') = @role + AND json_extract(output_json, '$.provider') = @provider + AND json_extract(output_json, '$.model') = @model + ORDER BY called_at DESC, id DESC + LIMIT 1`, + ) + .all({ role: "skillEvolver", provider: "anthropic", model: "claude-x" }); + + const usesIndex = plan.some((row) => + row.detail.includes("idx_api_logs_model_status"), + ); + expect(usesIndex).toBe(true); + } finally { + db.close(); + } + }); });