fix: sync versions, constants, type hints and standardize CLI errors#15
Closed
addyosmani wants to merge 5 commits intoMemPalace:mainfrom
Closed
fix: sync versions, constants, type hints and standardize CLI errors#15addyosmani wants to merge 5 commits intoMemPalace:mainfrom
addyosmani wants to merge 5 commits intoMemPalace:mainfrom
Conversation
__init__.py and mcp_server.py were still reporting 2.0.0 while pyproject.toml had already moved to 3.0.0. The MCP server now imports __version__ from the package so there is a single source of truth.
Scattered hardcoded values (chunk sizes, retrieval limits, snippet truncation lengths, graph traversal caps) are now defined once in constants.py and imported by miner, convo_miner, layers, mcp_server, and palace_graph.
Annotates all public functions in miner, convo_miner, searcher, palace_graph, mcp_server, layers, and config with explicit return types (dict, list[dict], None, bool, Path, etc.).
mcp_server tool_status/list_wings/list_rooms/get_taxonomy were calling col.get() without limits, loading every drawer's metadata into memory at once. miner.status() had a hardcoded limit=10000. Layer1.generate() fetched all documents unbounded. All now paginate in batches of GRAPH_BATCH_SIZE (1000) using offset-based iteration.
All error messages now go to stderr via a shared _error() helper or print(file=sys.stderr). The command dispatch is wrapped in try/except so unhandled errors produce a clean message and exit 1 instead of a traceback. KeyboardInterrupt exits with 130.
addyosmani
changed the title
IgorTavcar
added a commit
to IgorTavcar/mempalace
that referenced
this pull request
Apr 7, 2026
Cherry-picked from upstream PR MemPalace#15 (addyosmani). - Extract magic numbers to constants.py (chunk sizes, batch sizes, retrieval limits) - Add return type hints to all public functions across 7 modules - Unify version to single source in __init__.py (was 2.0.0 vs 3.0.0) - Paginate unbounded ChromaDB metadata queries (status command) - Standardize CLI error output to stderr with clean exit codes Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
igorls
added a commit
to igorls/mempalace
that referenced
this pull request
Apr 7, 2026
- Remove palace_path from _no_palace() error response (prevents leaking filesystem paths to the LLM) - Replace str(e) with generic 'Internal tool error' in MCP dispatch catch block (full error is still logged server-side via stderr) - Replace sys.exit(1) with return in searcher.search() CLI function (prevents process termination if called from library context) - Remove unused sys import from searcher.py Findings: MemPalace#12 (HIGH), MemPalace#5 (MEDIUM), MemPalace#15 (LOW) Includes test infrastructure from PR MemPalace#131. 92 tests pass.
Collaborator
|
Thanks for the thorough cleanup! A lot of these individual fixes are good but at +230/-102 touching multiple subsystems, it's hard to review as one PR. Several of the fixes here have also landed separately in recent merges. Would you be up for splitting the remaining unmerged changes into focused single-issue PRs? And we'd really value your help reviewing incoming PRs — you clearly have a good eye for consistency. Thanks! |
igorls
added a commit
to igorls/mempalace
that referenced
this pull request
Apr 7, 2026
- Remove palace_path from _no_palace() error response (prevents leaking filesystem paths to the LLM) - Replace str(e) with generic 'Internal tool error' in MCP dispatch catch block (full error is still logged server-side via stderr) - Replace sys.exit(1) with return in searcher.search() CLI function (prevents process termination if called from library context) - Remove unused sys import from searcher.py Findings: MemPalace#12 (HIGH), MemPalace#5 (MEDIUM), MemPalace#15 (LOW) Includes test infrastructure from PR MemPalace#131. 92 tests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Went through the codebase and knocked out a handful of things that could be improved. Nothing changes behavior.
Version was out of sync -
pyproject.tomlmoved to 3.0.0 a while back but__init__.pyand the MCP server still said 2.0.0. Fixed by having the MCP server import__version__from the package instead of hardcoding it. One source of truth now.Magic numbers everywhere - Chunk sizes, snippet limits, search defaults, graph traversal caps, etc. were all scattered as raw literals across half a dozen files. Pulled them into
constants.pyso there's one place to tweak them. The actual values haven't changed.Missing type hints - Public functions had parameter types but no return types, which made it harder to follow call chains. Added
-> dict,-> list[dict],-> None, etc. to all public functions across the core modules.Unbounded ChromaDB queries - Several places were doing
col.get(include=["metadatas"])with no limit, which pulls everything into memory. Fine for small palaces, not great once you have tens of thousands of drawers. These now paginate in batches of 1000.miner.status()had a hardcodedlimit=10000which has been replaced with the same approach.CLI error handling was inconsistent - Some errors went to stdout, some to stderr, some had exit codes, some just returned silently. Now all errors go to stderr through a shared helper, and the command dispatch is wrapped so unhandled exceptions produce a clean message instead of a traceback.
How to test
Checklist
python -m pytest tests/ -v)ruff check .)