Merge pull request #9 from moonrunnerkc/maintainability-improvements

Maintainability: enforce lint/types in CI, split cli.py, measure coverage

Author: moonrunnerkc

.github/workflows/ci.yml

@@ -44,8 +44,14 @@ jobs:
         with:
           python-version: "3.12"
 
-      - name: Compile check (all modules)
-        run: python -m compileall -q src/skillcheck
+      - name: Install dev dependencies
+        run: pip install -e ".[dev]"
+
+      - name: Ruff check
+        run: ruff check src tests
+
+      - name: Mypy strict
+        run: mypy src/skillcheck
 
   package:
     runs-on: ubuntu-latest

.gitignore

@@ -7,9 +7,12 @@ dist/
 build/
 .pytest_cache/
 .ruff_cache/
+.mypy_cache/
+.coverage
+.coverage.*
+htmlcov/
 *.egg
 CLAUDE.md
-copilot-instructions.md
 # Field test artifacts (kept locally, not in git)
 runs/
 

CHANGELOG.md

@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `[tool.ruff]` and `[tool.mypy]` configuration in `pyproject.toml`. Ruff lints `src` and `tests` with an explicit `E, F, I, UP, B` selection at line length 127 (`E501` ignored for unavoidable long string literals in JSON fixtures and schema text). Mypy runs `strict` against `src/skillcheck` under `python_version = "3.10"`, with an `ignore_missing_imports` override for the untyped `tiktoken` dependency and the `tomllib` 3.11+ stdlib backport branch. The source was brought clean under both with real annotations, not blanket ignores.
+- Test coverage measurement via `pytest-cov` (added to the `dev` extra). `[tool.coverage.run]` and `[tool.coverage.report]` configure the run, and `addopts` in `[tool.pytest.ini_options]` adds `--cov=skillcheck --cov-report=term-missing --cov-fail-under=68`, so the floor is enforced on every local run and in CI (which runs the same `pytest`). The floor sits a few points under the ~72% measured on CPython 3.10 to absorb matrix variance: the CLI modules run in subprocesses the in-process tracer does not see, the `tomllib` vs fallback-parser branch flips between Python 3.10 and 3.11+, and a few tests skip on Windows.
+
+### Changed
+
+- CI `lint` job now enforces `ruff check src tests` and `mypy src/skillcheck` (strict) on every push and pull request, replacing the prior `compileall`-only check. Either tool reporting a finding fails the job.
+- `cli.py` split to separate argument wiring from command execution. Parser construction, `skillcheck.toml` application, mode-conflict dispatch, and `main` stay in `cli.py`; the per-mode handlers (emit prompts and graphs, `--show-history`, the default validation pipeline) and the path and ingest IO helpers move to a new `skillcheck.commands` module. `skillcheck.cli:main` and the `skillcheck` console script are unchanged. Pure refactor, no behavior change.
+
 ## [1.4.0] - 2026-05-27
 
 ### Changed

pyproject.toml

