Skip to content

Conversation

@juanfran
Copy link

@juanfran juanfran commented Nov 4, 2025

No description provided.

Copy link
Collaborator

@opcode81 opcode81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made some suggestions.

Also, I noticed that the diff includes changes that are only whitespace changes. Did you apply an auto-formatter to this file?

Comment on lines +57 to +60
CRITICAL WORKFLOW - Follow these steps when working with Penpot designs:
1. ALWAYS use `penpotUtils` helper functions - NEVER reimplement shape searching
2. When working with unknown designs, start with `penpotUtils.shapeStructure(penpot.root, 3)` to understand hierarchy
3. Use `penpotUtils.findShapes()` or `penpotUtils.findShape()` with predicates to locate elements
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just general pointers, not a strict workflow sequence that the model should follow, so I would word it accordingly, use bullet items and not present it as a sequence of steps that must be followed, i.e. we should probably not call this a "critical workflow" or use caps.

Using penpotUtils.shapeStructure(penpot.root, 3) to get an overview of the design is perhaps misleading, as penpot.root is only the root of the current page. The model generally needs to determine the pages first (and at least Claude did not have any problems with this). We could suggest using the command to get an overview of a single page though.

Comment on lines +82 to +88
CRITICAL WARNINGS ABOUT IMAGES:
* Images appear in TWO ways in Penpot:
1. As standalone `Image` shapes (shape.type === 'image')
2. As fills on other shapes (Rectangle, Ellipse, etc.) via `fillImage` property
* Most images in designs are rectangles/shapes with image fills, NOT Image type shapes!
* To find ALL images, you MUST check both: shape.type === 'image' OR shape.fills with fillImage
* NEVER assume shape.type === 'image' is the only way images appear
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't thinkt his needs to be a "critical warning". The model did not explicitly have this information before, and adding it plainly should suffice. Let's reserve the strong words and all-caps for cases where a normal tone definitely does not work.

Also, given the information above, the last point seems redundant. Was it really necessary to add it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants