fix(plugin): stop clamping storage list queries to 500 rows

[akushonkamen]

Fixes #2401

---

Summary

Two defects in apps/memos-local-plugin/core/storage/repos/_helpers.ts made every
list query in the local plugin's SQLite storage layer silently truncate at 500 rows:

1. clampLimit hard-capped explicit limits at 500 (old _helpers.ts:58).
   Callers that deliberately asked for a wide window — the established
   "uncapped fetch" idiom limit: 100_000 / 5_000 / 2_000 — still got back the
   newest 500 rows.
2. buildPageClauses treated a missing limit as a 500-row page (old
   _helpers.ts:51, opts?.limit ?? 500). Unpaginated scans never looked past
   the newest 500 rows.

New semantics

• Omitting limit means all matching rows: buildPageClauses appends no
  LIMIT/OFFSET at all. This is the exact contract issue local-plugin v2.0.8: gateway CPU 100% — synchronous full-table vector scan (scanAndTopK) + unbounded trace re-insertion from dedup pagination cap #2076 already pinned
  for traces.list({ episodeId }) (the shouldUseUncappedEpisodeScan
  special case in core/storage/repos/traces.ts); this PR generalizes it to
  every repo instead of special-casing one filter shape. (SQLite has no
  OFFSET without LIMIT, so an offset without a limit is simply not a page
  request.)
