fix(deps): sync requirements files with pyproject and guard the drift - #144
fix(deps): sync requirements files with pyproject and guard the drift#144eldonm wants to merge 3 commits into
Conversation
requirements.txt and requirements-all.txt each listed 5 of the 9 runtime dependencies pyproject declares. aiohttp, pymupdf, packaging and mcp were absent from both, and both pinned jvspatial two releases behind. This is not only a stale doc. Dockerfile.base builds the runtime image from requirements-all.txt, so the image shipped without four runtime dependencies -- an install that succeeds and then fails as an ImportError deep in a run, when the action loader reaches for mcp or pymupdf. Add the missing dependencies to both files and align the jvspatial pin with pyproject. Then stop it recurring. tests/test_requirements_sync.py treats pyproject as the source of truth and asserts two properties over both files: every core dependency is listed, and every listed spec matches pyproject exactly. Nothing enforced either before, which is why the files could sit wrong indefinitely -- pip metadata never reads them, so no install ever disagreed. Both assertions were mutation-checked rather than trusted green: removing mcp fails the first, reverting the pin fails the second, and restoring passes.
The file's header promises "all core dependencies plus all optional dependencies from action info.yaml files". Across 57 action info.yaml files and their per-action requirements.txt, 22 distinct pip dependencies are declared; this file carried 3. Since Dockerfile.base builds the runtime image from it, the shipped image had no dependencies for Deepgram STT, ElevenLabs TTS, every Google action, Microsoft Excel, SerpAPI search, web_fetch, or PageIndex's LLM stack. Each fails as an ImportError when the action loads, not at install. Add the 13 that are unambiguous, taking the highest floor where actions declare different ones -- which is what pip resolves to anyway. Verified the file still resolves with a dry-run install. tiktoken is deliberately left out, and that is the finding worth reading: jvagent/pageindex declares tiktoken>=0.11.0 while Dockerfile.base installs 'tiktoken<0.8.0'. Those cannot both hold. Listing it here would let this file quietly override the Dockerfile's cap during the image build, so the contradiction wants resolving first -- by moving the cap or the floor, whichever is right. It sits in an allowlist in the test with that reason attached. Extend the guard accordingly: every action-declared pip dep must appear in requirements-all.txt, and an allowlist entry must still correspond to a real declaration, so tiktoken cannot linger after the fix. Both were mutation-checked -- dropping elevenlabs fails and names the declaring info.yaml; a bogus allowlist entry fails too. Not addressed here, reported instead: several packages are declared with different floors across actions (httpx has four variants, pyyaml three spellings). pip takes the highest, so these mislead rather than break, and harmonising them means editing ~20 info.yaml files.
|
Extended this to cover the action-level dependencies as well — pushed in 85316a5. The audit. Across 57 action Because Added the 13 unambiguous ones (highest floor where actions disagree — what pip resolves to anyway), and confirmed the file still resolves with a dry-run install.
Those cannot both hold. Listing it here would let Which way should this go? Raise the Dockerfile cap, or lower pageindex's floor? Also found, deliberately not changed: several packages are declared with different specs across actions.
pip takes the highest floor, so these mislead rather than break — except that one action declares an |
Dockerfile.base pinned 'tiktoken<0.8.0' while jvagent/pageindex declares tiktoken>=0.11.0. The cap is the wrong side of that, for three reasons. litellm -- declared by pageindex, and installed into the same image via requirements-all.txt -- requires tiktoken>=0.8.0,<1.0. So the cap was already unsatisfiable against the image's own dependency set; pageindex's floor only made it visible. The cap is not protecting the build either. cp312 manylinux x86_64 wheels exist at 0.7, 0.11 and 0.13, and the image is lambda/python:3.12 with --only-binary=:all:, so nothing forced a sub-0.8 pin. It dates to March and went stale. And jvagent's own use is version-agnostic: response/chunking.py and model/utils/token_estimation.py call get_encoding() and encoding_for_model(), stable across this whole range, both behind optional imports. Raise it to >=0.11.0,<1.0 -- pageindex's floor, litellm's ceiling. Verified the two co-resolve rather than assuming it. With the contradiction gone, tiktoken joins requirements-all.txt and ACTION_DEP_EXCEPTIONS empties, so the guard now covers all 22 declared action dependencies with no carve-outs. Not verified: an actual docker build of the image. The resolve is proven; the build is not.
|
Resolved the 1. The cap was already unsatisfiable against the image's own dependencies. 2. The cap was not protecting the build. cp312 manylinux x86_64 wheels exist at 0.7, 0.11 and 0.13; the image is 3. jvagent's own use is version-agnostic. The only direct consumers — Raised to With the contradiction gone, One thing I did not verify: an actual |
1 participant
Summary
requirements.txtandrequirements-all.txteach listed 5 of the 9 runtime dependenciespyproject.tomldeclares. Missing from both:aiohttp,pymupdf,packaging,mcp. Both also pinnedjvspatialtwo releases behind pyproject.This is not just a stale doc.
Dockerfile.basebuilds the runtime image fromrequirements-all.txt(lines 35-36), so the image shipped without four runtime dependencies — an install that succeeds and then fails as anImportErrordeep in a run, when the action loader reaches formcporpymupdf.Related issues
None filed. Found while checking whether uvicorn needed pinning here (it doesn't — jvagent has no uvicorn dependency of its own; see TrueSelph/jvspatial#42).
Type of change
Changes
requirements.txt/requirements-all.txt— add the four missing dependencies, align thejvspatialpin with pyproject.tests/test_requirements_sync.py— new. Treats pyproject as the source of truth and asserts, over both files, that every core dependency is listed and every listed spec matches pyproject exactly.Checklist
CONTRIBUTING.mdand the localCLAUDE.md.pre-commit run --all-filespasses.pytest tests/passes — 0 failures.file:linein the description.CHANGELOG.md— no entry; this restores files to their intended contents rather than changing jvagent's behavior. Happy to add one if you'd rather packaging fixes be listed.Notes for reviewers
Why this could sit wrong indefinitely. Nothing reads these files during a normal dev install —
pip install -e .uses pyproject metadata — so no install ever disagreed with them. The only consumers are the Docker build and the docs, and neither validates. The new test closes that: pyproject is authoritative, and the files must track it.The guards were mutation-checked, not trusted green. A test that cannot fail is worse than no test, so I broke each property deliberately:
mcpfromrequirements.txtjvspatial==0.0.15Each fails on exactly the drift that was actually present.
Coordination with #143. That PR moves the
jvspatialpin to0.0.17; this one sets it to0.0.16to match currentmain. Both touch that line, so whichever merges second needs a one-line rebase — I'll handle it.Worth noting the side effect: once both land, the spec-match test would have caught #143's stale requirements by itself. Bumping pyproject without updating the requirements files now fails CI.
Deliberately left alone:
requirements-all.txtalso carries action-level dependencies harvested frominfo.yamlfiles, which I did not audit — this PR only claims the core block matches pyproject. Auditing the action deps against everyinfo.yamlis a bigger, separate job, and the same test structure would extend to it.Steps to test