diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 794ed17a..a0ca65f2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,11 +45,15 @@ jobs: pip install -r requirements.txt -r requirements-test.txt pip install git+https://github.com/huggingface/diffusers - name: Format check - run: ruff format --check dw dw_mcp tests + run: ruff format --check dw dw_mcp tests scripts - name: Lint - run: ruff check dw dw_mcp tests + run: ruff check dw dw_mcp tests scripts - name: Tests run: pytest -q + # Guards the architecture metrics against drifting past the committed + # baseline (modules, size ceiling, cycles); see docs/stabilization/ + - name: Architecture ratchet + run: python scripts/arch_metrics.py --check docs/stabilization/baseline.json ui: runs-on: ubuntu-latest diff --git a/docs/stabilization/ROADMAP.md b/docs/stabilization/ROADMAP.md index 4c7acdd3..dea17876 100644 --- a/docs/stabilization/ROADMAP.md +++ b/docs/stabilization/ROADMAP.md @@ -22,6 +22,7 @@ Two kinds, kept small on purpose. **Ratchets.** These live in `scripts/arch_metrics.py`. Each is lower-is-better, and `--check` against `baseline.json` fails the build when one gets worse. +- Module size is a band (Phase 4b): the ratchet `modules_over_size_ceiling` counts modules over 1,100 lines, and a module between 1,001 and 1,100 prints a non-failing `warning:` line on every run (never in `baseline.json`). - Phase 0 set: modules, modules over 1,000 lines, functions over 150 lines, reference-prefix literals, test `patch("dw...")` targets, CLAUDE.md lines, duplicate blocks. - Added at the start of Phase 1 ("metrics v2"), re-baselined in the same commit: - **Cyclomatic complexity:** functions above 15, by ruff's C901 (already a dev dependency). Baseline 21. diff --git a/docs/stabilization/baseline.json b/docs/stabilization/baseline.json index 2e014edf..55241310 100644 --- a/docs/stabilization/baseline.json +++ b/docs/stabilization/baseline.json @@ -1,6 +1,6 @@ { "modules": 165, - "modules_over_1000_lines": 0, + "modules_over_size_ceiling": 0, "functions_over_150_lines": 0, "prefix_literals": 0, "prefix_handling": 0, diff --git a/scripts/arch_metrics.py b/scripts/arch_metrics.py index 16c95401..9b15ddb2 100644 --- a/scripts/arch_metrics.py +++ b/scripts/arch_metrics.py @@ -7,6 +7,10 @@ Counting rules, fixed so any commit measures the same way: - Sources are dw/ and dw_mcp/, minus EXCLUDED (vendored community pipelines). +- Module size is a band: modules_over_size_ceiling counts modules above + SIZE_CEILING (1,100 lines, ratcheted); a module above SIZE_WARNING (1,000) + and at or under the ceiling is printed as a `warning:` line on every run, + which is not a metric and never enters the baseline. - Cyclomatic complexity is ruff's C901 (mccabe) as ruff reports it: each function scored on its own body, nested functions counted into it too. - An import cycle is a strongly connected component of more than one module @@ -68,6 +72,13 @@ ("startswith", "removeprefix", "removesuffix", "replace", "split", "partition") ) PATCH_TARGET = re.compile(r"""patch\(\s*["']dw[._]""") +# Module size is a band: warning above SIZE_WARNING, failing above SIZE_CEILING +SIZE_WARNING = 1000 +SIZE_CEILING = 1100 +RERUN_RULE = ( + "Re-baseline rule: lower baseline.json freely when a ratchet improves;", + "raise it only in a commit whose message names the rise and why.", +) COMPLEXITY_LIMIT = 15 COMPLEXITY_MESSAGE = re.compile(r"^`(?P.+)` is too complex \((?P\d+) > 0\)$") @@ -405,7 +416,7 @@ def measure(root): engine = list(_sources(root, *PACKAGES)) metrics = { "modules": len(engine), - "modules_over_1000_lines": 0, + "modules_over_size_ceiling": 0, "functions_over_150_lines": 0, "prefix_literals": 0, "prefix_handling": 0, @@ -413,8 +424,8 @@ def measure(root): constants, prefixes = _reference_names(root.resolve()) for path in engine: text = path.read_text(encoding="utf-8") - if len(text.splitlines()) > 1000: - metrics["modules_over_1000_lines"] += 1 + if len(text.splitlines()) > SIZE_CEILING: + metrics["modules_over_size_ceiling"] += 1 tree = ast.parse(text) if path.relative_to(root).as_posix() not in PREFIX_OWNERS: metrics["prefix_handling"] += len( @@ -449,6 +460,20 @@ def measure(root): return metrics +def size_warnings(root): + """[(path relative to root, lines)] for each measured module in the warn + band: above SIZE_WARNING and at most SIZE_CEILING. Counted the way + measure() counts, so a module is in the band or over the ceiling, never + both.""" + root = pathlib.Path(root) + found = [] + for path in _sources(root, *PACKAGES): + lines = len(path.read_text(encoding="utf-8").splitlines()) + if SIZE_WARNING < lines <= SIZE_CEILING: + found.append((path.relative_to(root).as_posix(), lines)) + return found + + def regressions(current, baseline): worse = [] for name, before in baseline.items(): @@ -473,12 +498,19 @@ def main(argv=None): args = parser.parse_args(argv) current = measure(args.root) print(json.dumps(current, indent=2)) + for path, lines in size_warnings(args.root): + print( + f"warning: {path} is {lines:,} lines " + f"(warn above {SIZE_WARNING:,}, fail above {SIZE_CEILING:,})" + ) if args.write: pathlib.Path(args.write).write_text(json.dumps(current, indent=2) + "\n") if args.check: worse = regressions(current, json.loads(pathlib.Path(args.check).read_text())) for line in worse: print(line) + if worse: + print("\n".join(RERUN_RULE)) return 1 if worse else 0 return 0 diff --git a/scripts/arch_report.py b/scripts/arch_report.py index acddd42d..a4c25781 100644 --- a/scripts/arch_report.py +++ b/scripts/arch_report.py @@ -29,7 +29,7 @@ ARCHIVED = ["dw", "dw_mcp", "tests", "ui/src", ":(glob)**/CLAUDE.md"] RATCHETS = [ ("modules", "Engine + MCP modules"), - ("modules_over_1000_lines", "Modules over 1,000 lines"), + ("modules_over_size_ceiling", "Modules over 1,100 lines (ceiling)"), ("functions_over_150_lines", "Functions over 150 lines"), ("complex_functions", "Functions over cyclomatic complexity 15"), ("import_cycles", "Import cycles"), diff --git a/scripts/preflight.sh b/scripts/preflight.sh index 500dbe7d..37ab9720 100755 --- a/scripts/preflight.sh +++ b/scripts/preflight.sh @@ -1,6 +1,9 @@ #!/usr/bin/env bash -# Local pre-merge checks: ruff (format + fix), pytest (unit, then the -# real-model integration tests, which skip without an accelerator), and the UI's own +# Local pre-merge checks: ruff (format + fix, scoped to the Python packages so +# docs/*.md code blocks are left alone), pytest (unit, then the +# real-model integration tests, which skip without an accelerator), the +# architecture ratchet (scripts/arch_metrics.py --check against +# docs/stabilization/baseline.json), and the UI's own # preflight (check, lint, format, build, unit and e2e tests). Runs every step # and lists the ones that failed. Run it from anywhere: # @@ -22,10 +25,11 @@ run_step() { fi } -run_step "ruff format" ruff format . -run_step "ruff check" ruff check . --fix +run_step "ruff format" ruff format dw dw_mcp tests scripts +run_step "ruff check" ruff check dw dw_mcp tests scripts --fix run_step "pytest" python -m pytest run_step "pytest integration" python -m pytest -m integration -n0 +run_step "architecture ratchet" python scripts/arch_metrics.py --check docs/stabilization/baseline.json # e2e starts the fixture server on the same interpreter pytest just used, # wherever its venv lives (a worktree, .venv) - see ui/playwright.config.ts export DW_E2E_PYTHON="${DW_E2E_PYTHON:-$(command -v python)}" diff --git a/tests/test_arch_metrics.py b/tests/test_arch_metrics.py index 1f08763b..2c9d3c87 100644 --- a/tests/test_arch_metrics.py +++ b/tests/test_arch_metrics.py @@ -35,11 +35,11 @@ def test_a_long_function_and_a_long_module_are_counted(tmp_path): metrics = _load().measure( _tree( tmp_path, - {"dw/a.py": _long_function(151) + "\n" * 900, "dw/b.py": "x = 1\n"}, + {"dw/a.py": _long_function(151) + "\n" * 1000, "dw/b.py": "x = 1\n"}, ) ) assert metrics["functions_over_150_lines"] == 1 - assert metrics["modules_over_1000_lines"] == 1 + assert metrics["modules_over_size_ceiling"] == 1 assert metrics["modules"] == 2 @@ -95,6 +95,100 @@ def test_check_mode_exits_nonzero_on_a_regression(tmp_path): assert "modules: 0 ->" in result.stdout +def _module_of(lines): + # distinct lines: the duplicate-block scan is slow on a repeated one + return "".join(f"x{i} = {i}\n" for i in range(lines)) + + +def _run_script(root, *args): + return subprocess.run( + [sys.executable, str(SCRIPT), "--root", str(root), *args], + capture_output=True, + text=True, + ) + + +def test_a_module_in_the_warn_band_warns_and_passes(tmp_path): + root = _tree(tmp_path / "repo", {"dw/a.py": _module_of(1001)}) + baseline = tmp_path / "baseline.json" + baseline.write_text(json.dumps({"modules_over_size_ceiling": 0})) + result = _run_script(root, "--check", str(baseline)) + assert result.returncode == 0 + assert ( + "warning: dw/a.py is 1,001 lines (warn above 1,000, fail above 1,100)" + in result.stdout + ) + # the warning follows the JSON, which stays parseable up to it + assert result.stdout.index("{") < result.stdout.index("warning:") + + +def test_size_warnings_cover_the_band_and_nothing_else(tmp_path): + root = _tree( + tmp_path, + { + "dw/at_1000.py": _module_of(1000), + "dw/at_1100.py": _module_of(1100), + "dw/at_1101.py": _module_of(1101), + "dw/x/small.py": "x = 1\n", + }, + ) + assert _load().size_warnings(root) == [("dw/at_1100.py", 1100)] + assert _load().measure(root)["modules_over_size_ceiling"] == 1 + + +def test_a_module_at_1000_lines_does_not_warn(tmp_path): + root = _tree(tmp_path / "repo", {"dw/a.py": _module_of(1000)}) + assert "warning:" not in _run_script(root).stdout + + +def test_a_module_over_the_ceiling_fails_the_check(tmp_path): + root = _tree(tmp_path / "repo", {"dw/a.py": _module_of(1101)}) + baseline = tmp_path / "baseline.json" + baseline.write_text(json.dumps({"modules_over_size_ceiling": 0})) + result = _run_script(root, "--check", str(baseline)) + assert result.returncode == 1 + assert "modules_over_size_ceiling: 0 -> 1" in result.stdout + assert "warning:" not in result.stdout + + +_RULE = "raise it only in a commit whose message names the rise and why" + + +def test_a_failing_check_prints_the_rebaseline_rule_and_a_passing_one_does_not( + tmp_path, +): + root = _tree(tmp_path / "repo", {"dw/a.py": "x = 1\n"}) + baseline = tmp_path / "baseline.json" + baseline.write_text(json.dumps({"modules": 0})) + failing = _run_script(root, "--check", str(baseline)) + assert failing.returncode == 1 + assert "lower baseline.json freely when a ratchet improves" in failing.stdout + assert _RULE in failing.stdout + assert "arch-approved" not in failing.stdout + baseline.write_text(json.dumps({"modules": 5})) + passing = _run_script(root, "--check", str(baseline)) + assert passing.returncode == 0 + assert _RULE not in passing.stdout + + +def test_the_check_fails_closed_when_the_import_graph_cannot_be_built(tmp_path): + # the graph subprocess runs in the tree's root, so this shadows grimp + root = _tree( + tmp_path / "repo", + { + "dw/__init__.py": "", + "dw/a.py": "x = 1\n", + "grimp.py": "raise ImportError('grimp is gone')\n", + }, + ) + baseline = tmp_path / "baseline.json" + baseline.write_text(json.dumps({"modules": 99})) + result = _run_script(root, "--check", str(baseline)) + assert result.returncode != 0 + assert "CalledProcessError" in result.stderr + assert '"modules"' not in result.stdout # no metrics were reported as a pass + + def _branches(count): body = "".join(f" if x == {i}:\n return {i}\n" for i in range(count)) return f"def branchy(x):\n{body} return -1\n"