diff --git a/plugins/strapped/.claude-plugin/plugin.json b/plugins/strapped/.claude-plugin/plugin.json index ed55647..5135e88 100644 --- a/plugins/strapped/.claude-plugin/plugin.json +++ b/plugins/strapped/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "strapped", - "version": "0.8.0", + "version": "0.8.1", "description": "Adversarial plan → implement → stacked-PR coding harness: rule-partitioned reviewers, refute passes, DAG deliverables in persistent worktrees, CLAUDE.md learning loop", "author": { "name": "Christian Schuetz", diff --git a/plugins/strapped/workflows/strapped-run.js b/plugins/strapped/workflows/strapped-run.js index 523bbdd..937f455 100644 --- a/plugins/strapped/workflows/strapped-run.js +++ b/plugins/strapped/workflows/strapped-run.js @@ -726,7 +726,7 @@ var PR_SCHEMA = { // src/workflows/strapped-run/review-loop.ts var PLAN_LENSES = { a: "completeness: is every element of the original ask covered by some deliverable? Hunt for missing requirements, unhandled edge cases, acceptance criteria without tests, and parts of the ask that silently disappeared", - b: "soundness: wrong assumptions about the codebase, DAG dependency errors (missing or backwards deps, undeclared cross-deliverable coupling), deliverables that mix unrelated themes or whose estimated meaningful diff (excluding generated code, dependency bumps, and fixtures) exceeds ~1,000 lines and should be split, deliverables/chains that should be CONSOLIDATED (fragments of one theme, or a linear chain whose combined meaningful diff — excluding generated code, dependency bumps, and fixtures — is under the ~1,000-line threshold and could be a single deliverable/PR), planned work that is dead, duplicated, or superseded within the plan (steps or files a later step obviates, two deliverables doing the same work, or acceptance criteria/tests no remaining step produces), and steps that cannot work as written" + b: "soundness: wrong assumptions about the codebase, DAG dependency errors (missing or backwards deps, undeclared cross-deliverable coupling), and steps that cannot work as written. Meaningful diff = changed lines excluding generated code, dependency bumps, and fixtures: SPLIT any deliverable that mixes unrelated themes or whose meaningful diff exceeds ~1,000 lines, and CONSOLIDATE fragments of one theme (or a linear chain whose combined meaningful diff is under that threshold) into one deliverable/PR. Flag planned work that is dead, duplicated, or superseded within the plan — steps or files a later step obviates, two deliverables doing the same work, or acceptance criteria/tests no remaining step produces." }; function ruleBlock(rules) { return rules.map((r) => `- ${r.id} (${r.source}): ${r.text}`).join(` @@ -762,7 +762,7 @@ ${ruleBlock(rules)} Known findings from earlier rounds — do NOT re-report unless the revision failed to address them: ${digest(seen2)} -Enumerated ${opts.enumeratedItemsLabel} checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`${opts.enumeratedItemsSection}\` section, enumerate each item in order as ${opts.enumeratedItemsLabel}1..${opts.enumeratedItemsLabel}n across the whole artifact, and return one ac_checklist entry per item ({ id: "${opts.enumeratedItemsLabel}", verdict: pass|violation|na, evidence: one line }). An item the ${opts.artifactNoun} fails to satisfy, or that no step/test covers, is a BLOCKING finding carrying full guideline-rule weight — enumerating and checking these items is as load-bearing as the rule checklist. If no file has a \`${opts.enumeratedItemsSection}\` section, return \`ac_checklist: []\`. +Enumerated ${opts.enumeratedItemsLabel} checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`${opts.enumeratedItemsSection}\` section, enumerate each item in order as ${opts.enumeratedItemsLabel}1..${opts.enumeratedItemsLabel}n across the whole artifact, and return one ac_checklist entry per item ({ id: "${opts.enumeratedItemsLabel}", verdict: pass|violation|na, evidence: one line }). An item the ${opts.artifactNoun} fails to satisfy, or that no step/test covers, is a BLOCKING finding — checking these is as load-bearing as the rule checklist. If no file has a \`${opts.enumeratedItemsSection}\` section, return \`ac_checklist: []\`. Severity: "blocking" = the plan as written produces wrong or missing work; "concern" = likely gap needing a fix or an explicit justification; "suggestion" = optional polish (never drives revision). Stable key format ":". Confidence under ${cfg.confidenceMin} will be dropped. @@ -1045,13 +1045,13 @@ ${item.validations.map((v) => `- ${v}`).join(` Commit your work on ${item.branch} with a Conventional-Commits message (\`(${cfg.slug}): \` — scope is the run slug, no \`${item.id}:\` title prefix; reference ${item.id} and the feedback fix in the body). If validations pass, commit and return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (contradictory addendum, validation failure you cannot fix), commit what is safe, return status "blocked" with the blocker described — do NOT loop indefinitely.`; } - return `You are the implementation agent for deliverable ${item.id} of strapped run "${cfg.slug}". You have fresh context — everything you need is in the files below. + return `You are the implementation agent for deliverable ${item.id} of strapped run "${cfg.slug}" — fresh context; everything you need is below. -Work EXCLUSIVELY inside the worktree: ${item.worktree} (branch ${item.branch}, based on ${item.base}). This deliverable targets repo "${item.repo}" — never touch ${item.repoRoot} directly. +Work EXCLUSIVELY inside the worktree ${item.worktree} (branch ${item.branch}, based on ${item.base}). This deliverable targets repo "${item.repo}" — never touch ${item.repoRoot} directly. 1. Read your deliverable plan in full: ${item.planFile} 2. Read the shared research digest: ${cfg.dir}/research.md -3. Read the project guidelines: every CLAUDE.md that applies (repo root at minimum). +3. Read every applicable CLAUDE.md (repo root at minimum). ${item.resumeNote ? ` This deliverable is being RESUMED. Prior state: ${item.resumeNote} @@ -1062,7 +1062,7 @@ Before finishing, ALL validations must pass inside the worktree: ${item.validations.map((v) => `- ${v}`).join(` `)} -Commit your work on ${item.branch} with a Conventional-Commits message (\`(${cfg.slug}): \` — scope is the run slug, no \`${item.id}:\` title prefix; reference ${item.id} in the body). If validations pass, commit and return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (missing dependency, contradictory plan, validation failure you cannot fix), commit what is safe, return status "blocked" with the blocker described — do NOT loop indefinitely.`; +Commit on ${item.branch} with a Conventional-Commits message (\`(${cfg.slug}): \` — scope is the run slug, no \`${item.id}:\` title prefix; reference ${item.id} in the body). If validations pass, return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (missing dependency, contradictory plan, an unfixable validation failure), commit what is safe and return status "blocked" with the blocker described — do NOT loop indefinitely.`; } function fixPrompt(cfg, item, findings, round, recordSuffix) { return `You are the fix agent for deliverable ${item.id} of strapped run "${cfg.slug}", code-review round ${round}. Fresh context — everything you need is below. @@ -1296,9 +1296,9 @@ Conventions you MUST follow for every file format: ${cfg.conventionsFile} Procedure: 1. Read the source plan in full, then research each target repo's codebase thoroughly: architecture, the modules the ask touches, existing utilities to reuse, test patterns. 2. Write ${cfg.dir}/research.md — a distilled digest (~300 lines max): architecture notes, key files with one-line roles, library/API findings, decisions with rationale, known pitfalls. This is the only research context implementers will ever see. -3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR — split a theme into multiple deliverables only when its estimated meaningful diff (excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures) exceeds ~1,000 changed lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. -4. Write one self-contained file per deliverable at ${cfg.dir}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${cfg.slug}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). Set base per the cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main (roots, and any cross-repo child, base on their own repo's main — you can never branch across repos). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. -5. Cross-repo deps are ordering-only, NEVER a code dependency: a cross-repo child bases on its own repo's main and does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child has a true code dependency on its parent — either require the shared change to merge to the parent repo's main first, or keep both sides in the same repo/chain. +3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Meaningful diff = changed lines excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR; split a theme only when its estimated meaningful diff exceeds ~1,000 lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. +4. Write one self-contained file per deliverable at ${cfg.dir}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${cfg.slug}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. +5. Cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main — roots and every cross-repo child base on their own repo's main; you can never branch across repos. Cross-repo deps are therefore ordering-only, never a code dependency: a cross-repo child does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child truly needs its parent's code — merge the shared change to the parent repo's main first, or keep both sides in the same repo/chain. 6. Write ${cfg.dir}/manifest.md per the conventions (status: in-review, seed: ${cfg.seed}, budgets — record the EFFECTIVE budgets of this run: plan_rounds: ${cfg.planRounds}, code_rounds: ${cfg.codeRounds}, confidence_min: ${cfg.confidenceMin} — the repos: map listing every target repo above per the conventions — name, root, config path (repos: is an unordered set, no repo is special); the deliverables list with ids/files/repos/deps, theme summary, ASCII DAG sketch). 7. After all plan artifacts are written, run \`node ${stateScript} commit ${cfg.dir}\` via Bash so the run's state root is git-backed from birth (it git-inits the state root if absent and commits the artifacts). Best-effort: proceed even if it reports an error. diff --git a/src/eval/suites/harness/implementer.case.ts b/src/eval/suites/harness/implementer.case.ts index 027589d..152be73 100644 --- a/src/eval/suites/harness/implementer.case.ts +++ b/src/eval/suites/harness/implementer.case.ts @@ -31,7 +31,11 @@ const ITEM_REPO_ROOT = '/home/user/strapped' const ITEM_PLAN_FILE = `${FIXTURE_DIR}/deliverables/D1-dry-run-resolver.md` const ITEM_VALIDATIONS = ['bun run typecheck', 'bun run lint', 'bun test'] -const IMPLEMENTER_PROMPT = `You are the implementation agent for deliverable ${ITEM_ID} of strapped run "${FIXTURE_SLUG}". You have fresh context — everything you need is in the files below. +// Baseline implementer prompt, verbatim from the pre-D4 non-addendum branch of +// `implementPrompt`: filler lead-in ("You have fresh context — everything you +// need is in the files below."), a verbose read-list item 3, and a doubled +// "commit" in the closing paragraph. +const IMPLEMENTER_BASELINE = `You are the implementation agent for deliverable ${ITEM_ID} of strapped run "${FIXTURE_SLUG}". You have fresh context — everything you need is in the files below. Work EXCLUSIVELY inside the worktree: ${ITEM_WORKTREE} (branch ${ITEM_BRANCH}, based on ${ITEM_BASE}). This deliverable targets repo "${ITEM_REPO}" — never touch ${ITEM_REPO_ROOT} directly. @@ -46,11 +50,35 @@ ${ITEM_VALIDATIONS.map(v => `- ${v}`).join('\n')} Commit your work on ${ITEM_BRANCH} with a Conventional-Commits message (\`(${FIXTURE_SLUG}): \` — scope is the run slug, no \`${ITEM_ID}:\` title prefix; reference ${ITEM_ID} in the body). If validations pass, commit and return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (missing dependency, contradictory plan, validation failure you cannot fix), commit what is safe, return status "blocked" with the blocker described — do NOT loop indefinitely.` +// Candidate implementer prompt: filler lead-in folded into the header, read-list +// item 3 tightened, and the doubled "commit" removed. Mirrors the compacted live +// stage source. +const IMPLEMENTER_CANDIDATE = `You are the implementation agent for deliverable ${ITEM_ID} of strapped run "${FIXTURE_SLUG}" — fresh context; everything you need is below. + +Work EXCLUSIVELY inside the worktree ${ITEM_WORKTREE} (branch ${ITEM_BRANCH}, based on ${ITEM_BASE}). This deliverable targets repo "${ITEM_REPO}" — never touch ${ITEM_REPO_ROOT} directly. + +1. Read your deliverable plan in full: ${ITEM_PLAN_FILE} +2. Read the shared research digest: ${FIXTURE_DIR}/research.md +3. Read every applicable CLAUDE.md (repo root at minimum). + +Implement exactly what the plan specifies — its acceptance criteria are the contract. Write the tests the plan names (integration-style, public interfaces). Stay in scope: anything under "Out of scope" is off limits; note side-discoveries in your summary instead of fixing them. + +Before finishing, ALL validations must pass inside the worktree: +${ITEM_VALIDATIONS.map(v => `- ${v}`).join('\n')} + +Commit on ${ITEM_BRANCH} with a Conventional-Commits message (\`(${FIXTURE_SLUG}): \` — scope is the run slug, no \`${ITEM_ID}:\` title prefix; reference ${ITEM_ID} in the body). If validations pass, return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (missing dependency, contradictory plan, an unfixable validation failure), commit what is safe and return status "blocked" with the blocker described — do NOT loop indefinitely.` + export const implementerCase = defineCase({ id: 'implementer', tags: ['implementer'], appendSystemPrompt: STRAPPED_CONTEXT, - prompt: IMPLEMENTER_PROMPT, + // `prompt` tracks the LIVE (compacted) stage source; `variants` records the + // baseline→candidate A/B this deliverable verified (`bun run eval --ab`). + prompt: IMPLEMENTER_CANDIDATE, + variants: { + baseline: { label: 'baseline', prompt: IMPLEMENTER_BASELINE }, + candidate: { label: 'compacted', prompt: IMPLEMENTER_CANDIDATE }, + }, schema: asSchema(IMPLEMENT_SCHEMA), graders: [ schemaConforms(), diff --git a/src/eval/suites/harness/planner.case.ts b/src/eval/suites/harness/planner.case.ts index 35b7531..d6c8c02 100644 --- a/src/eval/suites/harness/planner.case.ts +++ b/src/eval/suites/harness/planner.case.ts @@ -32,7 +32,9 @@ interface PlanOutput { const nonEmptyString = (v: unknown): boolean => typeof v === 'string' && v.trim().length > 0 -const PLANNER_PROMPT = `You are the planning agent for strapped run "${FIXTURE_SLUG}". Produce a complete, reviewable implementation plan from a large source plan document. +/** Shared planner prompt scaffold — steps 3-5 are the compaction hot-spot. */ +function plannerPrompt(steps345: string): string { + return `You are the planning agent for strapped run "${FIXTURE_SLUG}". Produce a complete, reviewable implementation plan from a large source plan document. Source plan (the original ask): ${FIXTURE_SOURCE_PLAN} Target repos (the run state is keyed by the run slug, not by any repo; the work spans these repos — an unordered set): @@ -43,19 +45,41 @@ Conventions you MUST follow for every file format: ${FIXTURE_CONVENTIONS} Procedure: 1. Read the source plan in full, then research each target repo's codebase thoroughly: architecture, the modules the ask touches, existing utilities to reuse, test patterns. 2. Write ${FIXTURE_DIR}/research.md — a distilled digest (~300 lines max): architecture notes, key files with one-line roles, library/API findings, decisions with rationale, known pitfalls. This is the only research context implementers will ever see. -3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR — split a theme into multiple deliverables only when its estimated meaningful diff (excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures) exceeds ~1,000 changed lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. -4. Write one self-contained file per deliverable at ${FIXTURE_DIR}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${FIXTURE_SLUG}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). Set base per the cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main (roots, and any cross-repo child, base on their own repo's main — you can never branch across repos). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. -5. Cross-repo deps are ordering-only, NEVER a code dependency: a cross-repo child bases on its own repo's main and does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child has a true code dependency on its parent — either require the shared change to merge to the parent repo's main first, or keep both sides in the same repo/chain. +${steps345} 6. Write ${FIXTURE_DIR}/manifest.md per the conventions (status: in-review, seed: ${FIXTURE_SEED}, budgets — record the EFFECTIVE budgets of this run: plan_rounds: ${FIXTURE_PLAN_ROUNDS}, code_rounds: ${FIXTURE_CODE_ROUNDS}, confidence_min: ${FIXTURE_CONFIDENCE_MIN} — the repos: map listing every target repo above per the conventions — name, root, config path (repos: is an unordered set, no repo is special); the deliverables list with ids/files/repos/deps, theme summary, ASCII DAG sketch). 7. After all plan artifacts are written, run \`node ${FIXTURE_STATE_SCRIPT} commit ${FIXTURE_DIR}\` via Bash so the run's state root is git-backed from birth (it git-inits the state root if absent and commits the artifacts). Best-effort: proceed even if it reports an error. Return the deliverable list and a one-paragraph summary.` +} + +// Baseline steps 3-5, verbatim from the pre-D4 planner prompt: the +// meaningful-diff exclusion list is spelled out in step 3 and the cross-repo +// base rule is stated in BOTH step 4's tail and step 5. +const BASELINE_STEPS = `3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR — split a theme into multiple deliverables only when its estimated meaningful diff (excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures) exceeds ~1,000 changed lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. +4. Write one self-contained file per deliverable at ${FIXTURE_DIR}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${FIXTURE_SLUG}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). Set base per the cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main (roots, and any cross-repo child, base on their own repo's main — you can never branch across repos). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. +5. Cross-repo deps are ordering-only, NEVER a code dependency: a cross-repo child bases on its own repo's main and does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child has a true code dependency on its parent — either require the shared change to merge to the parent repo's main first, or keep both sides in the same repo/chain.` + +// Candidate steps 3-5: "meaningful diff" defined ONCE in step 3, and the +// cross-repo base rule + ordering-only consequence stated ONCE in step 5 (step +// 4's redundant restatement dropped). Mirrors the compacted live stage source. +const CANDIDATE_STEPS = `3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Meaningful diff = changed lines excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR; split a theme only when its estimated meaningful diff exceeds ~1,000 lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. +4. Write one self-contained file per deliverable at ${FIXTURE_DIR}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${FIXTURE_SLUG}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. +5. Cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main — roots and every cross-repo child base on their own repo's main; you can never branch across repos. Cross-repo deps are therefore ordering-only, never a code dependency: a cross-repo child does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child truly needs its parent's code — merge the shared change to the parent repo's main first, or keep both sides in the same repo/chain.` + +const PLANNER_BASELINE = plannerPrompt(BASELINE_STEPS) +const PLANNER_CANDIDATE = plannerPrompt(CANDIDATE_STEPS) export const plannerCase = defineCase({ id: 'planner', tags: ['planner'], appendSystemPrompt: STRAPPED_CONTEXT, - prompt: PLANNER_PROMPT, + // `prompt` tracks the LIVE (compacted) stage source; `variants` records the + // baseline→candidate A/B this deliverable verified (`bun run eval --ab`). + prompt: PLANNER_CANDIDATE, + variants: { + baseline: { label: 'baseline', prompt: PLANNER_BASELINE }, + candidate: { label: 'compacted', prompt: PLANNER_CANDIDATE }, + }, schema: asSchema(PLAN_SCHEMA), graders: [ schemaConforms(), diff --git a/src/eval/suites/harness/reviewer.case.ts b/src/eval/suites/harness/reviewer.case.ts index 605da91..798b26d 100644 --- a/src/eval/suites/harness/reviewer.case.ts +++ b/src/eval/suites/harness/reviewer.case.ts @@ -31,7 +31,9 @@ interface FindingsOutput { ac_checklist?: Array<{ id?: unknown; verdict?: unknown }> } -const REVIEWER_PROMPT = `You are an adversarial plan reviewer with fresh context. Your job is to find real gaps between a produced implementation plan and the original ask, before any code is written. +/** Shared reviewer prompt scaffold — the AC-checklist paragraph is the hot-spot. */ +function reviewerPrompt(acChecklistParagraph: string): string { + return `You are an adversarial plan reviewer with fresh context. Your job is to find real gaps between a produced implementation plan and the original ask, before any code is written. Original ask: ${FIXTURE_SOURCE_PLAN} Plan under review, in ${FIXTURE_DIR}: manifest.md, research.md, and every file in deliverables/. @@ -48,7 +50,7 @@ ${FIXTURE_RULE_BLOCK} Known findings from earlier rounds — do NOT re-report unless the revision failed to address them: (none — first round) -Enumerated AC checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`## Acceptance criteria\` section, enumerate each item in order as AC1..ACn across the whole artifact, and return one ac_checklist entry per item ({ id: "AC", verdict: pass|violation|na, evidence: one line }). An item the plan fails to satisfy, or that no step/test covers, is a BLOCKING finding carrying full guideline-rule weight — enumerating and checking these items is as load-bearing as the rule checklist. If no file has a \`## Acceptance criteria\` section, return \`ac_checklist: []\`. +${acChecklistParagraph} Severity: "blocking" = the plan as written produces wrong or missing work; "concern" = likely gap needing a fix or an explicit justification; "suggestion" = optional polish (never drives revision). Stable key format ":". Confidence under ${FIXTURE_CONFIDENCE_MIN} will be dropped. @@ -56,12 +58,31 @@ You MUST return a rule_checklist verdict (pass/violation/na + one line of eviden --- Artifact under review (inlined for this single-shot eval; the files above are provided here verbatim) --- ${SEEDED_GAP_PLAN}` +} + +// Baseline AC-checklist paragraph, verbatim from the pre-D4 reviewer prompt: it +// states the equal-weight point twice ("carrying full guideline-rule weight" AND +// "as load-bearing as the rule checklist"). +const BASELINE_AC = `Enumerated AC checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`## Acceptance criteria\` section, enumerate each item in order as AC1..ACn across the whole artifact, and return one ac_checklist entry per item ({ id: "AC", verdict: pass|violation|na, evidence: one line }). An item the plan fails to satisfy, or that no step/test covers, is a BLOCKING finding carrying full guideline-rule weight — enumerating and checking these items is as load-bearing as the rule checklist. If no file has a \`## Acceptance criteria\` section, return \`ac_checklist: []\`.` + +// Candidate AC-checklist paragraph: the doubled equal-weight clause collapsed to +// one. Mirrors the compacted live `review-loop.ts` shared reviewer prompt. +const CANDIDATE_AC = `Enumerated AC checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`## Acceptance criteria\` section, enumerate each item in order as AC1..ACn across the whole artifact, and return one ac_checklist entry per item ({ id: "AC", verdict: pass|violation|na, evidence: one line }). An item the plan fails to satisfy, or that no step/test covers, is a BLOCKING finding — checking these is as load-bearing as the rule checklist. If no file has a \`## Acceptance criteria\` section, return \`ac_checklist: []\`.` + +const REVIEWER_BASELINE = reviewerPrompt(BASELINE_AC) +const REVIEWER_CANDIDATE = reviewerPrompt(CANDIDATE_AC) export const reviewerCase = defineCase({ id: 'reviewer', tags: ['reviewer'], appendSystemPrompt: STRAPPED_CONTEXT, - prompt: REVIEWER_PROMPT, + // `prompt` tracks the LIVE (compacted) reviewer source; `variants` records the + // baseline→candidate A/B this deliverable verified (`bun run eval --ab`). + prompt: REVIEWER_CANDIDATE, + variants: { + baseline: { label: 'baseline', prompt: REVIEWER_BASELINE }, + candidate: { label: 'compacted', prompt: REVIEWER_CANDIDATE }, + }, schema: asSchema(FINDINGS_SCHEMA), graders: [ schemaConforms(), diff --git a/src/workflows/strapped-run/review-loop.ts b/src/workflows/strapped-run/review-loop.ts index 7e3f9e3..5f2e080 100644 --- a/src/workflows/strapped-run/review-loop.ts +++ b/src/workflows/strapped-run/review-loop.ts @@ -20,7 +20,7 @@ import type { export const PLAN_LENSES: Record = { a: 'completeness: is every element of the original ask covered by some deliverable? Hunt for missing requirements, unhandled edge cases, acceptance criteria without tests, and parts of the ask that silently disappeared', - b: 'soundness: wrong assumptions about the codebase, DAG dependency errors (missing or backwards deps, undeclared cross-deliverable coupling), deliverables that mix unrelated themes or whose estimated meaningful diff (excluding generated code, dependency bumps, and fixtures) exceeds ~1,000 lines and should be split, deliverables/chains that should be CONSOLIDATED (fragments of one theme, or a linear chain whose combined meaningful diff — excluding generated code, dependency bumps, and fixtures — is under the ~1,000-line threshold and could be a single deliverable/PR), planned work that is dead, duplicated, or superseded within the plan (steps or files a later step obviates, two deliverables doing the same work, or acceptance criteria/tests no remaining step produces), and steps that cannot work as written', + b: 'soundness: wrong assumptions about the codebase, DAG dependency errors (missing or backwards deps, undeclared cross-deliverable coupling), and steps that cannot work as written. Meaningful diff = changed lines excluding generated code, dependency bumps, and fixtures: SPLIT any deliverable that mixes unrelated themes or whose meaningful diff exceeds ~1,000 lines, and CONSOLIDATE fragments of one theme (or a linear chain whose combined meaningful diff is under that threshold) into one deliverable/PR. Flag planned work that is dead, duplicated, or superseded within the plan — steps or files a later step obviates, two deliverables doing the same work, or acceptance criteria/tests no remaining step produces.', } export function ruleBlock(rules: readonly Rule[]): string { @@ -82,7 +82,7 @@ ${ruleBlock(rules)} Known findings from earlier rounds — do NOT re-report unless the revision failed to address them: ${digest(seen)} -Enumerated ${opts.enumeratedItemsLabel} checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`${opts.enumeratedItemsSection}\` section, enumerate each item in order as ${opts.enumeratedItemsLabel}1..${opts.enumeratedItemsLabel}n across the whole artifact, and return one ac_checklist entry per item ({ id: "${opts.enumeratedItemsLabel}", verdict: pass|violation|na, evidence: one line }). An item the ${opts.artifactNoun} fails to satisfy, or that no step/test covers, is a BLOCKING finding carrying full guideline-rule weight — enumerating and checking these items is as load-bearing as the rule checklist. If no file has a \`${opts.enumeratedItemsSection}\` section, return \`ac_checklist: []\`. +Enumerated ${opts.enumeratedItemsLabel} checklist — you and the other reviewer BOTH return this every round (it is NOT partitioned like the guideline rules): read EVERY artifact file's \`${opts.enumeratedItemsSection}\` section, enumerate each item in order as ${opts.enumeratedItemsLabel}1..${opts.enumeratedItemsLabel}n across the whole artifact, and return one ac_checklist entry per item ({ id: "${opts.enumeratedItemsLabel}", verdict: pass|violation|na, evidence: one line }). An item the ${opts.artifactNoun} fails to satisfy, or that no step/test covers, is a BLOCKING finding — checking these is as load-bearing as the rule checklist. If no file has a \`${opts.enumeratedItemsSection}\` section, return \`ac_checklist: []\`. Severity: "blocking" = the plan as written produces wrong or missing work; "concern" = likely gap needing a fix or an explicit justification; "suggestion" = optional polish (never drives revision). Stable key format ":". Confidence under ${cfg.confidenceMin} will be dropped. diff --git a/src/workflows/strapped-run/stages/implement.ts b/src/workflows/strapped-run/stages/implement.ts index c92f510..71f7454 100644 --- a/src/workflows/strapped-run/stages/implement.ts +++ b/src/workflows/strapped-run/stages/implement.ts @@ -40,20 +40,20 @@ ${item.validations.map(v => `- ${v}`).join('\n')} Commit your work on ${item.branch} with a Conventional-Commits message (\`(${cfg.slug}): \` — scope is the run slug, no \`${item.id}:\` title prefix; reference ${item.id} and the feedback fix in the body). If validations pass, commit and return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (contradictory addendum, validation failure you cannot fix), commit what is safe, return status "blocked" with the blocker described — do NOT loop indefinitely.` } - return `You are the implementation agent for deliverable ${item.id} of strapped run "${cfg.slug}". You have fresh context — everything you need is in the files below. + return `You are the implementation agent for deliverable ${item.id} of strapped run "${cfg.slug}" — fresh context; everything you need is below. -Work EXCLUSIVELY inside the worktree: ${item.worktree} (branch ${item.branch}, based on ${item.base}). This deliverable targets repo "${item.repo}" — never touch ${item.repoRoot} directly. +Work EXCLUSIVELY inside the worktree ${item.worktree} (branch ${item.branch}, based on ${item.base}). This deliverable targets repo "${item.repo}" — never touch ${item.repoRoot} directly. 1. Read your deliverable plan in full: ${item.planFile} 2. Read the shared research digest: ${cfg.dir}/research.md -3. Read the project guidelines: every CLAUDE.md that applies (repo root at minimum). +3. Read every applicable CLAUDE.md (repo root at minimum). ${item.resumeNote ? `\nThis deliverable is being RESUMED. Prior state:\n${item.resumeNote}\n` : ''} Implement exactly what the plan specifies — its acceptance criteria are the contract. Write the tests the plan names (integration-style, public interfaces). Stay in scope: anything under "Out of scope" is off limits; note side-discoveries in your summary instead of fixing them. Before finishing, ALL validations must pass inside the worktree: ${item.validations.map(v => `- ${v}`).join('\n')} -Commit your work on ${item.branch} with a Conventional-Commits message (\`(${cfg.slug}): \` — scope is the run slug, no \`${item.id}:\` title prefix; reference ${item.id} in the body). If validations pass, commit and return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (missing dependency, contradictory plan, validation failure you cannot fix), commit what is safe, return status "blocked" with the blocker described — do NOT loop indefinitely.` +Commit on ${item.branch} with a Conventional-Commits message (\`(${cfg.slug}): \` — scope is the run slug, no \`${item.id}:\` title prefix; reference ${item.id} in the body). If validations pass, return status "implemented" with validations_green true. If you hit a blocker you cannot resolve (missing dependency, contradictory plan, an unfixable validation failure), commit what is safe and return status "blocked" with the blocker described — do NOT loop indefinitely.` } function fixPrompt(cfg: RunConfig, item: WaveItem, findings: ReadonlyArray>, round: number, recordSuffix: string): string { diff --git a/src/workflows/strapped-run/stages/plan.ts b/src/workflows/strapped-run/stages/plan.ts index 7506c5b..4ff9c10 100644 --- a/src/workflows/strapped-run/stages/plan.ts +++ b/src/workflows/strapped-run/stages/plan.ts @@ -23,9 +23,9 @@ Conventions you MUST follow for every file format: ${cfg.conventionsFile} Procedure: 1. Read the source plan in full, then research each target repo's codebase thoroughly: architecture, the modules the ask touches, existing utilities to reuse, test patterns. 2. Write ${cfg.dir}/research.md — a distilled digest (~300 lines max): architecture notes, key files with one-line roles, library/API findings, decisions with rationale, known pitfalls. This is the only research context implementers will ever see. -3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR — split a theme into multiple deliverables only when its estimated meaningful diff (excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures) exceeds ~1,000 changed lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. -4. Write one self-contained file per deliverable at ${cfg.dir}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${cfg.slug}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). Set base per the cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main (roots, and any cross-repo child, base on their own repo's main — you can never branch across repos). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. -5. Cross-repo deps are ordering-only, NEVER a code dependency: a cross-repo child bases on its own repo's main and does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child has a true code dependency on its parent — either require the shared change to merge to the parent repo's main first, or keep both sides in the same repo/chain. +3. Split the work into deliverables by discrete theme, forming a DAG: independent work has no deps, dependent work lists its parent deliverable ids. Meaningful diff = changed lines excluding generated code, dependency/lockfile bumps, generated clients/schemas, vendored code, and large fixtures. Keep one coherent theme in a single deliverable so a reviewer can grasp the whole change in one PR; split a theme only when its estimated meaningful diff exceeds ~1,000 lines. Prefer a few cohesive, independently-shippable nodes over many fragments that scatter one theme across PRs. Assign each deliverable to exactly one target repo. +4. Write one self-contained file per deliverable at ${cfg.dir}/deliverables/-.md per the conventions (frontmatter: id, title, deps, repo: , status: pending, branch: strapped/${cfg.slug}/-, base, worktree: null, pr: null, review_rounds_used: 0, feedback_rounds_used: 0, parked_reason: null, estimated_diff_lines; body: Context slice from your research, Files to touch, Implementation steps, Acceptance criteria, Tests, Out of scope). A fresh implementer seeded with ONLY this file plus research.md must be able to do the work. +5. Cross-repo base rule: a deliverable's base is a parent branch WITHIN THE SAME repo, otherwise that repo's main — roots and every cross-repo child base on their own repo's main; you can never branch across repos. Cross-repo deps are therefore ordering-only, never a code dependency: a cross-repo child does not have its parent's unmerged code. Reject or restructure any plan where a cross-repo child truly needs its parent's code — merge the shared change to the parent repo's main first, or keep both sides in the same repo/chain. 6. Write ${cfg.dir}/manifest.md per the conventions (status: in-review, seed: ${cfg.seed}, budgets — record the EFFECTIVE budgets of this run: plan_rounds: ${cfg.planRounds}, code_rounds: ${cfg.codeRounds}, confidence_min: ${cfg.confidenceMin} — the repos: map listing every target repo above per the conventions — name, root, config path (repos: is an unordered set, no repo is special); the deliverables list with ids/files/repos/deps, theme summary, ASCII DAG sketch). 7. After all plan artifacts are written, run \`node ${stateScript} commit ${cfg.dir}\` via Bash so the run's state root is git-backed from birth (it git-inits the state root if absent and commits the artifacts). Best-effort: proceed even if it reports an error.