FN-6134: extend surface-enumeration gates to UI affordance changes
Apply the surface-enumeration review gate to UI affordance add/remove work across triage, review, tests, and docs. - require triage prompts to add Surface Enumeration guidance for UI affordance add/remove tasks in standard and fast modes - tighten reviewer guidance to block single-surface UI affordance coverage and leftover shell cleanup gaps - expand engine prompt tests to cover the new UI affordance gate wording - document the shared checklist and motivating incidents in AGENTS.md and docs/testing.md Files changed: AGENTS.md | 3 +++ docs/testing.md | 6 +++++- packages/engine/src/__tests__/reviewer.test.ts | 26 ++++++++++++++++++++++++-- packages/engine/src/__tests__/triage.test.ts | 19 ++++++++++++++++++- packages/engine/src/reviewer.ts | 5 +++-- packages/engine/src/triage.ts | 24 ++++++++++++++---------- 6 files changed, 67 insertions(+), 16 deletions(-) Fusion-Task-Id: FN-6134 Fusion-Task-Lineage: 0bc733eb-d160-48cc-817d-69138290faec
This commit is contained in:
gsxdsm
@@ -297,15 +297,37 @@ describe("FN-5928 surface-enumeration review-gate wording", () => {
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("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).toContain("repro-only regression test");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("spanning the `## Surface Enumeration` checklist");
|
||||
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");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5787/FN-5789/FN-5803");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5797/FN-5875/FN-5919");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-5751");
|
||||
});
|
||||
|
||||
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");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("UI-affordance removals");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("For UI-affordance add/remove changes, apply the same surface-enumeration strictness");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("FN-6115/FN-6118/FN-6123");
|
||||
});
|
||||
|
||||
it("demonstrates the gate firing on a single-component UI-removal spec", () => {
|
||||
const singleComponentRemovalSpec =
|
||||
"## Mission\nRemove the workflow-row chevron from WorkflowRow.tsx only.";
|
||||
|
||||
expect(singleComponentRemovalSpec).toContain("WorkflowRow.tsx only");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("searches for ALL components rendering the affordance");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("not just the one the user pointed at");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("leftover shells after removal");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("empty button shells");
|
||||
expect(REVIEWER_SYSTEM_PROMPT).toContain("Issue REVISE when coverage stops at the single reported surface");
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — context-limit retry", () => {
|
||||
|
||||
@@ -676,6 +676,11 @@ describe("FN-5893 invariant regression wording", () => {
|
||||
expect(prompt).toContain("docs/testing.md");
|
||||
expect(prompt).toContain("duplicate / populated data states");
|
||||
expect(prompt).toContain("shared hooks/components/modules/helpers");
|
||||
expect(prompt).toContain("UI-affordance add/remove");
|
||||
expect(prompt).toContain("For bug fixes and UI-affordance add/remove tasks");
|
||||
expect(prompt).toContain(
|
||||
"For bug-fix and UI-affordance add/remove tasks, paste and fill in this checklist",
|
||||
);
|
||||
}
|
||||
|
||||
expect(corePromptSource).toContain("## Surface Enumeration");
|
||||
@@ -692,11 +697,23 @@ describe("FN-5893 invariant regression wording", () => {
|
||||
"Run targeted tests for changed files, asserting the invariant across all known surfaces",
|
||||
);
|
||||
expect(prompt).toContain(
|
||||
"For bug-fix tasks, paste and fill in this checklist in the `## Surface Enumeration` section",
|
||||
"For bug-fix and UI-affordance add/remove tasks, paste and fill in this checklist in the `## Surface Enumeration` 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");
|
||||
expect(prompt).toContain("every component that renders the affordance");
|
||||
expect(prompt).toContain("searching the codebase for the icon/class/testid");
|
||||
expect(prompt).toContain("leftover shells after removal");
|
||||
expect(prompt).toContain("empty buttons");
|
||||
}
|
||||
|
||||
expect(FAST_TRIAGE_SYSTEM_PROMPT).not.toContain("## Review Level");
|
||||
});
|
||||
|
||||
it("pins the canonical docs checklist heading", () => {
|
||||
const docsTestingSource = readFileSync(
|
||||
fileURLToPath(new URL("../../../../docs/testing.md", import.meta.url)),
|
||||
|
||||
@@ -121,7 +121,7 @@ Concrete examples:
|
||||
|
||||
### Test Gaps
|
||||
- [Missing test scenarios]
|
||||
- [For bug fixes, call out any repro-only regression test that does not assert the invariant across the enumerated surfaces. Issue REVISE when coverage stops at the single reported case instead of spanning the \`## Surface Enumeration\` checklist (FN-5893; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751).]
|
||||
- [For bug fixes and UI-affordance add/remove changes, call out any single-surface-only test that doesn't verify the invariant across the spec's enumerated surfaces. For UI-affordance removals, also flag tests that don't verify the removed affordance's container/wrapper is fully cleaned up on both desktop and mobile breakpoints. Issue REVISE when coverage stops at the single reported surface (FN-6134; see FN-6115→FN-6118→FN-6123 for the motivating multi-task incident). Keep enforcing FN-5893 for bug fixes; see FN-5787/FN-5789/FN-5803, FN-5797/FN-5875/FN-5919, and FN-5751.]
|
||||
|
||||
### Suggestions
|
||||
- [Optional improvements, not blocking]
|
||||
@@ -146,7 +146,7 @@ Concrete examples:
|
||||
- **File scope accuracy:** [All affected files listed? No extras?]
|
||||
- **Dependency correctness:** [Dependencies exist and are appropriate?]
|
||||
- **Testing requirements:** [Real automated tests required, not just typechecks?]
|
||||
- **Surface enumeration:** [For bug-fix 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? Missing or incomplete coverage is a blocking REVISE.]
|
||||
- **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.]
|
||||
- **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 +202,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 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
|
||||
|
||||
|
||||
@@ -120,7 +120,7 @@ Follow this structure exactly:
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: 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. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
{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.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
@@ -158,11 +158,13 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
For bug-fix and UI-affordance add/remove tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
- [ ] 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
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
@@ -238,9 +240,9 @@ files with assertions that run via a test runner. Typechecks and builds are NOT
|
||||
tests. Manual verification is NOT a test.
|
||||
|
||||
- Each implementation step should include writing tests for the code being changed
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, 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.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- 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)
|
||||
- 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
|
||||
@@ -424,7 +426,7 @@ Follow this structure exactly:
|
||||
|
||||
## Surface Enumeration
|
||||
|
||||
{Required for bug-fix tasks: 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. Use the canonical checklist in docs/testing.md as the starting point.}
|
||||
{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.}
|
||||
|
||||
## Dependencies
|
||||
|
||||
@@ -462,11 +464,13 @@ Follow this structure exactly:
|
||||
- [ ] {Specific, verifiable outcome}
|
||||
- [ ] Run targeted tests for changed files, asserting the invariant across all known surfaces (enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states)
|
||||
|
||||
For bug-fix tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
For bug-fix and UI-affordance add/remove tasks, paste and fill in this checklist in the \`## Surface Enumeration\` section:
|
||||
- [ ] Providers / bridges / execution paths touched by the invariant
|
||||
- [ ] Desktop + mobile breakpoints / platforms that exercise the behavior
|
||||
- [ ] Empty / undefined / duplicate / populated data states
|
||||
- [ ] Shared hooks / components / modules / helpers reusing the logic
|
||||
- [ ] 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
|
||||
|
||||
**Artifacts:**
|
||||
- \`path/to/file\` (new | modified)
|
||||
@@ -537,9 +541,9 @@ If this task REMOVES existing functionality (deleting modules, settings, API end
|
||||
## Testing requirements
|
||||
- Require real automated tests with assertions that run in the project's test runner
|
||||
- Typecheck/build/manual checks are not tests and cannot replace tests
|
||||
- For bug fixes, the spec MUST include a \`## Surface Enumeration\` section. During self-review via \`fn_review_spec()\`, treat a missing section on a bug-fix spec as a blocking REVISE.
|
||||
- For bug fixes, 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.
|
||||
- For bug fixes, regression tests must assert the invariant across all known surfaces — enumerate every provider/bridge, desktop + mobile breakpoints, and empty/undefined/populated data states — not just the reported repro (see FN-5787/FN-5789/FN-5803 and FN-5751)
|
||||
- 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)
|
||||
- Include targeted tests in implementation steps and full quality-gate runs in final verification
|
||||
|
||||
## Duplicate check
|
||||
|
||||
Reference in New Issue
Block a user