v0.5 audit-fixes + drift loop closing — B7 + rule #3 + B8 active + drifts-mapping.md - #6
Merged
Conversation
…tive diff Three audit-driven SKILL.md updates from internal pre-skill testing + skills.sh comparison. All fixes addressed in a single PR scope. SKILL.md changes: - frontmatter version 0.3 → 0.4 - B7 § File-path determination: read-before-write procedure. Run Read(target-path) first; on file-found prefer extend > replace; halt-on-doubt per regel-range. Addresses page-overwrite regression observed in pre-skill internal testing. - Rule #3: hardened from "refuse to introduce parallel paradigm" to binary halt-on-mismatch. Implement reads tokens.md § Project styling stack at B4.3 start, hardcodes the API, halts on second styling-API detection in same element. Addresses dual-styling regression. - B8 § Post-emit visual validation: rewritten as active diff with five sub-steps (B8.1-B8.5). B8.1 starts dev server, B8.2 captures rendered screenshot, B8.3 diffs against Figma reference, B8.4 applies 7-point check to delta-list, B8.5 prompts user with three options (rollback / accept as drift / update mapping) on critical mismatch. Closes post-emit-blindheid gap before PR-review. No new rule #13 — procedure update only. templates/post-emit-visual-check.md updated with B8.1-B8.5 procedure and rollback-prompt template matching SKILL.md B8. LESSONS.md entry 3 [confirmation] documents the three fixes with rationale and dogfood proposal. DEFERRED-FIXES.md audit items I1, I2, I3 moved to resolved-decisions. PLAN.md: Owner field removed (no individual names in skill files). v0.5 design-fidelity fixes (Resolution-mode 4 inline-fix audit-trail, asset-materialize, fidelity-priority) and v1.0 drift loop closing (drifts-implement.md write authority) are out of scope for this PR — captured in plan-file for follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR updates the figma-to-code-implement spec and templates: skill version bumped to 0.5, file-path rules changed to read-before-write, single-styling-API detection now halts the emit, post-emit validation becomes an active five-step visual-diff with in-session drift decisions, and an implement-owned ChangesSpec, templates, and project records
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
templates/post-emit-visual-check.md (1)
49-62: 💤 Low valueConsider adding language specifier to the code block.
The in-session prompt structure with three explicit options (rollback / accept as drift / update mapping) properly enforces the drift discipline. However, the code block at line 50 lacks a language specifier.
📝 Suggested improvement for consistency
While the block contains template output rather than executable code, adding a language specifier (e.g.,
textormarkdown) improves consistency with markdown best practices:-**On mismatch (1+ mismatches) — B8.5 in-session prompt:** -``` +**On mismatch (1+ mismatches) — B8.5 in-session prompt:** +```text ⚠️ B8 visual validation: <X> mismatch(es) detected.As per coding guidelines, this addresses the markdownlint warning about fenced code blocks requiring a language specification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/post-emit-visual-check.md` around lines 49 - 62, The fenced code block under the "**On mismatch (1+ mismatches) — B8.5 in-session prompt:**" heading (the block starting with "⚠️ B8 visual validation: <X> mismatch(es) detected.") is missing a language specifier; update the opening fence from ``` to a language-tagged fence such as ```text (or ```markdown) to satisfy markdownlint and maintain consistency, and apply the same change to any similar template blocks if present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@templates/post-emit-visual-check.md`:
- Around line 49-62: The fenced code block under the "**On mismatch (1+
mismatches) — B8.5 in-session prompt:**" heading (the block starting with "⚠️ B8
visual validation: <X> mismatch(es) detected.") is missing a language specifier;
update the opening fence from ``` to a language-tagged fence such as ```text (or
```markdown) to satisfy markdownlint and maintain consistency, and apply the
same change to any similar template blocks if present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 972ba3c2-c5c4-4260-b00b-7fa1c7876c2a
📒 Files selected for processing (5)
DEFERRED-FIXES.mdLESSONS.mdPLAN.mdSKILL.mdtemplates/post-emit-visual-check.md
💤 Files with no reviewable changes (1)
- PLAN.md
…ption
Sluit de "drift = archief" valkuil door drift-summary in chat na elke emit
met decision-prompt per drift, plus implement-owned drifts-mapping.md file
voor status-tracking. Onafhankelijk van mapping-coordination.
SKILL.md changes:
- frontmatter version 0.4 → 0.5
- Rule #8 uitgebreid: na emit-completion print drift-summary in chat met
decision-prompt per drift (Rollback / Accept / Update code / Update
mapping). User reageert per drift of in batch. Status-update direct
in drifts-mapping.md.
- Rule #10 uitzondering: implement schrijft in eigen drifts-mapping.md
(parallel aan mapping's drifts.md). Andere mapping-files blijven
read-only.
- § Mapping → implement contract § Write herstructureerd:
- Mapping-owned files (drifts.md, verify-queue.md, etc.) read-only met
twee propose-to-user exceptions
- Implement-owned drifts-mapping.md: direct schrijven na user-decision
(geen propose-to-user gate)
- B8.5 prompt-options uitgebreid: 4 opties met automatische
status-tracking in drifts-mapping.md
- Nieuwe B8.6 step: post-emit drift-summary in chat voor alle drifts
(B8 detectie + mapping's drifts.md items in scope)
templates/drifts-mapping.md (nieuw):
- Format documentatie: [date][severity][owner] origin — description.
Decision: <action>. Status: <STATUS>
- Schema-versie tracking voor toekomstige iteraties
- Append-only entry-sectie
LESSONS.md entry 4 [confirmation] documenteert loop-closing rationale +
proposal voor dogfood-validatie.
DEFERRED-FIXES.md C1 (Loop closing implement → propose-to-mapping)
verschoven naar resolved-decisions.
Architectuur-keuze: aparte file drifts-mapping.md ipv schrijven in
mapping's drifts.md. Heldere scheiding van write-authority — mapping
owns drifts.md, implement owns drifts-mapping.md. Geen
coordination-PR met mapping nodig voor v0.5 ship.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-rutten
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DEFERRED-FIXES.md`:
- Line 117: Update the option count text in the "Audit I3: Post-emit
screenshot-diff actief" entry so the in-session prompt count matches the actual
listed actions: change the phrase "3 opties" to "4 opties" (or otherwise adjust
the listed actions to three) to align with B8/B8.5 prompt semantics referenced
in the diff; specifically edit the line containing "B8 herzien naar vijf actieve
sub-stappen (..., in-session prompt bij critical mismatch met 3 opties)" to
reflect "4 opties" (or remove one action) so the procedure and prompt remain
consistent.
In `@LESSONS.md`:
- Line 22: The LESSONS.md text references the wrong artifact name: replace
"drifts-implement.md" with the actual artifact name "drifts-mapping.md" (or vice
versa if the artifact should be renamed) so the proposal line consistently
points to the correct file; update the string in the Proposal sentence in
LESSONS.md and any other occurrences to ensure the reference to the drifts
mapping/implementation artifact is consistent across the repo.
In `@SKILL.md`:
- Line 68: Rule `#8`'s drift actions and the Write schema / B8.5 enums are
inconsistent (examples: "Inline-fix-override" vs "Update mapping", "revert" vs
"rollback"); unify to a single canonical action set (choose one normalized set
such as: Rollback, Accept, UpdateMapping, InlineFixOverride) and replace all
occurrences accordingly in the decision generator, the drift-summary chat
prompt, and the writer that emits to drifts-mapping.md; update enum/constant
names in the components handling drift decisions (references: Rule `#8` text,
drifts.md consumer/generator, Write schema, B8.5 decision logic, and the
drifts-mapping.md writer) and ensure the logging/parsing code expects the
canonical tokens so parsing and status updates remain deterministic.
- Around line 319-345: B8 currently contains both detection steps (B8.3/B8.4)
and implementation decisions, conflicting with the SKILL.md verb-role rule;
update SKILL.md so detection lives in the mapping skill and implementation only
performs actions on decisions: change B8.3/B8.4 wording to mapping verbs like
"capture", "compare", "generate delta-list" and move any "detect" / "mark"
responsibilities to mapping, and keep B8.5 as the implement-step for user-driven
actions ("Rollback", "Accept", "Update code", "Update mapping"); ensure the
verbs list in the header and the lines for B8.3, B8.4, and B8.5 consistently use
mapping vs implement verbs per the guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f5560c1-4731-4b1d-9f15-cfbb2fc5a37f
📒 Files selected for processing (4)
DEFERRED-FIXES.mdLESSONS.mdSKILL.mdtemplates/drifts-mapping.md
| - **Confirmation gate before B7 emit** — resolved (v0.3): no skill-level confirmation gate. Host environment provides the safety nets — Claude Code's permission-system asks per file-write, and the project's PR-review process catches issues before merge to main. A skill-level halt-and-ask would duplicate those gates without adding safety. SKILL.md B7 makes this explicit. | ||
| - **Audit I1: Differential emit op bestaande files** — resolved (v0.4): B7 § File-path determination herzien naar Read-before-write. Bestaande file: prefer extend > replace. Twijfel → halt per regel-range. Voorkomt page-overschrijving regression uit pre-skill testing. | ||
| - **Audit I2: Stack-hardcoding refuse-on-mismatch** — resolved (v0.4): rule #3 verhard naar binary halt-on-mismatch. Implement leest `tokens.md § Project styling stack` bij start B4.3, halt direct op tweede styling-API. Voorkomt dual-styling regression uit pre-skill testing. | ||
| - **Audit I3: Post-emit screenshot-diff actief** — resolved (v0.4): B8 herzien naar vijf actieve sub-stappen (dev server start, screenshot capture, diff tegen Figma, 7-point check op diff, in-session prompt bij critical mismatch met 3 opties). Procedure-update, geen rule-text bloat. |
There was a problem hiding this comment.
Update option count to match B8.5 prompt.
This says “3 opties,” but the current flow lists 4 actions (Rollback, Accept, Update code, Update mapping). Keep these aligned to avoid procedural drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DEFERRED-FIXES.md` at line 117, Update the option count text in the "Audit
I3: Post-emit screenshot-diff actief" entry so the in-session prompt count
matches the actual listed actions: change the phrase "3 opties" to "4 opties"
(or otherwise adjust the listed actions to three) to align with B8/B8.5 prompt
semantics referenced in the diff; specifically edit the line containing "B8
herzien naar vijf actieve sub-stappen (..., in-session prompt bij critical
mismatch met 3 opties)" to reflect "4 opties" (or remove one action) so the
procedure and prompt remain consistent.
| [LESSON — 2026-05-12] [confirmation] | ||
| Situation: Audit-based v0.4 fixes from skills.sh comparison + pre-skill internal testing observations. Three fixes landed: B7 differential emit (Read-before-write to prevent page-overschrijving), Rule #3 binair halt-on-mismatch (dual-styling regression prevention), B8 actief screenshot-diff (post-emit verification in-session). | ||
| What worked: Each fix has Wat-dit-toevoegt rationale + minimal text-investering. B7 is workflow-safeguard for proven regression. Rule #3 hardening makes "refuse" actionable as halt. B8 active diff closes the post-emit-blindness gap before PR-review (workflow-context: Claude Code session = primary safety net, not PR). | ||
| Proposal: Dogfood v0.4 on a real implement-pass before adding v0.5 (design-fidelity) or v1.0 (drift loop closing via drifts-implement.md). Confirm fixes prevent observed failures; only then expand scope. |
There was a problem hiding this comment.
Fix artifact name mismatch (drifts-implement.md vs drifts-mapping.md).
This references a file name that doesn’t match the introduced artifact and may create tracking confusion in later updates.
- Proposal: Dogfood v0.4 on a real implement-pass before adding v0.5 (design-fidelity) or v1.0 (drift loop closing via drifts-implement.md).
+ Proposal: Dogfood v0.4 on a real implement-pass before adding v0.5 (design-fidelity) or v1.0 (drift loop closing via drifts-mapping.md).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Proposal: Dogfood v0.4 on a real implement-pass before adding v0.5 (design-fidelity) or v1.0 (drift loop closing via drifts-implement.md). Confirm fixes prevent observed failures; only then expand scope. | |
| Proposal: Dogfood v0.4 on a real implement-pass before adding v0.5 (design-fidelity) or v1.0 (drift loop closing via drifts-mapping.md). Confirm fixes prevent observed failures; only then expand scope. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LESSONS.md` at line 22, The LESSONS.md text references the wrong artifact
name: replace "drifts-implement.md" with the actual artifact name
"drifts-mapping.md" (or vice versa if the artifact should be renamed) so the
proposal line consistently points to the correct file; update the string in the
Proposal sentence in LESSONS.md and any other occurrences to ensure the
reference to the drifts mapping/implementation artifact is consistent across the
repo.
| 7. **Verify-queue blocks emit — same per-component spec only.** If `verify-queue.md` has an item linked to the **same per-component spec** as the emit-scope: pause and ask user. Items in other specs do not block (smallest meaningful blocking scope; broader definitions deadlock the user on every implement run). Don't improvise. | ||
|
|
||
| 8. **Surface mapping-recorded drift — never silently resolve.** Read `drifts.md` and per-component spec drift notes before emit. Drifts in scope are surfaced to the user as design decisions, not silently fixed. Implement does not detect new drift — mapping does that. | ||
| 8. **Surface drift — and close the decision loop.** Read `drifts.md` (mapping-owned) and per-component spec drift notes before emit. **After emit-completion, print a drift-summary in chat** with decision-prompt per drift (Rollback / Accept / Update code / Inline-fix-override). User reageert per drift of in batch. Implement writes status updates to **`drifts-mapping.md`** (implement-owned, parallel to mapping's `drifts.md`). Drift wordt beslispunt, niet archief. |
There was a problem hiding this comment.
Unify drift decision enums across Rule #8, Write schema, and B8.5.
The allowed actions are currently inconsistent (Inline-fix-override vs Update mapping, and revert vs rollback). This breaks deterministic handling and log parsing for drifts-mapping.md.
Suggested alignment (single canonical action set)
-8. **Surface drift — and close the decision loop.** ... (Rollback / Accept / Update code / Inline-fix-override).
+8. **Surface drift — and close the decision loop.** ... (Rollback / Accept / Update code / Update mapping).
-[YYYY-MM-DD] ... Decision: <revert | accept | update-code | inline-fix-override>. Status: ...
+[YYYY-MM-DD] ... Decision: <rollback | accept | update-code | update-mapping>. Status: ...Also applies to: 124-128, 338-343
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SKILL.md` at line 68, Rule `#8`'s drift actions and the Write schema / B8.5
enums are inconsistent (examples: "Inline-fix-override" vs "Update mapping",
"revert" vs "rollback"); unify to a single canonical action set (choose one
normalized set such as: Rollback, Accept, UpdateMapping, InlineFixOverride) and
replace all occurrences accordingly in the decision generator, the drift-summary
chat prompt, and the writer that emits to drifts-mapping.md; update
enum/constant names in the components handling drift decisions (references: Rule
`#8` text, drifts.md consumer/generator, Write schema, B8.5 decision logic, and
the drifts-mapping.md writer) and ensure the logging/parsing code expects the
canonical tokens so parsing and status updates remain deterministic.
Comment on lines
+319
to
+345
| ### B8. Post-emit visual validation — active diff | ||
|
|
||
| Compare the emit against the screenshot captured in B3. | ||
| Run actively in the same Claude Code session as B7. Five sub-steps: | ||
|
|
||
| | # | Check | | ||
| |---|---| | ||
| | 1 | Layout — spacing, alignment, sizing match the screenshot | | ||
| | 2 | Typography — font-family, size, weight, line-height | | ||
| | 3 | Colors — exact match on token values | | ||
| | 4 | Interactive states render per variant-mapping | | ||
| | 5 | Responsive behavior follows Figma constraints | | ||
| | 6 | Assets render correctly | | ||
| | 7 | Accessibility — aria-labels, alt text, semantic structure | | ||
|
|
||
| Mismatch found → surface as drift (rule #8), never as inline pixel-fix (rule #11). | ||
| - **B8.1 Start dev server.** Use project's documented run-command (`npm run dev`, `pnpm dev`, `bun dev`, etc.). If not running, start in background; if running, reuse existing port. Wait for "ready" output before continuing. | ||
| - **B8.2 Capture rendered screenshot.** Open the rendered route in Claude Code preview (or framework-equivalent), capture as image. | ||
| - **B8.3 Diff against Figma reference.** Compare rendered screenshot against `mcp__Figma__get_screenshot(<nodeId>)` from B3. Generate a delta-list per visual category. | ||
| - **B8.4 7-point check on the diff.** Apply original 7-point checklist to the delta-list, not the rendered screenshot alone. Mark each row as ✓ match, ⚠ minor delta, or ✗ critical delta: | ||
|
|
||
| | # | Check | | ||
| |---|---| | ||
| | 1 | Layout — spacing, alignment, sizing match the screenshot | | ||
| | 2 | Typography — font-family, size, weight, line-height | | ||
| | 3 | Colors — exact match on token values | | ||
| | 4 | Interactive states render per variant-mapping | | ||
| | 5 | Responsive behavior follows Figma constraints | | ||
| | 6 | Assets render correctly | | ||
| | 7 | Accessibility — aria-labels, alt text, semantic structure | | ||
|
|
||
| - **B8.5 In-session prompt on critical mismatch.** If ≥1 ✗ critical delta: do not let the emit pass without user-decision. Prompt (per drift of in batch): | ||
| - **Rollback** — `git restore` the changed files, end implement-pass with no commit. | ||
| - **Accept** — write drift-row in `drifts-mapping.md` met status `ACCEPTED`. Code blijft staan. | ||
| - **Update code** — implement past emit aan op user-aanwijzing. Drift-row in `drifts-mapping.md` met status `RESOLVED`. | ||
| - **Update mapping** — route to `/figma-to-code-mapping map <node>` before re-emit. Drift-row met status `SCHEDULED`. | ||
|
|
||
| Mismatch never resolved by inline pixel-fix (rule #11). All prompt-options write to `drifts-mapping.md` to keep the decision-loop closed (rule #8). | ||
|
|
There was a problem hiding this comment.
Resolve boundary conflict: B8 now detects drift, but spec still says implement “does not detect.”
B8.3/B8.4 explicitly perform detection (screenshot diff + delta classification). Keep one authoritative model; right now the contract is self-contradictory.
As per coding guidelines, "In SKILL.md, enforce the mapping/implementation line: verbs like 'consume, refuse, translate, search-and-adopt, halt, emit' belong in implement; verbs like 'document, detect, inventory, mark, link, capture' belong in mapping skill".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SKILL.md` around lines 319 - 345, B8 currently contains both detection steps
(B8.3/B8.4) and implementation decisions, conflicting with the SKILL.md
verb-role rule; update SKILL.md so detection lives in the mapping skill and
implementation only performs actions on decisions: change B8.3/B8.4 wording to
mapping verbs like "capture", "compare", "generate delta-list" and move any
"detect" / "mark" responsibilities to mapping, and keep B8.5 as the
implement-step for user-driven actions ("Rollback", "Accept", "Update code",
"Update mapping"); ensure the verbs list in the header and the lines for B8.3,
B8.4, and B8.5 consistently use mapping vs implement verbs per the guideline.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three audit-driven
SKILL.mdupdates from internal pre-skill testing + skills.sh comparison. Single PR scope: v0.3 → v0.4.No individual contributor names in any committed file. Verified via
grepbefore commit.Wat dit concreet toevoegt en bijdraagt
Read(target-path)vóór emit, prefer extend > replace, halt-op-twijfel per regel-rangeclassNamealongside Emotion in zelfde element (pre-skill internal testing)tokens.md § Project styling stackbij B4.3, halt direct op tweede styling-APIBewust niet in scope
v0.5 design-fidelity fixes (Resolution-mode 4 inline-fix met audit-trail, asset-materialize primary, fidelity-priority per element) — gecaptured in plan-file, na v0.4 dogfood-evidence.
v1.0 drift loop closing (drifts-implement.md write-authority, drift-summary in chat na emit, decision-routing) — onafhankelijk van mapping coordination via aparte
drifts-implement.mdfile, maar wacht op v0.4 dogfood.Files changed
SKILL.md— frontmatter0.3 → 0.4, B7 + rule Fase 3 templates + v0.3 — resolved beslissingen + scope clausule #3 + B8 herzieningentemplates/post-emit-visual-check.md— B8.1-B8.5 procedure + rollback-prompt templateLESSONS.md— entry 3 [confirmation] over drie fixes met evidenceDEFERRED-FIXES.md— audit items I1, I2, I3 → resolved-decisionsPLAN.md— Owner-veld verwijderd (geen individuele namen)Bloat-check verklaring
Per fix is een Cost/benefit geëxpliciteerd in commit-message. Bewust geschrapt:
Test plan
version: "0.4"in frontmatterRead(target-path)+ extend>replace + halt-on-doubt per regel-rangetemplates/post-emit-visual-check.mdbevat B8.1-B8.5 procedure-blokPLAN.mdheeft geenOwner:veldgrep -r "Pelle\|Kevin Rutten\|<andere namen>" --include="*.md"op deze branch geeft nul hits/figma-to-code-implement <Figma-link>op een echt project; verifieer dat replace-regression, dual-styling en visuele drift daadwerkelijk worden gevangenSummary by CodeRabbit