ci: enforce stub freshness in CI, fix existing stub drift#2912
ci: enforce stub freshness in CI, fix existing stub drift#2912michaeldwan merged 5 commits intomainfrom
Conversation
The generate:stubs mise task and stub:check already exist but weren't enforced in CI, so .pyi stubs could drift when Rust source changed. Adds a check-stubs job that runs `mise run --force stub:check` on Rust changes, using --force to bypass sources/outputs mtime freshness (which would skip regeneration in CI's fresh checkout).
Stubs drifted after the BuildInfo.dirty field was added in #2829 and docstring updates in recent PRs. Regenerated with `mise run generate:stubs`.
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| @property | ||
| def metrics(self) -> MetricRecorder: | ||
| r""" | ||
| The metric recorder for this prediction. |
There was a problem hiding this comment.
Type annotation regression: context was previously dict[builtins.str, builtins.str] but is now just dict. This reduces type safety and may break downstream type checking.\n\nsuggestion\n @property\n def context(self) -> dict[builtins.str, builtins.str]:\n
|
Posted a review on PR #2912 identifying a type annotation regression where |
stub_gen is a PyO3 binary that dynamically links libpython. setup-uv installs Python to a non-standard path, so the shared library isn't on LD_LIBRARY_PATH by default. Query sysconfig for the correct LIBDIR.
There was a problem hiding this comment.
This PR adds valuable CI enforcement for stub freshness. The check-stubs job implementation follows the established patterns used by check-llm-docs and docs:cli:check.
However, there's one issue:
Type annotation regression in generated stubs
The context property in Scope class has lost its generic type parameters. The regenerated stub shows -> dict but the docstring (and previous version) specifies dict[str, str]. This reduces type safety for downstream users.
This appears to be a limitation of pyo3-stub-gen 0.20, which doesn't preserve generic parameters for Py<PyDict>. Consider:
- Investigating if newer pyo3-stub-gen versions handle this better
- Adding post-processing in stub_gen.rs to fix specific regressions
- Accepting this as a known limitation for now (the docstring is correct)
|
I've reviewed PR #2912 and posted my feedback. The PR adds valuable CI enforcement for Python stub freshness, but there's a type annotation regression in the regenerated stubs where |
Regression from #2853: the context property returns Py<PyDict> which pyo3-stub-gen infers as bare `dict`. Add an override_return_type annotation so the generated stub preserves the intended dict[str, str] contract.
|
All files look correct. The type annotation regression has been properly fixed using the LGTM |
michaeldwan
changed the title
stub_gen needs libpython3.x.so at runtime (PyO3 auto-initialize). setup-uv's python-build-standalone is statically linked and doesn't ship the .so, causing 'cannot open shared object file' at runtime.
|
LGTM |
…icate/cog into mphelps/push-phase-progress * 'mphelps/push-phase-progress' of https://github.com/replicate/cog: (95 commits) feat: add metric name validation (#2911) Rename `cog run` to `cog exec` (#2916) chore(deps): bump github.com/google/go-containerregistry (#2884) fix: replace removed libgl1-mesa-glx in tensorflow integration test (#2914) ci: enforce stub freshness in CI, fix existing stub drift (#2912) feat: add schema-compare command to test harness (#2891) chore(deps): bump uuid from 1.22.0 to 1.23.0 in /crates (#2887) chore(deps): bump github.com/hashicorp/go-version from 1.7.0 to 1.9.0 (#2909) chore(deps): bump insta from 1.46.3 to 1.47.2 in /crates (#2908) fix: support list[X] | None inputs + integration tests for PEP 604 union File/Path coercion (#2882) ci: exclude Dependabot PRs from auto-code review (#2910) chore(deps): bump actions/checkout from 4 to 6 (#2904) chore(deps): bump github.com/testcontainers/testcontainers-go/modules/registry (#2886) fix: metrics bugs in coglet prediction server (#2896) Bump version to 0.17.2 (#2903) fix(coglet): propagate metric scope to async event loop thread (#2902) chore: remove unnecessary nolint directive in test (#2803) feat(coglet): add Sentry error reporting for infrastructure errors (#2865) fix: homebrew cask postflight xattr references wrong binary name (#2899) fix: include custom metrics in cog predict --json output (#2897) ...
2 participants
Coglet's Python type stubs (
.pyifiles generated bypyo3-stub-genfrom Rust source) can drift when someone changes Rust structs or docstrings and forgets to runmise run generate:stubs. Thestub:checkmise task already existed for local use but wasn't enforced in CI -- so the stubs have been quietly out of date since #2829.This PR does two things: prevents future drift and fixes the existing drift.
Preventing drift: Adds a
check-stubsCI job that regenerates stubs and diffs against HEAD, same pattern ascheck-llm-docsanddocs:cli:check. Usesmise run --forceto bypass thesources/outputsmtime freshness check, which would otherwise skip regeneration in CI's fresh checkout where all files share the same timestamp.Fixing existing drift:
BuildInfo.dirtyadded in feat: log coglet build info and SDK version at startup #2829, docstring updates from several PRs, formatting changes frompyo3-stub-genScope.contextwent fromdict[str, str]to baredict. The Rust method returnsPy<PyDict>whichpyo3-stub-gencan't infer generic params for -- added anoverride_return_typeannotation to preserve the intended contract.