fix(ai): validate stock photo targets
- Reject text, line, structural, and populated container targets before provider requests\n- Preserve closed shape and Boolean geometry support without mutating target bounds\n- Cover accepted and rejected node families with focused tests
This commit is contained in:
parent
bc8f973c79
commit
90b608863e
|
|
@ -37,6 +37,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- Prevent the stock photo tool from replacing text, lines, structural layers, or containers with content while supporting closed shape geometry.
|
||||
- Preserve explicit text alignment metadata on imported Figma vectors across save and reload.
|
||||
|
||||
- Preserve explicit normal blend modes on imported Figma text and vector nodes across save and reload.
|
||||
|
|
|
|||
|
|
@ -19,8 +19,9 @@ export const stockPhoto = defineTool({
|
|||
name: 'stock_photo',
|
||||
mutates: true,
|
||||
description:
|
||||
'Search stock photos and apply to nodes. Pass a JSON array — all fetched in parallel. ' +
|
||||
'Each item: {id, query, index?, orientation?}. Only works on leaf shapes (Rectangle/Ellipse).',
|
||||
'Search stock photos and apply to leaf image placeholders or closed area geometry. ' +
|
||||
'Pass a JSON array; each item is {id, query, index?, orientation?}. ' +
|
||||
'Containers with content, text, lines, and structural nodes are rejected.',
|
||||
params: {
|
||||
requests: {
|
||||
type: 'string',
|
||||
|
|
|
|||
|
|
@ -1,7 +1,22 @@
|
|||
import type { SceneNode } from '@open-pencil/scene-graph'
|
||||
|
||||
import type { FigmaAPI } from '#core/figma-api'
|
||||
|
||||
import type { StockPhotoProvider, StockPhotoResult } from './providers'
|
||||
|
||||
const STOCK_PHOTO_TARGET_TYPES: ReadonlySet<SceneNode['type']> = new Set([
|
||||
'FRAME',
|
||||
'RECTANGLE',
|
||||
'ROUNDED_RECTANGLE',
|
||||
'ELLIPSE',
|
||||
'STAR',
|
||||
'POLYGON',
|
||||
'VECTOR',
|
||||
'BOOLEAN_OPERATION',
|
||||
'COMPONENT',
|
||||
'INSTANCE'
|
||||
])
|
||||
|
||||
export interface PhotoRequest {
|
||||
id: string
|
||||
query: string
|
||||
|
|
@ -9,10 +24,6 @@ export interface PhotoRequest {
|
|||
orientation?: 'landscape' | 'portrait' | 'square'
|
||||
}
|
||||
|
||||
interface NodeWithChildren {
|
||||
children: unknown[]
|
||||
}
|
||||
|
||||
export interface PhotoResult {
|
||||
id: string
|
||||
photo?: {
|
||||
|
|
@ -33,9 +44,15 @@ export async function applyPhoto(
|
|||
const node = figma.getNodeById(req.id)
|
||||
if (!node) return { id: req.id, error: 'Not found' }
|
||||
|
||||
const children = 'children' in node ? (node as NodeWithChildren).children : []
|
||||
if (children.length > 0) {
|
||||
return { id: req.id, error: `"${node.name}" has children — use a leaf shape` }
|
||||
if (!STOCK_PHOTO_TARGET_TYPES.has(node.type)) {
|
||||
return {
|
||||
id: req.id,
|
||||
error: `"${node.name}" (${node.type}) is not a suitable stock photo target`
|
||||
}
|
||||
}
|
||||
|
||||
if (node.type !== 'BOOLEAN_OPERATION' && node.children.length > 0) {
|
||||
return { id: req.id, error: `"${node.name}" has children — use a leaf image placeholder` }
|
||||
}
|
||||
|
||||
const perPage = Math.min((req.index ?? 0) + 3, 15)
|
||||
|
|
|
|||
|
|
@ -96,14 +96,14 @@ No style={{}}, className, CSS. No named colors or rgb(). No percentage values. N
|
|||
|
||||
# Stock Photos
|
||||
|
||||
`stock_photo` places real Pexels images on leaf shapes (Rectangle/Ellipse). Pass a JSON array — **all photos fetched in parallel**:
|
||||
`stock_photo` places real Pexels images on leaf image placeholders. Prefer Rectangle, Rounded Rectangle, or Ellipse placeholders; closed vector and Boolean geometry are also supported. Pass a JSON array — **all photos fetched in parallel**:
|
||||
|
||||
```
|
||||
stock_photo({ requests: '[{"id":"0:30","query":"wall street trading floor"},{"id":"0:58","query":"AI chip semiconductor"},{"id":"0:65","query":"bank finance credit card"}]' })
|
||||
```
|
||||
|
||||
- Batch all photos in one call — don't call stock_photo 14 times separately
|
||||
- Only apply to leaf shapes (Rectangle/Ellipse), NOT to Frames with children
|
||||
- Only apply to leaf image placeholders, not text, lines, groups, sections, or containers with content
|
||||
- Use descriptive English queries: "aerial city skyline sunset", not "image1"
|
||||
- Orientation: "landscape" (default), "portrait" for tall cards, "square" for avatars
|
||||
- If Pexels key is not configured or returns 401, tell the user to add/check it in AI chat settings. Do NOT fall back to `eval` with manual gradients — leave placeholder colors as-is
|
||||
|
|
|
|||
191
tests/engine/tools/stock-photo/apply.test.ts
Normal file
191
tests/engine/tools/stock-photo/apply.test.ts
Normal file
|
|
@ -0,0 +1,191 @@
|
|||
import { describe, expect, test } from 'bun:test'
|
||||
|
||||
import { FigmaAPI, SceneGraph } from '@open-pencil/core'
|
||||
import type { NodeType } from '@open-pencil/scene-graph'
|
||||
import { copyFills } from '@open-pencil/scene-graph/copy'
|
||||
|
||||
import { applyPhoto } from '#core/tools/stock-photo/apply'
|
||||
import type { StockPhotoProvider } from '#core/tools/stock-photo/providers'
|
||||
|
||||
const PNG_BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])
|
||||
const PHOTO_URL = `data:image/png;base64,${PNG_BYTES.toBase64()}`
|
||||
|
||||
interface ProviderCall {
|
||||
query: string
|
||||
options: {
|
||||
perPage: number
|
||||
orientation: 'landscape' | 'portrait' | 'square'
|
||||
targetDim: number
|
||||
}
|
||||
}
|
||||
|
||||
function createProvider(calls: ProviderCall[]): StockPhotoProvider {
|
||||
return {
|
||||
name: 'test',
|
||||
async search(query, options) {
|
||||
calls.push({ query, options })
|
||||
return [
|
||||
{
|
||||
url: PHOTO_URL,
|
||||
width: 1600,
|
||||
height: 900,
|
||||
photographer: 'Test Photographer',
|
||||
sourceId: 'photo-1'
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function setup() {
|
||||
const graph = new SceneGraph()
|
||||
const page = graph.getPages()[0]
|
||||
if (!page) throw new Error('Expected default page')
|
||||
return { graph, page, figma: new FigmaAPI(graph) }
|
||||
}
|
||||
|
||||
const AREA_TARGET_TYPES = [
|
||||
'FRAME',
|
||||
'RECTANGLE',
|
||||
'ROUNDED_RECTANGLE',
|
||||
'ELLIPSE',
|
||||
'STAR',
|
||||
'POLYGON',
|
||||
'VECTOR',
|
||||
'COMPONENT',
|
||||
'INSTANCE'
|
||||
] as const satisfies readonly NodeType[]
|
||||
|
||||
const UNSUITABLE_TARGET_TYPES = [
|
||||
'CANVAS',
|
||||
'GROUP',
|
||||
'TEXT',
|
||||
'LINE',
|
||||
'SECTION',
|
||||
'COMPONENT_SET',
|
||||
'CONNECTOR',
|
||||
'SHAPE_WITH_TEXT'
|
||||
] as const satisfies readonly NodeType[]
|
||||
|
||||
describe('applyPhoto', () => {
|
||||
for (const type of AREA_TARGET_TYPES) {
|
||||
test(`applies a photo to an empty ${type}`, async () => {
|
||||
const { graph, page, figma } = setup()
|
||||
const node = graph.createNode(type, page.id, {
|
||||
name: `${type} target`,
|
||||
x: 12,
|
||||
y: 24,
|
||||
width: 320,
|
||||
height: 180
|
||||
})
|
||||
const calls: ProviderCall[] = []
|
||||
|
||||
const result = await applyPhoto(figma, createProvider(calls), {
|
||||
id: node.id,
|
||||
query: 'mountain sunset',
|
||||
orientation: 'landscape'
|
||||
})
|
||||
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.photo).toMatchObject({ sourceId: 'photo-1', provider: 'test' })
|
||||
expect(calls).toEqual([
|
||||
{
|
||||
query: 'mountain sunset',
|
||||
options: { perPage: 3, orientation: 'landscape', targetDim: 320 }
|
||||
}
|
||||
])
|
||||
expect(node.fills[0]).toMatchObject({ type: 'IMAGE', imageScaleMode: 'FILL' })
|
||||
expect(graph.images.size).toBe(1)
|
||||
expect({ x: node.x, y: node.y, width: node.width, height: node.height }).toEqual({
|
||||
x: 12,
|
||||
y: 24,
|
||||
width: 320,
|
||||
height: 180
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
test('applies a photo to Boolean geometry with operand children', async () => {
|
||||
const { graph, page, figma } = setup()
|
||||
const operation = graph.createNode('BOOLEAN_OPERATION', page.id, {
|
||||
name: 'Combined shape',
|
||||
width: 200,
|
||||
height: 120
|
||||
})
|
||||
graph.createNode('RECTANGLE', operation.id)
|
||||
graph.createNode('ELLIPSE', operation.id)
|
||||
const calls: ProviderCall[] = []
|
||||
|
||||
const result = await applyPhoto(figma, createProvider(calls), {
|
||||
id: operation.id,
|
||||
query: 'abstract texture'
|
||||
})
|
||||
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(calls).toHaveLength(1)
|
||||
expect(operation.fills[0]).toMatchObject({ type: 'IMAGE', imageScaleMode: 'FILL' })
|
||||
})
|
||||
|
||||
for (const type of UNSUITABLE_TARGET_TYPES) {
|
||||
test(`rejects ${type} before searching`, async () => {
|
||||
const { graph, page, figma } = setup()
|
||||
const node =
|
||||
type === 'CANVAS'
|
||||
? page
|
||||
: graph.createNode(type, page.id, {
|
||||
name: `${type} target`,
|
||||
fills: [
|
||||
{
|
||||
type: 'SOLID',
|
||||
color: { r: 1, g: 0, b: 0, a: 1 },
|
||||
opacity: 1,
|
||||
visible: true
|
||||
}
|
||||
]
|
||||
})
|
||||
const originalFills = copyFills(node.fills)
|
||||
const calls: ProviderCall[] = []
|
||||
|
||||
const result = await applyPhoto(figma, createProvider(calls), {
|
||||
id: node.id,
|
||||
query: 'should not run'
|
||||
})
|
||||
|
||||
expect(result.error).toContain(`(${type}) is not a suitable stock photo target`)
|
||||
expect(calls).toHaveLength(0)
|
||||
expect(node.fills).toEqual(originalFills)
|
||||
expect(graph.images.size).toBe(0)
|
||||
})
|
||||
}
|
||||
|
||||
for (const type of ['FRAME', 'COMPONENT', 'INSTANCE'] as const satisfies readonly NodeType[]) {
|
||||
test(`rejects ${type} content containers before searching`, async () => {
|
||||
const { graph, page, figma } = setup()
|
||||
const container = graph.createNode(type, page.id, {
|
||||
name: `${type} with content`,
|
||||
fills: [
|
||||
{
|
||||
type: 'SOLID',
|
||||
color: { r: 0, g: 0, b: 1, a: 1 },
|
||||
opacity: 1,
|
||||
visible: true
|
||||
}
|
||||
]
|
||||
})
|
||||
const child = graph.createNode('RECTANGLE', container.id)
|
||||
const originalFills = copyFills(container.fills)
|
||||
const calls: ProviderCall[] = []
|
||||
|
||||
const result = await applyPhoto(figma, createProvider(calls), {
|
||||
id: container.id,
|
||||
query: 'should not run'
|
||||
})
|
||||
|
||||
expect(result.error).toBe(`"${container.name}" has children — use a leaf image placeholder`)
|
||||
expect(calls).toHaveLength(0)
|
||||
expect(container.fills).toEqual(originalFills)
|
||||
expect(graph.getNode(child.id)).toBe(child)
|
||||
expect(graph.images.size).toBe(0)
|
||||
})
|
||||
}
|
||||
})
|
||||
Loading…
Reference in a new issue