Skip to content

feat(precommit): pluggable step framework + doctest/typecheck/doc-consistency steps#55

Merged
misnaej merged 7 commits into
devfrom
feat/precommit-step-framework
Jun 17, 2026
Merged

feat(precommit): pluggable step framework + doctest/typecheck/doc-consistency steps#55
misnaej merged 7 commits into
devfrom
feat/precommit-step-framework

Conversation

@misnaej

@misnaej misnaej commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Closes #6, #5, #48, #46. Folds in the tokenize bug fix (ex-#56). Addresses #4 (v1 scope). Follow-up: #57 (dogfood pyrefly on forge).

One cohesive PR for the pre-commit step subsystem — per CLAUDE.md's new PR-granularity rule (also added here), related work ships together rather than fragmenting into parallel PRs.

Summary

Turns the hardcoded step tuple into a registry with a uniform on/off layer, ships three opt-in pluggable steps, and folds in a real bug those steps surfaced. MINOR → v1.24.0.

Changes

Testing

~38 new tests (framework precedence, all three steps + edge cases, the CLI, #46 drift, tokenize regression). Full suite green. Dogfood: verify-forge-doc-consistency passes on forge.

Breaking Changes

None — every new step is opt-in and off by default; existing behavior unchanged.

misnaej added 2 commits June 17, 2026 14:07
…sistency 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
- 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
misnaej added 4 commits June 17, 2026 14:48
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.
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.
… work

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.
…ccuracy

- _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
@misnaej

misnaej commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

Squash-merge message (copy verbatim):

feat(precommit): pluggable step framework + opt-in steps

- StepDef registry with --only/--skip/enable/disable overrides (closes #6)
- Three opt-in steps: doctest, pyrefly typecheck, doc-consistency CLI (closes #5, #48, #4)
- Config-path validation; CONFIG_KEYS drift tests (closes #46)
- Fix tokenize.TokenError in audit modules; regression test (closes #56)

@misnaej

misnaej commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

PR Verification Results

verified-at: d46bd3a (PR #55, branch feat/precommit-step-framework)

Pre-run reports were generated at commit 4ae47b4. All LOW/MEDIUM findings were addressed in d46bd3a (current HEAD). Reports used directly per the pre-run fast-path.

Design Check

PASS (minor issues resolved). Clean additive: StepDef registry, _resolve_steps, --only/--skip, three opt-in steps, verify-forge-doc-consistency CLI. MEDIUM finding (run_all dispatches via module namespace for test monkeypatching) kept by deliberate choice — refactoring to monkeypatch the registry would churn production + tests for a style preference; accepted with improved explanatory comment. LOWs resolved: dead "no items ran" branch removed; _bad_scan_paths annotation fixed to list[object]; --skip now subtracts from --only set.

Security Review

PASS. All subprocess calls list-form with shell=False; config paths validated via Path.resolve().relative_to(repo_root). LOWs resolved: _bad_scan_paths now rejects empty/whitespace paths; pyrefly pinned >=1,<2. Tokenize fix is a correctness fix with no security impact.

Documentation Check

PASS. _bad_scan_paths annotation corrected to list[object]; step_doc_consistency docstring trimmed to reflect v1 scope (CLI names + agent counts only, not skill/hook/plugin.json). No remaining errors.

Issue Management

PR description carries bare Closes #6, #5, #48, #46 on the first line — auto-close wired correctly. #56 already closed prior to this PR landing. #4 addressed in v1 scope (name-lists + counts) via Addresses #4 — intentionally not auto-closed; v2+ work tracked in open issue. Follow-up #57 (dogfood pyrefly on forge, strict config + triage remaining type findings) filed and open — no action needed here.

Code Quality (from code_health/ logs)

  • Ruff: PASS — 87 files unchanged, all checks passed
  • Docstring verification: PASS — 0 errors, 0 warnings, 5 info (verbose fixture docstrings, non-blocking)
  • Docstring coverage: PASS — 100.0% (1199/1199 symbols)
  • Test naming: PASS — 27 files, 0 issues
  • Repo structure: PASS — 133 paths in sync
  • Manifest JSON: PASS
  • CLI wiring: PASS
  • Commit types parity: PASS
  • Plugin version: PASS — plugin.json 1.24.0 > latest tag 1.23.2
  • pip_audit: SKIP (pip-audit not on PATH)
  • CI: SUCCESS (GitHub Actions ci check at d46bd3a)

Recommendation: Ready for merge. Squash-merge message posted above.

…overage

The "<N> 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.
@misnaej

misnaej commented Jun 17, 2026

Copy link
Copy Markdown
Owner Author

Follow-up (dcb7ad9): trimmed doc_consistency scope further per review. Dropped the agent-count check entirely — it needed a hardcoded 0–20 number-word table and depended on exact prose phrasing ("twelve foundation agents"), a maintenance burden for low signal. verify-forge-doc-consistency now does only the robust, zero-maintenance check: every [project.scripts] CLI is documented in docs/cli-reference.md. Squash-merge message above is unaffected.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant