fix: Design agent can always access template - #2632
Conversation
|
Here's a visual recap of what changed: Open the full interactive recap |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d83af51 to
d234455
Compare
This comment has been minimized.
This comment has been minimized.
liamdebeasi
changed the title
This comment has been minimized.
This comment has been minimized.
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why |
- Capture quoted CSS font-family declarations - Respect access to linked design systems in get-design-template - Skip malformed Google Fonts URLs instead of throwing - Register get-design-template in the initial agent tool catalog
|
CI just kicked off from the push; checks are pending. All 4 review findings were legitimate and fixed:
Added regression tests for all four, all guards pass, and full test suite is green (unrelated Playwright-browser failures are pre-existing environment issues, not caused by these changes). Replied to each of the 4 review comment threads explaining the fix, and pushed to ⠀ |
There was a problem hiding this comment.
Builder reviewed your changes and found 3 potential issues 🟡
Review Details
Incremental Code Review Summary
The latest commit fixes all four previously reported issues: quoted font-family extraction now supports quoted names, malformed Google Fonts URLs are skipped, linked design-system IDs are access-filtered, and get-design-template is included in the initial tool catalog. Those four review threads were resolved before this review.
New Findings
🟡 MEDIUM
- Invalid JSON in
design.datais coerced to{}, so both template recovery and the required snapshot path can report an ordinary non-template design instead of preserving an explicit unreadable-provenance state. This can cause follow-up edits to lose template constraints. get-design-templateapplies template-level fallback dimensions to every file lacking its own frame, which can copy a sibling screen's dimensions into a multi-screen template. The exact per-file lookup helper explicitly treats missing frames as unknown.
The happy path and access-control fixes are well covered by the new tests, but malformed design metadata and multi-screen dimension fallback need separate handling.
🧪 Browser testing: Will run after this review (PR touches UI code)
| if (!designAccess) throw new Error("Design not found"); | ||
| const design = | ||
| designAccess.resource as typeof schema.designs.$inferSelect; | ||
| const source = readDesignTemplateSource( |
There was a problem hiding this comment.
🟡 Preserve unreadable design metadata as an error
parseDesignTemplateData returns {} when design.data is invalid JSON, so readDesignTemplateSource returns null and this action reports fromTemplate: false. A corrupted template-backed design is therefore indistinguishable from a design that never used a template, allowing the follow-up workflow to lose its constraints; preserve and surface the parse failure instead of treating it as absent provenance.
Additional Info
Reported by 1 of 3 agents; confirmed against parseDesignTemplateData lines 15-23 and the absent-vs-unreadable contract.
| const design = access.resource as typeof schema.designs.$inferSelect; | ||
|
|
||
| const snapshot = await buildDesignSnapshot(designId, design.data); | ||
| const templateSource = readDesignTemplateSource( |
There was a problem hiding this comment.
🟡 Do not silently drop unreadable template provenance
This required follow-up snapshot path also receives {} for invalid design.data, then omits createdFromTemplate entirely. The agent is not told that provenance is unreadable and may edit the design as unconstrained; return an explicit unreadable state or propagate the parse failure rather than silently dropping the template metadata.
Additional Info
Reported by 1 of 3 agents; confirmed against parseDesignTemplateData lines 15-23 and the refinement directive requiring this snapshot.
| files = rows.map((row) => { | ||
| const frame = templateFileDimensions(templateData, row.id); | ||
| return { | ||
| templateFileId: row.id, | ||
| filename: row.filename, | ||
| fileType: row.fileType, | ||
| content: row.content, | ||
| width: frame.width ?? fallbackWidth, |
There was a problem hiding this comment.
🟡 Do not reuse sibling dimensions for unframed template files
For a saved multi-screen template, fallbackWidth/fallbackHeight come from the template's preferred/first frame and are applied to every file whose own canvasFrames[row.id] is missing. That reports another screen's dimensions as authoritative for the unframed file, contrary to templateFileDimensions' exact-lookup contract; return unknown dimensions for missing per-file frames (or restrict fallback to an explicitly identified legacy single-file case).
Additional Info
Reported by 1 of 3 agents; confirmed against the exact lookup helper and the mapping at lines 141-150.
3 participants
The agent did not have access to the original template on subsequent prompts, so asking it to make multiple changes would result in quality degradation. This PR fixes this by ensuring the agent is always able to access the original template.