Skip to content

fix(status): paginate metadata fetch to support large palaces#851

Merged
bensig merged 3 commits intoMemPalace:developfrom
vnguyen-lexipol:fix/status-paginate-large-palaces
Apr 22, 2026
Merged

fix(status): paginate metadata fetch to support large palaces#851
bensig merged 3 commits intoMemPalace:developfrom
vnguyen-lexipol:fix/status-paginate-large-palaces

Conversation

@vnguyen-lexipol
Copy link
Copy Markdown
Contributor

Summary

  • Paginates col.get() in miner.py:status() using 5k batches with offset instead of a single col.get(limit=total) call
  • Uses col.count() for the header total instead of len(metas)
  • Aggregates wing/room counts incrementally per batch (no full metadata list in memory)

Problem

On palaces with >10k drawers:

Fix

total = col.count()
batch_size = 5000
offset = 0
while offset < total:
    r = col.get(limit=batch_size, offset=offset, include=["metadatas"])
    batch = r["metadatas"]
    if not batch:
        break
    for m in batch:
        wing_rooms[m.get("wing", "?")][m.get("room", "?")] += 1
    offset += len(batch)

Testing

Tested on a 122,686-drawer palace — correctly displays full count and complete wing/room breakdown with no SQLite errors.

Fixes #850
Related: #802, #723

🤖 Generated with Claude Code

