fix: add limit=10000 safety cap to all unbounded ChromaDB .get() calls by igorls · Pull Request #137 · MemPalace/mempalace

igorls · 2026-04-07T20:23:58Z

Problem

Several tool handlers call col.get(include=["metadatas"]) without a limit. As the palace grows, these calls load all metadata into memory at once. With sufficiently large palaces, this causes:

High memory usage / OOM crashes
Slow response times for status/taxonomy/list operations

Finding:
CRITICAL — unbounded data fetching causes OOM

Fix

Added limit=10000 to all unbounded col.get() calls:

Location	File	Change
`tool_status`	mcp_server.py:71	`col.get(include=["metadatas"], limit=10000)`
`tool_list_wings`	mcp_server.py:128	same
`tool_list_rooms`	mcp_server.py:143	same (kwargs dict)
`tool_get_taxonomy`	mcp_server.py:161	same
`tool_diary_read`	mcp_server.py:~406	same
`Layer1.generate`	layers.py:100	`{"include": ["documents", "metadatas"], "limit": 10000}`

Layer2 already had a limit param — no change needed.

10,000 is a conservative cap that covers typical palace sizes while preventing catastrophic memory usage. For palaces exceeding this, the counts will be approximate (reporting on the first 10K drawers).

Packaging changes (carried over from base)

This branch also includes the setuptools → hatchling build backend migration and [dependency-groups] adoption from the test infrastructure work. These are inherited from the shared base and not new to this PR.

