Conversation
find_sphinx_build() now checks the parent directory of sys.executable (without resolving symlinks) as the first search step. This ensures that when the MCP server runs via pipx, it finds sphinx-build in its own venv's bin/ directory rather than failing with FileNotFoundError. Previously, sphinx-build was only found if it was in a workspace .venv or on the system PATH, which excluded pipx installs where sphinx is a dependency of the package but not exposed to the system.
Separate 'When stuck or debugging' into two distinct triggers: - 'Encountering an error or failure' — emphasizes checking memory as the FIRST reaction before debugging from scratch - 'Stuck after initial attempts' — broadens search to related areas when initial debugging doesn't resolve the issue
There was a problem hiding this comment.
Pull request overview
Improves how the CLI locates sphinx-build (especially for non-workspace installs) and refines the README’s “when to recall” guidance.
Changes:
- Add an initial
sphinx-builddiscovery step based on the running Python environment. - Update Sphinx build search-order documentation/comments accordingly.
- Expand the README “When to recall” table with more specific recall triggers/commands.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/ai_memory_protocol/engine.py |
Updates find_sphinx_build() search order to first check alongside sys.executable. |
README.md |
Adjusts recommended recall triggers/commands in the workflow table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| own_bin = Path(sys.executable).parent / "sphinx-build" | ||
| if own_bin.exists(): | ||
| return str(own_bin) |
There was a problem hiding this comment.
The new “own environment” probe hardcodes sphinx-build and checks Path(...).exists(). This will miss Windows installs where the entry point is typically sphinx-build.exe, and it also doesn’t validate that the target is actually runnable. Consider using shutil.which("sphinx-build", path=str(Path(sys.executable).parent)) (or checking common suffixes) so PATHEXT/executability is handled correctly across platforms.
| own_bin = Path(sys.executable).parent / "sphinx-build" | |
| if own_bin.exists(): | |
| return str(own_bin) | |
| own_bin_dir = Path(sys.executable).parent | |
| own_bin = shutil.which("sphinx-build", path=str(own_bin_dir)) | |
| if own_bin: | |
| return own_bin |
| | Before a design decision | `recall --tag topic:<X> --type dec` | | ||
| | When stuck or debugging | `recall <error keywords>` | | ||
| | Encountering an error or failure | `recall <error message keywords>` — FIRST reaction before debugging; check if this problem was already solved | | ||
| | Stuck after initial attempts | `recall --tag topic:<X> --type mem,fact` — broaden search to related areas and past solutions | |
There was a problem hiding this comment.
The README suggests --type mem,fact, but the CLI’s --type filter is a single string comparison (need.get("type") != type_filter), so comma-separated values won’t work and will return no matches. Either update the docs to use a single type (or multiple commands), or extend the CLI to accept a comma-separated list / repeated --type flags.
| | Stuck after initial attempts | `recall --tag topic:<X> --type mem,fact` — broaden search to related areas and past solutions | | |
| | Stuck after initial attempts | `recall --tag topic:<X> --type mem` or `recall --tag topic:<X> --type fact` — broaden search to related areas and past solutions | |
1 participant
No description provided.