@@ -33,6 +33,7 @@ classifiers = [
 dev = [
     "mypy>=1.10",
     "pytest>=8.0",
+    "pytest-cov>=5.0",
     "ruff>=0.6",
     "types-PyYAML>=6.0",
 ]
@@ -48,3 +49,41 @@ packages = ["src/skillcheck"]
 
 [tool.pytest.ini_options]
 testpaths = ["tests"]
+# Coverage floor sits a few points under the ~72% measured on CPython 3.10 so it
+# survives matrix variance: the CLI modules (cli.py, commands.py) run in
+# subprocesses the in-process tracer does not see, the tomllib vs fallback-parser
+# branch flips between 3.10 and 3.11+, and a few tests skip on Windows.
+addopts = "--cov=skillcheck --cov-report=term-missing --cov-fail-under=68"
+
+[tool.ruff]
+target-version = "py310"
+line-length = 127
+
+[tool.ruff.lint]
+select = ["E", "F", "I", "UP", "B"]
+ignore = ["E501"]  # line-length in unchangeable string literals (JSON fixtures, base64, schema text)
+
+[tool.mypy]
+strict = true
+python_version = "3.10"
+files = ["src/skillcheck"]
+
+[[tool.mypy.overrides]]
+# tiktoken ships no type stubs; tomllib has no typeshed stubs under the pinned
+# python_version (3.10), where it is the optional 3.11+ stdlib backport branch.
+module = ["tiktoken", "tomllib"]
+ignore_missing_imports = true
+
+[tool.coverage.run]
+source = ["skillcheck"]
+# __main__.py is the `python -m skillcheck` shim; it only runs in a subprocess,
+# so the in-process test run never executes it.
+omit = ["*/__main__.py"]
+
+[tool.coverage.report]
+show_missing = true
+exclude_lines = [
+    "pragma: no cover",
+    "if __name__ == .__main__.:",
+    "raise NotImplementedError",
+]

src/skillcheck/agents/codex.py

@@ -18,10 +18,10 @@
 
 from skillcheck.agents.base import (
     SCHEMA_VERSION,
+    SelfCritiquePrompt,
     schema_reference,
     worked_example,
 )
-from skillcheck.agents.base import SelfCritiquePrompt
 from skillcheck.parser import ParsedSkill
 
 

src/skillcheck/agents/cursor.py

@@ -26,9 +26,9 @@
 
 from skillcheck.agents.base import (
     SCHEMA_VERSION,
+    SelfCritiquePrompt,
     compact_schema_signature,
 )
-from skillcheck.agents.base import SelfCritiquePrompt
 from skillcheck.parser import ParsedSkill
 
 

src/skillcheck/agents/graph_parser.py

@@ -11,6 +11,7 @@
 from __future__ import annotations
 
 import json
+from collections.abc import Mapping
 from typing import Literal
 
 from skillcheck.agents._response_text import strip_response_noise
@@ -102,7 +103,7 @@ class GraphValueError(GraphParseError):
 
 
 def _require_field(
-    obj: dict,
+    obj: dict[str, object],
     key: str,
     expected_type: type | tuple[type, ...],
     context: str = "",
@@ -140,7 +141,7 @@ def _require_field(
         raise GraphSchemaError(
             f"Field '{full_key}' must be int or null, got bool: {value!r}"
         )
-    if not isinstance(value, expected_type):  # type: ignore[arg-type]
+    if not isinstance(value, expected_type):
         if isinstance(expected_type, tuple):
             type_name = " or ".join(
                 "null" if t is type(None) else t.__name__ for t in expected_type
@@ -154,7 +155,7 @@ def _require_field(
     return value
 
 
-def _check_no_extra_fields(obj: dict, allowed: dict, context: str) -> None:
+def _check_no_extra_fields(obj: Mapping[str, object], allowed: Mapping[str, object], context: str) -> None:
     extra = set(obj) - set(allowed)
     if extra:
         raise GraphSchemaError(

src/skillcheck/agents/parser.py

@@ -73,7 +73,7 @@ class CritiqueValueError(CritiqueParseError):
     """
 
 
-def _require_field(obj: dict, key: str, expected_type: type | tuple[type, ...], context: str = "") -> object:
+def _require_field(obj: dict[str, object], key: str, expected_type: type | tuple[type, ...], context: str = "") -> object:
     """Extract a required field with type checking.
 
     Args:
@@ -98,7 +98,7 @@ def _require_field(obj: dict, key: str, expected_type: type | tuple[type, ...],
         raise CritiqueSchemaError(
             f"Field '{full_key}' must be int, got bool: {value!r}"
         )
-    if not isinstance(value, expected_type):  # type: ignore[arg-type]
+    if not isinstance(value, expected_type):
         type_name = (
             expected_type.__name__
             if isinstance(expected_type, type)