diff --git a/.changeset/fixture-quality.md b/.changeset/fixture-quality.md new file mode 100644 index 00000000..83ea1f3b --- /dev/null +++ b/.changeset/fixture-quality.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +The rule-authoring recipes now tell the agent where a rule's test cases should come from. `create-sg-rule`, `create-vale-rule`, `create-runtime-rule`, and `improve-rule --anonymous` have it copy cases from real code in the repository before inventing any, and name the cases it invented in its report. They also have it read the linters `detect --json` finds and write cases those linters would accept. Apart from the construct the rule checks, cases stay in the repository's idiom, with no placeholder types such as `Record` and no nesting a reviewer would send back. `create-remote-rule` and `improve-rule` apply the same standard to the cases and references they send to the generator. `verify` is unchanged and does not judge fixture style. diff --git a/.taskless/rules/sg/no-pii-in-telemetry/.tests/no-pii-in-telemetry-20260824-test.yml b/.taskless/rules/sg/no-pii-in-telemetry/.tests/no-pii-in-telemetry-20260824-test.yml deleted file mode 100644 index 4e745f2e..00000000 --- a/.taskless/rules/sg/no-pii-in-telemetry/.tests/no-pii-in-telemetry-20260824-test.yml +++ /dev/null @@ -1,25 +0,0 @@ -id: no-pii-in-telemetry -valid: - - | - posthog.capture({ - distinctId, - event: "cli_rule_create", - properties: { cli: xdgUuid, anonymous: false }, - groups: { organization: orgId }, - }); - - | - posthog.identify({ distinctId, properties: { cli: xdgUuid } }); - - | - const user = { email: account.email, displayName: account.name }; -invalid: - - | - posthog.capture({ - distinctId, - event: "cli_auth_login_completed", - properties: { cli: xdgUuid, email: account.email }, - }); - - | - posthog.identify({ - distinctId, - properties: { cli: xdgUuid, displayName: account.name }, - }); diff --git a/.taskless/rules/sg/no-pii-in-telemetry/.tests/no-pii-in-telemetry-20261014-test.yml b/.taskless/rules/sg/no-pii-in-telemetry/.tests/no-pii-in-telemetry-20261014-test.yml new file mode 100644 index 00000000..557d1b59 --- /dev/null +++ b/.taskless/rules/sg/no-pii-in-telemetry/.tests/no-pii-in-telemetry-20261014-test.yml @@ -0,0 +1,59 @@ +id: no-pii-in-telemetry +valid: + - | + posthog.identify({ + distinctId, + properties: { + cli: anonymousId, + cliVersion: CLI_VERSION, + scaffoldVersion, + ghOwner, + ...dimensions, + }, + }); + - | + telemetry.capture("cli_report_published", { + ...tierCounts(report), + withheldFixtures: report.redaction.withheldFixtures, + }); + - | + posthog.groupIdentify({ + groupType: "organization", + groupKey: String(orgSubject), + }); + - | + auth = { + user: whoami.user, + email: whoami.email, + orgs: whoami.orgs.map((o) => o.name), + }; +invalid: + - | + posthog.identify({ + distinctId, + properties: { + cli: anonymousId, + cliVersion: CLI_VERSION, + scaffoldVersion, + email, + ...dimensions, + }, + }); + - | + telemetry.capture("cli_authenticated", { email: whoami.email }); + - | + ph.capture({ + distinctId, + event, + properties: { + ...properties, + cli: anonymousId, + displayName: whoami.user, + }, + }); + - | + posthog.groupIdentify({ + groupType: "organization", + groupKey: String(orgSubject), + properties: { email: whoami.email }, + }); diff --git a/.taskless/rules/sg/no-pii-in-telemetry/no-pii-in-telemetry.yml b/.taskless/rules/sg/no-pii-in-telemetry/no-pii-in-telemetry.yml index a0b537df..238c6ece 100644 --- a/.taskless/rules/sg/no-pii-in-telemetry/no-pii-in-telemetry.yml +++ b/.taskless/rules/sg/no-pii-in-telemetry/no-pii-in-telemetry.yml @@ -9,11 +9,16 @@ note: | See .conventions/posthog.md — Privacy. rule: - kind: pair - has: - field: key - kind: property_identifier - regex: '^(email|userEmail|displayName|fullName|firstName|lastName|username)$' + any: + - kind: pair + has: + field: key + kind: property_identifier + regex: '^(email|userEmail|displayName|fullName|firstName|lastName|username)$' + # Shorthand (`{ email }`) is its own node kind, not a `pair`. The telemetry + # call sites write most properties this way (`scaffoldVersion, ghOwner,`). + - kind: shorthand_property_identifier + regex: '^(email|userEmail|displayName|fullName|firstName|lastName|username)$' inside: stopBy: end any: diff --git a/openspec/changes/archive/2026-10-14-fixture-quality/.openspec.yaml b/openspec/changes/archive/2026-10-14-fixture-quality/.openspec.yaml new file mode 100644 index 00000000..49ea85b3 --- /dev/null +++ b/openspec/changes/archive/2026-10-14-fixture-quality/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-14 diff --git a/openspec/changes/archive/2026-10-14-fixture-quality/design.md b/openspec/changes/archive/2026-10-14-fixture-quality/design.md new file mode 100644 index 00000000..c7d847f7 --- /dev/null +++ b/openspec/changes/archive/2026-10-14-fixture-quality/design.md @@ -0,0 +1,70 @@ +## Context + +See proposal.md, under Why, for the motivation. This section covers only what shapes the approach. + +- The authoring recipes are markdown files under `packages/cli/src/agent/`. They are rendered by `src/prompts/recipes.ts` through sprintf variables and embedded into the bundle at build time. `buildVariables` provides every variable on every render, and `prompts.test.ts` pins the full list of variable names. +- Fixture guidance today is per engine and lives in each recipe's own fixture step: + - `create-sg-rule`: step 6, "Write the tests". Step 2 already says "search the codebase for real instances". + - `create-vale-rule`: step 5, "Write the fixtures", which has detailed near-miss guidance for `pass/`. + - `create-runtime-rule`: the "Testing a rule you wrote yourself" section, with directory-shaped cases. + - `improve-rule.anonymous`: step 5, "Update test cases". + - `create-remote-rule` (step 4, "Enrich it") and `improve-rule` (step 5, "Collect supporting references") write no fixtures. They submit cases to the generator. Step 4 already says "Real matches beat invented ones" and reads linter configs, but only to find exclusion patterns. +- `detect --json` already returns `linters[]`, each with a `name` and `evidence` (the config paths). `route` runs detection as its step 1 through `%(DETECT_EVIDENCE)s`. That block has a mechanics-off form for callers that supply the evidence themselves. + +## Goals / Non-Goals + +**Goals:** + +- Write the fixture-quality guidance once and render it into every recipe that writes fixtures, so the recipes cannot drift apart. +- Keep each recipe's engine-specific fixture guidance where it is (Vale near-misses, ast-grep arity and awaited-call cases, runtime case directories). The shared passage adds to that guidance and replaces none of it. + +**Non-Goals:** + +- Any change to `verify`, `validate`, or the fixture file formats. +- The service-side generator's own fixture prompts. Whether it copies submitted `successCases` into fixtures word for word is the generator team's question. +- New `detect` output. `linters[]` already carries the evidence that is needed. + +## Decisions + +### One shared variable, `%(FIXTURE_SOURCES)s`, rendered as a whole block + +The passage is a new `buildVariables` entry, rendered with `indentBlock` like `NAME_DESTINATION`, and dropped into each fixture step as a sub-paragraph. + +- Alternative: copy the paragraph into four recipes. Rejected. Four copies pass the three-copy threshold for factoring out, and the copies would drift the first time one recipe's fixture step is revised. +- Alternative: a separate `agent fixtures` topic that each recipe tells the agent to fetch. Rejected. It costs an extra fetch at exactly the step where an agent is most likely to skip it, and the guidance is short enough to carry inline. + +The block holds only what is engine-agnostic: the source order, the linter read, and the rule about varying only on the rule's axis. Each recipe keeps the word for its buckets (`valid`/`invalid` or `pass`/`fail`). For that reason the passage says "every case" or "the failing bucket", never a key name. + +### Detection reuses `DETECT_EVIDENCE`'s two modes + +With mechanics on, the passage tells the agent to reuse `detect --json` output from routing if it has it, and otherwise to run it. With mechanics off, it uses the supplied-evidence wording, the same split `DETECT_EVIDENCE` makes, so a caller without the binary is never told to run it. Reusing earlier output matters because the common path is `route` → `create-*-rule` in one task, and running detection twice adds nothing. + +### Linters are read, never run + +The passage asks the agent to read the configuration of each detected linter that covers the rule's language, and to write passing fixtures that linter would accept. It tells the agent not to run a linter or formatter on a fixture. + +- Alternative: run the linter on every snippet. Rejected. A fixture is a fragment, and linting a fragment produces unused-variable and unresolved-import errors that have nothing to do with style. An agent fixing those pads the fixture with scaffolding that hides the case, which is the readability problem this change exists to fix. +- Alternative: run linters only on fixtures that are complete files. This was in the first draft and is rejected after the 4.3 trial. A tool's output becomes something to satisfy, and satisfying it is an open-ended loop with no rule-level payoff. The trial also showed the run is unreliable in practice: this repository's ESLint config ignores `.taskless/` entirely, so a run returns an "ignored file" warning that reads like a pass. + +### Failing cases come from the nearest real code + +A rule usually exists because the repository should not contain what it flags, so the failing bucket often has no real instance to copy. The passage directs the agent to take the nearest real code and change only the construct the rule flags, and counts only a case built from nothing as invented. Without this, "copy before you invent" marks every failing case as invented, and the disclosure stops carrying information. The 4.3 trial showed the payoff: a copied call site, written in the repository's shorthand style (`{ email }`), exposed a form the rule did not match. + +### The awaited-generic case is conditional on the repository + +`create-sg-rule` step 6 asked for an awaited generic call among the `invalid:` cases whenever a rule keys on a callee. That conflicts with "every case is code the repository would accept" for a callee the repository never awaits. The step now asks for that form only for a callee the repository does await with a type argument. + +### Submitting recipes get a short pointer, not the block + +`create-remote-rule` and `improve-rule` get one added bullet in their enrich and reference steps: submitted cases follow the same sourcing and idiom bar, and passing cases should be ones the detected linters would accept. They do not get the full block, whose step about writing fixture files does not apply to them. One short bullet in each of two places stays under the three-copy threshold. + +## Risks / Trade-offs + +- [The block's indentation differs between recipes] → `indentBlock` takes one indent. All four fixture steps sit inside a numbered list at three spaces, except the runtime section. The implementation checks each placement in the rendered output and gives the runtime recipe its own placement if it needs one. +- [The recipe parity tests compare rendered output against source] → Changing recipe text needs no test updates beyond the variable list, but running prettier over `src/` rewrites recipe markdown. Format only the files changed, by path. +- [An agent ignores the guidance anyway] → This is a recipe change with no gate behind it, by decision. Before archiving, check an authored rule by hand against a repository with ESLint, ideally the user's metric-counter rule. +- [The Vale near-miss guidance and "idiomatic" seem to pull in different directions] → They don't. A near-miss is idiomatic prose that varies on the rule's axis. The passage says so explicitly, so an agent does not read "idiomatic" as "correct prose, which proves nothing". + +## Open Questions + +- Could the user's actual rule and fixture file serve as a before/after example in the change's verification? This doesn't affect the approach. diff --git a/openspec/changes/archive/2026-10-14-fixture-quality/proposal.md b/openspec/changes/archive/2026-10-14-fixture-quality/proposal.md new file mode 100644 index 00000000..fd2a40ba --- /dev/null +++ b/openspec/changes/archive/2026-10-14-fixture-quality/proposal.md @@ -0,0 +1,33 @@ +## Why + +A user reviewed a rule an agent authored with Taskless. The rule was correct, but its fixtures were not code their team would accept. A `valid:` case used `Record` and deeply nested call chains, and broke their own ESLint config. In their words, "the safer code fails while the bad code passes". People read fixtures as examples of the codebase's style, so fixtures like these lose the reader's trust before anyone looks at what the rule checks. + +The recipes allow this. `create-sg-rule` asks for fixtures "drawn from real code where you can" and says nothing else about what a fixture should look like. None of the authoring recipes runs `detect` before writing fixtures, so none of them knows which linters the repository enforces. + +## What Changes + +- Every recipe in which the agent writes fixtures gets one shared fixture-quality passage. Those recipes are `create-sg-rule`, `create-vale-rule`, `create-runtime-rule`, and `improve-rule.anonymous`, which adds cases to an existing test file. The passage covers three things: + - **Real code first.** Fixtures are copied from real code in the repository and trimmed down to the case. When the repository has no instance, which is usual for the failing cases of a rule that forbids something, the agent takes the nearest real code and changes only what the rule flags. Invented snippets are the last resort, and the report says which cases were invented. + - **Respect the repository's linters.** The agent reads `linters[]` from `taskless detect --json`, reuses that output if routing already produced it, and reads each detected linter's config. It writes cases in both buckets that those linters would accept as source. It reads the linters and never runs a linter or formatter on a fixture: a fixture is a fragment, and fixing what a tool reports about a fragment is an endless loop. + - **A fixture differs only where the rule looks.** Apart from the thing the rule checks, both buckets are plain code in the repository's idiom. No placeholder types such as `Record` or `any`, and no nesting a reviewer would send back. Near-misses stay near-misses along the rule's own axis. +- `create-remote-rule` and `improve-rule` (logged in) apply the same quality bar to the `successCases`/`failureCases` and references they send to the generator, because the generator works from those inputs. +- **Out of scope:** `verify` does not lint fixtures and does not fail a rule over fixture style. Changes to the service-side generator are also out of scope. + +**Delivery shape: single PR.** This changes recipe prose, adds one shared block to `src/prompts/recipes.ts`, and adds the tests for that block. The whole change fits in one reviewable diff, and none of it is unsafe on its own. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `cli-agent-authoring`: adds a requirement covering where fixtures come from, how the repository's linters constrain them, and the scope rule for what a fixture may vary. This is an ADDED requirement. No existing requirement is rewritten. + +## Impact + +- `packages/cli/src/agent/create-sg-rule.md`, `create-vale-rule.md`, `create-runtime-rule.md`, `improve-rule.anonymous.md`: the fixture steps pull in the shared passage. +- `packages/cli/src/agent/create-remote-rule.md`, `improve-rule.md`: the enrich and reference steps apply the same bar to what is submitted. +- `packages/cli/src/prompts/recipes.ts`: a new shared variable for the passage. With mechanics off, it falls back to the existing supplied-evidence wording. +- Recipe tests (`prompts.test.ts`, `recipe-cross-references.test.ts`) and a changeset. Recipes ship inside the CLI bundle, so this is a `patch` release note. diff --git a/openspec/changes/archive/2026-10-14-fixture-quality/specs/cli-agent-authoring/spec.md b/openspec/changes/archive/2026-10-14-fixture-quality/specs/cli-agent-authoring/spec.md new file mode 100644 index 00000000..63f76191 --- /dev/null +++ b/openspec/changes/archive/2026-10-14-fixture-quality/specs/cli-agent-authoring/spec.md @@ -0,0 +1,67 @@ +## ADDED Requirements + +### Requirement: Fixtures are real, repo-idiomatic code that varies only where the rule looks + +Every authoring recipe in which the agent writes or extends fixtures SHALL instruct the agent to: + +1. Take fixtures from real code in the repository, trimmed down to the case, before inventing any. Where the repository has no instance of a case, take the nearest real code and change only the construct the rule flags, borrowing names and values from elsewhere in the repository where needed. A fixture invented outright is allowed only when nothing nearby exists, and the agent's report SHALL say which fixtures were invented. +2. Read the repository's detected linters before writing fixtures. The agent reuses `detect --json` output already in hand, or runs it otherwise. It writes fixtures in both buckets that those linters, as configured, would accept as source, including where their configuration ignores the rules directory. The agent reads the linters' configuration and SHALL NOT be directed to run a linter or formatter on a fixture. +3. Keep every fixture, in both buckets, in the repository's idiom everywhere except the construct the rule checks. A fixture SHALL NOT use placeholder types, catch-all types, or nesting that the repository's own conventions would reject. + +A recipe through which the agent submits cases to the generator, instead of writing fixtures itself, SHALL apply the same three instructions to the cases and references it submits. + +Fixtures are read as examples of the repository's style. A `valid:` case that the repository's own linter rejects reads as Taskless approving bad code, and a reader who sees that stops trusting the rule before checking what it actually matches. + +Enforcing this is the recipe's job. `verify` SHALL NOT lint fixtures and SHALL NOT fail a rule over fixture style. + +#### Scenario: Real instances are used before invented ones + +- **WHEN** an agent follows an authoring recipe and the repository contains code exhibiting the case +- **THEN** the recipe SHALL direct the agent to take that fixture from the repository rather than inventing one + +#### Scenario: A case with no real instance is adapted from nearby code + +- **WHEN** the repository contains no instance of a failing case, as is usual for a rule that forbids something +- **THEN** the recipe SHALL direct the agent to take the nearest real code and change only the construct the rule flags + +#### Scenario: Invented fixtures are disclosed + +- **WHEN** the repository has no instance of a case and the agent invents a fixture for it +- **THEN** the recipe SHALL direct the agent to name the invented fixtures in its report + +#### Scenario: Detected linters shape every case + +- **WHEN** `detect --json` reports one or more linters +- **THEN** the recipe SHALL direct the agent to read each one's configuration before writing fixtures +- **AND** to write fixtures in both buckets that those linters would accept + +#### Scenario: Linters are read, not run, against fixtures + +- **WHEN** the recipe directs the agent to respect the repository's linters +- **THEN** it SHALL direct the agent to read their configuration +- **AND** SHALL direct the agent not to run a linter or formatter on a fixture + +#### Scenario: Detection is not repeated when routing already ran it + +- **WHEN** the agent already has `detect --json` output from routing in the same task +- **THEN** the recipe SHALL let the agent reuse it rather than run detection again + +#### Scenario: A fixture varies only on the rule's axis + +- **WHEN** the agent writes a near-miss `pass` fixture next to a `fail` fixture +- **THEN** the recipe SHALL direct that the two differ only in the construct the rule checks + +#### Scenario: Placeholder types are not used to make a fixture compile + +- **WHEN** a fixture needs a type, value, or call that is not what the rule checks +- **THEN** the recipe SHALL direct the agent to use one the repository itself would use, not a placeholder or catch-all type + +#### Scenario: Submitted cases meet the same bar + +- **WHEN** an agent builds a generation or improvement request for the service +- **THEN** the recipe SHALL apply the same sourcing, linter, and idiom instructions to the submitted cases and references + +#### Scenario: verify does not judge fixture style + +- **WHEN** `verify` runs a rule whose `pass` fixture would fail the repository's linter +- **THEN** `verify` SHALL report on the rule's behavior alone, and SHALL NOT fail the rule over that fixture's style diff --git a/openspec/changes/archive/2026-10-14-fixture-quality/tasks.md b/openspec/changes/archive/2026-10-14-fixture-quality/tasks.md new file mode 100644 index 00000000..f75be9d6 --- /dev/null +++ b/openspec/changes/archive/2026-10-14-fixture-quality/tasks.md @@ -0,0 +1,30 @@ +## 1. Shared fixture passage + +- [x] 1.1 Add a `FIXTURE_SOURCES` block to `buildVariables` in `packages/cli/src/prompts/recipes.ts`, rendered through `indentBlock`. It covers real code first, disclosing invented fixtures, reading the detected linters, and varying only on the rule's axis, with no placeholder types or nesting the repo would reject. With mechanics on it says to reuse or run `detect --json`; with mechanics off it uses the supplied-evidence wording. Verify by adding `FIXTURE_SOURCES` to the variable list in `test/prompts.test.ts` and running that test. +- [x] 1.2 Add `prompts.test.ts` cases asserting that, with mechanics on, the rendered block names the `detect --json` invocation, and that with `mechanics: false` it names no command. Verify with `pnpm --filter @taskless/cli test prompts`. + +## 2. Recipes that write fixtures + +- [x] 2.1 Insert `%(FIXTURE_SOURCES)s` into the fixture step of `create-sg-rule.md` (step 6), replacing the weaker "drawn from real code where you can" phrasing. Verify the rendered `agent create-sg-rule` output keeps its numbered-list indentation. +- [x] 2.2 Insert it into `create-vale-rule.md` step 5, and state that a `pass/` near-miss is idiomatic prose varying only on the rule's axis. Verify the rendered output and that the existing near-miss bullets are unchanged. +- [x] 2.3 Insert it into `create-runtime-rule.md`'s "Testing a rule you wrote yourself" section, with whatever placement that section's indentation needs. Verify the rendered output. +- [x] 2.4 Insert it into `improve-rule.anonymous.md` step 5, so added cases follow the same bar. Verify the rendered output. + +## 3. Recipes that submit cases + +- [x] 3.1 Add one bullet to `create-remote-rule.md` step 4: submitted success and failure cases follow the same sourcing and idiom bar, and success cases should be ones the detected linters would accept. Verify the rendered output. +- [x] 3.2 Add the same bullet to `improve-rule.md` step 5 for the references. Verify the rendered output. + +## 4. Release and checks + +- [x] 4.1 Add a `patch` changeset for `@taskless/cli` that describes the recipe change from a consumer's point of view. Verify that `.changeset/` holds exactly one new file. +- [x] 4.2 Run prettier only on the changed `.ts` and changeset files, by explicit path. Never run it on `src/agent/*.md` or over `src/`, because it rewrites the recipe markdown. Then run `pnpm typecheck` and `pnpm lint`. Both must pass. +- [x] 4.3 Run `pnpm build`, then author one sg rule with `pnpm cli agent create-sg-rule` in a scratch repo that has an ESLint config. Confirm the fixtures come from real code, the passing cases pass ESLint, and the report names any invented fixture. Record the result in the PR body. + +## 5. Revision after the 4.3 trial + +- [x] 5.1 Revise `FIXTURE_SOURCES`: failing cases with no real instance are adapted from the nearest real code, not counted as invented; linters are read and never run, and neither are formatters; only linters covering the rule's language apply; real code is justified by coverage as well as credibility. Verify the prompts tests pass. +- [x] 5.2 Make `create-sg-rule` step 6's awaited-generic case conditional on a callee the repository actually awaits with a type argument. Verify the rendered recipe. +- [x] 5.3 Re-run the 4.3 trial on `no-pii-in-telemetry`, restored from `origin/main`, with a fresh subagent and the revised recipe. Record the result in the PR body. +- [x] 5.4 From the second trial: the linter bullet covers every case, not only the passing bucket; linters apply to fixtures as source even when their configuration ignores the rules directory; borrowing names and values from elsewhere in the repository is allowed when adapting a failing case. Verify the rendered recipe and prompts tests. +- [x] 5.5 Commit the `no-pii-in-telemetry` shorthand fix and the trial's fixtures. Verify `verify` passes, the `origin/main` rule fails the new fixtures, and `check` finds no hits in the repository. diff --git a/openspec/specs/cli-agent-authoring/spec.md b/openspec/specs/cli-agent-authoring/spec.md index 6ca787f5..afad256a 100644 --- a/openspec/specs/cli-agent-authoring/spec.md +++ b/openspec/specs/cli-agent-authoring/spec.md @@ -175,3 +175,69 @@ The remote generation recipe SHALL state the GitHub-owner requirement as a preco - **WHEN** the recipe's `## Errors` section is read - **THEN** it SHALL list every code the three no-remote populations can produce + +### Requirement: Fixtures are real, repo-idiomatic code that varies only where the rule looks + +Every authoring recipe in which the agent writes or extends fixtures SHALL instruct the agent to: + +1. Take fixtures from real code in the repository, trimmed down to the case, before inventing any. Where the repository has no instance of a case, take the nearest real code and change only the construct the rule flags, borrowing names and values from elsewhere in the repository where needed. A fixture invented outright is allowed only when nothing nearby exists, and the agent's report SHALL say which fixtures were invented. +2. Read the repository's detected linters before writing fixtures. The agent reuses `detect --json` output already in hand, or runs it otherwise. It writes fixtures in both buckets that those linters, as configured, would accept as source, including where their configuration ignores the rules directory. The agent reads the linters' configuration and SHALL NOT be directed to run a linter or formatter on a fixture. +3. Keep every fixture, in both buckets, in the repository's idiom everywhere except the construct the rule checks. A fixture SHALL NOT use placeholder types, catch-all types, or nesting that the repository's own conventions would reject. + +A recipe through which the agent submits cases to the generator, instead of writing fixtures itself, SHALL apply the same three instructions to the cases and references it submits. + +Fixtures are read as examples of the repository's style. A `valid:` case that the repository's own linter rejects reads as Taskless approving bad code, and a reader who sees that stops trusting the rule before checking what it actually matches. + +Enforcing this is the recipe's job. `verify` SHALL NOT lint fixtures and SHALL NOT fail a rule over fixture style. + +#### Scenario: Real instances are used before invented ones + +- **WHEN** an agent follows an authoring recipe and the repository contains code exhibiting the case +- **THEN** the recipe SHALL direct the agent to take that fixture from the repository rather than inventing one + +#### Scenario: A case with no real instance is adapted from nearby code + +- **WHEN** the repository contains no instance of a failing case, as is usual for a rule that forbids something +- **THEN** the recipe SHALL direct the agent to take the nearest real code and change only the construct the rule flags + +#### Scenario: Invented fixtures are disclosed + +- **WHEN** the repository has no instance of a case and the agent invents a fixture for it +- **THEN** the recipe SHALL direct the agent to name the invented fixtures in its report + +#### Scenario: Detected linters shape every case + +- **WHEN** `detect --json` reports one or more linters +- **THEN** the recipe SHALL direct the agent to read each one's configuration before writing fixtures +- **AND** to write fixtures in both buckets that those linters would accept + +#### Scenario: Linters are read, not run, against fixtures + +- **WHEN** the recipe directs the agent to respect the repository's linters +- **THEN** it SHALL direct the agent to read their configuration +- **AND** SHALL direct the agent not to run a linter or formatter on a fixture + +#### Scenario: Detection is not repeated when routing already ran it + +- **WHEN** the agent already has `detect --json` output from routing in the same task +- **THEN** the recipe SHALL let the agent reuse it rather than run detection again + +#### Scenario: A fixture varies only on the rule's axis + +- **WHEN** the agent writes a near-miss `pass` fixture next to a `fail` fixture +- **THEN** the recipe SHALL direct that the two differ only in the construct the rule checks + +#### Scenario: Placeholder types are not used to make a fixture compile + +- **WHEN** a fixture needs a type, value, or call that is not what the rule checks +- **THEN** the recipe SHALL direct the agent to use one the repository itself would use, not a placeholder or catch-all type + +#### Scenario: Submitted cases meet the same bar + +- **WHEN** an agent builds a generation or improvement request for the service +- **THEN** the recipe SHALL apply the same sourcing, linter, and idiom instructions to the submitted cases and references + +#### Scenario: verify does not judge fixture style + +- **WHEN** `verify` runs a rule whose `pass` fixture would fail the repository's linter +- **THEN** `verify` SHALL report on the rule's behavior alone, and SHALL NOT fail the rule over that fixture's style diff --git a/packages/cli/src/agent/create-remote-rule.md b/packages/cli/src/agent/create-remote-rule.md index 96956ee5..1573d77f 100644 --- a/packages/cli/src/agent/create-remote-rule.md +++ b/packages/cli/src/agent/create-remote-rule.md @@ -1,4 +1,4 @@ -# Topic: create-remote-rule (CLI v%(CLI_VERSION)s / topic v6) +# Topic: create-remote-rule (CLI v%(CLI_VERSION)s / topic v7) ## You are here This is `create-remote-rule`. It helps you have the Taskless service @@ -103,6 +103,12 @@ Two ways to legitimately be here: - Search the codebase for real instances and show the user what you found. Real matches beat invented ones. - Ask for further success and failure cases, especially near-misses. + - Hold every case to the bar a fixture meets, since the generator + works from what you send. Copy cases from the + repository rather than inventing them. Write success cases that the + linters in `detect --json` would accept. Outside the construct the + rule checks, keep each case in the repository's idiom, with no + placeholder types and no nesting a reviewer would send back. - Read `.gitignore`, the linter configs, and `tsconfig.json`'s `exclude` for exclusion patterns the repo already believes in. Present them as proposed defaults rather than applying them. diff --git a/packages/cli/src/agent/create-runtime-rule.md b/packages/cli/src/agent/create-runtime-rule.md index b95925ce..1f5bfd3f 100644 --- a/packages/cli/src/agent/create-runtime-rule.md +++ b/packages/cli/src/agent/create-runtime-rule.md @@ -1,4 +1,4 @@ -# Topic: create-runtime-rule (CLI v%(CLI_VERSION)s / topic v4) +# Topic: create-runtime-rule (CLI v%(CLI_VERSION)s / topic v5) ## You are here This is `create-runtime-rule`. It helps you write a runtime rule: a @@ -131,6 +131,8 @@ not run, names the flag, and exits 0. That is neither a pass nor a failure, and it is deliberately not a failure: nothing the holder of an unblessed rule can do would make a failure green. +%(FIXTURE_SOURCES)s + Both buckets have to be populated for the rule to pass, and every case has to match at least one capture. A case the captures do not match never reaches `check.ts`, and is reported as a defect in the case rather diff --git a/packages/cli/src/agent/create-sg-rule.md b/packages/cli/src/agent/create-sg-rule.md index 4bd2d575..31267f54 100644 --- a/packages/cli/src/agent/create-sg-rule.md +++ b/packages/cli/src/agent/create-sg-rule.md @@ -1,4 +1,4 @@ -# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v9) +# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v10) ## You are here This is `create-sg-rule`. It helps you write an ast-grep rule: a check @@ -113,11 +113,12 @@ whole rule. 6. **Write the tests.** Write `.taskless/rules/sg//.tests/-YYYYMMDD-test.yml` with the matching - `id` field plus `valid` and `invalid` arrays, at least two of each, - drawn from real code where you can. The `id` must match the rule's - `id` so ast-grep test filtering pairs them. These paths and this - shape are the same ones the service writes; do not invent a different - layout. + `id` field plus `valid` and `invalid` arrays, at least two of each. + The `id` must match the rule's `id` so ast-grep test filtering pairs + them. These paths and this shape are the same ones the service + writes; do not invent a different layout. + + %(FIXTURE_SOURCES)s **Both arrays must be non-empty, and `verify` fails the rule if either one is.** An empty `invalid:` is not a neutral starting point to fill @@ -129,8 +130,11 @@ whole rule. Where the rule has an arity boundary (the separator trap under Quirks), put a case on each side of it. A pattern that starts at two arguments when it was meant to start at one passes a test suite whose - fixtures all have two. Where the rule keys on a callee, put an - awaited generic call among the `invalid:` cases. + fixtures all have two. Where the rule keys on a callee that the + repository awaits with a type argument (`await f(...)`), put that + form among the `invalid:` cases. Write it only for such a callee: an + awaited generic call to a function the repository never awaits is + not code it would accept, and the case would teach the wrong shape. 7. **Run the feedback loop.** Both commands take the rule's directory as their argument: diff --git a/packages/cli/src/agent/create-vale-rule.md b/packages/cli/src/agent/create-vale-rule.md index aeaa3027..4440695d 100644 --- a/packages/cli/src/agent/create-vale-rule.md +++ b/packages/cli/src/agent/create-vale-rule.md @@ -1,4 +1,4 @@ -# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v16) +# Topic: create-vale-rule (CLI v%(CLI_VERSION)s / topic v17) ## You are here This is `create-vale-rule`. It helps you write a Vale rule: a check over @@ -863,6 +863,8 @@ it. fixture under a `[*.md]` matcher is never linted, so the `fail/` document silently passes. + %(FIXTURE_SOURCES)s + **The `pass/` bucket is not "correct prose".** Correct prose proves nothing. The rule was never going to fire on it. Fill it with the near-misses that would catch an over-broad pattern. What counts as a diff --git a/packages/cli/src/agent/improve-rule.anonymous.md b/packages/cli/src/agent/improve-rule.anonymous.md index 52faddab..c9741747 100644 --- a/packages/cli/src/agent/improve-rule.anonymous.md +++ b/packages/cli/src/agent/improve-rule.anonymous.md @@ -1,4 +1,4 @@ -# Topic: improve-rule (anonymous) (CLI v%(CLI_VERSION)s / topic v3) +# Topic: improve-rule (anonymous) (CLI v%(CLI_VERSION)s / topic v4) ## Goal Iterate on an existing ast-grep rule **locally** without contacting @@ -40,6 +40,8 @@ check it with `validate` and `verify` in a feedback loop. `valid:` list and the new false-negative examples to the `invalid:` list in `.taskless/rules/sg//.tests/-*.yml`. + %(FIXTURE_SOURCES)s + 6. **Run the feedback loop.** Run: ``` %(TASKLESS_CLI)s verify .taskless/rules/sg/ --json diff --git a/packages/cli/src/agent/improve-rule.md b/packages/cli/src/agent/improve-rule.md index 842c664f..b6994260 100644 --- a/packages/cli/src/agent/improve-rule.md +++ b/packages/cli/src/agent/improve-rule.md @@ -1,4 +1,4 @@ -# Topic: improve-rule (CLI v%(CLI_VERSION)s / topic v8) +# Topic: improve-rule (CLI v%(CLI_VERSION)s / topic v9) ## Goal Iterate on an existing Taskless rule. The CLI submits the user's @@ -67,6 +67,12 @@ If the user wants the local-only flow (no API call), fetch Each reference is `{ filename: string, content: string }`. Multiple references are an array. + Prefer references copied from the repository over invented ones, and + name any you invented. Hold them to the repository's idiom outside + the construct the rule checks. A not-flagged reference should be + code the linters in `detect --json` would accept, with no placeholder + types and no nesting a reviewer would send back. + 6. **Confirm the request.** Summarize the rule, the guidance, and the references before submitting. diff --git a/packages/cli/src/prompts/recipes.ts b/packages/cli/src/prompts/recipes.ts index 47ecab84..86fcde68 100644 --- a/packages/cli/src/prompts/recipes.ts +++ b/packages/cli/src/prompts/recipes.ts @@ -338,6 +338,59 @@ A destination that is not a runnable command is a category, and a category is not an answer.`; } +/** + * Where an authoring recipe's fixtures come from, shared by every recipe that + * has the agent write them. + * + * One block rather than a paragraph per recipe because four recipes write + * fixtures and the guidance is engine-agnostic; each recipe keeps its own + * engine-specific cases (Vale near-misses, sg arity) beside it. Bucket names + * stay out of it for the same reason: `valid` in sg, `pass/` everywhere else. + * + * Detection is reused before it is re-run because the common path is `route` + * then `create-*-rule` in one task, and `route` has already read the linters. + * With mechanics off the caller supplied that evidence and no command is named. + * + * The linters are READ, never run. Running a linter or formatter over a + * fixture invites a loop of fixing what it reports about the fixture rather + * than about the rule, and a fragment can never come back clean. + */ +function fixtureSourcesBlock(invocation: string | undefined): string { + const detection = + invocation === undefined + ? "Use the `linters` list in the detection evidence the caller supplies." + : `Use the \`linters\` list from \`detect --json\`: reuse the output if + routing already ran it, otherwise run \`${invocation} detect --json\`.`; + return `**Where the cases come from.** Readers take fixtures as examples of +this repository's style, so a case the repository would reject in review +costs the rule its credibility however correct the rule is. Real code +also finds what the rule misses: invented cases follow your idea of how +the construct is written, real ones follow how it is actually written. + +- **Copy before you invent.** Take each case from real code or prose in + the repository, trimmed to what the case needs, keeping its names. + A rule usually exists because the repository should not contain what + it flags, so the failing bucket often has no real instance. Then take + the nearest real code and change only the construct the rule flags, + borrowing names and values from elsewhere in the repository where the + code has no place for it. Invent a case outright only when nothing + nearby exists, and name the invented cases in your report. +- **Write every case to the repository's linters.** + ${detection} + For each linter that covers the rule's language, read its + configuration files among the \`evidence\` paths, and write cases it + would accept as source, even where its configuration ignores the rules + directory. Read them, do not run them: do not run a linter + or formatter on a fixture. Fixing what a tool reports about a fixture + is a loop that never ends and improves nothing about the rule. +- **Vary only where the rule looks.** Outside the construct the rule + checks, every case in both buckets is code the repository would + accept: no placeholder or catch-all types such as + \`Record\` or \`any\`, and no nesting a reviewer would + send back. A near-miss is still idiomatic. It differs from its failing + twin only on the rule's axis.`; +} + /** * Indent every line after the first, so a multi-line block substituted into an * indented `%(KEY)s` is still the markdown unit it replaces. @@ -565,6 +618,13 @@ export function buildVariables( ), " " ), + // Substituted into a numbered fixture step, so indented to its body. + FIXTURE_SOURCES: indentBlock( + fixtureSourcesBlock( + options.mechanics === false ? undefined : resolveInvocation(options) + ), + " " + ), // Three steps, in descending order of how much the resolver actually // knows: the caller was told how the CLI was launched; the build is a // nightly/dev/self that knows what it is; nobody knows, so ask the agent. diff --git a/packages/cli/test/prompts.test.ts b/packages/cli/test/prompts.test.ts index 19460b2b..255b6f4c 100644 --- a/packages/cli/test/prompts.test.ts +++ b/packages/cli/test/prompts.test.ts @@ -183,6 +183,7 @@ describe("the CLI invocation variable", () => { "AST_GREP_VERSION", "CLI_VERSION", "DETECT_EVIDENCE", + "FIXTURE_SOURCES", "HOST_TOOLS", "LOGIN_EVIDENCE", "NAME_DESTINATION", @@ -716,3 +717,40 @@ describe("Vale directives never reach a reader", () => { } }); }); + +describe("fixture sources", () => { + // The recipes in which the agent writes fixtures itself. A recipe that only + // submits cases to the generator carries its own sentence instead. + const FIXTURE_WRITERS = [ + "create-sg-rule", + "create-vale-rule", + "create-runtime-rule", + ] as const; + + it.each(FIXTURE_WRITERS)("%s carries the shared passage", (topic) => { + const rendered = getPrompt(topic); + expect(rendered).toContain("**Where the cases come from.**"); + expect(rendered).toContain("**Copy before you invent.**"); + }); + + it("the anonymous improve-rule carries it, since it adds cases itself", () => { + const rendered = getRecipe("improve-rule", { anonymous: true }); + expect(rendered).toContain("(anonymous)"); + expect(rendered).toContain("**Where the cases come from.**"); + }); + + it("names the detect command when the CLI is available", () => { + const rendered = getPrompt("create-sg-rule", { + invocation: "pnpm dlx @taskless/cli@latest", + }); + expect(rendered).toContain( + "otherwise run `pnpm dlx @taskless/cli@latest detect --json`" + ); + }); + + it("names no command when the caller supplies the evidence", () => { + const rendered = getPrompt("create-sg-rule", { mechanics: false }); + expect(rendered).toContain("detection evidence the caller supplies"); + expect(rendered).not.toContain("otherwise run"); + }); +});