Skip to content

ADR-338: Wire mypy --strict static analysis into ml/ build#244

Open
jodavis-claude wants to merge 3 commits into
feature/ADR-191-ml-pipelinefrom
dev/claude/ADR-338-mypy-strict-analysis
Open

ADR-338: Wire mypy --strict static analysis into ml/ build#244
jodavis-claude wants to merge 3 commits into
feature/ADR-191-ml-pipelinefrom
dev/claude/ADR-338-mypy-strict-analysis

Conversation

@jodavis-claude

Copy link
Copy Markdown
Collaborator

Work item

ADR-338 — Task 1 of the OOP ML pipeline epic (ADR-191). Wires mypy --strict static analysis into the ml/ build so every subsequent pipeline task is written strict-clean from the start instead of needing a later cleanup pass.

Changes

  • ml/requirements.txt — added mypy==2.3.0 (current stable release at implementation time).
  • ml/pyproject.toml (new) — [tool.mypy] section with strict = true and python_version = "3.12". No blanket ignore_missing_imports suppression, so later tasks handle each untyped import (tensorflow/librosa/edge_tts) explicitly as the spec expects.
  • ml/pipeline/__init__.py and ml/test/__init__.py (new, empty) — package markers so mypy --strict pipeline test has files to scan; without them mypy exits non-zero with "no .py[i] files".
  • scripts/validate-ml-build.sh / .cmd (new) — cds into ml/ and runs mypy --strict pipeline test, modeled on the existing validate-build.sh/.cmd structure and naming convention.
  • scripts/validate.sh / .cmd (modified) — added a call to validate-ml-build between validate-build and validate-tests.
  • CLAUDE.md (modified) — Quality Gates table: added a new row for scripts/validate-ml-build, renumbering the existing rows.
  • Directory.Build.props (modified) — added EnableSourceControlManagerQueries=false to fix a build failure in this worktree checkout (SDK-pinned SourceLink task errors on the extensions.relativeworktrees git config set by newer git versions on worktree checkouts; fixed upstream only in a later SDK line not present here). Verified Nerdbank.GitVersioning still resolves assembly version info correctly since it queries git independently of SourceLink.

Design decisions

  • Left ml/pyproject.toml minimal (strict + python_version only) rather than pre-adding [[tool.mypy.overrides]] for tensorflow/librosa/edge_tts — consistent with the spec's description of later tasks handling each untyped import as it's introduced.
  • Did not touch .github/workflows/build-and-test.yml — exit criteria only name scripts/validate.sh/.cmd and CLAUDE.md, not CI wiring.
  • No unit tests or E2E scenarios added — this task's Components in scope is explicitly empty (pure tooling/scaffolding with no app-facing observable behavior).

Testing completed

  • scripts/validate-build.sh (dotnet build /warnaserror) — clean, 312 unit tests passing.
  • scripts/validate-ml-build.sh (mypy --strict pipeline test) — passes cleanly against the package-marker-only tree.
  • Manual sanity check: temporarily added an untyped function to ml/pipeline/, confirmed mypy --strict flagged it (no-untyped-def), then removed it.

jodavis added 3 commits July 25, 2026 03:40
Adds mypy==2.3.0 to ml/requirements.txt and a greenfield ml/pyproject.toml
with [tool.mypy] strict = true (python_version 3.12, no blanket
ignore_missing_imports so later tasks handle untyped imports explicitly
per-import). Adds scripts/validate-ml-build.sh/.cmd, modeled on
validate-build.sh/.cmd, running mypy --strict pipeline test from ml/.
Wires it into scripts/validate.sh/.cmd after validate-build, and lists it
in CLAUDE.md's Quality Gates table. Adds empty ml/pipeline/__init__.py and
ml/test/__init__.py package markers so the strict-mode scan has files to
check against an otherwise-empty tree.
core.filemode=false in this container caused git add to record the new
script as 100644 despite the chmod +x on disk; explicitly set it to match
validate-build.sh's mode.
…ktree

The .NET SDK's bundled Microsoft.Build.Tasks.Git.targets fails with
"Unsupported repository extension 'relativeworktrees'" when building from
this worktree, because newer git versions set that extension on worktree
checkouts and the SDK-pinned task (fixed upstream only in the 10.0.2xx SDK
line, see dotnet/sourcelink#1468) does not recognize it. This broke
scripts/validate-build.sh for every project, unrelated to any ADR-338
source change.

Disabling EnableSourceControlManagerQueries in Directory.Build.props skips
the failing git query; Nerdbank.GitVersioning computes assembly version
info independently and is unaffected (verified ThisAssembly.GitCommitId is
still generated correctly).
@github-actions

Copy link
Copy Markdown

Test Results

401 tests  ±0   401 ✅ ±0   1m 58s ⏱️ -1s
  5 suites ±0     0 💤 ±0 
  5 files   ±0     0 ❌ ±0 

Results for commit 029247b. ± Comparison against base commit c4261cd.

@jodavis-claude jodavis-claude left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verified all six exit criteria: mypy pinned in ml/requirements.txt (confirmed 2.3.0 is genuinely the current latest release via pip index versions mypy in this environment), [tool.mypy] strict config in ml/pyproject.toml (no blanket ignore_missing_imports, correctly deferring per-import handling to later tasks), scripts/validate-ml-build.sh/.cmd added and modeled closely on the existing validate-build pair (including matching executable-bit fix), scripts/validate.sh/.cmd wired in after validate-build, CLAUDE.md Quality Gates table updated, and the script passes cleanly against the package-marker-only tree (re-ran it directly: Success: no issues found in 2 source files). Also re-ran dotnet build /warnaserror clean (0 warnings/errors) to confirm the Directory.Build.props fix doesn't regress the main build, and confirmed Nerdbank.GitVersioning's ThisAssembly.GitCommitId still resolves correctly with EnableSourceControlManagerQueries disabled. One non-blocking scope note left inline. No Priority 1-4 issues found.

Comment thread Directory.Build.props
@jodavis-claude
jodavis-claude marked this pull request as ready for review July 25, 2026 04:28
@jodavis-claude
jodavis-claude requested a review from jodavis July 25, 2026 04:40

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.

This should be added to the CI workflow. (The spec said so at one point, that must have been removed.)

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.

2 participants