Repository navigation
docs(agent): state where a runtime rule's fixtures go - #225
Conversation
|
@claude /review |
|
Claude finished @theCodeDrift's task in 3m 26s —— View job ReviewReview mode: incremental — read 0 prior review thread(s) before reviewing.
Scope: Assessment: the added content is accurate and consistent with the codebase. Checked against:
One finding posted inline ( No other correctness, clarity, or convention issues found. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The recipe gained a whole section it never had — the `.tests/` layout, the directory-per-case shape, and the bucket naming — while its header still said v1. Every other recipe that changed materially bumped: create-sg-rule is v4, create-vale-rule v3, improve-rule v4. The number is maintained by hand, so nothing else would have caught it, and a stale `topic vN` is exactly the failure CLAUDE.md records: an agent followed a recipe from an old `dist/` and authored rules against outdated guidance, missing the revision that documented the field it then got wrong. Deliberately NOT bumping `verify-rule`, whose change in this branch names the fixture buckets inside an existing row. The test there is whether acting on the stale copy is wrong, not whether the text differs: an agent reading the old verify-rule still learns runtime tests are not run locally, which remains true. An agent reading the old create-runtime-rule learns nothing about fixtures at all and invents a layout. Found by the review on #225.
The inline — AI Coding Agent |
`create-runtime-rule` described a rule directory as `captures/*.yml` plus `check.ts` and stopped. An agent authoring a runtime rule was told nothing about `.tests/` at all, and `verify-rule` reported runtime tests as "not run" without naming what it was not running. The shape existed only inside a generator. Now stated in both, with the part that is load-bearing rather than cosmetic: a case is a DIRECTORY whose path is the `root` the harness hands the check, with every `match.file` relative to it, so a case may hold as many files as it needs. A runtime rule exists because its evidence spans more than one file — a layout allowing one file per case could not express the rules the tier is for. That distinction was not academic. The generator's payload builder capped fixture path depth at 3, which admits `pass/case-1/input.ts` and refuses `pass/case-1/src/a.ts`, so the first cross-file fixture would have been rejected as a path-safety error: a security refusal for a legitimate test. The cap was written as a runaway guard and had silently become a shape constraint. They raised it to 8 and recorded which of the two it is. Buckets are `pass`/`fail`, matching Vale's. `valid`/`invalid` are key names inside an ast-grep test YAML and are right for `sg` for exactly one reason — that file is handed to `sg test` verbatim. Nothing hands a runtime fixture to ast-grep, so reusing the word there would make it mean two things, the same collision `captures/` avoids by not being called `matchers/`. Agreed with the cloud team as F1 and F2; their generator writes this shape and the restore endpoint shares the derivation.
The recipe gained a whole section it never had — the `.tests/` layout, the directory-per-case shape, and the bucket naming — while its header still said v1. Every other recipe that changed materially bumped: create-sg-rule is v4, create-vale-rule v3, improve-rule v4. The number is maintained by hand, so nothing else would have caught it, and a stale `topic vN` is exactly the failure CLAUDE.md records: an agent followed a recipe from an old `dist/` and authored rules against outdated guidance, missing the revision that documented the field it then got wrong. Deliberately NOT bumping `verify-rule`, whose change in this branch names the fixture buckets inside an existing row. The test there is whether acting on the stale copy is wrong, not whether the text differs: an agent reading the old verify-rule still learns runtime tests are not run locally, which remains true. An agent reading the old create-runtime-rule learns nothing about fixtures at all and invents a layout. Found by the review on #225.
7f389fb to
8024f3e
Compare
1 participant
Recipe text plus a changeset. No behaviour change.
The gap
create-runtime-ruledescribed a rule directory ascaptures/*.ymlpluscheck.tsand stopped. An agent authoring a runtime rule was told nothing about.tests/.verify-rulereported runtime tests as "not run" without naming what it was not running.So the fixture shape existed only inside the generator — which is the same "described in prose nowhere, implemented in one place" shape this whole alignment has been unpicking.
What is now stated
Cases live in
.tests/pass/and.tests/fail/, and a case is a directory whose path is therootthe harness hands the check, with everymatch.filerelative to it. So a case may hold as many files as it needs.That last part is load-bearing, not cosmetic: a runtime rule exists because its evidence spans more than one file, so a layout allowing one file per case could not express the rules the tier is for.
And it was not academic. The generator's payload builder capped fixture path depth at 3 — which admits
pass/case-1/input.tsand refusespass/case-1/src/a.ts. The first cross-file fixture would have been rejected as a path-safety error: a security refusal for a legitimate test, which is about the most misleading error that shape of bug can produce. The cap had been written as a runaway guard and silently become a shape constraint; two different questions wanting two different numbers. They raised it to 8 and recorded which one it answers.Bucket naming
pass/fail, matching Vale's, notvalid/invalid.valid/invalidare key names inside an ast-grep test YAML, and they are right forsgfor exactly one reason: that file is handed tosg testverbatim. Nothing hands a runtime fixture to ast-grep, so reusing the word there would make it mean two things — the same collisioncaptures/already avoids by not being calledmatchers/.Verification
1055tests, build, and house style clean. Agreed with the cloud team as F1 and F2; their generator writes this shape, and their restore endpoint shares the derivation, so there is one implementation behind the statement rather than two that could disagree.