fix(ci): pin the uv interpreter so the matrix tests what it claims - #1429
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
phernandez
force-pushed
the
ci-pin-venv-python
branch
from
September 1, 2026 20:13
8e9dee3 to
b440ea0
Compare
phernandez
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b440ea051e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
phernandez
force-pushed
the
ci-pin-venv-python
branch
from
September 1, 2026 20:18
b440ea0 to
6a92f4f
Compare
Every uv invocation in CI resolved its interpreter from `.python-version` (3.14) instead of the version the job set up. The matrix rows labeled "Python 3.12" and "Python 3.13" therefore built and ran against 3.14 — we have had no coverage of either version despite reporting it, while `requires-python` declares `>=3.12`. Pinning `uv venv` alone is not enough. Every `just` recipe shells out to `uv run`, which re-resolves the interpreter and DELETES a `.venv` that disagrees, recreating it from the lockfile. That drops anything installed by the job's `uv pip install -e ".[dev,milvus,pdf]"` step, which is how a pinned 3.12 venv ends up failing typecheck on an unresolved `pymilvus`. Set UV_PYTHON at job level instead: it governs every uv call, including the ones inside just recipes, so the environment survives and the job runs the version its name claims. Covers all ten jobs that build a uv environment. claude.yml and release.yml had the same defect — release.yml was building the published artifact on 3.14 while declaring 3.12 — and consolidated-packages.yml's hermes job ran `uv python install 3.12`, which only downloads that interpreter without selecting it. Signed-off-by: phernandez <paul@basicmachines.co>
phernandez
force-pushed
the
ci-pin-venv-python
branch
from
September 1, 2026 20:20
6a92f4f to
bf57f06
Compare
Pinning UV_PYTHON made the 3.12 and 3.13 matrix rows run their declared interpreter for the first time, which surfaced seven pre-existing test bugs. Neither is a regression from that change; both are tests that only ever ran on 3.14. CPU-budget auto-tuning (6 tests, 3.12 only) `os.process_cpu_count` is new in 3.13. `_available_cpu_count` already handles its absence, but the tests did not: `monkeypatch.setattr` refuses to set an attribute that does not exist, so every test that pinned a CPU budget raised AttributeError on 3.12. Each of those tests pins `process_cpu_count` and `cpu_count` to the same value, so none of them cares which API reports the budget - they assert on the resolved number. Collapse both patches into a `pin_cpu_budget` fixture that passes `raising=False`, which keeps the preferred branch under test on 3.12 as well. monkeypatch deletes an attribute it created during teardown, so `os` is restored exactly as found on every interpreter (verified). That left the genuine 3.12 branch - `process_cpu_count` absent, budget from `os.cpu_count()` - covered by no test on any interpreter, since 3.13+ never takes it and 3.12 now forces the attribute to exist. Add two tests that delete the attribute to reproduce the 3.12 runtime anywhere: one asserting the fallback resolves the same knobs as the preferred API, one asserting an unreported budget leaves FastEmbed on its own defaults. `_available_cpu_count` and `_resolve_fastembed_runtime_knobs` are now fully covered on 3.12 and 3.14. Zero-argument super() under dataclass slots (1 test, 3.12 and 3.13) `dataclass(slots=True)` cannot add `__slots__` in place, so it rebuilds the class. Before CPython 3.14 (gh-90562) the rebuilt methods keep a `__class__` cell pointing at the discarded original, and zero-argument `super()` rejects `self` with "obj must be an instance or subtype of type". Name the class in the super() call so it is re-looked-up at call time and resolves to the rebuilt class on every supported interpreter. A repo-wide AST scan for slots dataclasses containing zero-argument super() found this as the only occurrence. Verified: tests/repository/test_openai_provider.py, tests/repository/test_fastembed_provider.py and tests/indexing/test_project_index_maintenance.py all pass (84 tests) on 3.12, 3.13 and 3.14. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
This was referenced Sep 1, 2026
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
1 participant
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.
Problem
Every uv invocation in CI resolved its interpreter from
.python-version— which pins 3.14 — rather than the version the job had just set up.So the matrix has been lying. The rows labeled "Python 3.12" and "Python 3.13" installed those interpreters and then built and tested against 3.14. We have had no 3.12 or 3.13 coverage despite the matrix reporting it, even though
requires-pythondeclares>=3.12.Direct evidence, from the thread dump of a job named "Test SQLite Integration (windows-latest, Python 3.12)":
Why pinning
uv venvis not enoughMy first attempt added
--pythonto eachuv venv. That fixes the venv and nothing else, because everyjustrecipe shells out touv run, which re-resolves the interpreter independently.Reproduced directly, in a throwaway project with
.python-version= 3.14:uv rundeletes a.venvwhose version disagrees and rebuilds it from the lockfile, discarding whateveruv pip install -e ".[dev,milvus,pdf]"had put there. That is how the first attempt turned green static-checks intoerror[unresolved-import]: Cannot resolve imported module 'pymilvus'— the pinned 3.12 venv was destroyed beforetyever ran.Fix
Set
UV_PYTHONat job level. It governs every uv call, including those inside just recipes, so the environment survives and the job runs the version its name claims:Applied to all ten jobs that build a uv environment —
${{ matrix.python-version }}where a matrix drives it, literal3.12where the job hardcodes one:test.yml— seven jobsrelease.yml— was building the published artifact on 3.14 while declaring 3.12claude.ymlconsolidated-packages.yml(hermes) — ranuv python install 3.12, which only downloads that interpreter and never selects itA note on the force-pushes
An intermediate revision injected a second
env:block into the two Postgres jobs, which already had one. That is a duplicate YAML key: GitHub rejects the whole file, while PyYAML'sSafeLoadersilently keeps the last one — so local validation passed. The only symptom was the run appearing under the name.github/workflows/test.ymlinstead ofTests, with zero jobs and no error text anywhere in the API.It would also have silently dropped
BASIC_MEMORY_TEST_POSTGRES_URLfrom both jobs had it parsed. Now merged into the existing blocks, and validation uses a loader that raises on duplicate keys rather thanyaml.safe_load. Verified: every workflow parses with no duplicate keys, all ten uv jobs carryUV_PYTHON, the matrix expression appears exactly where a matrix defines that key, and both Postgres jobs retain both variables.Expect new failures
This makes 3.12 and 3.13 run for the first time, so it may surface real failures that were previously hidden. Any such failure is a pre-existing bug this was masking, not a regression introduced here. Related: #1430 (Windows integration hang), found while triaging the #1421 failure that started this.