Skip to content

Fix/sphinx build discovery #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bburda merged 2 commits into from
Feb 8, 2026
Merged

Fix/sphinx build discovery #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f06d970
fix: discover sphinx-build in running Python's own bin directory
bburda Feb 8, 2026
876d58c
docs: split error/stuck recall triggers in When to recall table
bburda Feb 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ Only after peeking — pick the 2-3 most relevant IDs and `get` them individuall
| New task or topic | `recall --tag topic:<X> --format brief` |
| Entering unfamiliar code | `recall --tag repo:<X> --type fact --format brief` |
| 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 |
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
| 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 |

Copilot uses AI. Check for mistakes.
| Before implementing a pattern | `recall --tag intent:coding-style --type pref` |

#### 2. WRITE — Record at specific trigger points
Expand Down
20 changes: 14 additions & 6 deletions src/ai_memory_protocol/engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,26 @@ def find_sphinx_build(workspace: Path) -> str:
"""Locate the sphinx-build executable.

Search order:
1. ``workspace/.venv/bin/sphinx-build``
2. Walk parent directories for ``.venv/bin/sphinx-build``
3. ``shutil.which("sphinx-build")`` (system PATH)
1. Same directory as the running Python interpreter (covers pipx / venv installs)
2. ``workspace/.venv/bin/sphinx-build``
3. Walk parent directories for ``.venv/bin/sphinx-build``
4. ``shutil.which("sphinx-build")`` (system PATH)

Returns the path string, or raises ``FileNotFoundError``.
"""
# 1. Workspace venv
# 1. Running Python's own environment (pipx, venv, conda, etc.)
# Use parent of sys.executable WITHOUT resolving symlinks, so that
# pipx/venv wrapper scripts point to the correct bin directory.
own_bin = Path(sys.executable).parent / "sphinx-build"
if own_bin.exists():
return str(own_bin)
Comment on lines +193 to +195
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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

Copilot uses AI. Check for mistakes.

# 2. Workspace venv
candidate = workspace / ".venv" / "bin" / "sphinx-build"
if candidate.exists():
return str(candidate)

# 2. Walk parent directories
# 3. Walk parent directories
for parent in workspace.parents:
candidate = parent / ".venv" / "bin" / "sphinx-build"
if candidate.exists():
Expand All @@ -205,7 +213,7 @@ def find_sphinx_build(workspace: Path) -> str:
return str(candidate)
break # Only check immediate parent's siblings

# 3. System PATH
# 4. System PATH
system = shutil.which("sphinx-build")
if system:
return system
Expand Down
Loading