Skip to content

Commit 665f188

Browse files
committed
feat: add verify-rule skill for Chromium-driven rule validation
Structured workflow for cross-checking rule source, unit tests, test.html cases, and real Chromium behavior via Playwright. Helps catch false positives and default value mismatches.
1 parent 9040994 commit 665f188

1 file changed

Lines changed: 179 additions & 0 deletions

File tree

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
---
2+
name: verify-rule
3+
description: Use when verifying a CSS noop rule against real Chromium behavior, checking rule correctness and e2e/unit test consistency. Triggers on "verify rule", "check rule", "re-verify", or reviewing rule accuracy after changes.
4+
argument-hint: <rule-id>
5+
---
6+
7+
# Verify Rule
8+
9+
Verify a CSS noop rule's correctness and test consistency by cross-checking **rule source**, **unit tests**, **test.html cases**, and **real Chromium computed styles** via Playwright.
10+
11+
## Correctness Standard
12+
13+
This project is **Chromium-behavior-driven**. The ground truth is what `getComputedStyle()` returns in current Chromium, not the CSS spec alone.
14+
15+
- **False positives are worse than false negatives.** If behavior is conditional, context-sensitive, or unclear in Chromium, the rule should stay silent.
16+
- A property that is **partially effective** (e.g. one axis of `place-items` still matters) is NOT a no-op — do not warn.
17+
- If you intentionally keep a known false negative, document the trade-off in the rule comment.
18+
19+
Apply this standard at every judgment point in the workflow below.
20+
21+
## When to Use
22+
23+
- After modifying a rule's logic or default values
24+
- When auditing whether a rule matches real Chromium behavior
25+
- When test.html cases might have false positives/negatives
26+
- Before merging rule changes
27+
28+
## Workflow
29+
30+
```dot
31+
digraph verify {
32+
rankdir=TB;
33+
"Read rule source" -> "Read unit tests";
34+
"Read unit tests" -> "Read test.html cases";
35+
"Read test.html cases" -> "Cross-check coverage";
36+
"Cross-check coverage" -> "Has gaps?" [shape=diamond];
37+
"Has gaps?" -> "Report missing coverage" [label="yes"];
38+
"Has gaps?" -> "Run Playwright verification" [label="no"];
39+
"Report missing coverage" -> "Run Playwright verification";
40+
"Run Playwright verification" -> "Run unit tests";
41+
"Run unit tests" -> "Report results";
42+
}
43+
```
44+
45+
### Step 1 — Gather Sources
46+
47+
Read these files for the given `<rule-id>`:
48+
49+
| File | Purpose |
50+
|---|---|
51+
| `src/rules/<rule-id>.ts` | Rule logic, checked properties, default values |
52+
| `src/rules/__tests__/<rule-id>.test.ts` | Unit test cases and assertions |
53+
| `examples/test.html` (grep for `data-rule="<rule-id>"`) | Browser test cases |
54+
| `docs/rules/<rule-id>.md` | Rule documentation (if exists) |
55+
56+
### Step 2 — Cross-Check Coverage
57+
58+
Compare the three layers and report gaps:
59+
60+
1. **Rule properties vs test.html cases** — Does test.html have at least one `expect-warn` case per property the rule checks? Does it have `expect-ok` cases for key bypass conditions (e.g. `will-change`, parent context)?
61+
2. **Rule logic vs unit tests** — Do unit tests cover all branches? (early returns, edge cases like multi-value properties, `will-change` bypass)
62+
3. **Unit test assumptions vs defaults** — Do the `defaultValue` constants in the rule match `DEFAULT_COMPUTED_STYLES` in `make-element.ts`? Do they match what Chromium actually returns?
63+
64+
Report any gaps found before proceeding to Playwright.
65+
66+
### Step 3 — Playwright Verification
67+
68+
Write a **temporary** Playwright test at `e2e/integration/verify-<rule-id>.test.ts`:
69+
70+
```typescript
71+
import { test, expect } from '@playwright/test';
72+
import { fileURLToPath } from 'node:url';
73+
import path from 'node:path';
74+
import { extractElementData } from '../helpers/extract-element-data.ts';
75+
import { analyzeElement } from '../../src/rules/engine.ts';
76+
77+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
78+
const TEST_HTML = `file://${path.resolve(__dirname, '../../examples/test.html')}`;
79+
80+
test('verify <RULE_ID> cases in detail', async ({ page }) => {
81+
await page.goto(TEST_HTML);
82+
83+
const cases = page.locator('.case[data-rule="<RULE_ID>"]');
84+
const count = await cases.count();
85+
console.log(`\n=== Found ${count} <RULE_ID> cases ===\n`);
86+
87+
for (let i = 0; i < count; i++) {
88+
const caseEl = cases.nth(i);
89+
const label = (await caseEl.locator('.label').textContent())?.trim() ?? '(no label)';
90+
const classList = await caseEl.evaluate((el) => Array.from(el.classList));
91+
const expectWarn = classList.includes('expect-warn');
92+
93+
const target = caseEl.locator('[data-target]');
94+
const data = await extractElementData(target);
95+
const warnings = analyzeElement(data);
96+
const matching = warnings.filter((w) => w.ruleId === '<RULE_ID>');
97+
98+
// Extract rule-relevant computed styles for inspection
99+
const relevantStyles: Record<string, string> = {};
100+
for (const key of Object.keys(data.computedStyles)) {
101+
// Filter to properties relevant to this rule — adjust the condition per rule
102+
relevantStyles[key] = data.computedStyles[key];
103+
}
104+
105+
const status = expectWarn
106+
? matching.length > 0 ? 'PASS' : 'FAIL'
107+
: matching.length === 0 ? 'PASS' : 'FAIL';
108+
109+
console.log(`${status} | ${label}`);
110+
console.log(` Computed: ${JSON.stringify(relevantStyles)}`);
111+
if (matching.length > 0) {
112+
console.log(` Warnings: ${matching.map((w) => w.property).join(', ')}`);
113+
} else {
114+
console.log(` Warnings: (none)`);
115+
}
116+
// Log unexpected warnings from OTHER rules
117+
const others = warnings.filter((w) => w.ruleId !== '<RULE_ID>');
118+
if (others.length > 0) {
119+
console.log(` Other rules fired: ${others.map((w) => `${w.ruleId}:${w.property}`).join(', ')}`);
120+
}
121+
console.log('');
122+
123+
if (expectWarn) {
124+
expect(matching.length, `"${label}" should warn`).toBeGreaterThan(0);
125+
} else {
126+
expect(matching.length, `"${label}" should NOT warn`).toBe(0);
127+
}
128+
}
129+
});
130+
```
131+
132+
Replace `<RULE_ID>` with the actual rule ID. Filter `relevantStyles` to only the properties declared in the rule's `requiredProperties` for readability.
133+
134+
Run:
135+
```bash
136+
pnpm playwright test e2e/integration/verify-<rule-id>.test.ts
137+
```
138+
139+
### Step 4 — Run Unit Tests
140+
141+
```bash
142+
pnpm vitest run src/rules/__tests__/<rule-id>.test.ts
143+
```
144+
145+
Check for failures. If a unit test fails but Playwright passes (or vice versa), the unit test's assumptions may be wrong — investigate the divergence.
146+
147+
### Step 5 — Clean Up and Report
148+
149+
1. **Delete** the temporary Playwright test file
150+
2. **Report** results in this format:
151+
152+
```
153+
## Verify: <rule-id>
154+
155+
### Coverage Check
156+
- Rule properties: [list]
157+
- test.html warn cases: [count] / ok cases: [count]
158+
- Unit test cases: [count]
159+
- Gaps: [any missing coverage]
160+
161+
### Playwright Results (Chromium)
162+
| Case | Expected | Result | Key Computed Values |
163+
|------|----------|--------|---------------------|
164+
| ... | warn | PASS | animationDuration: "2s" |
165+
166+
### Unit Test Results
167+
[pass/fail count]
168+
169+
### Divergences
170+
[Any inconsistencies between unit tests, e2e, and real browser behavior]
171+
```
172+
173+
## Key Checks
174+
175+
- **False positive?** — Does the rule warn on a declaration that actually has a visible effect in Chromium? This is the most serious defect. Verify by toggling the property in DevTools and observing layout/paint changes.
176+
- **Partial effectiveness** — A shorthand or composite property where one part matters (e.g. `place-items` where one axis is effective) is NOT a full no-op. The rule must not warn.
177+
- **Default value mismatch** — Rule says `defaultValue: 'ease'` but Chromium returns `cubic-bezier(...)` → silent false negatives. Compare rule defaults against real `getComputedStyle()` output.
178+
- **Shorthand expansion** — Rule checks longhand but style is set via shorthand → verify Chromium resolves it to the expected longhand value.
179+
- **Other rules firing** — A test case triggers warnings from unexpected rules → investigate cross-rule interaction.

0 commit comments

Comments
 (0)