diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 1fd0bc69487..b67b2c57877 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -9,7 +9,7 @@ { "name": "doperpowers", "description": "Emerges by humans, Converges by Agent", - "version": "7.59.0", + "version": "7.60.0", "source": "./", "author": { "name": "SSFSKIM", diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 7294c3b255d..23df58bb9d7 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "doperpowers", "description": "Emerges by humans, Converges by Agents", - "version": "7.59.0", + "version": "7.60.0", "author": { "name": "SSFSKIM", "email": "supremekim17@gmail.com" diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index bfdec7b573d..b4e220a7b8e 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "doperpowers", - "version": "7.59.0", + "version": "7.60.0", "description": "A two-track software-development methodology for coding agents: a human-gated controlled track (brainstorm, plan, TDD, review, ship) plus an autonomous board loop for unattended, well-scoped work.", "author": { "name": "SSFSKIM", diff --git a/README.md b/README.md index 7292d62e431..d9b66cb62f9 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Because every skill declares when it applies, you don't invoke any of this by ha Both tracks enforce the same non-negotiables — design before code, tests before implementation, evidence before "done." They differ only in where the human sits. **Controlled** — for work where taste and intent matter. -The agent refuses to jump straight to code. It interviews you (`brainstorming`), turns the conversation into a living design spec (`execspec`), breaks that into tasks small enough for an unsupervised junior to follow (`writing-plans`), then executes each one through a fresh subagent with two-stage review — spec compliance, then code quality (`subagent-driven-execution`). You approve the design; independent reviews gate the rest. +The agent refuses to jump straight to code. It interviews you (`brainstorming`), turns the conversation into a living design spec (`execspec`), breaks that into tasks an executor can own from one self-contained brief (`writing-plans`), then executes each one through a fresh subagent, reviewed at dependency frontiers — spec compliance, then code quality (`subagent-driven-execution`). You approve the design; independent reviews gate the rest. **Autonomous** — for work that's already well-scoped. A single self-contained plan (`execplan`) front-loads every decision so the agent can run to the letter without mid-flight questions. At larger scale, the board loop takes over: tickets live as GitHub issues (`issue-tracker`), workers pick them up and build (`executing`), a review loop lands the PRs (`qa-loops`), and durable background sessions keep it all running (`orchestrating-daemons`). Product feedback can even feed the board directly (`triaging-feedback`). @@ -74,7 +74,7 @@ Twenty-three skills, grouped by what they're for. Each one auto-triggers from it **Build it** - `test-driven-development` — RED → GREEN → REFACTOR, no code before a failing test -- `subagent-driven-execution` — one fresh subagent per task, two-stage review +- `subagent-driven-execution` — one fresh subagent per task, reviews at dependency frontiers - `execplan` — the autonomous single-plan track, gates front-loaded - `using-git-worktrees` — isolated workspaces so parallel work never clashes @@ -101,8 +101,8 @@ Twenty-three skills, grouped by what they're for. Each one auto-triggers from it 1. **brainstorming** — Activates before writing code. Refines rough ideas through questions, explores alternatives, presents the design in sections short enough to actually read. 2. **using-git-worktrees** — Activates after design approval. Creates an isolated workspace on a new branch and verifies a clean test baseline. -3. **writing-plans** — Breaks the approved design into tasks of a few minutes each, every one with exact file paths, complete code, and verification steps. -4. **subagent-driven-execution** — Dispatches a fresh subagent per task with two-stage review. +3. **writing-plans** — Breaks the approved design into tasks an executor can own from one brief, every one with exact file paths, complete code, and verification steps. +4. **subagent-driven-execution** — Dispatches a fresh subagent per task, reviews at dependency frontiers, and fixes by resuming the executor. 5. **test-driven-development** — Enforces the RED-GREEN-REFACTOR cycle throughout and deletes any code written before its test. 6. **verification-before-completion** — Before anything is called done, runs the check and shows the output; evidence, not assertions. 7. **finishing-a-development-branch** — Verifies tests, presents merge/PR/keep/discard, cleans up the worktree. diff --git a/docs/doperpowers/specs/2026-08-21-task-grain-and-review-cadence-design.md b/docs/doperpowers/specs/2026-08-21-task-grain-and-review-cadence-design.md new file mode 100644 index 00000000000..f399115f11e --- /dev/null +++ b/docs/doperpowers/specs/2026-08-21-task-grain-and-review-cadence-design.md @@ -0,0 +1,693 @@ +# Task Grain and Review Cadence — Design + +The controlled track (doperpowers:writing-plans → doperpowers:subagent-driven- +execution) pays a fixed cost at every task boundary: a fresh executor orients +in the codebase, a fresh reviewer orients in the diff, any fixer orients +again, and the controller composes a dispatch, reads a report, adjudicates +⚠️ items, and writes the ledger. writing-plans' standing rule — "a task is the +smallest unit that carries its own test cycle and is worth a fresh reviewer's +gate" — maximizes the number of those boundaries. Telemetry over the last four +real runs (§3) shows where the time actually goes: reviewer time often equals +or exceeds executor time, fixer dispatches are of the same order as +executor dispatches — each a fresh orientation — and practice has already +drifted to fewer, larger tasks than the rule describes. This +design moves the doctrine to where practice is heading and removes two +structural repetitions of orientation — per-task review when nothing +downstream needs it yet, and fresh fixers where the executor that wrote the +code could simply continue. + +After this lands one can observe: writing-plans sizes tasks by ownability and +interface frontiers, not minimality; subagent-driven-execution reviews at +dependency frontiers (batching sibling tasks, reviewers concurrent) and fixes +by resuming the executor; `scripts/sde-telemetry` prints per-role dispatch +counts, durations, and tokens for any session; and this spec carries the +four-run baseline and the monitoring protocol that decides whether the change +stands. + +Human-confirmed frame (2026-08-21): scope = grain + cadence + fix-loop +resume; wave boundary = dependency frontier; per-task review depth unchanged +(reviewer's judgment); evidence path = adopt now, monitor the next features +with the collector; track = direct, with this spec as the durable record. +Plan resolution (No Placeholders, complete code in every step) and +single-executor sequential execution are untouched. + +On the direct track this spec is the only record of the wording: §1 and §2 +quote the skill-bound text verbatim; commentary sits outside the quotes. + +## §1 Task right-sizing (doperpowers:writing-plans) + +The "Task Right-Sizing" section is replaced by the following text (the +human's wording; criteria, no calibration numbers — the reading model sizes +by judgment): + +> A task is the unit one executor can reliably own from a single +> self-contained brief, but it isn't the smallest unit a reviewer could +> gate. Every task boundary costs a fresh worker's orientation (executor, +> reviewer, and any fixer each read in from zero) plus your own +> dispatch-and-adjudicate turn; draw the fewest boundaries that keep these +> true: +> +> - **Interface frontiers.** Anything a later task consumes must be +> produced — and reviewed — before that task dispatches: keep producer +> and consumer in one task, or put the contract at a task edge. +> Interfaces internal to a task are free. +> - **Reviewable diff.** One reviewer reads one task's diff in a single +> pass. +> - **Ownability.** One task holds one coherent verification strategy and +> closely related state owners; its brief needs nothing from neighbors +> beyond the declared Interfaces. +> +> Fold setup, configuration, scaffolding, and documentation into the task +> whose deliverable needs them. Inside a task, organize the work as +> sequential deliverables, each with its full step sequence and its own +> commit. + +Supporting edits in the same skill, each quoted as it lands: + +- Task Structure template, `Consumes:` line — the controller's source for + the dependency frontier in §2 (the plan declares dependencies; the + controller derives the review schedule at run time): + + > - Consumes: [what this task uses from earlier tasks, naming the task — + > "from Task 2: `walk(cursor) -> list[dict]`" — exact signatures. The + > controller schedules reviews from these: a producer is reviewed before + > its consumer dispatches.] + +- The section "Bite-Sized Task Granularity" is retitled "Bite-Sized Steps" + — its content is about steps (2–5 minutes each), and "task granularity" + now means the opposite of bite-sized. The overview's "bite-sized tasks" + becomes "tasks built from bite-sized steps". Step content is unchanged. +- The Execution Handoff line "Fresh subagent per task + two-stage review" + becomes "Fresh executor per task; reviews at dependency frontiers; fixes + resume the executor". + +Unchanged, deliberately: Scope Check, Conditional Sub-Slicing (its "fewest +boundaries that make each important invariant independently understandable +and testable" already points the same way), File Structure, No Placeholders, +Spike Tasks, Final Verification Task, Self-Review. + +## §2 Review cadence and fix loop (doperpowers:subagent-driven-execution) + +The loop's steps 3–5 become: + +> 3. **Per task, in plan order:** extract the brief (`scripts/task-brief +> PLAN_FILE N`), record BASE (the current commit), dispatch the executor +> ([executor-prompt.md](executor-prompt.md)) and write the task's +> `executed` ledger line with its agent handle (Durable Progress below) +> — fixes resume it. Answer its questions before it proceeds. One +> executor at a time — parallel executors conflict in a shared worktree. +> 4. **Review at the frontier:** a task is reviewed clean — findings fixed +> and re-reviewed — before any task that consumes what it produced +> dispatches; the briefs' Interfaces name the producers, and the plan's +> final verification task consumes the whole branch. Tasks nothing +> downstream consumes yet may keep executing and are reviewed together +> when the frontier closes (the next task consumes from them, or the +> plan ends): one review package per task (`scripts/review-package +> PLAN_FILE BASE HEAD`, each task's own BASE..HEAD), one task reviewer +> per task ([task-reviewer-prompt.md](task-reviewer-prompt.md)) with the +> printed path, dispatched together when their focused tests cannot +> collide — reviews read their package, not the tree, so hermetic suites +> run concurrently; suites that share mutable state — a test database, a +> fixed port — run one reviewer at a time. +> Where no interface is declared but two tasks touch the same files, +> judge from their Files lists: an overlap that looks load-bearing is +> reviewed before the later task dispatches. The frontier is the ceiling +> on deferral, not the floor: a DONE_WITH_CONCERNS, a doubt of your own, +> or a first task whose brief style the executor may have misread are +> reasons to review that task now. +> A deferred review reads a tree that has moved past its package — +> Task 1's package is BASE1..HEAD1 while the checkout sits at the +> wave's last HEAD. Name the current HEAD and what landed since the +> task's own HEAD (the sibling commits and files) in the dispatch, so +> a sibling's effect is not read as this task's; a check that must see +> the task's own tree — a focused test, a named risk — runs in a +> detached worktree at the packaged HEAD (`git worktree add --detach +> /review-N `, removed after the review) rather +> than in the shared checkout. +> 5. **Findings:** Critical/Important findings go back to the executor that +> wrote the code — resume it with the findings; it holds the task's +> context and skips the orientation a fresh fixer pays. Several tasks +> with findings in one wave resume one at a time (shared worktree). +> Re-review by resuming the reviewer with the fix commits' package +> (`scripts/review-package PLAN_FILE FIX_BASE FIX_HEAD` — the fix range: +> the reviewer already holds the task's original package, and a deferred +> task's fix lands past its siblings' commits); repeat until both +> verdicts are clean. That message names the fix range, sends the +> reviewer back to the report (the executor appended the fix's test +> evidence there), and carries the refreshed checkout head and what +> landed since — plus a fresh detached worktree at the fix head if the +> review needs the task's own tree. A resumed reviewer otherwise judges +> the fix against its pre-fix memory. A fresh fixer when the executor +> cannot be resumed, or when its frame is the problem — two failed +> re-reviews is the usual sign. Record Minor findings in the ledger — +> the final review triages that list, so it is read, not discarded. Fix +> through a worker, not your own edits: manual fixes pollute your +> context and skip review. + +The skill's opening sentence says "a task review (spec compliance + code +quality) at each dependency frontier". Worked schedules (commentary, not +skill text): the PR #74 plan — helper primitives; four sibling migrations +consuming them; legacy removal consuming all four; final verification — gives +{1} | {2, 3, 4, 5} | {6} | {7}: four review points instead of seven, the +middle four reviewers concurrent; Tasks 3 and 4 share `test-read-verbs.sh` +without a declared interface, which is exactly the Files-list judgment call +the rule names. The PR #76 plan is a chain — Task 3 consumes Task 2's test +scaffolding, Task 4 consumes both — so its schedule is per-task: the rule +reproduces today's cadence where the plan gives it no siblings. + +**Model selection**, first paragraph: + +> Dispatch workers — executors, task reviewers, fixers — on opus at high +> reasoning effort; the task grain is calibrated to that tier. A simple +> task — a doc update, a mechanical rename, a verification walk with every +> command given — can go to sonnet. Never dispatch workers on the top tier +> (fable): it adds cost without adding reliability and is the controller's +> tier, not the worker's — the plan and the brief absorb the difficulty, +> not the model. When a worker reports BLOCKED on reasoning capacity rather +> than missing context, a sonnet task moves to opus; from opus there is no +> tier above — the difficulty moves into the brief: resolve the hard call +> yourself and re-dispatch, or split the task. + +The final-whole-branch-review paragraph and "name the model in every +dispatch" are unchanged. Executor statuses, DONE — the old "review package +→ task reviewer" contradicted step 4's deferral: "**DONE** → review at the +frontier (step 4): package and reviewer now when something downstream +consumes the task or an early-review reason applies, otherwise it waits for +the wave." BLOCKED: "reasoning capacity (sonnet → opus; from opus, +resolve the hard call in the brief)". The two +prompt templates' `[MODEL]` placeholders read "opus at high reasoning effort +per SKILL.md Model Selection (sonnet for a simple task; never the top tier)"; +executor-prompt.md's escalation sentence becomes "The controller can provide +more context, resolve the hard call in your brief, move the task to a +stronger worker tier where one exists, or break the task into smaller +pieces." + +**task-reviewer-prompt.md**, Diff Under Review: `[HEAD_SHA]` is documented +as "the task's last commit — the package's head", and an optional line +follows **Head:** in the block — + +> **Checkout:** [CHECKOUT_SHA] — the shared tree sits here, past this +> task's head; landed since: [SINCE] + +— filled for a deferred review with the sibling commits and files that +landed since the packaged head, and omitted when the checkout is at +`[HEAD_SHA]`. The fallback `git diff [BASE_SHA]..[HEAD_SHA]` stays: it is +task-scoped. One added sentence tells the reviewer where a check that must +see the task's own tree runs, keyed to the observable: in the detached +worktree the controller names (step 4) when the Checkout line is present — +the checkout has moved on — and in this checkout when it is absent, since +the checkout is then at the task's head. + +**Dispatch hygiene**, the fix-message bullet: + +> - Fix messages — to a resumed executor or a fresh fixer — carry the +> executor contract: re-run the covering tests (name them — a one-line +> fix doesn't need the whole suite), report the command and output; +> confirm all three are in the fix report before re-review. A resumed +> executor's view of the tree ends at its own HEAD: name what landed +> since (commits and files) and have it re-read before editing; its +> covering tests include sibling suites touching the same files. + +Two more hygiene bullets change — what a deferred review's dispatch carries, +and the range a re-review's package spans: + +> - The task reviewer gets three paths — brief, report, review package — plus +> the plan's binding constraints copied verbatim (exact values, formats, +> stated relationships) and, for a deferred review, the checkout head and +> what landed since (the template's Checkout line). Its template already +> carries the process rules. +> - `review-package` BASE is the commit you recorded before dispatching the +> executor — never `HEAD~1`, which silently drops all but the last +> commit of a multi-commit task. A re-review's BASE is the ledger's +> `fix-base` — the HEAD when the fix dispatched — so the package is the +> fix alone. + +**Durable progress** — with frontier review, several tasks can be executed +but unreviewed at once, and the old resume rule ("resume at the first task +without a `complete` line") would re-execute them after compaction — the +skill's own named most-expensive failure. The ledger bullets become: + +> - The ledger lives at `/progress.md`, first line +> `# SDE ledger — plan: `. If that line names your plan, +> tasks with a `Task : complete` line are done; a task with an +> `executed` line but no `complete` line is awaiting review or fixes — +> resume its review (or its handles), never re-execute it; resume +> executing at the first task with neither. A ledger naming a different +> plan file is another plan's progress: leave it, start your own. +> - At dispatch, append `Task N: executed (base , executor +> )`; add `head ` when the executor returns and +> `reviewer ` when the review dispatches — a fix resumes those +> handles, and after compaction the ledger is the only place they +> survive. When a fix dispatches append `fix-base ` (the HEAD at +> that moment) and `fix-head ` when it lands: a deferred task's +> fix commits sit past its siblings', so `base..head` no longer bounds +> the task's history. +> - When a task's review comes back clean, append `Task N: complete +> (commits ..[, fix ..], review clean)`. + +Unchanged: pre-flight; ⚠️ resolution by the controller; the remaining +dispatch hygiene bullets (including the final-review fix wave's single +fixer); the reviewer prompt's rubric, tests rule, and output format; the +final whole-branch review; Integration. + +## §3 Telemetry, baseline, and the monitoring protocol + +`scripts/sde-telemetry SESSION_JSONL [...]` (recovered from the closed PR +#72 branch, with the dispatch list sorted by start time and carrying the +model) prints, per session: wall-clock span, dispatch counts by role +(executor / task-reviewer / fixer / other, classified by dispatch +description), token totals by model for controller and workers, and the +dispatch list. Subagent transcripts are read from +`//subagents/`. + +**Baseline** — the four controlled-track runs of 2026-08-18..20 (session +`e92c7422`, all workers opus; times are summed dispatch spans, with active time — idle gaps over 15 minutes excluded, the measure monitoring rows compare against — in parentheses; run B's +reviewer time includes one review inflated to 3h47m by the 2026-08-18 opus +incident): + +| Run | Tasks | Dispatches | Executor n / span (active) | Reviewer n / span (active) | Fixer n / span (active) | Tasks needing a fix | Span (first executor → last task-level dispatch) | +|---|---|---|---|---|---|---|---| +| A arkho#11 read-surface | 11 | 37 | 11 / 1h49m (1h49m active) | 10 / 3h39m (3h15m active) | 14 / 2h05m (2h05m active) | 9 of 10 | 9h50m (incl. PR-review fix waves) | +| B dp PR #74 paged-reads | 7 | 22 | 8 / 6h00m (3h29m active) | 7 / 5h08m (2h04m active)* | 5 / 1h14m (1h14m active) | 2 of 7 | 8h13m | +| C arkho#17 search | 6 | 17 | 6 / 2h02m (2h02m active) | 7 / 0h38m (0h38m active) | 2 / 0h30m (0h30m active) | 2 of 6 | 3h34m | +| D dp PR #76 client reads | 5 | 14 | 5 / 0h44m (0h44m active) | 5 / 0h42m (0h42m active) | 2 / 0h11m (0h11m active) | 1 of 5 | 3h59m (1h33m to last task) | + +The collector reports both span and active time per dispatch and per role +— active excludes idle gaps over 15 minutes, so a resumed agent's waiting +does not count — and deduplicates token totals by message id; the table +carries both measures, active in parentheses. + +Fix rounds per task never exceeded two in the baseline (run A's tasks 2 and +8 took two fixers each; every other fixed task took one). Escaped defects at +the final whole-branch gate, where codex's P1 maps to Critical and P2 to +Important: B — four P2 over four codex rounds (all in the walk contract's +fail-closed edges, i.e. cross-task); D — none Critical/Important (two +Minors); C — none (two API.md polish clauses). Per-task reviews in B caught +two Important findings, both vacuous-assertion / undrilled-branch classes. + +**Protocol.** After each of the next three controlled-track features +finishes, run the collector over that session and append a row here (under +Surprises & Discoveries) with: tasks, dispatches by role, executor / reviewer +/ fixer time, tasks needing a fix, fix rounds per task, final-review +Critical+Important count, any executor BLOCKED citing task size. Reopen this +decision when any of these holds: two consecutive monitored features show +final-review Critical+Important above the baseline's worst (four); an +executor is BLOCKED citing task size; a task needs more than two fix rounds +(above the baseline maximum — the fresh-fixer fallback firing is itself the +signal). The instruction to run the collector lives here and in memory, not +in skill text — it is the monitoring window's, not the method's. + +## §4 Out of scope (recorded so they are not re-derived) + +- **Parallel executors** for independent tasks (worktree-isolated dispatch + off the Interfaces DAG). The most direct wall-clock lever, but tasks in + one plan routinely touch the same files (PR #74's Task-3→4 handoff on + `test-read-verbs.sh`), so it needs its own merge design. Separate goal. +- **Plan resolution.** Opus executors could take brief-level plans, but the + complete-code plan is the controlled track's stated advantage (frontier + intelligence spent once, up front) and changing it with the grain would + make any outcome unattributable. +- **Reviewer depth.** Reviewers' mutation batteries are expensive and have + caught real vacuous-assertion classes; depth stays the reviewer's call. + +## Acceptance + +1. `skills/writing-plans/SKILL.md` carries the §1 text under "Task + Right-Sizing"; `grep -c "smallest unit that carries its own test cycle" + skills/writing-plans/SKILL.md` prints `0` and `grep -c "isn't the + smallest unit" skills/writing-plans/SKILL.md` prints `1`; the Task + Structure `Consumes:` line names the producing task. +2. `skills/subagent-driven-execution/SKILL.md` carries the §2 text (loop + steps 3–5, model selection, the DONE status line, the fix-message and + two changed hygiene bullets, the three ledger bullets); + task-reviewer-prompt.md carries the Checkout line and its placeholder + docs; both prompt templates' `[MODEL]` placeholders and + executor-prompt.md's escalation sentence read as §2 states. +3. Wording smoke check (doperpowers:writing-skills micro-test, three reps — + a smoke check on the wording's binding effect, not the eval; the eval is + §3's monitoring): a fresh opus subagent given the new writing-plans + skill and the PR #74 spec + (`docs/doperpowers/specs/2026-08-18-board-client-paged-reads-design.md`) + against a checkout at the spec's fork point (64967f79), asked only for + the task breakdown with Interfaces, returns a breakdown in which every + `Consumes` from an earlier task names its producer, in all three reps; + the reps' median task count is no higher than a control rep on the old + text (same spec, same checkout). +4. A fresh opus subagent given the new subagent-driven-execution skill and + the PR #74 plan (`docs/doperpowers/plans/2026-08-18-board-client-paged- + reads.md`), asked for its review schedule before dispatching anything, + answers a frontier schedule — Task 1 alone, Task 7 alone, the four + migrations batched ({2,3,4,5}) or split only at the Task 3→4 + `test-read-verbs.sh` overlap it names as load-bearing ({2,3} | {4,5}) — + with concurrent reviewers inside each multi-task set, in at least two of + three reps; inferring the producers from the briefs where that plan's + older Interfaces blocks do not name them (the inference is intended; the + old skill text reviews after every task by construction, so no control + rep is needed). Asked what it appends to the ledger at dispatch and what + it does after compaction with `executed`-but-not-`complete` tasks, it + writes the `executed` line and does not re-execute them. +5. `scripts/sde-telemetry ~/.claude/projects//e92c7422-….jsonl` prints + `dispatches: N total` with N ≥ 277 (the session is still live), the + four role counts summing to N, and a dispatch list in start-time order. +6. `tests/claude-code/run-skill-tests.sh --test + test-subagent-driven-execution.sh` passes with `claude` shimmed to + `--plugin-dir ` so the keyword test reads the edited + skill, not the installed plugin; `scripts/bump-version.sh --check` + reports the new version in sync. + +## Decision Log + +- Decision: adopt the doctrine change now and monitor, instead of the + pre-registered paired double-run (PR #72, closed 2026-08-20 unmerged). + Rationale: the human chose this path; double-implementing a feature is the + costliest possible evidence; the four-run telemetry is a real baseline for + the same metrics; the change is a text revert away; the §3 protocol + reopens it on regression. This spec supersedes + `2026-08-19-task-grain-experiment.md` (never merged), whose V1 treatment + text seeded §1 and whose V2 (wave cadence) became §2. + Date/Author: 2026-08-21 / fable session + human + +- Decision: wave boundary = dependency frontier, over fixed batches (every + 2–3 tasks) and over end-only review. + Rationale: fixed batches let a consumer build on an unreviewed interface + (rework if the review fails); end-only moves the vacuous-assertion class + per-task reviews have caught to the end, where fixes are largest. The + frontier is the one boundary the plan already declares. + Date/Author: 2026-08-21 / human + +- Decision: undeclared dependencies — shared files without an interface — + are the controller's call from the Files lists, reviewing the earlier + task first when the overlap looks load-bearing; the ceiling clause + ("review that task now") covers the rest. Residual exposure accepted: a + sibling fix landing after a later sibling executed on the same file. + Rationale: the independent review showed both worked examples carry this + case (PR #74 Tasks 3→4; PR #76 Tasks 2→3); in the schedule micro-test, + reps flagged the PR #74 overlap unprompted and resolved it the same way. + Date/Author: 2026-08-21 / fable session (review finding adopted) + +- Decision: per-task review depth unchanged — the reviewer judges how deep + to go. + Rationale: mutation batteries are where the reviewer time goes, and also + where PR #74's two Important findings came from; "lighter per-task, + heavier final" was considered and declined for now — revisit with + monitoring data. + Date/Author: 2026-08-21 / human + +- Decision: fix by resuming the executor by default; a fresh fixer when + resume is impossible or the executor's frame is the problem (two failed + re-reviews the usual sign) — phrased as a default with its reason, not a + gate. + Rationale: fixer dispatches are of the same order as executors (70 to 92 + over the session; 13 to 11 in run A) and every one re-orients; the + executor holds the context; the reviewer's re-review — not the fixer's + freshness — is the independence guard. Upstream's SDD resumes the + implementer for rounds ≤ 3 (prior art). The resume message names what + landed since the executor's HEAD, because its context predates its + siblings' commits. + Date/Author: 2026-08-21 / fable session + +- Decision: the ledger gains an `executed` line at dispatch (base, executor + handle; head and reviewer handle added as they arrive), and the resume + rule distinguishes executed-awaiting-review from not-started. + Rationale: frontier review leaves several tasks executed-but-unreviewed; + the old rule would re-execute them after compaction — the failure the + section exists to prevent. Handles in the ledger are what make resume + survive compaction at all. + Date/Author: 2026-08-21 / fable session (review finding adopted) + +- Decision: the right-sizing text carries criteria only — no diff-line cap, + no target task count. + Rationale: the human's call: the reading model sizes by judgment; numbers + become gates. The author's expectations (roughly ≤ 500 changed lines per + task; a feature the old rule cut into 8–12 tasks landing around 3–6) are + recorded here for the monitoring readout, not in the skill. The smoke + check (Surprises) read out 6–7 tasks against a control of 8 on the same + spec — a modest shift, consistent with judgment rather than a cap. + Date/Author: 2026-08-21 / human + +- Decision: worker model text = opus/high default, sonnet for simple tasks, + never fable; reasoning-capacity BLOCKED escalates sonnet → opus, and from + opus into the brief or a split, not up the tier. + Rationale: the skill still said sonnet while every session overrode it to + opus by standing directive (2026-08-04, corrected 2026-08-19: fable + workers are not an acceptable fallback even as an upgrade); the grain is + calibrated to the worker tier, so the text must name it. With opus the + ceiling, "stronger model" had nowhere to go. + Date/Author: 2026-08-21 / human + +- Decision: the plan declares dependencies (Consumes names the producer); + the controller derives waves at run time — over plan-declared waves. + Rationale: run-time signals (DONE_WITH_CONCERNS, a controller doubt) can + close a wave early; the plan author cannot foresee them. + Date/Author: 2026-08-21 / fable session + +- Decision: one reviewer per task within a wave, concurrent — over one + reviewer for the whole wave diff. + Rationale: per-task packages already exist and keep each review within + one pass; cross-task interplay is the final whole-branch review's job + today and stays there. Reviews read the package, not the tree, so + concurrency in a shared worktree is safe as long as their focused tests + are hermetic; suites that share mutable state — a test database, a fixed + port — run one reviewer at a time. + Date/Author: 2026-08-21 / fable session + +- Decision: parallel executors and plan resolution out of scope (§4). + Rationale: separate designs; bundling them would make the grain outcome + unattributable. + Date/Author: 2026-08-21 / human + +- Decision: direct track with a spec (no plan, no SDE), telemetry + instruction in spec + memory rather than skill text; the spec quotes the + skill-bound text verbatim. + Rationale: ~150 lines of doctrine prose — a plan would be the diff + itself; the spec is needed as the durable home of the baseline and the + monitoring protocol; on a direct track it is also the only record of the + wording; the collector instruction is the monitoring window's. + Date/Author: 2026-08-21 / human + +- Decision: a deferred review's dispatch names the current HEAD and what + landed since the package's HEAD, and a check that must see the task's + own tree runs in a detached worktree at the packaged HEAD — the + conditional default, not every deferred review. + Rationale: once review is deferred to the frontier the shared checkout + sits at the wave's last HEAD, so a focused test or named-risk check + outside the package's diff observes sibling changes and can hide a + failure or manufacture one. Always-detaching was declined: a fresh + worktree may need its dependencies installed before anything runs — a + real per-review cost — and most deferred reviews read the package + rather than run something outside it. + Date/Author: 2026-08-21 / fable session (codex finding adopted) + +- Decision: fix loops record their own commit range (fix-base/fix-head) + and re-review packages cover only that range. + Rationale: a deferred task's fix lands after sibling commits; the + original base..head no longer bounds it, and a package from the original + base would drag siblings into a task-scoped re-review. + Date/Author: 2026-08-21 / fable session (codex finding adopted) + +- Decision: declined — codex round 3 asked for adversarial before/after + pressure evals (a real run exercising deferred review, fixes, and + compaction) before release. + Rationale: the human chose adopt-now-and-monitor as the evidence path + (first Decision Log entry); the monitored next features are exactly that + before/after run on real work, with reopen criteria pre-stated in §3; the + smoke checks cover the wording's binding effect. Recorded so the + disagreement is visible, not relitigated. + Date/Author: 2026-08-21 / fable session + +## Surprises & Discoveries + +- Observation: across the four runs reviewer time equals or exceeds executor + time in two (A: 3h39m vs 1h50m; D: 42m vs 45m) — reviewers run mutation + batteries, 20–40 minutes and 26–77k output tokens per review in run A. + Evidence: `scripts/sde-telemetry` + per-dispatch durations, session + `e92c7422`, computed 2026-08-21. +- Observation: fixer dispatches are of the same order as executors, not + more numerous as the pre-v2 classification read — session-wide 70 fixers + to 92 executors; within run A's task window (2026-08-18 01:30–08:30 UTC) + 13 to 11, where they do outnumber them. Each fixer is a fresh dispatch + paying orientation again. + Evidence: same, recomputed 2026-08-21 after the collector learned the + task-prefixed description forms (`T1 implementer: …`). +- Observation: practice had drifted coarse before the doctrine moved — July + plans carried 8–17 tasks, the four August runs 11/7/6/5; the coarsest + (PR #76, 300- and 276-line briefs) was the fastest to last task (1h33m) + with the lowest fix rate (1 of 5). n=1, directional. + Evidence: `docs/doperpowers/plans/*.md` task counts; telemetry. +- Observation: in PR #74 the per-task gate's catches were test-quality + classes (vacuous drill, undrilled branch); the four correctness holes were + cross-task walk-contract edges, all found by the final codex review. The + per-task gate's unique value is early, local; correctness across tasks is + the final gate's. + Evidence: `.doperpowers/sde/2026-08-18-board-client-paged-reads/progress.md`. +- Observation: the controller's own output (2.9M tokens, fable) is about a + third of all workers combined (9.0M) over the session, and larger than any + single worker role except executors (3.5M) — the "frontier plans once, + cheap executes" picture understates the controller: every task costs + controller turns, so fewer tasks cut controller cost too. + Evidence: telemetry token totals, deduplicated by message id; the earlier + 7.0M / 8.8M reading double-counted multi-row assistant messages, which + inflated the fable controller far more than the workers. +- Observation: PR #72's recommended feature (arkho#9 paged envelope) had + already shipped in arkho PR #11 on 2026-08-18, the day before the spec was + written — the experiment's target was stale at birth. + Evidence: memory `read-surface-epic-shipped`; arkho#11 merge 591ba00. +- Observation (acceptance 3 smoke check, 2026-08-21): three opus planners on + the new writing-plans text cut the PR #74 spec into 7, 6, 6 tasks (median + 6); every `Consumes` from an earlier task named its producer in all + three. The control rep on the old text cut it into 8 — splitting the + primitives into a by-id task and a walk task and giving drill-lib its own + task, the "smallest unit" rule in action. The shipped plan, written by a + frontier session under the old text, had 7. The shift is modest and in + the expected direction; the wording's firmly binding effect is structural + (producer-naming Consumes). + Evidence: four subagent transcripts, this session. +- Observation (acceptance 4, 2026-08-21): three opus controllers on the new + subagent-driven-execution text all answered {1} | {2,3,4,5} | {6} | {7} + for the PR #74 plan, each describing serial executors, per-task BASE, + concurrent reviewers, and resume-based fixes; two of three flagged the + Task 3/4 shared-file overlap unprompted as a reason to review Task 3 + early if load-bearing. Inference was needed — that plan's Interfaces + blocks predate producer naming. + Evidence: three subagent transcripts, this session. +- Observation (acceptance 4 re-run on the v1.1 text, 2026-08-21): two more + opus controllers answered {1} | {2,3} | {4,5} | {6} | {7} and + {1} | {2,3,4,5} | {6} | {7} respectively — the new undeclared-overlap + clause tipped one rep to review Task 3 before Task 4 (matching the real + run's ledger, which recorded that handoff as MUST-carry) and left the + other batching with the promotion rule held ready. Both wrote the + `executed` ledger line with base and executor handle, refused to + re-execute executed-but-unreviewed tasks after a simulated compaction, + and resumed Task 3's executor with the list of what landed since its + HEAD. Five of five reps across both texts produced a frontier schedule + with at least one multi-task set and concurrent reviewers — the old text + cannot produce that. + Evidence: two subagent transcripts, this session. +- Observation (historical-spec before/after, 2026-08-21, human-proposed): + the eight controlled-track specs that shipped between 2026-08-03 and + 2026-08-20 (six dp, two arkho) were re-cut from their pre-execution spec + text at their fork-point checkouts — two opus planners on the new + writing-plans text and one opus planner on the old text per spec (24 + breakdowns, Interfaces only, no plan bodies; plans directories hidden; + the shipped plan and its real run are the "before" evidence). Totals: + shipped plans 72 tasks; same-model controls on the old text 81; new-text + medians 49.5 (−31% vs shipped, −39% vs the same-model control); review + points under the frontier rule derived from the new breakdowns' + `Consumes` 37 against 72 per-task reviews (−49%), with concurrent + reviewers inside each multi-task set. Every new-text breakdown named its + producer in every earlier-task `Consumes` (16 of 16); old-text controls + cut finer than the frontier-authored shipped plans in five of eight + specs. Spread between new-text reps is up to three tasks (#43: 6 vs 9) — + the wording leaves the judgment room it was meant to. Alignment with the + runs' pain: the two runs whose fix loops dominated (#50 — 19 fixers for + 14 tasks, 13 fixed, a five-round final loop; #43 — every task fixed, + fixer active time twice the executors') are exactly where resume-based + fixes remove re-orientation, and the run where per-task review was + mostly idle (#65 — 9 reviews, 1 fix) loses the most review points + (−68%); the chain-shaped specs (arkho#11, arkho#17, #76) gain from grain + only, their frontier schedule staying per-task. PR #74's four cross-task + final-review defects sat inside the primitives task in every cut, old + and new — grain does not move that class, the final gate does. This is + before/after on proposals, not executed runs: no quality measurement of + the new cuts exists yet; that remains the §3 monitoring's job. + Evidence: 24 subagent transcripts; the table below, this session. + + | Spec (real run) | Shipped plan tasks | Real run: dispatches exec/rev/fix · tasks needing fix · final-gate | Control (old text, opus) | New text (opus) reps | New frontier review points | Δ tasks new-median vs shipped | Δ review points vs shipped per-task | + |---|---|---|---|---|---|---|---| + | #43 codex-workflow-engine (08-03..05) | 8 | 37: 8/8/11 · every task (retro) · 4 P1 + 2 P2; dogfood 17 in 3 waves | 13 | 6, 9 (median 7.5) | 5, 6 | -6% | -31% | + | #50 a2-board-adapter (08-09..10) | 14 | 50: 14/14/19 · 13 of 14 · 5 rounds: 33→8→3→3→1 confirmed | 14 | 9, 8 (median 8.5) | 8, 6 | -39% | -50% | + | #61 dp51-a1-consumption (08-11) | 10 | 63: 22/21/16 · 10 task-ids fixed · PR-review flow-back (api-scale) | 9 | 8, 8 (median 8.0) | 5, 5 | -20% | -50% | + | #65 dp51-deferrals-dp60 (08-12..13) | 11 | 28: 10/9/4 · 1 of 11 (+final panel) · final panel findings, 1 wave | 12 | 6, 6 (median 6.0) | 4, 3 | -45% | -68% | + | #74 board-client-paged-reads (08-18) | 7 | 22: 8/7/5 · 2 of 7 · 4 P2 over 4 rounds | 10 | 5, 6 (median 5.5) | 3, 4 | -21% | -50% | + | #76 client-agent-grade-reads (08-20) | 5 | 14: 5/5/2 · 1 of 5 · 0 Critical/Important | 7 | 3, 4 (median 3.5) | 3, 4 | -30% | -30% | + | arkho#11 read-surface (08-18) | 11 | 37: 11/10/14 · 9 of 10 · 3 codex + 3 PR-level | 11 | 6, 7 (median 6.5) | 6, 4 | -41% | -55% | + | arkho#17 agent-grade-reads (08-19) | 6 | 17: 6/7/2 · 2 of 6 · 0 Critical/Important | 5 | 4, 4 (median 4.0) | 4, 4 | -33% | -33% | +- Observation: `claude -p --plugin-dir ` loads the worktree's + skill text over the installed plugin (probe quoted both new sentences + verbatim), so the keyword tests can exercise an unreleased edit through a + one-line `claude` shim. + Evidence: `plugin-dir-probe.txt`, this session. + +## Outcomes & Retrospective + +Shipped 2026-08-21 as v7.60.0 on `task-grain-cadence` (7.59.0 was taken by PR +#80 while the branch flew; direct track with this +spec; no plan, no SDE run). Everything the purpose named is in place: +writing-plans sizes tasks by ownability and interface frontiers in the +human's wording, with `Consumes` naming its producer; subagent-driven- +execution reviews clean at dependency frontiers, batches siblings with +concurrent reviewers when their tests are hermetic, resumes the executor +for fixes and the reviewer for re-reviews over a fix-range package, and +keeps an `executed`/fix-range ledger that survives compaction without +re-executing; the worker-model text matches the standing directive; and +`scripts/sde-telemetry` reports dispatches by role with span, active time, +and message-id-deduplicated tokens. + +Review shape: one independent spec review (fable, 15 findings, all but the +relitigation of settled human calls adopted in v1.1) and five codex native +rounds over the branch (3 → 2 → 4 → 1 → clean), each fixed by a single +fix-wave subagent. Every codex finding was a real seam the first drafts +left open — concurrent reviewers racing shared test state, the telemetry's +double-counted usage rows and idle-inflated spans, a deferred review's +moved-on checkout, fix commits breaking a task's contiguous range, the DONE +status row contradicting deferral, a resumed reviewer's stale report. One +codex finding was declined on the human's evidence decision (before/after +pressure evals before release) and is recorded as such. + +Evidence collected: the four-run baseline; the wording smoke checks +(7/6/6 tasks vs 8 on the old text; producer-naming Consumes 3/3; frontier +schedules with batched siblings 5/5; ledger `executed` lines and no +re-execution after simulated compaction); the SDE keyword test green +against the worktree skill through a `--plugin-dir` shim. + +Lessons: the first telemetry read overstated the controller's share by +2.5× (duplicate `message.id` rows) — verify a collector's arithmetic on +one transcript before quoting its totals; a direct-track spec that quotes +the skill text verbatim made five review rounds cheap to apply and check +(byte-identical quote checks); step 4 of the SDE loop is dense after those +rounds — each clause closes a validated gap, but a wording-diet pass is the +natural follow-up once monitoring shows which clauses bind. + +Residue: the §3 monitoring duty (next three controlled-track features); +the version collision with PR #80 resolved by rebumping to 7.60.0 after +merging origin/main; §4's deferred levers (parallel +executors, plan resolution, reviewer depth). + +## Revision Notes + +- v1 (2026-08-21): initial design, approved in session (direct track with + spec); §1 text is the human's wording. +- v1.1 (2026-08-21): independent review (fable) adopted — acceptance 1's + grep was self-defeating (new text contains "smallest unit"); acceptance 4 + dropped the "stricter schedule" clause that admitted per-task behavior + and states that inference from older Interfaces is intended; acceptance 5 + pinned to a live session (≥ 277, roles sum); acceptance 3 relabeled a + smoke check with its real criterion; §2 now quotes the skill-bound text + verbatim; frontier rule gained "reviewed clean", the final-task clause, + the undeclared-dependency default, and "in plan order"; concurrency + claim narrowed to package-reading reviews; fresh-fixer clause rephrased + as a default with reason; resume messages carry what landed since the + executor's HEAD; ledger gained `executed` lines and the executed-vs-not- + started resume rule; escalation text covers sonnet → opus and the + executor prompt's "more capable model" sentence; PR #76 example corrected + (it is a chain — per-task schedule); baseline max fix rounds (2) and the + P1/P2 mapping recorded. +- v1.2 (2026-08-21): acceptance 4 admits the split-at-overlap reading the + v1.1 clause licenses; re-test observations recorded. +- v1.3 (2026-08-21): codex review adopted — concurrent-review predicate, + telemetry dedupe and active time; token figures recomputed. +- v1.4 (2026-08-21): codex round 2 adopted — deferred-review tree clause; + telemetry recognizes task-prefixed noun descriptions; dispatch-count + citations corrected. +- v1.5 (2026-08-21): codex round 3 — fix-range ledger and re-review + packaging, DONE routes through the frontier, reviewer template gains + task-head/checkout-head; eval-before-release finding recorded as + declined. +- v1.6 (2026-08-21): codex round 4 — re-review messages refresh the + reviewer's report, checkout, and worktree inputs. +- v1.7 (2026-08-21): historical-spec before/after comparison (8 specs × + new×2 + control×1) recorded under Surprises. +- v1.8 (2026-08-21): GitHub Codex review adopted — telemetry role order + (fixer/reviewer before the broad task-N executor match) and resume-aware + idle rule; reviewer template's detached-worktree sentence conditional on + the Checkout line. diff --git a/package.json b/package.json index 2b1d0a4780c..969b1360ae0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "doperpowers", - "version": "7.59.0", + "version": "7.60.0", "description": "Doperpowers skills and runtime bootstrap for coding agents", "type": "module", "keywords": [ diff --git a/scripts/sde-telemetry b/scripts/sde-telemetry new file mode 100755 index 00000000000..322207750bf --- /dev/null +++ b/scripts/sde-telemetry @@ -0,0 +1,212 @@ +#!/usr/bin/env python3 +"""sde-telemetry — post-hoc metrics for a controlled-track (SDE) run. + +For each session transcript given, prints: wall-clock span, dispatch counts +by role, and token totals by model for the controller and its subagents. +Token totals are deduplicated by message id (one assistant response is +written as several rows repeating the same usage). Durations are reported +both as the transcript's span and as active time — the span with idle gaps +excluded: gaps over 15 minutes that precede a new user message (a resume) +rather than a tool result, which is one slow tool call and stays counted. +Subagent transcripts are read from the sibling directory +//subagents/. + +Usage: sde-telemetry SESSION_JSONL [SESSION_JSONL ...] +""" +import collections +import datetime +import glob +import json +import os +import re +import sys + +# A gap this long is the agent idling between resumes, unless a tool result +# follows it — then it is one slow tool call, which counts as active. +IDLE_GAP = datetime.timedelta(minutes=15) + +# Tried in order: the narrow roles before the broad "task N" executor match, +# so "Task 2: fix pagination" reads as a fixer rather than an executor. +ROLE_PATTERNS = [ + (re.compile(r"re-review|review task|^t\d+ reviewer|^task \d+:\s*review", + re.I), "task-reviewer"), + (re.compile(r"^t\d+ fixer|\bfix", re.I), "fixer"), + (re.compile(r"^(implement|execute|run) task|^task \d+:" + r"|^t\d+ (implementer|executor)", re.I), "executor"), +] + + +def role_of(description): + for pattern, role in ROLE_PATTERNS: + if pattern.search(description or ""): + return role + return "other" + + +def is_tool_result(entry): + """True when this entry is a tool result — the gap before it was one call. + + Human turns and tool results share `type: "user"`: a tool result carries a + `tool_result` content block, while a resume is a string or plain text + blocks. Everything else — assistant entries and the sidecar entries a + human turn arrives with (attachments, queued messages, hook output) — + is not a tool result, so a gap before one is the agent waiting. + """ + if entry.get("type") != "user": + return False + content = (entry.get("message") or {}).get("content") + return isinstance(content, list) and any( + isinstance(block, dict) and block.get("type") == "tool_result" + for block in content + ) + + +def scan_transcript(path): + """Return (token totals by model, first timestamp, last timestamp, active time). + + Usage is deduped by (model, message id), keeping the last snapshot seen + for that id: one assistant response is written as several rows — thinking, + tool use — that share a message id and repeat its usage. + """ + usage_by_id = {} + anonymous = 0 + times = [] + with open(path) as fh: + for line in fh: + try: + entry = json.loads(line) + except json.JSONDecodeError: + continue + ts = entry.get("timestamp") + if ts: + times.append((datetime.datetime.fromisoformat(ts.replace("Z", "+00:00")), + is_tool_result(entry))) + message = entry.get("message") + if isinstance(message, dict) and isinstance(message.get("usage"), dict): + model = message.get("model") or "unknown" + message_id = message.get("id") + if message_id is None: + anonymous += 1 + message_id = ("no-id", anonymous) + usage_by_id[(model, message_id)] = message["usage"] + + totals = collections.defaultdict(collections.Counter) + for (model, _), usage in usage_by_id.items(): + for key in ( + "input_tokens", + "output_tokens", + "cache_read_input_tokens", + "cache_creation_input_tokens", + ): + totals[model][key] += usage.get(key) or 0 + + times.sort() + first = times[0][0] if times else None + last = times[-1][0] if times else None + return totals, first, last, active_time(times) + + +def active_time(times): + """Sum of the segment spans, splitting at each idle gap. + + `times` is (timestamp, is_tool_result) per entry. A gap over IDLE_GAP is + idle unless the entry after it is a tool result: then the gap is one slow + tool call (a 20-minute suite), which is work, not waiting. Every other + long gap ends in a new human turn — a resume — so it is excluded. + """ + active = datetime.timedelta() + if not times: + return active + segment_start = previous = times[0][0] + for t, tool_result in times[1:]: + if not tool_result and t - previous > IDLE_GAP: + active += previous - segment_start + segment_start = t + previous = t + return active + previous - segment_start + + +def merge(into, totals): + for model, counter in totals.items(): + into[model].update(counter) + + +def fmt_totals(totals, indent=" "): + lines = [] + for model in sorted(totals): + c = totals[model] + lines.append( + f"{indent}{model}: in={c['input_tokens']:,} out={c['output_tokens']:,} " + f"cache_read={c['cache_read_input_tokens']:,} " + f"cache_write={c['cache_creation_input_tokens']:,}" + ) + return "\n".join(lines) or f"{indent}(none)" + + +def report(session_jsonl): + session_jsonl = os.path.abspath(session_jsonl) + session_id = os.path.splitext(os.path.basename(session_jsonl))[0] + print(f"=== session {session_id} ===") + + controller_totals, first, last, controller_active = scan_transcript(session_jsonl) + if first and last: + span = last - first + print(f"wall-clock: {first.isoformat()} -> {last.isoformat()} " + f"(span {span}, active {controller_active})") + + subagent_dir = os.path.join(os.path.dirname(session_jsonl), session_id, "subagents") + role_counts = collections.Counter() + role_totals = collections.defaultdict(lambda: collections.defaultdict(collections.Counter)) + role_span = collections.defaultdict(datetime.timedelta) + role_active = collections.defaultdict(datetime.timedelta) + dispatches = [] + for meta_path in sorted(glob.glob(os.path.join(subagent_dir, "*.meta.json"))): + with open(meta_path) as fh: + meta = json.load(fh) + description = meta.get("description", "") + role = role_of(description) + role_counts[role] += 1 + agent_jsonl = meta_path.replace(".meta.json", ".jsonl") + if os.path.exists(agent_jsonl): + totals, a_first, a_last, a_active = scan_transcript(agent_jsonl) + merge(role_totals[role], totals) + duration = (a_last - a_first) if a_first and a_last else None + if duration is not None: + role_span[role] += duration + role_active[role] += a_active + out_tokens = sum(c["output_tokens"] for c in totals.values()) + dispatches.append( + (a_first, role, meta.get("model") or "?", out_tokens, description, duration, a_active) + ) + + print(f"dispatches: {sum(role_counts.values())} total " + f"({', '.join(f'{r}={n}' for r, n in sorted(role_counts.items())) or 'none'})") + print("controller tokens:") + print(fmt_totals(controller_totals)) + for role in sorted(role_totals): + print(f"{role} tokens ({role_counts[role]} dispatches):") + print(fmt_totals(role_totals[role])) + print(f" time: span={role_span[role]} active={role_active[role]}") + if dispatches: + print("dispatch list (start time, span, active, model, output tokens):") + for start, role, model, out_tokens, description, duration, active in sorted( + dispatches, key=lambda d: (d[0] is None, d[0]) + ): + when = start.strftime("%m-%d %H:%M") if start else "?" + dur = str(duration).split(".")[0] if duration is not None else "?" + act = str(active).split(".")[0] + print(f" {when} {dur:>8} {act:>8} {model:6} out={out_tokens:>7,} [{role}] {description}") + print() + + +def main(argv): + if len(argv) < 2: + print(__doc__.strip(), file=sys.stderr) + return 2 + for path in argv[1:]: + report(path) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/skills/subagent-driven-execution/SKILL.md b/skills/subagent-driven-execution/SKILL.md index edc37280888..0efd18eea2a 100644 --- a/skills/subagent-driven-execution/SKILL.md +++ b/skills/subagent-driven-execution/SKILL.md @@ -6,8 +6,8 @@ description: Use when running an execution plan with independent tasks in the cu # Subagent-Driven Execution Execute a plan by dispatching a fresh executor subagent per task, a task -review (spec compliance + code quality) after each, and a broad whole-branch -review at the end. +review (spec compliance + code quality) at each dependency frontier, and a +broad whole-branch review at the end. **Why subagents:** each worker gets exactly the context its task needs — no session history, no other tasks' noise — and your own context stays free for @@ -34,20 +34,59 @@ Tightly-coupled tasks or no plan yet → work manually or brainstorm first. asserts nothing). Present findings to your human partner as one batched question — each beside the plan text that mandates it — before execution; a clean scan proceeds without comment. -3. **Per task:** extract the brief (`scripts/task-brief PLAN_FILE N`), - record BASE (the current commit), dispatch the executor - ([executor-prompt.md](executor-prompt.md)). Answer its questions - before it proceeds. One executor at a time — parallel executors - conflict in a shared worktree. -4. On DONE: generate the review package - (`scripts/review-package PLAN_FILE BASE HEAD`), dispatch the task - reviewer ([task-reviewer-prompt.md](task-reviewer-prompt.md)) with the - printed path. -5. **Findings:** dispatch a fix subagent for Critical/Important findings, - then re-review; repeat until both verdicts are clean. Record Minor - findings in the ledger — the final review triages that list, so it is - read, not discarded. Fix through a subagent, not your own edits: manual - fixes pollute your context and skip review. +3. **Per task, in plan order:** extract the brief (`scripts/task-brief + PLAN_FILE N`), record BASE (the current commit), dispatch the executor + ([executor-prompt.md](executor-prompt.md)) and write the task's + `executed` ledger line with its agent handle (Durable Progress below) + — fixes resume it. Answer its questions before it proceeds. One + executor at a time — parallel executors conflict in a shared worktree. +4. **Review at the frontier:** a task is reviewed clean — findings fixed + and re-reviewed — before any task that consumes what it produced + dispatches; the briefs' Interfaces name the producers, and the plan's + final verification task consumes the whole branch. Tasks nothing + downstream consumes yet may keep executing and are reviewed together + when the frontier closes (the next task consumes from them, or the + plan ends): one review package per task (`scripts/review-package + PLAN_FILE BASE HEAD`, each task's own BASE..HEAD), one task reviewer + per task ([task-reviewer-prompt.md](task-reviewer-prompt.md)) with the + printed path, dispatched together when their focused tests cannot + collide — reviews read their package, not the tree, so hermetic suites + run concurrently; suites that share mutable state — a test database, a + fixed port — run one reviewer at a time. + Where no interface is declared but two tasks touch the same files, + judge from their Files lists: an overlap that looks load-bearing is + reviewed before the later task dispatches. The frontier is the ceiling + on deferral, not the floor: a DONE_WITH_CONCERNS, a doubt of your own, + or a first task whose brief style the executor may have misread are + reasons to review that task now. + A deferred review reads a tree that has moved past its package — + Task 1's package is BASE1..HEAD1 while the checkout sits at the + wave's last HEAD. Name the current HEAD and what landed since the + task's own HEAD (the sibling commits and files) in the dispatch, so + a sibling's effect is not read as this task's; a check that must see + the task's own tree — a focused test, a named risk — runs in a + detached worktree at the packaged HEAD (`git worktree add --detach + /review-N `, removed after the review) rather + than in the shared checkout. +5. **Findings:** Critical/Important findings go back to the executor that + wrote the code — resume it with the findings; it holds the task's + context and skips the orientation a fresh fixer pays. Several tasks + with findings in one wave resume one at a time (shared worktree). + Re-review by resuming the reviewer with the fix commits' package + (`scripts/review-package PLAN_FILE FIX_BASE FIX_HEAD` — the fix range: + the reviewer already holds the task's original package, and a deferred + task's fix lands past its siblings' commits); repeat until both + verdicts are clean. That message names the fix range, sends the + reviewer back to the report (the executor appended the fix's test + evidence there), and carries the refreshed checkout head and what + landed since — plus a fresh detached worktree at the fix head if the + review needs the task's own tree. A resumed reviewer otherwise judges + the fix against its pre-fix memory. A fresh fixer when the executor + cannot be resumed, or when its frame is the problem — two failed + re-reviews is the usual sign. Record Minor findings in the ledger — + the final review triages that list, so it is read, not discarded. Fix + through a worker, not your own edits: manual fixes pollute your + context and skip review. 6. Mark the task complete in todos and the ledger; route anything that changed design understanding into the spec's living tail (doperpowers:execspec). Implementation noise stays in commit messages. @@ -61,13 +100,16 @@ Tightly-coupled tasks or no plan yet → work manually or brainstorm first. ## Model selection -Dispatch workers — executors, task reviewers, fixers — on the mid-tier -model at high reasoning effort (Claude: sonnet, effort high). This default -is empirical: cheap-tier executors take 2–3× the turns and cost more -overall, and top-tier workers add cost without adding reliability — the -plan and the brief absorb the difficulty, not the model. Escalate per -incident: when a worker reports BLOCKED on reasoning capacity rather than -missing context, re-dispatch that one task on a stronger model. +Dispatch workers — executors, task reviewers, fixers — on opus at high +reasoning effort; the task grain is calibrated to that tier. A simple +task — a doc update, a mechanical rename, a verification walk with every +command given — can go to sonnet. Never dispatch workers on the top tier +(fable): it adds cost without adding reliability and is the controller's +tier, not the worker's — the plan and the brief absorb the difficulty, +not the model. When a worker reports BLOCKED on reasoning capacity rather +than missing context, a sonnet task moves to opus; from opus there is no +tier above — the difficulty moves into the brief: resolve the hard call +yourself and re-dispatch, or split the task. The final whole-branch review is the deliberate exception: strongest available model, highest effort — it is the last gate before merge and the @@ -78,13 +120,16 @@ session's, usually the most expensive. ## Executor statuses -- **DONE** → review package → task reviewer. +- **DONE** → review at the frontier (step 4): package and reviewer now + when something downstream consumes the task or an early-review reason + applies, otherwise it waits for the wave. - **DONE_WITH_CONCERNS** → read the concerns first: correctness or scope concerns get addressed before review; observations ride along to it. - **NEEDS_CONTEXT** → provide the missing context, re-dispatch. - **BLOCKED** → diagnose before retrying: missing context (provide it), - reasoning capacity (stronger model), task too large (split it), plan - wrong (escalate to the human). Something must change — a bare retry + reasoning capacity (sonnet → opus; from opus, resolve the hard call in + the brief), task too large (split it), plan wrong (escalate to the + human). Something must change — a bare retry answers an escalation with nothing. **Reviewer ⚠️ items** — requirements the reviewer could not verify from the @@ -111,20 +156,27 @@ pasted prior-task history): status, commits, a one-line test summary, and concerns. - The task reviewer gets three paths — brief, report, review package — plus the plan's binding constraints copied verbatim (exact values, formats, - stated relationships). Its template already carries the process rules. + stated relationships) and, for a deferred review, the checkout head and + what landed since (the template's Checkout line). Its template already + carries the process rules. - `review-package` BASE is the commit you recorded before dispatching the executor — never `HEAD~1`, which silently drops all but the last - commit of a multi-commit task. + commit of a multi-commit task. A re-review's BASE is the ledger's + `fix-base` — the HEAD when the fix dispatched — so the package is the + fix alone. - Let the reviewer judge: don't pre-rate severity or list things not to flag ("don't treat X as a defect", "at most Minor") — that impulse is usually you sparing yourself a review loop. Adjudicate findings when they come back. A finding that conflicts with the plan's own text is the human's decision: present the finding and the plan text, ask which governs. -- Fix dispatches carry the executor contract: re-run the covering tests - (name them in the dispatch — a one-line fix doesn't need the whole - suite), report the command and output; confirm all three are in the fix - report before re-review. +- Fix messages — to a resumed executor or a fresh fixer — carry the + executor contract: re-run the covering tests (name them — a one-line + fix doesn't need the whole suite), report the command and output; + confirm all three are in the fix report before re-review. A resumed + executor's view of the tree ends at its own HEAD: name what landed + since (commits and files) and have it re-read before editing; its + covering tests include sibling suites touching the same files. - Final-review findings go to ONE fixer with the complete list — per-finding fixers each rebuild context and re-run suites; a real session's per-finding fix wave cost more than all its tasks combined. @@ -141,11 +193,21 @@ expensive failure observed. The ledger file, not your todos, is the record: directory is never yours to read or write. - The ledger lives at `/progress.md`, first line `# SDE ledger — plan: `. If that line names your plan, - tasks with a `Task : complete` line are done — resume at the first - task without one. A ledger naming a different plan file is another plan's - progress: leave it, start your own. -- When a task's review comes back clean, append - `Task N: complete (commits .., review clean)`. + tasks with a `Task : complete` line are done; a task with an + `executed` line but no `complete` line is awaiting review or fixes — + resume its review (or its handles), never re-execute it; resume + executing at the first task with neither. A ledger naming a different + plan file is another plan's progress: leave it, start your own. +- At dispatch, append `Task N: executed (base , executor + )`; add `head ` when the executor returns and + `reviewer ` when the review dispatches — a fix resumes those + handles, and after compaction the ledger is the only place they + survive. When a fix dispatches append `fix-base ` (the HEAD at + that moment) and `fix-head ` when it lands: a deferred task's + fix commits sit past its siblings', so `base..head` no longer bounds + the task's history. +- When a task's review comes back clean, append `Task N: complete + (commits ..[, fix ..], review clean)`. - After compaction, trust the ledger and `git log` over your own recollection. (`git clean -fdx` destroys the workspace — recover from `git log`.) diff --git a/skills/subagent-driven-execution/executor-prompt.md b/skills/subagent-driven-execution/executor-prompt.md index 43ce4ffbaf1..91edb2f3e7d 100644 --- a/skills/subagent-driven-execution/executor-prompt.md +++ b/skills/subagent-driven-execution/executor-prompt.md @@ -5,8 +5,8 @@ Use this template when dispatching an executor subagent. ``` Subagent (general-purpose): description: "Implement Task N: [task name]" - model: [MODEL — REQUIRED: the mid-tier model at high reasoning effort - (Claude: sonnet, effort high) per SKILL.md Model Selection; an + model: [MODEL — REQUIRED: opus at high reasoning effort per SKILL.md + Model Selection (sonnet for a simple task; never the top tier); an omitted model silently inherits the session's most expensive one] prompt: | You are implementing Task N: [task name] @@ -75,8 +75,9 @@ Subagent (general-purpose): **How to escalate:** Report back with status BLOCKED or NEEDS_CONTEXT. Describe specifically what you're stuck on, what you've tried, and what kind of help you need. - The controller can provide more context, re-dispatch with a more capable model, - or break the task into smaller pieces. + The controller can provide more context, resolve the hard call in your + brief, move the task to a stronger worker tier where one exists, or + break the task into smaller pieces. ## Before Reporting Back: Self-Review diff --git a/skills/subagent-driven-execution/task-reviewer-prompt.md b/skills/subagent-driven-execution/task-reviewer-prompt.md index 1b86c75a398..8bebf0c6540 100644 --- a/skills/subagent-driven-execution/task-reviewer-prompt.md +++ b/skills/subagent-driven-execution/task-reviewer-prompt.md @@ -10,8 +10,8 @@ more, nothing less) and is well-built (clean, tested, maintainable) ``` Subagent (general-purpose): description: "Review Task N (spec + quality)" - model: [MODEL — REQUIRED: the mid-tier model at high reasoning effort - (Claude: sonnet, effort high) per SKILL.md Model Selection; an + model: [MODEL — REQUIRED: opus at high reasoning effort per SKILL.md + Model Selection (sonnet for a simple task; never the top tier); an omitted model silently inherits the session's most expensive one] prompt: | You are reviewing one task's implementation: first whether it matches its @@ -34,6 +34,8 @@ Subagent (general-purpose): **Base:** [BASE_SHA] **Head:** [HEAD_SHA] + **Checkout:** [CHECKOUT_SHA] — the shared tree sits here, past this + task's head; landed since: [SINCE] **Diff file:** [DIFF_FILE] Read the diff file once — it contains the commit list, a stat summary, @@ -51,7 +53,11 @@ Subagent (general-purpose): checking the call sites is the right method. Your review is read-only on this checkout. Do not mutate the working - tree, the index, HEAD, or branch state in any way. + tree, the index, HEAD, or branch state in any way. A check that must + see this task's own tree — a focused test, a named risk — runs in the + detached worktree the controller names when a Checkout line appears + above, since this checkout then sits past the task's head; with no + Checkout line the checkout is at the task's head, so run it here. ## Do Not Trust the Report @@ -177,7 +183,11 @@ Subagent (general-purpose): - `[REPORT_FILE]` — REQUIRED: the file the executor wrote its detailed report to - `[BASE_SHA]` — commit before this task -- `[HEAD_SHA]` — current commit +- `[HEAD_SHA]` — the task's last commit — the package's head +- `[CHECKOUT_SHA]`, `[SINCE]` — for a deferred review: where the shared + checkout sits, and the sibling commits and files that landed since + `[HEAD_SHA]`; omit the Checkout line when the checkout is at + `[HEAD_SHA]` - `[DIFF_FILE]` — REQUIRED: the path the controller wrote the review package to (`scripts/review-package PLAN_FILE BASE HEAD` prints the unique path it wrote; the package never enters the controller's context) diff --git a/skills/writing-plans/SKILL.md b/skills/writing-plans/SKILL.md index 082f0870f56..403779791ab 100644 --- a/skills/writing-plans/SKILL.md +++ b/skills/writing-plans/SKILL.md @@ -7,7 +7,7 @@ description: Use when you have a spec or requirements for a multi-step task, bef ## Overview -Write comprehensive execution plans assuming the engineer has zero context for our codebase and questionable taste. Document everything they need to know: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as bite-sized tasks. DRY. YAGNI. TDD. Frequent commits. +Write comprehensive execution plans assuming the engineer has zero context for our codebase and questionable taste. Document everything they need to know: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as tasks built from bite-sized steps. DRY. YAGNI. TDD. Frequent commits. Assume they are a skilled developer, but know almost nothing about our toolset or problem domain. Assume they don't know good test design very well. @@ -63,14 +63,29 @@ This structure informs the task decomposition. Each task should produce self-con ## Task Right-Sizing -A task is the smallest unit that carries its own test cycle and is worth a -fresh reviewer's gate. When drawing task boundaries: fold setup, -configuration, scaffolding, and documentation steps into the task whose -deliverable needs them; split only where a reviewer could meaningfully -reject one task while approving its neighbor. Each task ends with an -independently testable deliverable. - -## Bite-Sized Task Granularity +A task is the unit one executor can reliably own from a single +self-contained brief, but it isn't the smallest unit a reviewer could +gate. Every task boundary costs a fresh worker's orientation (executor, +reviewer, and any fixer each read in from zero) plus your own +dispatch-and-adjudicate turn; draw the fewest boundaries that keep these +true: + +- **Interface frontiers.** Anything a later task consumes must be + produced — and reviewed — before that task dispatches: keep producer + and consumer in one task, or put the contract at a task edge. + Interfaces internal to a task are free. +- **Reviewable diff.** One reviewer reads one task's diff in a single + pass. +- **Ownability.** One task holds one coherent verification strategy and + closely related state owners; its brief needs nothing from neighbors + beyond the declared Interfaces. + +Fold setup, configuration, scaffolding, and documentation into the task +whose deliverable needs them. Inside a task, organize the work as +sequential deliverables, each with its full step sequence and its own +commit. + +## Bite-Sized Steps **Each step is one action (2-5 minutes):** - "Write the failing test" - step @@ -123,7 +138,10 @@ include this section.] - Test: `tests/exact/path/to/test.py` **Interfaces:** -- Consumes: [what this task uses from earlier tasks — exact signatures] +- Consumes: [what this task uses from earlier tasks, naming the task — + "from Task 2: `walk(cursor) -> list[dict]`" — exact signatures. The + controller schedules reviews from these: a producer is reviewed before + its consumer dispatches.] - Produces: [what later tasks rely on — exact function names, parameter and return types. A task's executor sees only their own task; this block is how they learn the names and types neighboring tasks use.] @@ -201,4 +219,4 @@ Evaluate its findings rather than accepting them wholesale; fix what survives. Then execute: - **REQUIRED SUB-SKILL:** Use doperpowers:subagent-driven-execution -- Fresh subagent per task + two-stage review +- Fresh executor per task; reviews at dependency frontiers; fixes resume the executor