diff --git a/dev/.claude-plugin/plugin.json b/dev/.claude-plugin/plugin.json index ae8825b..e8403dc 100644 --- a/dev/.claude-plugin/plugin.json +++ b/dev/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "dev", - "version": "3.2.0", + "version": "3.3.0", "description": "Development workflow skills: scope changes with argued decisions, build across unit/integration/e2e with every scenario proven by tests, ship with a deterministic quality gauntlet and an adversarially verified review, create structured commits that feed a decision ledger, and render pitches or comprehension quizzes.", "author": { "name": "Tobrun" diff --git a/dev/README.md b/dev/README.md index 336b89d..baad37d 100644 --- a/dev/README.md +++ b/dev/README.md @@ -14,7 +14,8 @@ Every skill is explicit-invocation only: Claude Code and Pi use `disable-model-i Specs a change by interviewing for the real problem behind the request, cataloging every design decision (with a subagent blind-spot pass on full-size changes), and arguing each one against alternatives in a `✓`/`✗`/`?`/`⚠`/`⊘` notation with evidence marks. Writes a self-contained spec at `.dev/{plan-name}/spec.md` - research decisions, scope with invariants and a Validation block of the repo's real commands, and a change plan of numbered change sets each ending in a layer-tagged `tests:` line - designed as a fresh-context handoff to `build`. -A checker (`scripts/lint-spec.py`) enforces the spec's mechanics - unique slugs, argued alternatives, echoes that match their decision, tagged test scenarios - so the prose stays about judgment. +A checker (`scripts/lint-spec.py`) enforces the spec's mechanics - unique slugs, argued alternatives, echoes that match their decision, tagged test scenarios, at most 25 scenarios per change set - so the prose stays about judgment. +On a clean spec it prints the build waves the file lists allow and the shared files that make change sets wait, so the plan is shaped for parallel work before build starts. Promotes durable decisions to `docs/decisions.md` and cross-boundary invariants to `docs/contracts.md`, renders an expandable-card spec view, and has a reverse mode that audits the implicit decisions already embedded in existing code. ### scope-review @@ -28,7 +29,9 @@ An APPROVED verdict means every finding was refined or answered: `build` can sta Executes a spec's change sets at the layer each `tests:` scenario is tagged with - unit for business logic, integration for real cross-component seams, e2e for driving the actual running application. Enforces outcomes rather than rituals: every test must have been seen to fail before its green counts, with strict failing-test-first reserved for bug fixes, where red is the proof the issue was actually reproduced. +The red is kept cheap: free when the test is written first, one break per slice and one test file otherwise. Runs independent change sets in parallel as waves of subagents batched by disjoint file lists in spec order, committing each change set and appending to a running `implementation-notes.md` that logs any deviations forced by an edge case. +Each subagent starts from a brief that `scripts/change-set-brief.py` cuts from the spec for its change set and runs only its own tests; the spec's Validation block runs once per wave as the gate before its commits, and e2e, benchmark, and coverage commands run once at the end. Once every change set is committed, drives the real app against a mocked environment, loops until every e2e scenario passes, then renders the e2e report: screenshots per scenario for frontend systems, Test Scenario and Data Model State tables for everything else. Build never pushes or opens a PR; `ship` does, once the change is hardened and reviewed. diff --git a/dev/evals/README.md b/dev/evals/README.md index 2f3b956..0dbef3c 100644 --- a/dev/evals/README.md +++ b/dev/evals/README.md @@ -6,6 +6,7 @@ Eval definitions for the `dev` plugin's skills: realistic prompts and objective - `{skill}.json` - one file per skill: the eval prompt(s), the fixture each expects, and the assertions to grade the output against. Covers all 7 skills: `scope`, `scope-review`, `commit`, `build`, `ship`, `to-pitch`, `to-quiz`. - `results.md` - the record of the most recent full run: scores, methodology, and findings. +- `tests/` - unit tests for the deterministic scripts the skills loop against (`lint-spec.py`, `change-set-brief.py`), run by `scripts/validate.sh` as check D01. `build` runs in `"functional"` mode (a real fixture, a real subagent run, assertions checked against the actual output). `scope`, `scope-review`, `commit`, `ship`, `to-pitch`, and `to-quiz` run in `"comprehension"` mode instead - each depends on either an interactive question loop, a live codebase, or prior artifacts (a finished spec, implementation notes, an e2e report) that are too expensive to stage on every iteration, so these check policy comprehension of the skill text directly. diff --git a/dev/evals/build.json b/dev/evals/build.json index 93368cb..147d02d 100644 --- a/dev/evals/build.json +++ b/dev/evals/build.json @@ -22,6 +22,8 @@ "Tests are behavior-named (e.g. \"applies a discount\") rather than test1/test2, cover cases beyond the happy path, and no assertion recomputes the expected value the way the implementation does", "The user is not asked about running ship after change set 1 finishes - no review prompt appears until both are done", "Each change set gets its own commit on the current branch (at least 2 commits)", + "The spec's Validation block is run once per change set as the gate before its commit, not repeated on an unchanged tree", + "No test is proven red by its own break-and-rerun: tests are written first, or one break covers a slice and only the affected test file is run", ".dev/{plan-name}/implementation-notes.md is created and has one entry per change set", "After both change sets are done, the user is asked to run ship rather than a review panel or gauntlet being launched automatically", "Each implementation-notes.md entry carries a Tests added: line naming real path::test name references", @@ -38,7 +40,9 @@ "The subagents do not commit, stage, or otherwise touch git state - the orchestrator makes the commits", "The subagents do not write to implementation-notes.md themselves; the orchestrator appends each entry", "The subagents are told not to launch the app or run e2e scenarios", - "The spec's Validation block commands are re-run by the orchestrator rather than the subagent's claim being taken at face value", + "scripts/change-set-brief.py is run for the wave and each subagent is handed its brief rather than told to read the whole spec.md and implementation-notes.md", + "The subagents are told to run only their own test files plus typecheck and lint, not the spec's Validation block", + "The spec's Validation block commands are run by the orchestrator, once for the wave, rather than the subagents' claims being taken at face value", "Three commits exist, one per change set, in spec order", "The orchestrator carries each subagent's testsAdded into that change set's Tests added: line rather than discarding it" ] @@ -77,6 +81,7 @@ "fixture": "a repo whose spec Validation block runs unit/lint/build, while .github/workflows/pr.yml also requires a project-owned screenshot-matrix command. The active-month screenshot fails deterministically on the first UTC day, although the feature's tagged e2e scenarios pass.", "assertions": [ "The pull-request workflow is inspected and the screenshot-matrix command is run before the user is offered a PR", + "A required command that already ran green on the unchanged final tree is recorded, not run a second time", "The screenshot failure is diagnosed and fixed rather than labeled pre-existing, flaky, unrelated, or an accepted deviation", "The fix removes the time-dependent assumption without adding a retry, sleep, timeout increase, or looser assertion", "The full screenshot-matrix command is rerun and green before the PR question", diff --git a/dev/evals/results.md b/dev/evals/results.md index 36fae3f..f2755d4 100644 --- a/dev/evals/results.md +++ b/dev/evals/results.md @@ -1,10 +1,37 @@ # Eval Results -Status: no run recorded for the current skill set. +Status: no full run recorded for the current skill set; one partial run below. The last recorded run (2026-07-24) covered the pre-pivot five-skill chain and was invalidated by the pivot to the scope pipeline; its scores were removed rather than left to invite false confidence. Run the harness below against the current 6 skills (`scope`, `commit`, `build`, `ship`, `to-pitch`, `to-quiz`) and replace this file with the dated results. +## Partial run, 2026-10-01: build `parallel-wave`, before and after the build-speed change + +One functional run per version, not the full harness. +Old is the build skill before this change, new is the version that adds the wave gate, the change-set brief, and the cheap-red rule. +The fixture was a Node library with a 20 second legacy test in its suite, and a spec with change sets 1 and 2 disjoint and change set 3 editing files of both. +Every run of a Validation command was written to a log by the fixture's package scripts, so the counts are measured, not reported. + +| Check | Old | New | +| ----- | --- | --- | +| Change sets 1 and 2 launched as subagents in one message | pass | pass | +| Change set 3 started only after 1 and 2 were committed | pass | pass | +| Subagents left git state and `implementation-notes.md` alone | pass | pass | +| Subagents told not to launch the app | pass | pass | +| Each subagent handed a brief from `change-set-brief.py` instead of the whole spec | fail (reads all of `spec.md`) | pass | +| Subagents run only their own test file plus syntax checks | fail (each ran the Validation block) | pass | +| Orchestrator runs the Validation block once per wave | pass | pass | +| Three change-set commits in spec order, `check-tests.py` clean | pass | pass | +| Runs of the full Validation block (log) | 4 | 2 | +| Extra break-and-rerun cycles to see a test red | 2 | 2 | +| Duration by `skill-metrics.py` | 7m 10s | 8m 55s | + +Findings: + +- The Validation block ran half as often, and no subagent ran it. +- Wall time did not improve on this fixture: the suite costs 20 seconds, so two saved runs are under a minute, less than the difference between two model runs. The saving grows with the cost of the block and the number of change sets; a real build is needed to measure it. +- The old run's notes named a test with a comma in it, which `check-tests.py` then split in two. The checker now splits only where a `path::name` follows the comma. + ## Re-running this harness 1. Pick a baseline commit (the last commit before the change under test) and the working tree as "new". diff --git a/dev/evals/scope.json b/dev/evals/scope.json index bb1f22a..8189bbd 100644 --- a/dev/evals/scope.json +++ b/dev/evals/scope.json @@ -12,6 +12,8 @@ "The change is sized small or full, the choice is told to the user with a reason, and the user can override", "The spec is written to .dev/{plan-name}/spec.md with a research section of D- slugged decisions using the marks (chosen, rejected, open, accepted downside, not doing)", "The scope section ends with a Validation block listing the repo's real commands, discovered rather than guessed", + "The Validation block lists each check once, and any e2e suite, benchmark, or coverage re-run in it is marked (end of build)", + "No change set carries more than 25 scenarios, and edits to a file several change sets need are gathered into one change set rather than repeated across them", "The spec does not implement code; no source files are modified", "The run ends by recommending build; it is never invoked directly", "Every change set ends with one tests: line whose scenarios carry [unit], [integration], or [e2e] tags, or tests: none with a reason", diff --git a/dev/evals/tests/__init__.py b/dev/evals/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/dev/evals/tests/test_build_speed.py b/dev/evals/tests/test_build_speed.py new file mode 100644 index 0000000..8f3ac7c --- /dev/null +++ b/dev/evals/tests/test_build_speed.py @@ -0,0 +1,251 @@ +"""Unit tests for the two checks that keep a build fast: lint-spec.py's change-set +size limit and wave report, and build's change-set-brief.py. + +Run from the repo root: python3 -m unittest discover -s dev/evals/tests -t . +Each test writes a scratch plan directory and runs the script as a subprocess, so +it proves the CLI contract the scope and build skills loop against. +""" + +from __future__ import annotations + +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[3] +LINT = REPO_ROOT / "dev" / "skills" / "scope" / "scripts" / "lint-spec.py" +BRIEF = REPO_ROOT / "dev" / "skills" / "build" / "scripts" / "change-set-brief.py" +CHECK = REPO_ROOT / "dev" / "skills" / "build" / "scripts" / "check-tests.py" + +HEAD = """# Coupons + +## Research + +D-coupon-store: Where do coupons live? + ✓ a table - one query reads them + ✗ a config file - a redeploy per coupon + +D-expiry-clock: Which clock decides expiry? + ✓ the server clock - one source of time + ✗ the client clock - a user can set it back + +## Scope + +Inputs: a coupon code. Outputs: a discounted total. + +### Validation + +- `npm run typecheck` +- `npm test` + +## Change plan + +""" + + +def scenarios(count: int) -> str: + return "; ".join(f"[unit] case {n} -> outcome {n}" for n in range(1, count + 1)) + + +def change_set(number: int, files: str, count: int = 1, decision: str = "") -> str: + link = f" - decisions: {decision}" if decision else "" + return f"{number}. Change set {number}\n a. {files} - edit{link}\n tests: {scenarios(count)}\n\n" + + +def run(script: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(script), *args], check=False, capture_output=True, text=True + ) + + +class PlanCase(unittest.TestCase): + def setUp(self) -> None: + scratch = tempfile.TemporaryDirectory() + self.addCleanup(scratch.cleanup) + self.plan = Path(scratch.name) / "coupons" + self.plan.mkdir() + + def write_spec(self, *change_sets: str) -> Path: + spec = self.plan / "spec.md" + spec.write_text(HEAD + "".join(change_sets), encoding="utf-8") + return spec + + def write_notes(self, text: str) -> None: + (self.plan / "implementation-notes.md").write_text(text, encoding="utf-8") + + +class ScenarioLimitTest(PlanCase): + def test_change_set_at_the_limit_is_clean(self) -> None: + spec = self.write_spec(change_set(1, "`src/a.ts`", 25)) + result = run(LINT, str(spec)) + self.assertEqual(result.returncode, 0, result.stdout) + + def test_change_set_over_the_limit_fails_naming_it_and_its_count(self) -> None: + spec = self.write_spec(change_set(1, "`src/a.ts`"), change_set(2, "`src/b.ts`", 26)) + result = run(LINT, str(spec)) + self.assertEqual(result.returncode, 1) + self.assertIn("change set 2 carries 26 scenarios", result.stdout) + self.assertNotIn("change set 1 carries", result.stdout) + + def test_oversized_change_set_already_built_is_not_resized(self) -> None: + spec = self.write_spec(change_set(1, "`src/a.ts`", 40), change_set(2, "`src/b.ts`")) + self.write_notes("## Change set 1: Change set 1\n- What was done: it\n") + result = run(LINT, str(spec)) + self.assertEqual(result.returncode, 0, result.stdout) + + +class WaveReportTest(PlanCase): + def waves(self, *change_sets: str) -> str: + result = run(LINT, str(self.write_spec(*change_sets))) + self.assertEqual(result.returncode, 0, result.stdout) + return result.stdout + + def test_disjoint_change_sets_share_one_wave(self) -> None: + out = self.waves(change_set(1, "`src/a.ts`"), change_set(2, "`src/b.ts`")) + self.assertIn("2 change sets in 1 waves - [1 2]", out) + self.assertNotIn("waits on", out) + + def test_shared_file_queues_change_sets_and_is_named(self) -> None: + out = self.waves( + change_set(1, "`src/a.ts`, `src/compose.ts`"), + change_set(2, "`src/b.ts`, `src/compose.ts`"), + change_set(3, "`src/c.ts`, `src/compose.ts`"), + ) + self.assertIn("3 change sets in 3 waves - [1] [2] [3]", out) + self.assertIn(" 2 waits on 1: src/compose.ts", out) + self.assertIn(" 3 waits on 2: src/compose.ts", out) + + def test_later_change_set_skips_ahead_only_past_change_sets_it_shares_nothing_with(self) -> None: + out = self.waves( + change_set(1, "`src/a.ts`"), + change_set(2, "`src/a.ts`, `src/b.ts`"), + change_set(3, "`src/b.ts`"), + change_set(4, "`src/d.ts`"), + ) + # 3 shares nothing with 1 but must not overtake 2, which is still held back. + self.assertIn("4 change sets in 3 waves - [1 4] [2] [3]", out) + + def test_bare_file_name_matches_the_path_that_ends_in_it(self) -> None: + out = self.waves(change_set(1, "`src/app/compose.ts`"), change_set(2, "`compose.ts`")) + self.assertIn("[1] [2]", out) + + def test_code_in_backticks_and_the_tests_line_are_not_files(self) -> None: + out = self.waves( + change_set(1, "`src/a.ts` - sets `embedder.model` and `job.stages`"), + change_set(2, "`src/b.ts` - reads `embedder.model` and `job.stages`"), + ) + self.assertIn("[1 2]", out) + + def test_built_change_sets_are_left_out_of_the_waves(self) -> None: + spec = self.write_spec( + change_set(1, "`src/a.ts`"), change_set(2, "`src/a.ts`"), change_set(3, "`src/c.ts`") + ) + self.write_notes("## Change set 1: Change set 1\n- What was done: it\n") + result = run(LINT, str(spec)) + self.assertIn("2 change sets in 1 waves - [2 3]", result.stdout) + + def test_single_change_set_prints_no_waves(self) -> None: + out = self.waves(change_set(1, "`src/a.ts`")) + self.assertNotIn("build waves", out) + + +class ChangeSetBriefTest(PlanCase): + def setUp(self) -> None: + super().setUp() + self.write_spec( + change_set(1, "`src/store.ts`", decision="D-coupon-store (✓ a table)"), + change_set(2, "`src/expiry.ts`", decision="D-expiry-clock (✓ the server clock)"), + ) + + def brief(self, number: int = 2) -> str: + result = run(BRIEF, str(self.plan), str(number)) + self.assertEqual(result.returncode, 0, result.stderr) + return result.stdout + + def test_brief_keeps_its_change_set_and_the_scope_section_verbatim(self) -> None: + out = self.brief() + self.assertIn("2. Change set 2\n a. `src/expiry.ts` - edit", out) + self.assertIn("Inputs: a coupon code. Outputs: a discounted total.", out) + self.assertIn("- `npm run typecheck`", out) + + def test_brief_keeps_only_the_decisions_its_change_set_links(self) -> None: + out = self.brief() + self.assertIn("D-expiry-clock: Which clock decides expiry?", out) + self.assertIn(" ✗ the client clock - a user can set it back", out) + self.assertNotIn("D-coupon-store: Where do coupons live?", out) + + def test_brief_names_the_other_change_sets_without_their_plans(self) -> None: + out = self.brief() + self.assertIn("- 1. Change set 1", out) + self.assertNotIn("`src/store.ts`", out) + + def test_brief_carries_earlier_work_and_deviations_but_no_test_inventory(self) -> None: + self.write_notes( + "# Implementation notes\n\n## Change set 1: Change set 1\n" + "- What was done: added the coupon table\n" + "- Seams tested: the store\n" + "- Tests added: test/store.test.ts::reads a coupon\n" + "- Deviations from spec: codes are stored upper-case\n" + ) + out = self.brief() + self.assertIn("### Change set 1: Change set 1", out) + self.assertIn("- What was done: added the coupon table", out) + self.assertIn("- Deviations from spec: codes are stored upper-case", out) + self.assertNotIn("Tests added", out) + self.assertNotIn("Seams tested", out) + + def test_linked_decision_missing_from_research_is_said_so(self) -> None: + self.write_spec(change_set(1, "`src/a.ts`", decision="D-ghost (✓ x)")) + self.assertIn("D-ghost: not argued in the spec's research section", self.brief(1)) + + def test_out_dir_writes_one_brief_per_change_set_and_prints_each_path(self) -> None: + out_dir = self.plan.parent / "briefs" + result = run(BRIEF, str(self.plan), "1", "2", "--out-dir", str(out_dir)) + self.assertEqual(result.returncode, 0, result.stderr) + for number in (1, 2): + written = out_dir / f"change-set-{number}.md" + self.assertIn(f"# Brief: change set {number} of coupons", written.read_text(encoding="utf-8")) + self.assertIn(str(written.resolve()), result.stdout) + + def test_unknown_change_set_exits_2_naming_the_ones_that_exist(self) -> None: + result = run(BRIEF, str(self.plan), "9") + self.assertEqual(result.returncode, 2) + self.assertIn("no change set 9 (change sets: 1, 2)", result.stderr) + + def test_several_change_sets_without_out_dir_exit_2(self) -> None: + self.assertEqual(run(BRIEF, str(self.plan), "1", "2").returncode, 2) + + def test_missing_spec_exits_2(self) -> None: + self.assertEqual(run(BRIEF, str(self.plan.parent / "absent"), "1").returncode, 2) + + +class TestNameWithCommaTest(PlanCase): + """A test name is prose; a comma in it must not send the build back around the check loop.""" + + def check(self, tests_added: str) -> subprocess.CompletedProcess[str]: + self.write_spec(change_set(1, "`src/a.ts`", 2)) + repo = self.plan.parent + (repo / "a.test.ts").write_text( + 'test("a set name, and a longer text counts more", () => {});\n' + 'test("an empty text gives a unit vector", () => {});\n', + encoding="utf-8", + ) + self.write_notes(f"## Change set 1: Change set 1\n- Tests added: {tests_added}\n") + return run(CHECK, str(self.plan), "--repo-root", str(repo)) + + def test_test_name_holding_a_comma_is_one_test(self) -> None: + result = self.check( + "a.test.ts::a set name, and a longer text counts more, a.test.ts::an empty text gives a unit vector" + ) + self.assertEqual(result.returncode, 0, result.stdout) + + def test_named_test_that_is_not_in_the_file_still_fails(self) -> None: + result = self.check("a.test.ts::a set name, and a longer text counts more, a.test.ts::never written") + self.assertEqual(result.returncode, 1) + self.assertIn("contains no test named 'never written'", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/dev/references/ci-parity.md b/dev/references/ci-parity.md index 382ac06..92157c9 100644 --- a/dev/references/ci-parity.md +++ b/dev/references/ci-parity.md @@ -21,6 +21,8 @@ Record the commands and outcomes in the plan's `implementation-notes.md`. Run every reproducible project-owned required-check command against the final checkout, after feature E2E and after any hardening edits. +A command that already ran green on this exact tree - no file changed since - +is not run a second time: record its result and where it came from. - A failure is work to fix, including a test described as flaky, unrelated, or pre-existing. Diagnose and remove its nondeterminism; do not add retries, diff --git a/dev/skills/build/SKILL.md b/dev/skills/build/SKILL.md index f1f696e..962d4b0 100644 --- a/dev/skills/build/SKILL.md +++ b/dev/skills/build/SKILL.md @@ -17,11 +17,11 @@ Read [references/layers.md](references/layers.md), [references/tests.md](referen 1. Run `python3 {build-skill-root}/../../scripts/skill-metrics.py start build`, then read `spec.md` in full: the research section (the decisions and their rationale), the scope section (including its Validation block of real repo commands), and the change plan. Explore the relevant code. If the Validation block is absent, discover the repo's real test and typecheck commands yourself from `package.json`, a `Makefile`, or CI config, and log them in `implementation-notes.md`. 2. Build waves by disjoint batching per [references/parallel.md](references/parallel.md): sequential in spec order by default, batched only when file lists are disjoint and nothing a wave-mate or earlier unfinished change set introduces is consumed. Every change set in the plan is in scope, not just the first. -3. For each wave, run its change sets in parallel per the same reference, then commit each finished change set on the current branch and append its entry to `implementation-notes.md`. A change set that adds, removes, moves, or rewires a component, flow, or boundary updates `docs/architecture.md` in the same commit and passes `architecture-check.py` first, per [../../references/architecture.md](../../references/architecture.md). +3. For each wave, run its change sets in parallel per the same reference, pass its wave gate once, then commit each finished change set on the current branch and append its entry to `implementation-notes.md`. A change set that adds, removes, moves, or rewires a component, flow, or boundary updates `docs/architecture.md` in the same commit and passes `architecture-check.py` first, per [../../references/architecture.md](../../references/architecture.md). 4. Move straight to the next wave. Never stop after one change set or wave to ask about review. 5. When every change set is committed, loop `python3 {build-skill-root}/scripts/check-tests.py .dev/{plan-name}` until it exits clean: it proves every specced scenario has a test that really exists, rather than one that was reported. 6. Then run the full e2e pass per "The e2e layer" below over the whole spec, and loop on failures until it is green. -7. Run the repository's required pull-request commands per [../../references/ci-parity.md](../../references/ci-parity.md), starting them in the background as soon as the e2e loop is green and rendering the e2e report while they run - the two share nothing. A known-red CI scenario is not an acceptable deviation. +7. Run the repository's required pull-request commands per [../../references/ci-parity.md](../../references/ci-parity.md), plus every Validation command the wave gates left for the end, starting them in the background as soon as the e2e loop is green and rendering the e2e report while they run - the two share nothing. A known-red CI scenario is not an acceptable deviation. 8. After the e2e report and CI-parity gate, close per "Closing message". Build never pushes or opens a PR; that is `ship`'s phase 3. ## Jira sync @@ -58,12 +58,12 @@ Neither does a failed e2e loop: say what is blocked, then still point at `ship`. Each change set, whether you run it yourself or a subagent runs it, follows the same loop: - Test at the seams the spec's scope section declares, per [references/tests.md](references/tests.md); if the declared boundary is wrong or missing, follow its fallback and log the change under Deviations - do not stall on it. -- Implement in **vertical slices**: one scenario's behavior at a time, its test written before or right after the code - the enforced outcome is what matters, not the ritual order. Each `tests:` scenario's test lives at its tagged layer ([references/layers.md](references/layers.md)); a scenario isn't met until a real test exists there. -- Run the change set's own tests and typecheck continuously; once the change set is green, run the spec's Validation block verbatim - it is the wider suite plus typecheck/lint - and only report done when it passes clean. +- Implement in **vertical slices**: one scenario's behavior at a time, its test written before the code or right after it - before is the cheap order, because its first run is then the red the rules below ask for. Each `tests:` scenario's test lives at its tagged layer ([references/layers.md](references/layers.md)); a scenario isn't met until a real test exists there. +- Run the change set's own tests and typecheck continuously - the test files it adds or edits, never the whole suite - and report done when they are green. The spec's Validation block is not run per change set: it is the wave gate, run once per wave before its commits, per "The wave gate" in [references/parallel.md](references/parallel.md). ## Rules of the loop -- **Every test must have been seen red.** A test that has never failed proves nothing: earn its green by writing it before the code, or by briefly breaking the behavior once after. Bug fixes are strictly test-first: a defect change set starts with a failing test that reproduces the reported issue - red is the proof it was actually reproduced - only then fix, and watch that same test go green. +- **Every test must have been seen red, once and cheaply.** A test that has never failed proves nothing. Written before the code, its first run is that red and costs nothing. Written after, break the behavior once per slice - one break covers every test of the slice - and run only the affected test file: never one break per test, never the suite for a red. Bug fixes are strictly test-first: a defect change set starts with a failing test that reproduces the reported issue - red is the proof it was actually reproduced - only then fix, and watch that same test go green. - **One slice at a time.** One seam, one behavior, one test, one minimal implementation per cycle. - **Refactoring is not part of the loop.** It belongs to `ship`'s review phase. - **Keep going.** A red test, a failing e2e scenario, or an edge case that contradicts the spec is work to do, not a reason to hand back. Fix it, log the deviation, continue. Stop early only when a blocking question makes further work unsafe or wasted. diff --git a/dev/skills/build/references/parallel.md b/dev/skills/build/references/parallel.md index d3b6a05..3aac303 100644 --- a/dev/skills/build/references/parallel.md +++ b/dev/skills/build/references/parallel.md @@ -20,17 +20,27 @@ Never start work belonging to the next wave while the current wave is in flight. Launch one `Agent` per change set, **all in a single message** so they run concurrently. A wave of one change set needs no subagent: implement it yourself in the main thread. +First write the wave's briefs, one command for the whole wave: + +```bash +python3 {build-skill-root}/scripts/change-set-brief.py .dev/{plan-name} {N} {N} --out-dir /tmp/{project-slug}/briefs/{plan-name} +``` + +A brief is the spec cut down to one change set: the scope section, the decisions that change set links, its own plan, and what earlier change sets did and deviated on. +Every line is the spec's own, so an agent starts from about a third of the reading and loses nothing it builds against. +Write them per wave, never once up front: the notes they carry grow with every committed change set. + Each agent prompt contains: 1. The role: `You implement exactly one change set of a spec. Other agents implement sibling change sets concurrently; stay inside your change set's file list.` -2. The absolute path to `spec.md`, the number of the change set the agent owns, and this skill's `layers.md`, `tests.md`, and `mocking.md` - the agent reads them itself rather than receiving them inlined. The spec is a self-contained handoff by design; the agent reads all of it, then implements only its own change set. +2. The absolute path to the change set's brief, the number of the change set the agent owns, and this skill's `layers.md`, `tests.md`, and `mocking.md` - the agent reads them itself rather than receiving them inlined. The brief stands in for `spec.md` and `implementation-notes.md`: the agent reads neither whole, and opens the spec only to follow something the brief points at. 3. The absolute path to this skill's `SKILL.md`, with the instruction to follow its "The change-set loop" and "Rules of the loop" sections - read like the other references, not pasted into the prompt. 4. Hard constraints: - Implement only this change set's `[unit]` and `[integration]` scenarios. `[e2e]` scenarios are run once per spec by the orchestrator afterwards - do not launch the app. - Do not commit, stage, or touch git state. The orchestrator commits. - Do not edit files outside your change set's file list. If the change set genuinely needs a file another change set owns, stop and report it as a conflict instead of editing it. - Do not edit `implementation-notes.md`. Report your entry; the orchestrator appends it. - - Run the spec's Validation block and report its real result. A red result is a fact to report, not something to hide or paper over. + - Run only your own checks: the test files this change set adds or edits, plus typecheck and lint over the packages it touches. Never run the spec's Validation block, the whole suite, the e2e suite, or a benchmark - siblings are mid-edit in the same tree, so a wider run measures their half-finished work, and the orchestrator runs the Validation block once for the whole wave. Report the commands and their real result. A red result is a fact to report, not something to hide or paper over. 5. The output contract below. Output contract (the agent's final message must be exactly one fenced JSON block): @@ -50,13 +60,22 @@ Output contract (the agent's final message must be exactly one fenced JSON block ## After a wave -1. Verify rather than trust the reports: run the spec's Validation block yourself, once per wave - the shared tree already holds the whole wave's changes, so one run covers every change set in it. +1. Verify rather than trust the reports: run the wave gate below yourself, once per wave - the shared tree already holds the whole wave's changes, so one run covers every change set in it. 2. Commit each change set's work on the current branch, in number order, one commit per change set. A skipped-ahead change set's commit waits until every earlier change set is committed, so history keeps the spec's order. 3. Append each change set's entry to `implementation-notes.md` from `whatWasDone`, `seamsTested`, `testsAdded`, and `deviations`. `testsAdded` becomes the entry's `Tests added:` line, which the scenario checker reads. A `status: blocked` change set, a failing validation, or a reported conflict is yours to finish in the main thread before the next wave starts - do not carry a red change set forward and do not relaunch the same agent on the same failure more than once. If two change sets in a wave edited the same file anyway, reconcile it yourself and log it under Deviations. +## The wave gate + +The spec's Validation block is the gate between a wave and its commits, and the only full run a wave gets - a wave of one you implemented yourself included. +Each of its commands runs once per tree: a green result stands until a file changes, so nothing is re-run "to be sure" before committing, and after a fix the failed command runs first and the rest only once it is green. +Start commands that share no state together (lint, typecheck, and the unit suite), and write the wave's notes entries while they run. + +A command the block marks `(end of build)` is left out of the wave gate, and so are three kinds even when a spec lists one unmarked, because each costs minutes and tells a wave nothing it needs to commit: the e2e suite (the e2e pass runs it), benchmarks, and any analysis that re-runs the suite to measure it - coverage, complexity, mutation. +Each of those runs once, on the final tree, with the CI-parity gate. + ## When not to parallelize - The spec has one change set, or every change set's files overlap with the one before it: run them sequentially yourself. diff --git a/dev/skills/build/scripts/change-set-brief.py b/dev/skills/build/scripts/change-set-brief.py new file mode 100755 index 0000000..a44b2ba --- /dev/null +++ b/dev/skills/build/scripts/change-set-brief.py @@ -0,0 +1,169 @@ +#!/usr/bin/env python3 +"""Cut a spec down to what one change set's implementer needs. + +Usage: python3 change-set-brief.py .dev/{plan-name} N [N ...] [--out-dir DIR] + +A brief is spec.md minus the decisions the change set does not link and minus +the other change sets, followed by what earlier change sets did and deviated on +from implementation-notes.md. Every kept line is verbatim, so an agent reading +the brief reads the spec's own words, a third of them. + +With --out-dir, writes DIR/change-set-N.md per change set and prints each path +with its size against the spec. Without it, prints the one brief to stdout. +Exit 2 on a missing spec or an unknown change set. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +HEADING = re.compile(r"^##\s+(.*?)\s*$") +DECISION = re.compile(r"^(D-[a-z0-9]+(?:-[a-z0-9]+)*):") +LINKED = re.compile(r"\bD-[a-z0-9]+(?:-[a-z0-9]+)*") +CHANGE_SET = re.compile(r"^\s*(\d+)\.\s+\S") +NOTE_ENTRY = re.compile(r"^##\s+Change set\s+(\d+)\b", re.IGNORECASE) +# The two note lines only the scenario checker and the reviewers read. +NOTE_NOISE = re.compile(r"^\s*-\s*(Tests added|Seams tested):", re.IGNORECASE) + + +def split_sections(lines: list[str]) -> list[tuple[str, list[str]]]: + """The spec as (lower-cased ## heading, lines) pairs; the text before the first is ''.""" + sections: list[tuple[str, list[str]]] = [("", [])] + for line in lines: + heading = HEADING.match(line) + if heading: + sections.append((heading.group(1).lower(), [line])) + else: + sections[-1][1].append(line) + return sections + + +def decision_entries(research: list[str]) -> dict[str, list[str]]: + """Each decision's header and its indented alternative lines, by slug.""" + entries: dict[str, list[str]] = {} + current = None + for line in research[1:]: + header = DECISION.match(line) + if header: + current = entries.setdefault(header.group(1), []) + current.append(line) + elif current is not None and line[:1] in (" ", "\t") and line.strip(): + current.append(line) + elif line.strip(): + current = None + return entries + + +def change_set_blocks(plan: list[str]) -> dict[int, list[str]]: + """Each change set's lines, from its numbered line to the next one.""" + blocks: dict[int, list[str]] = {} + current = None + for line in plan[1:]: + change_set = CHANGE_SET.match(line) + if change_set: + current = blocks.setdefault(int(change_set.group(1)), []) + if current is not None: + current.append(line) + for block in blocks.values(): + while block and not block[-1].strip(): + block.pop() + return blocks + + +def notes_digest(notes: Path) -> list[str]: + """implementation-notes.md without its test and seam inventories.""" + if not notes.is_file(): + return [] + kept = [ + line for line in notes.read_text(encoding="utf-8").splitlines() + if not NOTE_NOISE.match(line) and not line.startswith("# ") + ] + return kept if any(NOTE_ENTRY.match(line) for line in kept) else [] + + +def brief(plan_dir: Path, number: int) -> str: + spec = plan_dir / "spec.md" + sections = split_sections(spec.read_text(encoding="utf-8").splitlines()) + by_name = dict(sections) + blocks = change_set_blocks(by_name.get("change plan", [""])) + if number not in blocks: + known = ", ".join(str(n) for n in sorted(blocks)) or "none" + raise LookupError(f"{spec} has no change set {number} (change sets: {known})") + block = blocks[number] + decisions = decision_entries(by_name.get("research", [""])) + linked = list(dict.fromkeys(slug for line in block for slug in LINKED.findall(line))) + + out = [ + f"# Brief: change set {number} of {plan_dir.name}", + "", + f"This is {spec.resolve()} cut down to change set {number}: every line below is the spec's own.", + "Left out are the decisions this change set does not link and the other change sets' plans.", + "Open the spec only to follow something this brief points at, never to read it whole.", + "", + ] + for name, lines in sections: + if name == "research": + out += ["## Research (the decisions this change set links)", ""] + for slug in linked: + out += decisions.get(slug, [f"{slug}: not argued in the spec's research section"]) + [""] + if not linked: + out += ["This change set links no decision.", ""] + elif name == "change plan": + out += [f"## Change plan (change set {number} only)", ""] + block + [""] + others = [blocks[n][0].strip() for n in sorted(blocks) if n != number] + if others: + out += ["The other change sets, whose files are not yours:"] + [f"- {o[:140]}" for o in others] + [""] + else: + out += lines + ([""] if lines and lines[-1].strip() else []) + digest = notes_digest(plan_dir / "implementation-notes.md") + if digest: + out += ["## What earlier change sets did (from implementation-notes.md)", ""] + out += [("#" + line) if line.startswith("## ") else line for line in digest] + return "\n".join(out).rstrip() + "\n" + + +def main(argv: list[str]) -> int: + args: list[str] = [] + out_dir = None + rest = argv[1:] + while rest: + value = rest.pop(0) + if value == "--out-dir" and rest: + out_dir = Path(rest.pop(0)) + else: + args.append(value) + numbers = args[1:] + if not numbers or not all(n.isdigit() for n in numbers) or (out_dir is None and len(numbers) != 1): + print( + "usage: change-set-brief.py [ ...] [--out-dir DIR]\n" + " several change sets need --out-dir", + file=sys.stderr, + ) + return 2 + plan_dir = Path(args[0]) + spec = plan_dir / "spec.md" + if not spec.is_file(): + print(f"no spec.md at {spec}", file=sys.stderr) + return 2 + try: + briefs = {int(n): brief(plan_dir, int(n)) for n in numbers} + except LookupError as error: + print(error, file=sys.stderr) + return 2 + if out_dir is None: + sys.stdout.write(briefs[int(numbers[0])]) + return 0 + out_dir.mkdir(parents=True, exist_ok=True) + spec_bytes = len(spec.read_bytes()) + for number, text in briefs.items(): + target = out_dir / f"change-set-{number}.md" + target.write_text(text, encoding="utf-8") + size = len(text.encode("utf-8")) + print(f"{target.resolve()}: {size} bytes, {round(100 * size / spec_bytes)}% of spec.md") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/dev/skills/build/scripts/check-tests.py b/dev/skills/build/scripts/check-tests.py index 7d90d25..ccbb1aa 100755 --- a/dev/skills/build/scripts/check-tests.py +++ b/dev/skills/build/scripts/check-tests.py @@ -18,6 +18,8 @@ TESTS = re.compile(r"^\s*tests:\s*(.*)$", re.IGNORECASE) NOTE_ENTRY = re.compile(r"^##\s+Change set\s+(\d+)\b", re.IGNORECASE) NOTE_TESTS = re.compile(r"^\s*-\s*Tests added:\s*(.*)$", re.IGNORECASE) +# A comma separates two references only when a path::name follows it; a test name may hold commas. +NOTE_SEPARATOR = re.compile(r",\s*(?=[^,]*::)") def spec_scenarios(spec: Path, problem) -> dict[int, int]: @@ -65,7 +67,7 @@ def note_entries(notes: Path) -> dict[int, list[str]]: value = named.group(1).strip() if value.lower().startswith("none"): continue - entries[current].extend(t.strip() for t in value.split(",") if t.strip()) + entries[current].extend(t.strip() for t in NOTE_SEPARATOR.split(value) if t.strip()) return entries diff --git a/dev/skills/scope-review/references/lenses.md b/dev/skills/scope-review/references/lenses.md index 0e5fa5f..ed4f6d9 100644 --- a/dev/skills/scope-review/references/lenses.md +++ b/dev/skills/scope-review/references/lenses.md @@ -11,7 +11,8 @@ Does the change plan survive contact with the repo? - Every file a change set names exists, or the set says it is new; the described edit is possible at that site - the function, hook, or config it assumes is really there. - Prior art and idioms the spec cites exist where it says they do. - Premises about current behavior are checked against the code, never trusted: an "X already handles Y" claim that is false is a BLOCK naming the site. -- The Validation block's commands exist in the repo's manifests and run the layers the plan relies on. +- The Validation block's commands exist in the repo's manifests and run the layers the plan relies on - and nothing slower: an e2e suite, benchmark, or coverage re-run listed there without its `(end of build)` mark is a CONCERN, because build pays for the block every wave. +- The plan is as wide as the change allows: change sets queued behind one shared file that a single change set could own are a CONCERN naming the file (`lint-spec.py` prints the waves). ## completeness diff --git a/dev/skills/scope/SKILL.md b/dev/skills/scope/SKILL.md index e7d432f..298c3bb 100644 --- a/dev/skills/scope/SKILL.md +++ b/dev/skills/scope/SKILL.md @@ -90,6 +90,7 @@ Name the components and flows the change adds, removes, or reshapes, in the over Efforts have second-order effects - capture them as nested sub-efforts, each carrying its own decisions back into the research section (rate limiting in scope means Redis setup, which carries config and deploy decisions). Record considered non-goals as `⊘` lines with a because clause - things someone weighed and cut, not mere omissions. End the scope with a `### Validation` block listing the repo's real typecheck/test/lint/build commands, discovered from `package.json`, a `Makefile`, CI config, or equivalent - never guess `npm test` into a `pytest` repo; ask if you cannot determine them. +`build` runs this block once per wave, so list each check once and mark every command a wave does not need - the e2e suite, a benchmark, a coverage or complexity run that repeats the suite - `(end of build)`: build runs those once, on the final tree. Writing style for the spec: ELI12, no similes or metaphors. ## 5. Review and research @@ -104,7 +105,9 @@ Writing style for the spec: ELI12, no similes or metaphors. Short fragmented sentences. Link decisions by ID wherever one applies, echoing the choice. Each change set ends with one `;`-separated `tests:` line - concrete scenarios as input -> expected outcome, each tagged `[unit]`, `[integration]`, or `[e2e]`, covering happy path, edge cases, and failure paths; a set with nothing to test says `tests: none - {reason}`. Specific enough that whoever writes the tests invents nothing; the author tags layers here because a fresh implementation session can't recover that intent. -Order change sets so each builds only on the ones before it; keep file lists disjoint where possible - `build` parallelizes consecutive change sets whose files don't overlap. +Order change sets so each builds only on the ones before it, and shape the plan wide: `build` runs change sets in parallel only while their file lists are disjoint, so a file three change sets edit makes them queue. +Land what several change sets share - a port, a schema, a registry, the composition root, a regenerated artifact - in one change set, and let the ones building on it own disjoint files. +Size each change set for one agent: past 25 scenarios it is several change sets, split along a seam. ``` 1. Change set 1 @@ -119,6 +122,7 @@ Order change sets so each builds only on the ones before it; keep file lists dis Then loop `python3 {scope-skill-root}/scripts/lint-spec.py .dev/{plan-name}/spec.md` until it exits clean. It owns the mechanics above; the spec is not final while it reports anything. +Once clean it prints the build waves the file lists allow and the files that make a change set wait; where change sets queue behind a shared file, move that file's edits into one change set and lint again. ## 7. Visualize diff --git a/dev/skills/scope/scripts/lint-spec.py b/dev/skills/scope/scripts/lint-spec.py index 2056f78..2cb73b3 100755 --- a/dev/skills/scope/scripts/lint-spec.py +++ b/dev/skills/scope/scripts/lint-spec.py @@ -3,6 +3,8 @@ Usage: python3 lint-spec.py .dev/{plan-name}/spec.md Exit 0 when the spec is clean, 1 with one problem per line otherwise. +A clean spec also gets the build waves its file lists allow, so the author sees +which shared files make change sets wait for each other. """ from __future__ import annotations @@ -20,8 +22,19 @@ TESTS = re.compile(r"^\s*tests:\s*(.*)$", re.IGNORECASE) LAYER = re.compile(r"^\[(unit|integration|e2e)\]\s+\S") HEADING = re.compile(r"^##\s+(.*?)\s*$") +NOTE_ENTRY = re.compile(r"^##\s+Change set\s+(\d+)\b", re.IGNORECASE) +BACKTICKED = re.compile(r"`([^`\s]+)`") +# A backticked token is a file when it ends in a known file extension; +# `embedder.model` and `job.stages` are code, not paths. +FILE_NAME = re.compile( + r"(?:^|/)(?:Makefile|Dockerfile|[\w.@+-]+\." + r"(?:c|cc|cpp|cs|css|go|gradle|graphql|h|html|java|js|json|jsx|kt|lock|md|mjs|cjs|php|proto" + r"|py|rb|rs|scss|sh|sql|svelte|swift|tf|toml|ts|tsx|txt|vue|xml|yaml|yml))$" +) -CHOSEN, REJECTED, OPEN, NOT_DOING = "✓", "✗", "?", "⊘" +CHOSEN, OPEN, NOT_DOING = "✓", "?", "⊘" +# One agent builds one change set; past this many scenarios it stops fitting in one sitting. +SCENARIO_LIMIT = 25 def sections(lines: list[str]) -> dict[str, list[tuple[int, str]]]: @@ -122,7 +135,16 @@ def check_echoes( problem(number, f"the change plan links {slug}, which is still open or flagged") -def check_change_plan(body: list[tuple[int, str]], problem) -> None: +def built_change_sets(spec: Path) -> set[int]: + """Change sets build already logged; their size is history, and they never renumber.""" + notes = spec.parent / "implementation-notes.md" + if not notes.is_file(): + return set() + entries = (NOTE_ENTRY.match(line) for line in notes.read_text(encoding="utf-8").splitlines()) + return {int(entry.group(1)) for entry in entries if entry} + + +def check_change_plan(body: list[tuple[int, str]], built: set[int], problem) -> None: counts: dict[int, int] = {} current = None for number, line in body: @@ -148,15 +170,87 @@ def check_change_plan(body: list[tuple[int, str]], problem) -> None: if not value: problem(number, "tests: line is empty") continue - for scenario in value.split(";"): - scenario = scenario.strip() - if scenario and not LAYER.match(scenario): + scenarios = [scenario.strip() for scenario in value.split(";") if scenario.strip()] + for scenario in scenarios: + if not LAYER.match(scenario): problem(number, f"scenario '{scenario[:40]}' carries no [unit]/[integration]/[e2e] tag") + if len(scenarios) > SCENARIO_LIMIT and current not in built: + problem( + number, + f"change set {current} carries {len(scenarios)} scenarios; split it along a seam " + f"so no change set carries more than {SCENARIO_LIMIT}", + ) for change_set, seen in sorted(counts.items()): if seen != 1: problem(None, f"change set {change_set} has {seen} tests: lines; expected exactly one") +def change_set_files(body: list[tuple[int, str]]) -> dict[int, set[str]]: + """Files each change set names, read from the backticked paths outside its tests: line.""" + files: dict[int, set[str]] = {} + current = None + for _, line in body: + change_set = CHANGE_SET.match(line) + if change_set: + current = int(change_set.group(1)) + files.setdefault(current, set()) + if current is None or TESTS.match(line): + continue + files[current].update(t for t in BACKTICKED.findall(line) if FILE_NAME.search(t)) + return files + + +def shared_files(one: set[str], other: set[str]) -> list[str]: + """Paths two change sets both name; a bare file name matches any path that ends in it.""" + shared = one & other + for mine, theirs in ((one, other), (other, one)): + bare = {path for path in theirs if "/" not in path} + shared |= {path for path in mine if "/" in path and path.rsplit("/", 1)[-1] in bare} + return sorted(shared) + + +def build_waves(files: dict[int, set[str]]) -> tuple[list[list[int]], dict[int, tuple[int, list[str]]]]: + """The waves build's consecutive-disjoint batching gives when only file lists decide. + + A change set joins the current wave when it shares no file with the wave or with + any earlier change set still held back. Also returns, per change set that had to + wait, the change set it waited on last and the files they share. + """ + waves: list[list[int]] = [] + waited: dict[int, tuple[int, list[str]]] = {} + remaining = sorted(files) + while remaining: + wave: list[int] = [] + held: list[int] = [] + for change_set in remaining: + for earlier in wave + held: + shared = shared_files(files[change_set], files[earlier]) + if shared: + waited[change_set] = (earlier, shared) + held.append(change_set) + break + else: + wave.append(change_set) + waves.append(wave) + remaining = held + return waves, waited + + +def wave_report(body: list[tuple[int, str]], built: set[int]) -> list[str]: + files = {number: paths for number, paths in change_set_files(body).items() if number not in built} + if len(files) < 2: + return [] + waves, waited = build_waves(files) + layout = " ".join("[" + " ".join(str(n) for n in wave) + "]" for wave in waves) + lines = [ + f"build waves by file lists alone: {len(files)} change sets in {len(waves)} waves - {layout}" + ] + for change_set, (other, shared) in sorted(waited.items()): + more = f" (+{len(shared) - 3} more)" if len(shared) > 3 else "" + lines.append(f" {change_set} waits on {other}: {', '.join(shared[:3])}{more}") + return lines + + def check_validation(lines: list[str], problem) -> None: for index, line in enumerate(lines): if line.strip().lower() == "### validation": @@ -192,7 +286,8 @@ def problem(number: int | None, message: str) -> None: check_decisions(decisions, problem) check_echoes(found.get("scope", []), decisions, False, problem) check_echoes(found.get("change plan", []), decisions, True, problem) - check_change_plan(found.get("change plan", []), problem) + built = built_change_sets(path) + check_change_plan(found.get("change plan", []), built, problem) check_validation(lines, problem) for _, message in sorted(problems): @@ -201,6 +296,8 @@ def problem(number: int | None, message: str) -> None: print(f"\n{len(problems)} problem(s); the spec is not final.") return 1 print(f"{path}: clean - {len(decisions)} decisions argued.") + for line in wave_report(found.get("change plan", []), built): + print(line) return 0 diff --git a/docs/architecture.md b/docs/architecture.md index 8cc1c35..fa1ff3d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -27,7 +27,7 @@ Captured: 2026-09-17 (full, scope) - Updated: 2026-09-20 (the factory plugin and ### Building the Codex distribution 1. `scripts/build_codex_plugin.py` copies skills/, references/, and `scripts/` of the source plugin into plugins/{name}/, strips Claude-only frontmatter, and writes agents/openai.yaml. -2. `scripts/validate.sh` checks structure, links, skill length, the Codex distribution (C01), and the Pi package and transport (P01); `--check` mode of the generator fails when `plugins/` is stale. +2. `scripts/validate.sh` checks structure, links, skill length, the Codex distribution (C01), the Pi package and transport (P01), and the dev spec-lint and change-set-brief tests (D01); `--check` mode of the generator fails when `plugins/` is stale. ## Boundaries diff --git a/docs/decisions.md b/docs/decisions.md index 9d6801f..37312d6 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -17,3 +17,26 @@ D-complexity-threshold: Where does the ship gauntlet's coverage-weighted complex D-remove-factory: The factory plugin and its runner are removed from this repository (2026-09-20) ✓ delete `factory/`, `plugins/factory/`, the runner end-to-end test, and the F01 through F04 validator checks - the implementation was not working out (user, 2026-09-20); the repository goes back to shipping `dev` alone ✗ keep it unmaintained behind a flag - dead weight in every validation run and every plugin build, and it would keep drifting from `dev` + +## Build speed + +D-wave-gate: How often does build run a spec's Validation block? (2026-10-01, user feedback on the contexia searchable-kb build) + ✓ once per wave, by the orchestrator, as the gate before the wave's commits; an implementer runs only the test files its change set adds or edits plus typecheck and lint, and e2e, benchmark, and suite-repeating analysis commands wait for the end of the build - the block ran more than once per change set, and it held the e2e suite, the benchmarks, and a complexity script that re-runs the suite with coverage (user, 2026-10-01; contexia `.dev/metrics.jsonl`: builds of 11,773 s and 17,565 s); in a shared tree an agent's full run also measures its siblings' half-finished work ⚠ a regression outside a change set's own tests is found at the wave gate, not by the agent that caused it, and a complexity or benchmark miss only at the end + ✗ once per build - the commits in between would be unverified, and a late red has to be bisected across change sets + ✗ two separate blocks in the spec, a fast one and a full one - a format change every reader of the block would have to learn; an `(end of build)` mark on a command says the same inside the one block, and build recognizes the three kinds unmarked, so older specs get the gate too +D-seen-red-cost: What may proving a test red cost? (2026-10-01, user feedback on the contexia searchable-kb build) + ✓ nothing when the test is written first, and one break per slice, running one test file, when it is written after - the rule guarantees that no test is unable to fail, and one break proves that for every test of the slice; one change set spent 47 break-and-rerun cycles on 75 tests (user, 2026-10-01) + ✗ drop the rule and lean on ship's mutation testing - it runs after the tests are already trusted, and only over the files the gauntlet scopes in + ✗ require test-first everywhere - build enforces the outcome, not the order; strict test-first stays reserved for bug fixes +D-change-set-size: How large may a change set be? (2026-10-01, user feedback on the contexia searchable-kb build) + ✓ at most 25 scenarios, enforced by `lint-spec.py`; a change set already logged in `implementation-notes.md` is exempt, because change sets never renumber - a 38-scenario change set took 73 minutes against 18 and 23 for the waves before it (contexia git log, 2026-10-01), and the limit flags 4 of the 61 change sets in the four contexia specs (27, 32, 38, 62 scenarios) ⚠ it counts scenarios, not files, so a change set with few scenarios and many files passes + ✗ a file-count limit - file lists are prose with abbreviations, so the count would be a guess the author can argue with + ✗ prose guidance alone - "iterate small" was already asked and did not hold; per P-deterministic-guards-over-prose +D-wave-report: How does scope see the parallelism its plan allows? (2026-10-01, user feedback on the contexia searchable-kb build) + ✓ `lint-spec.py` prints, on a clean spec, the build waves the file lists allow and the shared files that make a change set wait - "keep file lists disjoint where possible" was already asked and still produced three change sets queued behind `compose.ts`; printed rather than failed, because a chain is sometimes the honest shape of a change ⚠ file detection is a heuristic over backticked names, and what one change set consumes from another stays build's judgment + ✗ fail the lint on a chain - no threshold separates a careless chain from a necessary one + ✗ build overlapping change sets in separate worktrees and merge - spec order is the dependency order, so an overlap usually is a real dependency +D-change-set-brief: What does a change-set agent read before it starts? (2026-10-01, user feedback on the contexia searchable-kb build) + ✓ a brief `change-set-brief.py` cuts from the spec: every section but research and the change plan, the decisions its change set links, its own plan, and the notes without their test inventories - 42 KB against 130 KB of spec and notes for change set 5 of the contexia searchable-kb plan (measured 2026-10-01); every line is verbatim, so nothing is paraphrased ⚠ a decision a change set leans on but does not link is left out; the spec's path stays in the prompt for that + ✗ the whole spec and the notes - every agent pays for every other change set's plan + ✗ a summary the orchestrator writes per agent - output tokens are the slow ones, and a paraphrase can be wrong diff --git a/package.json b/package.json index 39d5398..628c1b3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tobrun/dev-workflow", - "version": "3.2.0", + "version": "3.3.0", "description": "Test-focused development workflow skills for Claude Code, Codex, and Pi.", "keywords": [ "pi-package", diff --git a/plugins/dev/.codex-plugin/plugin.json b/plugins/dev/.codex-plugin/plugin.json index 8201b0a..d8c10dc 100644 --- a/plugins/dev/.codex-plugin/plugin.json +++ b/plugins/dev/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "dev", - "version": "3.2.0", + "version": "3.3.0", "description": "Development workflow skills: scope changes with argued decisions, build across unit/integration/e2e with every scenario proven by tests, ship with a deterministic quality gauntlet and an adversarially verified review, create structured commits that feed a decision ledger, and render pitches or comprehension quizzes.", "author": { "name": "Tobrun" diff --git a/plugins/dev/references/ci-parity.md b/plugins/dev/references/ci-parity.md index 382ac06..92157c9 100644 --- a/plugins/dev/references/ci-parity.md +++ b/plugins/dev/references/ci-parity.md @@ -21,6 +21,8 @@ Record the commands and outcomes in the plan's `implementation-notes.md`. Run every reproducible project-owned required-check command against the final checkout, after feature E2E and after any hardening edits. +A command that already ran green on this exact tree - no file changed since - +is not run a second time: record its result and where it came from. - A failure is work to fix, including a test described as flaky, unrelated, or pre-existing. Diagnose and remove its nondeterminism; do not add retries, diff --git a/plugins/dev/skills/build/SKILL.md b/plugins/dev/skills/build/SKILL.md index 4659ac3..4cb02f4 100644 --- a/plugins/dev/skills/build/SKILL.md +++ b/plugins/dev/skills/build/SKILL.md @@ -16,11 +16,11 @@ Read [references/layers.md](references/layers.md), [references/tests.md](referen 1. Run `python3 {build-skill-root}/../../scripts/skill-metrics.py start build`, then read `spec.md` in full: the research section (the decisions and their rationale), the scope section (including its Validation block of real repo commands), and the change plan. Explore the relevant code. If the Validation block is absent, discover the repo's real test and typecheck commands yourself from `package.json`, a `Makefile`, or CI config, and log them in `implementation-notes.md`. 2. Build waves by disjoint batching per [references/parallel.md](references/parallel.md): sequential in spec order by default, batched only when file lists are disjoint and nothing a wave-mate or earlier unfinished change set introduces is consumed. Every change set in the plan is in scope, not just the first. -3. For each wave, run its change sets in parallel per the same reference, then commit each finished change set on the current branch and append its entry to `implementation-notes.md`. A change set that adds, removes, moves, or rewires a component, flow, or boundary updates `docs/architecture.md` in the same commit and passes `architecture-check.py` first, per [../../references/architecture.md](../../references/architecture.md). +3. For each wave, run its change sets in parallel per the same reference, pass its wave gate once, then commit each finished change set on the current branch and append its entry to `implementation-notes.md`. A change set that adds, removes, moves, or rewires a component, flow, or boundary updates `docs/architecture.md` in the same commit and passes `architecture-check.py` first, per [../../references/architecture.md](../../references/architecture.md). 4. Move straight to the next wave. Never stop after one change set or wave to ask about review. 5. When every change set is committed, loop `python3 {build-skill-root}/scripts/check-tests.py .dev/{plan-name}` until it exits clean: it proves every specced scenario has a test that really exists, rather than one that was reported. 6. Then run the full e2e pass per "The e2e layer" below over the whole spec, and loop on failures until it is green. -7. Run the repository's required pull-request commands per [../../references/ci-parity.md](../../references/ci-parity.md), starting them in the background as soon as the e2e loop is green and rendering the e2e report while they run - the two share nothing. A known-red CI scenario is not an acceptable deviation. +7. Run the repository's required pull-request commands per [../../references/ci-parity.md](../../references/ci-parity.md), plus every Validation command the wave gates left for the end, starting them in the background as soon as the e2e loop is green and rendering the e2e report while they run - the two share nothing. A known-red CI scenario is not an acceptable deviation. 8. After the e2e report and CI-parity gate, close per "Closing message". Build never pushes or opens a PR; that is `ship`'s phase 3. ## Jira sync @@ -57,12 +57,12 @@ Neither does a failed e2e loop: say what is blocked, then still point at `ship`. Each change set, whether you run it yourself or a subagent runs it, follows the same loop: - Test at the seams the spec's scope section declares, per [references/tests.md](references/tests.md); if the declared boundary is wrong or missing, follow its fallback and log the change under Deviations - do not stall on it. -- Implement in **vertical slices**: one scenario's behavior at a time, its test written before or right after the code - the enforced outcome is what matters, not the ritual order. Each `tests:` scenario's test lives at its tagged layer ([references/layers.md](references/layers.md)); a scenario isn't met until a real test exists there. -- Run the change set's own tests and typecheck continuously; once the change set is green, run the spec's Validation block verbatim - it is the wider suite plus typecheck/lint - and only report done when it passes clean. +- Implement in **vertical slices**: one scenario's behavior at a time, its test written before the code or right after it - before is the cheap order, because its first run is then the red the rules below ask for. Each `tests:` scenario's test lives at its tagged layer ([references/layers.md](references/layers.md)); a scenario isn't met until a real test exists there. +- Run the change set's own tests and typecheck continuously - the test files it adds or edits, never the whole suite - and report done when they are green. The spec's Validation block is not run per change set: it is the wave gate, run once per wave before its commits, per "The wave gate" in [references/parallel.md](references/parallel.md). ## Rules of the loop -- **Every test must have been seen red.** A test that has never failed proves nothing: earn its green by writing it before the code, or by briefly breaking the behavior once after. Bug fixes are strictly test-first: a defect change set starts with a failing test that reproduces the reported issue - red is the proof it was actually reproduced - only then fix, and watch that same test go green. +- **Every test must have been seen red, once and cheaply.** A test that has never failed proves nothing. Written before the code, its first run is that red and costs nothing. Written after, break the behavior once per slice - one break covers every test of the slice - and run only the affected test file: never one break per test, never the suite for a red. Bug fixes are strictly test-first: a defect change set starts with a failing test that reproduces the reported issue - red is the proof it was actually reproduced - only then fix, and watch that same test go green. - **One slice at a time.** One seam, one behavior, one test, one minimal implementation per cycle. - **Refactoring is not part of the loop.** It belongs to `ship`'s review phase. - **Keep going.** A red test, a failing e2e scenario, or an edge case that contradicts the spec is work to do, not a reason to hand back. Fix it, log the deviation, continue. Stop early only when a blocking question makes further work unsafe or wasted. diff --git a/plugins/dev/skills/build/references/parallel.md b/plugins/dev/skills/build/references/parallel.md index d3b6a05..3aac303 100644 --- a/plugins/dev/skills/build/references/parallel.md +++ b/plugins/dev/skills/build/references/parallel.md @@ -20,17 +20,27 @@ Never start work belonging to the next wave while the current wave is in flight. Launch one `Agent` per change set, **all in a single message** so they run concurrently. A wave of one change set needs no subagent: implement it yourself in the main thread. +First write the wave's briefs, one command for the whole wave: + +```bash +python3 {build-skill-root}/scripts/change-set-brief.py .dev/{plan-name} {N} {N} --out-dir /tmp/{project-slug}/briefs/{plan-name} +``` + +A brief is the spec cut down to one change set: the scope section, the decisions that change set links, its own plan, and what earlier change sets did and deviated on. +Every line is the spec's own, so an agent starts from about a third of the reading and loses nothing it builds against. +Write them per wave, never once up front: the notes they carry grow with every committed change set. + Each agent prompt contains: 1. The role: `You implement exactly one change set of a spec. Other agents implement sibling change sets concurrently; stay inside your change set's file list.` -2. The absolute path to `spec.md`, the number of the change set the agent owns, and this skill's `layers.md`, `tests.md`, and `mocking.md` - the agent reads them itself rather than receiving them inlined. The spec is a self-contained handoff by design; the agent reads all of it, then implements only its own change set. +2. The absolute path to the change set's brief, the number of the change set the agent owns, and this skill's `layers.md`, `tests.md`, and `mocking.md` - the agent reads them itself rather than receiving them inlined. The brief stands in for `spec.md` and `implementation-notes.md`: the agent reads neither whole, and opens the spec only to follow something the brief points at. 3. The absolute path to this skill's `SKILL.md`, with the instruction to follow its "The change-set loop" and "Rules of the loop" sections - read like the other references, not pasted into the prompt. 4. Hard constraints: - Implement only this change set's `[unit]` and `[integration]` scenarios. `[e2e]` scenarios are run once per spec by the orchestrator afterwards - do not launch the app. - Do not commit, stage, or touch git state. The orchestrator commits. - Do not edit files outside your change set's file list. If the change set genuinely needs a file another change set owns, stop and report it as a conflict instead of editing it. - Do not edit `implementation-notes.md`. Report your entry; the orchestrator appends it. - - Run the spec's Validation block and report its real result. A red result is a fact to report, not something to hide or paper over. + - Run only your own checks: the test files this change set adds or edits, plus typecheck and lint over the packages it touches. Never run the spec's Validation block, the whole suite, the e2e suite, or a benchmark - siblings are mid-edit in the same tree, so a wider run measures their half-finished work, and the orchestrator runs the Validation block once for the whole wave. Report the commands and their real result. A red result is a fact to report, not something to hide or paper over. 5. The output contract below. Output contract (the agent's final message must be exactly one fenced JSON block): @@ -50,13 +60,22 @@ Output contract (the agent's final message must be exactly one fenced JSON block ## After a wave -1. Verify rather than trust the reports: run the spec's Validation block yourself, once per wave - the shared tree already holds the whole wave's changes, so one run covers every change set in it. +1. Verify rather than trust the reports: run the wave gate below yourself, once per wave - the shared tree already holds the whole wave's changes, so one run covers every change set in it. 2. Commit each change set's work on the current branch, in number order, one commit per change set. A skipped-ahead change set's commit waits until every earlier change set is committed, so history keeps the spec's order. 3. Append each change set's entry to `implementation-notes.md` from `whatWasDone`, `seamsTested`, `testsAdded`, and `deviations`. `testsAdded` becomes the entry's `Tests added:` line, which the scenario checker reads. A `status: blocked` change set, a failing validation, or a reported conflict is yours to finish in the main thread before the next wave starts - do not carry a red change set forward and do not relaunch the same agent on the same failure more than once. If two change sets in a wave edited the same file anyway, reconcile it yourself and log it under Deviations. +## The wave gate + +The spec's Validation block is the gate between a wave and its commits, and the only full run a wave gets - a wave of one you implemented yourself included. +Each of its commands runs once per tree: a green result stands until a file changes, so nothing is re-run "to be sure" before committing, and after a fix the failed command runs first and the rest only once it is green. +Start commands that share no state together (lint, typecheck, and the unit suite), and write the wave's notes entries while they run. + +A command the block marks `(end of build)` is left out of the wave gate, and so are three kinds even when a spec lists one unmarked, because each costs minutes and tells a wave nothing it needs to commit: the e2e suite (the e2e pass runs it), benchmarks, and any analysis that re-runs the suite to measure it - coverage, complexity, mutation. +Each of those runs once, on the final tree, with the CI-parity gate. + ## When not to parallelize - The spec has one change set, or every change set's files overlap with the one before it: run them sequentially yourself. diff --git a/plugins/dev/skills/build/scripts/change-set-brief.py b/plugins/dev/skills/build/scripts/change-set-brief.py new file mode 100755 index 0000000..a44b2ba --- /dev/null +++ b/plugins/dev/skills/build/scripts/change-set-brief.py @@ -0,0 +1,169 @@ +#!/usr/bin/env python3 +"""Cut a spec down to what one change set's implementer needs. + +Usage: python3 change-set-brief.py .dev/{plan-name} N [N ...] [--out-dir DIR] + +A brief is spec.md minus the decisions the change set does not link and minus +the other change sets, followed by what earlier change sets did and deviated on +from implementation-notes.md. Every kept line is verbatim, so an agent reading +the brief reads the spec's own words, a third of them. + +With --out-dir, writes DIR/change-set-N.md per change set and prints each path +with its size against the spec. Without it, prints the one brief to stdout. +Exit 2 on a missing spec or an unknown change set. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +HEADING = re.compile(r"^##\s+(.*?)\s*$") +DECISION = re.compile(r"^(D-[a-z0-9]+(?:-[a-z0-9]+)*):") +LINKED = re.compile(r"\bD-[a-z0-9]+(?:-[a-z0-9]+)*") +CHANGE_SET = re.compile(r"^\s*(\d+)\.\s+\S") +NOTE_ENTRY = re.compile(r"^##\s+Change set\s+(\d+)\b", re.IGNORECASE) +# The two note lines only the scenario checker and the reviewers read. +NOTE_NOISE = re.compile(r"^\s*-\s*(Tests added|Seams tested):", re.IGNORECASE) + + +def split_sections(lines: list[str]) -> list[tuple[str, list[str]]]: + """The spec as (lower-cased ## heading, lines) pairs; the text before the first is ''.""" + sections: list[tuple[str, list[str]]] = [("", [])] + for line in lines: + heading = HEADING.match(line) + if heading: + sections.append((heading.group(1).lower(), [line])) + else: + sections[-1][1].append(line) + return sections + + +def decision_entries(research: list[str]) -> dict[str, list[str]]: + """Each decision's header and its indented alternative lines, by slug.""" + entries: dict[str, list[str]] = {} + current = None + for line in research[1:]: + header = DECISION.match(line) + if header: + current = entries.setdefault(header.group(1), []) + current.append(line) + elif current is not None and line[:1] in (" ", "\t") and line.strip(): + current.append(line) + elif line.strip(): + current = None + return entries + + +def change_set_blocks(plan: list[str]) -> dict[int, list[str]]: + """Each change set's lines, from its numbered line to the next one.""" + blocks: dict[int, list[str]] = {} + current = None + for line in plan[1:]: + change_set = CHANGE_SET.match(line) + if change_set: + current = blocks.setdefault(int(change_set.group(1)), []) + if current is not None: + current.append(line) + for block in blocks.values(): + while block and not block[-1].strip(): + block.pop() + return blocks + + +def notes_digest(notes: Path) -> list[str]: + """implementation-notes.md without its test and seam inventories.""" + if not notes.is_file(): + return [] + kept = [ + line for line in notes.read_text(encoding="utf-8").splitlines() + if not NOTE_NOISE.match(line) and not line.startswith("# ") + ] + return kept if any(NOTE_ENTRY.match(line) for line in kept) else [] + + +def brief(plan_dir: Path, number: int) -> str: + spec = plan_dir / "spec.md" + sections = split_sections(spec.read_text(encoding="utf-8").splitlines()) + by_name = dict(sections) + blocks = change_set_blocks(by_name.get("change plan", [""])) + if number not in blocks: + known = ", ".join(str(n) for n in sorted(blocks)) or "none" + raise LookupError(f"{spec} has no change set {number} (change sets: {known})") + block = blocks[number] + decisions = decision_entries(by_name.get("research", [""])) + linked = list(dict.fromkeys(slug for line in block for slug in LINKED.findall(line))) + + out = [ + f"# Brief: change set {number} of {plan_dir.name}", + "", + f"This is {spec.resolve()} cut down to change set {number}: every line below is the spec's own.", + "Left out are the decisions this change set does not link and the other change sets' plans.", + "Open the spec only to follow something this brief points at, never to read it whole.", + "", + ] + for name, lines in sections: + if name == "research": + out += ["## Research (the decisions this change set links)", ""] + for slug in linked: + out += decisions.get(slug, [f"{slug}: not argued in the spec's research section"]) + [""] + if not linked: + out += ["This change set links no decision.", ""] + elif name == "change plan": + out += [f"## Change plan (change set {number} only)", ""] + block + [""] + others = [blocks[n][0].strip() for n in sorted(blocks) if n != number] + if others: + out += ["The other change sets, whose files are not yours:"] + [f"- {o[:140]}" for o in others] + [""] + else: + out += lines + ([""] if lines and lines[-1].strip() else []) + digest = notes_digest(plan_dir / "implementation-notes.md") + if digest: + out += ["## What earlier change sets did (from implementation-notes.md)", ""] + out += [("#" + line) if line.startswith("## ") else line for line in digest] + return "\n".join(out).rstrip() + "\n" + + +def main(argv: list[str]) -> int: + args: list[str] = [] + out_dir = None + rest = argv[1:] + while rest: + value = rest.pop(0) + if value == "--out-dir" and rest: + out_dir = Path(rest.pop(0)) + else: + args.append(value) + numbers = args[1:] + if not numbers or not all(n.isdigit() for n in numbers) or (out_dir is None and len(numbers) != 1): + print( + "usage: change-set-brief.py [ ...] [--out-dir DIR]\n" + " several change sets need --out-dir", + file=sys.stderr, + ) + return 2 + plan_dir = Path(args[0]) + spec = plan_dir / "spec.md" + if not spec.is_file(): + print(f"no spec.md at {spec}", file=sys.stderr) + return 2 + try: + briefs = {int(n): brief(plan_dir, int(n)) for n in numbers} + except LookupError as error: + print(error, file=sys.stderr) + return 2 + if out_dir is None: + sys.stdout.write(briefs[int(numbers[0])]) + return 0 + out_dir.mkdir(parents=True, exist_ok=True) + spec_bytes = len(spec.read_bytes()) + for number, text in briefs.items(): + target = out_dir / f"change-set-{number}.md" + target.write_text(text, encoding="utf-8") + size = len(text.encode("utf-8")) + print(f"{target.resolve()}: {size} bytes, {round(100 * size / spec_bytes)}% of spec.md") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv)) diff --git a/plugins/dev/skills/build/scripts/check-tests.py b/plugins/dev/skills/build/scripts/check-tests.py index 7d90d25..ccbb1aa 100755 --- a/plugins/dev/skills/build/scripts/check-tests.py +++ b/plugins/dev/skills/build/scripts/check-tests.py @@ -18,6 +18,8 @@ TESTS = re.compile(r"^\s*tests:\s*(.*)$", re.IGNORECASE) NOTE_ENTRY = re.compile(r"^##\s+Change set\s+(\d+)\b", re.IGNORECASE) NOTE_TESTS = re.compile(r"^\s*-\s*Tests added:\s*(.*)$", re.IGNORECASE) +# A comma separates two references only when a path::name follows it; a test name may hold commas. +NOTE_SEPARATOR = re.compile(r",\s*(?=[^,]*::)") def spec_scenarios(spec: Path, problem) -> dict[int, int]: @@ -65,7 +67,7 @@ def note_entries(notes: Path) -> dict[int, list[str]]: value = named.group(1).strip() if value.lower().startswith("none"): continue - entries[current].extend(t.strip() for t in value.split(",") if t.strip()) + entries[current].extend(t.strip() for t in NOTE_SEPARATOR.split(value) if t.strip()) return entries diff --git a/plugins/dev/skills/scope-review/references/lenses.md b/plugins/dev/skills/scope-review/references/lenses.md index 0e5fa5f..ed4f6d9 100644 --- a/plugins/dev/skills/scope-review/references/lenses.md +++ b/plugins/dev/skills/scope-review/references/lenses.md @@ -11,7 +11,8 @@ Does the change plan survive contact with the repo? - Every file a change set names exists, or the set says it is new; the described edit is possible at that site - the function, hook, or config it assumes is really there. - Prior art and idioms the spec cites exist where it says they do. - Premises about current behavior are checked against the code, never trusted: an "X already handles Y" claim that is false is a BLOCK naming the site. -- The Validation block's commands exist in the repo's manifests and run the layers the plan relies on. +- The Validation block's commands exist in the repo's manifests and run the layers the plan relies on - and nothing slower: an e2e suite, benchmark, or coverage re-run listed there without its `(end of build)` mark is a CONCERN, because build pays for the block every wave. +- The plan is as wide as the change allows: change sets queued behind one shared file that a single change set could own are a CONCERN naming the file (`lint-spec.py` prints the waves). ## completeness diff --git a/plugins/dev/skills/scope/SKILL.md b/plugins/dev/skills/scope/SKILL.md index f621d9c..8767cde 100644 --- a/plugins/dev/skills/scope/SKILL.md +++ b/plugins/dev/skills/scope/SKILL.md @@ -89,6 +89,7 @@ Name the components and flows the change adds, removes, or reshapes, in the over Efforts have second-order effects - capture them as nested sub-efforts, each carrying its own decisions back into the research section (rate limiting in scope means Redis setup, which carries config and deploy decisions). Record considered non-goals as `⊘` lines with a because clause - things someone weighed and cut, not mere omissions. End the scope with a `### Validation` block listing the repo's real typecheck/test/lint/build commands, discovered from `package.json`, a `Makefile`, CI config, or equivalent - never guess `npm test` into a `pytest` repo; ask if you cannot determine them. +`build` runs this block once per wave, so list each check once and mark every command a wave does not need - the e2e suite, a benchmark, a coverage or complexity run that repeats the suite - `(end of build)`: build runs those once, on the final tree. Writing style for the spec: ELI12, no similes or metaphors. ## 5. Review and research @@ -103,7 +104,9 @@ Writing style for the spec: ELI12, no similes or metaphors. Short fragmented sentences. Link decisions by ID wherever one applies, echoing the choice. Each change set ends with one `;`-separated `tests:` line - concrete scenarios as input -> expected outcome, each tagged `[unit]`, `[integration]`, or `[e2e]`, covering happy path, edge cases, and failure paths; a set with nothing to test says `tests: none - {reason}`. Specific enough that whoever writes the tests invents nothing; the author tags layers here because a fresh implementation session can't recover that intent. -Order change sets so each builds only on the ones before it; keep file lists disjoint where possible - `build` parallelizes consecutive change sets whose files don't overlap. +Order change sets so each builds only on the ones before it, and shape the plan wide: `build` runs change sets in parallel only while their file lists are disjoint, so a file three change sets edit makes them queue. +Land what several change sets share - a port, a schema, a registry, the composition root, a regenerated artifact - in one change set, and let the ones building on it own disjoint files. +Size each change set for one agent: past 25 scenarios it is several change sets, split along a seam. ``` 1. Change set 1 @@ -118,6 +121,7 @@ Order change sets so each builds only on the ones before it; keep file lists dis Then loop `python3 {scope-skill-root}/scripts/lint-spec.py .dev/{plan-name}/spec.md` until it exits clean. It owns the mechanics above; the spec is not final while it reports anything. +Once clean it prints the build waves the file lists allow and the files that make a change set wait; where change sets queue behind a shared file, move that file's edits into one change set and lint again. ## 7. Visualize diff --git a/plugins/dev/skills/scope/scripts/lint-spec.py b/plugins/dev/skills/scope/scripts/lint-spec.py index 2056f78..2cb73b3 100755 --- a/plugins/dev/skills/scope/scripts/lint-spec.py +++ b/plugins/dev/skills/scope/scripts/lint-spec.py @@ -3,6 +3,8 @@ Usage: python3 lint-spec.py .dev/{plan-name}/spec.md Exit 0 when the spec is clean, 1 with one problem per line otherwise. +A clean spec also gets the build waves its file lists allow, so the author sees +which shared files make change sets wait for each other. """ from __future__ import annotations @@ -20,8 +22,19 @@ TESTS = re.compile(r"^\s*tests:\s*(.*)$", re.IGNORECASE) LAYER = re.compile(r"^\[(unit|integration|e2e)\]\s+\S") HEADING = re.compile(r"^##\s+(.*?)\s*$") +NOTE_ENTRY = re.compile(r"^##\s+Change set\s+(\d+)\b", re.IGNORECASE) +BACKTICKED = re.compile(r"`([^`\s]+)`") +# A backticked token is a file when it ends in a known file extension; +# `embedder.model` and `job.stages` are code, not paths. +FILE_NAME = re.compile( + r"(?:^|/)(?:Makefile|Dockerfile|[\w.@+-]+\." + r"(?:c|cc|cpp|cs|css|go|gradle|graphql|h|html|java|js|json|jsx|kt|lock|md|mjs|cjs|php|proto" + r"|py|rb|rs|scss|sh|sql|svelte|swift|tf|toml|ts|tsx|txt|vue|xml|yaml|yml))$" +) -CHOSEN, REJECTED, OPEN, NOT_DOING = "✓", "✗", "?", "⊘" +CHOSEN, OPEN, NOT_DOING = "✓", "?", "⊘" +# One agent builds one change set; past this many scenarios it stops fitting in one sitting. +SCENARIO_LIMIT = 25 def sections(lines: list[str]) -> dict[str, list[tuple[int, str]]]: @@ -122,7 +135,16 @@ def check_echoes( problem(number, f"the change plan links {slug}, which is still open or flagged") -def check_change_plan(body: list[tuple[int, str]], problem) -> None: +def built_change_sets(spec: Path) -> set[int]: + """Change sets build already logged; their size is history, and they never renumber.""" + notes = spec.parent / "implementation-notes.md" + if not notes.is_file(): + return set() + entries = (NOTE_ENTRY.match(line) for line in notes.read_text(encoding="utf-8").splitlines()) + return {int(entry.group(1)) for entry in entries if entry} + + +def check_change_plan(body: list[tuple[int, str]], built: set[int], problem) -> None: counts: dict[int, int] = {} current = None for number, line in body: @@ -148,15 +170,87 @@ def check_change_plan(body: list[tuple[int, str]], problem) -> None: if not value: problem(number, "tests: line is empty") continue - for scenario in value.split(";"): - scenario = scenario.strip() - if scenario and not LAYER.match(scenario): + scenarios = [scenario.strip() for scenario in value.split(";") if scenario.strip()] + for scenario in scenarios: + if not LAYER.match(scenario): problem(number, f"scenario '{scenario[:40]}' carries no [unit]/[integration]/[e2e] tag") + if len(scenarios) > SCENARIO_LIMIT and current not in built: + problem( + number, + f"change set {current} carries {len(scenarios)} scenarios; split it along a seam " + f"so no change set carries more than {SCENARIO_LIMIT}", + ) for change_set, seen in sorted(counts.items()): if seen != 1: problem(None, f"change set {change_set} has {seen} tests: lines; expected exactly one") +def change_set_files(body: list[tuple[int, str]]) -> dict[int, set[str]]: + """Files each change set names, read from the backticked paths outside its tests: line.""" + files: dict[int, set[str]] = {} + current = None + for _, line in body: + change_set = CHANGE_SET.match(line) + if change_set: + current = int(change_set.group(1)) + files.setdefault(current, set()) + if current is None or TESTS.match(line): + continue + files[current].update(t for t in BACKTICKED.findall(line) if FILE_NAME.search(t)) + return files + + +def shared_files(one: set[str], other: set[str]) -> list[str]: + """Paths two change sets both name; a bare file name matches any path that ends in it.""" + shared = one & other + for mine, theirs in ((one, other), (other, one)): + bare = {path for path in theirs if "/" not in path} + shared |= {path for path in mine if "/" in path and path.rsplit("/", 1)[-1] in bare} + return sorted(shared) + + +def build_waves(files: dict[int, set[str]]) -> tuple[list[list[int]], dict[int, tuple[int, list[str]]]]: + """The waves build's consecutive-disjoint batching gives when only file lists decide. + + A change set joins the current wave when it shares no file with the wave or with + any earlier change set still held back. Also returns, per change set that had to + wait, the change set it waited on last and the files they share. + """ + waves: list[list[int]] = [] + waited: dict[int, tuple[int, list[str]]] = {} + remaining = sorted(files) + while remaining: + wave: list[int] = [] + held: list[int] = [] + for change_set in remaining: + for earlier in wave + held: + shared = shared_files(files[change_set], files[earlier]) + if shared: + waited[change_set] = (earlier, shared) + held.append(change_set) + break + else: + wave.append(change_set) + waves.append(wave) + remaining = held + return waves, waited + + +def wave_report(body: list[tuple[int, str]], built: set[int]) -> list[str]: + files = {number: paths for number, paths in change_set_files(body).items() if number not in built} + if len(files) < 2: + return [] + waves, waited = build_waves(files) + layout = " ".join("[" + " ".join(str(n) for n in wave) + "]" for wave in waves) + lines = [ + f"build waves by file lists alone: {len(files)} change sets in {len(waves)} waves - {layout}" + ] + for change_set, (other, shared) in sorted(waited.items()): + more = f" (+{len(shared) - 3} more)" if len(shared) > 3 else "" + lines.append(f" {change_set} waits on {other}: {', '.join(shared[:3])}{more}") + return lines + + def check_validation(lines: list[str], problem) -> None: for index, line in enumerate(lines): if line.strip().lower() == "### validation": @@ -192,7 +286,8 @@ def problem(number: int | None, message: str) -> None: check_decisions(decisions, problem) check_echoes(found.get("scope", []), decisions, False, problem) check_echoes(found.get("change plan", []), decisions, True, problem) - check_change_plan(found.get("change plan", []), problem) + built = built_change_sets(path) + check_change_plan(found.get("change plan", []), built, problem) check_validation(lines, problem) for _, message in sorted(problems): @@ -201,6 +296,8 @@ def problem(number: int | None, message: str) -> None: print(f"\n{len(problems)} problem(s); the spec is not final.") return 1 print(f"{path}: clean - {len(decisions)} decisions argued.") + for line in wave_report(found.get("change plan", []), built): + print(line) return 0 diff --git a/scripts/validate.sh b/scripts/validate.sh index d6332d7..1c75b7a 100755 --- a/scripts/validate.sh +++ b/scripts/validate.sh @@ -614,6 +614,27 @@ check_pi() { fi } +# =========================================================================== +# D01: the dev plugin's spec-lint and change-set-brief tests pass +# =========================================================================== +check_dev_script() { + local dir="dev/evals/tests" + [ -d "$dir" ] || return + if ! python3 - "$dir" >"$LOG_DIR/dev-script-tests.log" 2>&1 <<'PYEOF' +import sys, unittest + +loader = unittest.TestLoader() +suite = loader.discover(start_dir=sys.argv[1], top_level_dir=".") +result = unittest.TextTestRunner(verbosity=2).run(suite) +sys.exit(0 if result.wasSuccessful() and result.testsRun > 0 else 1) +PYEOF + then + tail -60 "$LOG_DIR/dev-script-tests.log" >&2 + fail "D01" "$dir" \ + "Dev script tests failed (full output: $LOG_DIR/dev-script-tests.log); run: python3 -m unittest discover -s dev/evals/tests -t ." + fi +} + # =========================================================================== # Main # =========================================================================== @@ -635,6 +656,7 @@ check_skill_length check_links check_codex check_pi +check_dev_script if [ -s "$ERROR_FILE" ]; then echo ""