#657 Add test-structure discipline to agent guidance and review pipeline

packages/agents/content/_partials/test-structure-audit-checklist.md

@@ -0,0 +1,7 @@
+Before saving a spec file, scan adjacent tests for shared-setup runs.
+
+- **Parameterize** with `it.each` when the variation is small and the test body is structurally identical.
+- **Extract a named helper** when the variation is bigger, when shared setup should disappear into a helper signature, or when assertion shapes differ per case.
+- **Check the assertions, too.** When the rule is a count or predicate, specific-fixture-label assertions should not repeat per row. Assert specific data flow _once_ in a focused test; assert structurally for the rest.
+
+Three or more `it` blocks with identical setup and a single varying input is the most common failure mode in interactive sessions. That material belongs behind a helper or a parameterized table, not in N copies of the same shape.

packages/agents/content/skills/code-patterns/SKILL.md

@@ -60,7 +60,10 @@ Follow the naming rules in [naming-conventions.md](../_data/naming-conventions.m
 
 ### Test structure
 
-- **Use parameterized tests** with `it.each([...])` to avoid verbose, repetitive test cases
+Rules live in [`testing-conventions/SKILL.md`](../testing-conventions/SKILL.md): parameterization, helpers, and signal-to-noise as the bar for both.
+
+### Test organization
+
 - **Use function/class reference as describe argument** - `describe(myFunction, ...)` instead of `describe('myFunction', ...)`
 - **Place tests in `__tests__` directory** as sibling to the file being tested
 

packages/agents/content/skills/testing-conventions/SKILL.md

@@ -67,6 +67,30 @@ Comment discipline applies to test files the same as to source. The full rule se
 
 Everything else is over-commenting. Test names already communicate intent; assertions communicate the check.
 
+## Test structure
+
+Tests should make their variation easy to see. When N adjacent tests differ only in one input but share a wall of identical setup, the reader has to diff three or four nearly-identical render calls to find what's actually being tested. The fix is not "delete things"; it's "factor the shared part out so the variation reads as variation."
+
+This is the same signal-buried-in-noise failure mode that [`comment-discipline.md`](../_data/comment-discipline.md) addresses on the comment side. Different mechanism, same principle.
+
+### Rules
+
+1. **Test bodies should be mostly variation and assertion, not setup.** Any prop, fixture, or boilerplate identical across N adjacent tests is noise. Factor it into a default-bearing helper, or collapse the tests into a single parameterized test where the variation reads as a table.
+2. **Use `it.each` when N adjacent tests differ only in a small set of inputs and the body is structurally identical.** Use a named helper when the variation is bigger, when shared setup should disappear into a helper signature, or when assertion shapes differ per case.
+3. **Test the rule, not the data.** Prefer counts, predicates, and structural assertions (`expect(getVisibleChipCount()).toBe(N)`, `expect(queryPillElement()).toBeInTheDocument()`) over enumerating specific fixture labels in every row. Assert specific data flow _once_, in a focused test, not per row of a table.
+
+### Diagnostic
+
+Before writing a third test in the same `describe` block, scan the previous two: How many tokens does a reader have to diff to find what's actually different between them? If the answer is more than a handful, the signal is buried. Parameterize the trio or extract a helper before continuing.
+
+### When N copies are right
+
+The smell is shared _setup with one variable_, not shared _shape with different intents_. Three tests that read as genuinely distinct behavioral claims should stay as three `it` blocks even if their bodies superficially resemble each other. Do not collapse distinct intents into a table.
+
+### Audit before save
+
+<!-- include: ../../_partials/test-structure-audit-checklist.md / -->
+
 ## Additional patterns
 
 ### Omit "should" from test names

packages/agents/content/subagents/code-simplification-reviewer.md

@@ -90,6 +90,10 @@ Focus exclusively on simplification opportunities in changed code:
   - Domain leaks in shared/common code
   - Inline "what" comments that describe what the code does instead of explaining why a non-obvious decision was made
   - `eslint-disable` rationales that explain the surrounding decision rather than the specific suppression
+- Test-structure violations. See `{platform_home_dir}/skills/testing-conventions/SKILL.md` for the full rule set. Common patterns to flag:
+  - Adjacent tests with near-identical setup where only one input varies (parameterize with `it.each` or extract a helper)
+  - Specific-fixture-label assertions repeated per row when the rule is a count or predicate
+  - Helpers whose parameters the test bodies still re-specify (the abstraction failed to absorb the duplication)
 - Logic that can be consolidated without sacrificing clarity
 
 ### Simplification principles
@@ -125,8 +129,8 @@ See the "Finding references" section in the `review-criteria` skill for path-for
 Classify the overall review into exactly one level (none/low/medium/high) per the `review-criteria` skill. Domain context for this reviewer:
 
 - `none`: Code is already clean and well-structured — no simplification opportunities
-- `low`: Minor opportunities (e.g., a handful of redundant or paraphrasing comments, one unused import)
-- `medium`: Several meaningful simplification opportunities, including file-header-scale comment violations (tutorial headers, repeated conversation memorialization, broad library re-teaching)
+- `low`: Minor opportunities (e.g., a handful of redundant or paraphrasing comments, one or two adjacent-test runs with shared-setup duplication, one unused import)
+- `medium`: Several meaningful simplification opportunities, including file-header-scale comment violations (tutorial headers, repeated conversation memorialization, broad library re-teaching) or pervasive shared-setup duplication across a spec file
 - `high`: Pervasive unnecessary complexity indicating the code needs a simplification pass
 
 ## Output format