fix: CI lint debt + deps + quickstart bug (green CI)#33
Conversation
Auto-fixed 7 (unsorted imports in cli.py, 5x F541 f-strings without placeholders) and manually fixed the rest: - B904 in core.py: re-raise ImportError with `from err` to preserve chain - F401 in cli.py: replace unused `import faiss` with importlib.util.find_spec availability check (preserves doctor-command semantics) - F841 in cli.py: drop two unused `result` assignments `ruff check alma/ tests/` and `ruff format --check alma/ tests/` now pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The MCP server hardcoded server_version="0.6.0" while the package is at 0.10.0. Default to alma.__version__ so the version can no longer drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ytest marker - Remove torch>=2.0.0 from requirements.txt: not imported directly, it is a transitive dependency of sentence-transformers (the [local] extra), so the direct pin only created drift. - Add a [tokenizer] extra (tiktoken>=0.5.0) for alma/utils/tokenizer.py, which is an optional capability with graceful fallback; fold it into [all]. - Make pyproject.toml authoritative; rewrite requirements.txt as a documented convenience mirror that points to the extras. - Register the `integration` pytest marker to kill PytestUnknownMarkWarning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The stacked @classmethod decorators made ALMA.quickstart() raise "'classmethod' object is not callable" on Python 3.13+, breaking the README's headline zero-config entry point. Verified callable after the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughCLI messaging, warning suppression, and faiss detection are refactored; ChangesCLI, Core, and Packaging Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request cleans up formatting, reorganizes dependencies in requirements.txt and pyproject.toml, and updates the MCP server to use the package's version dynamically. It also removes a duplicate @classmethod decorator in alma/core.py and changes the faiss diagnostic check in alma/cli.py to use importlib.util.find_spec. Feedback suggests reverting the faiss check to a direct import to ensure compiled C++ extensions load correctly, and adding the --no-password flag to the psql subprocess call to prevent the CLI from hanging on hidden password prompts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import argparse | ||
| import importlib.util | ||
| import os |
There was a problem hiding this comment.
If we revert the faiss check to use a direct import, importlib.util is no longer needed in this file and can be removed to keep the imports clean.
| import argparse | |
| import importlib.util | |
| import os | |
| import argparse | |
| import os |
| if importlib.util.find_spec("faiss") is not None: | ||
| _check("faiss (fast vector search)", True) | ||
| except ImportError: | ||
| _check("faiss (fast vector search)", False, | ||
| "pip install 'alma-memory[local]' — uses numpy fallback until then") | ||
| else: | ||
| _check( | ||
| "faiss (fast vector search)", | ||
| False, | ||
| "pip install 'alma-memory[local]' — uses numpy fallback until then", | ||
| ) |
There was a problem hiding this comment.
Using importlib.util.find_spec("faiss") only checks for the presence of the module files on disk, but does not attempt to load the module. Since faiss relies heavily on compiled C++ extensions, it is common for imports to fail due to missing system shared libraries (e.g., libgomp) even if the package is installed. Reverting to a direct import faiss wrapped in a try/except block (with a # noqa: F401 comment to satisfy Ruff) ensures that the diagnostic check actually verifies the library is fully functional.
| if importlib.util.find_spec("faiss") is not None: | |
| _check("faiss (fast vector search)", True) | |
| except ImportError: | |
| _check("faiss (fast vector search)", False, | |
| "pip install 'alma-memory[local]' — uses numpy fallback until then") | |
| else: | |
| _check( | |
| "faiss (fast vector search)", | |
| False, | |
| "pip install 'alma-memory[local]' — uses numpy fallback until then", | |
| ) | |
| try: | |
| import faiss # noqa: F401 | |
| _check("faiss (fast vector search)", True) | |
| except ImportError: | |
| _check( | |
| "faiss (fast vector search)", | |
| False, | |
| "pip install 'alma-memory[local]' — uses numpy fallback until then", | |
| ) |
| subprocess.run( | ||
| [ | ||
| "psql", | ||
| f"--host={host}", | ||
| f"--port={port}", | ||
| f"--dbname={database}", | ||
| f"--username={user}", | ||
| "--command", | ||
| PG_SETUP_SQL, | ||
| ], | ||
| capture_output=True, | ||
| text=True, | ||
| check=True, | ||
| ) |
There was a problem hiding this comment.
Since subprocess.run is called with capture_output=True, any interactive prompts (such as a password prompt from psql if the password is not set or cached) will be hidden from the user, causing the CLI to hang indefinitely. Adding the --no-password (or -w) flag to the psql command ensures that it fails immediately with a clear error message instead of hanging when a password is required but not provided.
| subprocess.run( | |
| [ | |
| "psql", | |
| f"--host={host}", | |
| f"--port={port}", | |
| f"--dbname={database}", | |
| f"--username={user}", | |
| "--command", | |
| PG_SETUP_SQL, | |
| ], | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| ) | |
| subprocess.run( | |
| [ | |
| "psql", | |
| "--no-password", | |
| f"--host={host}", | |
| f"--port={port}", | |
| f"--dbname={database}", | |
| f"--username={user}", | |
| "--command", | |
| PG_SETUP_SQL, | |
| ], | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@alma/cli.py`:
- Around line 147-152: The setup hint in the CLI output is using the wrong
option name, so users copying it will fail before running pg-setup. Update the
pg-setup message in alma.cli.main() to use the actual registered flag name from
the parser, matching the --database argument instead of --db, and keep the rest
of the command text unchanged.
- Around line 223-230: The FAISS availability check in the CLI doctor path is
only using find_spec, which can report a false positive even when importing
faiss would fail. Update the logic in the doctor check block to perform an
actual import of faiss and treat ImportError as unavailable, so the _check call
reports the numpy fallback path correctly; use the existing _check helper in
alma.cli to keep the behavior consistent.
In `@pyproject.toml`:
- Line 102: The project.optional-dependencies.all entry is missing the
quickstart extra, so update the all extras list in pyproject.toml to include
quickstart alongside the existing optional groups. Make the change in the
optional-dependencies definition for all so that pip install alma-memory[all]
pulls in the complete set of extras.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de8e3577-0544-4390-ad8f-d8f724342924
📒 Files selected for processing (5)
alma/cli.pyalma/core.pyalma/mcp/server.pypyproject.tomlrequirements.txt
| print( | ||
| f" {_yellow('!')} Set password: export ALMA_DB_PASSWORD='your-password'" | ||
| ) | ||
| print( | ||
| f" {_yellow('!')} Enable pgvector: alma pg-setup --host {host} --port {port} --db {database} --user {user}" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the actual CLI flag in this setup command.
Line 151 prints --db, but main() only registers --database. Copy/pasting this instruction currently fails before pg-setup runs.
Suggested fix
- f" {_yellow('!')} Enable pgvector: alma pg-setup --host {host} --port {port} --db {database} --user {user}"
+ f" {_yellow('!')} Enable pgvector: alma pg-setup --host {host} --port {port} --database {database} --user {user}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| print( | |
| f" {_yellow('!')} Set password: export ALMA_DB_PASSWORD='your-password'" | |
| ) | |
| print( | |
| f" {_yellow('!')} Enable pgvector: alma pg-setup --host {host} --port {port} --db {database} --user {user}" | |
| ) | |
| print( | |
| f" {_yellow('!')} Set password: export ALMA_DB_PASSWORD='your-password'" | |
| ) | |
| print( | |
| f" {_yellow('!')} Enable pgvector: alma pg-setup --host {host} --port {port} --database {database} --user {user}" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@alma/cli.py` around lines 147 - 152, The setup hint in the CLI output is
using the wrong option name, so users copying it will fail before running
pg-setup. Update the pg-setup message in alma.cli.main() to use the actual
registered flag name from the parser, matching the --database argument instead
of --db, and keep the rest of the command text unchanged.
| if importlib.util.find_spec("faiss") is not None: | ||
| _check("faiss (fast vector search)", True) | ||
| except ImportError: | ||
| _check("faiss (fast vector search)", False, | ||
| "pip install 'alma-memory[local]' — uses numpy fallback until then") | ||
| else: | ||
| _check( | ||
| "faiss (fast vector search)", | ||
| False, | ||
| "pip install 'alma-memory[local]' — uses numpy fallback until then", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant section and imports
python3 - <<'PY'
from pathlib import Path
p = Path('alma/cli.py')
lines = p.read_text().splitlines()
for start, end in [(1, 80), (200, 250)]:
print(f"\n--- {p}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PY
# Find all faiss references in the repo
rg -n --hidden --glob '!**/.git/**' '\bfaiss\b' almaRepository: RBKunnela/ALMA-memory
Length of output: 5069
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how faiss is consumed in the storage backend
python3 - <<'PY'
from pathlib import Path
p = Path('alma/storage/sqlite_local.py')
lines = p.read_text().splitlines()
for start, end in [(1, 90), (450, 590)]:
print(f"\n--- {p}:{start}-{end} ---")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PY
# Demonstrate that find_spec can succeed while import fails
python3 - <<'PY'
import importlib.util
import importlib
import tempfile
import os
import sys
from pathlib import Path
with tempfile.TemporaryDirectory() as d:
p = Path(d)
(p / "brokenmod.py").write_text("raise ImportError('simulated shared library/ABI failure')\n")
sys.path.insert(0, d)
try:
print("find_spec:", importlib.util.find_spec("brokenmod") is not None)
try:
importlib.import_module("brokenmod")
print("import: succeeded")
except Exception as e:
print("import:", type(e).__name__, str(e))
finally:
sys.path.remove(d)
PYRepository: RBKunnela/ALMA-memory
Length of output: 9855
Import FAISS instead of checking only for a spec
find_spec("faiss") can report success even when the module fails to import, so alma doctor can show a false positive. Use an actual import and fall back to the numpy path on ImportError.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@alma/cli.py` around lines 223 - 230, The FAISS availability check in the CLI
doctor path is only using find_spec, which can report a false positive even when
importing faiss would fail. Update the logic in the doctor check block to
perform an actual import of faiss and treat ImportError as unavailable, so the
_check call reports the numpy fallback path correctly; use the existing _check
helper in alma.cli to keep the behavior consistent.
| ] | ||
| all = [ | ||
| "alma-memory[local,azure,postgres,qdrant,chroma,pinecone,mcp,rag,observability,dev]", | ||
| "alma-memory[local,azure,postgres,qdrant,chroma,pinecone,mcp,rag,tokenizer,observability,dev]", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List all optional dependency extras defined in pyproject.toml and compare against 'all'
# Extract all extra names from [project.optional-dependencies]
python3 -c "
import tomllib
with open('pyproject.toml', 'rb') as f:
data = tomllib.load(f)
opt = data.get('project', {}).get('optional-dependencies', {})
all_extras = set(opt.keys())
print('Defined extras:', sorted(all_extras))
all_entry = opt.get('all', [''])[0] if 'all' in opt else ''
included = set()
if all_entry:
# Parse bracket contents like alma-memory[...]
import re
m = re.search(r'\[(.*?)\]', all_entry)
if m:
included = set(x.strip() for x in m.group(1).split(','))
print('In [all]:', sorted(included))
missing = all_extras - included - {'all'}
print('Missing from [all]:', sorted(missing) if missing else 'None')
"Repository: RBKunnela/ALMA-memory
Length of output: 469
Add quickstart to [all] project.optional-dependencies.all is missing the quickstart extra, so pip install alma-memory[all] does not include the full optional set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyproject.toml` at line 102, The project.optional-dependencies.all entry is
missing the quickstart extra, so update the all extras list in pyproject.toml to
include quickstart alongside the existing optional groups. Make the change in
the optional-dependencies definition for all so that pip install
alma-memory[all] pulls in the complete set of extras.
1 participant
Summary
Clears accumulated CI lint debt, reconciles dependency metadata, and fixes a real bug that broke the library's headline entry point. Goal: green CI across the supported Python matrix.
Changes (4 commits)
fix(core): remove doubled @classmethod on ALMA.quickstart(f008bfe) — the important one.ALMA.quickstart()— the entry point featured at the top of the README — carried a doubled@classmethoddecorator. On Python 3.13+ this raised at call time, so the documented "first thing you run" was broken for anyone on a current interpreter. Removed the duplicate decorator;ALMA.quickstart()works again.fix(lint): resolve 11 ruff errors blocking CI(c020e64) — 11 ruff violations that were failing the lint gate.fix(mcp): derive server_version from alma.__version__(f8330ad) — MCP server version is now sourced fromalma.__version__instead of a hardcoded literal, so it cannot drift from the package version.chore(deps): reconcile requirements.txt with pyproject and register pytest marker(5185fa2) — alignedrequirements.txtwithpyproject, and registered the previously-unknown pytest marker so the test run is warning-clean.Validation
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
tokenizerinstall extra and included it in the all-in-one install option.Bug Fixes
Chores