Skip to content

ci: add strict mypy type checking - #207

Merged
Kaap10 merged 11 commits into
codeforstartups:developmentfrom
be-student:ci/112-add-mypy
Sep 26, 2026
Merged

Kaap10 merged 11 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

This adds strict MyPy checking to the package, make check / make run-ci, and a dedicated Python 3.12 CI job. The typecheck extra installs the real LangChain, LlamaIndex, DSPy, and Haystack adapters; only the untyped DSPy boundary uses narrow local stubs.

The branch now includes current development through acfd80e at head 43c108a2b9938b7a913579fd2e7f98fa448a5ef2. The merge preserves namespace cache invalidation, optimistic DynamoDB updates, graph shortest paths, OpenAI retry handling, and the new namespace batch-search API while retaining strict annotations.

Namespace search_many overloads describe plain, explained, and runtime-Boolean result shapes. Consumer fixtures and runtime tests cover return types, namespace isolation, input ordering, explained results, and empty batches. OpenAI retries forward the same typed request parameters on every attempt; the actual Haystack dependency supplies typed component decorators. Contributor documentation records the typecheck dependencies and consumer checks.

Earlier contract corrections remain: direct and batch explained-search result shapes agree with runtime behavior, namespace searches accept dynamic Boolean flags, and CachedEmbedder.dimension delegates to the inner embedder rather than retaining a stale snapshot.

Validation at 43c108a on macOS/Python 3.11.15:

  • Ruff passes; strict MyPy reports no issues across 58 source files and the consumer fixtures.
  • Full offline suite: 714 passed, 2 live-AWS tests skipped. Two warnings report the optional SciPy fallback; no live AWS test is claimed.
  • Separate Sol agents checked cache invalidation, namespace consumer/runtime cases, OpenAI retries, Haystack typing, and preservation of upstream merge behavior.
  • The PR-specific diff against current development passes whitespace checks. Upstream's unrelated tracked files and existing whitespace were preserved rather than silently cleaned up.
  • A fresh Sol verifier independently passed 111 focused tests and strict MyPy, finding no actionable regression in the final commit.
  • All four hosted CI checks pass on the new head.

OpenAI Codex and Sol subagents assisted with implementation, integration review, and automated tests. No human-review or manual-testing claim is made. Maintainer adoption and merge remain pending.

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. 🙌

@be-student

Copy link
Copy Markdown
Contributor Author

Current development advanced again, so I merged it into the strict-mypy branch and resolved the overlaps in the client, namespace proxy, and LlamaIndex adapter. The resolution preserves the new explained-search return shape and filter translation while keeping the PR's typed API. The new head is ff15ffe689a97a99541effdb3f58ee3532728a79, and GitHub now reports the PR mergeable.

make check passed Ruff and strict MyPy across 56 source files. The local suite passed with the optional LlamaIndex module excluded; that module could not be collected on this Mac because its environment's Pillow native binary was blocked by system code-signing policy. Focused client, namespace, retriever, integration, and MCP tests passed. I am not claiming a local LlamaIndex run; hosted CI is checking the complete branch.

@be-student

Copy link
Copy Markdown
Contributor Author

An adversarial AI-assisted review of the merge resolution found three concrete contract defects, fixed in 852c6802021908027374fdfbc4d144a3e85f506f:

  1. search_many(explain=True) returned explained objects while its annotation promised nested result lists. Precise overloads and typed executor callbacks replace the unsound callable cast.
  2. Namespace search rejected explain supplied as a runtime bool; its fallback overload now matches direct search.
  3. CachedEmbedder.dimension had become a snapshot, so Ollama custom-model inference could leave it at 768 after the underlying embedder discovered another dimension. Live delegation is restored.

The first two failures were reproduced with strict consumer checks, and the third with failing Ollama adapter regressions before the fix. The new head passes Ruff, strict MyPy (57 source files), and the full Python 3.12 offline suite: 636 passed, 2 live-AWS skips, with 2 optional-SciPy fallback warnings. This includes all 15 real LlamaIndex filter tests; reinstalling the same Pillow version repaired the previous local collection failure. Hosted CI is rerunning. No live-AWS or human manual testing is claimed.

# Conflicts:
#	src/dynavec/client.py
#	src/dynavec/stores/dynamodb.py
# Conflicts:
#	src/dynavec/cache.py
#	src/dynavec/client.py
#	src/dynavec/embeddings/bedrock.py
#	src/dynavec/namespace.py
@be-student

Copy link
Copy Markdown
Contributor Author

Resolved the new upstream conflict in 43c108a by merging current development and preserving its cache invalidation, optimistic DynamoDB updates, graph path, retry, and namespace batch-search changes. The strict gate now checks the new paths too, with real Haystack typecheck dependencies and consumer/runtime coverage for namespace batch results. Local validation passes: Ruff, strict MyPy across 58 source files, and 714 offline tests (2 live-AWS skips). GitHub confirms this head is mergeable; its four new CI jobs are running. The PR description now records the exact merge and validation.

@Kaap10 Kaap10 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you @be-student for the comprehensive strict MyPy integration across the entire codebase, the thorough type annotations, and for maintaining the branch against recent upstream changes. All CI checks and typecheck suites look solid. Merging now.

@Kaap10
Kaap10 merged commit 9ce96d8 into codeforstartups:development Sep 26, 2026
4 checks passed
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

4 participants