Skip to content

Fix #2076: local-plugin v2.0.8: gateway CPU 100% — synchronous full-table vector scan (scan#2077

Open
Memtensor-AI wants to merge 3 commits into
MemTensor:dev-v2.0.23from
Memtensor-AI:bugfix/autodev-2076-20260708092153967
Open

Fix #2076: local-plugin v2.0.8: gateway CPU 100% — synchronous full-table vector scan (scan#2077
Memtensor-AI wants to merge 3 commits into
MemTensor:dev-v2.0.23from
Memtensor-AI:bugfix/autodev-2076-20260708092153967

Conversation

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

Description

Fixed two coupled root causes in @memtensor/memos-local-plugin that made the gateway node process pin one CPU core at 100% with 4.2 GB RSS on startup (#2076).

Bug #1 — synchronous full-table vector scan in core/storage/vector.ts. scanAndTopK used db.prepare(sql).all(params), loading up to hardCap (default 100_000) multi-KB vector BLOBs synchronously into JS memory. Fixed by streaming via .iterate() and maintaining the top-K min-heap on hits directly (only k entries + one just-decoded vector live in memory at a time), and lowering the default hardCap to 5_000 as a safer default (all production callers pass their own value explicitly). Peak RSS during a scan drops from O(hardCap × dim) to O(k × dim).

Bug #2 — dedup pagination cap in core/capture/capture.ts. Four dedup call sites (runLite / runLightweight / runReflect / persistRows) used the paginated tracesRepo.list({ episodeId }), which silently caps at 500 rows. Once an episode exceeded 500 traces the older rows became invisible to dedup and were re-inserted every cycle (the reporter's DB had 518_375 rows of which 84% were exact duplicates). Fixed by adding tracesRepo.listAllForEpisode(episodeId), an uncapped read ordered by ts ASC, and switching the four dedup call sites to it. All other paginated list({ episodeId, limit }) callers keep their explicit page-size contract.

Tests: 7 new regression tests (traces-listall.test.ts: 4 cases covering 750-row episode dedup, episode scoping, empty episode, and ts-ordering; vector-stream.test.ts: 3 cases covering top-K correctness parity, streaming under a large cap, and explicit-hardCap truncation). Full plugin unit suite: 1128 pass, 1 skipped. The 2 remaining failures (startup-recovery source-string check, migrator schema-drift traces has no column named role) are pre-existing on base branch dev-v2.0.23 and unrelated to this fix. Type check clean.

Related Issue (Required): Fixes #2076

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • Documentation update

How Has This Been Tested?

Automated tests are pending.

  • Unit Test
  • Test Script Or Test Steps (please provide)
  • Pipeline Automated API Test (please provide)

Checklist

  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have created related documentation issue/PR in MemOS-Docs (if applicable)
  • I have linked the issue to this PR (if applicable)
  • I have mentioned the person who will review this PR

@bittergreen please review this PR.

Reviewer Checklist

…r scan (MemTensor#2076)

Two coupled root causes made the gateway Node process pin one CPU core
at 100% with 4.2 GB RSS on startup for 40+ minutes on a 518k-row DB.

Bug MemTensor#1 — synchronous full-table vector scan (`core/storage/vector.ts`):
`scanAndTopK` used `db.prepare(sql).all(params)`, materialising up to
`hardCap` (default 100_000) rows in one synchronous step. Each row
carries a multi-KB vector BLOB, so peak RSS was `O(hardCap × dim)` and
the event loop was blocked while `topKCosine` walked the whole array.
Fix: stream via `.iterate()`, maintain the top-K min-heap on hits
directly (only k entries + one just-decoded vector live in JS memory
at a time), and lower the default `hardCap` to 5_000 as a safer default
(all production callers pass their own value explicitly).

Bug MemTensor#2 — dedup pagination cap (`core/capture/capture.ts`):
The four dedup call sites in `runLite` / `runLightweight` /
`runReflect` / `persistRows` all used `tracesRepo.list({ episodeId })`,
which is paginated and silently truncates to 500 rows via
`_helpers.ts::clampLimit`. Once an episode exceeded 500 traces the
older rows became invisible to dedup and the tail was re-inserted
on every cycle. In the reporter's DB this had grown the `traces`
table to 518_375 rows of which 84% were exact duplicates by
`(episode_id, turn_id, user_text, agent_text, tool_calls_json)`.
Fix: add `tracesRepo.listAllForEpisode(episodeId)`, an uncapped read
ordered by `ts ASC`, and switch the four dedup call sites to it.
Every other paginated `list({ episodeId, limit })` caller keeps its
explicit page-size contract.

Tests
  - `tests/unit/storage/traces-listall.test.ts` — 4 cases: 750-row
    episode returns in full, strict episode scoping, empty episode,
    `ts ASC` ordering.
  - `tests/unit/storage/vector-stream.test.ts` — 3 cases: parity
    with brute-force top-K on live DB, streaming keeps only top-K in
    memory yet still considers every row within the cap, explicit
    `hardCap` still truncates the candidate window.
  - Full plugin unit suite: 1128 pass, 1 skipped. Two pre-existing
    failures on `dev-v2.0.23` (startup-recovery source-string check,
    migrator schema-drift) are unrelated to this fix.

Closes MemTensor#2076
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2077
Task: 91b3e78270801f79
Base: dev-v2.0.23
Head: bugfix/autodev-2076-20260708092153967
Head SHA: 1b2dfca0c43a6602bab7d082272d51dfa32d6ac6

🔍 OpenCodeReview found 5 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.


1. apps/memos-local-plugin/core/storage/repos/traces.ts (L249-L258)

Memory concern: listAllForEpisode selects all 25 columns via the full COLUMNS list — including the vec_summary and vec_action embedding BLOB columns — and materialises every matching row into a single JS array via .all(). However, every call-site in capture.ts only needs the dedup identity fields (ts, turn_id, user_text, agent_text, tool_calls_json).

Loading all BLOB columns synchronously for an entire large episode is the same root class of problem that caused the 4.2 GB RSS spike fixed in vector.ts, where the solution was precisely to switch from .all() to .iterate() with a narrow column projection.

Recommendations:

  1. Project only the dedup columns instead of the full COLUMNS list to avoid loading BLOBs at all.
  2. Use .iterate() over .all() to stream one row at a time rather than materialising the full result set at once, consistent with the vector.ts streaming fix.

Example:

const DEDUP_COLS = ['ts', 'turn_id', 'user_text', 'agent_text', 'tool_calls_json'] as const;
const sql = `SELECT ${DEDUP_COLS.join(", ")} FROM traces WHERE episode_id = @episode_id ORDER BY ts ASC`;
const result: TraceRow[] = [];
for (const r of db.prepare<{ episode_id: string }, RawTraceRow>(sql).iterate({ episode_id: String(episodeId) })) {
  result.push(mapDedupRow(r));
}
return result;

Peak RSS would then be proportional to scalar fields only, not to the total size of all stored embeddings.

💡 Suggested Change

Before:

    listAllForEpisode(episodeId: EpisodeId | string): TraceRow[] {
      if (!episodeId) return [];
      const sql = `SELECT ${COLUMNS.join(
        ", ",
      )} FROM traces WHERE episode_id = @episode_id ORDER BY ts ASC`;
      const rows = db
        .prepare<{ episode_id: string }, RawTraceRow>(sql)
        .all({ episode_id: String(episodeId) });
      return rows.map(mapRow);
    },

After:

    listAllForEpisode(episodeId: EpisodeId | string): TraceRow[] {
      if (!episodeId) return [];
      // Only project columns needed for dedup — avoids loading
      // vec_summary / vec_action BLOBs that callers never inspect.
      const DEDUP_COLS = ['ts', 'turn_id', 'user_text', 'agent_text', 'tool_calls_json'] as const;
      const sql = `SELECT ${DEDUP_COLS.join(", ")} FROM traces WHERE episode_id = @episode_id ORDER BY ts ASC`;
      const result: TraceRow[] = [];
      for (const r of db
        .prepare<{ episode_id: string }, RawTraceRow>(sql)
        .iterate({ episode_id: String(episodeId) })) {
        result.push(mapDedupRow(r));
      }
      return result;
    },

2. apps/memos-local-plugin/core/storage/vector.ts (L255)

The new streaming loop silently continues on dimension mismatches (vec.length !== query.length), whereas the old topKCosine path emitted a structured log.warn('search.dim_mismatch', { expected, got, rowId }) for the same case.

This is a behavioural regression: in production, schema drift (re-embedding with a different model dimension) or data corruption will cause the function to silently return fewer than k results with no observable signal. The old warning was the primary mechanism for operators to detect such drift.

Suggestion: restore the warning log:

if (vec.length === 0 || vec.length !== query.length) {
  if (vec.length !== 0) {
    log.warn("search.dim_mismatch", {
      expected: query.length,
      got: vec.length,
      rowId: String(r["id"]),
    });
  }
  continue;
}

3. apps/memos-local-plugin/core/storage/vector.ts (L260-L272)

The meta-object construction block is duplicated verbatim in both branches of the heap.length < k / score > heap[0]!.score conditional. This violates DRY and means future edits to meta projection must be applied in two places, risking divergence.

The existing pushBounded helper in this file already encapsulates the heap-push logic. Consider either reusing pushBounded (which would require building meta before the call) or extracting meta construction above the branch:

const meta = selectExtra.length > 0
  ? (Object.fromEntries(selectExtra.map((c) => [c, r[c]])) as TMeta)
  : (undefined as TMeta);
if (heap.length < k) {
  heap.push({ id: String(r["id"]), score, meta });
  siftUp(heap, heap.length - 1);
} else if (score > heap[0]!.score) {
  heap[0] = { id: String(r["id"]), score, meta };
  siftDown(heap, 0);
}

Alternatively, the existing pushBounded helper already handles both branches correctly and could be reused directly.


4. apps/memos-local-plugin/core/capture/capture.ts (L783-L784)

persistRows calls listAllForEpisode unconditionally on every invocation, but its callers (runLite, runLightweight, runReflect) have each already called listAllForEpisode earlier in the same execution path for their own dedup step. This results in 2 full-episode table scans per capture cycle — with all columns projected, including the large embedding BLOBs (vec_summary, vec_action) and tool_calls_json — which can be very expensive for large episodes and partially re-introduces the CPU pressure that #2076 aimed to fix.

Consider passing the already-fetched existingTraces result (or a pre-built signature Set) as a parameter to persistRows, so the second scan can be avoided:

async function persistRows(
  rows: TraceRow[],
  input: CaptureInput,
  warnings: CaptureResult["warnings"],
  opts: { skipActionVectorRetry?: boolean } = {},
  existingSignatures?: Set<string>,   // caller may pass its own scan result
): Promise<boolean> {
  const seenSignatures =
    existingSignatures ??
    new Set(deps.tracesRepo.listAllForEpisode(input.episode.id).map(traceIdentitySignature));
  ...
}

Callers that already hold existingTraces can then pass new Set(existingTraces.map(traceIdentitySignature)), eliminating the redundant DB round-trip.


5. apps/memos-local-plugin/core/capture/capture.ts (L140-L142)

The dedup Set built here (seenTs) is derived from existingTraces which fetches all columns including large BLOBs (vec_summary, vec_action) and tool_calls_json. However, only the ts field is actually needed at this point. Consider projecting only the required fields in listAllForEpisode, or creating a lighter sibling (e.g., listTsForEpisode) that only selects ts for the timestamp-based dedup check, to reduce memory pressure for very large episodes.


🧹 Filtered 15 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 15).

Generated by cloud-assistant via Open Code Review.

Address the five findings from open code review on PR MemTensor#2077:

1. `traces.ts::listAllForEpisode` was hydrating all 25 columns including
   `vec_summary`/`vec_action` BLOBs while dedup callers only ever read
   the identity fields. Added `listDedupRowsForEpisode` — a streaming
   sibling that projects only the five dedup columns
   (ts/turn_id/user_text/agent_text/tool_calls_json) and uses
   `.iterate()` so peak RSS scales with scalar payload rather than
   total embedding footprint. `listAllForEpisode` kept as-is for
   `runReflect`, which still needs the full row (id/tags/vecSummary/
   vecAction).

2. `vector.ts::scanAndTopK` streaming loop silently `continue`d on
   dimension mismatches, losing the `search.dim_mismatch` warning the
   old `topKCosine` path emitted. Restored the warn log with
   {expected,got,rowId} — the primary operator signal for detecting
   schema drift (re-embedding with a new model dimension) in
   production. Empty vectors keep the silent-skip semantics as before.

3. `vector.ts::scanAndTopK` had the meta-object construction
   duplicated verbatim in both heap-push branches. Hoisted into a
   single `buildMeta()` closure above the branch so future edits to
   the projection cannot drift between the two paths.

4. `capture.ts::persistRows` unconditionally called
   `listAllForEpisode` even when its caller (`runLite` /
   `runLightweight` / `runReflect`) had just scanned the same
   episode. Added an optional `existingSignatures?: Set<string>` param;
   all three call-sites now pass the pre-built signature set derived
   from their own dedup scan, eliminating the second full-episode
   scan. The set is cloned inside `persistRows` so the intra-batch
   dedup doesn't leak new signatures back to the caller.

5. `capture.ts::runLite` (and `runLightweight`) previously fetched
   full `TraceRow[]` via `listAllForEpisode` and threw everything
   away except the `ts` / `turnId` field. Switched both to
   `listDedupRowsForEpisode` so no BLOBs load for the extraction
   dedup pass.

`traceIdentitySignature` widened to `Pick<TraceRow, "toolCalls" |
"turnId" | "ts" | "agentText" | "userText">` so it can accept either
`TraceRow` (runReflect path) or the narrow `TraceDedupRow` produced
by the streaming helper.

Tests
  - tests/unit/storage/traces-listall.test.ts — 4 new cases pin the
    narrow-projection contract (fields returned, empty episode,
    750-row uncapped stream, strict episode scoping + ts ASC).
  - tests/unit/storage/vector-stream.test.ts — 2 new cases pin the
    dim-mismatch warning fires for non-empty mismatched vectors and
    stays silent for zero-length vectors.

Verification
  - `npx tsc -p tsconfig.json --noEmit` → clean.
  - `npx vitest run tests/unit/storage/traces-listall.test.ts
     tests/unit/storage/vector-stream.test.ts` → 13/13 pass.
  - `npx vitest run tests/unit/capture` → 110/110 pass.
  - Full `npx vitest run tests/unit` → 1134/1137 pass; the two
    failures (storage/migrator, startup-recovery) reproduce on the
    unmodified PR head, so they are pre-existing and unrelated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2077
Task: 91b3e78270801f79
Base: dev-v2.0.23
Head: bugfix/autodev-2076-20260708092153967
Head SHA: 3e25bc3372cfd210c43e2464d2482a4747e943a6

🔍 OpenCodeReview found 5 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.


1. apps/memos-local-plugin/core/storage/vector.ts (L278-L281)

A new buildMeta closure is allocated on every iteration of the streaming loop (up to cap iterations), even though it is only called on the subset of rows that beat the heap threshold. Each closure creation allocates a new function object on the JS heap, adding unnecessary GC pressure in the hot path — which is the very thing this refactor aims to reduce.

Because selectExtra is a loop-invariant constant, the meta-projection logic can be expressed as a plain function that accepts r as a parameter, defined once outside the loop. The two call sites then become straightforward function calls with no per-iteration allocation.

💡 Suggested Change

Before:

    const buildMeta = (): TMeta =>
      selectExtra.length > 0
        ? (Object.fromEntries(selectExtra.map((c) => [c, r[c]])) as TMeta)
        : (undefined as TMeta);

After:

  // Defined once, outside the loop — `selectExtra` is loop-invariant.
  const buildMeta = (row: ScanRow): TMeta =>
    selectExtra.length > 0
      ? (Object.fromEntries(selectExtra.map((c) => [c, row[c]])) as TMeta)
      : (undefined as TMeta);

  // ...then inside the loop:
  //   heap.push({ id: String(r["id"]), score, meta: buildMeta(r) });
  //   heap[0] = { id: String(r["id"]), score, meta: buildMeta(r) };

2. apps/memos-local-plugin/core/storage/repos/traces.ts (L308-L325)

The JSDoc claims this method "streams via .iterate() so peak memory scales with the scalar payload", but the implementation accumulates every row into the rows: TraceDedupRow[] array before returning. Using .iterate() to fill an array gives the same final heap footprint as calling .all() — both load the entire result set into JS memory. For the 518,375-row episodes described in #2076 this still materialises hundreds of thousands of objects at once.

Since all call-sites in capture.ts do .map(...) on the returned array, they can't lazily consume a generator today. But the JSDoc "streaming" claim is factually wrong and will mislead future maintainers. Either:

  1. Change the comment to accurately describe the eager materialisation (the scalar-only projection is the real saving, not streaming), or
  2. Change the return type to IterableIterator<TraceDedupRow> / a generator and update callers to avoid building the intermediate array.
💡 Suggested Change

Before:

    listDedupRowsForEpisode(episodeId: EpisodeId | string): TraceDedupRow[] {
      if (!episodeId) return [];
      const sql = `SELECT ${DEDUP_COLUMNS.join(
        ", ",
      )} FROM traces WHERE episode_id = @episode_id ORDER BY ts ASC`;
      const stmt = db.prepare<{ episode_id: string }, RawDedupRow>(sql);
      const rows: TraceDedupRow[] = [];
      for (const r of stmt.iterate({ episode_id: String(episodeId) })) {
        rows.push({
          ts: r.ts,
          turnId: r.turn_id,
          userText: r.user_text,
          agentText: r.agent_text,
          toolCalls: fromJsonText<ToolCallDTO[]>(r.tool_calls_json, []),
        });
      }
      return rows;
    },

After:

    /**
     * Narrow-projection episode fetch for dedup identity.
     *
     * Projects only the five scalar dedup-identity columns — never
     * `vec_summary` / `vec_action` BLOBs — so per-row size is small
     * even though the full result set is materialised into an array.
     * Uses `.iterate()` to avoid an internal `.all()` intermediate
     * allocation, but callers still receive a fully-materialised
     * `TraceDedupRow[]`.
     *
     * Same episode-scoping / `ts ASC` ordering contract as
     * `listAllForEpisode`; use this whenever you don't need the BLOBs.
     */
    listDedupRowsForEpisode(episodeId: EpisodeId | string): TraceDedupRow[] {
      if (!episodeId) return [];
      const sql = `SELECT ${DEDUP_COLUMNS.join(
        ", ",
      )} FROM traces WHERE episode_id = @episode_id ORDER BY ts ASC`;
      const stmt = db.prepare<{ episode_id: string }, RawDedupRow>(sql);
      const rows: TraceDedupRow[] = [];
      for (const r of stmt.iterate({ episode_id: String(episodeId) })) {
        rows.push({
          ts: r.ts,
          turnId: r.turn_id,
          userText: r.user_text,
          agentText: r.agent_text,
          toolCalls: fromJsonText<ToolCallDTO[]>(r.tool_calls_json, []),
        });
      }
      return rows;
    },

3. apps/memos-local-plugin/core/storage/repos/traces.ts (L261-L283)

The JSDoc for listAllForEpisode says "All hot fields required by dedup are projected, so a caller that only needs ts, turnId, or the identity signature can still iterate at full speed without paying the paginated round trips." This is misleading: the method selects ALL columns including vec_summary and vec_action BLOBs (the root cause of the 4.2 GB OOM in #2076). A developer reading this doc might reach for listAllForEpisode for dedup purposes, paying the full BLOB cost. The comment should steer dedup callers explicitly to listDedupRowsForEpisode.

💡 Suggested Change

Before:

    /**
     * Full episode-scoped trace fetch with NO pagination cap.
     *
     * The paginated `list({ episodeId })` path silently truncates to
     * `PageOptions.limit`, which caps at 500 by default. That cap
     * breaks capture-side dedup (#2076): when an episode grows past
     * the cap, the next runLite / runReflect only sees the newest
     * 500 rows, treats every older step as "novel", and re-inserts
     * the whole tail every cycle. In the reporter's 4.2 GB / 6.8 GB
     * failure, 518,375 trace rows had shrunk to 80,583 distinct
     * `(episode_id, turn_id, user_text, agent_text, tool_calls_json)`
     * signatures — 84 % duplicates driven by exactly this loop.
     *
     * Use this helper for any dedup / reconciliation read that must
     * see the whole episode. All hot fields required by dedup are
     * projected, so a caller that only needs `ts`, `turnId`, or the
     * identity signature can still iterate at full speed without
     * paying the paginated round trips.
     *
     * Rows are ordered by `ts ASC` so the causal chain matches the
     * order runLite / runReflect built.
     */
    listAllForEpisode(episodeId: EpisodeId | string): TraceRow[] {

After:

    /**
     * Full episode-scoped trace fetch with NO pagination cap.
     *
     * Fetches ALL columns including the large `vec_summary` and
     * `vec_action` BLOB columns. Only use this when the caller genuinely
     * needs the full `TraceRow` shape (e.g. `runReflect` which re-embeds
     * and updates every field).
     *
     * **For dedup-only reads** — where only `ts`, `turnId`, `userText`,
     * `agentText`, and `toolCalls` are needed — use
     * {@link listDedupRowsForEpisode} instead to avoid loading
     * multi-GB of BLOB data into JS memory.
     *
     * Rows are ordered by `ts ASC` so the causal chain matches the
     * order runLite / runReflect built.
     */
    listAllForEpisode(episodeId: EpisodeId | string): TraceRow[] {

4. apps/memos-local-plugin/core/storage/repos/traces.ts (L283-L287)

The JSDoc for this method says "All hot fields required by dedup are projected, so a caller that only needs ts, turnId, or the identity signature can still iterate at full speed without paying the paginated round trips." This is misleading: the method selects ${COLUMNS.join(", ")} which includes vec_summary and vec_action — the large BLOB columns explicitly identified as the root cause of the 4.2 GB OOM in #2076. A developer reading this comment might reach for listAllForEpisode for a dedup-only read and silently pay the full BLOB cost.

The JSDoc should clearly state that all columns (including BLOBs) are fetched, and redirect dedup-only callers to listDedupRowsForEpisode.


5. apps/memos-local-plugin/core/storage/repos/traces.ts (L314-L324)

The JSDoc above claims this method "streams via .iterate() so peak memory scales with the scalar payload", but the implementation accumulates every row into the rows: TraceDedupRow[] array before returning. Using .iterate() to fill an array gives the same final heap footprint as .all() — both load the entire result set into JS memory at once. For a 518,375-row episode (the #2076 failure case), this still allocates hundreds of thousands of objects synchronously.

The real saving is the narrow column projection (no vec_summary/vec_action BLOBs), not streaming. The JSDoc should be corrected to avoid misleading future maintainers. If true streaming is desired, change the return type to IterableIterator<TraceDedupRow> and update call-sites to consume lazily.


🧹 Filtered 8 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 7, existing_code_mismatch: 1).

Generated by cloud-assistant via Open Code Review.

…round 2)

Second-pass open-code-review fixes on top of 3e25bc3. All three are
minimal-diff / non-behavior-changing.

1. `vector.ts::scanAndTopK` was building the `buildMeta` closure
   inside the per-row streaming loop. On a hot search over the full
   `hardCap` this allocated one closure per iteration — pure GC
   pressure since only the two heap-push branches consume it.
   Hoisted the projection above the loop as a plain function taking
   the current row as an argument. Two variants pre-selected once
   based on `selectExtra.length` so filtered-out rows still pay
   nothing.

2. `traces.ts::listDedupRowsForEpisode` JSDoc claimed the helper
   "streams via `.iterate()` so peak memory scales with the scalar
   payload". That was misleading: the method uses `stmt.iterate()`
   internally but still accumulates every row into a `TraceDedupRow[]`
   before returning. The genuine saving is the narrow scalar-only
   projection (never touches `vec_summary` / `vec_action`), not
   streaming. Rewrote the JSDoc to describe the real cost model and
   name the `.iterate()` benefit accurately (avoids an intermediate
   `.all()` allocation, does not stream to caller).

3. `traces.ts::listAllForEpisode` JSDoc said "all hot fields required
   by dedup are projected, so a caller that only needs `ts`, `turnId`,
   or the identity signature can still iterate at full speed". That
   accidentally invited dedup callers back onto this method, which
   loads every BLOB column. Rewrote the JSDoc to state explicitly
   that vec_summary/vec_action BLOBs are loaded and redirect
   dedup-only callers to `listDedupRowsForEpisode`. Preserved the
   MemTensor#2076 context on why the uncapped variant exists.

No behavioral change. No test changes needed.

Verification
  - `npx tsc -p tsconfig.json --noEmit` → clean.
  - `npx vitest run tests/unit/storage/traces-listall.test.ts \
     tests/unit/storage/vector-stream.test.ts` → 13/13 pass.
  - `npx vitest run tests/unit/capture` → 110/110 pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

🤖 Open Code Review

Target: PR #2077
Task: 91b3e78270801f79
Base: dev-v2.0.23
Head: bugfix/autodev-2076-20260708092153967
Head SHA: 98f779cc482670b347237728d3773f895ab1a380

🔍 OpenCodeReview found 2 issue(s) in this PR.


1. apps/memos-local-plugin/core/storage/vector.ts (L287-L293)

The streaming loop inlines the min-heap push logic (siftUp / siftDown) directly instead of calling the existing pushBounded helper defined at line 141, which already encapsulates exactly this pattern. This duplication means a future change to heap logic (e.g. tie-breaking policy, score comparison operator) must be updated in two places, creating a silent divergence risk.

Consider reusing pushBounded:

pushBounded(heap, { id: String(r["id"]), score, meta: buildMeta(r) }, k);
💡 Suggested Change

Before:

    if (heap.length < k) {
      heap.push({ id: String(r["id"]), score, meta: buildMeta(r) });
      siftUp(heap, heap.length - 1);
    } else if (score > heap[0]!.score) {
      heap[0] = { id: String(r["id"]), score, meta: buildMeta(r) };
      siftDown(heap, 0);
    }

After:

    pushBounded(heap, { id: String(r["id"]), score, meta: buildMeta(r) }, k);

2. apps/memos-local-plugin/core/capture/capture.ts (L136-L137)

The comment describes listDedupRowsForEpisode as 'streaming', but the implementation in traces.ts materialises all rows into a TraceDedupRow[] array before returning (it uses stmt.iterate() internally only to avoid better-sqlite3's intermediate .all() allocation, not to return a lazy iterator). The real benefit is the narrow column projection (no BLOB columns), not streaming. Calling this 'streaming' is misleading to future readers and could create false confidence about peak memory usage for very large episodes. Consider replacing 'streaming' with 'uncapped narrow-projection' or simply 'fully scanned' to match the actual behaviour documented in the traces.ts JSDoc.

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

⚠️ Open Code Review automatic OCR fix limit reached

Open Code Review still found 2 issue(s), but the automatic OCR fix loop has reached 2/2 attempts.

I stopped auto-fixing this PR to avoid an infinite loop. Please review the latest OCR comment and fix or dismiss the remaining findings manually.

PR: #2077

@Memtensor-AI Memtensor-AI added area:plugin Plugin, adapter, bridge, or apps layer | 插件、适配器、桥接层或 apps 目录 area:memory Memory storage, retrieval, evolution, and mem cube | 记忆存储、检索、演化与 mem cube labels Jul 8, 2026
@Memtensor-AI Memtensor-AI added the status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 label Jul 8, 2026
@Memtensor-AI

Copy link
Copy Markdown
Collaborator Author

✅ Automated Test Results: PASSED

All tests passed (25/25 executed). memos_local_plugin/unit: 25/25. Duration: 5s [advisory, non-gating] AI-generated tests on branch test/auto-gen-91b3e78270801f79-20260708230538: 0/58 passed, 58 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: bugfix/autodev-2076-20260708092153967

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai:generated Generated or modified by AI | 由 AI 生成或修改 area:memory Memory storage, retrieval, evolution, and mem cube | 记忆存储、检索、演化与 mem cube area:plugin Plugin, adapter, bridge, or apps layer | 插件、适配器、桥接层或 apps 目录 bug status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants