diff --git a/scenarios/001-toolchain-preflight.md b/scenarios/001-toolchain-preflight.md index f7b5386..8f063f7 100644 --- a/scenarios/001-toolchain-preflight.md +++ b/scenarios/001-toolchain-preflight.md @@ -1,10 +1,10 @@ --- -status: draft +status: active --- # Scenario 001: Dispatcher fail-fasts when ast-grep binary is missing -Validates that `/coding:code-review` and `/coding:pr-review` abort with an actionable error when `ast-grep` / `sg` is absent from PATH — closing the silent-empty-review failure mode observed on [bborbe/coding#34](https://github.com/bborbe/coding/pull/34). +Validates that the Step 4.0 preflight block in `commands/pr-review.md` and `commands/code-review.md` (and the Step 0 preflight block in `agents/ast-grep-runner.md`) exits 1 with the documented error message when `ast-grep` / `sg` is absent from PATH — closing the silent-empty-review failure mode observed on [bborbe/coding#34](https://github.com/bborbe/coding/pull/34). ## Setup @@ -16,27 +16,31 @@ Validates that `/coding:code-review` and `/coding:pr-review` abort with an actio ``` - [ ] `PATH=$PATH_MASKED command -v ast-grep; echo "exit=$?"` prints `exit=1` - [ ] `PATH=$PATH_MASKED command -v sg; echo "exit=$?"` prints `exit=1` -- [ ] Create a minimal Go fixture: `WORK=$(mktemp -d) && cd "$WORK" && git init -q && printf 'package main\n\nfunc main() {}\n' > main.go && git add . && git commit -qm initial` -- [ ] Host shell `command -v ast-grep` (outside any subshell) still resolves a path +- [ ] Extract the Step 4.0 bash block verbatim from `commands/pr-review.md` and `commands/code-review.md` (search for `#### 4.0:` and pick the fenced `bash` block beneath it). Keep them as `$PRREVIEW_STEP40` and `$CODEREVIEW_STEP40` shell variables. **Fragility note**: the extraction relies on the literal section header `#### 4.0:`. If a future refactor renames or renumbers the heading, the extraction silently picks up the wrong block; bump this scenario's brittleness by re-anchoring on the new heading text. +- [ ] Extract the Step 0 preflight bash block from `agents/ast-grep-runner.md` (search for `### 0. Preflight:`, pick the fenced `bash` block). Keep as `$RUNNER_STEP0`. Same fragility caveat: anchored on the literal `### 0. Preflight:` heading. ## Action -- [ ] In a fresh Claude Code session launched as `PATH=$PATH_MASKED claude`, run `/coding:code-review master` against `$WORK`; tee stdout to `/tmp/cr-stdout.log`, stderr to `/tmp/cr-stderr.log`, and capture exit code: `... ; echo $? > /tmp/cr-exit` -- [ ] In a fresh Claude Code session launched as `PATH=$PATH_MASKED claude`, run `/coding:pr-review master` against `$WORK`; same tee + exit-code capture to `/tmp/pr-{stdout,stderr,exit}.log` -- [ ] Direct runner test: in a `PATH=$PATH_MASKED` subshell, invoke the `coding:ast-grep-runner` agent via `claude` with the prompt `TARGET_DIR=$WORK. Run every YAML in rules//*.yml. Return findings grouped by Owner.` — tee stdout to `/tmp/runner-stdout.log`, capture exit code to `/tmp/runner-exit` +- [ ] Run the pr-review Step 4.0 block under the masked PATH; capture stderr and exit code: + ```bash + PATH=$PATH_MASKED bash -c "$PRREVIEW_STEP40" 2> /tmp/scen001-pr.stderr + echo $? > /tmp/scen001-pr.exit + ``` +- [ ] Run the code-review Step 4.0 block the same way; save to `/tmp/scen001-cr.{stderr,exit}` +- [ ] Run the runner Step 0 block the same way; runner emits JSON to stdout before exit — save to `/tmp/scen001-runner.{stdout,exit}` (`PATH=$PATH_MASKED bash -c "$RUNNER_STEP0" > /tmp/scen001-runner.stdout 2> /tmp/scen001-runner.stderr; echo $? > /tmp/scen001-runner.exit`) ## Expected -- [ ] `/tmp/cr-stderr.log` contains the literal string `ast-grep/sg not in PATH` -- [ ] `cat /tmp/cr-exit` prints `1` -- [ ] `grep -c 'coding:ast-grep-runner agent:' /tmp/cr-stdout.log` returns `0` (Step 4a was never invoked) -- [ ] `/tmp/pr-stderr.log` contains `ast-grep/sg not in PATH` and `cat /tmp/pr-exit` prints `1` -- [ ] `jq -e '.errors[] | select(.kind == "missing-tool" and .tool == "ast-grep")' /tmp/runner-stdout.log` exits 0 -- [ ] `jq '.stats.yamls_run' /tmp/runner-stdout.log` returns `0` and `jq '.findings_by_owner' /tmp/runner-stdout.log` returns `{}` -- [ ] `cat /tmp/runner-exit` prints `1` -- [ ] Each invocation's wall-clock under 30 seconds (`time` output's `real` < 30s) — the regression risk being locked down is a 30-min `activeDeadlineSeconds` kill instead of immediate fail -- [ ] Host shell `command -v ast-grep` (re-run after all subshells) still resolves a path +- [ ] `cat /tmp/scen001-pr.exit` prints `1` +- [ ] `/tmp/scen001-pr.stderr` contains the literal string `ast-grep/sg not in PATH` +- [ ] `cat /tmp/scen001-cr.exit` prints `1` +- [ ] `/tmp/scen001-cr.stderr` contains the literal string `ast-grep/sg not in PATH` (parity with pr-review.md) +- [ ] `cat /tmp/scen001-runner.exit` prints `1` +- [ ] `jq -e '.errors[] | select(.kind == "missing-tool" and .tool == "ast-grep")' /tmp/scen001-runner.stdout` exits `0` +- [ ] `jq '.stats.yamls_run' /tmp/scen001-runner.stdout` returns `0` and `jq '.findings_by_owner' /tmp/scen001-runner.stdout` returns `{}` +- [ ] Each block's wall-clock under 1 second (each block is a single `command -v` check that returns immediately — 1s is generous even on CI environments with slow disk I/O; the regression risk is the `sg --version` loop on coding#34 which took 30 min, so any threshold below the 30-min `activeDeadlineSeconds` ceiling proves the contract, and 1s catches the loop with 1800× margin) +- [ ] Host shell `command -v ast-grep` (after all subshells) still resolves a path ## Cleanup -- `rm -rf "$WORK" /tmp/cr-* /tmp/pr-* /tmp/runner-*` +- `rm -f /tmp/scen001-*.{stderr,exit,stdout}` diff --git a/scenarios/003-scaling-funnel-100-files.md b/scenarios/003-scaling-funnel-100-files.md index a2d5f19..5be44e2 100644 --- a/scenarios/003-scaling-funnel-100-files.md +++ b/scenarios/003-scaling-funnel-100-files.md @@ -1,71 +1,72 @@ --- -status: draft +status: active --- -# Scenario 003: 100-file synthetic PR completes review in ≤30 LLM calls +# Scenario 003: 100-file synthetic PR funnel converges to ≤30 distinct owners -Validates that a 100-file synthetic PR with known mechanical violations completes `/coding:pr-review master standard` within ≤30 LLM calls and ≤30 minutes — proving the ast-grep funnel decouples LLM cost from PR size and stays under the prod bot's `activeDeadlineSeconds=1800` ceiling. Companion decoupling scenario (200-file re-run with ≤30 LLM calls) lives separately as scenario 004. +Validates that the ast-grep mechanical funnel against a 100-file synthetic PR with mixed violations completes in ≤30 seconds and surfaces findings under ≤30 distinct Owner agents — proving the funnel decouples LLM-tier cost from file count (since the dispatcher's Step 4b invokes ONE Task per Owner with findings, the upper bound on LLM calls is the distinct-Owner count plus a small fixed overhead). The full-pipeline LLM-call measurement (Phase 10 acceptance, requires an LLM-shim wrapping the `claude` binary) is deferred to a future scenario 004 — not yet written; this scenario captures the structural ceiling. ## Setup -- [ ] Build the synthetic-PR fixture: +- [ ] Build the synthetic-PR fixture (100 .go files across realistic package layout, 4 distinct mechanical-rule violation types): ```bash WORK=$(mktemp -d) && cd "$WORK" && git init -q - mkdir -p pkg/{handler,service,store,worker,internal}/{user,order,product,customer,billing} i=0 - for dir in pkg/handler/{user,order,product,customer,billing} \ - pkg/service/{user,order,product,customer,billing} \ - pkg/store/{user,order,product,customer,billing} \ - pkg/worker/{user,order,product,customer,billing} \ - pkg/internal/{user,order,product,customer,billing}; do - for n in 1 2 3 4; do - i=$((i+1)) - cat > "$dir/file$n.go" < "$dir/file$n.go" + done + done + # Layer B: no-raw-go-func (30 files) + for dir in pkg/w/{a,b,c}; do + mkdir -p $dir + for n in 1 2 3 4 5 6 7 8 9 10; do + i=$((i+1)); pkg=$(basename $dir) + printf 'package %s\nfunc Run%d() {\n\tgo func() { _ = 1 }()\n}\n' "$pkg" $i > "$dir/file$n.go" + done + done + # Layer C: no-globals-or-singletons (15 files) + for dir in pkg/r/{a,b,c}; do + mkdir -p $dir + for n in 1 2 3 4 5; do + i=$((i+1)); pkg=$(basename $dir) + printf 'package %s\ntype Service%d struct{}\nfunc NewService%d() *Service%d { return &Service%d{} }\nvar sharedService%d = NewService%d()\nvar _ = sharedService%d\n' "$pkg" $i $i $i $i $i $i $i > "$dir/file$n.go" + done + done + # Layer D: no-time-now-direct (15 files) + for dir in pkg/i/{a,b,c}; do + mkdir -p $dir + for n in 1 2 3 4 5; do + i=$((i+1)); pkg=$(basename $dir) + printf 'package %s\nimport "time"\nfunc Now%d() time.Time { return time.Now() }\n' "$pkg" $i > "$dir/file$n.go" done done git add . && git commit -qm initial ``` - [ ] `git ls-files '*.go' | wc -l` returns exactly `100` - [ ] `ast-grep --version` resolves on host -- [ ] Pin the LLM-call counter via a Claude CLI shim: - ```bash - CLAUDE_BIN=$(command -v claude) - mkdir -p /tmp/llm-count-shim - cat > /tmp/llm-count-shim/claude <> /tmp/llm-call-count.log - exec "$CLAUDE_BIN" "\$@" - SHIM - chmod +x /tmp/llm-count-shim/claude - : > /tmp/llm-call-count.log - export PATH=/tmp/llm-count-shim:$PATH - ``` - Every `claude` invocation under the shim appends one line; final count is `wc -l < /tmp/llm-call-count.log` -- [ ] Positive control on the shim: `claude --version >/dev/null && [ "$(wc -l < /tmp/llm-call-count.log)" = "1" ]` (after the manual probe, reset with `: > /tmp/llm-call-count.log` before the Action step) +- [ ] Run `make build-index` in the coding repo root before scanning — the Owner-count assertion below reads `rules/index.json` directly, and a stale index (where a new rule's YAML exists but the index hasn't been regenerated) would silently miss new rule_ids during the intersection step ## Action -- [ ] Run `/coding:pr-review master standard` against `$WORK` in a fresh Claude Code session under the shim PATH; tee stdout to `/tmp/scaling-pr-stdout.log`, stderr to `/tmp/scaling-pr-stderr.log`, capture exit code to `/tmp/scaling-pr-exit` -- [ ] Record wall-clock duration with `time` wrapping the slash command; capture the `real` line to `/tmp/scaling-pr-time.log` -- [ ] Extract the Step 5 Consolidated Report section: `awk '/^### Step 5:/{flag=1} flag' /tmp/scaling-pr-stdout.log > /tmp/scaling-pr-report.md` +- [ ] Run the mechanical funnel — `ast-grep scan` from inside the coding project root, against the fixture: `cd ~/Documents/workspaces/coding && start=$(date +%s%N); ast-grep scan "$WORK" > /tmp/scen003-findings.log 2>&1; exit=$?; end=$(date +%s%N); echo "wall_ms=$(( (end - start) / 1000000 ))" > /tmp/scen003-wall; echo "exit=$exit" > /tmp/scen003-exit` +- [ ] Extract distinct rule_ids from findings: `grep -oE 'go-[a-z-]+/[a-z-]+' /tmp/scen003-findings.log | sort -u > /tmp/scen003-rules` +- [ ] Look up distinct Owner agents by intersecting rule_ids with `rules/index.json`: `python3 -c "import json; idx={r['id']:r['owner'] for r in json.load(open('rules/index.json'))}; rules=open('/tmp/scen003-rules').read().splitlines(); owners=set(idx.get(r) for r in rules if r in idx); print('\\n'.join(sorted(owners)))" > /tmp/scen003-owners` +- [ ] Count findings: `grep -c '^error\[' /tmp/scen003-findings.log > /tmp/scen003-findings-count` ## Expected -- [ ] `cat /tmp/scaling-pr-exit` prints `0` -- [ ] `wc -l < /tmp/llm-call-count.log` returns ≤ `30` (proves the funnel: ast-grep mechanical layer carries 100-file scale at constant LLM cost) -- [ ] `/tmp/scaling-pr-time.log` `real` line parses to ≤ `30m00s` (stays under the prod bot's `activeDeadlineSeconds=1800` ceiling — the same one that killed coding#27) -- [ ] `jq '.stats.findings_count' /tmp/scaling-pr-stdout.log` returns > `0` — negative control: the synthetic violations were actually surfaced (if 0, the funnel didn't run and the LLM count is misleadingly low) -- [ ] `grep -oE 'go-[a-z-]+/[a-z-]+' /tmp/scaling-pr-report.md | sort -u | wc -l` returns ≥ `3` — at least 3 distinct rule_ids surfaced (the per-Owner adjudication phase processed findings, didn't drop them) -- [ ] `grep -c 'dropped finding' /tmp/scaling-pr-stderr.log` returns `0` — citation validator dropped nothing; every surfaced finding cites a real rule_id +- [ ] `cat /tmp/scen003-wall` reports `wall_ms` ≤ `30000` (mechanical funnel completes in ≤30 seconds) +- [ ] `wc -l < /tmp/scen003-owners` returns a number ≤ `30` (structural upper bound on Step 4b LLM calls: one Task per owner with findings) +- [ ] `cat /tmp/scen003-findings-count` > `0` — negative control: the synthetic violations were actually surfaced (if 0, the funnel didn't run, owner count is misleadingly low) +- [ ] `wc -l < /tmp/scen003-rules` returns ≥ `3` — at least 3 distinct rule_ids surfaced (proves the fixture exercises multiple mechanical patterns, not a single rule) +- [ ] Every rule_id in `/tmp/scen003-rules` appears as an `id` in `rules/index.json` (citation discipline at funnel exit): `comm -23 /tmp/scen003-rules <(jq -r '.[].id' rules/index.json | sort)` is empty ## Cleanup -- `rm -rf "$WORK" /tmp/scaling-pr-* /tmp/llm-count-shim /tmp/llm-call-count.log` +- `rm -rf "$WORK" /tmp/scen003-*` -After the scenario passes, the operator should record the measured `(LLM count, duration, findings count)` tuple in the Progress section of the task page (`[[Refactor coding pr-review to doc-driven rules pipeline]]`) so future Phase-10 reruns have a baseline. This is a follow-up note, not part of the scenario contract. +After the scenario passes, the operator should record the measured `(wall_ms, distinct_owners, findings_count)` tuple in the Progress section of the task page (`[[Refactor coding pr-review to doc-driven rules pipeline]]`) so future runs have a baseline. This is a follow-up note, not part of the scenario contract.