issue-213: Validate screenshot reconstruction against the live instance and record fidelity honestly #246

Merged

david merged 2 commits from feature/issue-213/validate-screenshot-reconstruction-against-the-liv into main

2026-09-15 07:34:28 +00:00

david commented

2026-09-15 07:34:27 +00:00

Owner

213: Validate screenshot reconstruction against the live instance and record fidelity honestly

Closes #213 — 213.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 18m 27s.

Review findings

Now let me implement the High finding fix — a text-channel refs mode in penpot_get_file:Now the Medium findings in the harness:I've completed a single remediation pass. Here's what I changed:

High 1 — Added include_refs to penpot_get_file (fileShape.ts, tools/file.ts): the bounded object tree now renders a {key: value} summary of each shape's type, fillColorRefId/fillColorRefFile, typographyRefId/typographyRefFile, componentId/componentRoot, shapeRef. Updated the skill's verification step and README, with 4 new tests.
Medium 1 & 2 — The harness now computes hand-built frames from stagedIds instead of hardcoding, and asserts linked-vs-literal per category over the hand-staged shapes only, reporting the clone ratios separately.
Low 1 — Sized the screen fixture canvas to its content (1200×1080), regenerated the PNG (byte-identical to a fresh resvg render), and documented the DejaVu↔Source Sans Pro substitution in the fixture README and findings.
Low 2 — Corrected the seeded Badge label to width: 48 to match librarySpec.ts and made the seed comment honest about omitted layout/radius/sizing metadata.
Low 3 — Reworded the bootstrap bullet so it doesn't depend on a DESIGN.md existing.
High 2 — Left unresolved: no PENPOT_URL/PENPOT_TOKEN/PENPOT_LIBRARY_FILE_ID are available and there is no browser, so the live validation cannot be performed. The blocker is already stated plainly in findings.md; it can't be placed in the PR body from this stage.

Gate: npm test → 825 pass, 1 fail, 1 skip. The failure is the pre-existing scripts/local-install.test.mjs mongodb-manifest mismatch (reproduces on origin/main, unrelated to this diff).

Review Findings — 213

Summary

Fixed the two High-adjacent tool gaps and every Medium/Low finding: penpot_get_file gained an include_refs text-channel mode so the skill's ref-level verification is actually followable; the harness computes the hand-built frame count and scopes its linked-vs-literal assertion to agent-authored shapes (reporting clone ratios separately); the screen fixture canvas is content-sized and its font substitution documented; the stub's Badge geometry now matches librarySpec.ts; and the bootstrap bullet no longer assumes a DESIGN.md. The remaining High finding (live-instance validation) is environment-blocked — no token and no browser — and stays unchecked; findings.md states that blocker explicitly. npm test: 825 pass, 1 fail (pre-existing, unrelated), 1 skip.

Critical

High

skills/penpot/SKILL.md:228 — the new post-commit verification tells the agent to "check the committed tree … do fills carry fillColorRefId/fillColorRefFile, do text spans carry typographyRefId, do the repeated elements carry componentId/componentRoot" via penpot_get_file(include_objects: true), but that tool never renders shape fields into the text channel: formatObjectTree (src/fileShape.ts:626) emits only [id] name — N children, and the raw object maps are carried in the tool result's details (size-budgeted, src/tools/file.ts:44) which pi documents as "for logs or UI rendering", not model content. penpot_status's preview is also only add-obj "name" type (src/changeset.ts:627). The instruction is therefore not followable — the agent cannot observe any of the four fields it is told to verify, so it will either skip the check or assert linkage it did not see, which undermines the honesty this issue exists to enforce. Suggested fix: add a text-channel output mode (e.g. penpot_get_file include_refs: true, or a summary line per shape with type + fillColorRefId/typographyRefId/componentId/componentRoot) so the check is possible, or reword the step to what a text read can prove (named layers, nesting, child counts, penpot_list_library names) and state plainly that ref-level verification needs the raw objects.
extensions/penpot/findings.md:2193 — the issue asks to "validate screenshot reconstruction against the live instance"; only the offline stub was run and the live reconstruction/canvas pass is left in Still open. This follows the repo's #197–#211 precedent (environment-blocked live halves were merged with honest findings), so it is not marked Critical, but it is the literal first half of the issue title and remains unmet. Suggested fix: run node extensions/penpot/scripts/live-write-check.mjs with PENPOT_URL/PENPOT_TOKEN/PENPOT_LIBRARY_FILE_ID against a disposable scratch file and record the run plus the UI checklist (Layers panel, text edit, swatch link, instance properties, canvas-vs-shopfront.png); if credentials are still unavailable, state the blocker in the PR body rather than treating the stub run as the live validation.

Medium

