Add pyarrow-stubs and improve type coverage#1
Merged
Merged
Conversation
rustyconover
force-pushed
the
fix-types3
branch
2 times, most recently
from
b12d6f9 to
48c643f
Compare
- Add pyarrow-stubs dependency for proper PyArrow type hints - Fix type annotations throughout vgi/ to satisfy mypy with stubs - Add explicit structlog.stdlib.BoundLogger annotations for loggers - Use TYPE_CHECKING imports for forward references (Scalar types) - Add from __future__ import annotations for deferred type evaluation - Fix BufferedReader and PythonFile type annotations - Achieve 95.78% mypy type coverage (up from ~94%) - Add pre-commit check reminder to CLAUDE.md (ruff check --fix, format, mypy) - Centralize test schema helper to tests/utils.py All 412 tests pass with mypy reporting no issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rustyconover
force-pushed
the
fix-types3
branch
from
48c643f to
05760bc
Compare
rustyconover
added a commit
that referenced
this pull request
May 16, 2026
… tests Blocker fix from senior review #1. ``TableBufferingFinalizeState`` inherited the no-op ``StreamState.on_cancel`` from the base, so the user-facing ``TableBufferingFunction.on_cancel`` classmethod was dead code — documented but never invoked. Streaming peers (``TableProducerState.on_cancel``) already forwarded their cancel hooks; the buffered finalize path was the asymmetric outlier. Wire: * ``TableBufferingFinalizeState.on_cancel(ctx)`` resolves func_cls + params via the same cold-load path as ``produce()`` (no in-process cache; HTTP rehydration may land on a different worker process), deserializes the user's last-emitted state from ``state_blob``, then calls ``cls.on_cancel(params, finalize_state_id, state)``. * Pre-init cancel (state_initialized=False) short-circuits — no user state to forward. * Implementation lookup failures and user-callback exceptions are swallowed via ``contextlib.suppress`` — we're on a teardown path and must not double-fault during pipeline unwind. Tests (tests/test_table_buffering_function.py — 5 new cases): * Happy-path: cls.on_cancel runs with deserialized state. * state_initialized=False: no invocation. * state_initialized=True + empty blob: cls.on_cancel runs with state=None. * ctx.implementation=None: silent no-op (no crash). * User on_cancel raising: silent no-op (no propagation). The tests sit at the protocol layer rather than driving an SQL LIMIT end-to-end, because the wire cancel is delivered asynchronously by ``VgiCancelDispatcher`` (one thread writes the cancel batch; the subprocess worker reads it on its own schedule) and racing that against a probe-file assertion produces flaky tests. The contract that matters — "when the framework calls state.on_cancel, the user's classmethod runs with deserialized state" — is fully exercised here. A ``SlowCancellableBufferingFunction`` fixture is also registered in the example worker, mirroring the existing ``slow_cancellable`` / ``slow_cancellable_inout`` fixtures, so a future deterministic integration test has the building block ready when the async-cancel delivery story stabilizes. Side improvement: ``SumAllColumnsFunction.combine`` now emits a ``params.client_log`` line under ``logging=true``, mirroring its process()-side log. Locks in the combine-side unary-RPC client_log path (review #4) so adding new buffered-table fixtures can't silently regress that. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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
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.
Summary
pyarrow-stubsdependency for proper PyArrow type hintsvgi/to satisfy mypy with stubsstructlog.stdlib.BoundLoggerannotations for loggersChanges
Dependencies
pyarrow-stubsfor type hintsType Annotations
TYPE_CHECKINGimports for forward references (Scalartypes)from __future__ import annotationsfor deferred type evaluationBufferedReaderandPythonFiletype annotationsstructlog.stdlib.BoundLoggertype annotationsFiles Modified
vgi/andtests/Test plan
uv run mypy vgi/reports no issuesuv run ruff check .passes🤖 Generated with Claude Code