Skip to content

Commit d5fc418

Browse files
fix(explore-feature): harden Phase 1 and local-run gate from pilot findings
Piloted the skill on two real PRs (a multi-surface fix needing an unusual seed shape, and a perf change with no UX delta). Five gaps found and closed: - Phase 1: fix PRs must gate the repro condition, not the easy path that also passes against pre-fix code (vacuous gate). - Phase 1: N equivalent surfaces -> gate the most representative, list the rest under Not covered. - Phase 1: seed-feasibility check before authoring -> if the repro shape isn't seedable via bridge/SDK/fixtures, the deliverable is a bridge/fixture addition or QA hand-off, not a gate. - Phase 3: probe backend health (:8080/is-alive/ver), not just the FE; a half-up stack was passing the gate. - Skip-check: perf/internals changes with no behavior delta -> labelled generic regression or skip-with-note, not a fake PR-specific happy path. - Template: add a required "Repro condition" field for fix PRs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a765d58 commit d5fc418

2 files changed

Lines changed: 38 additions & 6 deletions

File tree

.agents/skills/explore-feature/SKILL.md

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,18 +64,41 @@ Produce the **ScopeSpec**:
6464
- `happyPath` — the one end-to-end flow to gate. Multi-PR → the **combined assembled-feature
6565
flow, as one test** (earlier PRs don't each get a gate).
6666

67+
Three things to resolve while shaping the happy path — each caught a false or unbuildable gate in piloting:
68+
69+
- **Fix PRs — gate the repro, not the easy path.** For a `fix:`, the happy path must exercise the
70+
exact condition the bug needed. If the state can be reached two ways and only one triggered the
71+
bug (e.g. a trace shows the bug only when `source=sdk` via manual reference-linking, not via
72+
`evaluate()`), seeding the easy way makes the test pass against the *pre-fix* code too — a
73+
vacuous gate. Identify the repro condition from the PR's root-cause description and seed that shape.
74+
- **N equivalent surfaces — gate the most representative one.** If the change fixes the same
75+
behavior in several places (e.g. experiment "Go to logs", a shared sidebar, and a Playground
76+
cell link), don't try to cover them all — pick the single most representative entry point for
77+
the gate and list the rest under the plan's "Not covered" for QA. Keeps it cheap.
78+
- **Seed-feasibility check (do this before authoring).** Can the suite actually seed the state the
79+
happy path needs — via the bridge (`services/opik-sdk-driver`), the SDK clients, or an existing
80+
fixture? Check now, not at Phase 4. If the repro shape isn't seedable (e.g. the bridge only
81+
exposes `evaluate()` but the bug needs manual reference-linking), the deliverable for this PR is
82+
a **bridge/fixture addition first** (or a hand-off to QA), *not* a gate test. Say so and stop —
83+
don't march to authoring on a state you can't create.
84+
6785
**Two gates here, before expensive authoring:**
6886

69-
1. **Scope gate** — state the resolved happy path + target path + stamp back to the dev and get a
70-
yes. Multi-PR especially: "One combined test `<lead>.spec.ts` covering X→Y→Z, stamped
71-
`<version>`. OK?"
87+
1. **Scope gate** — state the resolved happy path + repro seed shape + target path + stamp back to
88+
the dev and get a yes. Multi-PR especially: "One combined test `<lead>.spec.ts` covering X→Y→Z,
89+
stamped `<version>`. OK?"
7290
2. **Skip check** — if the change is pure refactor / infra / docs with no user-facing behavior,
7391
say so, point at the skip label, and stop. This is the escape hatch for the "every user-facing
7492
PR" policy. Note two cases that *are* user-facing even though they look like config: a
7593
capability-map / constants change that adds a user-visible option (e.g. a new model in a
7694
dropdown → happy path: "open the page, the option is selectable"), and a backend-dominant
7795
change whose only visible effect is subtle (e.g. a trace that should *not* appear in a default
78-
list) — find the user-observable effect and gate that, don't skip.
96+
list) — find the user-observable effect and gate that, don't skip. The opposite case also
97+
happens: a perf / internals change with **no behavior delta by design** (e.g. swapping a slow
98+
probe for a fast one, same rendered result). There's no PR-specific happy path to gate — so
99+
either gate a *generic* regression on the affected page and label it as such in the plan, or
100+
skip-with-a-note if QA's regression suite already covers that page. Don't dress a generic
101+
regression up as a PR-specific gate.
79102

80103
### Version stamp
81104

@@ -107,7 +130,10 @@ git show origin/main:version.txt # the stamp
107130

108131
Before authoring can be verified, confirm the dev has a local stack **with their changes**:
109132

110-
1. Probe for a running stack (`http://localhost:5173`, and `:5174` for FE-from-source).
133+
1. Probe for a running stack — the **frontend** (`http://localhost:5173`, and `:5174` for
134+
FE-from-source) **and the backend** (`http://localhost:8080/is-alive/ver`). A FE answering with
135+
the BE down is a half-up stack: every seeded test fails on the first API call for env reasons,
136+
not the feature. Require the backend up, not just "something answers on 5173."
111137
2. **Gate the dev:** confirm the running stack actually contains their changes. A stale prebuilt
112138
`opik.sh` stack won't show new `data-testid`s — if the feature adds testids, the dev must be on
113139
FE-from-source `:5174` (`dev-runner --restart`).

.agents/skills/explore-feature/test-plan-template.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@ drive spec generation. **Never committed to the repo.**
2020
## Preconditions / seed
2121
- <state the test seeds via the SDK/bridge before the browser opens>
2222

23+
## Repro condition (fix PRs only)
24+
- <the exact state that triggered the bug — the shape the seed MUST create. Omit for feat PRs.>
25+
2326
## Not covered (out of scope for the gate)
24-
- <edge cases, error paths, perf — flagged for QA's deeper pass>
27+
- <edge cases, error paths, perf, and any equivalent surfaces not gated — flagged for QA's deeper pass>
2528

2629
## Open questions for QA
2730
- <anything the skill was unsure about — the explicit feedback prompt>
@@ -33,6 +36,9 @@ drive spec generation. **Never committed to the repo.**
3336
not edge cases — those go under "Not covered" for QA.
3437
- **Observable results.** Each step names what the *user sees* (a row appears, a badge turns
3538
green), not an internal state change — this is what the spec will assert.
39+
- **For fix PRs, "Repro condition" is mandatory.** Name the exact state that triggered the bug,
40+
and make the seed create that shape — not a similar one the pre-fix code already handled. A gate
41+
that passes against the pre-fix behavior gates nothing.
3642
- **"Not covered" and "Open questions" are mandatory sections**, even if short. They are the
3743
dev↔QA boundary and the feedback surface that improves this skill. An empty "Open questions"
3844
means "I'm confident"; say that rather than deleting the section.

0 commit comments

Comments
 (0)