FN-6229: enforce symptom verification in triage and review
Require bug-class specs and reviews to prove original symptoms are reproduced and fixed. - Add Symptom Verification guidance to triage prompt templates and self-review rules. - Teach reviewer prompts to REVISE missing symptom verification or green-build-only acceptance. - Document symptom-based acceptance alongside Surface Enumeration and cover the prompt contracts with tests. Files changed: AGENTS.md | 1 + docs/testing.md | 12 +++++++++++ packages/engine/src/__tests__/reviewer.test.ts | 29 +++++++++++++++++++++++++- packages/engine/src/__tests__/triage.test.ts | 23 +++++++++++++++++--- packages/engine/src/reviewer.ts | 2 ++ packages/engine/src/triage.ts | 20 ++++++++++++++++++ 6 files changed, 83 insertions(+), 4 deletions(-) Fusion-Task-Id: FN-6229 Fusion-Task-Lineage: 01ad963f-c62b-410f-8571-57bc68307288
This commit is contained in:
@@ -294,13 +294,18 @@ describe("reviewStep — spec review type", () => {
|
||||
describe("FN-5928 surface-enumeration review-gate wording", () => {
|
||||
it("requires spec reviews to block missing or incomplete surface enumeration for bug-fix specs", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("**Surface enumeration:**");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Missing or incomplete coverage is a blocking REVISE");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toMatch(
|
||||
/For bug-fix specs and UI-affordance add\/remove specs, is `## Surface Enumeration` present[\s\S]*Missing or incomplete coverage is a blocking REVISE\./,
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("desktop + mobile breakpoints/platforms");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("shared hooks/components/modules/helpers");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("bug-fix specs and UI-affordance add/remove specs");
|
||||
});
|
||||
|
||||
it("requires code reviews to reject repro-only regression tests for bug fixes", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toMatch(
|
||||
/For bug fixes, apply FN-5893 strictly: if the regression test only reproduces the reported case instead of asserting the invariant across the spec's `## Surface Enumeration` surfaces, issue REVISE\./,
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("single-surface-only test");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("doesn't verify the invariant across the spec's enumerated surfaces");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Keep enforcing FN-5893 for bug fixes");
|
||||
@@ -309,6 +314,28 @@ describe("FN-5928 surface-enumeration review-gate wording", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5751");
|
||||
});
|
||||
|
||||
it("requires spec reviews to block bug-class specs missing symptom verification", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("**Symptom verification:**");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toMatch(
|
||||
/For bug-class\/bug-fix specs only, is `## Symptom Verification` present and complete with \*\*Original symptom\*\*, \*\*Exact reproduction\*\*, and \*\*Assertion it is gone\*\*\?/,
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"A bug-class spec whose final verification only checks green build/tests without reproducing the original failure and asserting it no longer occurs is a blocking REVISE under FN-5893",
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain(
|
||||
"Missing, empty, or incomplete `## Symptom Verification` is a blocking REVISE for bug-class specs",
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("feature/docs/non-bug specs are not required to carry it");
|
||||
});
|
||||
|
||||
it("requires code reviews to reject green-build-only symptom acceptance for bug fixes", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toMatch(
|
||||
/For bug-class\/bug-fix specs, also enforce symptom-based acceptance:[\s\S]*final verification only checks green build\/tests without reproducing the original failure condition and asserting it no longer occurs, issue REVISE\./,
|
||||
);
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("lacks **Original symptom**, **Exact reproduction**, or **Assertion it is gone**");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Do not require `## Symptom Verification` for feature/docs/non-bug specs");
|
||||
});
|
||||
|
||||
it("requires spec/code reviews to enforce surface enumeration for UI-affordance add/remove tasks", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("leftover shells after removal");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("For bug fixes and UI-affordance add/remove changes");
|
||||
|
||||
@@ -668,11 +668,13 @@ describe("FN-5893 invariant regression wording", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("requires a Surface Enumeration section and blocking REVISE guidance for bug-fix specs", () => {
|
||||
it("requires a Surface Enumeration section and proves missing sections are blocking REVISEs for bug-fix specs", () => {
|
||||
const missingSectionRevisePattern =
|
||||
/For bug fixes and UI-affordance add\/remove tasks, the spec MUST include a `## Surface Enumeration` section\. During self-review via `fn_review_spec\(\)`, treat a missing section on a bug-fix or UI-affordance add\/remove spec as a blocking REVISE\./;
|
||||
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain("## Surface Enumeration");
|
||||
expect(prompt).toContain("spec MUST include a `## Surface Enumeration` section");
|
||||
expect(prompt).toContain("blocking REVISE");
|
||||
expect(prompt).toMatch(missingSectionRevisePattern);
|
||||
expect(prompt).toContain("docs/testing.md");
|
||||
expect(prompt).toContain("duplicate / populated data states");
|
||||
expect(prompt).toContain("shared hooks/components/modules/helpers");
|
||||
@@ -702,6 +704,21 @@ describe("FN-5893 invariant regression wording", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("defines the FN-6229 Symptom Verification contract in standard and fast prompts", () => {
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain("## Symptom Verification");
|
||||
expect(prompt).toContain("Use the exact heading `## Symptom Verification`");
|
||||
expect(prompt).toContain("**Original symptom** — what the user/issue reported was broken");
|
||||
expect(prompt).toContain("**Exact reproduction** — the precise steps, inputs, fixture, or automated repro that triggered the failure");
|
||||
expect(prompt).toContain("**Assertion it is gone**");
|
||||
expect(prompt).toContain("final verification must reproduce that original failure condition and assert it no longer occurs");
|
||||
expect(prompt).toContain("Green build/tests alone are insufficient");
|
||||
expect(prompt).toContain("symptom-based acceptance");
|
||||
expect(prompt).toContain("bug-class/bug-fix tasks");
|
||||
expect(prompt).toContain("feature/docs/non-bug tasks do not need this section");
|
||||
}
|
||||
});
|
||||
|
||||
it("requires Surface Enumeration for UI-affordance add/remove tasks regardless of review-level analysis", () => {
|
||||
for (const prompt of [TRIAGE_SYSTEM_PROMPT, FAST_TRIAGE_SYSTEM_PROMPT]) {
|
||||
expect(prompt).toContain("bug-fix tasks and UI-affordance add/remove tasks");
|
||||
|
||||
@@ -147,6 +147,7 @@ Concrete examples:
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Surface enumeration:** [For bug-fix specs and UI-affordance add/remove specs, is \`## Surface Enumeration\` present and does it enumerate the relevant providers/bridges/execution paths, desktop + mobile breakpoints/platforms, empty/undefined/duplicate/populated states, and shared hooks/components/modules/helpers? For UI-affordance add/remove tasks, also verify: (a) the spec searches for ALL components rendering the affordance, not just the one the user pointed at; (b) the spec explicitly addresses leftover shells after removal across desktop and mobile breakpoints. Missing or incomplete coverage is a blocking REVISE.]
|
||||
- **Symptom verification:** [For bug-class/bug-fix specs only, is \`## Symptom Verification\` present and complete with **Original symptom**, **Exact reproduction**, and **Assertion it is gone**? A bug-class spec whose final verification only checks green build/tests without reproducing the original failure and asserting it no longer occurs is a blocking REVISE under FN-5893. Missing, empty, or incomplete \`## Symptom Verification\` is a blocking REVISE for bug-class specs; feature/docs/non-bug specs are not required to carry it.]
|
||||
- **Documentation completeness:** [Must Update / Check If Affected sections present?]
|
||||
- **Dangling task-document references:** [No \`.fusion/tasks/<id>/<file>\` path is cited in Context, Steps, or File Scope unless the file exists or is explicitly created as a \`(new)\` artifact in this spec. References to nonexistent task-local artifacts are a blocking REVISE.]
|
||||
- **Sizing & review level:** [Size and review level appropriate for the work?]
|
||||
@@ -202,6 +203,7 @@ Do NOT demand function-level implementation checklists.
|
||||
When reviewing tests, check that they verify observable behavior and regression risk (not only implementation trivia).
|
||||
Flag REVISE when key edge cases or failure modes for changed behavior are untested.
|
||||
For bug fixes, apply FN-5893 strictly: if the regression test only reproduces the reported case instead of asserting the invariant across the spec's \`## Surface Enumeration\` surfaces, issue REVISE. Use the motivating recurrences (FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751) as concrete examples of why repro-only coverage is insufficient.
|
||||
For bug-class/bug-fix specs, also enforce symptom-based acceptance: if the spec is missing \`## Symptom Verification\`, leaves it empty/incomplete, lacks **Original symptom**, **Exact reproduction**, or **Assertion it is gone**, or its final verification only checks green build/tests without reproducing the original failure condition and asserting it no longer occurs, issue REVISE. Do not require \`## Symptom Verification\` for feature/docs/non-bug specs.
|
||||
For UI-affordance add/remove changes, apply the same surface-enumeration strictness: if the test only checks the single surface the user reported instead of all enumerated surfaces, issue REVISE. For UI-affordance removals, require coverage/evidence that empty button shells, orphaned click targets, now-unused wrappers, and dangling aria-labels are cleaned up across desktop and mobile breakpoints; FN-6115/FN-6118/FN-6123 is the motivating recurrence.
|
||||
|
||||
## Worktree Boundary Review
|
||||
|
||||
@@ -124,6 +124,10 @@ Follow this structure exactly:
|
||||
|
||||
{Required for bug-fix tasks and UI-affordance add/remove tasks (adding, removing, or restructuring icons, buttons, chevrons/arrows, toggles, badges, menu entries, click targets): a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. For UI-affordance add/remove tasks, enumerate every component that renders the affordance by searching the codebase for the icon/class/testid — not just the component the user pointed at. Explicitly check for leftover shells after removal (empty buttons, orphaned click targets, now-unused wrappers, dangling aria-labels) across both desktop and mobile breakpoints. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Symptom Verification
|
||||
|
||||
{Required for bug-class/bug-fix tasks only; feature/docs/non-bug tasks do not need this section. Use the exact heading \`## Symptom Verification\` and include: (1) **Original symptom** — what the user/issue reported was broken; (2) **Exact reproduction** — the precise steps, inputs, fixture, or automated repro that triggered the failure; (3) **Assertion it is gone** — the executor's final verification must reproduce that original failure condition and assert it no longer occurs via a real automated test. Green build/tests alone are insufficient without symptom-based acceptance.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -168,6 +172,11 @@ For bug-fix and UI-affordance add/remove tasks, paste and fill in this checklist
|
||||
- [ ] Every component that renders the affordance (search the codebase for the icon/class/testid, not just the one the user pointed at)
|
||||
- [ ] Leftover shells after removal — empty buttons, orphaned click targets, now-unused wrappers, dangling aria-labels — are explicitly checked and fixed/hidden
|
||||
|
||||
For bug-class/bug-fix tasks, add and fill in the exact \`## Symptom Verification\` section:
|
||||
- [ ] **Original symptom** — what the user/issue reported was broken
|
||||
- [ ] **Exact reproduction** — the precise steps, inputs, fixture, or automated repro that triggered the failure
|
||||
- [ ] **Assertion it is gone** — final verification reproduces the original failure condition and asserts it no longer occurs via a real automated test; green build/tests alone are insufficient
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -245,6 +254,7 @@ tests. Manual verification is NOT a test.
|
||||
- For bug fixes and UI-affordance add/remove tasks, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix or UI-affordance add/remove spec as a blocking REVISE.
|
||||
- For bug fixes and UI-affordance add/remove tasks, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers; every component that renders the affordance; leftover shells after removal.
|
||||
- For bug fixes and UI-affordance add/remove tasks, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, empty/undefined/populated data states, and for UI-affordance changes every component rendering the affordance plus leftover shells after removal — not just the reported repro (see FN-5787/FN-5789/FN-5803, FN-5751, and FN-6115/FN-6118/FN-6123)
|
||||
- For bug-class/bug-fix tasks, the spec MUST include a \`## Symptom Verification\` section with **Original symptom**, **Exact reproduction**, and **Assertion it is gone**. The final verification step must perform symptom-based acceptance: reproduce the original failure and prove it is gone with a real automated test. Green build/tests alone are insufficient. Feature/docs/non-bug tasks are not required to carry \`## Symptom Verification\`.
|
||||
- The final Testing step runs lint, impacted/package-scoped tests first, and project typecheck when the repo exposes one. Run workspace-wide suites only when explicitly required by the task/workflow or during final integration after impacted checks pass.
|
||||
- Specs must instruct executors to fix lint failures and quality-gate failures directly, even when the required edits extend beyond the original File Scope
|
||||
- If the project has no test framework, the Testing step must include setting one up
|
||||
@@ -438,6 +448,10 @@ Follow this structure exactly:
|
||||
|
||||
{Required for bug-fix tasks and UI-affordance add/remove tasks (adding, removing, or restructuring icons, buttons, chevrons/arrows, toggles, badges, menu entries, click targets): a checklist enumerating every surface the fixed invariant must hold across. Include every provider/bridge for streaming and agent paths; desktop AND mobile breakpoints; empty/undefined/duplicate/populated data states; and every hook/component/module that shares the affected logic. For UI-affordance add/remove tasks, enumerate every component that renders the affordance by searching the codebase for the icon/class/testid — not just the component the user pointed at. Explicitly check for leftover shells after removal (empty buttons, orphaned click targets, now-unused wrappers, dangling aria-labels) across both desktop and mobile breakpoints. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
|
||||
## Symptom Verification
|
||||
|
||||
{Required for bug-class/bug-fix tasks only; feature/docs/non-bug tasks do not need this section. Use the exact heading \`## Symptom Verification\` and include: (1) **Original symptom** — what the user/issue reported was broken; (2) **Exact reproduction** — the precise steps, inputs, fixture, or automated repro that triggered the failure; (3) **Assertion it is gone** — the executor's final verification must reproduce that original failure condition and assert it no longer occurs via a real automated test. Green build/tests alone are insufficient without symptom-based acceptance.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **None**
|
||||
@@ -482,6 +496,11 @@ For bug-fix and UI-affordance add/remove tasks, paste and fill in this checklist
|
||||
- [ ] Every component that renders the affordance (search the codebase for the icon/class/testid, not just the one the user pointed at)
|
||||
- [ ] Leftover shells after removal — empty buttons, orphaned click targets, now-unused wrappers, dangling aria-labels — are explicitly checked and fixed/hidden
|
||||
|
||||
For bug-class/bug-fix tasks, add and fill in the exact \`## Symptom Verification\` section:
|
||||
- [ ] **Original symptom** — what the user/issue reported was broken
|
||||
- [ ] **Exact reproduction** — the precise steps, inputs, fixture, or automated repro that triggered the failure
|
||||
- [ ] **Assertion it is gone** — final verification reproduces the original failure condition and asserts it no longer occurs via a real automated test; green build/tests alone are insufficient
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
|
||||
@@ -554,6 +573,7 @@ If this task REMOVES existing functionality (deleting modules, settings, API end
|
||||
- For bug fixes and UI-affordance add/remove tasks, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix or UI-affordance add/remove spec as a blocking REVISE.
|
||||
- For bug fixes and UI-affordance add/remove tasks, populate \`## Surface Enumeration\` with this checklist from \`docs/testing.md\`: providers/bridges/execution paths; desktop + mobile breakpoints/platforms; empty/undefined/duplicate/populated data states; shared hooks/components/modules/helpers; every component that renders the affordance; leftover shells after removal.
|
||||
- For bug fixes and UI-affordance add/remove tasks, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, empty/undefined/populated data states, and for UI-affordance changes every component rendering the affordance plus leftover shells after removal — not just the reported repro (see FN-5787/FN-5789/FN-5803, FN-5751, and FN-6115/FN-6118/FN-6123)
|
||||
- For bug-class/bug-fix tasks, the spec MUST include a \`## Symptom Verification\` section with **Original symptom**, **Exact reproduction**, and **Assertion it is gone**. The final verification step must perform symptom-based acceptance: reproduce the original failure and prove it is gone with a real automated test. Green build/tests alone are insufficient. Feature/docs/non-bug tasks are not required to carry \`## Symptom Verification\`.
|
||||
- Include targeted tests in implementation steps and full quality-gate runs in final verification
|
||||
|
||||
## Duplicate check
|
||||
|
||||
Reference in New Issue
Block a user