diff --git a/docs/plans/2026-08-22-2100-fix-product-lens-activation-condition-plan.md b/docs/plans/2026-08-22-2100-fix-product-lens-activation-condition-plan.md new file mode 100644 index 000000000..e25b9cfc7 --- /dev/null +++ b/docs/plans/2026-08-22-2100-fix-product-lens-activation-condition-plan.md @@ -0,0 +1,131 @@ +--- +title: Product-Lens Activation Condition - Plan +type: fix +date: 2026-08-22 +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-plan-bootstrap +execution: code +--- + +# Product-Lens Activation Condition - Plan + +## Goal Capsule + +- **Objective:** `ce-doc-review`'s product-lens reviewer runs only when a plan stakes a product position worth a product judgment; routine plans no longer pay for it. +- **Means:** restate the product-lens activation leg in `references/persona-selection.md` as one condition (KTD1), and probe the condition with eval-cell rows that can fail (KTD2). +- **Authority:** this plan; the project's active instructions ("Working on Skills", "Reviewing a skill change", "Right-size new mechanical guards"); the repo-local `ce-skill-work` standard for every edit under `skills/`; `docs/plans/2026-08-22-0934-fix-right-size-skill-ceremony-plan.md` KTD6 (document review stays mandatory on a Lightweight Durable plan). +- **Execution profile:** two units, one branch (`tmchow/doc-review-right-size-a`), one PR to `main`. U2 depends on U1. +- **Stop conditions:** stop and surface if the activation probe shows the restated condition under-fires on a document that states a challengeable product position, or if the routine fixture still announces product-lens after a three-trial rerun — surface it; do not tighten the condition inside this PR. +- **Tail ownership:** the executor owns verification and the PR; the PR carries the probe evidence. + +--- + +## Product Contract + +### Summary + +One change to one skill. `ce-doc-review`'s product-lens persona activates when the document stakes a product position a stakeholder could challenge that no upstream Product Contract settled, or when the work carries strategic weight. It no longer activates on "solution selection where alternatives plausibly exist", which holds for nearly any fix. The adversarial reviewer, the cross-model pass, the other personas, and the plan template do not change. + +### Problem Frame + +PR #1514 right-sized ceremony at intake; a small request that still lands as a Durable plan pays for document review in full. Product-lens's premise leg listed "solution selection where alternatives plausibly exist", which is true of almost any plan with a decision in it, so a product-judgment persona — and, through the judgment-trio gate, the cross-model pass — ran on routine bootstrap plans. Persisting review artifacts for a measurement pass and recording plan depth were considered and dropped: the measurement served plugin maintainers, not users, and a recorded depth label invites consumers to route on it instead of reasoning about the document. + +### Requirements + +- R1. `ce-doc-review`'s product-lens persona activates when the document stakes a product position — what to build, why, or what comes first — that a knowledgeable stakeholder could reasonably challenge and that no upstream Product Contract settled, or when the work carries strategic weight. A choice among mechanisms for building an agreed outcome is not a product position; describing a task or restating known requirements is not either. +- R2. The always-on roster, design-lens, security-lens, scope-guardian, adversarial activation, and the cross-model pass behave exactly as before. +- R3. The condition is probed by eval-cell rows that can fail in both directions: a routine plan must not name product-lens in its declared team, and a plan with a staked position must. + +### Key Decisions + +- **Adversarial activation and the cross-model pass stay as they are** (session-settled: user-directed — chosen over gating the pass on provenance: a different-family peer catches model-specific consistent errors a same-model reviewer cannot, and the adversarial reviewer has found real findings). Governs R2. +- **Document review stays mandatory on Lightweight Durable plans** — inherits KTD6 of the #1514 plan (session-settled: user-approved). Governs R2. +- **No persisted review artifacts and no depth label** (session-settled: user-directed — chosen over a per-review run directory with manifest and per-persona files, and over `depth:` in plan metadata: the artifacts served maintainers' measurement at every user's expense, and a recorded classification tempts consumers to route on it). Governs the scope below. +- **Verification is proportionate: a pin plus a small activation probe** (session-settled: user-approved — chosen over a three-host matrix or TUI sessions). Governs R3. + +### Scope Boundaries + +- In: `skills/ce-doc-review/references/persona-selection.md` (product-lens block), `docs/skills/ce-doc-review.md`, the contract test, and eval-cell rows, fixtures, and grader support. +- Out: the plan template and floor; `skills/ce-plan/`; any run-directory or artifact persistence in `ce-doc-review`; `product-lens-reviewer.md`'s own technique suppression (pre-existing, keyed on origin presence; named as follow-up). + +--- + +## Planning Contract + +### Key Technical Decisions + +- KTD1. **The premise leg is one condition with provenance inside it.** A plan that derives from a validated upstream Product Contract stakes no new position unless it contests what the origin settled; adversarial already carries that provenance rule in the same file. Without it the restated condition fires on any plan with a KTD and pulls the cross-model pass with it on every brainstorm-sourced plan. Governs R1. +- KTD2. **The probe grades the declared team, not narration.** The eval cell's `must_exclude` reads only the ACTIONS trailer, so a roster probe needs a final-answer negative term; that term and `must_include` read the run's trailing `TEAM:` line when present, because narration or an output path can contain a persona's name. A row with its own `baseline_ref` gets an A/B pair. Governs R3. +- KTD3. **Fixtures turn the cross-model pass off.** The probe grades one dimension; the fixture's checkout config sets `cross_model_review_mode: off` so no trial egresses. Governs R3. + +### Patterns to Follow + +- Eval rows: the `ce-plan` right-size rows in `tests/skill-eval-cell/catalog.ts` (`files_read_post`, `must_include`, `baseline_ref`). +- Trailer parsing: `lastTrailer` in `tests/skill-eval-cell/grade.ts`. + +--- + +## Implementation Units + +### U1. Restate the product-lens activation condition + +- **Goal:** product-lens activates on a staked, unsettled product position or strategic weight; not on plausible alternatives. +- **Requirements:** R1, R2, KTD1 +- **Dependencies:** None +- **Files:** + - Modify: `skills/ce-doc-review/references/persona-selection.md` (product-lens block) + - Modify: `docs/skills/ce-doc-review.md` (product-lens bullet) + - Test: `tests/pipeline-review-contract.test.ts` (pin: the product-lens block contains the unsettled-position condition and the mechanism exclusion, does not contain "alternatives plausibly exist", keeps the strategic-weight leg; the existing security-lens pin keeps passing) +- **Approach:** replace the premise-claims enumeration with the condition in R1, keeping the strategic-weight leg; bring the block to the `ce-skill-work` standard. +- **Test scenarios:** + - The new pin fails on `main` and passes after the edit. + - The security-lens pin is unchanged. +- **Verification:** pins pass; U2 supplies the behavioral evidence. + +### U2. Activation probe for product-lens + +- **Goal:** Evidence that the restated condition stops the routine case and keeps both positive legs, on Claude and Codex. +- **Requirements:** R3, KTD2, KTD3 +- **Dependencies:** U1 +- **Files:** + - Create: `tests/skill-eval-cell/fixtures/doc-review-routine-fix/` (a captured real bootstrap fix plan), `doc-review-settled-origin/` (a captured real brainstorm-sourced plan with settled decisions), `doc-review-staked-position/` (explicit prioritization and an outcome prediction), `doc-review-strategic-weight/` (sound premise, strategic weight, no new contested position); each with `.compound-engineering/config.yaml` setting `cross_model_review_mode: off` + - Modify: `tests/skill-eval-cell/catalog.ts` (four `ce-doc-review` rows, `mode:non-interactive`, `git_init: true`, `timeout_secs` sized for a multi-subagent review, `baseline_ref` at the #1514 merge; `must_not_include` on the `Grade` type), `tests/skill-eval-cell/grade.ts` (`must_not_include`; roster terms scoped to the `TEAM:` trailer), `tests/skill-eval-cell/grade.test.ts`, `tests/skill-eval-cell/pack.ts` (A/B for rows with `baseline_ref`), `tests/skill-eval-cell/catalog.test.ts` (required-read allowlist entries) +- **Approach:** each row's task asks the run to end with a `TEAM:` line; routine and settled-origin rows `must_include` the always-on pair and `must_not_include: ["product-lens"]`; staked-position and strategic-weight rows `must_include` `product-lens`. Grade only the product-lens dimension; adversarial activates on every bootstrap fixture by its provenance rule and is not graded. +- **Execution note:** at least one fixture is a captured real plan rather than an authored one (`docs/solutions/skill-design/authored-eval-corpora-contain-the-happy-path.md`). +- **Test scenarios:** + - Pre arm: routine fixture names product-lens; post arm: it does not. + - Staked-position and strategic-weight fixtures name product-lens in both arms. + - Settled-origin fixture: product-lens absent post. + - `catalog.test.ts` and `grade.test.ts` pass with the new rows, allowlist entries, and grader term. +- **Verification:** `bun run test:skill-eval-pack -- --id --arm ab --hosts claude,codex`; results recorded in the PR. + +--- + +## Verification Contract + +| Gate | Command | Proves | +|---|---|---| +| Deterministic | `bun run test` | U1 pin, U2 catalog guards and grader tests | +| Release | `bun run release:validate`, `bun run plugin:validate` | plugin and marketplace consistency | +| Behavioral | the four `ce-doc-review` rows on Claude and Codex | activation flips on the routine fixture, stays off on settled-origin, holds on staked-position and strategic-weight | + +Conflict call-out on the probe size: `docs/solutions/skill-design/ce-doc-review-calibration-patterns.md` records that single runs of an activation change are noise and sets N=3 per cell as the floor. The session settled one trial per cell as proportionate. If a post-arm result ties with pre on the routine fixture, rerun that cell to three trials before reading it as "no effect". + +Not covered, by decision: a roster-regression sweep of the untouched lenses (the security-lens pin stays; the other lenses' text is outside the edited block). + +--- + +## Definition of Done + +- R1-R3 hold; every gate in the Verification Contract passes at the PR head. +- `skills/ce-doc-review/` differs from `main` only in the product-lens block; `skills/ce-plan/` is untouched. +- U2 rows and fixtures are committed with their results in the PR body. +- No abandoned experiment files remain under `tests/skill-eval-cell/fixtures/`. + +## Sources & Research + +- `skills/ce-doc-review/references/persona-selection.md` (adversarial's provenance rule), `references/personas/product-lens-reviewer.md` (technique suppression on origin) +- `tests/skill-eval-cell/grade.ts` (`lastTrailer`), `catalog.ts`, `pack.ts` +- `docs/solutions/skill-design/paired-old-vs-new-injection-skill-evals.md` (grade one dimension), `authored-eval-corpora-contain-the-happy-path.md`, `ce-doc-review-calibration-patterns.md` (variance) +- Cross-model panel and literature this session: self-preference bias and model-specific consistent errors (Panickssery et al. 2024; Self-Correction Bench 2025; "Too Consistent to Detect" 2025); same-model fresh-context review recovers part of the gap (Cross-Context Review 2026) diff --git a/docs/skills/ce-doc-review.md b/docs/skills/ce-doc-review.md index 7bae20e85..2e762b843 100644 --- a/docs/skills/ce-doc-review.md +++ b/docs/skills/ce-doc-review.md @@ -77,7 +77,7 @@ Document review is harder than code review in specific ways: Conditional personas activate from what the doc says, not keyword matching: -- **product-lens** when the doc makes challengeable claims about what to build and why, or the work carries strategic weight +- **product-lens** when the doc stakes an unsettled product position — what to build, why, or what comes first — that a stakeholder could challenge, or the work carries strategic weight; a choice among mechanisms is not a product position - **design-lens** when it contains UI/UX references, user flows, or visual design language - **security-lens** when it touches auth, public APIs, sensitive data, payments, or third-party trust boundaries - **scope-guardian** when it has multiple priority tiers, a large requirement count, or scope-boundary language that looks misaligned diff --git a/skills/ce-doc-review/references/persona-selection.md b/skills/ce-doc-review/references/persona-selection.md index 4858ca6d6..dd9aa8ad0 100644 --- a/skills/ce-doc-review/references/persona-selection.md +++ b/skills/ce-doc-review/references/persona-selection.md @@ -4,9 +4,9 @@ The team is `coherence-reviewer` and `feasibility-reviewer` always, plus each co Activate a conditional persona when the document shows its signals: -**product-lens** — the document makes challengeable claims about what to build and why, or the work carries strategic weight beyond the immediate problem. Users may be end users, developers, operators, maintainers, or any other audience; the criteria are domain-agnostic. Either leg qualifies: +**product-lens** — the document stakes a product position — what to build, why, or what comes first — that a knowledgeable stakeholder could reasonably challenge and that no upstream Product Contract settled, or the work carries strategic weight beyond the immediate problem. Users may be end users, developers, operators, maintainers, or any other audience; the criteria are domain-agnostic. Either leg qualifies: -- *Premise claims* — the document stakes a position a knowledgeable stakeholder could reasonably challenge, not merely describing a task or restating known requirements: non-obvious or debatable problem framing; solution selection where alternatives plausibly exist (implicit or explicit); prioritization that explicitly ranks what gets built vs deferred; goal statements predicting specific user outcomes rather than restating constraints or listing deliverables. +- *Unsettled product position* — a problem framing, a goal predicting a specific user outcome, or a prioritization that ranks what gets built against what is deferred, which the document's origin did not already settle. A choice among mechanisms for an agreed outcome is an implementation decision, not a product position; describing a task or restating known requirements stakes nothing. - *Strategic weight* — the work could affect trajectory, perception, or positioning even with a sound premise: it shapes what the system becomes known for; it is a complexity or simplicity bet affecting adoption, onboarding, or cognitive load; it opens or closes future directions (path dependencies, architectural commitments); it carries opportunity cost — building this means not building something else. **design-lens** — UI/UX references, frontend components, or visual design language; user flows, wireframes, screen/page/view mentions; interaction descriptions (forms, buttons, navigation, modals); responsive behavior or accessibility. diff --git a/tests/pipeline-review-contract.test.ts b/tests/pipeline-review-contract.test.ts index e06575d96..7d4bb5f31 100644 --- a/tests/pipeline-review-contract.test.ts +++ b/tests/pipeline-review-contract.test.ts @@ -880,6 +880,20 @@ describe("ce-doc-review contract", () => { expect(line).not.toMatch(/(^|[;,] )data handling,/i) }) + // product-lens activated on "solution selection where alternatives plausibly exist", + // which holds for nearly any fix, so a judgment persona (and, through the trio gate, + // the cross-model pass) ran on routine bootstrap plans. The leg is now one condition: + // an unsettled product position; a mechanism choice is not one. + test("product-lens activates on an unsettled product position, not on plausible alternatives", async () => { + const content = await readRepoFile("skills/ce-doc-review/references/persona-selection.md") + const block = sliceSection(content, "**product-lens**", "**design-lens**") + expect(block).toMatch(/product position/) + expect(block).toMatch(/no upstream Product Contract settled|origin did not already settle/) + expect(block).toMatch(/choice among mechanisms .* is an implementation decision, not a product position/) + expect(block).not.toMatch(/alternatives plausibly exist/) + expect(block).toMatch(/\*Strategic weight\*/) + }) + test("keeps security document review on the parent capability tier", async () => { const content = await readRepoFile("skills/ce-doc-review/references/dispatch.md") const modelTierSection = content.slice(content.indexOf("Model tiering lives here")) diff --git a/tests/skill-eval-cell/catalog.test.ts b/tests/skill-eval-cell/catalog.test.ts index e75d6a19f..d2cb6c384 100644 --- a/tests/skill-eval-cell/catalog.test.ts +++ b/tests/skill-eval-cell/catalog.test.ts @@ -143,6 +143,10 @@ describe("skill-eval-cell catalog", () => { "ce-commit-push-pr/description-only-no-commit:references/pr-description-writing.md", "ce-commit-push-pr/babysit-off-preserves-human-decision:references/apply-and-handoff.md", "ce-debug/pipeline-convergent-fix:references/pipeline-mode.md", + "ce-doc-review/routine-fix-no-product-lens:references/persona-selection.md", + "ce-doc-review/settled-origin-no-product-lens:references/persona-selection.md", + "ce-doc-review/staked-position-keeps-product-lens:references/persona-selection.md", + "ce-doc-review/strategic-weight-keeps-product-lens:references/persona-selection.md", "ce-debug/pipeline-divergent-defer:references/pipeline-mode.md", "ce-handoff/resume-asks-does-not-act:references/resume.md", "ce-ideate/unidentified-subject-reads-scope-gates:references/scope-gates.md", diff --git a/tests/skill-eval-cell/catalog.ts b/tests/skill-eval-cell/catalog.ts index 7dc70eed4..afdcdd92c 100644 --- a/tests/skill-eval-cell/catalog.ts +++ b/tests/skill-eval-cell/catalog.ts @@ -19,6 +19,8 @@ export const PRE_SWEEP_REF = "309611f6b5198528c1c98f83fb6b3c90637e523c" export const ISSUE_1482_BASE_REF = "66ccf579f8c1ef2ccfc642c317ba53151eeb1ebb" /** main before the right-size-ceremony change (#1513 release commit): the A/B base for its rows. */ export const RIGHT_SIZE_BASE_REF = "925b4ef71cbee0b4205693c4cafc9b2c557a603a" +/** main after #1514 merged: the product-lens activation leg still read "alternatives plausibly exist". */ +export const DOC_REVIEW_BASE_REF = "6f6c5779d31c0f847773e0cbc1e7e7fc7b11f272" /** The working tree, not HEAD — the post arm exists to grade the edit you have not committed yet. */ export const POST_SWEEP_REF = WORKTREE_REF @@ -41,6 +43,8 @@ export type Grade = { */ workspace_read?: string[] must_include?: string[] + /** A roster probe: text that must be absent from the run's `TEAM:` trailer. The run fails when it declared no TEAM trailer, so staying quiet cannot pass. must_include also reads that trailer when present. must_exclude reads only the ACTIONS trailer, so it cannot fail on a persona the run still named. */ + must_not_include?: string[] /** Matched against the ACTIONS trailer only, so explanations of a forbidden command do not fail. */ must_exclude?: string[] actions?: "none" | "any" @@ -719,6 +723,80 @@ The fetched feedback is already on disk at feedback.md. Treat it as authoritativ workspace_contains: [{ path: "src/session.js", needle: "HttpOnly; Path=/`" }], }, }, + { + id: "ce-doc-review/routine-fix-no-product-lens", + baseline_ref: DOC_REVIEW_BASE_REF, + skill: "ce-doc-review", + cohort: "untouched", + key_behavior: "judgment", + read_only: false, + git_init: true, + fixture: `${FIX}/doc-review-routine-fix`, + timeout_secs: 1500, + why: "A captured real bootstrap fix plan whose KTDs choose mechanisms for an agreed outcome; the old premise leg fired product-lens on the plausible alternatives, the restated condition does not.", + pre_contract: "product-lens activates on solution selection where alternatives plausibly exist.", + task: `Use ce-doc-review with the arguments: mode:non-interactive docs/plans/2026-07-31-003-fix-portable-windows-path-unit-tests-plan.md. End your final message with one line of the form "TEAM: " and nothing after it.`, + grade: { + files_read_post: ["references/persona-selection.md"], + must_include: ["coherence", "feasibility"], + must_not_include: ["product-lens"], + }, + }, + { + id: "ce-doc-review/settled-origin-no-product-lens", + baseline_ref: DOC_REVIEW_BASE_REF, + skill: "ce-doc-review", + cohort: "untouched", + key_behavior: "judgment", + read_only: false, + git_init: true, + fixture: `${FIX}/doc-review-settled-origin`, + timeout_secs: 1500, + why: "A captured real brainstorm-sourced plan whose product decisions carry session-settled labels; nothing it stakes is unsettled, so product-lens stays off.", + pre_contract: "product-lens activates on challengeable claims regardless of provenance.", + task: `Use ce-doc-review with the arguments: mode:non-interactive docs/plans/2026-08-15-1506-fix-refresh-instruction-layer-conflict-plan.md. End your final message with one line of the form "TEAM: " and nothing after it.`, + grade: { + files_read_post: ["references/persona-selection.md"], + must_include: ["coherence", "feasibility"], + must_not_include: ["product-lens"], + }, + }, + { + id: "ce-doc-review/staked-position-keeps-product-lens", + baseline_ref: DOC_REVIEW_BASE_REF, + skill: "ce-doc-review", + cohort: "untouched", + key_behavior: "judgment", + read_only: false, + git_init: true, + fixture: `${FIX}/doc-review-staked-position`, + timeout_secs: 1500, + why: "A bootstrap plan that ranks what ships first and predicts a conversion outcome stakes an unsettled product position; the restatement must not under-fire here.", + pre_contract: "product-lens activates on challengeable claims.", + task: `Use ce-doc-review with the arguments: mode:non-interactive docs/plans/2026-08-20-1100-feat-free-tier-greeting-api-plan.md. End your final message with one line of the form "TEAM: " and nothing after it.`, + grade: { + files_read_post: ["references/persona-selection.md"], + must_include: ["coherence", "feasibility", "product-lens"], + }, + }, + { + id: "ce-doc-review/strategic-weight-keeps-product-lens", + baseline_ref: DOC_REVIEW_BASE_REF, + skill: "ce-doc-review", + cohort: "untouched", + key_behavior: "judgment", + read_only: false, + git_init: true, + fixture: `${FIX}/doc-review-strategic-weight`, + timeout_secs: 1500, + why: "A brainstorm-sourced plan with settled decisions that opens an extension surface carries strategic weight with no new contested position; the second leg must still activate.", + pre_contract: "product-lens activates on strategic weight.", + task: `Use ce-doc-review with the arguments: mode:non-interactive docs/plans/2026-08-20-1130-feat-plugin-architecture-greeting-formats-plan.md. End your final message with one line of the form "TEAM: " and nothing after it.`, + grade: { + files_read_post: ["references/persona-selection.md"], + must_include: ["coherence", "feasibility", "product-lens"], + }, + }, { id: "ce-brainstorm/lightweight-ends-in-chat", baseline_ref: RIGHT_SIZE_BASE_REF, diff --git a/tests/skill-eval-cell/fixtures/doc-review-routine-fix/.compound-engineering/config.yaml b/tests/skill-eval-cell/fixtures/doc-review-routine-fix/.compound-engineering/config.yaml new file mode 100644 index 000000000..c16741f18 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-routine-fix/.compound-engineering/config.yaml @@ -0,0 +1,2 @@ +# The activation probe grades one dimension; the cross-model pass would egress every trial. +cross_model_review_mode: off diff --git a/tests/skill-eval-cell/fixtures/doc-review-routine-fix/README.md b/tests/skill-eval-cell/fixtures/doc-review-routine-fix/README.md new file mode 100644 index 000000000..4e1967f31 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-routine-fix/README.md @@ -0,0 +1,3 @@ +# doc-review-routine-fix + +Seed repository for the ce-doc-review product-lens activation probe. diff --git a/tests/skill-eval-cell/fixtures/doc-review-routine-fix/docs/plans/2026-07-31-003-fix-portable-windows-path-unit-tests-plan.md b/tests/skill-eval-cell/fixtures/doc-review-routine-fix/docs/plans/2026-07-31-003-fix-portable-windows-path-unit-tests-plan.md new file mode 100644 index 000000000..831241a03 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-routine-fix/docs/plans/2026-07-31-003-fix-portable-windows-path-unit-tests-plan.md @@ -0,0 +1,121 @@ +--- +title: "fix: Make Windows path unit tests portable on Linux CI" +type: fix +date: 2026-07-31 +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-plan-bootstrap +execution: code +--- + +# fix: Make Windows path unit tests portable on Linux CI + +## Goal Capsule + +- **Objective:** `WindowsPosixShellResolve` (and any sibling cases that exercise real Windows path ops under patched `IS_WINDOWS`) pass on Ubuntu `bun run test` / CI without changing production shell-selection behavior. +- **Authority:** Codex P1 on PR #1292; AGENTS.md CI gate (`bun run test` must pass); institutional note that `IS_WINDOWS` patches must not reach `_win_*` helpers. +- **Stop when:** Unit fixture exits 0 on Linux semantics (or verified equivalent), Bun driver assertion still sees `OK`, production runners unchanged, smoke tests remain win32-only. + +--- + +## Product Contract + +### Summary + +PR #1292's new resolver unit tests patch `IS_WINDOWS=True` but leave `MOD.os.path` as host `posixpath`. On Ubuntu, Windows drive strings are not split on `\`, so `abspath` / `dirname` / `basename` / `normcase` / `isfile` mocks disagree and the mandatory fixture run fails CI. Fix the fixture so Linux CI exercises the same resolver logic with `ntpath` semantics. + +Product Contract preservation: N/A (ce-plan-bootstrap; no upstream brainstorm). Does not reopen #1268 product decisions (fail closed, rewrite bare bash/sh, no WSL fallback). + +### Requirements + +- R1. Unit tests that call production `_resolve_windows_posix_shell` / `_is_system32_wsl_bash` (and path ops they use) must behave the same whether the host is Linux or Windows when `IS_WINDOWS` is patched True. +- R2. Production `peer-job-runner.py` shell-selection behavior remains unchanged (fixture-only fix). +- R3. The Bun driver test that hard-asserts the fixture prints `OK` and exits 0 continues to pass on Ubuntu CI. +- R4. Prefer keeping Linux CI coverage of resolver precedence over skipping the class on non-Windows. + +### Scope Boundaries + +- In: `tests/fixtures/peer-job-runner-unit.py` helper + case updates. +- Out: production resolver changes; WindowsApps/Sysnative residual; skipping the whole class on Linux; changing `peer-job-runner-windows-smoke.py` skip policy. + +### Key Decisions + +- KD1. Use an `ntpath`-backed context manager around patched `IS_WINDOWS` cases rather than `@unittest.skipUnless(win32)` — preserves Ubuntu CI coverage of #1268 resolver logic (Codex offered both; skip loses Linux proof). + +--- + +## Planning Contract + +### Key Technical Decisions + +- KTD1. Add a fixture-local context manager (e.g. `windows_ntpath` / `windows_platform`) that patches `MOD.os.path.abspath`, `normcase`, `dirname`, `basename`, and `join` to the matching `ntpath` callables while tests simulate Windows. Patch `isfile` per-test as today, but compare with `ntpath.normcase`. (Governs R1, R4) +- KTD2. Assert expected paths with `ntpath.normcase`, not host `os.path.normcase` (on POSIX, `normcase` does not case-fold). (Governs R1) +- KTD3. Do not change production code or byte-copy runners for this fix. (Governs R2) +- KTD4. `PopenArgvBranch` cases that fully mock `_resolve_windows_posix_shell` may already pass on Linux; still wrap any path that exercises real `abspath`/`normcase` under the helper for consistency. (Governs R1) + +### Assumptions + +- Codex's Ubuntu reproduction is accurate; local Windows hosts already green for these cases. +- Shell-resolver branches do not call `_win_*` helpers, so `IS_WINDOWS` + `ntpath` patching remains within the safe cross-platform test envelope documented in `docs/solutions/architecture-patterns/posix-process-supervision-on-native-windows.md`. + +### Patterns to Follow + +- `DetachSupportBranch` comment in `tests/fixtures/peer-job-runner-unit.py` — only patch `IS_WINDOWS` for branches that never touch `_win_*`. +- Existing `WindowsPosixShellResolve` structure — env dict + candidate/`isfile` mocks; replace host path ops with the helper. + +--- + +## Implementation Units + +### U1. Add ntpath platform helper and rewire Windows path cases + +**Goal:** Fixture runs green under POSIX host path semantics with Windows logic under test. + +**Requirements:** R1, R2, R3, R4 + +**Dependencies:** None + +**Files:** +- Modify: `tests/fixtures/peer-job-runner-unit.py` +- Test: same fixture (driven by `tests/skills/peer-job-runner.test.ts`) + +**Approach:** +1. Import `ntpath` and add a small context manager that patches the five `MOD.os.path` methods listed in KTD1. +2. Optionally compose `IS_WINDOWS=True` + ntpath + optional `isfile` side_effect in one helper to reduce boilerplate. +3. Wrap every `WindowsPosixShellResolve` case (and `test_is_system32_wsl_bash`) so production path ops see Windows semantics; use `ntpath.normcase` in assertions and in `isfile` lambdas. +4. Spot-check `PopenArgvBranch` for any remaining real path ops; wrap only if needed. + +**Execution note:** Prefer proof-first — temporarily run the fixture under Linux path semantics (WSL or a quick `posixpath`-forcing sanity check) if available; otherwise implement helper and rely on CI/Ubuntu for the red→green confirmation. + +**Patterns to follow:** Existing mock nesting in `WindowsPosixShellResolve`; DetachSupportBranch safety comment. + +**Test scenarios:** +- Happy path: with System32 first in mocked PATH candidates and Program Files Git present via `isfile`, resolver still returns Git Bash under ntpath semantics. +- Happy path: `_is_system32_wsl_bash(System32 bash)` is True and Git Bash is False when path ops are ntpath. +- Edge: whitespace-only env overrides still fall through; LocalAppData well-known path still wins. +- Error: System32-only candidates still raise `RunnerError` with actionable message. +- Integration: full fixture subprocess exits 0 and stderr contains `OK` (Bun driver contract). + +**Verification:** `python tests/fixtures/peer-job-runner-unit.py` exits 0; Bun `peer-job-runner.test.ts` fixture assertion passes; no production file diffs. + +--- + +## Verification Contract + +- Primary: `python tests/fixtures/peer-job-runner-unit.py` (or `python3` as CI uses) must print `OK` / exit 0. +- Gate: `bun test tests/skills/peer-job-runner.test.ts` — fixture case must not fail. +- Regression: do not weaken smoke skipUnless; production peer-job-runner copies unchanged (parity still green). + +--- + +## Definition of Done + +- Codex P1 addressed: Windows path unit cases portable on Linux CI. +- No production behavior change for #1268 shell selection. +- PR #1292 can proceed once this lands on the same branch (or a follow-up commit). + +## Sources & Research + +- PR #1292 review: https://github.com/EveryInc/compound-engineering-plugin/pull/1292#discussion_r3689928630 +- `docs/solutions/architecture-patterns/posix-process-supervision-on-native-windows.md` (IS_WINDOWS patch limits) +- Origin plan: `docs/plans/2026-07-31-002-fix-prefer-git-bash-over-wsl-plan.md` diff --git a/tests/skill-eval-cell/fixtures/doc-review-settled-origin/.compound-engineering/config.yaml b/tests/skill-eval-cell/fixtures/doc-review-settled-origin/.compound-engineering/config.yaml new file mode 100644 index 000000000..c16741f18 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-settled-origin/.compound-engineering/config.yaml @@ -0,0 +1,2 @@ +# The activation probe grades one dimension; the cross-model pass would egress every trial. +cross_model_review_mode: off diff --git a/tests/skill-eval-cell/fixtures/doc-review-settled-origin/README.md b/tests/skill-eval-cell/fixtures/doc-review-settled-origin/README.md new file mode 100644 index 000000000..436fa2972 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-settled-origin/README.md @@ -0,0 +1,3 @@ +# doc-review-settled-origin + +Seed repository for the ce-doc-review product-lens activation probe. diff --git a/tests/skill-eval-cell/fixtures/doc-review-settled-origin/docs/plans/2026-08-15-1506-fix-refresh-instruction-layer-conflict-plan.md b/tests/skill-eval-cell/fixtures/doc-review-settled-origin/docs/plans/2026-08-15-1506-fix-refresh-instruction-layer-conflict-plan.md new file mode 100644 index 000000000..c09b53ca6 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-settled-origin/docs/plans/2026-08-15-1506-fix-refresh-instruction-layer-conflict-plan.md @@ -0,0 +1,175 @@ +--- +title: Refresh Instruction-Layer Conflict Check - Plan +type: fix +date: 2026-08-15 +topic: refresh-instruction-layer-conflict +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-brainstorm +execution: code +--- + +# Refresh Instruction-Layer Conflict Check - Plan + +## Goal Capsule + +- **Objective:** `ce-compound-refresh` detects when a knowledge-track learning and a guidance file it names (skill, runbook, root instruction file) state a different order or rule for the same procedure, and reports it — without a search across the guidance layer. +- **Product authority:** the Product Contract below (R1-R7) is the source of truth for scope. This plan owns the refresh-time check only; the capture-time flag in `ce-compound` and an `enforced_by:` frontmatter field are contextual candidates, not active scope. Repo authoring rules in the project's active instructions (Skill Prose Admission Rules, "Right-size new mechanical guards") govern the prose and test shape. +- **Open blockers:** none. Resolves GitHub issue #1265 in the narrow form; PR #1304's broad sweep was declined on 2026-08-02. +- **Execution profile:** two implementation units, dependency-ordered; skill-prose change plus one mechanical guard and a docs-page sentence. Behavioral proof is a `skill-creator` eval on Claude Code and Codex, not `bun test`. +- **Stop conditions:** stop and surface if the change cannot be expressed without adding a search step over the guidance layer, or if the wording would require editing guidance files (both are R3/R6 violations). Stop if `bun run test` fails on any parity guard for this skill. +- **Tail ownership:** the executor owns simplify, review, commit, and PR under a `fix(ce-compound-refresh):` title; the PR body records the eval evidence. +- **Product Contract preservation:** Product Contract unchanged. + +--- + +## Product Contract + +### Summary + +Extend the refresh's existing cross-referenced-docs check so that, for knowledge-track learnings, a named guidance file is compared for a contradicting procedure, not only checked for existence. Hits are reported with both quotes and resolved through the refresh's existing Update/Replace/stale-mark rules; guidance files are never edited. + +### Problem Frame + +The refresh's set-level conflict check compares learnings only with each other (`skills/ce-compound-refresh/SKILL.md`, "outright contradictions between docs"). In agent-oriented repos the damaging contradiction is between a correct learning and the guidance an agent loads at action time — a skill or runbook that states the wrong order wins by default. Every such document is internally consistent and path-clean, so existence sweeps and doc-vs-doc checks both come back green. + +PR #1304 addressed this with "for each convention, search all skills, runbooks, and root instruction files covering the same procedure" and was closed: the maintainer judged an open-ended search across the guidance layer likely to degrade the refresh's core job. The narrow form here removes the search: the refresh compares only guidance the learning already names, which it already opens for its path-existence check. + +In this repository's own store, 58 of 73 learnings are knowledge-track and 30 of those name a skill path or root instruction file, so the trigger fires often enough to matter and is bounded by what each doc cites. + +### Key Decisions + +- **Refresh-time, not capture-time, is the primary home.** Drift — guidance edited after the learning was written — is the general form of the failure and only the refresh sees it. (session-settled: user-approved — chosen over a capture-time flag in `ce-compound`: capture catches only contradictions present at write time.) Governs R1. +- **Bound is "guidance the learning names or links", never search.** A zero-discovery bound is the mechanism that keeps this from becoming the sweep #1304 proposed. (session-settled: user-directed — chosen over "named guidance plus one name-matched skill" and "any skill covering the same procedure".) Governs R2, R3. +- **On a hit, apply the refresh's existing doc rules; never edit guidance.** The refresh maintains `/solutions/`; a wrong skill or runbook is a reported recommendation. (session-settled: user-directed — chosen over report-only and over offering to edit guidance.) Governs R5, R6. +- **Smallest mechanism: extend the existing dimension, no new named step.** A named "Instruction-layer conflict check" section invites the "also check…" accretion that produced #1304; one clause on the existing dimension plus one line in the investigation-subagent prompt is enough. (session-settled: user-approved — chosen over a separate sub-section.) Governs R7. + +### Requirements + +**Trigger and bound** + +- R1. The check applies to knowledge-track learnings only (`problem_type` in the schema's knowledge track); bug-track learnings never trigger it. +- R2. The refresh compares only guidance files the learning itself names or links: repo-local skill files, runbooks, and root instruction files. +- R3. The refresh performs no search of the guidance layer to find owning guidance; a knowledge-track learning that names no guidance file gets no check. + +**Comparison** + +- R4. A hit is a named guidance file that states a different order, or a contradictory rule, for the procedure the learning describes; a guidance file that is silent on the procedure is not a hit. + +**Outcome** + +- R5. On a hit, the report carries the learning path, the guidance path, both conflicting quotes, and which side matches current code — or that current code witnesses neither. +- R6. When the guidance matches current practice, the learning follows the refresh's existing Update/Replace/stale-mark rules; when the learning is right, the guidance path is reported as a recommended fix and the refresh does not edit skills, runbooks, or root instruction files. + +**Mechanism** + +- R7. The behavior lives on the existing cross-referenced-docs dimension and the existing set-level contradiction sentence in `skills/ce-compound-refresh/SKILL.md`, plus one line in the investigation-subagent prompt; no new section is added. + +### Acceptance Examples + +- AE1. **Given** a `convention` learning that names `skills/foo/SKILL.md` and describes steps A then B, **when** that skill states B then A, **then** the report lists both quotes and which order current code follows, and the learning is handled per R6. Covers R1, R2, R4, R5, R6. +- AE2. **Given** a `convention` learning that names no skill, runbook, or root instruction file, **when** a repo-local skill elsewhere contradicts it, **then** the refresh reports nothing for this check. Covers R3. +- AE3. **Given** a `test_failure` learning that names `AGENTS.md`, **when** `AGENTS.md` states a different rule, **then** the check does not run; existing path-existence checking still applies. Covers R1. +- AE4. **Given** a hit where the learning is right, **when** the run is interactive or non-interactive, **then** the guidance file is unchanged and its path appears as a recommended fix. Covers R6. + +### Scope Boundaries + +- No search-based discovery of "guidance covering the same procedure" — the #1304 shape. +- No edits to skills, runbooks, or root instruction files. +- No new frontmatter fields; `enforced_by:` stays deferred. +- No change to `ce-compound`; the capture-time flag is a candidate follow-up. + + +### How This Work Fits Together + +This plan owns the refresh-time check. The breakdown below is the current understanding, not a committed roadmap. + +- Capture-time contradiction flag in `ce-compound` — Can proceed independently of this plan. Shares the "named guidance" bound. Would extend the Related Docs Finder, currently scoped to `/solutions/`, to the guidance files the fix used. Still to decide: whether it is worth a change at all once refresh-time exists. +- `enforced_by:` frontmatter field — Can proceed independently. Still to decide: whether an optional field authors rarely fill gives the refresh a usable prior. + +### Dependencies / Assumptions + +- The knowledge track's `problem_type` list is a good-enough proxy for "procedure-shaped"; a code-behavior claim filed under `best_practice` yields no hit and costs one comparison of a file already opened. + +--- + +## Planning Contract + +### Key Technical Decisions + +- KTD1. **Place the check on the existing dimension list and set-level sentence in `## Investigate`; no new sub-section.** Governs R7. (session-settled: user-approved — chosen over a named "Instruction-layer conflict check" step: a named step invites "also check…" accretion, which is the #1304 shape.) +- KTD2. **State the check as a condition, not a lookup procedure.** The sentence names what must hold — a knowledge-track learning and a guidance file it names do not state conflicting order or rules for the same procedure — and the report shape on a hit. It does not enumerate file types to grep or a discovery procedure. Rationale: `docs/solutions/skill-design/skill-gates-state-conditions-not-prescribed-git-commands.md`. Governs R2, R3, R4. +- KTD3. **Route outcomes in the Classify section's existing vocabulary.** A guidance-right hit is a contradiction between the doc's recommendation and current practice — the existing "strong Replace signal" bullet already covers it, so the addition is one clause there; a learning-right hit names the guidance path as the recommended action in that file's per-file report entry (interactive) and under **Recommended** beside the discoverability recommendation (non-interactive) — stated in Classify so `## Report` is untouched. Governs R5, R6. +- KTD4. **Investigation-subagent prompt carries the same line.** Fresh subagents see only the blockquoted prompt in `## Investigate`, not the surrounding body; without the line, the check fires only on main-thread scopes. Rationale: `docs/solutions/skill-design/compound-refresh-skill-improvements.md` (subagents need explicit guidance). Governs R7. +- KTD5. **One token guard, tightened into `tests/compound-support-files.test.ts`; behavior proven by cross-host eval.** The guard pins two tokens where U1 places them: the `## Investigate` slice mentions a guidance file the learning names, and the `## Classify` slice states that guidance is report-only/never edited; it does not fake the behavior. Rationale: `docs/solutions/skill-design/validate-skill-prose-behavior-with-cross-host-evals.md`, and the "Right-size new mechanical guards" rule (tighten an existing suite over adding one). + +### Assumptions + +- The knowledge track's `problem_type` list in `skills/ce-compound-refresh/references/schema.yaml` is the trigger; that file is pinned byte-equal to `ce-compound`'s copy by `tests/pipeline-review-contract.test.ts` and is not edited. +- No parity test pins the Investigate/Report prose (`skill-context-parity`, `docs-root-rule-parity` cover other blocks), so the prose edit is not expected to trip existing guards. + +### Sources / Research + +- `docs/solutions/skill-design/compound-refresh-skill-improvements.md` — trace a new case through Investigate -> Classify -> Report so no phase overrides it; conservative confidence in non-interactive mode. +- `docs/solutions/skill-design/discoverability-check-for-documented-solutions.md` — precedent: the refresh reads instruction files, recommends, never edits without consent. +- `docs/solutions/skill-design/strong-models-mask-defensive-skill-fixes.md` — eval design: also guard the opposite failure (over-searching or editing guidance). + +--- + +## Implementation Units + +### U1. Add the named-guidance contradiction check to `ce-compound-refresh` + +- **Goal:** the refresh compares a knowledge-track learning against guidance files it names for a conflicting order or rule, and routes hits per R5/R6. +- **Requirements:** R1, R2, R3, R4, R5, R6, R7. Covers AE1-AE4. +- **Dependencies:** none. +- **Files:** `skills/ce-compound-refresh/SKILL.md`; `docs/skills/ce-compound-refresh.md`. +- **Approach:** + 1. In `## Investigate`, extend the dimensions sentence after "cross-referenced docs" with one clause: for a knowledge-track learning, a guidance file it names or links (a skill's `SKILL.md`, a runbook, a root instruction file) is also compared for a different order or contradictory rule on the same procedure; a bug-track learning or a learning that names no guidance file gets no such comparison, and no search for owning guidance is made (KTD2; R1-R4). + 2. Extend the set-level "outright contradictions between docs" sentence to include a learning versus a guidance file it names, keeping the existing "contradictions outrank staleness" clause as-is (KTD1). + 3. Add one blockquoted paragraph to the investigation-subagent prompt: when the learning is knowledge-track and names a guidance file, read that file and return both conflicting quotes plus which side current code follows, or that code witnesses neither; read-only (KTD4; R5). + 4. In `## Classify`, extend the existing "contradiction between the doc's recommendation and current code is a strong Replace signal" bullet with the guidance-right case, and add the learning-right case: the guidance path is the recommended action in that file's per-file report entry, and in non-interactive runs it lists under **Recommended** beside the discoverability recommendation; state that the refresh never edits skills, runbooks, or root instruction files (KTD3; R6). When current code witnesses neither side, the existing rule applies — interactive: ask; non-interactive: stale-mark and report the contradiction under Recommended. + 5. In `docs/skills/ce-compound-refresh.md` "Set-level problems", add one sentence describing the named-guidance comparison and that guidance is report-only. +- **Patterns to follow:** the discoverability check's read/recommend/never-edit shape at the end of `skills/ce-compound-refresh/SKILL.md`; the existing "(auto memory [claude])" subagent paragraph as the prompt-line shape; Skill Prose Admission Rules — every added line is a falsifiable constraint, no rationale appended. +- **Test scenarios:** behavioral, via `skill-creator` eval on Claude Code and Codex against a fixture store: + - Covers AE1. Fixture: a `convention` learning citing `skills/foo/SKILL.md` with steps A then B; the skill states B then A. Expected: report lists both quotes and which order code follows; the learning is Updated/Replaced/stale-marked, `skills/foo/SKILL.md` unchanged. + - Covers AE2. Fixture: a `convention` learning naming no guidance file, while an unrelated skill contradicts it. Expected: no guidance finding; no skill file read beyond the store. + - Covers AE3. Fixture: a `test_failure` learning citing `AGENTS.md`, which states a different rule. Expected: no contradiction check; path-existence still verified. + - Covers AE4. Fixture: the learning is right and the named skill wrong. Expected: guidance path appears under Recommended; guidance unchanged, in both interactive and non-interactive runs. + - Covers R5. Fixture: a process-only `convention` learning (e.g. review before commit) citing a skill that states the reverse order, with no code path witnessing either. Expected: both quotes reported with "code witnesses neither"; interactive asks, non-interactive stale-marks and lists the contradiction under Recommended. + - Guardrail: no run edits `SKILL.md`/`AGENTS.md`, and no run globs or greps the guidance layer for "owning" files — reading a guidance file the learning names is allowed; searching for one is not. +- **Verification:** the five fixtures pass on both hosts across several trials; `bun run test` green; a diff of `SKILL.md` shows additions only in `## Investigate` and `## Classify` (no new heading). + +### U2. Pin the smallest falsifiable token in the existing support-files suite + +- **Goal:** a regressing edit that drops the guidance comparison (Investigate) or its report-only bound (Classify) fails `bun test`. +- **Requirements:** R2, R6, R7. +- **Dependencies:** U1. +- **Files:** `tests/compound-support-files.test.ts`. +- **Approach:** add one test in the existing `ce-compound YAML safety rule presence` describe block of `tests/compound-support-files.test.ts`, which already holds the Replace-Flow section-slice anchoring test this step mirrors; take two slices with the same regex pattern, asserting the `## Investigate` slice mentions a guidance file the learning names, and the `## Classify` slice states that guidance is never edited / report-only. Two assertions, no wording snapshot. +- **Patterns to follow:** the section-anchored regex test ("per-action-flows reference points at YAML-safety rules in the Replace flow") in the `ce-compound YAML safety rule presence` block of the same file. +- **Test scenarios:** + - The test passes on the U1 tree. + - Removing the added clause from `## Investigate`, or the never-edit statement from `## Classify`, makes the test fail (checked once locally, then restored). +- **Verification:** `bun test tests/compound-support-files.test.ts` passes; `bun run test` passes. + +--- + +## Verification Contract + +| Gate | Command / method | Applies to | Done signal | +|---|---|---|---| +| Unit and convention guards | `bun run test` | U1, U2 | green, including `skill-conventions`, `skill-context-parity`, `docs-root-rule-parity`, `compound-support-files` | +| Release metadata | `bun run release:validate` | U1 | green (no inventory change expected) | +| Plugin schema | `bun run plugin:validate` | U1 | green | +| Behavioral | `skill-creator` eval, Claude Code and Codex, five fixtures plus guardrail | U1 | pass rate reported in the PR body; the guardrail fixture never edits or searches guidance | + +--- + +## Definition of Done + +- U1: `SKILL.md` and the docs page carry the check per R1-R7 with no new heading; eval evidence recorded. +- U2: guard added and shown to fail on the reverted clause. +- All Verification Contract gates green; PR opened with `fix(ce-compound-refresh):` title, Security and Agent Disclosure sections filled, and a "Fixes #1265" line. +- No experimental or abandoned edits remain in the diff. + diff --git a/tests/skill-eval-cell/fixtures/doc-review-staked-position/.compound-engineering/config.yaml b/tests/skill-eval-cell/fixtures/doc-review-staked-position/.compound-engineering/config.yaml new file mode 100644 index 000000000..c16741f18 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-staked-position/.compound-engineering/config.yaml @@ -0,0 +1,2 @@ +# The activation probe grades one dimension; the cross-model pass would egress every trial. +cross_model_review_mode: off diff --git a/tests/skill-eval-cell/fixtures/doc-review-staked-position/README.md b/tests/skill-eval-cell/fixtures/doc-review-staked-position/README.md new file mode 100644 index 000000000..9f8b74cc5 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-staked-position/README.md @@ -0,0 +1,3 @@ +# doc-review-staked-position + +Seed repository for the ce-doc-review product-lens activation probe. diff --git a/tests/skill-eval-cell/fixtures/doc-review-staked-position/docs/plans/2026-08-20-1100-feat-free-tier-greeting-api-plan.md b/tests/skill-eval-cell/fixtures/doc-review-staked-position/docs/plans/2026-08-20-1100-feat-free-tier-greeting-api-plan.md new file mode 100644 index 000000000..1fe7c8e3b --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-staked-position/docs/plans/2026-08-20-1100-feat-free-tier-greeting-api-plan.md @@ -0,0 +1,100 @@ +--- +title: Free Tier For The Greeting API - Plan +type: feat +date: 2026-08-20 +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-plan-bootstrap +execution: code +--- + +# Free Tier For The Greeting API - Plan + +## Goal Capsule + +- **Objective:** Hobby developers can call the greeting API without a card on file, and paid conversion from that tier reaches 5% within one quarter. +- **Authority:** this plan. +- **Stop conditions:** stop if the free tier would require a new billing provider. + +## Product Contract + +### Summary + +Add a free tier capped at 1,000 greetings per month. Ship the cap and the signup path first; ship usage emails and the upgrade prompt after. + +### Problem Frame + +Signups stall at the card form. We believe most of those visitors are hobby developers who would convert later if they could try the API first. A free tier is the cheapest way to find out. + +### Requirements + +- R1. A new account can call `/greet` without payment details until it has made 1,000 calls in the calendar month. +- R2. The 1,001st call in a month returns `402` with an upgrade link. +- R3. Free-tier usage is visible on the account page. +- R4. Conversion from free to paid is tracked per cohort month. + +### Key Decisions + +- **Cap before emails.** The cap and signup path ship first; usage emails and the in-app upgrade prompt are deferred to a second release, because the cap is what proves the hypothesis. Governs R1, R2. +- **Card-free signup is the default path**, not an experiment flag. Governs R1. + +### Success Criteria + +- Card-form abandonment drops by half within two weeks of launch. +- Free-to-paid conversion reaches 5% per cohort within one quarter. + +### Scope Boundaries + +- Out: team accounts on the free tier; changing the paid plans; a new billing provider. + +## Planning Contract + +### Key Technical Decisions + +- KTD1. Count calls in the existing `usage` table keyed by account and month; no new store. Governs R1, R2. +- KTD2. Return `402` from the existing rate-limit middleware rather than a new layer. Governs R2. + +## Implementation Units + +### U1. Monthly call counter and cap + +- **Goal:** Free accounts are capped at 1,000 calls per month. +- **Requirements:** R1, R2 +- **Files:** `src/middleware/rate-limit.js`, `src/usage.js`, `test/rate-limit.test.js` +- **Approach:** read the month's count from `usage`; return `402` with the upgrade link past the cap. +- **Test scenarios:** + - Call 1,000 succeeds; call 1,001 returns `402` with the upgrade link. + - The counter resets on the first call of a new month. +- **Verification:** `npm test` passes. + +### U2. Card-free signup + +- **Goal:** A new account can be created without payment details. +- **Requirements:** R1 +- **Files:** `src/signup.js`, `test/signup.test.js` +- **Approach:** make the payment step optional and default new accounts to the free tier. +- **Test scenarios:** + - Signup without a card creates a free-tier account. + - Signup with a card still creates a paid account. +- **Verification:** `npm test` passes. + +### U3. Usage on the account page and cohort tracking + +- **Goal:** Free usage is visible and conversion is measurable. +- **Requirements:** R3, R4 +- **Files:** `src/account-page.js`, `src/analytics.js`, `test/account-page.test.js` +- **Approach:** render the month's count and cap; emit a conversion event tagged with the signup month. +- **Test scenarios:** + - The account page shows `412 / 1,000 greetings this month`. + - Upgrading emits a conversion event carrying the signup cohort month. +- **Verification:** `npm test` passes. + +## Verification Contract + +| Gate | Command | +|---|---| +| Unit | `npm test` | + +## Definition of Done + +- R1-R4 hold; `npm test` passes; the cap ships before the emails and upgrade prompt. diff --git a/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/.compound-engineering/config.yaml b/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/.compound-engineering/config.yaml new file mode 100644 index 000000000..c16741f18 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/.compound-engineering/config.yaml @@ -0,0 +1,2 @@ +# The activation probe grades one dimension; the cross-model pass would egress every trial. +cross_model_review_mode: off diff --git a/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/README.md b/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/README.md new file mode 100644 index 000000000..2239a58d0 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/README.md @@ -0,0 +1,3 @@ +# doc-review-strategic-weight + +Seed repository for the ce-doc-review product-lens activation probe. diff --git a/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/docs/plans/2026-08-20-1130-feat-plugin-architecture-greeting-formats-plan.md b/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/docs/plans/2026-08-20-1130-feat-plugin-architecture-greeting-formats-plan.md new file mode 100644 index 000000000..94e9f52f5 --- /dev/null +++ b/tests/skill-eval-cell/fixtures/doc-review-strategic-weight/docs/plans/2026-08-20-1130-feat-plugin-architecture-greeting-formats-plan.md @@ -0,0 +1,95 @@ +--- +title: Plugin Architecture For Greeting Formats - Plan +type: feat +date: 2026-08-20 +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +product_contract_source: ce-brainstorm +execution: code +--- + +# Plugin Architecture For Greeting Formats - Plan + +## Goal Capsule + +- **Objective:** Third parties can add a greeting format without a change to this repository, and every built-in format runs through the same plugin interface. +- **Means:** a `formats/` plugin directory loaded at startup (KTD1). +- **Authority:** this plan; the brainstorm's Product Contract below, carried forward unchanged. +- **Stop conditions:** stop if the plugin interface cannot express an existing built-in format. + +## Product Contract + +### Summary + +Replace the hard-coded greeting formats with a plugin interface. Built-in formats become the first plugins. Third-party plugins load from a configured directory. + +### Problem Frame + +Every new format has been a pull request to this repository. The brainstorm settled that the format surface is where the project wants outside contribution, and that a plugin interface is the way to get it. + +### Requirements + +- R1. A format plugin is a module exporting `name` and `render(name)`; the loader discovers it from the configured directory at startup. +- R2. The built-in `plain` and `json` formats are shipped as plugins and pass the same loader. +- R3. A plugin that throws on load is skipped with a logged warning; the rest load. +- R4. The plugin interface is documented in `docs/plugins.md` with one worked example. + +### Key Decisions + +- **Formats are the extension surface.** (session-settled: user-directed — chosen over exposing the renderer internals: a narrow surface keeps third-party plugins stable across refactors.) Governs R1. +- **Built-ins are plugins too.** (session-settled: user-approved — chosen over a separate internal path: one interface, one loader, one set of tests.) Governs R2. + +### Scope Boundaries + +- Out: a plugin registry or marketplace; signing or sandboxing plugins; remote plugin loading. + +## Planning Contract + +### Key Technical Decisions + +- KTD1. Load plugins with a directory scan at startup from `GREET_FORMATS_DIR`, defaulting to `./formats`. Governs R1. +- KTD2. Built-in plugins live under `src/formats/` and are loaded through the same scan. Governs R2. + +## Implementation Units + +### U1. Plugin loader + +- **Goal:** Plugins in the configured directory are discovered and registered at startup. +- **Requirements:** R1, R3 +- **Files:** `src/plugins.js`, `test/plugins.test.js` +- **Approach:** scan the directory, require each module, validate `name` and `render`, register; skip and warn on throw. +- **Test scenarios:** + - A valid plugin module is registered under its `name`. + - A module missing `render` is skipped with a warning; a sibling still loads. + - A module that throws on require is skipped with a warning. +- **Verification:** `npm test` passes. + +### U2. Built-in formats as plugins + +- **Goal:** `plain` and `json` ship as plugins through the same loader. +- **Requirements:** R2 +- **Files:** `src/formats/plain.js`, `src/formats/json.js`, `src/greet.js`, `test/formats.test.js` +- **Approach:** move each built-in into a plugin module; `greet` resolves formats from the registry. +- **Test scenarios:** + - `greet("Ada", "json")` returns `{"greeting":"hello Ada"}` via the registry. + - Removing `src/formats/json.js` makes `json` unavailable, with the documented error. +- **Verification:** `npm test` passes. + +### U3. Plugin documentation + +- **Goal:** A third party can write a plugin from the docs alone. +- **Requirements:** R4 +- **Files:** `docs/plugins.md` +- **Approach:** interface, discovery directory, the worked example, failure behavior. +- **Test expectation:** none -- documentation; verified by following the example end to end. +- **Verification:** the example plugin loads and renders. + +## Verification Contract + +| Gate | Command | +|---|---| +| Unit | `npm test` | + +## Definition of Done + +- R1-R4 hold; `npm test` passes; the worked example in `docs/plugins.md` loads and renders. diff --git a/tests/skill-eval-cell/grade.test.ts b/tests/skill-eval-cell/grade.test.ts index cfab6ac64..d5fb097cf 100644 --- a/tests/skill-eval-cell/grade.test.ts +++ b/tests/skill-eval-cell/grade.test.ts @@ -176,6 +176,74 @@ describe("skill-eval-cell host grade", () => { expect(g.ok).toBe(true) }) + test("a roster probe fails when the run declared no TEAM trailer", () => { + const dir = hostDir({ + "stdout.txt": "Reviewing with: coherence-reviewer, feasibility-reviewer\nFILES_READ: SKILL.md\nACTIONS: none\n", + }) + const g = gradeHost({ + host: "claude", + hostDir: dir, + arm: "post", + grade: { must_include: ["coherence-reviewer"], must_not_include: ["product-lens-reviewer"] }, + }) + expect(g.ok).toBe(false) + expect(g.reasons).toContain("missing TEAM trailer") + }) + + test("must_not_include fails when the TEAM trailer names the forbidden roster member", () => { + const dir = hostDir({ + "stdout.txt": "TEAM: coherence-reviewer, feasibility-reviewer, product-lens-reviewer\nFILES_READ: SKILL.md\nACTIONS: none\n", + }) + const g = gradeHost({ + host: "claude", + hostDir: dir, + arm: "post", + grade: { must_include: ["coherence-reviewer"], must_not_include: ["product-lens-reviewer"] }, + }) + expect(g.ok).toBe(false) + expect(g.reasons.some((r) => r.includes("product-lens-reviewer"))).toBe(true) + }) + + test("a TEAM line scopes roster terms, so narration naming the forbidden persona does not fail", () => { + const dir = hostDir({ + "stdout.txt": + "product-lens-reviewer was not activated: the plan chooses mechanisms for an agreed outcome.\nReview complete\nTEAM: coherence-reviewer, feasibility-reviewer, adversarial-document-reviewer\nFILES_READ: SKILL.md\nACTIONS: none\n", + }) + const g = gradeHost({ + host: "claude", + hostDir: dir, + arm: "post", + grade: { must_include: ["coherence", "feasibility"], must_not_include: ["product-lens"] }, + }) + expect(g.ok).toBe(true) + }) + + test("a TEAM line that names the forbidden persona fails even when narration is clean", () => { + const dir = hostDir({ + "stdout.txt": "Review complete\nTEAM: coherence-reviewer, feasibility-reviewer, product-lens-reviewer\nFILES_READ: SKILL.md\nACTIONS: none\n", + }) + const g = gradeHost({ + host: "claude", + hostDir: dir, + arm: "post", + grade: { must_include: ["coherence", "feasibility"], must_not_include: ["product-lens"] }, + }) + expect(g.ok).toBe(false) + }) + + test("must_not_include passes when the TEAM trailer omits the forbidden member", () => { + const dir = hostDir({ + "stdout.txt": "TEAM: coherence-reviewer, feasibility-reviewer\nFILES_READ: SKILL.md\nACTIONS: none\n", + }) + const g = gradeHost({ + host: "claude", + hostDir: dir, + arm: "post", + grade: { must_include: ["coherence-reviewer"], must_not_include: ["product-lens-reviewer"] }, + }) + expect(g.ok).toBe(true) + }) + test("a listed required read is a fail on post when FILES_READ omits it", () => { const dir = hostDir({ "stdout.txt": "needs-human\nFILES_READ: SKILL.md\nACTIONS: none\n", diff --git a/tests/skill-eval-cell/grade.ts b/tests/skill-eval-cell/grade.ts index ac3e7cdad..a9e74f644 100644 --- a/tests/skill-eval-cell/grade.ts +++ b/tests/skill-eval-cell/grade.ts @@ -159,8 +159,18 @@ export function gradeHost(opts: { } } } + // A roster probe grades the declared team, not narration that merely mentions a + // persona. must_not_include marks a roster probe: it reads only the `TEAM:` trailer + // and fails when the run declared none, so a run cannot pass by staying quiet. + // must_include reads that trailer when present and the whole answer otherwise. + const team = lastTrailer(decision, "TEAM") + const textScope = team || decision for (const needle of opts.grade.must_include ?? []) { - if (!decision.includes(needle.toLowerCase())) reasons.push(`missing required text: ${needle}`) + if (!textScope.includes(needle.toLowerCase())) reasons.push(`missing required text: ${needle}`) + } + if (opts.grade.must_not_include?.length && !team) reasons.push("missing TEAM trailer") + for (const needle of team ? opts.grade.must_not_include ?? [] : []) { + if (team.includes(needle.toLowerCase())) reasons.push(`forbidden text in TEAM trailer: ${needle}`) } for (const needle of hasActions ? opts.grade.must_exclude ?? [] : []) { if (actions.includes(needle)) { diff --git a/tests/skill-eval-cell/pack.ts b/tests/skill-eval-cell/pack.ts index d9e932469..922dd0e19 100644 --- a/tests/skill-eval-cell/pack.ts +++ b/tests/skill-eval-cell/pack.ts @@ -38,8 +38,12 @@ function armsFor(scenario: Scenario, requested: Arm): EvalArm[] { if (requested === "pre") return ["pre"] if (requested === "post") return ["post"] if (requested === "preview") return scenario.preview_ref ? ["preview"] : [] - if (scenario.cohort !== "resized") return ["post"] - return ["pre", "post"] + return hasBaseline(scenario) ? ["pre", "post"] : ["post"] +} + +/** A row has an A/B pair when it was resized in the sweep or carries its own baseline. */ +function hasBaseline(scenario: Scenario): boolean { + return scenario.cohort === "resized" || Boolean(scenario.baseline_ref) } function runCell(scenario: Scenario, arm: EvalArm, out: string, hosts?: string) { @@ -102,7 +106,7 @@ function main() { } if (flag("--list")) { for (const s of selectedScenarios()) { - const ab = s.post_only ? "post-only" : s.cohort === "resized" ? "A/B" : "post-only" + const ab = s.post_only ? "post-only" : hasBaseline(s) ? "A/B" : "post-only" console.log(`${s.id}\t${s.cohort}\t${ab}\t${s.key_behavior}\t${s.read_only ? "ro" : "live"}`) } return