• An explicit limit is honored, clamped only by a 100_000-row safety
  ceiling (the shape fix: remove 500-row cap that truncated viewer count displays #1954 intended). The invalid-input fallback
  (NaN / <= 0) stays at 500.

Root cause

// before — _helpers.ts
export function buildPageClauses(opts: PageOptions | undefined, tsColumn: string): string {
  const newestFirst = opts?.newestFirst !== false;
  const limit = clampLimit(opts?.limit ?? 500);   // missing limit => 500-row page
  ...
}

export function clampLimit(n: number): number {
  if (!Number.isFinite(n) || n <= 0) return 500;
  return Math.min(Math.trunc(n), 500);            // explicit limit clamped to 500
}

#1954 (merged to dev-v2.0.25) only raised the ceiling on a release branch and
never landed on main; the ?? 500 default was left untouched. This PR
includes and supersedes #1954: it fixes both the ceiling and the default.

Affected call sites (all fixed by the two helper changes)

core/pipeline/memory-core.ts

Function	Site	Call shape	Defect
countPolicies	:3303 (no-q :3312, with-q :3320)	list({status, limit:100_000}) / list({status})	A + B
countWorldModels	:3396 (list :3399)	worldModel.list({limit:100_000})	A
countEpisodes	:3575 (list :3569, :3582)	episodes.list({sessionId, limit:100_000})	A
countSkills	:4043 (list :4050)	skills.list({status, limit:100_000})	A
countTraces q path	:3838	traces.list({...}) — "Walk all matching traces (no limit)"	B
listTraces search+group scan	:3909–3914	traces.list({...}) — "scan, filter, then paginate by distinct turn key"	B
exportBundle	:4338 (lists :4347–4350)	limit:100_000 / 5_000 / 2_000	A — exports dropped data

Outside memory-core.ts (defect B — no-limit scans)

File:line	Call	Consequence
core/memory/l3/l3.ts:107	policies.list({status:"active"})	L3 clustering saw only the newest 500 active policies
core/memory/l2/l2.ts:466	policies.list({status:"candidate"})	candidates beyond the newest 500 were permanently skipped
core/memory/l2/l2.ts:602	policies.list({limit:5_000})	widened window still 500 → duplicate induction
core/retrieval/decision-guidance.ts:113	policies.list({status:"active"})	guidance ranked over newest 500 only
core/pipeline/retrieval-repos.ts:136–139	repos.policies.list({...})	retrieval candidates ranked over newest 500 only

Untouched on purpose (below the old cap, per "one logical change per PR"):
core/pipeline/orchestrator.ts:242 (limit 20), core/skill/skill.ts:347 and
core/feedback/feedback.ts:258 (limit 200), and the separate local clampLimit
in core/storage/repos/hub.ts:628. The fixed apiLogs.list window reported in
#2380 is a different root cause and is not addressed here.

Behavior-change notes

The no-limit default change is deliberate: the no-limit call sites above
(countPolicies with-q, countTraces q path, the listTraces search+group
scan, and the four scans outside memory-core.ts) are all "give me everything"
intents on low-frequency background paths, and
their memory use now scales with table size instead of silently under-reading.
Viewer list endpoints (listTraces/listPolicies/...) are unaffected — they
always pass explicit limit/offset pages. episodes.listClosedPage keeps its
explicit ?? 500 page size (a real page request, unchanged).

Tests

New regression coverage (fails on the pre-fix code, passes now):

• tests/unit/storage/page-limit.test.ts (new, 10 tests)
  • clampLimit(100_000) > 500, mid-size limits preserved, small limits untouched, invalid input still falls back positive;
  • buildPageClauses without limit emits no LIMIT; explicit limits kept, large limits not clamped;
  • repo layer with 600 seeded rows: policies.list({status}) (L3/L2 call shape) and policies.list({limit:5_000}) (L2 dedup shape) return everything; policies.count exact; traces.list({sessionId}) (q-count call shape) returns everything.
• tests/unit/pipeline/memory-core.test.ts (+3 tests, "> 500 rows (2.0.20 still truncates internal queries at 500 rows: countPolicies/countEpisodes/countSkills/countWorldModels report 500, and no-limit call sites lose rows (#1954 only changed the ceiling) #2401)")
  • all four viewer counters (countPolicies/countEpisodes/countSkills/countWorldModels) return 600, not 500;
  • countTraces({q}) substring path counts hits outside the newest-500 window (12, was 2);
  • exportBundle returns all 600 rows per table, not 500.
• tests/unit/storage/traces-count.test.ts — the two assertions that pinned
  the old cap ("list with no limit still caps at 500") now assert the new
  contract (600 rows; small pages still page), keeping the Fix #1593: Web UI memory count stuck at 500, actual traces exceed 1400 #1877 count()
  accuracy assertions.

Red→green evidence (pre-fix run on main @ a7367d0):

× clampLimit … → expected 500 to be greater than 500
× buildPageClauses … → expected 'ORDER BY updated_at DESC LIMIT 500 …' not to match /LIMIT/
× policies.list … → to have a length of 600 but got 500
× traces.list … → to have a length of 600 but got 500
× viewer counters … → expected 500 to be 600
× countTraces q path … → expected 2 to be 12
× exportBundle … → to have a length of 600 but got 500

Validation (all offline, local SQLite):

• npx vitest run tests/unit/storage/ — 95/95
• npx vitest run tests/unit/pipeline/memory-core.test.ts — 51/51
• npx vitest run tests/unit/bridge/methods.test.ts tests/unit/server/http.test.ts — 86/86
• npm run test:unit — 182 files, 1602 passed, 2 skipped (zero regressions)
• npm run lint (tsc -p tsconfig.json --noEmit) — clean

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2464
Task: 275f498229250ce3
Base: main
Head: fix/plugin-storage-limit-clamp

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

⚠️ 1 warning(s) occurred during review.

---

1. apps/memos-local-plugin/core/storage/types.ts (L77-L84)

The JSDoc says "An invalid value (non-finite, 0, negative) is treated exactly like an omitted one: no LIMIT clause, never a substituted page." — This is accurate for buildPageClauses, but it contradicts clampLimit's own behavior: clampLimit is also called directly by external callers (e.g. episodes.listClosedPage) and it returns MAX_QUERY_LIMIT (100,000) for invalid input, NOT "no clause". A reader who calls clampLimit(0) directly expecting "no limit" behavior based on this type-level comment will be surprised. Consider narrowing the statement to clarify it only applies when limit is passed via PageOptions to buildPageClauses, not when clampLimit is called standalone.

💡 Suggested Change

Before:

  /**
   * Omit for all matching rows — the repo adds no LIMIT clause (#2401),
   * mirroring the #2076 episode-scan contract. An invalid value (non-finite,
   * 0, negative) is treated exactly like an omitted one: no LIMIT clause,
   * never a substituted page. Explicit positive finite values are honored
   * up to `MAX_QUERY_LIMIT` (see `clampLimit`).
   */
  limit?: number;

After:

  /**
   * Omit for all matching rows — the repo adds no LIMIT clause (#2401),
   * mirroring the #2076 episode-scan contract. An invalid value (non-finite,
   * 0, negative) is treated exactly like an omitted one when passed through
   * `buildPageClauses`: no LIMIT clause, never a substituted page.
   * Explicit positive finite values are honored up to `MAX_QUERY_LIMIT`
   * (see `clampLimit`). Note: calling `clampLimit` directly with an invalid
   * value returns `MAX_QUERY_LIMIT` as a defensive ceiling, not "no limit".
   */
  limit?: number;

---

2. apps/memos-local-plugin/core/storage/repos/_helpers.ts (L73-L75)

Unsafe property access on opts after type-narrowing through opts?.limit. The hasUsableLimit guard uses opts?.limit (optional chaining), which confirms opts.limit is a usable number — but opts itself could still be undefined at this point. TypeScript may not narrow opts from PageOptions | undefined to PageOptions based on the check on opts?.limit.

For example, opts being undefined and opts?.limit being undefined are two different cases — both pass the early return, so after if (!hasUsableLimit(opts?.limit)) return order;, opts is still typed as PageOptions | undefined. Accessing opts.offset directly will throw a runtime TypeError if opts is undefined.

Use optional chaining consistently:

const offset = Math.max(opts?.offset ?? 0, 0);

or restructure the guard to narrow opts itself.

💡 Suggested Change

Before:

  if (!hasUsableLimit(opts?.limit)) return order;
  const offset = Math.max(opts.offset ?? 0, 0);
  return `${order} LIMIT ${clampLimit(opts.limit)} OFFSET ${offset}`;

After:

  if (!hasUsableLimit(opts?.limit)) return order;
  const offset = Math.max(opts?.offset ?? 0, 0);
  return `${order} LIMIT ${clampLimit(opts.limit)} OFFSET ${offset}`;

---

3. apps/memos-local-plugin/core/storage/repos/_helpers.ts (L75)

The OFFSET clause is always emitted even when offset is 0. LIMIT X OFFSET 0 is functionally equivalent to LIMIT X, but the unnecessary OFFSET 0 adds noise to SQL queries. Consider omitting it when the offset is 0:

const offsetClause = offset > 0 ? ` OFFSET ${offset}` : "";
return `${order} LIMIT ${clampLimit(opts.limit)}${offsetClause}`;

---

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

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Environment preparation failed before any gating tests executed. Failed scopes: memos_local_plugin
Branch: fix/plugin-storage-limit-clamp

[akushonkamen]

Thanks @Memtensor-AI for the careful review — all three points are fair. Updates in cd70df6:

Addressed in this commit

• feat: add internet retrieval and CoT functionality to MemOS #1 (invalid limits vs. JSDoc): buildPageClauses now treats an invalid limit (non-finite, 0, negative) exactly like an omitted one — no LIMIT clause, never a substituted page. The PageOptions JSDoc in types.ts was updated to match the implementation.
• Bug: json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0) #3 (magic number): the 100,000 ceiling is now the exported MAX_QUERY_LIMIT constant, and clampLimit's defensive fallback for invalid input returns that ceiling instead of a 500-row page, so the behavior is consistent end-to-end. Tests were updated red→green to pin both behaviors.

