Skip to content

Nothing checks that the repo typechecks at its declared dependency floors #236

Description

@lesnik512

The gap

_checks.yml's lowest-direct job installs each target at the floors pyproject.toml declares and
then runs exactly one thing:

- name: Install at the declared floors
  run: uv pip install --resolution lowest-direct ".[${{ matrix.extras }}]"
- name: Bootstrap at the floors
  run: .venv/bin/python scripts/floor_smoke.py ${{ matrix.target }}

floor_smoke.py is a runtime smoke: it builds a config, bootstraps, exercises the calls that set a
floor, and tears down. Nothing type-checks and nothing runs the test suite at the floor, so a type
annotation that is valid against the newest resolution and invalid at the floor passes CI in every
job.

How it surfaced

faststream 0.7.6 gave BrokerUsecase a third type parameter. The fix (#232) spells the annotation
BrokerUsecase[typing.Any, typing.Any, typing.Any], which I verified by hand against 0.6.0, 0.7.4
and 0.7.6. CI verified only the newest of those three. If 0.6 and 0.7.6 had disagreed, nothing
here would have told us; the floor job would still have gone green, because floor_smoke.py never
evaluates that annotation.

The daily dependency check added in #233 closes the ceiling side of this. The floor side is still
open.

Why the obvious fixes do not work

Both of the cheap options are already blocked, for reasons the repo documents.

Running pytest at the floor is ruled out by scripts/floor_smoke.py's own docstring:

Not a pytest test: conftest.py hard-imports opentelemetry, sentry_sdk and structlog, which most
of these targets do not install.

Running ty check at the floor hits the mirror image. A floor environment for one target
installs only that target's extras, so checking the whole repo would report unresolved imports for
every framework the job deliberately did not install. That is noise, not findings, and AGENTS.md is
explicit that ty diagnostics are meant to be acted on.

Options

  1. Accept and write it down. Add a line to the lowest-direct comment in _checks.yml saying
    the floor is smoke-only, so an annotation touching an optional dependency's internals has to be
    written to span the declared range and checked by hand. Costs nothing, keeps the gap.
  2. A narrow typecheck per target. Run ty at the floor over a restricted file set for that
    target only. Needs a way to stop ty from following imports into modules the job did not
    install, which may not be expressible; worth a spike before committing.
  3. Make floor_smoke.py import the annotated helpers. Catches arity changes, since subscripting
    a generic with too many parameters raises at import. Would not have caught test: annotate the broker helper for faststream 0.7.6 #232, which was an
    assignability failure, not an arity one. Cheap but mostly theatre.
  4. Raise floors more aggressively so the declared range is narrow enough that one resolution
    represents it. A dependency policy change, not a CI one.

Recommendation

Option 1 unless someone wants to spike option 2. The failure mode is narrow: it needs an annotation
that references an optional dependency's internals and a floor that disagrees with the ceiling
about it. That has happened once, it was caught within hours, and the blast radius was a test helper.
The cost of options 2 and 3 looks higher than the risk.

Filing it so the gap is a decision rather than an omission. Not labelled ready-for-agent: the work
here is picking an option, not writing the patch.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-triageMaintainer needs to evaluate this issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions