From 589a6b3d59ce272cbf9fed25ce3126e40cb9bb05 Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 14:07:17 +0200 Subject: [PATCH 1/7] feat(precommit): pluggable step framework + doctest/typecheck/doc-consistency steps [tool.forge.precommit] enable/disable + --only/--skip over a step registry (#6) Opt-in doctest (#5), typecheck (pyrefly default, #48), and doc_consistency + verify-forge-doc-consistency CLI (#4) CONFIG_KEYS couples registry to its readers (#46) Bump plugin.json to 1.24.0 --- .claude-plugin/plugin.json | 2 +- CHANGELOG.md | 24 ++ REPO_STRUCTURE.md | 2 + docs/api-digest.md | 23 +- docs/cli-reference.md | 26 +- docs/configuration.md | 40 +++ pyproject.toml | 1 + src/forge/forge_config.py | 40 +++ src/forge/precommit.py | 386 +++++++++++++++++++++++---- src/forge/verify_doc_consistency.py | 178 ++++++++++++ tests/test_forge_config.py | 68 ++++- tests/test_precommit.py | 341 +++++++++++++++++++++++ tests/test_verify_doc_consistency.py | 134 ++++++++++ 13 files changed, 1206 insertions(+), 59 deletions(-) create mode 100644 src/forge/verify_doc_consistency.py create mode 100644 tests/test_verify_doc_consistency.py diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index b14009c..92840e0 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "forge", - "version": "1.23.2", + "version": "1.24.0", "description": "Automate Python CI/CD and code-quality standards — deterministic CLIs + a drop-in pre-commit hook, runnable with or without an AI agent. This optional Claude Code plugin orchestrates them; the gate is the CLI, never the model.", "author": { "name": "Jean Simonnet", diff --git a/CHANGELOG.md b/CHANGELOG.md index 9af9816..61648ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,30 @@ change groups by conventional-commit type (**Features / Fixes / Refactor Follows [Keep a Changelog](https://keepachangelog.com/) in spirit; versions follow forge's rolling-next convention. +## v1.24.0 — 2026-06-17 + +All additive and opt-in — no consumer action required to upgrade. + +### Features +- **Pluggable pre-commit step framework** — `[tool.forge.precommit] + enable` / `disable` (plus `forge-precommit --only` / `--skip`) turn any + step on or off uniformly, on top of each step's own self-skip (#6). +- **Opt-in `doctest` step** — `pytest --doctest-modules` over + `[tool.forge.doctest].paths` (default `["src"]`); non-blocking by + default (#5). +- **Opt-in `typecheck` step** — `[tool.forge.typecheck].checker` + (`pyrefly` default; `mypy` / `ty` / `pyright` / `none`); non-blocking by + default (#48). +- **Opt-in `doc_consistency` step** + `verify-forge-doc-consistency` CLI — + checks CLI name-lists and agent counts asserted in docs against the + actual repo state; non-blocking (#4). + +### Tooling +- `forge-config --list` now enumerates the new + `[tool.forge.precommit/doctest/typecheck]` keys, and a drift test + couples `CONFIG_KEYS` to its readers so the registry can't silently go + stale (#46). + ## v1.23.0 — 2026-06-17 ### Features diff --git a/REPO_STRUCTURE.md b/REPO_STRUCTURE.md index f714aa8..68463be 100644 --- a/REPO_STRUCTURE.md +++ b/REPO_STRUCTURE.md @@ -37,6 +37,7 @@ Code. structure drift check - verify_test_naming.py: `verify-forge-test-naming` — test naming check - verify_manifest.py: `verify-forge-manifest` — `.claude-plugin/*.json` JSON validation + - verify_doc_consistency.py: `verify-forge-doc-consistency` — checks machine-checkable doc claims (CLI name-lists, agent counts) against repo state; backs the opt-in `doc_consistency` pre-commit step (non-blocking) - verify_plugin_version.py: `verify-forge-plugin-version` — rolling-next guard (plugin.json["version"] > latest git tag) - gen_cli_reference.py: `forge-gen-cli-reference` — CLI reference doc generator @@ -159,6 +160,7 @@ Pytest suite mirroring the `src/forge/` layout: - test_verify_docstrings.py: tests for verify_docstrings - test_verify_docstring_coverage.py: tests for verify_docstring_coverage - test_verify_manifest.py: tests for verify_manifest + - test_verify_doc_consistency.py: tests for verify_doc_consistency - test_verify_plugin_version.py: tests for verify_plugin_version - test_verify_repo_structure.py: tests for verify_repo_structure - test_verify_test_naming.py: tests for verify_test_naming diff --git a/docs/api-digest.md b/docs/api-digest.md index 74adb2e..79d39b6 100644 --- a/docs/api-digest.md +++ b/docs/api-digest.md @@ -4,7 +4,7 @@ A compact index of this codebase's symbols — every top-level function and clas > **Generated file — do not edit by hand.** Regenerate with `forge-gen-api-digest`; check for drift with `forge-gen-api-digest --check`. -_40 modules, 362 symbols._ +_41 modules, 374 symbols._ ## `forge._hook_helpers` @@ -158,7 +158,7 @@ _40 modules, 362 symbols._ ## `forge.config` -- `class ForgeConfig` — Branch-name configuration sourced from ``[tool.forge]``. +- `class ForgeConfig` — Repo configuration sourced from ``[tool.forge]``. - `dual_track(self) -> bool` — Return ``True`` when base and dev are distinct branches. - `read_pyproject_raw(repo_root: Path) -> dict` — Return the full parsed ``pyproject.toml`` dict, or ``{}`` on failure. - `load_config(repo_root: Path) -> ForgeConfig` — Read ``[tool.forge]`` from *repo_root*'s ``pyproject.toml``. @@ -326,6 +326,7 @@ _40 modules, 362 symbols._ - `_read_plugin_version_at_ref(repo_root: Path, ref: str) -> str | None` _(internal)_ — Return ``plugin.json["version"]`` at the given git ref, or ``None`` when absent. - `_check_promote_pending_message(repo_root: Path, dev_branch: str, base_branch: str) -> str | None` _(internal)_ — Return a one-line user-facing prompt when promotion is pending, else ``None``. +- `_changelog_lacks_entry(changelog_text: str, minor_tag: str) -> bool` _(internal)_ — Return True when *changelog_text* has no ``## `` heading. - `_promotion_status_lines(repo_root: Path, dev_branch: str, base_branch: str) -> list[str]` _(internal)_ — Build the read-only promotion-status report. - `_git(*args: str, cwd: Path | None = None, check: bool = True) -> str` _(internal)_ — Run ``git`` with *args*, return stripped stdout. - `_read_plugin_version(repo_root: Path) -> str | None` _(internal)_ — Return ``.claude-plugin/plugin.json["version"]`` or ``None`` if absent. @@ -369,6 +370,8 @@ _40 modules, 362 symbols._ - `_color(code: str) -> str` _(internal)_ — Return *code* if stdout is a TTY, else an empty string. - `class StepResult` — Outcome of a single pre-commit step. +- `class StepDef` — A registry entry: a step's name, its function, and whether it runs by default. +- `_forge_step_config(repo_root: Path, step: str) -> dict` _(internal)_ — Return the ``[tool.forge.]`` table, or ``{}`` when absent. - `_run(cmd: list[str], cwd: Path) -> tuple[bool, str]` _(internal)_ — Run *cmd* and capture combined output. - `step_ruff(repo_root: Path) -> StepResult` — Run ``fix-forge-ruff`` — owns the ruff phase end-to-end. - `step_docstrings(repo_root: Path) -> StepResult` — Run ``verify-forge-docstrings`` over the current diff vs main. @@ -382,9 +385,15 @@ _40 modules, 362 symbols._ - `step_cli_wiring(repo_root: Path) -> StepResult` — Run ``verify-forge-cli-wiring`` — assert every script has a real caller. - `_cli_wiring_enabled(repo_root: Path) -> bool` _(internal)_ — Return True when the repo has opted into the cli_wiring check. - `step_plugin_version(repo_root: Path) -> StepResult` — Run ``verify-forge-plugin-version`` — owns the rolling-next guard. +- `step_doctest(repo_root: Path) -> StepResult` — Run ``pytest --doctest-modules`` over docstring examples (opt-in). +- `step_typecheck(repo_root: Path) -> StepResult` — Run a configurable static type checker (opt-in). +- `step_doc_consistency(repo_root: Path) -> StepResult` — Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). - `_write_log(repo_root: Path, result: StepResult) -> None` _(internal)_ — Persist *result*'s output to ``code_health/.log``. - `_print_step_line(result: StepResult) -> None` _(internal)_ — Print a one-line status for *result* (SKIP/PASS/WARN/FAIL). -- `run_all(repo_root: Path | None = None, *, print_progress: bool = True) -> list[StepResult]` — Run every step in order and return their results. +- `_validate_step_names(names: Sequence[str]) -> None` _(internal)_ — Raise ``ValueError`` listing any *names* that are not registered steps. +- `_resolve_steps(repo_root: Path, *, skip: Sequence[str] = (), only: Sequence[str] = ()) -> list[StepDef]` _(internal)_ — Resolve which steps to run, in registry order. +- `run_all(repo_root: Path | None = None, *, print_progress: bool = True, skip: Sequence[str] = (), only: Sequence[str] = ()) -> list[StepResult]` — Run the resolved step sequence in order and return their results. +- `_split_csv(values: Sequence[str]) -> list[str]` _(internal)_ — Flatten repeatable / comma-separated CLI values into a clean name list. - `main() -> int` — CLI entry point. ## `forge.run_context` @@ -432,6 +441,12 @@ _40 modules, 362 symbols._ - `_check_wiring(root: Path) -> int` _(internal)_ — Run the reachability check and report findings. - `main() -> int` — Entry point for ``verify-forge-cli-wiring``. +## `forge.verify_doc_consistency` + +- `_check_cli_coverage(repo_root: Path) -> list[str]` _(internal)_ — Return findings for ``[project.scripts]`` names missing from the CLI reference. +- `_check_agent_count(repo_root: Path) -> list[str]` _(internal)_ — Return a finding when FOUNDATION's agent-count claim disagrees with disk. +- `main() -> int` — CLI entry point. + ## `forge.verify_docstring_coverage` - `_interrogate_config(data: dict) -> tuple[InterrogateConfig, float, list[str]]` _(internal)_ — Build the interrogate config + threshold + excludes from TOML data. @@ -464,7 +479,7 @@ _40 modules, 362 symbols._ ## `forge.verify_plugin_version` -- `_is_release_commit(repo_root: Path, tag: str) -> bool` _(internal)_ — Return True when ``HEAD`` carries the same file content as *tag*. +- `_is_release_commit(repo_root: Path) -> bool` _(internal)_ — Return True when ``HEAD``'s tree reproduces ANY published ``v*`` tag. - `main() -> int` — Enforce plugin.json version > latest git tag. ## `forge.verify_repo_structure` diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 9bf1f36..a64f266 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -375,7 +375,8 @@ options: ## forge-precommit ```text -usage: forge-precommit [-h] [--json] +usage: forge-precommit [-h] [--json] [--skip STEP[,STEP...]] + [--only STEP[,STEP...]] Run the forge pre-commit check sequence: ruff (format + check, self-healing with --unsafe-fixes on failure) + docstring verification (diff vs main) + @@ -387,8 +388,14 @@ default sequence — run it in CI or wire it into .githooks/pre-commit explicitly. Used by any repo that adopts forge via install-forge-githooks. options: - -h, --help show this help message and exit - --json Emit a JSON summary on stdout instead of human output. + -h, --help show this help message and exit + --json Emit a JSON summary on stdout instead of human output. + --skip STEP[,STEP...] + Force-skip these steps for this run (repeatable or + comma-separated). + --only STEP[,STEP...] + Run exactly these steps (repeatable or comma- + separated). ``` ## forge-slow-tests-report @@ -527,6 +534,19 @@ options: -h, --help show this help message and exit ``` +## verify-forge-doc-consistency + +```text +usage: verify-forge-doc-consistency [-h] + +Check machine-checkable documentation claims (CLI name-lists, agent counts) +against the actual repo state. Non-blocking reporter for the doc_consistency +pre-commit step. + +options: + -h, --help show this help message and exit +``` + ## verify-forge-docstring-coverage ```text diff --git a/docs/configuration.md b/docs/configuration.md index 4de6ca7..e3ca15e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -62,6 +62,46 @@ to see what, if anything, is worth adding for your repo. |---|---|---|---| | `enabled` | `false` | Opt into the `cli_wiring` pre-commit step: every `[project.scripts]` entry must be reachable from a wiring source (install bootstrap, pre-commit, audit, hooks, agents, skills…). | Your repo ships `[project.scripts]` and follows forge's layout and you want unreachable CLIs caught. | +## `[tool.forge.precommit]` — turn steps on and off + +The uniform lever over the pre-commit sequence. Applied on top of each +step's own self-skip (it never *weakens* a self-skip), and `disable` beats +`enable` when a name appears in both. The same effect for a single run is +available as `forge-precommit --only ` / `--skip `. + +| Key | Default | What it does | Set it when | +|---|---|---|---| +| `disable` | `[]` | Force-skip these default steps by name (e.g. `["pip_audit"]`). | You want a default step off repo-wide. | +| `enable` | `[]` | Opt into normally-off steps by name: `doctest`, `typecheck`, `doc_consistency`. | You want one of the opt-in steps below to run. | + +## `[tool.forge.doctest]` — opt-in doctest step + +Runs `pytest --doctest-modules` so docstring `>>>` examples are executed, +not just present. Enable via `[tool.forge.precommit] enable = ["doctest"]`. + +| Key | Default | What it does | Set it when | +|---|---|---|---| +| `paths` | `["src"]` | Scan roots for `--doctest-modules`. | Your doctested code lives elsewhere. | +| `blocking` | `false` | Fail the commit on a broken example (else non-blocking WARN). | You want doctest drift to refuse a commit. | + +## `[tool.forge.typecheck]` — opt-in type-check step + +Runs a configurable static type checker. Enable via +`[tool.forge.precommit] enable = ["typecheck"]`. When enabled but the +chosen checker binary is absent, the step fails loudly (it does not +silently pass). Non-blocking by default — a type-checker false positive +that refuses a commit trains `--no-verify`. + +| Key | Default | What it does | Set it when | +|---|---|---|---| +| `checker` | `pyrefly` | `pyrefly` (Rust, stable, reads/migrates `[tool.mypy]`), `mypy`, `ty`, `pyright`, or `none` (skip). | You prefer a specific checker. `pyrefly` is the recommended fast default. | +| `paths` | `["src"]` | Scan roots passed to the checker. | Your source lives elsewhere. | +| `blocking` | `false` | Fail the commit on a checker error (else non-blocking WARN). | Your type baseline is clean and you want it enforced. | + +The `doc_consistency` step (`verify-forge-doc-consistency`, enabled the +same way) has no config table — it checks CLI name-lists and agent counts +in docs against repo state, and is always non-blocking. + ## `[tool.forge.docstring_coverage]` Forge-specific keys for the docstring-coverage reporter. (The coverage *gate* diff --git a/pyproject.toml b/pyproject.toml index bc4173a..bd60696 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,7 @@ verify-forge-repo-structure = "forge.verify_repo_structure:main" install-forge-labels = "forge.install_labels:main" verify-forge-test-naming = "forge.verify_test_naming:main" verify-forge-cli-wiring = "forge.verify_cli_wiring:main" +verify-forge-doc-consistency = "forge.verify_doc_consistency:main" forge-gen-cli-reference = "forge.gen_cli_reference:main" forge-gen-api-digest = "forge.gen_api_digest:main" forge-gen-commit-types = "forge.gen_commit_types:main" diff --git a/src/forge/forge_config.py b/src/forge/forge_config.py index 75e72b7..d0961f4 100644 --- a/src/forge/forge_config.py +++ b/src/forge/forge_config.py @@ -101,6 +101,46 @@ class ConfigKey: "Per-tool override of the coverage scan roots; otherwise inherits " "the repo-wide [tool.forge].source_dirs + test_dirs.", ), + ConfigKey( + ("tool", "forge", "precommit", "disable"), + default=[], + description="Force-skip these pre-commit steps by name (over each " + "step's own self-skip).", + ), + ConfigKey( + ("tool", "forge", "precommit", "enable"), + default=[], + description="Opt into normally-off pre-commit steps by name " + "(doctest, typecheck, doc_consistency).", + ), + ConfigKey( + ("tool", "forge", "doctest", "paths"), + ["src"], + "Scan roots for the opt-in doctest step (pytest --doctest-modules).", + ), + ConfigKey( + ("tool", "forge", "doctest", "blocking"), + default=False, + description="Make the doctest step fail the commit on a broken " + "example (default: non-blocking WARN).", + ), + ConfigKey( + ("tool", "forge", "typecheck", "checker"), + "pyrefly", + "Type checker for the opt-in typecheck step: pyrefly | mypy | ty | " + "pyright | none.", + ), + ConfigKey( + ("tool", "forge", "typecheck", "paths"), + ["src"], + "Scan roots for the opt-in typecheck step.", + ), + ConfigKey( + ("tool", "forge", "typecheck", "blocking"), + default=False, + description="Make the typecheck step fail the commit on a checker " + "error (default: non-blocking WARN).", + ), ) # Third-party tools forge reads from their OWN native section rather than diff --git a/src/forge/precommit.py b/src/forge/precommit.py index eebdb1a..a6b1cce 100644 --- a/src/forge/precommit.py +++ b/src/forge/precommit.py @@ -1,21 +1,32 @@ """forge-precommit — pre-commit dispatcher CLI. Generic Python pre-commit check dispatcher. Auto-detects source -directories and runs a fixed sequence on every commit: ruff (always -``ruff format`` + ``ruff check --fix --unsafe-fixes`` in-place, with -modified tracked files re-staged via ``git add``), docstring -verification (over the diff vs main), test-name verification (over the -diff vs main), repo-structure verification (``REPO_STRUCTURE.md`` vs -the actual tree), plugin manifest JSON validation, Claude Code -plugin-version drift guard (when applicable), and ``pip-audit`` -dependency vulnerability scan (non-blocking — warns but does not refuse -a commit). Shipped to consumers via the ``forge-scripts`` pip package -and invoked by ``.githooks/pre-commit`` after ``install-forge-githooks``. - -Pytest is intentionally NOT part of the default sequence — it is too -slow for pre-commit and belongs in CI. Consumers that want it on commit -can call ``pytest`` directly in ``.githooks/pre-commit`` around the -``forge-precommit`` invocation. +directories and runs an ordered sequence on every commit. The default +sequence: ruff (always ``ruff format`` + ``ruff check --fix +--unsafe-fixes`` in-place, with modified tracked files re-staged via +``git add``), docstring verification (over the diff vs main), test-name +verification (over the diff vs main), repo-structure verification +(``REPO_STRUCTURE.md`` vs the actual tree), plugin manifest JSON +validation, Claude Code plugin-version drift guard (when applicable), +and ``pip-audit`` dependency vulnerability scan (non-blocking — warns +but does not refuse a commit). Shipped to consumers via the +``forge-scripts`` pip package and invoked by ``.githooks/pre-commit`` +after ``install-forge-githooks``. + +Three further steps are **opt-in** (off by default): ``doctest`` +(``pytest --doctest-modules``), ``typecheck`` (pyrefly / mypy / ty / +pyright), and ``doc_consistency`` (doc claims vs repo state). Opt in by +listing them in ``[tool.forge.precommit] enable``. The same table's +``disable`` list force-skips any default step; ``--only`` / ``--skip`` +do the same for a single run. This override layer sits on top of each +step's own self-skip — it never weakens one, and ``disable`` beats +``enable`` when a name appears in both. + +Pytest is intentionally NOT in the default sequence — it is too slow for +pre-commit and belongs in CI. The opt-in ``doctest`` step runs pytest +only over docstring examples in configured paths, and is non-blocking by +default for the same speed reason. Consumers that want the full suite on +commit call ``pytest`` directly in ``.githooks/pre-commit``. Step outputs are written to ``code_health/.log`` per FOUNDATION §13 so downstream tooling can read the latest results without re-running. @@ -24,6 +35,8 @@ - ``forge-precommit`` — run the default sequence - ``forge-precommit --json`` — machine-readable summary on stdout +- ``forge-precommit --only ruff,doctest`` — run just these steps +- ``forge-precommit --skip pip_audit`` — default sequence minus a step Consumers add repo-specific bash steps by editing ``.githooks/pre-commit`` directly — lines before the ``forge-precommit`` call run first; lines @@ -38,10 +51,10 @@ import shutil import subprocess import sys -import tomllib from dataclasses import asdict, dataclass from typing import TYPE_CHECKING +from forge import config from forge.git_utils import ( detect_existing_source_dirs, emit, @@ -52,8 +65,11 @@ if TYPE_CHECKING: + from collections.abc import Callable, Sequence from pathlib import Path + StepFn = Callable[[Path], "StepResult"] + # ANSI colors (suppressed if stdout isn't a TTY — keep machine-readable). def _color(code: str) -> str: @@ -100,6 +116,46 @@ class StepResult: non_blocking: bool = False +@dataclass(frozen=True) +class StepDef: + """A registry entry: a step's name, its function, and whether it runs by default. + + Co-locating the three properties keeps the step registry a single + source of truth — a name can't drift between a separate ordered list + and a separate default-on set. ``default_on=False`` marks an opt-in + step that runs only when listed in ``[tool.forge.precommit] enable`` + (or ``--only``). + + Attributes: + name: Step identifier used in config, CLI flags, and the log slug. + fn: The ``step_*`` callable producing this step's ``StepResult``. + default_on: Whether the step is part of the default sequence. + """ + + name: str + fn: StepFn + default_on: bool = True + + +def _forge_step_config(repo_root: Path, step: str) -> dict: + """Return the ``[tool.forge.]`` table, or ``{}`` when absent. + + Single navigation point for the repeated ``tool → forge → `` + lookup every config-reading step performs. Uses the forgiving + :func:`config.read_pyproject_raw` loader, so a missing or malformed + ``pyproject.toml`` yields ``{}`` rather than raising. + + Args: + repo_root: Git repo root. + step: The ``[tool.forge.]`` subsection name (e.g. ``"doctest"``). + + Returns: + The subsection dict, or ``{}`` when any level is missing. + """ + data = config.read_pyproject_raw(repo_root) + return ((data.get("tool") or {}).get("forge") or {}).get(step) or {} + + def _run( cmd: list[str], cwd: Path, @@ -480,16 +536,7 @@ def _cli_wiring_enabled(repo_root: Path) -> bool: otherwise (including when ``pyproject.toml`` is missing or malformed). """ - pyproject = repo_root / "pyproject.toml" - if not pyproject.is_file(): - return False - try: - with pyproject.open("rb") as fh: - data = tomllib.load(fh) - except tomllib.TOMLDecodeError: - return False - section = ((data.get("tool") or {}).get("forge") or {}).get("cli_wiring") or {} - return bool(section.get("enabled")) + return bool(_forge_step_config(repo_root, "cli_wiring").get("enabled")) def step_plugin_version(repo_root: Path) -> StepResult: @@ -516,6 +563,133 @@ def step_plugin_version(repo_root: Path) -> StepResult: ) +def step_doctest(repo_root: Path) -> StepResult: + """Run ``pytest --doctest-modules`` over docstring examples (opt-in). + + Executes the ``>>>`` examples embedded in docstrings so they cannot + rot silently — forge already enforces docstring presence (ruff D), + Args/Returns accuracy (``verify_docstrings``), and coverage + (``docstring_coverage``), but not example *execution*. Scans + ``[tool.forge.doctest] paths`` (default ``["src"]``). Non-blocking + unless ``blocking = true``: pytest is slow and a false failure that + refuses a commit trains ``--no-verify``. + + Opt-in only — runs when listed in ``[tool.forge.precommit] enable`` + (or ``--only``). ``pytest`` exiting 5 (no examples collected) is + treated as a skip, not a failure. + + Args: + repo_root: Git repo root. + + Returns: + ``StepResult`` for the doctest run; ``non_blocking`` is the + inverse of ``[tool.forge.doctest] blocking``. + + Raises: + SystemExit: If ``pytest`` is not on PATH. + """ + cfg = _forge_step_config(repo_root, "doctest") + paths = cfg.get("paths") or ["src"] + blocking = bool(cfg.get("blocking", False)) + require_cli("pytest", caller="forge-precommit") + passed, output = _run( + ["pytest", "--doctest-modules", "--doctest-continue-on-failure", *paths], + cwd=repo_root, + ) + if "no tests ran" in output: + return StepResult(name="doctest", passed=True, output=output, skipped=True) + return StepResult( + name="doctest", passed=passed, output=output, non_blocking=not blocking + ) + + +# Type-checker → argv head. ``checker = "none"`` short-circuits before +# this lookup. pyrefly/mypy read existing ``[tool.]`` config. +_TYPECHECK_COMMANDS: dict[str, list[str]] = { + "pyrefly": ["pyrefly", "check"], + "mypy": ["mypy"], + "ty": ["ty", "check"], + "pyright": ["pyright"], +} + + +def step_typecheck(repo_root: Path) -> StepResult: + """Run a configurable static type checker (opt-in). + + Dispatches on ``[tool.forge.typecheck] checker`` — ``pyrefly`` + (default; Rust, stable, reads/migrates ``[tool.mypy]``), ``mypy`` + (universal, reads ``[tool.mypy]``), ``ty``, ``pyright``, or ``none`` + (skip). Scans ``paths`` (default ``["src"]``). Non-blocking unless + ``blocking = true``: a type-checker false positive that refuses a + commit trains ``--no-verify``, so the gate is advisory by default. + + Opt-in only — runs when listed in ``[tool.forge.precommit] enable``. + When opted in but the chosen checker binary is absent, fails loudly + (the consumer configured it and must fix their environment) rather + than silently passing. + + Args: + repo_root: Git repo root. + + Returns: + ``StepResult``; ``skipped`` when ``checker = "none"``, + ``non_blocking`` the inverse of ``blocking``. + + Raises: + SystemExit: If the configured checker binary is not on PATH. + """ + cfg = _forge_step_config(repo_root, "typecheck") + checker = str(cfg.get("checker") or "pyrefly") + paths = cfg.get("paths") or ["src"] + blocking = bool(cfg.get("blocking", False)) + if checker == "none": + return StepResult( + name="typecheck", + passed=True, + output="(checker = none — skipped)", + skipped=True, + ) + command = _TYPECHECK_COMMANDS.get(checker) + if command is None: + valid = ", ".join([*_TYPECHECK_COMMANDS, "none"]) + return StepResult( + name="typecheck", + passed=False, + output=f"unknown checker {checker!r} — valid: {valid}", + non_blocking=not blocking, + ) + require_cli(command[0], caller="forge-precommit") + passed, output = _run([*command, *paths], cwd=repo_root) + return StepResult( + name="typecheck", passed=passed, output=output, non_blocking=not blocking + ) + + +def step_doc_consistency(repo_root: Path) -> StepResult: + """Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). + + Checks the machine-checkable subset of documentation claims: agent / + skill / CLI / hook name-lists and counts asserted in docs against the + actual files on disk and ``[project.scripts]`` / ``plugin.json``. + Opt-in via ``[tool.forge.precommit] enable``; non-blocking (doc drift + is a warning, not grounds to refuse a commit). + + Args: + repo_root: Git repo root. + + Returns: + ``StepResult`` mirroring the CLI exit code; always ``non_blocking``. + + Raises: + SystemExit: If ``verify-forge-doc-consistency`` is not on PATH. + """ + require_cli("verify-forge-doc-consistency", caller="forge-precommit") + passed, output = _run(["verify-forge-doc-consistency"], cwd=repo_root) + return StepResult( + name="doc_consistency", passed=passed, output=output, non_blocking=True + ) + + # --------------------------------------------------------------------------- # Dispatcher # --------------------------------------------------------------------------- @@ -555,44 +729,120 @@ def _print_step_line(result: StepResult) -> None: emit(f"{result.name:<24} {color}{marker}{NC}") +# Ordered registry — the single source of truth for which steps exist, +# their run order, and which are on by default. Opt-in steps +# (default_on=False) run only when named in [tool.forge.precommit].enable +# or --only. ruff is first because it mutates + re-stages files. +_STEP_REGISTRY: tuple[StepDef, ...] = ( + StepDef("ruff", step_ruff), + StepDef("docstring_verification", step_docstrings), + StepDef("docstring_coverage", step_docstring_coverage), + StepDef("test_naming_check", step_test_naming), + StepDef("repo_structure_check", step_repo_structure), + StepDef("manifest_json", step_manifest_json), + StepDef("cli_wiring", step_cli_wiring), + StepDef("commit_types_parity", step_commit_types_parity), + StepDef("plugin_version", step_plugin_version), + StepDef("pip_audit", step_pip_audit), + StepDef("doctest", step_doctest, default_on=False), + StepDef("typecheck", step_typecheck, default_on=False), + StepDef("doc_consistency", step_doc_consistency, default_on=False), +) + +_DEFAULT_ON: frozenset[str] = frozenset(d.name for d in _STEP_REGISTRY if d.default_on) + + +def _validate_step_names(names: Sequence[str]) -> None: + """Raise ``ValueError`` listing any *names* that are not registered steps. + + Args: + names: Step names referenced by config or CLI flags. + + Raises: + ValueError: When one or more names are unknown; the message names + the offenders and lists every valid step. + """ + unknown = sorted(set(names) - {d.name for d in _STEP_REGISTRY}) + if unknown: + valid = ", ".join(d.name for d in _STEP_REGISTRY) + msg = f"unknown step name(s): {unknown}. Valid names: {valid}" + raise ValueError(msg) + + +def _resolve_steps( + repo_root: Path, + *, + skip: Sequence[str] = (), + only: Sequence[str] = (), +) -> list[StepDef]: + """Resolve which steps to run, in registry order. + + ``--only`` overrides everything (run exactly those). Otherwise the run + set is the default-on steps plus ``[tool.forge.precommit] enable``, + minus ``[tool.forge.precommit] disable`` and ``skip``. An explicit + exclusion wins: ``disable`` / ``skip`` beat ``enable`` when a name + appears in both. Every referenced name is validated against the + registry; an unknown name raises ``ValueError`` so the caller prints + one clean message instead of leaking a traceback on a config typo. + + Args: + repo_root: Git repo root. + skip: Step names to force-skip for this run (CLI ``--skip``). + only: When non-empty, the exact set of steps to run (CLI ``--only``). + + Returns: + The selected ``StepDef`` entries in registry order. + + Raises: + ValueError: When any referenced name is not a registered step. + """ + precommit_cfg = _forge_step_config(repo_root, "precommit") + enable = list(precommit_cfg.get("enable") or []) + disable = list(precommit_cfg.get("disable") or []) + _validate_step_names([*enable, *disable, *skip, *only]) + if only: + chosen = set(only) + else: + chosen = (_DEFAULT_ON | set(enable)) - set(disable) - set(skip) + return [d for d in _STEP_REGISTRY if d.name in chosen] + + def run_all( repo_root: Path | None = None, *, print_progress: bool = True, + skip: Sequence[str] = (), + only: Sequence[str] = (), ) -> list[StepResult]: - """Run every step in order and return their results. + """Run the resolved step sequence in order and return their results. - ``step_ruff`` shells out to ``fix-forge-ruff`` which applies ruff - fixes (``ruff format`` + ``ruff check --fix --unsafe-fixes``) and - re-stages modified tracked files. Other steps verify only. + The sequence is resolved from the registry via :func:`_resolve_steps` + (``[tool.forge.precommit] enable/disable`` plus ``skip`` / ``only``). + ``step_ruff`` runs first and shells out to ``fix-forge-ruff`` (applies + ruff fixes and re-stages modified tracked files); the rest verify only. Args: repo_root: Override the auto-detected git repo root. Useful in tests. print_progress: Print one-line PASS/FAIL/SKIP per step. Disable for JSON output to keep stdout machine-readable. + skip: Step names to force-skip for this run. + only: When non-empty, run exactly these steps. Returns: - List of ``StepResult``, one per step, in execution order. + List of ``StepResult``, one per executed step, in execution order. + + Raises: + ValueError: When ``skip`` / ``only`` / config names an unknown step. """ root = repo_root if repo_root is not None else get_repo_root() results: list[StepResult] = [] - ruff_result = step_ruff(root) - if print_progress: - _print_step_line(ruff_result) - _write_log(root, ruff_result) - results.append(ruff_result) - for step in ( - step_docstrings, - step_docstring_coverage, - step_test_naming, - step_repo_structure, - step_manifest_json, - step_cli_wiring, - step_commit_types_parity, - step_plugin_version, - step_pip_audit, - ): - result = step(root) + this_module = sys.modules[__name__] + for step_def in _resolve_steps(root, skip=skip, only=only): + # Resolve the step function through the module namespace (not the + # registry's captured reference) so tests can monkeypatch a + # ``step_*`` function and have run_all pick up the stub. + fn = getattr(this_module, step_def.fn.__name__) + result = fn(root) if print_progress: _print_step_line(result) _write_log(root, result) @@ -600,6 +850,24 @@ def run_all( return results +def _split_csv(values: Sequence[str]) -> list[str]: + """Flatten repeatable / comma-separated CLI values into a clean name list. + + ``--skip a,b --skip c`` and ``--skip a --skip b --skip c`` both yield + ``["a", "b", "c"]``. Empty fragments (from stray commas) are dropped. + + Args: + values: Raw ``append``-collected argument values. + + Returns: + Flattened, stripped, non-empty tokens in order. + """ + out: list[str] = [] + for value in values: + out.extend(token.strip() for token in value.split(",") if token.strip()) + return out + + def main() -> int: """CLI entry point. @@ -626,9 +894,29 @@ def main() -> int: action="store_true", help="Emit a JSON summary on stdout instead of human output.", ) + parser.add_argument( + "--skip", + action="append", + default=[], + metavar="STEP[,STEP...]", + help="Force-skip these steps for this run (repeatable or comma-separated).", + ) + parser.add_argument( + "--only", + action="append", + default=[], + metavar="STEP[,STEP...]", + help="Run exactly these steps (repeatable or comma-separated).", + ) args = parser.parse_args() - results = run_all(print_progress=not args.json) + skip = _split_csv(args.skip) + only = _split_csv(args.only) + try: + results = run_all(print_progress=not args.json, skip=skip, only=only) + except ValueError as exc: + emit(f"{RED}forge-precommit: {exc}{NC}") + return 1 blocking_failures = [r for r in results if not r.passed and not r.non_blocking] non_blocking_warnings = [r for r in results if not r.passed and r.non_blocking] diff --git a/src/forge/verify_doc_consistency.py b/src/forge/verify_doc_consistency.py new file mode 100644 index 0000000..f2ce9a8 --- /dev/null +++ b/src/forge/verify_doc_consistency.py @@ -0,0 +1,178 @@ +"""verify-forge-doc-consistency — check machine-checkable doc claims vs repo state. + +Backs the opt-in ``doc_consistency`` pre-commit step. Nothing in forge +otherwise checks documentation for factual drift against the repo: a doc +can claim "ten foundation agents" or list a CLI that no longer exists, +and only a careful human reread catches it. This CLI closes the +**structured, NLP-free subset** of that hole — names and counts a doc +asserts about the repo's own inventory, verified against the filesystem +and ``pyproject.toml``. + +Checks (each self-skips when its inputs are absent, so the CLI is safe in +any repo): + +1. **CLI coverage** — every ``[project.scripts]`` entry name appears at + least once in ``docs/cli-reference.md``. A new CLI added without a doc + line is drift. +2. **Agent count** — a ``" foundation agents"`` claim in + ``FOUNDATION.md`` matches the number of ``agents/*.md`` files + (excluding the underscore-prefixed ``_TEMPLATE.md``). + +Scope is deliberately conservative for v1: name-lists and counts only. +Internal-link validation and repo-state facts (visibility, default +branch) are intentionally out of scope — they are tracked separately. + +Exit code: ``0`` when consistent or nothing to check, ``1`` when any +drift is found. The ``doc_consistency`` step renders a non-zero result as +a non-blocking ``WARN``. +""" + +from __future__ import annotations + +import argparse +import logging +import re +import sys +import tomllib +from typing import TYPE_CHECKING + +from forge.git_utils import configure_cli_logging +from forge.git_utils import repo_root as get_repo_root + + +if TYPE_CHECKING: + from pathlib import Path + + +configure_cli_logging() +logger = logging.getLogger(__name__) + + +# Spelled-out cardinals forge docs actually use for inventory counts. +# Digits are matched directly; this map covers the prose form. +_NUMBER_WORDS: dict[str, int] = { + "zero": 0, + "one": 1, + "two": 2, + "three": 3, + "four": 4, + "five": 5, + "six": 6, + "seven": 7, + "eight": 8, + "nine": 9, + "ten": 10, + "eleven": 11, + "twelve": 12, + "thirteen": 13, + "fourteen": 14, + "fifteen": 15, + "sixteen": 16, + "seventeen": 17, + "eighteen": 18, + "nineteen": 19, + "twenty": 20, +} + +# " foundation agents" — count as a digit run or a spelled cardinal. +_AGENT_COUNT_RE = re.compile( + r"\b(\d+|" + "|".join(_NUMBER_WORDS) + r")\s+foundation agents\b", + re.IGNORECASE, +) + + +def _check_cli_coverage(repo_root: Path) -> list[str]: + """Return findings for ``[project.scripts]`` names missing from the CLI reference. + + Skips silently when ``pyproject.toml`` has no ``[project.scripts]`` + table or ``docs/cli-reference.md`` is absent — a repo without either + has nothing to drift. + + Args: + repo_root: Git repo root. + + Returns: + One finding string per script name absent from the reference doc; + empty when consistent or not applicable. + """ + pyproject = repo_root / "pyproject.toml" + reference = repo_root / "docs" / "cli-reference.md" + if not pyproject.is_file() or not reference.is_file(): + return [] + try: + data = tomllib.loads(pyproject.read_text(encoding="utf-8")) + except (tomllib.TOMLDecodeError, OSError): + return [] + scripts = (data.get("project") or {}).get("scripts") or {} + if not scripts: + return [] + reference_text = reference.read_text(encoding="utf-8") + return [ + f"docs/cli-reference.md: no entry for [project.scripts] CLI '{name}'" + for name in sorted(scripts) + if name not in reference_text + ] + + +def _check_agent_count(repo_root: Path) -> list[str]: + """Return a finding when FOUNDATION's agent-count claim disagrees with disk. + + Counts ``agents/*.md`` files (excluding underscore-prefixed templates) + and compares against the highest ``" foundation agents"`` claim in + ``FOUNDATION.md``. Skips silently when the ``agents/`` directory or the + claim is absent. + + Args: + repo_root: Git repo root. + + Returns: + A single-element finding list on mismatch; empty when consistent + or not applicable. + """ + agents_dir = repo_root / "agents" + foundation = repo_root / "FOUNDATION.md" + if not agents_dir.is_dir() or not foundation.is_file(): + return [] + actual = sum(1 for path in agents_dir.glob("*.md") if not path.name.startswith("_")) + match = _AGENT_COUNT_RE.search(foundation.read_text(encoding="utf-8")) + if match is None: + return [] + token = match.group(1).lower() + claimed = int(token) if token.isdigit() else _NUMBER_WORDS[token] + if claimed != actual: + return [ + f"FOUNDATION.md claims {claimed} foundation agents, " + f"but agents/ holds {actual}" + ] + return [] + + +def main() -> int: + """CLI entry point. + + Returns: + ``0`` when every applicable check is consistent (or nothing + applies); ``1`` when any drift is found. + """ + argparse.ArgumentParser( + prog="verify-forge-doc-consistency", + description=( + "Check machine-checkable documentation claims (CLI name-lists, " + "agent counts) against the actual repo state. Non-blocking " + "reporter for the doc_consistency pre-commit step." + ), + ).parse_args() + + repo_root = get_repo_root() + findings = _check_cli_coverage(repo_root) + _check_agent_count(repo_root) + if findings: + logger.error("Documentation drift detected:") + for finding in findings: + logger.error(" - %s", finding) + return 1 + logger.info("Documentation claims consistent with repo state.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_forge_config.py b/tests/test_forge_config.py index 5b20282..be9c888 100644 --- a/tests/test_forge_config.py +++ b/tests/test_forge_config.py @@ -2,17 +2,81 @@ from __future__ import annotations +import re +from pathlib import Path from typing import TYPE_CHECKING from forge import forge_config if TYPE_CHECKING: - from pathlib import Path - import pytest +_REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _forge_source_files(*, exclude: str) -> dict[Path, str]: + """Return ``src/forge`` ``*.py`` sources keyed by path, minus *exclude*. + + Args: + exclude: Filename to drop (e.g. the declaration site itself). + + Returns: + Mapping of file path to its text for every other module. + """ + return { + path: path.read_text(encoding="utf-8") + for path in (_REPO_ROOT / "src" / "forge").rglob("*.py") + if path.name != exclude + } + + +def test_every_config_key_leaf_is_read_by_some_module() -> None: + """Every CONFIG_KEYS leaf appears in a reader module — no stale entries. + + #46: CONFIG_KEYS is forge's single declared enumeration of the + ``[tool.forge.*]`` keys forge reads. This couples the registry to + reality — a key whose leaf no longer appears as a string literal in + any ``src/forge`` module (other than ``forge_config.py``, the + declaration site) is a stale entry that makes ``forge-config --list`` + advertise a key nothing reads. + """ + sources = _forge_source_files(exclude="forge_config.py") + for key in forge_config.CONFIG_KEYS: + leaf = key.path[-1] + read_somewhere = any( + f'"{leaf}"' in text or f"'{leaf}'" in text for text in sources.values() + ) + assert read_somewhere, ( + f"CONFIG_KEYS leaf {leaf!r} ({'.'.join(key.path)}) is referenced " + f"by no src/forge module — stale registry entry?" + ) + + +def test_forge_step_config_sections_are_declared() -> None: + """Every ``[tool.forge.
]`` read via ``_forge_step_config`` is declared. + + #46 reverse direction: a new pre-commit step reading + ``[tool.forge.]`` without a CONFIG_KEYS entry would make + ``forge-config --list`` silently incomplete. Extract the section names + passed to ``precommit._forge_step_config`` and assert each is a + declared third-level token under ``[tool.forge]``. + """ + precommit_src = (_REPO_ROOT / "src" / "forge" / "precommit.py").read_text( + encoding="utf-8" + ) + sections = set( + re.findall(r'_forge_step_config\([^,]+,\s*"([^"]+)"\)', precommit_src) + ) + declared = {key.path[2] for key in forge_config.CONFIG_KEYS if len(key.path) >= 3} + undocumented = sections - declared + assert not undocumented, ( + f"[tool.forge.{sorted(undocumented)}] read via _forge_step_config but " + f"absent from CONFIG_KEYS" + ) + + def test_lookup_returns_value_and_unset() -> None: """`_lookup` returns nested values, or the `_UNSET` sentinel when absent.""" data = {"tool": {"forge": {"base_branch": "trunk"}}} diff --git a/tests/test_precommit.py b/tests/test_precommit.py index d5491c1..f1c7c96 100644 --- a/tests/test_precommit.py +++ b/tests/test_precommit.py @@ -592,3 +592,344 @@ def _failing_test_naming(_root: object) -> precommit.StepResult: # Each failed step listed with its log path assert "docstring_verification: see code_health/docstring_verification.log" in out assert "test_naming_check: see code_health/test_naming_check.log" in out + + +# --------------------------------------------------------------------------- +# Step framework: registry, resolution, CLI overrides (#6) +# --------------------------------------------------------------------------- + + +def _write_pyproject(tmp_path: Path, body: str) -> None: + """Write *body* as ``pyproject.toml`` in *tmp_path* (config-test helper). + + Args: + tmp_path: Temporary directory path. + body: TOML content to write. + """ + (tmp_path / "pyproject.toml").write_text(body, encoding="utf-8") + + +def _names(step_defs: list[precommit.StepDef]) -> list[str]: + """Return the names of resolved ``StepDef`` entries (readability helper). + + Args: + step_defs: List of step definitions. + + Returns: + List of step names extracted from the definitions. + """ + return [d.name for d in step_defs] + + +def _present(monkeypatch: pytest.MonkeyPatch) -> None: + """Make every binary resolve on PATH (so ``require_cli`` passes).""" + monkeypatch.setattr(precommit.shutil, "which", lambda name: f"/usr/bin/{name}") + + +def test_forge_step_config_reads_section(tmp_path: Path) -> None: + """`_forge_step_config` returns the `[tool.forge.]` table.""" + _write_pyproject(tmp_path, '[tool.forge.doctest]\npaths = ["lib"]\n') + assert precommit._forge_step_config(tmp_path, "doctest") == {"paths": ["lib"]} + + +def test_forge_step_config_missing_returns_empty(tmp_path: Path) -> None: + """`_forge_step_config` returns `{}` when the section or pyproject is absent.""" + assert precommit._forge_step_config(tmp_path, "doctest") == {} + _write_pyproject(tmp_path, "[tool.forge]\n") + assert precommit._forge_step_config(tmp_path, "doctest") == {} + + +def test_validate_step_names_accepts_known() -> None: + """`_validate_step_names` is silent for registered step names.""" + precommit._validate_step_names(["ruff", "doctest", "pip_audit"]) + + +def test_validate_step_names_rejects_unknown() -> None: + """`_validate_step_names` raises ValueError naming the offender and valid set.""" + with pytest.raises(ValueError, match="unknown step name") as exc: + precommit._validate_step_names(["ruff", "nope"]) + assert "nope" in str(exc.value) + assert "ruff" in str(exc.value) + + +def test_resolve_steps_default_excludes_opt_in(tmp_path: Path) -> None: + """The default run set is the default-on steps; opt-in steps stay out.""" + names = _names(precommit._resolve_steps(tmp_path)) + assert "ruff" in names + assert "doctest" not in names + assert "typecheck" not in names + assert "doc_consistency" not in names + + +def test_resolve_steps_enable_adds_opt_in(tmp_path: Path) -> None: + """`[tool.forge.precommit] enable` opts a normally-off step in.""" + _write_pyproject(tmp_path, '[tool.forge.precommit]\nenable = ["doctest"]\n') + assert "doctest" in _names(precommit._resolve_steps(tmp_path)) + + +def test_resolve_steps_disable_removes_default(tmp_path: Path) -> None: + """`[tool.forge.precommit] disable` force-skips a default step.""" + _write_pyproject(tmp_path, '[tool.forge.precommit]\ndisable = ["pip_audit"]\n') + assert "pip_audit" not in _names(precommit._resolve_steps(tmp_path)) + + +def test_resolve_steps_disable_beats_enable(tmp_path: Path) -> None: + """When a name is in both `enable` and `disable`, `disable` wins.""" + _write_pyproject( + tmp_path, + '[tool.forge.precommit]\nenable = ["doctest"]\ndisable = ["doctest"]\n', + ) + assert "doctest" not in _names(precommit._resolve_steps(tmp_path)) + + +def test_resolve_steps_skip_removes(tmp_path: Path) -> None: + """The `skip` argument removes a step for this run only.""" + assert "ruff" not in _names(precommit._resolve_steps(tmp_path, skip=["ruff"])) + + +def test_resolve_steps_only_overrides_in_registry_order(tmp_path: Path) -> None: + """`only=[...]` runs exactly those steps, ordered by the registry not the arg.""" + resolved = _names(precommit._resolve_steps(tmp_path, only=["pip_audit", "ruff"])) + assert resolved == ["ruff", "pip_audit"] + + +def test_resolve_steps_unknown_name_raises(tmp_path: Path) -> None: + """An unknown name in config / skip / only raises ValueError.""" + with pytest.raises(ValueError, match="unknown step name"): + precommit._resolve_steps(tmp_path, only=["bogus"]) + + +def test_split_csv_flattens_repeats_and_commas() -> None: + """`_split_csv` flattens repeated and comma-separated values, dropping blanks.""" + assert precommit._split_csv(["a,b", "c"]) == ["a", "b", "c"] + assert precommit._split_csv(["a, ,b"]) == ["a", "b"] + assert precommit._split_csv([]) == [] + + +def test_run_all_only_dispatches_monkeypatched_steps( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """run_all(only=...) runs exactly the named steps via the module dispatch. + + MOCK SETUP: ``step_ruff`` and ``step_pip_audit`` are replaced with + canned passing stubs; run_all is called with ``only`` those two. + EXPECTED BEHAVIOR: both stubs run (proving the registry resolves the + monkeypatched functions, not its captured references) and no other + step executes. + """ + + def _ruff(_root: object) -> precommit.StepResult: + return precommit.StepResult(name="ruff", passed=True, output="x") + + def _audit(_root: object) -> precommit.StepResult: + return precommit.StepResult(name="pip_audit", passed=True, output="x") + + monkeypatch.setattr(precommit, "step_ruff", _ruff) + monkeypatch.setattr(precommit, "step_pip_audit", _audit) + results = precommit.run_all( + tmp_path, print_progress=False, only=["ruff", "pip_audit"] + ) + assert [r.name for r in results] == ["ruff", "pip_audit"] + + +def test_main_only_flag_runs_subset( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """`--only ruff --json` runs just ruff and emits a one-entry JSON list. + + MOCK SETUP: get_repo_root pinned to tmp_path; ``step_ruff`` stubbed to + pass; argv drives main() with ``--only ruff --json``. + """ + monkeypatch.setattr(precommit, "get_repo_root", lambda: tmp_path) + + def _ruff(_root: object) -> precommit.StepResult: + return precommit.StepResult(name="ruff", passed=True, output="x") + + monkeypatch.setattr(precommit, "step_ruff", _ruff) + with patch.object( + precommit.sys, "argv", ["forge-precommit", "--only", "ruff", "--json"] + ): + rc = precommit.main() + assert rc == 0 + data = json.loads(capsys.readouterr().out) + assert [r["name"] for r in data] == ["ruff"] + + +def test_main_unknown_step_name_exits_one( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """An unknown `--skip` name prints a clean error and exits 1 (no traceback).""" + monkeypatch.setattr(precommit, "get_repo_root", lambda: tmp_path) + with patch.object(precommit.sys, "argv", ["forge-precommit", "--skip", "bogus"]): + rc = precommit.main() + assert rc == 1 + assert "unknown step name" in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# Opt-in steps: doctest (#5), typecheck (#48), doc_consistency (#4) +# --------------------------------------------------------------------------- + + +def test_step_doctest_passes_non_blocking_by_default( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Doctest passes as non-blocking when `blocking` is unset. + + MOCK SETUP: pytest present on PATH; ``_run`` returns a passing canned + result; no ``[tool.forge.doctest]`` config so defaults apply. + """ + _present(monkeypatch) + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (True, "3 passed")) + result = precommit.step_doctest(tmp_path) + assert result.passed + assert result.non_blocking + assert not result.skipped + + +def test_step_doctest_uses_configured_paths( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Doctest runs `pytest --doctest-modules` over `[tool.forge.doctest].paths`.""" + _present(monkeypatch) + _write_pyproject(tmp_path, '[tool.forge.doctest]\npaths = ["lib", "app"]\n') + captured: dict[str, list[str]] = {} + + def _run(cmd: list[str], **_kw: object) -> tuple[bool, str]: + captured["cmd"] = cmd + return True, "1 passed" + + monkeypatch.setattr(precommit, "_run", _run) + precommit.step_doctest(tmp_path) + assert "--doctest-modules" in captured["cmd"] + assert captured["cmd"][-2:] == ["lib", "app"] + + +def test_step_doctest_no_examples_is_skipped( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pytest 'no tests ran' (exit 5) counts as a skip, not a failure.""" + _present(monkeypatch) + monkeypatch.setattr( + precommit, "_run", lambda _cmd, **_kw: (False, "no tests ran in 0.01s") + ) + result = precommit.step_doctest(tmp_path) + assert result.skipped + assert result.passed + + +def test_step_doctest_blocking_config_is_blocking( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`blocking = true` makes a failing doctest a blocking failure.""" + _present(monkeypatch) + _write_pyproject(tmp_path, "[tool.forge.doctest]\nblocking = true\n") + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "1 failed")) + result = precommit.step_doctest(tmp_path) + assert not result.passed + assert not result.non_blocking + + +def test_step_doctest_missing_pytest_exits( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Doctest fails loudly (SystemExit) when pytest is not on PATH.""" + monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) + with pytest.raises(SystemExit): + precommit.step_doctest(tmp_path) + + +def test_step_typecheck_default_pyrefly( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Typecheck defaults to the `pyrefly check` command and is non-blocking. + + MOCK SETUP: pyrefly present on PATH; ``_run`` captures the command and + returns a passing result; no config so the default checker applies. + """ + _present(monkeypatch) + captured: dict[str, list[str]] = {} + + def _run(cmd: list[str], **_kw: object) -> tuple[bool, str]: + captured["cmd"] = cmd + return True, "0 errors" + + monkeypatch.setattr(precommit, "_run", _run) + result = precommit.step_typecheck(tmp_path) + assert captured["cmd"][:2] == ["pyrefly", "check"] + assert result.passed + assert result.non_blocking + + +def test_step_typecheck_none_skips(tmp_path: Path) -> None: + """`checker = "none"` skips the step without invoking any tool.""" + _write_pyproject(tmp_path, '[tool.forge.typecheck]\nchecker = "none"\n') + result = precommit.step_typecheck(tmp_path) + assert result.skipped + assert result.passed + + +def test_step_typecheck_unknown_checker_fails(tmp_path: Path) -> None: + """An unrecognized checker name fails with a message listing valid names.""" + _write_pyproject(tmp_path, '[tool.forge.typecheck]\nchecker = "bogus"\n') + result = precommit.step_typecheck(tmp_path) + assert not result.passed + assert "pyrefly" in result.output + + +def test_step_typecheck_missing_binary_exits( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A configured-but-absent checker binary fails loudly (SystemExit).""" + monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) + with pytest.raises(SystemExit): + precommit.step_typecheck(tmp_path) + + +def test_step_typecheck_blocking_config( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`blocking = true` makes a checker error a blocking failure.""" + _present(monkeypatch) + _write_pyproject( + tmp_path, '[tool.forge.typecheck]\nchecker = "mypy"\nblocking = true\n' + ) + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "error: x")) + result = precommit.step_typecheck(tmp_path) + assert not result.passed + assert not result.non_blocking + + +def test_step_doc_consistency_non_blocking( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """doc_consistency mirrors the CLI exit and is always non-blocking.""" + _present(monkeypatch) + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "drift")) + result = precommit.step_doc_consistency(tmp_path) + assert not result.passed + assert result.non_blocking + + +def test_step_doc_consistency_missing_cli_exits( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """doc_consistency fails loudly when its CLI is not on PATH.""" + monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) + with pytest.raises(SystemExit): + precommit.step_doc_consistency(tmp_path) diff --git a/tests/test_verify_doc_consistency.py b/tests/test_verify_doc_consistency.py new file mode 100644 index 0000000..9f3e43c --- /dev/null +++ b/tests/test_verify_doc_consistency.py @@ -0,0 +1,134 @@ +"""Tests for ``forge.verify_doc_consistency``. + +# MOCKING STRATEGY: each test builds a throwaway repo tree under tmp_path +# (pyproject, docs/cli-reference.md, agents/*.md, FOUNDATION.md) and runs +# the real check functions against it. ``main`` tests pin get_repo_root to +# tmp_path and patch sys.argv so argparse does not see pytest's argv. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from forge import verify_doc_consistency as vdc + + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +def _write(path: Path, text: str) -> None: + """Write *text* to *path*, creating parent directories as needed. + + Args: + path: Destination file path. + text: Contents to write. + """ + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +def _make_agents(tmp_path: Path, count: int) -> None: + """Create *count* agent files plus an excluded ``_TEMPLATE.md``. + + Args: + tmp_path: Repo root to populate. + count: Number of real ``agents/*.md`` files to create. + """ + agents = tmp_path / "agents" + agents.mkdir(parents=True, exist_ok=True) + for i in range(count): + (agents / f"agent{i}.md").write_text("x", encoding="utf-8") + (agents / "_TEMPLATE.md").write_text("template", encoding="utf-8") + + +def test_cli_coverage_skips_without_inputs(tmp_path: Path) -> None: + """No pyproject or no cli-reference → nothing to check.""" + assert vdc._check_cli_coverage(tmp_path) == [] + + +def test_cli_coverage_clean_when_all_documented(tmp_path: Path) -> None: + """Every `[project.scripts]` name present in the reference → no findings.""" + _write(tmp_path / "pyproject.toml", '[project.scripts]\nfoo-cli = "x:main"\n') + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n- foo-cli does things\n") + assert vdc._check_cli_coverage(tmp_path) == [] + + +def test_cli_coverage_flags_missing(tmp_path: Path) -> None: + """A script absent from the reference doc is reported by name.""" + _write( + tmp_path / "pyproject.toml", + '[project.scripts]\nfoo-cli = "x:main"\nbar-cli = "y:main"\n', + ) + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n- foo-cli only\n") + findings = vdc._check_cli_coverage(tmp_path) + assert len(findings) == 1 + assert "bar-cli" in findings[0] + + +def test_cli_coverage_malformed_pyproject_skips(tmp_path: Path) -> None: + """A malformed pyproject yields no findings rather than raising.""" + _write(tmp_path / "pyproject.toml", "this is = = not [[[ toml") + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n") + assert vdc._check_cli_coverage(tmp_path) == [] + + +def test_agent_count_skips_without_inputs(tmp_path: Path) -> None: + """No agents/ directory or no FOUNDATION.md → nothing to check.""" + assert vdc._check_agent_count(tmp_path) == [] + + +def test_agent_count_clean_on_word_match(tmp_path: Path) -> None: + """A spelled-out count matching the file count passes.""" + _make_agents(tmp_path, 3) + _write(tmp_path / "FOUNDATION.md", "The three foundation agents are: a, b, c.") + assert vdc._check_agent_count(tmp_path) == [] + + +def test_agent_count_clean_on_digit_match_excluding_template(tmp_path: Path) -> None: + """A digit count matching passes, and ``_TEMPLATE.md`` is not counted.""" + _make_agents(tmp_path, 12) + _write(tmp_path / "FOUNDATION.md", "forge ships 12 foundation agents total.") + assert vdc._check_agent_count(tmp_path) == [] + + +def test_agent_count_flags_mismatch(tmp_path: Path) -> None: + """A claim that disagrees with the actual file count is reported.""" + _make_agents(tmp_path, 5) + _write(tmp_path / "FOUNDATION.md", "the ten foundation agents are listed below") + findings = vdc._check_agent_count(tmp_path) + assert len(findings) == 1 + assert "10" in findings[0] + assert "5" in findings[0] + + +def test_main_returns_zero_when_consistent( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """main() exits 0 on an empty repo — every check skips, nothing drifts. + + MOCK SETUP: get_repo_root pinned to an empty tmp_path; argv patched so + argparse does not consume pytest's arguments. + """ + monkeypatch.setattr(vdc, "get_repo_root", lambda: tmp_path) + monkeypatch.setattr(vdc.sys, "argv", ["verify-forge-doc-consistency"]) + assert vdc.main() == 0 + + +def test_main_returns_one_on_drift( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """main() exits 1 when a check reports drift. + + MOCK SETUP: a repo whose FOUNDATION claims nine agents but holds two; + get_repo_root pinned to it and argv patched. + """ + _make_agents(tmp_path, 2) + _write(tmp_path / "FOUNDATION.md", "the nine foundation agents are") + monkeypatch.setattr(vdc, "get_repo_root", lambda: tmp_path) + monkeypatch.setattr(vdc.sys, "argv", ["verify-forge-doc-consistency"]) + assert vdc.main() == 1 From 2302154813305d26053b054f67279152f7c52255 Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 14:38:18 +0200 Subject: [PATCH 2/7] refactor(precommit): typecheck runs pyrefly only; validate config paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop the mypy/ty/pyright/none checker dispatch — typecheck runs pyrefly (YAGNI; per maintainer) - Validate [tool.forge.*] paths (reject option-like / repo-escaping) before they reach subprocess argv - doctest treats "no items ran" as skip; declare pyrefly as the typecheck extra --- CHANGELOG.md | 5 +- docs/api-digest.md | 7 +- docs/configuration.md | 16 ++--- pyproject.toml | 4 +- src/forge/forge_config.py | 8 +-- src/forge/precommit.py | 106 ++++++++++++++++------------ src/forge/verify_doc_consistency.py | 2 +- tests/test_precommit.py | 43 ++++++----- 8 files changed, 101 insertions(+), 90 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61648ad..2fb9419 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,8 @@ All additive and opt-in — no consumer action required to upgrade. - **Opt-in `doctest` step** — `pytest --doctest-modules` over `[tool.forge.doctest].paths` (default `["src"]`); non-blocking by default (#5). -- **Opt-in `typecheck` step** — `[tool.forge.typecheck].checker` - (`pyrefly` default; `mypy` / `ty` / `pyright` / `none`); non-blocking by - default (#48). +- **Opt-in `typecheck` step** — runs `pyrefly` over + `[tool.forge.typecheck].paths`; non-blocking by default (#48). - **Opt-in `doc_consistency` step** + `verify-forge-doc-consistency` CLI — checks CLI name-lists and agent counts asserted in docs against the actual repo state; non-blocking (#4). diff --git a/docs/api-digest.md b/docs/api-digest.md index 79d39b6..a6323c2 100644 --- a/docs/api-digest.md +++ b/docs/api-digest.md @@ -4,7 +4,7 @@ A compact index of this codebase's symbols — every top-level function and clas > **Generated file — do not edit by hand.** Regenerate with `forge-gen-api-digest`; check for drift with `forge-gen-api-digest --check`. -_41 modules, 374 symbols._ +_41 modules, 375 symbols._ ## `forge._hook_helpers` @@ -371,7 +371,7 @@ _41 modules, 374 symbols._ - `_color(code: str) -> str` _(internal)_ — Return *code* if stdout is a TTY, else an empty string. - `class StepResult` — Outcome of a single pre-commit step. - `class StepDef` — A registry entry: a step's name, its function, and whether it runs by default. -- `_forge_step_config(repo_root: Path, step: str) -> dict` _(internal)_ — Return the ``[tool.forge.]`` table, or ``{}`` when absent. +- `_forge_step_config(repo_root: Path, step: str) -> dict[str, object]` _(internal)_ — Return the ``[tool.forge.]`` table, or ``{}`` when absent. - `_run(cmd: list[str], cwd: Path) -> tuple[bool, str]` _(internal)_ — Run *cmd* and capture combined output. - `step_ruff(repo_root: Path) -> StepResult` — Run ``fix-forge-ruff`` — owns the ruff phase end-to-end. - `step_docstrings(repo_root: Path) -> StepResult` — Run ``verify-forge-docstrings`` over the current diff vs main. @@ -385,8 +385,9 @@ _41 modules, 374 symbols._ - `step_cli_wiring(repo_root: Path) -> StepResult` — Run ``verify-forge-cli-wiring`` — assert every script has a real caller. - `_cli_wiring_enabled(repo_root: Path) -> bool` _(internal)_ — Return True when the repo has opted into the cli_wiring check. - `step_plugin_version(repo_root: Path) -> StepResult` — Run ``verify-forge-plugin-version`` — owns the rolling-next guard. +- `_bad_scan_paths(paths: list[str], repo_root: Path) -> list[str]` _(internal)_ — Return config scan-path values that are option-like or escape the repo. - `step_doctest(repo_root: Path) -> StepResult` — Run ``pytest --doctest-modules`` over docstring examples (opt-in). -- `step_typecheck(repo_root: Path) -> StepResult` — Run a configurable static type checker (opt-in). +- `step_typecheck(repo_root: Path) -> StepResult` — Run pyrefly over the source tree (opt-in). - `step_doc_consistency(repo_root: Path) -> StepResult` — Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). - `_write_log(repo_root: Path, result: StepResult) -> None` _(internal)_ — Persist *result*'s output to ``code_health/.log``. - `_print_step_line(result: StepResult) -> None` _(internal)_ — Print a one-line status for *result* (SKIP/PASS/WARN/FAIL). diff --git a/docs/configuration.md b/docs/configuration.md index e3ca15e..3777d21 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -86,17 +86,17 @@ not just present. Enable via `[tool.forge.precommit] enable = ["doctest"]`. ## `[tool.forge.typecheck]` — opt-in type-check step -Runs a configurable static type checker. Enable via -`[tool.forge.precommit] enable = ["typecheck"]`. When enabled but the -chosen checker binary is absent, the step fails loudly (it does not -silently pass). Non-blocking by default — a type-checker false positive -that refuses a commit trains `--no-verify`. +Runs [pyrefly](https://github.com/facebook/pyrefly) (Rust, stable, +pyproject-native, reads/migrates `[tool.mypy]`). Enable via +`[tool.forge.precommit] enable = ["typecheck"]`. When enabled but +`pyrefly` is absent, the step fails loudly (it does not silently pass). +Non-blocking by default — a type-checker false positive that refuses a +commit trains `--no-verify`. | Key | Default | What it does | Set it when | |---|---|---|---| -| `checker` | `pyrefly` | `pyrefly` (Rust, stable, reads/migrates `[tool.mypy]`), `mypy`, `ty`, `pyright`, or `none` (skip). | You prefer a specific checker. `pyrefly` is the recommended fast default. | -| `paths` | `["src"]` | Scan roots passed to the checker. | Your source lives elsewhere. | -| `blocking` | `false` | Fail the commit on a checker error (else non-blocking WARN). | Your type baseline is clean and you want it enforced. | +| `paths` | `["src"]` | Scan roots passed to pyrefly. | Your source lives elsewhere. | +| `blocking` | `false` | Fail the commit on a type error (else non-blocking WARN). | Your type baseline is clean and you want it enforced. | The `doc_consistency` step (`verify-forge-doc-consistency`, enabled the same way) has no config table — it checks CLI name-lists and agent counts diff --git a/pyproject.toml b/pyproject.toml index bd60696..8b46550 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,12 +61,14 @@ forge-audit-all = "forge.audit.all:main" test = ["pytest>=8.0"] audit = ["vulture>=2.10", "jsonschema>=4.0", "PyYAML>=6.0"] audit-tach = ["tach>=0.30"] +# Type checker for the opt-in `typecheck` pre-commit step (pyrefly only). +typecheck = ["pyrefly>=0.1"] # Contributor / dev-loop umbrella. Aggregates the targeted extras so # every reference to `pip install -e ".[dev]"` in forge's own source, # docs, hook error messages, and test assertions resolves to a real # install target inside forge's own repo. Consumer repos typically # carry their own `[dev]` extra; this one is for forge itself. -dev = ["forge-scripts[test,audit]"] +dev = ["forge-scripts[test,audit,typecheck]"] [project.urls] Homepage = "https://github.com/misnaej/forge" diff --git a/src/forge/forge_config.py b/src/forge/forge_config.py index d0961f4..e5a6a8c 100644 --- a/src/forge/forge_config.py +++ b/src/forge/forge_config.py @@ -124,16 +124,10 @@ class ConfigKey: description="Make the doctest step fail the commit on a broken " "example (default: non-blocking WARN).", ), - ConfigKey( - ("tool", "forge", "typecheck", "checker"), - "pyrefly", - "Type checker for the opt-in typecheck step: pyrefly | mypy | ty | " - "pyright | none.", - ), ConfigKey( ("tool", "forge", "typecheck", "paths"), ["src"], - "Scan roots for the opt-in typecheck step.", + "Scan roots for the opt-in pyrefly typecheck step.", ), ConfigKey( ("tool", "forge", "typecheck", "blocking"), diff --git a/src/forge/precommit.py b/src/forge/precommit.py index a6b1cce..1ddd6a1 100644 --- a/src/forge/precommit.py +++ b/src/forge/precommit.py @@ -14,9 +14,9 @@ after ``install-forge-githooks``. Three further steps are **opt-in** (off by default): ``doctest`` -(``pytest --doctest-modules``), ``typecheck`` (pyrefly / mypy / ty / -pyright), and ``doc_consistency`` (doc claims vs repo state). Opt in by -listing them in ``[tool.forge.precommit] enable``. The same table's +(``pytest --doctest-modules``), ``typecheck`` (``pyrefly``), and +``doc_consistency`` (doc claims vs repo state). Opt in by listing them in +``[tool.forge.precommit] enable``. The same table's ``disable`` list force-skips any default step; ``--only`` / ``--skip`` do the same for a single run. This override layer sits on top of each step's own self-skip — it never weakens one, and ``disable`` beats @@ -137,7 +137,7 @@ class StepDef: default_on: bool = True -def _forge_step_config(repo_root: Path, step: str) -> dict: +def _forge_step_config(repo_root: Path, step: str) -> dict[str, object]: """Return the ``[tool.forge.]`` table, or ``{}`` when absent. Single navigation point for the repeated ``tool → forge → `` @@ -563,6 +563,33 @@ def step_plugin_version(repo_root: Path) -> StepResult: ) +def _bad_scan_paths(paths: list[str], repo_root: Path) -> list[str]: + """Return config scan-path values that are option-like or escape the repo. + + Config-supplied scan roots (``[tool.forge.] paths``) are spliced + into a subprocess argv. A value starting with ``-`` would be parsed as + a tool flag, and an absolute / ``..`` path would scan outside the repo. + Both are misconfiguration and are rejected rather than passed through. + + Args: + paths: The configured scan roots. + repo_root: Git repo root the paths must stay within. + + Returns: + The offending entries; empty when every path is a safe in-repo root. + """ + bad: list[str] = [] + for raw in paths: + if not isinstance(raw, str) or raw.startswith("-"): + bad.append(str(raw)) + continue + try: + (repo_root / raw).resolve().relative_to(repo_root.resolve()) + except ValueError: + bad.append(raw) + return bad + + def step_doctest(repo_root: Path) -> StepResult: """Run ``pytest --doctest-modules`` over docstring examples (opt-in). @@ -575,8 +602,9 @@ def step_doctest(repo_root: Path) -> StepResult: refuses a commit trains ``--no-verify``. Opt-in only — runs when listed in ``[tool.forge.precommit] enable`` - (or ``--only``). ``pytest`` exiting 5 (no examples collected) is - treated as a skip, not a failure. + (or ``--only``). When pytest output contains ``"no tests ran"`` (no + docstring examples collected), the step is treated as a skip rather + than a failure. Args: repo_root: Git repo root. @@ -591,75 +619,63 @@ def step_doctest(repo_root: Path) -> StepResult: cfg = _forge_step_config(repo_root, "doctest") paths = cfg.get("paths") or ["src"] blocking = bool(cfg.get("blocking", False)) + bad = _bad_scan_paths(paths, repo_root) + if bad: + return StepResult( + name="doctest", + passed=False, + output=f"invalid [tool.forge.doctest] paths: {bad}", + ) require_cli("pytest", caller="forge-precommit") passed, output = _run( ["pytest", "--doctest-modules", "--doctest-continue-on-failure", *paths], cwd=repo_root, ) - if "no tests ran" in output: + if "no tests ran" in output or "no items ran" in output: return StepResult(name="doctest", passed=True, output=output, skipped=True) return StepResult( name="doctest", passed=passed, output=output, non_blocking=not blocking ) -# Type-checker → argv head. ``checker = "none"`` short-circuits before -# this lookup. pyrefly/mypy read existing ``[tool.]`` config. -_TYPECHECK_COMMANDS: dict[str, list[str]] = { - "pyrefly": ["pyrefly", "check"], - "mypy": ["mypy"], - "ty": ["ty", "check"], - "pyright": ["pyright"], -} - - def step_typecheck(repo_root: Path) -> StepResult: - """Run a configurable static type checker (opt-in). + """Run pyrefly over the source tree (opt-in). - Dispatches on ``[tool.forge.typecheck] checker`` — ``pyrefly`` - (default; Rust, stable, reads/migrates ``[tool.mypy]``), ``mypy`` - (universal, reads ``[tool.mypy]``), ``ty``, ``pyright``, or ``none`` - (skip). Scans ``paths`` (default ``["src"]``). Non-blocking unless - ``blocking = true``: a type-checker false positive that refuses a - commit trains ``--no-verify``, so the gate is advisory by default. + pyrefly is forge's type checker — same Astral model as ruff (single + Rust binary, pyproject-native, reads/migrates ``[tool.mypy]`` config), + so it slots into forge's existing toolchain with no Node runtime. + Scans ``[tool.forge.typecheck] paths`` (default ``["src"]``). + Non-blocking unless ``blocking = true``: a type-checker false positive + that refuses a commit trains ``--no-verify``, so the gate is advisory + by default. Opt-in only — runs when listed in ``[tool.forge.precommit] enable``. - When opted in but the chosen checker binary is absent, fails loudly - (the consumer configured it and must fix their environment) rather - than silently passing. + When opted in but ``pyrefly`` is absent, fails loudly (the consumer + opted in and must install it) rather than silently passing. Args: repo_root: Git repo root. Returns: - ``StepResult``; ``skipped`` when ``checker = "none"``, - ``non_blocking`` the inverse of ``blocking``. + ``StepResult`` mirroring pyrefly's exit code; ``non_blocking`` is + the inverse of ``blocking``. A blocking failure when the + configured ``paths`` are option-like or escape the repo. Raises: - SystemExit: If the configured checker binary is not on PATH. + SystemExit: If ``pyrefly`` is not on PATH. """ cfg = _forge_step_config(repo_root, "typecheck") - checker = str(cfg.get("checker") or "pyrefly") paths = cfg.get("paths") or ["src"] blocking = bool(cfg.get("blocking", False)) - if checker == "none": - return StepResult( - name="typecheck", - passed=True, - output="(checker = none — skipped)", - skipped=True, - ) - command = _TYPECHECK_COMMANDS.get(checker) - if command is None: - valid = ", ".join([*_TYPECHECK_COMMANDS, "none"]) + bad = _bad_scan_paths(paths, repo_root) + if bad: return StepResult( name="typecheck", passed=False, - output=f"unknown checker {checker!r} — valid: {valid}", - non_blocking=not blocking, + output=f"invalid [tool.forge.typecheck] paths: {bad}", ) - require_cli(command[0], caller="forge-precommit") - passed, output = _run([*command, *paths], cwd=repo_root) + require_cli("pyrefly", caller="forge-precommit") + passed, output = _run(["pyrefly", "check", *paths], cwd=repo_root) return StepResult( name="typecheck", passed=passed, output=output, non_blocking=not blocking ) diff --git a/src/forge/verify_doc_consistency.py b/src/forge/verify_doc_consistency.py index f2ce9a8..28f2117 100644 --- a/src/forge/verify_doc_consistency.py +++ b/src/forge/verify_doc_consistency.py @@ -118,7 +118,7 @@ def _check_agent_count(repo_root: Path) -> list[str]: """Return a finding when FOUNDATION's agent-count claim disagrees with disk. Counts ``agents/*.md`` files (excluding underscore-prefixed templates) - and compares against the highest ``" foundation agents"`` claim in + and compares against the first ``" foundation agents"`` claim in ``FOUNDATION.md``. Skips silently when the ``agents/`` directory or the claim is absent. diff --git a/tests/test_precommit.py b/tests/test_precommit.py index f1c7c96..c48ee97 100644 --- a/tests/test_precommit.py +++ b/tests/test_precommit.py @@ -872,27 +872,11 @@ def _run(cmd: list[str], **_kw: object) -> tuple[bool, str]: assert result.non_blocking -def test_step_typecheck_none_skips(tmp_path: Path) -> None: - """`checker = "none"` skips the step without invoking any tool.""" - _write_pyproject(tmp_path, '[tool.forge.typecheck]\nchecker = "none"\n') - result = precommit.step_typecheck(tmp_path) - assert result.skipped - assert result.passed - - -def test_step_typecheck_unknown_checker_fails(tmp_path: Path) -> None: - """An unrecognized checker name fails with a message listing valid names.""" - _write_pyproject(tmp_path, '[tool.forge.typecheck]\nchecker = "bogus"\n') - result = precommit.step_typecheck(tmp_path) - assert not result.passed - assert "pyrefly" in result.output - - -def test_step_typecheck_missing_binary_exits( +def test_step_typecheck_missing_pyrefly_exits( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """A configured-but-absent checker binary fails loudly (SystemExit).""" + """An opted-in-but-absent pyrefly binary fails loudly (SystemExit).""" monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) with pytest.raises(SystemExit): precommit.step_typecheck(tmp_path) @@ -902,17 +886,32 @@ def test_step_typecheck_blocking_config( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """`blocking = true` makes a checker error a blocking failure.""" + """`blocking = true` makes a pyrefly error a blocking failure.""" _present(monkeypatch) - _write_pyproject( - tmp_path, '[tool.forge.typecheck]\nchecker = "mypy"\nblocking = true\n' - ) + _write_pyproject(tmp_path, "[tool.forge.typecheck]\nblocking = true\n") monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "error: x")) result = precommit.step_typecheck(tmp_path) assert not result.passed assert not result.non_blocking +def test_step_typecheck_rejects_option_like_paths(tmp_path: Path) -> None: + """An option-like `paths` entry is rejected as a blocking misconfiguration.""" + _write_pyproject(tmp_path, '[tool.forge.typecheck]\npaths = ["--output=x"]\n') + result = precommit.step_typecheck(tmp_path) + assert not result.passed + assert not result.non_blocking + assert "--output=x" in result.output + + +def test_step_doctest_rejects_paths_escaping_repo(tmp_path: Path) -> None: + """A `paths` entry resolving outside the repo is rejected (blocking).""" + _write_pyproject(tmp_path, '[tool.forge.doctest]\npaths = ["/etc"]\n') + result = precommit.step_doctest(tmp_path) + assert not result.passed + assert not result.non_blocking + + def test_step_doc_consistency_non_blocking( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From 5f0aa7327dac814e2be6837e3bc78a285778c9b2 Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 14:48:41 +0200 Subject: [PATCH 3/7] chore(dev): setup.sh installs the [dev] umbrella, not just [test] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit forge's own bootstrap should carry every tool it ships and dogfoods — pytest, the audit deps (vulture/jsonschema/PyYAML), and pyrefly for the opt-in typecheck step. Installing only [test] left audit and typecheck tools missing from contributors' envs. --- dev/setup.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/dev/setup.sh b/dev/setup.sh index 1294c7b..2b1c9ef 100755 --- a/dev/setup.sh +++ b/dev/setup.sh @@ -65,9 +65,12 @@ if [ "${CONDA_DEFAULT_ENV:-}" != "$ENV_NAME" ]; then exit 1 fi -# 4. Editable install with test deps. -echo -e "${YELLOW}→${NC} pip install -e .[test]" -pip install -e ".[test]" +# 4. Editable install with the full contributor umbrella (test + audit + +# typecheck). forge's own dev env should carry every tool it ships and +# dogfoods — pytest, the audit deps (vulture / jsonschema / PyYAML), and +# pyrefly for the opt-in typecheck step. `[dev]` is that umbrella. +echo -e "${YELLOW}→${NC} pip install -e .[dev]" +pip install -e ".[dev]" # 5. Run the consumer-facing umbrella installer. Dogfood: forge is the # source of install-forge-bootstrap, so running it here exercises the From cfe3b3147e514745f5a89224317233c0e474b18c Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 14:51:04 +0200 Subject: [PATCH 4/7] fix(audit): catch tokenize.TokenError, not nonexistent TokenizeError Three except clauses (claims, suppressions, dup) named a stdlib exception that does not exist, causing tokenization failures to raise AttributeError instead of being caught. Add regression test for unterminated source. --- src/forge/audit/claims.py | 2 +- src/forge/audit/dup.py | 2 +- src/forge/audit/suppressions.py | 2 +- tests/audit/test_dup.py | 12 ++++++++++++ 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/forge/audit/claims.py b/src/forge/audit/claims.py index 8ff3dec..535e8c7 100644 --- a/src/forge/audit/claims.py +++ b/src/forge/audit/claims.py @@ -318,7 +318,7 @@ def _comment_findings( evidence=(comment_body[:COMMENT_PREVIEW],), ), ) - except tokenize.TokenizeError as exc: + except tokenize.TokenError as exc: logger.debug("tokenize failed in %s: %s", rel, exc) return findings diff --git a/src/forge/audit/dup.py b/src/forge/audit/dup.py index 67bc19e..20e8e04 100644 --- a/src/forge/audit/dup.py +++ b/src/forge/audit/dup.py @@ -177,7 +177,7 @@ def _tokenize_body(source: str) -> list[str]: tokens.append(tok.string if keyword.iskeyword(tok.string) else "ID") else: tokens.append(tok.string) - except tokenize.TokenizeError: + except tokenize.TokenError: logger.debug("tokenize failed on a snippet — skipping") return [] return tokens diff --git a/src/forge/audit/suppressions.py b/src/forge/audit/suppressions.py index 13205ff..263468b 100644 --- a/src/forge/audit/suppressions.py +++ b/src/forge/audit/suppressions.py @@ -283,7 +283,7 @@ def _iter_comments(text: str) -> list[tuple[int, str]]: seen_lines.add(line_no) if 1 <= line_no <= len(source_lines): pairs.append((line_no, source_lines[line_no - 1])) - except tokenize.TokenizeError as exc: + except tokenize.TokenError as exc: logger.debug("tokenize failed: %s", exc) return pairs diff --git a/tests/audit/test_dup.py b/tests/audit/test_dup.py index 8ceab1e..b0e5418 100644 --- a/tests/audit/test_dup.py +++ b/tests/audit/test_dup.py @@ -107,6 +107,18 @@ def test_tokenize_body_collapses_strings_and_numbers() -> None: assert "42" not in tokens +def test_tokenize_body_returns_empty_on_token_error() -> None: + """Unterminated source is swallowed and yields ``[]``, not an exception. + + Regression: the handler caught ``tokenize.TokenizeError`` — a name that + does not exist in the stdlib (the real exception is + ``tokenize.TokenError``) — so a tokenization failure raised + ``AttributeError`` instead of being caught. The unclosed parenthesis + below makes ``tokenize`` raise ``TokenError`` at EOF. + """ + assert _tokenize_body("x = (1 +") == [] + + def test_shingles_returns_empty_when_shorter_than_k() -> None: """A token sequence below ``k`` length yields no shingles.""" assert _shingles(["a", "b"], k=5) == frozenset() From 4ae47b44bb9b20e5b1a61c01ac36b984d334dfdf Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 14:55:19 +0200 Subject: [PATCH 5/7] =?UTF-8?q?docs(claude):=20add=20PR=20&=20branch=20gra?= =?UTF-8?q?nularity=20guideline=20=E2=80=94=20bundle=20cohesive=20work?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Default to one PR for related changes; splitting multiplies rolling-next version-bump + tag ceremony for isolation nobody needs. Split only for concrete reasons (independent risk, blocking dep, oversized diff). Separate issues for deferred work are cheap; separate PRs add burden. --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index ecaeaa0..277ea8e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,6 +13,7 @@ ## Forge-specific rules - **Version derivation**: pip package version comes from the latest git tag via setuptools-scm. There is no manual `version = "x.y.z"` in `pyproject.toml`. Release flow: `git tag vX.Y.Z && git push origin vX.Y.Z`. +- **PR & branch granularity — bundle cohesive work, don't reflex-split.** A PR is the smallest *cohesive* unit, not the smallest possible one. In forge every PR also costs a rolling-next `plugin.json` bump + a `dev` tag (and, at minor boundaries, a promotion), so splitting *related* changes across PRs multiplies that ceremony to buy isolation nobody needs. **Default to ONE PR** when the changes share a subsystem, must land together, or one half is meaningless without the other — e.g. a bug a type checker surfaces *while you build the feature that runs it* belongs in that feature's PR, not a parallel one; a follow-up cleanup to code this PR introduces belongs here too. **Split only for a concrete reason:** independent risk/rollback, different reviewers or release cadence, a hard blocking dependency, or a diff too large to review in one sitting. "It's a separate concern" does **not** justify a separate PR when the concerns ship together — separateness of *topic* ≠ separateness of *delivery*. When unsure, bundle. (File a separate *issue* freely for deferred work — that's cheap; it's separate *PRs/branches* that add the burden.) - **Release process — versioning, tag cadence, and `dev → main` promotion** are specified in **[`docs/release-process.md`](docs/release-process.md)**, the single source of truth + the **invariant→test contract**. Do not restate its mechanics elsewhere; point here. Operational summary: - `.claude-plugin/plugin.json["version"]` is **rolling-next** — always names the version about to be released; `step_plugin_version` enforces `plugin.json > latest_tag` on every commit (skipped when `HEAD` reproduces a tagged release's tree). After tagging `vX.Y.Z`, the next PR must bump it. - Tag `dev` `vX.Y.Z` after **every** merge to dev (`forge-next-prep --tag`, run by `/next` — **never drop `--tag`**); tag `main` **only** at the minor-boundary promotion. `@dev` gets every version, `@main` minors only. From d46bd3aa264b7111e2e7ca02c65a142e316ae20a Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 15:47:40 +0200 Subject: [PATCH 6/7] =?UTF-8?q?refactor(precommit):=20address=20PR=20revie?= =?UTF-8?q?w=20=E2=80=94=20path=20guard,=20only/skip,=20doc=20accuracy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _bad_scan_paths rejects empty/whitespace paths; --skip now subtracts from the --only set too (never silently ignored) - Drop the dead "no items ran" branch; clarify the run_all module-dispatch comment - Fix _bad_scan_paths type + doc_consistency docstring accuracy; pin pyrefly >=1,<2 --- docs/api-digest.md | 2 +- pyproject.toml | 2 +- src/forge/precommit.py | 51 +++++++++++++++++++++++------------------ tests/test_precommit.py | 10 ++++++++ 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/docs/api-digest.md b/docs/api-digest.md index a6323c2..89f12c9 100644 --- a/docs/api-digest.md +++ b/docs/api-digest.md @@ -385,7 +385,7 @@ _41 modules, 375 symbols._ - `step_cli_wiring(repo_root: Path) -> StepResult` — Run ``verify-forge-cli-wiring`` — assert every script has a real caller. - `_cli_wiring_enabled(repo_root: Path) -> bool` _(internal)_ — Return True when the repo has opted into the cli_wiring check. - `step_plugin_version(repo_root: Path) -> StepResult` — Run ``verify-forge-plugin-version`` — owns the rolling-next guard. -- `_bad_scan_paths(paths: list[str], repo_root: Path) -> list[str]` _(internal)_ — Return config scan-path values that are option-like or escape the repo. +- `_bad_scan_paths(paths: list[object], repo_root: Path) -> list[str]` _(internal)_ — Return config scan-path values that are option-like or escape the repo. - `step_doctest(repo_root: Path) -> StepResult` — Run ``pytest --doctest-modules`` over docstring examples (opt-in). - `step_typecheck(repo_root: Path) -> StepResult` — Run pyrefly over the source tree (opt-in). - `step_doc_consistency(repo_root: Path) -> StepResult` — Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). diff --git a/pyproject.toml b/pyproject.toml index 8b46550..c3c90a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,7 +62,7 @@ test = ["pytest>=8.0"] audit = ["vulture>=2.10", "jsonschema>=4.0", "PyYAML>=6.0"] audit-tach = ["tach>=0.30"] # Type checker for the opt-in `typecheck` pre-commit step (pyrefly only). -typecheck = ["pyrefly>=0.1"] +typecheck = ["pyrefly>=1,<2"] # Contributor / dev-loop umbrella. Aggregates the targeted extras so # every reference to `pip install -e ".[dev]"` in forge's own source, # docs, hook error messages, and test assertions resolves to a real diff --git a/src/forge/precommit.py b/src/forge/precommit.py index 1ddd6a1..e17c0fd 100644 --- a/src/forge/precommit.py +++ b/src/forge/precommit.py @@ -563,7 +563,7 @@ def step_plugin_version(repo_root: Path) -> StepResult: ) -def _bad_scan_paths(paths: list[str], repo_root: Path) -> list[str]: +def _bad_scan_paths(paths: list[object], repo_root: Path) -> list[str]: """Return config scan-path values that are option-like or escape the repo. Config-supplied scan roots (``[tool.forge.] paths``) are spliced @@ -580,7 +580,7 @@ def _bad_scan_paths(paths: list[str], repo_root: Path) -> list[str]: """ bad: list[str] = [] for raw in paths: - if not isinstance(raw, str) or raw.startswith("-"): + if not isinstance(raw, str) or not raw.strip() or raw.startswith("-"): bad.append(str(raw)) continue try: @@ -631,7 +631,9 @@ def step_doctest(repo_root: Path) -> StepResult: ["pytest", "--doctest-modules", "--doctest-continue-on-failure", *paths], cwd=repo_root, ) - if "no tests ran" in output or "no items ran" in output: + # pytest>=8 prints "no tests ran" when zero examples were collected + # (exit 5) — a skip for an advisory doctest sweep, not a failure. + if "no tests ran" in output: return StepResult(name="doctest", passed=True, output=output, skipped=True) return StepResult( name="doctest", passed=passed, output=output, non_blocking=not blocking @@ -684,11 +686,12 @@ def step_typecheck(repo_root: Path) -> StepResult: def step_doc_consistency(repo_root: Path) -> StepResult: """Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). - Checks the machine-checkable subset of documentation claims: agent / - skill / CLI / hook name-lists and counts asserted in docs against the - actual files on disk and ``[project.scripts]`` / ``plugin.json``. - Opt-in via ``[tool.forge.precommit] enable``; non-blocking (doc drift - is a warning, not grounds to refuse a commit). + Checks the machine-checkable subset of documentation claims: every + ``[project.scripts]`` CLI name against ``docs/cli-reference.md``, and + the ``" foundation agents"`` count in ``FOUNDATION.md`` against the + actual ``agents/*.md`` count on disk. Opt-in via + ``[tool.forge.precommit] enable``; non-blocking (doc drift is a + warning, not grounds to refuse a commit). Args: repo_root: Git repo root. @@ -793,13 +796,16 @@ def _resolve_steps( ) -> list[StepDef]: """Resolve which steps to run, in registry order. - ``--only`` overrides everything (run exactly those). Otherwise the run - set is the default-on steps plus ``[tool.forge.precommit] enable``, - minus ``[tool.forge.precommit] disable`` and ``skip``. An explicit - exclusion wins: ``disable`` / ``skip`` beat ``enable`` when a name - appears in both. Every referenced name is validated against the - registry; an unknown name raises ``ValueError`` so the caller prints - one clean message instead of leaking a traceback on a config typo. + ``--only`` selects the base set (those steps instead of the defaults); + otherwise the base set is the default-on steps plus + ``[tool.forge.precommit] enable``, minus ``[tool.forge.precommit] + disable``. ``skip`` then subtracts from **either** base — so + ``--only X --skip X`` runs nothing, and ``--skip`` is never silently + ignored. An explicit exclusion wins: ``disable`` / ``skip`` beat + ``enable`` when a name appears in both. Every referenced name is + validated against the registry; an unknown name raises ``ValueError`` + so the caller prints one clean message instead of leaking a traceback + on a config typo. Args: repo_root: Git repo root. @@ -816,10 +822,8 @@ def _resolve_steps( enable = list(precommit_cfg.get("enable") or []) disable = list(precommit_cfg.get("disable") or []) _validate_step_names([*enable, *disable, *skip, *only]) - if only: - chosen = set(only) - else: - chosen = (_DEFAULT_ON | set(enable)) - set(disable) - set(skip) + base = set(only) if only else (_DEFAULT_ON | set(enable)) - set(disable) + chosen = base - set(skip) return [d for d in _STEP_REGISTRY if d.name in chosen] @@ -854,9 +858,12 @@ def run_all( results: list[StepResult] = [] this_module = sys.modules[__name__] for step_def in _resolve_steps(root, skip=skip, only=only): - # Resolve the step function through the module namespace (not the - # registry's captured reference) so tests can monkeypatch a - # ``step_*`` function and have run_all pick up the stub. + # Resolve each step by name through the module namespace rather than + # calling ``step_def.fn`` directly. The registry captured the + # original function objects at import time, so a test that does + # ``monkeypatch.setattr(precommit, "step_ruff", stub)`` would be + # invisible to a direct ``step_def.fn`` call. Re-resolving by name is + # the load-bearing seam that keeps per-step monkeypatching working. fn = getattr(this_module, step_def.fn.__name__) result = fn(root) if print_progress: diff --git a/tests/test_precommit.py b/tests/test_precommit.py index c48ee97..e058177 100644 --- a/tests/test_precommit.py +++ b/tests/test_precommit.py @@ -693,6 +693,16 @@ def test_resolve_steps_only_overrides_in_registry_order(tmp_path: Path) -> None: assert resolved == ["ruff", "pip_audit"] +def test_resolve_steps_only_still_honors_skip(tmp_path: Path) -> None: + """`skip` subtracts from the `only` set too — it is never silently ignored.""" + resolved = _names( + precommit._resolve_steps( + tmp_path, only=["ruff", "pip_audit"], skip=["pip_audit"] + ) + ) + assert resolved == ["ruff"] + + def test_resolve_steps_unknown_name_raises(tmp_path: Path) -> None: """An unknown name in config / skip / only raises ValueError.""" with pytest.raises(ValueError, match="unknown step name"): From dcb7ad9f6d2f01a01e3fbdb4e4a55de92bdf3d98 Mon Sep 17 00:00:00 2001 From: Jean Date: Wed, 17 Jun 2026 16:06:28 +0200 Subject: [PATCH 7/7] refactor(doc-consistency): drop brittle agent-count check, keep CLI coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The " foundation agents" check needed a hardcoded number-word table and depended on exact prose phrasing — maintenance burden for low signal. verify-forge-doc-consistency now does only the robust check: every [project.scripts] CLI is documented in docs/cli-reference.md. --- CHANGELOG.md | 4 +- REPO_STRUCTURE.md | 2 +- docs/api-digest.md | 3 +- docs/configuration.md | 4 +- src/forge/precommit.py | 8 +- src/forge/verify_doc_consistency.py | 114 +++++---------------------- tests/test_verify_doc_consistency.py | 55 ++----------- 7 files changed, 36 insertions(+), 154 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fb9419..40c5aa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,8 +34,8 @@ All additive and opt-in — no consumer action required to upgrade. - **Opt-in `typecheck` step** — runs `pyrefly` over `[tool.forge.typecheck].paths`; non-blocking by default (#48). - **Opt-in `doc_consistency` step** + `verify-forge-doc-consistency` CLI — - checks CLI name-lists and agent counts asserted in docs against the - actual repo state; non-blocking (#4). + checks that every `[project.scripts]` CLI is documented in + `docs/cli-reference.md`; non-blocking (#4). ### Tooling - `forge-config --list` now enumerates the new diff --git a/REPO_STRUCTURE.md b/REPO_STRUCTURE.md index 68463be..dc30f6e 100644 --- a/REPO_STRUCTURE.md +++ b/REPO_STRUCTURE.md @@ -37,7 +37,7 @@ Code. structure drift check - verify_test_naming.py: `verify-forge-test-naming` — test naming check - verify_manifest.py: `verify-forge-manifest` — `.claude-plugin/*.json` JSON validation - - verify_doc_consistency.py: `verify-forge-doc-consistency` — checks machine-checkable doc claims (CLI name-lists, agent counts) against repo state; backs the opt-in `doc_consistency` pre-commit step (non-blocking) + - verify_doc_consistency.py: `verify-forge-doc-consistency` — checks every `[project.scripts]` CLI is documented in `docs/cli-reference.md`; backs the opt-in `doc_consistency` pre-commit step (non-blocking) - verify_plugin_version.py: `verify-forge-plugin-version` — rolling-next guard (plugin.json["version"] > latest git tag) - gen_cli_reference.py: `forge-gen-cli-reference` — CLI reference doc generator diff --git a/docs/api-digest.md b/docs/api-digest.md index 89f12c9..2547ab9 100644 --- a/docs/api-digest.md +++ b/docs/api-digest.md @@ -4,7 +4,7 @@ A compact index of this codebase's symbols — every top-level function and clas > **Generated file — do not edit by hand.** Regenerate with `forge-gen-api-digest`; check for drift with `forge-gen-api-digest --check`. -_41 modules, 375 symbols._ +_41 modules, 374 symbols._ ## `forge._hook_helpers` @@ -445,7 +445,6 @@ _41 modules, 375 symbols._ ## `forge.verify_doc_consistency` - `_check_cli_coverage(repo_root: Path) -> list[str]` _(internal)_ — Return findings for ``[project.scripts]`` names missing from the CLI reference. -- `_check_agent_count(repo_root: Path) -> list[str]` _(internal)_ — Return a finding when FOUNDATION's agent-count claim disagrees with disk. - `main() -> int` — CLI entry point. ## `forge.verify_docstring_coverage` diff --git a/docs/configuration.md b/docs/configuration.md index 3777d21..1a3d8dc 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -99,8 +99,8 @@ commit trains `--no-verify`. | `blocking` | `false` | Fail the commit on a type error (else non-blocking WARN). | Your type baseline is clean and you want it enforced. | The `doc_consistency` step (`verify-forge-doc-consistency`, enabled the -same way) has no config table — it checks CLI name-lists and agent counts -in docs against repo state, and is always non-blocking. +same way) has no config table — it checks that every `[project.scripts]` +CLI is documented in `docs/cli-reference.md`, and is always non-blocking. ## `[tool.forge.docstring_coverage]` diff --git a/src/forge/precommit.py b/src/forge/precommit.py index e17c0fd..33a2cec 100644 --- a/src/forge/precommit.py +++ b/src/forge/precommit.py @@ -687,11 +687,9 @@ def step_doc_consistency(repo_root: Path) -> StepResult: """Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). Checks the machine-checkable subset of documentation claims: every - ``[project.scripts]`` CLI name against ``docs/cli-reference.md``, and - the ``" foundation agents"`` count in ``FOUNDATION.md`` against the - actual ``agents/*.md`` count on disk. Opt-in via - ``[tool.forge.precommit] enable``; non-blocking (doc drift is a - warning, not grounds to refuse a commit). + ``[project.scripts]`` CLI name appears in ``docs/cli-reference.md``. + Opt-in via ``[tool.forge.precommit] enable``; non-blocking (doc drift + is a warning, not grounds to refuse a commit). Args: repo_root: Git repo root. diff --git a/src/forge/verify_doc_consistency.py b/src/forge/verify_doc_consistency.py index 28f2117..2a6f7c0 100644 --- a/src/forge/verify_doc_consistency.py +++ b/src/forge/verify_doc_consistency.py @@ -2,25 +2,21 @@ Backs the opt-in ``doc_consistency`` pre-commit step. Nothing in forge otherwise checks documentation for factual drift against the repo: a doc -can claim "ten foundation agents" or list a CLI that no longer exists, -and only a careful human reread catches it. This CLI closes the -**structured, NLP-free subset** of that hole — names and counts a doc -asserts about the repo's own inventory, verified against the filesystem -and ``pyproject.toml``. - -Checks (each self-skips when its inputs are absent, so the CLI is safe in -any repo): - -1. **CLI coverage** — every ``[project.scripts]`` entry name appears at - least once in ``docs/cli-reference.md``. A new CLI added without a doc - line is drift. -2. **Agent count** — a ``" foundation agents"`` claim in - ``FOUNDATION.md`` matches the number of ``agents/*.md`` files - (excluding the underscore-prefixed ``_TEMPLATE.md``). - -Scope is deliberately conservative for v1: name-lists and counts only. -Internal-link validation and repo-state facts (visibility, default -branch) are intentionally out of scope — they are tracked separately. +can list a CLI that no longer exists (or omit a new one) and only a +careful human reread catches it. This CLI closes the **structured, +NLP-free subset** of that hole. + +Check (self-skips when its inputs are absent, so the CLI is safe in any +repo): + +- **CLI coverage** — every ``[project.scripts]`` entry name appears at + least once in ``docs/cli-reference.md``. A CLI added or removed without + a matching doc line is drift. + +Scope is deliberately conservative for v1: the one robust, no-NLP, +no-maintenance check. Name-list/count checks that depend on prose +phrasing, internal-link validation, and repo-state facts (visibility, +default branch) are intentionally out of scope — tracked separately. Exit code: ``0`` when consistent or nothing to check, ``1`` when any drift is found. The ``doc_consistency`` step renders a non-zero result as @@ -31,7 +27,6 @@ import argparse import logging -import re import sys import tomllib from typing import TYPE_CHECKING @@ -48,39 +43,6 @@ logger = logging.getLogger(__name__) -# Spelled-out cardinals forge docs actually use for inventory counts. -# Digits are matched directly; this map covers the prose form. -_NUMBER_WORDS: dict[str, int] = { - "zero": 0, - "one": 1, - "two": 2, - "three": 3, - "four": 4, - "five": 5, - "six": 6, - "seven": 7, - "eight": 8, - "nine": 9, - "ten": 10, - "eleven": 11, - "twelve": 12, - "thirteen": 13, - "fourteen": 14, - "fifteen": 15, - "sixteen": 16, - "seventeen": 17, - "eighteen": 18, - "nineteen": 19, - "twenty": 20, -} - -# " foundation agents" — count as a digit run or a spelled cardinal. -_AGENT_COUNT_RE = re.compile( - r"\b(\d+|" + "|".join(_NUMBER_WORDS) + r")\s+foundation agents\b", - re.IGNORECASE, -) - - def _check_cli_coverage(repo_root: Path) -> list[str]: """Return findings for ``[project.scripts]`` names missing from the CLI reference. @@ -114,57 +76,23 @@ def _check_cli_coverage(repo_root: Path) -> list[str]: ] -def _check_agent_count(repo_root: Path) -> list[str]: - """Return a finding when FOUNDATION's agent-count claim disagrees with disk. - - Counts ``agents/*.md`` files (excluding underscore-prefixed templates) - and compares against the first ``" foundation agents"`` claim in - ``FOUNDATION.md``. Skips silently when the ``agents/`` directory or the - claim is absent. - - Args: - repo_root: Git repo root. - - Returns: - A single-element finding list on mismatch; empty when consistent - or not applicable. - """ - agents_dir = repo_root / "agents" - foundation = repo_root / "FOUNDATION.md" - if not agents_dir.is_dir() or not foundation.is_file(): - return [] - actual = sum(1 for path in agents_dir.glob("*.md") if not path.name.startswith("_")) - match = _AGENT_COUNT_RE.search(foundation.read_text(encoding="utf-8")) - if match is None: - return [] - token = match.group(1).lower() - claimed = int(token) if token.isdigit() else _NUMBER_WORDS[token] - if claimed != actual: - return [ - f"FOUNDATION.md claims {claimed} foundation agents, " - f"but agents/ holds {actual}" - ] - return [] - - def main() -> int: """CLI entry point. Returns: - ``0`` when every applicable check is consistent (or nothing - applies); ``1`` when any drift is found. + ``0`` when the check is consistent (or nothing applies); ``1`` when + drift is found. """ argparse.ArgumentParser( prog="verify-forge-doc-consistency", description=( - "Check machine-checkable documentation claims (CLI name-lists, " - "agent counts) against the actual repo state. Non-blocking " - "reporter for the doc_consistency pre-commit step." + "Check that every [project.scripts] CLI is documented in " + "docs/cli-reference.md. Non-blocking reporter for the " + "doc_consistency pre-commit step." ), ).parse_args() - repo_root = get_repo_root() - findings = _check_cli_coverage(repo_root) + _check_agent_count(repo_root) + findings = _check_cli_coverage(get_repo_root()) if findings: logger.error("Documentation drift detected:") for finding in findings: diff --git a/tests/test_verify_doc_consistency.py b/tests/test_verify_doc_consistency.py index 9f3e43c..65ff5d5 100644 --- a/tests/test_verify_doc_consistency.py +++ b/tests/test_verify_doc_consistency.py @@ -30,20 +30,6 @@ def _write(path: Path, text: str) -> None: path.write_text(text, encoding="utf-8") -def _make_agents(tmp_path: Path, count: int) -> None: - """Create *count* agent files plus an excluded ``_TEMPLATE.md``. - - Args: - tmp_path: Repo root to populate. - count: Number of real ``agents/*.md`` files to create. - """ - agents = tmp_path / "agents" - agents.mkdir(parents=True, exist_ok=True) - for i in range(count): - (agents / f"agent{i}.md").write_text("x", encoding="utf-8") - (agents / "_TEMPLATE.md").write_text("template", encoding="utf-8") - - def test_cli_coverage_skips_without_inputs(tmp_path: Path) -> None: """No pyproject or no cli-reference → nothing to check.""" assert vdc._check_cli_coverage(tmp_path) == [] @@ -75,40 +61,11 @@ def test_cli_coverage_malformed_pyproject_skips(tmp_path: Path) -> None: assert vdc._check_cli_coverage(tmp_path) == [] -def test_agent_count_skips_without_inputs(tmp_path: Path) -> None: - """No agents/ directory or no FOUNDATION.md → nothing to check.""" - assert vdc._check_agent_count(tmp_path) == [] - - -def test_agent_count_clean_on_word_match(tmp_path: Path) -> None: - """A spelled-out count matching the file count passes.""" - _make_agents(tmp_path, 3) - _write(tmp_path / "FOUNDATION.md", "The three foundation agents are: a, b, c.") - assert vdc._check_agent_count(tmp_path) == [] - - -def test_agent_count_clean_on_digit_match_excluding_template(tmp_path: Path) -> None: - """A digit count matching passes, and ``_TEMPLATE.md`` is not counted.""" - _make_agents(tmp_path, 12) - _write(tmp_path / "FOUNDATION.md", "forge ships 12 foundation agents total.") - assert vdc._check_agent_count(tmp_path) == [] - - -def test_agent_count_flags_mismatch(tmp_path: Path) -> None: - """A claim that disagrees with the actual file count is reported.""" - _make_agents(tmp_path, 5) - _write(tmp_path / "FOUNDATION.md", "the ten foundation agents are listed below") - findings = vdc._check_agent_count(tmp_path) - assert len(findings) == 1 - assert "10" in findings[0] - assert "5" in findings[0] - - def test_main_returns_zero_when_consistent( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """main() exits 0 on an empty repo — every check skips, nothing drifts. + """main() exits 0 on an empty repo — the check skips, nothing drifts. MOCK SETUP: get_repo_root pinned to an empty tmp_path; argv patched so argparse does not consume pytest's arguments. @@ -122,13 +79,13 @@ def test_main_returns_one_on_drift( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """main() exits 1 when a check reports drift. + """main() exits 1 when CLI coverage drifts. - MOCK SETUP: a repo whose FOUNDATION claims nine agents but holds two; - get_repo_root pinned to it and argv patched. + MOCK SETUP: a repo with a [project.scripts] CLI absent from the + reference doc; get_repo_root pinned to it and argv patched. """ - _make_agents(tmp_path, 2) - _write(tmp_path / "FOUNDATION.md", "the nine foundation agents are") + _write(tmp_path / "pyproject.toml", '[project.scripts]\nundocumented = "x:main"\n') + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n") monkeypatch.setattr(vdc, "get_repo_root", lambda: tmp_path) monkeypatch.setattr(vdc.sys, "argv", ["verify-forge-doc-consistency"]) assert vdc.main() == 1