/refine-tests

name: refine-tests
description: >-
  Reviews existing test code for conformance to the test-designing-guide and
  test-writing-guide, then applies the refinements. Use this skill when the
  user wants to review or refine existing test code so it follows the
  project's test design and writing conventions.
  Typically invoked as `/refine-tests <PATH>`.
argument-hint: "[file paths]"
license: Unlicense
metadata:
  author: Koji Hasegawa

Reviews existing test code for conformance to the test-designing-guide and test-writing-guide, then applies the refinements.

Scope Check

This skill is for refining **existing** tests for conformance to the guides. If the request is out of scope, redirect:

• Adding tests for a new feature or spec change → use `/plan-feature` instead
• A failing test, or a test that verifies incorrect behavior → use `/fix-bug` instead

Input

One or more file path arguments. Resolve them to the concrete set of test files to review before proceeding.

If no path argument is given, use `AskUserQuestion` to ask the user for the targets. Do not derive targets from `git status` — that would silently widen the scope beyond what was requested.

Workflow

**Recording implementation notes:** Notes for this run go in `<scratchpad>/implementation-notes.md`, where `<scratchpad>` is the scratchpad directory listed in your system prompt (write its absolute path literally). Immediately before your **first** append in this run — and only then — delete that file if it exists (usually it will not), so this run starts from an empty one: every `/fix-bug`, `/refine-tests`, and plan-file run in the session shares this file, so an earlier run abandoned before its Implementation Notes step would otherwise leak its notes into this one.

While working through Steps 5–6, whenever one of the following occurs, immediately append a line to that file — do not wait until the end to reconstruct these from memory:

• A Finding was ambiguous, or proved wrong once applied, and you made a judgment call → **Design decisions**
• You intentionally departed from a Finding, and why → **Deviations**
• You considered alternatives and chose one, and why → **Tradeoffs**
• You changed production code — a method demoted to `private`, a dead test-only seam removed, or any other production edit — and why → **Production code changes**

Append with `Bash` — the `Write` tool cannot append:

cat >> "<scratchpad>/implementation-notes.md" <<'EOF'
- **Production code changes**: <one line>
EOF

When a step delegates to another skill (`/simplify` and `/resolve-diagnostics` in Step 6), append the note yourself from what it returns — they do not write to this file.

Step 1: Read the Target Tests

Launch Explore agent(s) to read the target test file(s) and the production code they exercise. Reading the production code is necessary to judge layer-appropriateness and structural-vs-spec-based issues.

Step 2: Conformance Review

Load the `test-designing-guide` and `test-writing-guide` skills. Apply all rules that are **verifiable from the test code alone** — no requirements document is available.

The following sections of `test-designing-guide` require requirements input or production-design changes and are **out of scope**:

• Section 5 (requirements coverage / traceability / same-layer witness)
• Section 6 (design-document output format)
• Section 7 (Testability Assessment — remedies require production-design changes)

Produce a **Findings** list. Each finding records:

• Location: file path + test method name
• Category: which guide + rule violated, or *duplicate test* (see Step 3)
• Concrete proposed change

Step 3: Duplicate Detection

Compare the target test files against each other and against other tests in the same test class.

A **true duplicate** has **both** of the following in common with another test:

• **Same condition** — identical setup / input
• **Same assertion** — identical observation / expected value

Do NOT flag tests that share only one:

• Different condition → not a duplicate
• Same condition but different assertion → not a duplicate

For each true duplicate pair, append a Finding to the Findings list from Step 2:

• Proposed change: delete the redundant test (the less accurately named one) and keep the more accurately named one. Name both explicitly.
• Do NOT propose merging same-condition tests into a single multi-assert test.

**Exception — defer to Step 4:** If one test in a duplicate pair is `[Category("Internal")]` and the other is a public-seam test, do **not** apply the name-quality tiebreaker here. Do not delete the public-seam test on naming grounds. Defer the pair to Step 4, which always keeps the public-seam test.

Step 4: Seam Redundancy — Internal-Method Tests

Test through the same seam production code uses. A `[Category("Internal")]` test exercises an `internal` method directly. For each such test in the targets, append Findings via two passes. Overriding rule: **never trade coverage for a tidier seam; when in doubt, keep the test (and keep `internal`).**

**Layer scope:** This step operates within the **unit test layer only**. Tests with `[Category("Integration")]` or `[Category("VisualVerification")]` applied run under different execution contexts and are not candidates for a covering test — do not consider them when searching for a public-seam test that covers the same scenario.

**Pass 1 — classify each `[Category("Internal")]` test:** 1. **A public-seam test already covers it** (from the test code alone): a separate **unit** test asserts the **same observable outcome** for an **equivalent scenario** through a **public** method → Finding: delete the internal test, keep the public-seam test (name both). 2. **No public-seam test covers it** — using the production code read in Step 1: a. **Not fully observable through public** — a public caller masks or only partially exposes the asserted outcome → keep the internal test; no Finding. "Cheap to extract