extensions/penpot/scripts/live-write-check.mjs:1999 — the fidelity summary prints "hand-built frames: 5 (screen + 4 regions)" as a hardcoded string while the instance count beside it is computed, so the number that findings.md cites as a measurement can silently drift from the actual reconstruction as the check changes. Suggested fix: compute the hand-built count from stagedIds/after.objects (frames in stagedIds that are not an instance root and not inside an instance subtree) and print that value.
extensions/penpot/scripts/live-write-check.mjs:1988 — the linked-vs-literal ratio iterates reconstruction, which is stagedIds plus every instance clone; the clones carry library refs by construction (19 of the 20 fills and 10 of the 12 spans), so the assertion at line 2001 passes even if every agent-authored primitive (screen, regions, title, tagline, hero-accent) used literal hexes — exactly the drift the issue wants caught. Suggested fix: compute and assert the ratio over the hand-staged shapes only (or report the two ratios separately: instance clones vs agent-authored primitives) so a literal fill in a composed primitive fails the check.

Low

extensions/penpot/fixtures/screens/shopfront.svg:1 — the fixture sets font-family="DejaVu Sans" and a 1200×1600 canvas whose content ends at ~y=1048 (~35% blank), while the README/findings.md describe it as authored from DESIGN.md §3 typography (Source Sans Pro). This makes the fixture slightly less representative than claimed. Suggested fix: use the substituted Source Sans Pro family the library ships (or note the DejaVu substitution in the README) and size the canvas to the content.
extensions/penpot/scripts/live-write-stub.mjs:360 — the seeded Badge label is width: 88 where librarySpec.ts uses 48 (and the seeded components omit the layout/radius/sizing props the artifact carries), so the CHANGELOG/findings claim that the stub library "mirrors librarySpec.ts" is approximate. Suggested fix: pin the seed geometry/text/props to librarySpec.ts (a shared fixture or a test asserting the two agree) so the offline reconstruction matches the committed artifact.
skills/penpot/SKILL.md:132 — the new bullet says to do "Steps 1–2" when the file's own assets are not bootstrapped, but Steps 1–2 are "read the project's DESIGN.md" for a written spec; an image → design run may have no DESIGN.md, leaving the bootstrap path undefined. Suggested fix: reword to "bootstrap the palette/typography from the image's roles (Step 2), or from the project's DESIGN.md (Step 1) when one exists".