On #2 (unbounded scan as a breaking change) — this one is intentional, for the maintainer's call:

• The existing callers that omit limit are precisely the ones whose semantics are "I want all matching rows": viewer counters, retrieval/dedup sweeps, L3 clustering, export paths. Under the old code they were silently truncated to 500 rows — that silent truncation is the bug reported in 2.0.20 still truncates internal queries at 500 rows: countPolicies/countEpisodes/countSkills/countWorldModels report 500, and no-limit call sites lose rows (#1954 only changed the ceiling) #2401.
• The new behavior is not a raw unbounded scan: every explicit limit is still capped at MAX_QUERY_LIMIT (100,000), consistent with the direction of fix: remove 500-row cap that truncated viewer count displays #1954.
• That said, if you'd prefer the explicit-sentinel approach (e.g. a NO_LIMIT constant with callers migrated one by one, keeping "omitted = default page"), I'm happy to refactor that way — whichever the maintainers prefer.

CI note: the autotest run reports ENV ISSUE ("Environment preparation failed ... failed scopes: memos_local_plugin") — the environment failed before any tests executed, so this is not a code failure. Could a maintainer / CI admin take a look or trigger a rerun? Happy to help debug the scope setup if needed.

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Environment preparation failed before any gating tests executed. Failed scopes: memos_local_plugin
Branch: fix/plugin-storage-limit-clamp

[akushonkamen]

Follow-up on the heads-up above: the re-run at 16:43 UTC on this PR's head (cd70df6) hit the same ENV ISSUE — memos_local_plugin environment preparation failed before any gating tests executed — and #2466's re-run at 18:06 UTC failed identically. This is now affecting all four open PRs (#2464, #2465, #2466, #2468) over a ~5-hour window, so it looks like a shared infra problem rather than anything transient or PR-specific. Tests never execute, so the red status does not reflect this branch's code.

Opened #2476 to track with the full timeline. Could a CI admin check the memos_local_plugin scope environment, or manually trigger a rerun once it's healthy? Happy to help debug the scope setup.