Skip to content

Plan 009: Triage the 128 ty diagnostics and add type checking to CI #22

Description

@strickvl

Plan 009: Triage the 128 ty diagnostics and add type checking to CI

Executor instructions: Follow this plan step by step. Run every
verification command and confirm the expected result before moving to the
next step. If anything in the "STOP conditions" section occurs, stop and
report — do not improvise. When done, update the status row for this plan
in plans/README.md.

Drift check (run first): git diff --stat 5b5c634..HEAD -- pyproject.toml justfile .github/workflows/test.yml
Also re-run the baseline: uv run ty check . 2>&1 | tail -1 — if the
diagnostic count differs wildly from 128, re-derive the rule breakdown
before proceeding.

Status

  • Priority: P3
  • Effort: M
  • Risk: LOW (config + annotation fixes; behavior must not change)
  • Depends on: 007 (dead-code removal shrinks the surface; do 007 first)
  • Category: dx
  • Planned at: commit 5b5c634, 2026-06-12

Why this matters

The ty type checker (Astral's fast Python type checker) is already in the
dev dependency group but is run nowhere — not in the justfile, not in CI.
Type errors currently surface only as runtime crashes. At the planning
commit, uv run ty check . reports 128 diagnostics. Getting that to
zero (through a mix of real annotation fixes and explicit, documented rule
downgrades) and wiring ty into just ci + GitHub Actions gives every
future PR a type-level safety net — particularly valuable here because the
codebase passes Union[str, Tuple[str, str]] entity keys and dynamically
built Pydantic models around, exactly where type confusion bites.

Current state

  • Baseline (verified): uv run ty check .Found 128 diagnostics, broken
    down by rule:

    Count Rule Likely shape
    31 error[invalid-parameter-default] param: str = None style — fix with Optional[...]
    28 warning[possibly-unbound-attribute] attributes set conditionally (e.g. self._loop in src/utils/embeddings/manager.py:343-347)
    23 error[invalid-type-form] dynamic forms like response_model=List[Entity] with runtime classes
    15 error[invalid-argument-type] real mismatches — inspect individually
    6 error[unresolved-reference] inspect individually
    6 error[unresolved-import] optional extras (anthropic, sentence-transformers) imported conditionally
    5 error[unsupported-operator] inspect individually
    5 error[missing-argument] real call-site issues — inspect individually
    3 error[invalid-return-type] inspect
    2 error[call-non-callable] inspect
    4 one each: unresolved-attribute, not-iterable, invalid-raise, invalid-assignment inspect
  • pyproject.toml: ty>=0.0.1a9 in [dependency-groups] dev; no
    [tool.ty] section exists yet
    . ty is alpha — consult the installed
    version's docs (uv run ty help, and https://docs.astral.sh/ty/) for the
    exact config syntax for rule severity overrides (expected shape:
    [tool.ty.rules] with rule-name = "ignore") and inline suppressions
    (# ty: ignore[rule-name]).

  • justfile: lint recipe = ruff check + ruff format check; ci recipe =
    lint + pytest. CLAUDE.md promises just ci runs exactly what GitHub
    Actions runs — keep that property: whatever lands in the workflow must
    also land in the ci recipe.

  • .github/workflows/test.yml: lint job (ruff only) and test job.

  • Repo type-hint convention: typing.Dict / typing.Tuple over builtin
    generics, typing.Optional over X | None (match existing code).

Commands you will need

Purpose Command Expected on success
Baseline uv run ty check . (initially) 128 diagnostics
Per-rule view uv run ty check . 2>&1 | grep 'error\[invalid-parameter-default\]' the 31 sites
Tests just test all pass
Full CI parity just ci exit 0 (including the new ty step at the end)

Scope

In scope:

  • Type annotations and small, behavior-preserving signature fixes anywhere
    in src/ and scripts/ that a diagnostic points at
  • pyproject.toml ([tool.ty] config; pin ty to the currently locked
    version with ty==<locked> to stop alpha churn breaking CI)
  • justfile (new typecheck recipe; extend ci)
  • .github/workflows/test.yml (add ty step to the lint job)
  • CLAUDE.md (one line documenting just typecheck)

Out of scope (do NOT touch):

  • ANY behavior change. If fixing a diagnostic requires changing what the
    code does (not just its annotations/defaults), that's a STOP condition.
  • tests/ — don't annotate tests in this pass; exclude them via ty config
    if needed.
  • Adding mypy or any second type checker.

Git workflow

  • Branch: advisor/009-ty-typecheck-in-ci
  • Commit per rule-cluster (e.g. "Fix invalid-parameter-default diagnostics
    (31 sites)") so review is mechanical.
  • Do NOT push or open a PR unless the operator instructed it.

Steps

Step 1: Pin ty and add config skeleton

Find the locked version (grep -A1 'name = "ty"' uv.lock | grep version),
pin it in the dev group (ty==<that version>), and add a [tool.ty]
section (exact syntax per the installed version's docs) configured to check
src and scripts and exclude tests.

Verify: uv sync && uv run ty check . → still runs, diagnostic count unchanged ±0

Step 2: Mechanical fixes — invalid-parameter-default (31)

For each site: param: str = Noneparam: Optional[str] = None (import
Optional where missing). These must be annotation-only changes.

Verify: uv run ty check . 2>&1 | grep -c "invalid-parameter-default"0; just test → all pass

Step 3: Conditional-import and optional-extra rules

For the 6 unresolved-import sites: if they are guarded imports of optional
extras (anthropic, sentence_transformers), suppress per-site with the
inline ignore comment and a short reason, or per-module via config — choose
inline (it documents itself). Do NOT make optional deps unconditional.

Verify: uv run ty check . 2>&1 | grep -c "unresolved-import"0; just test → all pass

Step 4: Judgment fixes — the remaining errors

Work through invalid-argument-type (15), missing-argument (5),
unresolved-reference (6), unsupported-operator (5), and the singletons.
For each: if it's a genuine annotation gap, fix the annotation; if it's a
true false positive of alpha ty, add an inline ignore with a reason comment;
if it looks like a real bug (a call genuinely missing an argument on a
reachable path), record it in your final report and suppress with
# ty: ignore[...] — possible real bug, see plans/README.md notes rather
than changing behavior.

For possibly-unbound-attribute (28, warnings): prefer initializing the
attribute in __init__ (e.g. self._loop: Optional[asyncio.AbstractEventLoop] = None
plus adjusting the hasattr check to self._loop is None or self._loop.is_closed())
ONLY where that is transparently equivalent; otherwise downgrade the rule to
"ignore" in config with a comment, and say so in the report.

Verify: uv run ty check .Found 0 diagnostics (or All checks passed); just test → all pass

Step 5: Wire into justfile and CI

  • justfile: add

    # Type-check with ty
    typecheck:
        uv run ty check .

    and append uv run ty check . to the ci recipe (after the ruff lines).

  • .github/workflows/test.yml: in the lint job, after "Ruff format
    check", add a step name: Type check / run: uv run ty check ..

  • CLAUDE.md: add just typecheck to the development commands list.

Verify: just ci → exit 0; grep -n "ty check" justfile .github/workflows/test.yml → 2 matches

Test plan

No new tests. The invariants are: just test passes after every step
(annotations must not change behavior) and uv run ty check . ends at zero.

Done criteria

  • uv run ty check . → 0 diagnostics
  • just ci exits 0 and includes the ty step
  • .github/workflows/test.yml runs ty in the lint job
  • ty version is pinned exactly in pyproject.toml
  • Every inline # ty: ignore carries a reason
  • Final report lists: count fixed vs suppressed per rule, and any
    suspected real bugs found
  • plans/README.md status row updated

STOP conditions

Stop and report back (do not improvise) if:

  • Fixing any diagnostic requires changing runtime behavior (different
    default value, different call arguments, reordered logic) — report the
    site and the suspected bug instead.
  • More than ~15 sites would need inline ignores for a single rule — that
    rule should be configured off globally instead; if the config syntax for
    the installed ty version doesn't support per-rule severity, report.
  • The installed ty version's CLI/config differs materially from what this
    plan assumes (alpha software) — report the actual syntax found before
    proceeding past Step 1.

Maintenance notes

  • ty is pinned; bumping it is a deliberate act that may introduce new
    diagnostics — bump in its own PR.
  • The suppressed-with-reason sites (especially "possible real bug" ones) are
    a ready-made worklist for a future correctness pass.
  • Reviewer should scrutinize: that no diff hunk changes anything except
    annotations, imports of typing names, ignores-with-reasons, and config.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions