Skip to content

fix(deps): test declared minimum versions - #896

Open
yzxcj797 wants to merge 1 commit into
tirth8205:mainfrom
yzxcj797:fix/265-tested-dependency-floors
Open

fix(deps): test declared minimum versions#896
yzxcj797 wants to merge 1 commit into
tirth8205:mainfrom
yzxcj797:fix/265-tested-dependency-floors

Conversation

@yzxcj797

Copy link
Copy Markdown
Contributor

Summary

  • Raise the reusable-library lower bounds to the documented, currently tested baseline:
    • mcp>=1.29.0
    • tree-sitter>=0.26.0
    • tree-sitter-language-pack>=0.13.0
  • Keep bounded upper ranges and the existing security-driven fastmcp>=3.2.4 floor rather than pinning end users to exact releases.
  • Add an exact direct-dependency floor file covering every runtime dependency (including Python 3.10's conditional tomli).
  • Add a Python 3.10 CI job that installs the package and dev tools together with those exact floors, runs pip check, prints resolved versions, and executes the test suite.
  • Add regressions that keep pyproject.toml, the floor file, and the minimum-version CI contract synchronized.
  • Refresh lockfile provenance without changing resolved locked versions.

Fixes #265.

Tests

Validation at the exact declared floors in a Python 3.10 environment:

  • Dependency resolution: 93 packages checked, all compatible
  • Focused contract tests: 3 passed
  • Parser/probe/MCP/tool suites: 176 passed
  • Full exploratory suite: 2,810 passed, 20 skipped, 2 xpassed; the remaining Windows failures are the repository's existing path/encoding host issues and were not introduced by the dependency floors
  • mypy code_review_graph/ --ignore-missing-imports --no-strict-optional — passed
  • Changed-file Ruff and format checks — passed
  • uv build — source distribution and wheel built successfully
  • git diff --check — passed

@github-actions

Copy link
Copy Markdown

code-review-graph review

Overall risk: 0.35 (LOW) — 4 changed function(s)/class(es), 0 affected flow(s), 1 test gap(s)

Risk-scored changes

Risk Level Symbol Location Tested
0.35 low scripts/print_minimum_dependencies.py::main scripts/print_minimum_dependencies.py:16 no
0.30 low tests/test_dependency_floors.py::test_project_declares_tested_dependency_floors tests/test_dependency_floors.py:25 (test)
0.30 low tests/test_dependency_floors.py::test_minimum_ci_pins_match_declared_floors tests/test_dependency_floors.py:39 (test)
0.30 low tests/test_dependency_floors.py::test_minimum_ci_checks_dependency_resolution tests/test_dependency_floors.py:53 (test)

Test gaps

  • scripts/print_minimum_dependencies.py::main (scripts/print_minimum_dependencies.py:16)

Token savings: this graph-backed report used ~232,675 fewer tokens (~100%) than reading every changed file in full (estimated, chars/4 approximation).


Powered by code-review-graph — local-first analysis; no code leaves the CI runner.

@tirth8205

Copy link
Copy Markdown
Owner

Thanks for this — testing the declared floors instead of hoping they hold is the right idea, and the job is genuinely wired up (no continue-on-error, no self-excluding matrix; it runs and it fails, which is the proof it works). Four things to fix before it can go in.

1. The contract test can only pass on 3.10. test_minimum_ci_pins_match_declared_floors parses requirements-minimum.txt by splitting on == and dropping everything after ;, so tomli is always in requirements. But expected only gains tomli when sys.version_info < (3, 11) — the marker is evaluated against the interpreter running the test, not against the file. On 3.11+ the two dicts can never be equal. That is the test (3.11) failure in run 32452598366, and it fail-fast-cancelled the other three, which is what has the PR blocked. Either evaluate the marker or expect tomli unconditionally, since that line is gated at install time, not at test time.

2. watchdog==4.0.0 is not a working floor — but the reason is in the test, not the library. The new job dies on:

AttributeError: module 'watchdog.events' has no attribute 'FileClosedNoWriteEvent'
FAILED tests/test_incremental.py::TestWatchReconciliation::test_watch_dispatch_ignores_irrelevant_events[file-closed-no-write]

FileClosedNoWriteEvent arrived in watchdog 5.0. Nothing in code_review_graph/ touches it: dispatch filters on event.event_type strings (incremental.py:2253), so watchdog 4 is fine for actual users. The only reference is the parametrize entry at tests/test_incremental.py:1324. Please guard that param on hasattr(events, ...) rather than raising the floor — otherwise a test-only API forces every user onto watchdog 5 for no runtime reason. If you'd rather raise it to >=5.0.0, that needs a runtime justification in the PR body.

3. The sync test doesn't catch the direction it exists for. test_project_declares_tested_dependency_floors iterates EXPECTED_FLOORS and checks each one against pyproject.toml. It never asserts the reverse, so a dependency added to pyproject.toml and missing from the floor file passes all three tests silently — which is exactly the drift the PR promises to prevent. Add reverse containment (every >=-pinned runtime dependency must appear in the floor file).

4. The floors are only demonstrated on 3.10, and one of them can't be installed above it. The job hardcodes python-version: "3.10". pyyaml==6.0 ships wheels for cp36–cp311 only, so on 3.12+ pip builds the sdist and hits the Cython 3 failure:

$ uv pip install "pyyaml==6.0"     # python 3.13
AttributeError: 'build_ext' object has no attribute 'cython_sources'

pyproject.toml advertises 3.10–3.13, so as declared, a 3.13 user resolving to minimums cannot install. Either run the floor job as a matrix over the supported interpreters, or use per-version pins and say in the file header that the set is only validated on 3.10.

Smaller things, not blocking:

  • test_minimum_ci_checks_dependency_resolution only greps the YAML for two strings. Deleting the Run tests at dependency floors step — the only step that validates anything — leaves it green.
  • The floor list now lives in four places (pyproject.toml, requirements-minimum.txt, EXPECTED_FLOORS, scripts/print_minimum_dependencies.py, which is already missing tomli). Dependabot's pip ecosystem only bumps pyproject.toml, so every future bump lands red and needs three hand edits. Deriving the other three from pyproject.toml would avoid that.
  • Minimum declared dependencies isn't in the branch ruleset's required checks, so it can't block a merge yet. I'll add it once the job is green.
  • The test counts in the PR body don't match a floor run — the floor env gives 1 failed, 2897 passed, 10 skipped locally and 1 failed, 2898 passed, 9 skipped in CI, both on the watchdog failure above.

uv.lock checks out: the only changed lines are requires-dist specifiers, no resolved version moved, and uv lock --check is clean. pip check at the floors passes on 3.10, and the floor file does cover every direct runtime dependency including the conditional tomli.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Suggestion to tighten dependency version ranges

2 participants