Repository navigation
Restore the api extra (fastapi, uvicorn, jinja2) and install it in CI - #35
Open
Nitjsefnie wants to merge 3 commits into
Open
Nitjsefnie wants to merge 3 commits into
Nitjsefnie wants to merge 3 commits into
Conversation
The CLI already tells users to install 'mnema-mcp[api]' (cli.py) and docs/backends.md documents the extra, but it did not exist in pyproject.toml, so 15 REST-API tests were silently skipped. Add api = ["fastapi>=0.115", "uvicorn>=0.30"] matching the lower-bound pinning style of the existing extras. Co-Authored-By: Kimi K3 <noreply@kimi.com>
The CI install step used '.[all,dev]' and 'all' deliberately excludes the API-serving group, so tests/test_api.py skipped 15 tests on every run. Install '.[all,api,dev]' instead. Co-Authored-By: Kimi K3 <noreply@kimi.com>
cli.py's dashboard error message states the api extra is 'fastapi + uvicorn + jinja2', but the extra only carried fastapi and uvicorn, so '.[api]' alone still hit that ImportError handler. Adding jinja2 makes the repo's own description of the extra true. python-multipart is deliberately not added: the core mcp dependency already requires it (python-multipart>=0.0.9). Co-Authored-By: Kimi K3 <noreply@kimi.com>
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
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.
Summary
Defines the
apiextra that the CLI already recommends (mnema-mcp[api]) and installs it in CI, so the 15 REST-API tests stop skipping. The extra is not new βc17ff41(#22) shipped it, and the cherry-pickaf4bc31rolledpyproject.tomlback to an older revision and dropped it. This restores it.Related issue
Closes #32
Relates to #34
Changes
packages/mnema-python/pyproject.tomlβ addapi = ["fastapi>=0.115", "uvicorn>=0.30", "jinja2>=3.1"].github/workflows/python-ci.ymlβ install.[all,api,dev]instead of.[all,dev], so the REST-API tests actually runChecklist
ruff check .is clean β ran CI's own invocation,ruff check src/ tests/βAll checks passed!pytestpasses (with the relevant[extra]installed if you touched a backend) β164 passed, 1 skippedon.[all,api,dev]; the skip istests/test_backends.py:113, pgvector'sMNEMA_PGVECTOR_TEST_DSNnot being set.tests/test_api.pygoes from1 passed, 15 skippedto16 passed, verified as a before/after in two fresh venvs rather than only after.README.md,SKILL.md,docs/) if user-facing behavior changed β n/a, and worth noting why:README.md:81anddocs/backends.md:45already listapi. The docs were never rolled back; onlypyproject.tomlwas, which is why the docs and the package disagreed.Notes for reviewer
Why
jinja2is in the extra, since the original wasn't.src/mnema/cli.py:517already tells the user the dashboard "requires the 'api' extra (fastapi + uvicorn + jinja2)". Withapi = [fastapi, uvicorn], that sentence is false in an unhelpful way: someone who installs.[api]and runsmnema dashboardgets that exact message, telling them to install the extra they already have. Addingjinja2makes your existing sentence true rather than editing your prose. If you would ratherapistayfastapi + uvicornand the message change instead, say so and I will flip it.python-multipartdeliberately not added, even though the dashboard'sForm(...)routes need it:mcpis a core dependency and hard-requirespython-multipart>=0.0.9, so it is present in every install already. I checked this by registering aFormroute in a venv without it (RuntimeError: Form data requires "python-multipart" to be installed.) and confirming it resolves in a plain.[all,dev]install.Version floors. I used
fastapi>=0.115/uvicorn>=0.30rather than the original>=0.110/>=0.29fromc17ff41. Both match the file's uniform>=X.Yfloor style; happy to restore the originals if you prefer minimal drift from what you had.One honest caveat on #32's framing. "15 tests never run in CI" holds against today's index, where
.[all,dev]resolves 148 packages with no fastapi. Olderchromadb(<1.0) carried fastapi as a core dependency, so a past resolution would have pulled it in and those tests would have run. The missing extra is real either way; the CI symptom is resolution-dependent.Still not fixed, and not bundled here.
.[api]alone still cannot runmnema dashboardend to end, because no vector backend ships in it β the practical minimum is[api,chroma,local].mnema servehas the same shape, since both constructMemoryServiceeagerly. Tell me if you would like the error message to say that and I will send a follow-up.Bugs discovered
Generated by Claude Opus 5 (review), Kimi K3 (brief, implementation, verification)