92 tests pass (includes test infrastructure from PR #131).

Copilot

Pull request overview

This PR addresses OOM/memory-pressure risks caused by unbounded ChromaDB collection.get() calls by introducing a conservative safety cap (limit=10000) in read-heavy endpoints, and updates surrounding project/testing infrastructure included in the diff.

Changes:

Add limit=10000 to several metadata/document retrieval paths in MCP tools and Layer1 generation to prevent unbounded in-memory loads.
Add/expand pytest coverage for MCP server handlers, the KnowledgeGraph, Dialect, and the search_memories API (via shared fixtures).
Update packaging/tooling configuration (pyproject build backend + dev dependency declarations) and remove requirements.txt.

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`mempalace/mcp_server.py`	Caps metadata fetches used for status/list/taxonomy handlers to avoid unbounded loads.
`mempalace/layers.py`	Caps Layer1 drawer retrieval to prevent loading the entire collection into memory.
`tests/conftest.py`	Adds shared fixtures and HOME isolation to keep tests hermetic.
`tests/test_mcp_server.py`	Adds direct tool-handler + dispatch-layer coverage for MCP server behaviors.
`tests/test_searcher.py`	Adds coverage for the programmatic `search_memories` API.
`tests/test_knowledge_graph.py`	Adds CRUD + temporal query/timeline/stats coverage for KnowledgeGraph.
`tests/test_dialect.py`	Adds coverage for Dialect compression, detection, encoding/decoding, and stats.
`pyproject.toml`	Updates build-system/tooling configuration and dev dependency declarations.
`requirements.txt`	Removed in favor of pyproject/lock-based dependency management.
`mempalace/__init__.py`	Updates `__version__` to `3.0.0`.

Comments suppressed due to low confidence (1)

mempalace/mcp_server.py:75

With the new limit=10000, total_drawers can exceed the summed counts in wings/rooms (since those are computed from only the first 10k metadatas). To avoid silently returning internally inconsistent stats, consider adding an explicit truncation indicator (e.g., sampled_drawers, sample_limit, truncated: count > sampled_drawers) so clients/agents know the breakdown is approximate when the cap is hit.

    count = col.count()
    wings = {}
    rooms = {}
    try:
        all_meta = col.get(include=["metadatas"], limit=10000)["metadatas"]
        for m in all_meta:
            w = m.get("wing", "unknown")
            r = m.get("room", "unknown")
            wings[w] = wings.get(w, 0) + 1

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T20:27:22Z

 def tool_get_taxonomy():
    col = _get_collection()
    if not col:
        return _no_palace()
    taxonomy = {}
    try:
-        all_meta = col.get(include=["metadatas"])["metadatas"]
+        all_meta = col.get(include=["metadatas"], limit=10000)["metadatas"]
        for m in all_meta:


This PR aims to cap unbounded col.get() calls, but tool_diary_read in this same module still calls col.get(..., include=["documents","metadatas"]) without any limit, then sorts/slices in Python. That path can still load an unbounded number of documents/metadatas into memory and trigger the same OOM risk. Consider adding a safety cap there as well (e.g., limit=min(max(last_n, 1000), 10000) plus a truncation flag/message when capped).

Copilot · 2026-04-07T20:27:22Z

            return "## L1 — No palace found. Run: mempalace mine <dir>"

        # Fetch all drawers (with optional wing filter)
-        kwargs = {"include": ["documents", "metadatas"]}
+        kwargs = {"include": ["documents", "metadatas"], "limit": 10000}
        if self.wing:
            kwargs["where"] = {"wing": self.wing}



The comment says "Fetch all drawers" but the query is now capped (limit=10000). Updating the comment to reflect the safety cap (and that L1 summaries may be based on a sample when the palace is larger) would prevent documentation drift and confusion during future maintenance.

Copilot · 2026-04-07T20:27:23Z

 [project.optional-dependencies]
-dev = ["pytest>=7.0", "build>=1.0", "twine>=4.0"]
+dev = ["pytest>=7.0", "ruff>=0.4.0"]
+
+[dependency-groups]
+dev = ["pytest>=7.0", "ruff>=0.4.0"]
+
+[build-system]
+requires = ["hatchling"]
+build-backend = "hatchling.build"
+
+[tool.hatch.build.targets.wheel]
+packages = ["mempalace"]


The PR title/description focuses on adding a limit=10000 safety cap, but this diff also changes packaging/tooling (switches build backend to Hatchling, adds dependency-groups, and removes setuptools config). If these packaging changes are intended, the PR description should call them out explicitly (or consider splitting into a separate PR) to reduce review/merge risk for what is otherwise a targeted runtime fix.

bensig · 2026-04-07T20:45:00Z

Hey Igor — CI is failing, looks like it needs a rebase on main (we've merged a bunch of changes recently). Can you rebase and push?

Prevents OOM when the palace grows large. The following unbounded metadata fetches now have a safety cap: - tool_status: col.get(include=['metadatas'], limit=10000) - tool_list_wings: same - tool_list_rooms: same (including wing-filtered variant) - tool_get_taxonomy: same - Layer1.generate: col.get(include=['documents','metadatas'], limit=10000) Layer2 already had a limit parameter — no change needed. Finding: MemPalace#3 (CRITICAL — unbounded data fetching causes OOM) Includes test infrastructure from PR MemPalace#131. 92 tests pass.

…nused fixture param

Copilot AI review requested due to automatic review settings April 7, 2026 20:24

Copilot started reviewing on behalf of igorls April 7, 2026 20:24 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

igorls added 2 commits April 7, 2026 18:23

fix: cap diary_read query and update stale comment

161a0d1

igorls force-pushed the fix/bounded-queries branch from 9123379 to 161a0d1 Compare April 7, 2026 21:28

igorls closed this Apr 7, 2026

igorls force-pushed the fix/bounded-queries branch from 161a0d1 to 45c2c92 Compare April 7, 2026 21:57

fix: update dialect tests for PR MemPalace#147 stats API and remove u…

b5a5855

…nused fixture param

igorls reopened this Apr 7, 2026

adv3nt3 mentioned this pull request Apr 7, 2026

Bugs and improvements #159

Closed

bensig merged commit c30bc9e into MemPalace:main Apr 7, 2026
8 checks passed

RobertoGEMartin mentioned this pull request Apr 8, 2026

fix: batch remaining unbounded ChromaDB reads missed by #66 #146

Open

6 tasks

igorls mentioned this pull request Apr 8, 2026

bench: add scale benchmark suite (106 tests) #223

Merged

3 tasks

This was referenced Apr 9, 2026

security: harden inputs, fix shell injection, optimize DB access #387

Merged

CLI status silently truncates drawer count at 10,000 with no warning #180

Closed

chore: bump version to 3.1.0 #409

Merged

Multiple issues between README claims and codebase #27

Open

jfcampbl mentioned this pull request Apr 12, 2026

status command hides wings at scale (hardcoded limit=10000 in miner.py) #671

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add limit=10000 safety cap to all unbounded ChromaDB .get() calls#137

fix: add limit=10000 safety cap to all unbounded ChromaDB .get() calls#137
bensig merged 3 commits intoMemPalace:mainfrom
igorls:fix/bounded-queries

igorls commented Apr 7, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

bensig commented Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

igorls commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Packaging changes (carried over from base)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

bensig commented Apr 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

igorls commented Apr 7, 2026 •

edited

Loading