`col.get(limit=total)` causes SQLite "too many SQL variables"
on palaces with >10k drawers (MemPalace#802) and on older versions the
hardcoded limit=10000 silently truncated the count (MemPalace#850).

Paginate in 5k batches using offset and aggregate wing/room
counts incrementally. Also use `col.count()` for the header
instead of `len(metas)` so the displayed total is always correct.

Tested on a 122,686-drawer palace.

Fixes MemPalace#850
Related: MemPalace#802, MemPalace#723
@vnguyen-lexipol
Copy link
Copy Markdown
Contributor Author

CI note (run 24379493758)

test-linux failed during pip install -e ".[dev]", not during pytest. Logs show a transient PyPI/TLS error (ssl.SSLError: [SSL: DECRYPTION_FAILED_OR_BAD_RECORD_MAC]) while downloading wheels, so tests did not run for that attempt.

Action: re-run failed jobs or the full workflow. If the same install step fails repeatedly, it is still an install/network flake unless pytest starts and reports failures.

Same pytest + --cov-fail-under=80 command passes locally on Python 3.9 / 3.11 / 3.13 for this branch.

@bensig
Copy link
Copy Markdown
Collaborator

bensig commented Apr 19, 2026

Code review

Found 1 issue:

  1. The pagination loop drops the m = m or {} None-guard that exists on develop (added in commit feba7e8 to handle ChromaDB returning None metadata entries). Without it, m.get("wing", "?") will raise AttributeError: 'NoneType' object has no attribute 'get' on any palace with null-metadata drawers -- a regression that crashes status() on the same large palaces this PR was tested against.

break
for m in batch:
wing_rooms[m.get("wing", "?")][m.get("room", "?")] += 1
offset += len(batch)

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@bensig
Copy link
Copy Markdown
Collaborator

bensig commented Apr 19, 2026

@vnguyen-lexipol two small items before this can merge: (1) the pagination loop is missing the m = m or {} None-guard from develop (see review comment above), and (2) the branch has merge conflicts against develop and needs a rebase.

@bensig bensig self-requested a review April 19, 2026 06:32
Upstream develop commit feba7e8 (2026-04-18) added `m = m or {}` to the
single-shot `for m in metas:` loop after this branch already rewrote
status() to paginate. Without porting the guard forward, merging this PR
would silently drop jp's fix and crash again on palaces with null-metadata
drawers.

Addresses bensig's review on MemPalace#851.
@vnguyen-lexipol
Copy link
Copy Markdown
Contributor Author

Good catch, thanks. You're right — feba7e8 landed on develop 5 days after this branch was written, so the m = m or {} guard wasn't in the code when I rewrote the loop. Pushed 3004ac4 which ports the guard into the paginated loop so the fix doesn't get silently dropped when this merges.

Didn't pull in a full rebase onto develop — this branch's base is a root commit in my worktree so the rebase produced 77 add/add conflicts across unrelated files. Happy to re-do as a clean branch off develop if you'd prefer that over the surgical port.

@vnguyen-lexipol
Copy link
Copy Markdown
Contributor Author

Also — on item (2), the branch does show CONFLICTING against develop. Happy to resolve conflicts and push, but wanted to check: do you prefer a clean conflict-free branch, or are you fine squashing and resolving at merge time? Either works on my end.

…ate-large-palaces

# Conflicts:
#	mempalace/miner.py
@vnguyen-lexipol
Copy link
Copy Markdown
Contributor Author

Went ahead and resolved the conflict — just one hunk in status(), kept the paginated version since that's what this PR delivers. Merge commit 5d2da04 now on the branch; mergeable: MERGEABLE. Ready whenever you are.

jphein added a commit to jphein/mempalace that referenced this pull request Apr 19, 2026
… open PRs, not 8)

MemPalace#851 (vnguyen-lexipol) was approved by bensig on 2026-04-15 and
already fixes the miner.status() SQLITE_MAX_VARIABLE_NUMBER crash
plus MemPalace#850 silent-truncation. I filed MemPalace#1036 this morning missing
that context despite having triaged MemPalace#851 the prior day. Closed
MemPalace#1036 in favor of MemPalace#851, updated lead paragraph count + Still-ahead
row + Open-PRs table accordingly.
@jphein
Copy link
Copy Markdown
Collaborator

jphein commented Apr 19, 2026

Field note from the fork side: a semantically equivalent paginated col.get() has been running on jphein/mempalace main since 2026-04-10 against a 152,794-drawer palace — 9 days, no crashes. Data point above the 122,686-drawer test. Your version is the cleaner fix — switching to col.count() for the header resolves #850 at the same time, and per-batch aggregation stays lower-memory.

@jphein
Copy link
Copy Markdown
Collaborator

jphein commented Apr 21, 2026

Bumping this with production data from the last ~20 hours on v3.3.2:

All three hit the same miner.py:852col.get(limit=total)SQLITE_MAX_VARIABLE_NUMBER, exactly what this PR's +15/-9 addresses. Running the semantically-equivalent paginated version on the fork's 165K-drawer palace since 2026-04-10 — zero crashes, no measurable perf regression.

Happy to help land this however useful — rebase, additional reviewer ping, splitting scope, anything.

@bensig bensig merged commit 810f9a5 into MemPalace:develop Apr 22, 2026
jphein added a commit to jphein/mempalace that referenced this pull request Apr 22, 2026
… (2026-04-22)

Ben's batched queue-clear pass merged four PRs at 00:38 UTC:
graph cache (MemPalace#661), deterministic hook saves (MemPalace#673), Claude Code
2.1.114 hook stdout + silent_save guard (MemPalace#1021), and upstream's
own MemPalace#851 pagination fix (closing MemPalace#1036 as superseded).

Moved four rows out of the "Fork Changes" / "Fork change queue"
tables into their respective merged-upstream history sections.
Intro sentence PR count reduced from 7 → 4 open.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein pushed a commit to jphein/mempalace that referenced this pull request Apr 24, 2026
Restore-integrity release. Unbreaks fresh `pip install mempalace` from
v3.3.2 by re-tagging current develop, which carries both the plugin.json
consumer (shipped in 3.3.2) and the matching mempalace-mcp entry point
in pyproject.toml (added on develop ~10h after the 3.3.2 tag via MemPalace#340
by @messelink). MemPalace#1093 diagnosed by @jphein.

Bumps (all 5 sources agree per Version Guard / CLAUDE.md):
- mempalace/version.py              3.3.2 → 3.3.3
- pyproject.toml                     3.3.2 → 3.3.3
- .claude-plugin/plugin.json         3.3.2 → 3.3.3
- .claude-plugin/marketplace.json    3.3.2 → 3.3.3
- .codex-plugin/plugin.json          3.3.2 → 3.3.3
- CHANGELOG.md                        new [3.3.3] entry

No code changes. The fix for MemPalace#1093 is already on develop via merged PRs
MemPalace#340, MemPalace#1021, MemPalace#851, MemPalace#942, MemPalace#833, MemPalace#673, MemPalace#661, MemPalace#659, MemPalace#1097, MemPalace#1051, MemPalace#1001,
MemPalace#945.

Branch name intentionally outside the `release/*` ruleset so follow-up
CI-fix commits aren't gated behind a nested PR. (Supersedes MemPalace#1143 —
closed for exactly that reason after it missed 3 of 5 version files.)

Smoke-tested locally from a fresh develop clone:
  grep mempalace-mcp pyproject.toml .claude-plugin/plugin.json   # both ✓
  python -m build --wheel                                        # ✓
  pip install …-py3-none-any.whl                                 # ✓
  which mempalace-mcp                                            # ✓
  mempalace-mcp --help                                           # ✓
jphein added a commit to jphein/mempalace that referenced this pull request Apr 24, 2026
…safe graph cache

31 commits including:
- v3.3.3 release (restores pip install integrity post-v3.3.2 MemPalace#1093 regression)
- MemPalace#1097 empty-string filter normalization in mempalace_search
- MemPalace#659 diary wing parameter (our fork PR, now upstream)
- MemPalace#851/MemPalace#1097/MemPalace#1021/MemPalace#661/MemPalace#673 incorporated
- website Crystal Lattice brand refresh
- thread-safe graph cache in palace_graph.py

Conflict resolutions (10 files):
- README.md            keep fork version; bump badge 3.3.1 → 3.3.3 for test compat
- hooks/README.md      keep fork silent/block architecture docs; keep MEMPAL_PYTHON
                       (correct for legacy hook, upstream's rename is stale)
- examples/HOOKS_TUTORIAL.md  same treatment
- mcp_server.py        take upstream's sanitize_name(wing) — strictly better than
                       our crude lowercase+underscore normalization
- miner.py             keep fork 10K batch size + comma formatting on status;
                       adopt upstream's pagination rationale comment
- palace_graph.py      take upstream entirely — thread-safety improvements layered
                       on top of our MemPalace#661 (which upstream already merged)
- hooks_cli.py         take upstream (Windows path-separator compat in
                       _wing_from_transcript_path, Codex CLI format in
                       _extract_recent_messages), then re-apply fork-ahead: use
                       _wing_from_transcript_path in _ingest_transcript instead
                       of hardcoded "sessions" — keeps transcript mining coherent
                       with the diary wing derivation from MemPalace#659
- tests/test_hooks_cli.py  take upstream's updated wing-kwarg assertions and
                       new test_stop_hook_derives_wing_from_transcript_path;
                       take upstream's mock-based security test (simpler than
                       our three-way assertion, same property tested)

Post-merge test state:
- 1096 passed, 10 failed in tests/test_claude_plugin_hook_wrappers.py
- The 10 failures are the fork-ahead MemPalace#19 divergence already documented in
  CLAUDE.md: our venv-aware hooks use `dirname`/`cat` which the test's
  scrubbed-PATH environment doesn't provide. Same class that correctly caught
  MemPalace#1115 and led us to withdraw it pending MemPalace#1069 arbitration. Expected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jphein added a commit to jphein/mempalace that referenced this pull request Apr 24, 2026
… state

Three stale sections updated:

- Fork change queue: row 8 (.blob_seq_ids_migrated marker) struck
  through → FILED as MemPalace#1177. Two new rows added for segfault fixes
  discovered today (MemPalace#1171 concurrent-write lock, MemPalace#1173 quarantine in
  make_client) that weren't in the queue because the bugs surfaced
  today, not during the original 2026-04-21 triage.

- Open upstream PRs: was showing 3 of 10 PRs. Now shows all 10 with
  current CI/review state. All rebased onto current upstream/develop
  and MERGEABLE as of today.

- Merged since v3.3.1: added v3.3.3 release (2026-04-24) with its
  constituent merges — MemPalace#942, MemPalace#833, MemPalace#1097, MemPalace#1145, MemPalace#1147, MemPalace#1148/1150/1157
  entity-detection overhaul (via @igorls's MemPalace#1175 stacked-PR rescue),
  MemPalace#1166 palace-path security, MemPalace#340/MemPalace#1093 install regression, plus MemPalace#851
  from the 2026-04-22 batch.
sha2fiddy added a commit to sha2fiddy/mempalace that referenced this pull request Apr 26, 2026
Mirror the pagination pattern PR MemPalace#851 landed in miner.py:status().
A single drawers_col.get(limit=total, ...) on palaces larger than
SQLite's SQLITE_MAX_VARIABLE_NUMBER (32766) crashes inside chromadb.

Fetch drawers in batch_size=5000 chunks, stepping offset until the
collection is drained. by_source aggregation semantics are preserved
exactly — grouping, wing filter, meta capture all unchanged.

Closes MemPalace#1073. Related: MemPalace#802, MemPalace#850, MemPalace#1016.
sha2fiddy added a commit to sha2fiddy/mempalace that referenced this pull request Apr 29, 2026
Mirror the pagination pattern PR MemPalace#851 landed in miner.py:status().
A single drawers_col.get(limit=total, ...) on palaces larger than
SQLite's SQLITE_MAX_VARIABLE_NUMBER (32766) crashes inside chromadb.

Fetch drawers in batch_size=5000 chunks, stepping offset until the
collection is drained. by_source aggregation semantics are preserved
exactly — grouping, wing filter, meta capture all unchanged.

Closes MemPalace#1073. Related: MemPalace#802, MemPalace#850, MemPalace#1016.
sha2fiddy added a commit to sha2fiddy/mempalace that referenced this pull request Apr 29, 2026
Mirror the pagination pattern PR MemPalace#851 landed in miner.py:status().
A single drawers_col.get(limit=total, ...) on palaces larger than
SQLite's SQLITE_MAX_VARIABLE_NUMBER (32766) crashes inside chromadb.

Fetch drawers in batch_size=5000 chunks, stepping offset until the
collection is drained. by_source aggregation semantics are preserved
exactly — grouping, wing filter, meta capture all unchanged.

Closes MemPalace#1073. Related: MemPalace#802, MemPalace#850, MemPalace#1016.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli CLI commands bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mempalace status silently truncates at 10,000 drawers (incorrect count displayed)

4 participants