Skip to content

ci: add strict mypy type checking - #207

Open
be-student wants to merge 7 commits into
codeforstartups:developmentfrom
be-student:ci/112-add-mypy
Open

be-student wants to merge 7 commits into
codeforstartups:developmentfrom
be-student:ci/112-add-mypy

Conversation

@be-student

@be-student be-student commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

What

  • Add strict MyPy configuration and a dedicated Python 3.12 type-check job.
  • Include strict type checking in make check and make run-ci.
  • Fix the existing internal findings while preserving runtime behavior and real optional-framework inheritance.

Why

The package had no enforced static type-checking gate. Running strict MyPy against the original source reported 135 errors across 29 files, including ambiguous optional values, incomplete collection types, and untyped integration boundaries.

How

  • Enable strict = true for all src/dynavec modules and a checked consumer fixture.
  • Add a named typecheck extra for LangChain, LlamaIndex, and DSPy so CI and the documented make install path check adapters against their real framework bases.
  • Keep real runtime inheritance for all three integrations. DSPy 3.3.1 does not publish py.typed, so a narrow local stub models the public Retrieve(Parameter) state and retrieval API used by Dynavec and the consumer fixture.
  • Preserve EvalRunner's existing handling of extended tuple/list rows and malformed short rows while making the accepted triplet contract precise.
  • Fix internal annotations and optional-state handling without adding inline ignores or per-module strict-mode relaxations.
  • Rebase directly onto current development (e5da8f9) and preserve its cross-encoder reranking, graph deletion, item-size validation, eval-trend, and embedding-cache behavior while resolving the overlapping strict types.

Testing

  • Original strict run: 135 errors across 29 files.
  • Final strict MyPy: success across 55 source files.
  • Python 3.11 full suite: 512 passed, 2 live-AWS tests skipped.
  • Python 3.9 full suite: 509 passed, 3 skipped; 69 existing NumPy numerical warnings remain.
  • Focused affected-area suite for client/reranking, graph deletion, DynamoDB sizing, embedding caching, quantizers, and DSPy: passed.
  • LangChain, LlamaIndex, and DSPy inheritance consumers passed static checks and runtime probes; DSPy retrieval returns real Prediction objects and its state round-trip passed.
  • Hosted CI passed Python 3.9, 3.11, 3.12, and the strict typecheck job on the rebased head.
  • An independent context-free Sol review inspected the exact rebased head and found no actionable defects.

Risks / Impact

Most source changes are annotation precision and explicit narrowing. The rebase resolution was compared with the previous PR range and retains the newly merged runtime features. Local full runtime suites pass in clean Python 3.9 and 3.11 environments; hosted Python 3.12 is also green.

Docs / Follow-ups

Contributor and Makefile help describe the integration extras required for strict type checking.

Implemented and tested with AI assistance, followed by independent context-free Sol review. No human manual or live-AWS testing is claimed.

Closes #112

@codeforstartups

Copy link
Copy Markdown
Owner

Thanks @be-student — this is a valuable type-safety sweep: a mypy CI job, a typecheck extra, and thorough annotations (typed params, S3Payload/DDBPayload/HotPayload aliases, a proper __exit__ signature), with the new mypy job green.

Two things before it can land:

  1. Needs a maintainer UI merge — it edits .github/workflows/ci.yml (the new typecheck job), which our automation token cannot merge (missing workflow scope). @codeforstartups will need to click merge, same as Run the integration suite locally against an AWS emulator #142/ci: add Python 3.13 to CI matrix #158/feat: add DSPy retrieval integration for #68 #195.
  2. Please rebase on development — several PRs merged today that heavily touch client.py, config.py, graph.py, and stores/dynamodb.py (cross-encoder rerank feat(rerank): add cross-encoder reranking #208, graph delete feat(graph): add graph_delete_node / graph_delete_edge (#31) #209, item-size validation feat(dynamodb): validate item size before writing (#25) #211), so this will need a rebase to resolve overlaps and re-run mypy against the current code.

Once rebased, ping me and Ill re-verify the mypy job + full suite before it goes to @codeforstartups for the click. Really nice to see strict typing come in. 🙌

@be-student

Copy link
Copy Markdown
Contributor Author

@codeforstartups Rebased directly onto current development (e5da8f9) as requested and resolved the overlaps with cross-encoder reranking, graph deletion, item-size validation, eval-trend tracking, and embedding caching. The exact rebased head is 33dcdf722c9ece16c4bd0fa115994fca1cf5ae78 with no merge commits.

Exact-head local terminal evidence is attached: make check passed Ruff and strict MyPy across 55 source files, and the Python 3.11 full suite passed 512 tests with two intentional live-AWS skips. Python 3.9 separately passed 509 tests with three skips and 69 existing NumPy numerical warnings. Hosted CI also passed its Python 3.9, 3.11, 3.12, and typecheck jobs. A fresh context-free Sol review found no actionable defects. No live-AWS run is claimed.

Dynavec exact-head terminal evidence

@be-student

Copy link
Copy Markdown
Contributor Author

Rebased this branch onto current development (ad114b2) and resolved overlaps in the CLI export/import flow, client, hot-tier eviction, and retrieval tool integrations while preserving the newly added upstream behavior.

The new head is 27b90137e66d160a0cf8e0680c9f185d84b85473. make check passes Ruff and strict MyPy across 56 source files. Focused CLI, hot-tier, retriever, OpenAI tool, CrewAI tool, and export/import tests pass (103 tests). No live-AWS testing is claimed.

@codeforstartups codeforstartups left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really well done, @be-student — and thanks for rebasing onto current development. This is a disciplined strict-mypy adoption: a dedicated typecheck CI job, zero # type: ignore (real annotations throughout, refined aliases like RescoreSpec = dict[str, float], dspy stubs under typings/), and no runtime-logic changes. CI green (4/4).

Two notes:

  1. This is a maintainer policy decision, @codeforstartups — merging it means every future PR must pass strict mypy in CI. Given the contributor churn weve had, that raises the bar (good for quality, slightly higher for newcomers). Your call.
  2. I cant merge it from automation — it edits .github/workflows/ci.yml (the new typecheck job), which needs the workflow OAuth scope my token lacks. Its a clean one-click UI merge once you decide to adopt it.

Approved on quality — the adoption decision + merge are yours. 🙌

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add mypy to CI

2 participants