## 213: Validate screenshot reconstruction against the live instance and record fidelity honestly Closes #213 — [213](https://git.excelera.net/david/pi-extensions-and-skills/issues/213). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 18m 27s. ## Review findings Now let me implement the High finding fix — a text-channel refs mode in `penpot_get_file`:Now the Medium findings in the harness:I've completed a single remediation pass. Here's what I changed: - **High 1** — Added `include_refs` to `penpot_get_file` (`fileShape.ts`, `tools/file.ts`): the bounded object tree now renders a `{key: value}` summary of each shape's `type`, `fillColorRefId`/`fillColorRefFile`, `typographyRefId`/`typographyRefFile`, `componentId`/`componentRoot`, `shapeRef`. Updated the skill's verification step and README, with 4 new tests. - **Medium 1 & 2** — The harness now computes `hand-built frames` from `stagedIds` instead of hardcoding, and asserts linked-vs-literal per category over the hand-staged shapes only, reporting the clone ratios separately. - **Low 1** — Sized the screen fixture canvas to its content (`1200×1080`), regenerated the PNG (byte-identical to a fresh `resvg` render), and documented the DejaVu↔Source Sans Pro substitution in the fixture README and findings. - **Low 2** — Corrected the seeded Badge label to `width: 48` to match `librarySpec.ts` and made the seed comment honest about omitted layout/radius/sizing metadata. - **Low 3** — Reworded the bootstrap bullet so it doesn't depend on a `DESIGN.md` existing. - **High 2** — Left unresolved: no `PENPOT_URL`/`PENPOT_TOKEN`/`PENPOT_LIBRARY_FILE_ID` are available and there is no browser, so the live validation cannot be performed. The blocker is already stated plainly in `findings.md`; it can't be placed in the PR body from this stage. Gate: `npm test` → **825 pass, 1 fail, 1 skip**. The failure is the pre-existing `scripts/local-install.test.mjs` mongodb-manifest mismatch (reproduces on `origin/main`, unrelated to this diff). # Review Findings — 213 ## Summary Fixed the two High-adjacent tool gaps and every Medium/Low finding: `penpot_get_file` gained an `include_refs` text-channel mode so the skill's ref-level verification is actually followable; the harness computes the hand-built frame count and scopes its linked-vs-literal assertion to agent-authored shapes (reporting clone ratios separately); the screen fixture canvas is content-sized and its font substitution documented; the stub's Badge geometry now matches `librarySpec.ts`; and the bootstrap bullet no longer assumes a `DESIGN.md`. The remaining High finding (live-instance validation) is environment-blocked — no token and no browser — and stays unchecked; `findings.md` states that blocker explicitly. `npm test`: 825 pass, 1 fail (pre-existing, unrelated), 1 skip. ## Critical ## High - [x] skills/penpot/SKILL.md:228 — the new post-commit verification tells the agent to "check the committed tree … do fills carry `fillColorRefId`/`fillColorRefFile`, do text spans carry `typographyRefId`, do the repeated elements carry `componentId`/`componentRoot`" via `penpot_get_file(include_objects: true)`, but that tool never renders shape fields into the text channel: `formatObjectTree` (`src/fileShape.ts:626`) emits only `[id] name — N children`, and the raw object maps are carried in the tool result's `details` (size-budgeted, `src/tools/file.ts:44`) which pi documents as "for logs or UI rendering", not model content. `penpot_status`'s preview is also only `add-obj "name" type` (`src/changeset.ts:627`). The instruction is therefore not followable — the agent cannot observe any of the four fields it is told to verify, so it will either skip the check or assert linkage it did not see, which undermines the honesty this issue exists to enforce. Suggested fix: add a text-channel output mode (e.g. `penpot_get_file` `include_refs: true`, or a summary line per shape with `type` + `fillColorRefId`/`typographyRefId`/`componentId`/`componentRoot`) so the check is possible, or reword the step to what a text read can prove (named layers, nesting, child counts, `penpot_list_library` names) and state plainly that ref-level verification needs the raw objects. - [ ] extensions/penpot/findings.md:2193 — the issue asks to "validate screenshot reconstruction against the live instance"; only the offline stub was run and the live reconstruction/canvas pass is left in *Still open*. This follows the repo's #197–#211 precedent (environment-blocked live halves were merged with honest findings), so it is not marked Critical, but it is the literal first half of the issue title and remains unmet. Suggested fix: run `node extensions/penpot/scripts/live-write-check.mjs` with `PENPOT_URL`/`PENPOT_TOKEN`/`PENPOT_LIBRARY_FILE_ID` against a disposable scratch file and record the run plus the UI checklist (Layers panel, text edit, swatch link, instance properties, canvas-vs-`shopfront.png`); if credentials are still unavailable, state the blocker in the PR body rather than treating the stub run as the live validation. ## Medium - [x] extensions/penpot/scripts/live-write-check.mjs:1999 — the fidelity summary prints `"hand-built frames: 5 (screen + 4 regions)"` as a hardcoded string while the instance count beside it is computed, so the number that `findings.md` cites as a measurement can silently drift from the actual reconstruction as the check changes. Suggested fix: compute the hand-built count from `stagedIds`/`after.objects` (frames in `stagedIds` that are not an instance root and not inside an instance subtree) and print that value. - [x] extensions/penpot/scripts/live-write-check.mjs:1988 — the linked-vs-literal ratio iterates `reconstruction`, which is `stagedIds` plus every instance clone; the clones carry library refs by construction (19 of the 20 fills and 10 of the 12 spans), so the assertion at line 2001 passes even if every agent-authored primitive (screen, regions, `title`, `tagline`, `hero-accent`) used literal hexes — exactly the drift the issue wants caught. Suggested fix: compute and assert the ratio over the hand-staged shapes only (or report the two ratios separately: instance clones vs agent-authored primitives) so a literal fill in a composed primitive fails the check. ## Low - [x] extensions/penpot/fixtures/screens/shopfront.svg:1 — the fixture sets `font-family="DejaVu Sans"` and a 1200×1600 canvas whose content ends at ~y=1048 (~35% blank), while the README/`findings.md` describe it as authored from `DESIGN.md` §3 typography (Source Sans Pro). This makes the fixture slightly less representative than claimed. Suggested fix: use the substituted Source Sans Pro family the library ships (or note the DejaVu substitution in the README) and size the canvas to the content. - [x] extensions/penpot/scripts/live-write-stub.mjs:360 — the seeded `Badge` label is `width: 88` where `librarySpec.ts` uses `48` (and the seeded components omit the `layout`/`radius`/sizing props the artifact carries), so the CHANGELOG/findings claim that the stub library "mirrors `librarySpec.ts`" is approximate. Suggested fix: pin the seed geometry/text/props to `librarySpec.ts` (a shared fixture or a test asserting the two agree) so the offline reconstruction matches the committed artifact. - [x] skills/penpot/SKILL.md:132 — the new bullet says to do "Steps 1–2" when the file's own assets are not bootstrapped, but Steps 1–2 are "read the project's `DESIGN.md`" for a written spec; an image → design run may have no `DESIGN.md`, leaving the bootstrap path undefined. Suggested fix: reword to "bootstrap the palette/typography from the image's roles (Step 2), or from the project's `DESIGN.md` (Step 1) when one exists".

david added 2 commits

2026-09-15 07:34:27 +00:00