feat: AIRE — AI-powered node rename tab for Figma plugin#31
Open
jacksonkasi1 wants to merge 8 commits intomainfrom
Open
feat: AIRE — AI-powered node rename tab for Figma plugin#31jacksonkasi1 wants to merge 8 commits intomainfrom
jacksonkasi1 wants to merge 8 commits intomainfrom
Conversation
- Add 'AI' tab to plugin tab switch - Add types: AIModelProvider, AIRenameStatus, AISettings, AIRenameGroup, AIRenameResult, AIState - Add AI event handler types: AIRenameRequest, AIBatchReady, AIApplyRename, AIRenameProgress, AIRenameComplete, AIRenameError - Add useAIStore (Zustand) with settings, statuses, progress, isRunning - Add core/ai: svg-detector, section-grouper, export-for-ai, rename-handler - Add AIPage with AINodeList, AISettingsPanel, AIRunButton, AIStatusBadge components - Wire full flow: AI_RENAME_REQUEST → classify/group/export → AI_BATCH_READY → POST /api/ai/rename-batch → AI_APPLY_RENAME → AI_RENAME_PROGRESS - Add server route POST /api/ai/rename-batch with GeminiAdapter (OpenAI/Anthropic stubs) - Add server promise-pool utility (max 5 concurrent AI calls) - Persist AI settings via useStorageManager (clientStorage) - Fix IconButton: rename loading prop to isLoading to avoid JSX img attribute collision - Plugin build: passes TypeScript typecheck clean
Reviewer's GuideImplements an end-to-end AI-powered rename flow for the ImagePro Figma plugin by adding an AI tab and UI, a Figma-side orchestration pipeline (classify → group → optional export → emit), and a server-side Gemini-based rename-batch API with a pluggable model adapter layer, plus some utility and styling adjustments. Sequence diagram for AI-powered rename flow from Figma UI to GeminisequenceDiagram
title AI-powered node rename end-to-end flow
actor Designer
participant AIPage as PluginAIPageUI
participant AIS as AIStore
participant Main as FigmaMainProcess
participant AIR as AIRenameRouter
participant Factory as ModelAdapterFactory
participant Adapter as GeminiAdapter
participant Gemini as GoogleGenAIAPI
Designer->>AIPage: Click Run AIRunButton
AIPage->>AIS: validate settings and selection
AIPage->>Main: emit AI_RENAME_REQUEST(nodeIds, readImage, settings)
Main->>Main: handleAIRenameRequest(payload)
Main->>Main: collectTargetNodes(nodeIds)
Main->>Main: classifyNode(node)
Main->>Main: buildRenameGroupsFromNodes(targets)
Main->>Main: sequentialPool(exportTasks, 2)
Main->>Main: exportNodeForAI(contextNode)
Main-->>AIPage: emit AI_BATCH_READY(groups)
AIPage->>AIS: setProgress(done=0, total)
AIPage->>AIS: setRenameStatus(nodeId, pending)
AIPage->>AIR: POST /api/ai/rename-batch {groups, settings}
AIR->>Factory: create(modelProvider, apiKey, model)
Factory-->>AIR: IModelAdapter (GeminiAdapter)
AIR->>Adapter: rename(groups, systemPrompt, caseOption, 5)
Adapter->>Adapter: buildUserPrompt(group, caseOption)
Adapter->>Gemini: generateContent(model, systemPrompt, parts)
Gemini-->>Adapter: content response
Adapter->>Adapter: parseResponse(text, group)
Adapter-->>AIR: AIRenameResult[]
AIR-->>AIPage: {renames: AIRenameResult[]}
loop For each AIRenameResult
AIPage->>Main: emit AI_APPLY_RENAME(rename)
Main->>Main: applyRename(nodeId, suggestedName)
Main->>Main: node.name = suggestedName
Main-->>AIPage: emit AI_RENAME_PROGRESS({nodeId, newName, status})
AIPage->>AIS: setRenameStatus(nodeId, status, newName)
AIPage->>AIS: update progress.done
end
Main-->>AIPage: emit AI_RENAME_COMPLETE
AIPage->>AIS: setIsRunning(false)
Class diagram for plugin AI types and storeclassDiagram
title Plugin-side AI types settings and Zustand store
class AISettings {
+AIModelProvider modelProvider
+string model
+string apiKey
+string systemPrompt
+boolean readImage
+CaseOption caseOption
}
class AIRenameGroupClient {
+string groupId
+string contextText
+string imageBase64
+AIRenameTargetClient[] targetNodes
}
class AIRenameTargetClient {
+string nodeId
+string currentName
+string nodeType
+boolean isSvg
}
class AIRenameResultClient {
+string nodeId
+string suggestedName
}
class AIState {
+AISettings settings
+map~string, AIRenameStatus~ renameStatuses
+map~string, string~ renamedNames
+boolean isRunning
+boolean isSettingsOpen
+Progress progress
+setSettings(partial: AISettings)
+setRenameStatus(nodeId: string, status: AIRenameStatus, newName: string)
+resetStatuses()
+setIsRunning(running: boolean)
+setIsSettingsOpen(open: boolean)
+setProgress(progress: Progress)
}
class Progress {
+number done
+number total
}
class useAIStore {
<<ZustandStore>>
+AIState state
}
class AINodeClassification {
<<enum>>
svg_leaf
raster_leaf
section
}
class AIRenameStatus {
<<enum>>
idle
pending
done
error
}
AIState --> AISettings
AIState --> Progress
useAIStore --> AIState
AIRenameGroupClient --> AIRenameTargetClient
Class diagram for server-side AI adapters and typesclassDiagram
title Server-side AI adapter layer and shared types
class AIRenameTargetNode {
+string nodeId
+string currentName
+string nodeType
+boolean isSvg
}
class AIRenameGroupServer {
+string groupId
+string contextText
+string imageBase64
+AIRenameTargetNode[] targetNodes
}
class AIRenameResultServer {
+string nodeId
+string suggestedName
}
class AIRenameBatchRequest {
+AIRenameGroupServer[] groups
+AISettingsServer settings
}
class AISettingsServer {
+AIModelProvider modelProvider
+string model
+string apiKey
+string systemPrompt
+string caseOption
}
class IModelAdapter {
<<interface>>
+rename(groups: AIRenameGroupServer[], systemPrompt: string, caseOption: string, concurrency: number) AIRenameResultServer[]
}
class GeminiAdapter {
-GoogleGenAI ai
-string model
+GeminiAdapter(apiKey: string, model: string)
+rename(groups: AIRenameGroupServer[], systemPrompt: string, caseOption: string, concurrency: number) AIRenameResultServer[]
-renameGroup(group: AIRenameGroupServer, systemPrompt: string, caseOption: string) AIRenameResultServer[]
-buildUserPrompt(group: AIRenameGroupServer, caseOption: string) string
-parseResponse(text: string, group: AIRenameGroupServer) AIRenameResultServer[]
}
class OpenAIAdapter {
+OpenAIAdapter(apiKey: string, model: string)
+rename(groups: AIRenameGroupServer[], systemPrompt: string, caseOption: string, concurrency: number) AIRenameResultServer[]
}
class AnthropicAdapter {
+AnthropicAdapter(apiKey: string, model: string)
+rename(groups: AIRenameGroupServer[], systemPrompt: string, caseOption: string, concurrency: number) AIRenameResultServer[]
}
class ModelAdapterFactory {
+create(provider: AIModelProvider, apiKey: string, model: string) IModelAdapter
}
class promisePoolUtil {
+promisePool(tasks: Function[], concurrency: number) Promise~any[]~
}
AIRenameBatchRequest --> AIRenameGroupServer
AIRenameBatchRequest --> AISettingsServer
AIRenameGroupServer --> AIRenameTargetNode
IModelAdapter <|.. GeminiAdapter
IModelAdapter <|.. OpenAIAdapter
IModelAdapter <|.. AnthropicAdapter
ModelAdapterFactory ..> IModelAdapter
GeminiAdapter ..> promisePoolUtil
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a full AI-assisted rename workflow: new AI tab and UI components, plugin-side node classification/grouping and low-quality image export, event-driven orchestration (AI_RENAME_REQUEST → AI_BATCH_READY → AI_APPLY_RENAME), and a server POST /ai/rename-batch with a ModelAdapter factory (Gemini, OpenAI, Anthropic stubs). Changes
sequenceDiagram
actor User
participant UI as Plugin UI
participant Main as Plugin Main
participant Server as AI Server
participant Adapter as ModelAdapter
participant Provider as AI Provider
User->>UI: select nodes & click "Rename"
UI->>Main: AI_RENAME_REQUEST(nodeIds, readImage, settings)
Main->>Main: classifyNode / buildRenameGroups
Main->>Main: exportNodeForAI (throttled ≤2 concurrent)
Main->>Server: POST /ai/rename-batch { groups, settings }
Server->>Adapter: adapter.rename(groups, systemPrompt, caseOption)
loop per-group (server concurrency)
Adapter->>Provider: send prompt + optional image
Provider-->>Adapter: JSON array of renames
Adapter-->>Server: parsed AIRenameResult[]
end
Server-->>Main: { renames }
Main->>Main: applyRename per node -> emit AI_RENAME_PROGRESS / AI_RENAME_ERROR
Main->>UI: AI_RENAME_PROGRESS / AI_RENAME_COMPLETE
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The AIPage currently hardcodes
SERVER_URL = 'http://localhost:3000', which will break in non-local environments; consider sourcing this from configuration (e.g., env or a shared config module) and handling missing/misconfigured URLs gracefully. - In
GeminiAdapter.rename/renameGroup, thecaseOptionis only fed into the prompt and not enforced on the returned names; consider adding a small case-conversion helper to normalizesuggestedNameclient-side so naming conventions are guaranteed even if the model responds inconsistently. - In
AINodeList,URL.createObjectURLis called for each node render without ever revoking the URLs, which can leak memory over time; consider memoizing blob URLs per node and revoking them in a cleanup effect when the component or item unmounts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The AIPage currently hardcodes `SERVER_URL = 'http://localhost:3000'`, which will break in non-local environments; consider sourcing this from configuration (e.g., env or a shared config module) and handling missing/misconfigured URLs gracefully.
- In `GeminiAdapter.rename/renameGroup`, the `caseOption` is only fed into the prompt and not enforced on the returned names; consider adding a small case-conversion helper to normalize `suggestedName` client-side so naming conventions are guaranteed even if the model responds inconsistently.
- In `AINodeList`, `URL.createObjectURL` is called for each node render without ever revoking the URLs, which can leak memory over time; consider memoizing blob URLs per node and revoking them in a cleanup effect when the component or item unmounts.
## Individual Comments
### Comment 1
<location path="server/imagepro-file-process/src/routes/ai/adapters/gemini.ts" line_range="60-69" />
<code_context>
+ }
+
+ try {
+ const response = await this.ai.models.generateContent({
+ model: this.model,
+ contents: [{ role: 'user', parts }],
+ config: {
+ systemInstruction: systemPrompt,
+ temperature: 0.2, // Low temp for consistent, deterministic names
+ maxOutputTokens: 512,
+ },
+ });
+
+ const text = response.text ?? '';
+ return this.parseResponse(text, group);
+ } catch (error) {
</code_context>
<issue_to_address>
**issue (bug_risk):** The Gemini SDK usage looks off and may not match @google/genai’s actual API surface.
This call shape doesn’t match the current @google/genai 1.43.0 API: recent versions use `client.responses.generate({ model, contents, systemInstruction, generationConfig })`, and text is accessed via something like `result.response.text()` / `result.output_text`, not `response.text`. Please align with the pinned SDK version by updating the method (`models.generateContent` vs `responses.generate`), the option names (`config` vs `generationConfig` / `systemInstruction`), and how the text is read, so this doesn’t fail at runtime.
</issue_to_address>
### Comment 2
<location path="plugin/src/pages/AIPage/index.tsx" line_range="31-32" />
<code_context>
+import AISettingsPanel from './_components/AISettingsPanel';
+import AIRunButton from './_components/AIRunButton';
+
+// Server base URL — adjust via env if needed
+const SERVER_URL = 'http://localhost:3000';
+
+const AIPage = () => {
</code_context>
<issue_to_address>
**issue (bug_risk):** Hardcoding the server URL to localhost will break in non-local environments.
This literal `http://localhost:3000` ties the UI to a dev server and will cause the AI rename flow to fail in production or when the server runs elsewhere.
Instead, read the base URL from configuration that’s available in the plugin context (e.g., build‑time env var or a runtime config object passed in), with `localhost` as a fallback only when nothing is provided.
</issue_to_address>
### Comment 3
<location path="plugin/src/pages/AIPage/_components/AINodeList.tsx" line_range="39-40" />
<code_context>
+ isSelected: selectedNodeIds.includes(node.id),
+ status: renameStatuses[node.id] ?? 'idle',
+ newName: renamedNames[node.id],
+ thumbnail: node.imageData
+ ? URL.createObjectURL(new Blob([node.imageData], { type: 'image/png' }))
+ : null,
+ })),
</code_context>
<issue_to_address>
**issue (performance):** Object URLs for thumbnails are never revoked, which can leak memory for large selections.
Creating a new `URL.createObjectURL` on every render without ever calling `URL.revokeObjectURL` will leak memory as nodes are added/removed or the selection changes.
Consider memoizing the URL per node and cleaning it up in an effect (e.g., track `{id, url}` in state and revoke when the node is removed or on unmount). Alternatively, generate and manage the blob URL in the layer where `imageData` is produced so its lifecycle is explicitly controlled.
</issue_to_address>
### Comment 4
<location path="server/imagepro-file-process/src/routes/ai/adapters/gemini.ts" line_range="25-31" />
<code_context>
+export class AnthropicAdapter implements IModelAdapter {
+ constructor(_apiKey: string, _model: string) {}
+
+ async rename(
+ groups: AIRenameGroup[],
+ _systemPrompt: string,
</code_context>
<issue_to_address>
**suggestion:** The naming convention is only enforced via prompt; it might be safer to apply caseOption in code too.
Since `caseOption` is only used in the prompt, you’re relying on the model to format `suggestedName` correctly, which can lead to inconsistent casing. Consider extending `sanitizeName` (or a similar post-processing step) to normalize the returned name to the requested `caseOption` (kebab, snake, camel, Pascal), treating the model output as the raw label and enforcing the final format in code.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+60
to
+69
| const response = await this.ai.models.generateContent({ | ||
| model: this.model, | ||
| contents: [{ role: 'user', parts }], | ||
| config: { | ||
| systemInstruction: systemPrompt, | ||
| temperature: 0.2, // Low temp for consistent, deterministic names | ||
| maxOutputTokens: 512, | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
issue (bug_risk): The Gemini SDK usage looks off and may not match @google/genai’s actual API surface.
This call shape doesn’t match the current @google/genai 1.43.0 API: recent versions use client.responses.generate({ model, contents, systemInstruction, generationConfig }), and text is accessed via something like result.response.text() / result.output_text, not response.text. Please align with the pinned SDK version by updating the method (models.generateContent vs responses.generate), the option names (config vs generationConfig / systemInstruction), and how the text is read, so this doesn’t fail at runtime.
Comment on lines
+25
to
+31
| async rename( | ||
| groups: AIRenameGroup[], | ||
| systemPrompt: string, | ||
| caseOption: string, | ||
| concurrency = 5 | ||
| ): Promise<AIRenameResult[]> { | ||
| const tasks = groups.map((group) => () => this.renameGroup(group, systemPrompt, caseOption)); |
There was a problem hiding this comment.
suggestion: The naming convention is only enforced via prompt; it might be safer to apply caseOption in code too.
Since caseOption is only used in the prompt, you’re relying on the model to format suggestedName correctly, which can lead to inconsistent casing. Consider extending sanitizeName (or a similar post-processing step) to normalize the returned name to the requested caseOption (kebab, snake, camel, Pascal), treating the model output as the raw label and enforcing the final format in code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/src/hooks/useStorageManager.ts (1)
23-33:⚠️ Potential issue | 🟠 Major
aiSettingshydration will not apply becausehandleis missing inRECEIVE_DATA.This sync path depends on
handle, butplugin/src/main.tsLine 65 emits only{ data }. As a result,stores.find((s) => s.storageKey === handle)won’t match and persisted state is skipped.🔧 Proposed fix (in
plugin/src/main.ts)on<GetDataHandler>('GET_DATA', async ({ handle }) => { const data = await figma.clientStorage.getAsync(handle); - emit('RECEIVE_DATA', { data }); + emit('RECEIVE_DATA', { handle, data }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/hooks/useStorageManager.ts` around lines 23 - 33, The RECEIVE_DATA flow fails because handleReceiveData expects a {handle, data} payload but the emitter only sends {data}; update the emitter so it sends the storage handle along with data (e.g., { handle: 'aiSettings' or 'historyStore', data }) so stores.find(s => s.storageKey === handle) matches; this will ensure handleReceiveData (ReceiveDataHandler) can locate the entry in the stores array (storageKey 'aiSettings' / 'historyStore') and call the appropriate storeApi.setState (useAIStore / useHistoryStore).
🧹 Nitpick comments (10)
plan.md (1)
1-282: Excellent design documentation.The plan clearly documents architecture, data flow, algorithms, and coding standards. This will be valuable for onboarding and future maintenance.
Minor nit: Several code blocks lack language specifiers (e.g., lines 19, 46, 85). Adding identifiers like
text,plaintext, orpseudocodewould satisfy linters and improve syntax highlighting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plan.md` around lines 1 - 282, The markdown has several fenced code blocks missing language specifiers (e.g., the UX layout block, the classifyNode() pseudocode, groupNodesByParent(), promisePool(), and Default System Prompt blocks); update each fenced block to include an appropriate language identifier such as "text", "pseudocode", or "json" to satisfy linters and improve highlighting—specifically edit the blocks around "UX Layout", "classifyNode()", "groupNodesByParent()", "promisePool()", and the Default System Prompt so their opening fences include the chosen specifier.server/imagepro-file-process/src/routes/ai/adapters/types.ts (1)
1-40: Well-structured adapter types.The type definitions establish a clean contract for the adapter pattern. The
IModelAdapterinterface properly abstracts provider-specific implementations.Consider sharing these types between plugin and server (e.g., via a shared package) to avoid drift between
plugin/src/types/ai.tsand this file, as both define similar structures likeAIRenameGroupandAIRenameResult.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/imagepro-file-process/src/routes/ai/adapters/types.ts` around lines 1 - 40, The types in this file (AIModelProvider, AIRenameTargetNode, AIRenameGroup, AIRenameResult, AIRenameBatchRequest, and IModelAdapter) duplicate similar definitions in plugin/src/types/ai.ts; to prevent drift, extract and publish a single shared types module (e.g., a shared package or workspace package) and replace the local declarations with imports from that shared module, update all references (rename usages of AIRenameGroup, AIRenameResult, AIRenameBatchRequest, and IModelAdapter to use the shared exports) and remove the duplicate definitions here so both server and plugin consume the same authoritative types.todo.md (1)
5-53: TODO checklist appears out of sync with implementation.Many items listed as "not started" (
[ ]) have been implemented in this PR (e.g., AI tab option, adapters, types). Consider updating the checkboxes to reflect actual progress, or remove this file if it was only used for planning.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@todo.md` around lines 5 - 53, The TODO checklist in todo.md is out of sync; update the file to reflect actual work by marking implemented items as completed (e.g., TabSwitch `ai` option, `AIPage` registration, created types like `AISettings`/`AIRenameResult`, store `use-ai-store`, page components such as `AIStatusBadge`/`AINodeList`/`AISettingsPanel`/`AIRunButton`, core/ai modules like `svg-detector`/`section-grouper`/`export-for-ai`/`rename-handler`, server adapters (e.g., `server/src/routes/ai/adapters/gemini.ts`) and routes, or remove the file if it was only planning notes); ensure each changed checklist entry references the matching implemented artifact names (AIPage, use-ai-store, core/ai/*, server/src/routes/ai/*, etc.) and commit the updated todo.md so the PR reflects current progress.plugin/src/pages/AIPage/_components/AISettingsPanel.tsx (1)
1-1: Remove unusedhimport.Same as
AINodeList.tsx— thehimport is not used.🧹 Proposed fix
-import { h } from 'preact'; +// preact JSX is auto-transformed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx` at line 1, The import "h" from 'preact' in AISettingsPanel.tsx is unused; remove the unused import line (the same change done in AINodeList.tsx) so the file no longer imports h unnecessarily and linter/no-unused-vars errors are resolved—edit the top of AISettingsPanel.tsx to delete the "import { h } from 'preact';" statement.plugin/src/pages/AIPage/index.tsx (1)
63-63: Lint warnings:forEachcallbacks should not return values.The callbacks implicitly return the result of
setRenameStatus. While functionally harmless, this triggers lint warnings. Use a block body to make the void return explicit.🧹 Proposed fix
// Mark all as pending - groups.forEach((g) => g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'pending'))); + groups.forEach((g) => { + g.targetNodes.forEach((t) => { + setRenameStatus(t.nodeId, 'pending'); + }); + }); // ... later in catch block ... // Mark all pending as error - groups.forEach((g) => - g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'error')) - ); + groups.forEach((g) => { + g.targetNodes.forEach((t) => { + setRenameStatus(t.nodeId, 'error'); + }); + });Also applies to: 94-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/index.tsx` at line 63, The forEach callbacks on groups that call setRenameStatus are implicitly returning its value which triggers lint warnings; update the callbacks to use a block body (e.g., groups.forEach((g) => { g.targetNodes.forEach((t) => { setRenameStatus(t.nodeId, 'pending'); }); });) so the return is explicit void and repeat the same change for the similar occurrences around the setRenameStatus calls at the other site (lines referenced as 94-95) to eliminate the lint warnings.plugin/src/core/ai/export-for-ai.ts (1)
24-40: Consider handling case where fallback image still exceeds threshold.If the 0.25x scale export still exceeds 150KB (e.g., very large or complex nodes), the oversized image is still sent. This could impact API costs and latency.
A third-tier fallback (skip image entirely) or a warning log could be helpful, though this may be acceptable for the MVP.
💡 Optional: Add final size check
if (imageData.byteLength > MAX_BYTES_THRESHOLD) { imageData = await node.exportAsync({ format: 'JPG', constraint: { type: 'SCALE', value: SCALE_FALLBACK }, }); + // If still too large, skip image to avoid excessive payload + if (imageData.byteLength > MAX_BYTES_THRESHOLD) { + console.warn(`[exportNodeForAI] Image still too large after fallback (${imageData.byteLength} bytes), skipping`); + return ''; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/export-for-ai.ts` around lines 24 - 40, The export logic currently retries at SCALE_FALLBACK if the initial node.exportAsync (used with SCALE_NORMAL) exceeds MAX_BYTES_THRESHOLD but doesn't check the size after the fallback; add a final size check after the second exportAsync call and, if imageData.byteLength still > MAX_BYTES_THRESHOLD, skip sending the image (e.g., return empty string) or emit a warning via the existing logger, so oversized images are never passed through; update the code around node.exportAsync, SCALE_NORMAL, SCALE_FALLBACK, and MAX_BYTES_THRESHOLD to perform this final check and early-return/log as appropriate.plugin/src/core/ai/svg-detector.ts (1)
9-17: Consider whether RECTANGLE should always be classified as vector primitive.A
RECTANGLEnode can have an image fill (raster content), but it's currently classified as a vector primitive. This means a frame containing only image-filled rectangles would be classified assvg_leafrather thanraster_leaf.This may be acceptable depending on the intended behavior, but worth noting that the classification is based on node type rather than content.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/svg-detector.ts` around lines 9 - 17, The VECTOR_PRIMITIVE_TYPES set currently treats 'RECTANGLE' as always a vector primitive which misclassifies rectangles with image fills; update the classification logic instead of only the VECTOR_PRIMITIVE_TYPES constant: keep 'RECTANGLE' removable from VECTOR_PRIMITIVE_TYPES (or leave but treat specially) and modify the code paths that use VECTOR_PRIMITIVE_TYPES (e.g., the function that checks node type for svg vs raster) to inspect a Rectangle node's fills for image/fill.type === 'IMAGE' (or equivalent FILL_KIND) and treat such rectangles as raster_leaf; reference VECTOR_PRIMITIVE_TYPES and the Rectangle node handling code so the decision is based on content (fills) for rectangles.plugin/src/pages/AIPage/_components/AINodeList.tsx (1)
1-1: Remove unusedhimport.The
himport is not used since JSX is transformed automatically.🧹 Proposed fix
-import { h, Fragment } from 'preact'; +import { Fragment } from 'preact';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/_components/AINodeList.tsx` at line 1, Remove the unused `h` import from the preact import in AINodeList.tsx: locate the import statement that currently reads `import { h, Fragment } from 'preact';` and drop `h` so it becomes `import { Fragment } from 'preact';` (this removes the unused symbol while keeping `Fragment` used by the AINodeList component).plugin/src/types/ai.ts (1)
34-43: Extract a namedAIRenameTargetNodetype to prevent contract drift.The inline
targetNodesobject shape is repeated elsewhere; promoting it to a named interface improves reuse and keeps client/server mapping safer.♻️ Suggested refactor
+export interface AIRenameTargetNode { + nodeId: string; + currentName: string; + nodeType: string; + isSvg: boolean; +} + export interface AIRenameGroup { groupId: string; contextText: string; imageBase64?: string; - targetNodes: Array<{ - nodeId: string; - currentName: string; - nodeType: string; - isSvg: boolean; - }>; + targetNodes: AIRenameTargetNode[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/types/ai.ts` around lines 34 - 43, The AIRenameGroup interface uses an inline object type for targetNodes which is duplicated elsewhere; define and export a named interface AIRenameTargetNode (with nodeId, currentName, nodeType, isSvg) and replace the inline Array<{...}> in AIRenameGroup with Array<AIRenameTargetNode>; update any other files or type usages that previously duplicated the shape to reference AIRenameTargetNode to keep client/server contracts consistent.plugin/src/types/events.ts (1)
58-61: Remove duplicatereadImagefrom request payload shape.
AISettingsalready carriesreadImage, so keeping it both inpayload.readImageandpayload.settings.readImagecreates drift risk.♻️ Suggested contract simplification
export interface AIRenameRequestHandler extends EventHandler { name: 'AI_RENAME_REQUEST'; - handler: (payload: { nodeIds: string[]; readImage: boolean; settings: AISettings }) => void; + handler: (payload: { nodeIds: string[]; settings: AISettings }) => void; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/types/events.ts` around lines 58 - 61, The payload currently duplicates readImage — update the AIRenameRequestHandler type so its handler signature removes the top-level readImage and uses only settings: AISettings (i.e., change handler: (payload: { nodeIds: string[]; settings: AISettings }) => void), then search for all usages of AIRenameRequestHandler/AIRename_REQUEST event handlers and callers to stop passing payload.readImage (instead reference payload.settings.readImage) or remove the extra arg; ensure any type annotations and tests are updated to the single-source-of-truth in AISettings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/src/core/ai/rename-handler.ts`:
- Around line 105-135: The current exportTasks closure pushes finalGroup into
the shared array groups, causing nondeterministic order depending on task
completion; change to preserve rawGroups order by creating tasks that capture
the source index (from rawGroups.map((group, idx) => async () => { ... })) and
assign directly into groups[idx] instead of pushing; also initialize groups to
the correct length (e.g., new Array(rawGroups.length)) before running
sequentialPool, and keep the rest of the logic inside the task (use
exportNodeForAI, contextNode lookup, imageBase64, and finalGroup) unchanged so
result ordering matches rawGroups deterministically.
- Around line 94-99: The early returns in renameHandler (when nodeIds.length ===
0 and when collectTargetNodes(nodeIds) returns no targets) leave the UI stuck
because setIsRunning(true) is not cleared; update the function to emit a
failure/completion event instead of simple returns—use the existing event
constants AI_RENAME_ERROR (or emit AI_RENAME_COMPLETE/AI_BATCH_READY as
appropriate) and include an error payload or reason so the caller can
setIsRunning(false); additionally wrap the call site listener in main.ts with a
try/catch that catches unexpected errors from renameHandler and emits
AI_RENAME_ERROR (or triggers a guaranteed cleanup event) to ensure the running
state is always reset.
In `@plugin/src/core/ai/section-grouper.ts`:
- Line 2: The import line in section-grouper.ts includes an unused symbol
AINodeContextChild; remove AINodeContextChild from the import so only
AIRenameGroup is imported, and verify there are no remaining references to
AINodeContextChild elsewhere in this module (e.g., in functions or type
annotations) before committing the change.
In `@plugin/src/main.ts`:
- Line 10: The import list in main.ts includes an unused symbol
signalRenameComplete; remove signalRenameComplete from the import statement that
currently reads "handleAIRenameRequest, applyRename, signalRenameComplete" so
only the actually used symbols (handleAIRenameRequest and applyRename) are
imported, or alternatively wire signalRenameComplete into the code where
intended; update the import to match the used identifiers to eliminate the
unused import.
In `@plugin/src/pages/AIPage/_components/AINodeList.tsx`:
- Around line 32-44: The thumbnail URLs created in the nodeItems useMemo via
URL.createObjectURL are never revoked, causing a memory leak; in the AINodeList
component, move URL.createObjectURL usage into a useEffect that runs when
allNodes (or the relevant image data list) changes and generate/store object
URLs (e.g., in state or a ref), and in that effect return a cleanup that calls
URL.revokeObjectURL for each previously created URL and also revoke on unmount;
update nodeItems to reference these managed URLs instead of calling
createObjectURL directly so all object URLs are revoked when dependencies change
or the component unmounts.
In `@plugin/src/pages/AIPage/index.tsx`:
- Around line 31-32: Replace the hardcoded SERVER_URL constant with a
configurable value sourced from an environment/config variable (e.g.,
process.env.REACT_APP_SERVER_URL or import from a config module) so the app can
use different endpoints per environment; update the SERVER_URL declaration in
plugin/src/pages/AIPage/index.tsx to read the env var with a sensible default
fallback and ensure any usages of SERVER_URL continue to work with the new
constant name.
- Around line 52-141: The issue is that finalize is referenced inside the
useEffect before it's defined and omitted from the effect's dependency array;
move the finalize function (the useCallback that calls setIsRunning, resets
pendingRef, and any cleanup logic) above the useEffect so the effect captures
the correct reference, remove the eslint-disable comment, and include finalize
in the effect dependency array (or inline its logic into the effect) to ensure
no stale closures; update any references to pendingRef, setIsRunning, and
useAIStore inside finalize so they remain consistent.
In `@plugin/src/types/ai.ts`:
- Around line 10-17: The AISettings interface currently includes a secret field
apiKey which causes API keys to be stored in persisted aiSettings; remove apiKey
from the persisted contract (AISettings) and instead store it in a runtime-only
structure (e.g., RuntimeAISettings or session-scoped storage) that is kept
in-memory and never serialized; update any code referencing AISettings.apiKey to
read from the new runtime-only holder and ensure persistence/serialization paths
only use the sanitized AISettings shape (modelProvider, model, systemPrompt,
readImage, caseOption).
In `@plugin/src/types/events.ts`:
- Around line 73-76: The AIRenameProgressHandler's handler currently requires
newName even when status is 'error'; change the update payload to a
discriminated union so newName is required for { status: 'done' } but
omitted/optional for { status: 'error' } (e.g. update: { nodeId: string; status:
'done'; newName: string } | { nodeId: string; status: 'error' }). Update the
AIRenameProgressHandler.handler signature accordingly.
In `@server/imagepro-file-process/src/routes/ai/adapters/gemini.ts`:
- Around line 101-137: parseResponse currently returns only the validated parsed
items which can omit some group.targetNodes, causing silent skips downstream
(see renameGroup). Modify parseResponse to build a map of sanitized parsed
results (use parsedMap) keyed by nodeId from the parsed array (after the
existing validation), then return group.targetNodes.map(...) that for each
nodeId returns parsedMap.get(nodeId) ?? node.currentName so every target node
has a suggestion fallback; keep existing error handling and use sanitizeName
when populating parsedMap.
In `@server/imagepro-file-process/src/routes/ai/index.ts`:
- Around line 61-66: The catch block for the "AI rename-batch" route currently
returns error?.message to clients; change it to only log the full error
server-side (keep or enhance the existing console.error('[AI rename-batch]
Error:', error) call or use the app logger) and send a generic client response
via res.status(500).json({ error: 'AI rename failed', message: 'An internal
error occurred' }) or omit the detailed message entirely; remove any exposure of
error?.message from the response so internal/provider details are not leaked.
- Around line 31-44: The request validation currently checks settings.apiKey,
settings.modelProvider, and groups but not settings.model or
settings.systemPrompt, causing client errors to surface as adapter failures;
update the same validation block (where settings.apiKey and
settings.modelProvider are checked) to also verify settings.model is
present/non-empty and settings.systemPrompt is present/non-empty and return
res.status(400).json({ error: 'model is required' }) or res.status(400).json({
error: 'systemPrompt is required' }) accordingly so invalid client input is
rejected up front.
In `@server/imagepro-file-process/src/utils/promise-pool.ts`:
- Around line 18-35: The pool currently uses Promise.race(executing) which can
short-circuit when a single task rejects; fix by making the promises tracked in
executing non-rejecting "safe" wrappers and collect errors separately so the
loop waits for a freed slot without aborting in-flight work, then rethrow after
draining. Concretely: introduce an errors array, for each task create the real
promise p = task().then(result => results.push(result)).catch(err => {
errors.push(err); }).finally(() => executing.delete(safe)); then create safe =
p.catch(() => undefined) and add safe to executing (use executing.delete(safe)
in finally), use await Promise.race(executing) as before, await
Promise.all(executing) at the end, and if errors.length > 0 throw the first or
an AggregateError to rethrow failures after full drain (referencing variables
task, results, executing, concurrency, Promise.race, Promise.all).
---
Outside diff comments:
In `@plugin/src/hooks/useStorageManager.ts`:
- Around line 23-33: The RECEIVE_DATA flow fails because handleReceiveData
expects a {handle, data} payload but the emitter only sends {data}; update the
emitter so it sends the storage handle along with data (e.g., { handle:
'aiSettings' or 'historyStore', data }) so stores.find(s => s.storageKey ===
handle) matches; this will ensure handleReceiveData (ReceiveDataHandler) can
locate the entry in the stores array (storageKey 'aiSettings' / 'historyStore')
and call the appropriate storeApi.setState (useAIStore / useHistoryStore).
---
Nitpick comments:
In `@plan.md`:
- Around line 1-282: The markdown has several fenced code blocks missing
language specifiers (e.g., the UX layout block, the classifyNode() pseudocode,
groupNodesByParent(), promisePool(), and Default System Prompt blocks); update
each fenced block to include an appropriate language identifier such as "text",
"pseudocode", or "json" to satisfy linters and improve highlighting—specifically
edit the blocks around "UX Layout", "classifyNode()", "groupNodesByParent()",
"promisePool()", and the Default System Prompt so their opening fences include
the chosen specifier.
In `@plugin/src/core/ai/export-for-ai.ts`:
- Around line 24-40: The export logic currently retries at SCALE_FALLBACK if the
initial node.exportAsync (used with SCALE_NORMAL) exceeds MAX_BYTES_THRESHOLD
but doesn't check the size after the fallback; add a final size check after the
second exportAsync call and, if imageData.byteLength still >
MAX_BYTES_THRESHOLD, skip sending the image (e.g., return empty string) or emit
a warning via the existing logger, so oversized images are never passed through;
update the code around node.exportAsync, SCALE_NORMAL, SCALE_FALLBACK, and
MAX_BYTES_THRESHOLD to perform this final check and early-return/log as
appropriate.
In `@plugin/src/core/ai/svg-detector.ts`:
- Around line 9-17: The VECTOR_PRIMITIVE_TYPES set currently treats 'RECTANGLE'
as always a vector primitive which misclassifies rectangles with image fills;
update the classification logic instead of only the VECTOR_PRIMITIVE_TYPES
constant: keep 'RECTANGLE' removable from VECTOR_PRIMITIVE_TYPES (or leave but
treat specially) and modify the code paths that use VECTOR_PRIMITIVE_TYPES
(e.g., the function that checks node type for svg vs raster) to inspect a
Rectangle node's fills for image/fill.type === 'IMAGE' (or equivalent FILL_KIND)
and treat such rectangles as raster_leaf; reference VECTOR_PRIMITIVE_TYPES and
the Rectangle node handling code so the decision is based on content (fills) for
rectangles.
In `@plugin/src/pages/AIPage/_components/AINodeList.tsx`:
- Line 1: Remove the unused `h` import from the preact import in AINodeList.tsx:
locate the import statement that currently reads `import { h, Fragment } from
'preact';` and drop `h` so it becomes `import { Fragment } from 'preact';` (this
removes the unused symbol while keeping `Fragment` used by the AINodeList
component).
In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx`:
- Line 1: The import "h" from 'preact' in AISettingsPanel.tsx is unused; remove
the unused import line (the same change done in AINodeList.tsx) so the file no
longer imports h unnecessarily and linter/no-unused-vars errors are
resolved—edit the top of AISettingsPanel.tsx to delete the "import { h } from
'preact';" statement.
In `@plugin/src/pages/AIPage/index.tsx`:
- Line 63: The forEach callbacks on groups that call setRenameStatus are
implicitly returning its value which triggers lint warnings; update the
callbacks to use a block body (e.g., groups.forEach((g) => {
g.targetNodes.forEach((t) => { setRenameStatus(t.nodeId, 'pending'); }); });) so
the return is explicit void and repeat the same change for the similar
occurrences around the setRenameStatus calls at the other site (lines referenced
as 94-95) to eliminate the lint warnings.
In `@plugin/src/types/ai.ts`:
- Around line 34-43: The AIRenameGroup interface uses an inline object type for
targetNodes which is duplicated elsewhere; define and export a named interface
AIRenameTargetNode (with nodeId, currentName, nodeType, isSvg) and replace the
inline Array<{...}> in AIRenameGroup with Array<AIRenameTargetNode>; update any
other files or type usages that previously duplicated the shape to reference
AIRenameTargetNode to keep client/server contracts consistent.
In `@plugin/src/types/events.ts`:
- Around line 58-61: The payload currently duplicates readImage — update the
AIRenameRequestHandler type so its handler signature removes the top-level
readImage and uses only settings: AISettings (i.e., change handler: (payload: {
nodeIds: string[]; settings: AISettings }) => void), then search for all usages
of AIRenameRequestHandler/AIRename_REQUEST event handlers and callers to stop
passing payload.readImage (instead reference payload.settings.readImage) or
remove the extra arg; ensure any type annotations and tests are updated to the
single-source-of-truth in AISettings.
In `@server/imagepro-file-process/src/routes/ai/adapters/types.ts`:
- Around line 1-40: The types in this file (AIModelProvider, AIRenameTargetNode,
AIRenameGroup, AIRenameResult, AIRenameBatchRequest, and IModelAdapter)
duplicate similar definitions in plugin/src/types/ai.ts; to prevent drift,
extract and publish a single shared types module (e.g., a shared package or
workspace package) and replace the local declarations with imports from that
shared module, update all references (rename usages of AIRenameGroup,
AIRenameResult, AIRenameBatchRequest, and IModelAdapter to use the shared
exports) and remove the duplicate definitions here so both server and plugin
consume the same authoritative types.
In `@todo.md`:
- Around line 5-53: The TODO checklist in todo.md is out of sync; update the
file to reflect actual work by marking implemented items as completed (e.g.,
TabSwitch `ai` option, `AIPage` registration, created types like
`AISettings`/`AIRenameResult`, store `use-ai-store`, page components such as
`AIStatusBadge`/`AINodeList`/`AISettingsPanel`/`AIRunButton`, core/ai modules
like `svg-detector`/`section-grouper`/`export-for-ai`/`rename-handler`, server
adapters (e.g., `server/src/routes/ai/adapters/gemini.ts`) and routes, or remove
the file if it was only planning notes); ensure each changed checklist entry
references the matching implemented artifact names (AIPage, use-ai-store,
core/ai/*, server/src/routes/ai/*, etc.) and commit the updated todo.md so the
PR reflects current progress.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
plan.mdplugin/src/components/tab-switch/index.tsxplugin/src/components/ui/icon-button.tsxplugin/src/core/ai/export-for-ai.tsplugin/src/core/ai/rename-handler.tsplugin/src/core/ai/section-grouper.tsplugin/src/core/ai/svg-detector.tsplugin/src/hooks/useStorageManager.tsplugin/src/main.tsplugin/src/pages/AIPage/_components/AINodeList.tsxplugin/src/pages/AIPage/_components/AIRunButton.tsxplugin/src/pages/AIPage/_components/AISettingsPanel.tsxplugin/src/pages/AIPage/_components/AIStatusBadge.tsxplugin/src/pages/AIPage/index.tsxplugin/src/pages/HistoryPage/_components/ImageSelector/ImageGridListView/ListView.tsxplugin/src/pages/HistoryPage/_components/ImageSelector/index.tsxplugin/src/pages/index.tsxplugin/src/store/use-ai-store.tsplugin/src/styles/output.cssplugin/src/types/ai.tsplugin/src/types/events.tsserver/imagepro-file-process/package.jsonserver/imagepro-file-process/src/routes/ai/adapters/anthropic.tsserver/imagepro-file-process/src/routes/ai/adapters/gemini.tsserver/imagepro-file-process/src/routes/ai/adapters/index.tsserver/imagepro-file-process/src/routes/ai/adapters/openai.tsserver/imagepro-file-process/src/routes/ai/adapters/types.tsserver/imagepro-file-process/src/routes/ai/index.tsserver/imagepro-file-process/src/routes/index.tsserver/imagepro-file-process/src/utils/promise-pool.tstodo.md
Comment on lines
+94
to
+99
| if (nodeIds.length === 0) return; | ||
|
|
||
| // Step 1: Collect and classify | ||
| const targets = await collectTargetNodes(nodeIds); | ||
| if (targets.length === 0) return; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where running state is toggled.
rg -nP --type=ts --type=tsx -C3 '\bsetIsRunning\s*\('
# 2) Trace AI rename event lifecycle emit/listen sites.
rg -nP --type=ts --type=tsx -C3 '\bAI_RENAME_REQUEST\b|\bAI_BATCH_READY\b|\bAI_RENAME_COMPLETE\b|\bAI_RENAME_ERROR\b'
# 3) Locate all call sites of the orchestrator.
rg -nP --type=ts --type=tsx -C3 '\bhandleAIRenameRequest\s*\('Repository: jacksonkasi1/ImagePro-Export
Length of output: 99
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find where running state is toggled.
rg -nP --type=ts -C3 'setIsRunning\s*\('
# 2) Trace AI rename event lifecycle emit/listen sites.
rg -nP --type=ts -C3 'AI_RENAME_REQUEST|AI_BATCH_READY|AI_RENAME_COMPLETE|AI_RENAME_ERROR'
# 3) Locate all call sites of the orchestrator.
rg -nP --type=ts -C3 'handleAIRenameRequest\s*\('Repository: jacksonkasi1/ImagePro-Export
Length of output: 8665
🏁 Script executed:
#!/bin/bash
# Read the rename-handler.ts file to see lines around 94-99
cat -n plugin/src/core/ai/rename-handler.ts | sed -n '80,110p'Repository: jacksonkasi1/ImagePro-Export
Length of output: 1245
🏁 Script executed:
#!/bin/bash
# Check what happens in the AI_BATCH_READY listener in AIPage
cat -n plugin/src/pages/AIPage/index.tsx | sed -n '54,75p'Repository: jacksonkasi1/ImagePro-Export
Length of output: 1205
🏁 Script executed:
#!/bin/bash
# Check if there's any error handling or timeout at the main.ts level
cat -n plugin/src/main.ts | sed -n '113,130p'Repository: jacksonkasi1/ImagePro-Export
Length of output: 408
🏁 Script executed:
#!/bin/bash
# Check if there's error handling in the AI_BATCH_READY listener for network failures
cat -n plugin/src/pages/AIPage/index.tsx | sed -n '54,100p'Repository: jacksonkasi1/ImagePro-Export
Length of output: 2184
🏁 Script executed:
#!/bin/bash
# Check if there's any error handling wrapping the handleAIRenameRequest call in main.ts
cat -n plugin/src/main.ts | sed -n '113,125p'Repository: jacksonkasi1/ImagePro-Export
Length of output: 408
Early returns at lines 94–98 leave UI stuck without resetting running state.
Both nodeIds.length === 0 (line 94) and targets.length === 0 (line 98) return early without emitting AI_BATCH_READY, AI_RENAME_ERROR, or AI_RENAME_COMPLETE. Since setIsRunning(true) is set before this function is called, the UI remains in a running state indefinitely. Additionally, the event listener in main.ts (line 115) has no try-catch to handle unexpected errors, which would also bypass all event emissions.
Emit AI_RENAME_ERROR or a new error event for these cases, or add error handling at the call site to ensure setIsRunning(false) is always triggered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/core/ai/rename-handler.ts` around lines 94 - 99, The early returns
in renameHandler (when nodeIds.length === 0 and when collectTargetNodes(nodeIds)
returns no targets) leave the UI stuck because setIsRunning(true) is not
cleared; update the function to emit a failure/completion event instead of
simple returns—use the existing event constants AI_RENAME_ERROR (or emit
AI_RENAME_COMPLETE/AI_BATCH_READY as appropriate) and include an error payload
or reason so the caller can setIsRunning(false); additionally wrap the call site
listener in main.ts with a try/catch that catches unexpected errors from
renameHandler and emits AI_RENAME_ERROR (or triggers a guaranteed cleanup event)
to ensure the running state is always reset.
Comment on lines
+73
to
+76
| export interface AIRenameProgressHandler extends EventHandler { | ||
| name: 'AI_RENAME_PROGRESS'; | ||
| handler: (update: { nodeId: string; newName: string; status: 'done' | 'error' }) => void; | ||
| } |
There was a problem hiding this comment.
Fix inconsistent progress payload typing for error state.
Line 75 permits status: 'error' but still requires newName; that payload is not valid for rename failures.
🛠️ Suggested type fix (match current emitter behavior)
export interface AIRenameProgressHandler extends EventHandler {
name: 'AI_RENAME_PROGRESS';
- handler: (update: { nodeId: string; newName: string; status: 'done' | 'error' }) => void;
+ handler: (update: { nodeId: string; newName: string; status: 'done' }) => void;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/types/events.ts` around lines 73 - 76, The
AIRenameProgressHandler's handler currently requires newName even when status is
'error'; change the update payload to a discriminated union so newName is
required for { status: 'done' } but omitted/optional for { status: 'error' }
(e.g. update: { nodeId: string; status: 'done'; newName: string } | { nodeId:
string; status: 'error' }). Update the AIRenameProgressHandler.handler signature
accordingly.
- plugin/typings/types.d.ts: declare process.env ambient global so VITE_SERVER_URL env var compiles without @types/node - core/ai/rename-handler.ts: use index-based array assignment instead of push to preserve group order deterministically - core/ai/section-grouper.ts: remove unused AINodeContextChild import - main.ts: remove unused signalRenameComplete import - pages/AIPage/_components/AINodeList.tsx: replace useMemo blob URL creation with useThumbnailUrls hook that revokes URLs via useEffect cleanup (fixes memory leak) - pages/AIPage/index.tsx: move finalize before useEffect, add full deps array, use errBody.error field from server response, remove eslint-disable comment - types/events.ts: narrow AIRenameProgressHandler status to 'done' only (errors are handled by AIRenameErrorHandler) - server routes/ai/index.ts: add model + systemPrompt validation; remove raw error.message from 500 response body - server utils/promise-pool.ts: collect per-task errors so one failure doesn't abort the pool; drain all in-flight work before rethrowing - server routes/ai/adapters/gemini.ts: parseResponse builds fallback map so every target node always receives a result even on partial AI output
… storage Per CodeRabbit review: API key should not be co-mingled with non-sensitive persisted settings. - types/ai.ts: introduce AIPersistedSettings (modelProvider, model, systemPrompt, readImage, caseOption); AISettings extends it with apiKey - store/use-ai-store.ts: manage own persistence — non-sensitive settings saved under 'aiSettings' clientStorage key, apiKey saved separately under 'aiApiKey' key; listen on RECEIVE_DATA filtered by handle so each field hydrates correctly - main.ts: emit handle back in RECEIVE_DATA so listeners can filter by key (matches the existing ReceiveDataHandler type that already included handle) - hooks/useStorageManager.ts: remove aiSettings — AI store now self-manages persistence; avoids the broken handle-less setState pattern
…nstall - components/SearchBox.tsx: remove unsupported 'name' prop from SearchTextbox (not in SearchTextboxProps in updated @create-figma-plugin/ui) - components/ui/icon-button.tsx: extend JSX.ButtonHTMLAttributes instead of JSX.HTMLAttributes so 'disabled' prop is recognised - helpers/file-operation/array-buffer.ts: cast Uint8Array to Uint8Array<ArrayBuffer> for Blob constructor (stricter lib types) - pages/AIPage/_components/AINodeList.tsx: same Uint8Array cast for Blob - pages/HistoryPage/_components/ImageSelector/index.tsx: remove 'name' prop from SearchTextbox; 'disabled' on IconButton now resolved by icon-button fix above
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
plugin/src/pages/AIPage/index.tsx (2)
71-71:⚠️ Potential issue | 🟡 MinorUse block callbacks for
forEachto avoid implicit return lint errors.Line 71 and Line 102 currently use expression callbacks that implicitly return a value from
forEach, which Biome flags (lint/suspicious/useIterableCallbackReturn).🔧 Proposed fix
- groups.forEach((g) => g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'pending'))); + groups.forEach((g) => { + g.targetNodes.forEach((t) => { + setRenameStatus(t.nodeId, 'pending'); + }); + }); @@ - groups.forEach((g) => - g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'error')) - ); + groups.forEach((g) => { + g.targetNodes.forEach((t) => { + setRenameStatus(t.nodeId, 'error'); + }); + });Also applies to: 102-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/index.tsx` at line 71, Replace the expression-style forEach callbacks with block-style callbacks to avoid implicit returns: where you iterate over groups (the callback that calls setRenameStatus(t.nodeId, 'pending')) and the other groups.forEach usage around the rename status logic, change the arrow callbacks to use braces and an explicit statement body (e.g., groups.forEach(g => { g.targetNodes.forEach(t => { setRenameStatus(t.nodeId, 'pending'); }); });) so the callbacks are block callbacks rather than implicitly returning an expression; update both occurrences that reference setRenameStatus and any similar nested forEach usages.
31-34:⚠️ Potential issue | 🟠 MajorUse
import.meta.envforVITE_SERVER_URLin the UI bundle.Line 33 reads from
process.env, which often resolves toundefinedin Vite browser builds, so this can silently fall back to localhost.🔧 Proposed fix
-const SERVER_URL = - (typeof process !== 'undefined' && (process.env as Record<string, string>)['VITE_SERVER_URL']) || - 'http://localhost:3000'; +const SERVER_URL = import.meta.env.VITE_SERVER_URL ?? 'http://localhost:3000';#!/bin/bash set -euo pipefail # Verify env access patterns in UI code rg -n --type=ts --type=tsx 'VITE_SERVER_URL|import\.meta\.env|process\.env' # Verify Vite env typing setup exists rg -n --type=ts 'vite/client|interface\s+ImportMetaEnv|interface\s+ImportMeta'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/index.tsx` around lines 31 - 34, The SERVER_URL constant currently reads VITE_SERVER_URL from process.env which is undefined in browser Vite bundles; change the initialization of SERVER_URL (the SERVER_URL constant in AIPage/index.tsx) to read from import.meta.env.VITE_SERVER_URL with the same localhost:3000 fallback, e.g. use (import.meta.env.VITE_SERVER_URL || 'http://localhost:3000'), and ensure the project has Vite env typings (ImportMetaEnv/ImportMeta) so TypeScript recognizes import.meta.env access.plugin/src/core/ai/rename-handler.ts (1)
87-99:⚠️ Potential issue | 🔴 CriticalGuarantee a terminal event on every
handleAIRenameRequestexit path.Line 94 and Line 98 return silently, and any thrown error before Line 137 can also exit without notifying the UI. That can leave the run state hanging.
🔧 Proposed fix
export async function handleAIRenameRequest(payload: { nodeIds: string[]; readImage: boolean; settings: AISettings; }): Promise<void> { const { nodeIds, readImage } = payload; - if (nodeIds.length === 0) return; - - // Step 1: Collect and classify - const targets = await collectTargetNodes(nodeIds); - if (targets.length === 0) return; + if (nodeIds.length === 0) { + emit<AIRenameCompleteHandler>('AI_RENAME_COMPLETE'); + return; + } + + try { + // Step 1: Collect and classify + const targets = await collectTargetNodes(nodeIds); + if (targets.length === 0) { + emit<AIRenameCompleteHandler>('AI_RENAME_COMPLETE'); + return; + } @@ - await sequentialPool(exportTasks, 2); - - // Step 4: Emit to UI side - emit<AIBatchReadyHandler>('AI_BATCH_READY', groups); + await sequentialPool(exportTasks, 2); + + // Step 4: Emit to UI side + emit<AIBatchReadyHandler>('AI_BATCH_READY', groups); + } catch (error) { + emit<AIRenameCompleteHandler>('AI_RENAME_COMPLETE'); + emit<AIRenameErrorHandler>('AI_RENAME_ERROR', { + nodeId: nodeIds[0] ?? '', + error: `Failed to prepare AI rename batch: ${String(error)}`, + }); + } }Also applies to: 134-138
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/rename-handler.ts` around lines 87 - 99, The handleAIRenameRequest function can return early (e.g., when nodeIds.length === 0 or targets.length === 0) or throw before the run-end notification, leaving the UI hanging; wrap the main body of handleAIRenameRequest (starting after extracting nodeIds/readImage and including collectTargetNodes and subsequent logic) in a try/finally (or ensure a single exit path) so that the terminal run-state event is always emitted in the finally block; ensure every early return calls the same terminal notifier (the function that informs the UI of run completion) or remove early returns and short-circuit via the shared exit path so that UI is always notified regardless of errors or early exits.
🧹 Nitpick comments (9)
plugin/src/components/ui/icon-button.tsx (1)
53-53: Consider exposing loading state to assistive tech.Optional: add
aria-busy={isLoading}(and optionallyaria-liveon spinner container) so loading is announced semantically.Suggested small a11y tweak
<button {...props} // ** Spread standard button attributes disabled={isLoading || props.disabled} // ** Disable button when loading or explicitly disabled + aria-busy={isLoading || undefined} className={cn( 'flex items-center justify-center p-1 rounded cursor-pointer',Also applies to: 63-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/components/ui/icon-button.tsx` at line 53, The button's loading state (isLoading) isn't exposed to assistive tech; update the IconButton component so when isLoading is true it sets aria-busy="true" on the button (and preserve disabled behavior via disabled={isLoading || props.disabled}), and add an aria-live region on the spinner container (or role="status"/aria-live="polite") so screen readers announce the loading change; update references to isLoading, the button element in IconButton, and the spinner container rendering to include these attributes.plugin/src/helpers/file-operation/array-buffer.ts (1)
6-6: Remove the unnecessary double assertion in Blob construction.Line 6 uses
as unknown as Uint8Array<ArrayBuffer>which bypasses type safety without benefit. The parameter is already typed asUint8Array, and all other Blob constructions in the codebase pass the typed value directly without casts.♻️ Proposed fix
- const blob = new Blob([buffer as unknown as Uint8Array<ArrayBuffer>], { type: mimeType }); + const blob = new Blob([buffer], { type: mimeType });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/helpers/file-operation/array-buffer.ts` at line 6, The Blob construction uses an unnecessary double type assertion; in the expression creating `blob` replace the casted value `[buffer as unknown as Uint8Array<ArrayBuffer>]` with the already-typed `buffer` (or `[buffer]`) so you pass the Uint8Array directly into `new Blob([...], { type: mimeType })`; update the `blob` construction in `array-buffer.ts` (the `buffer` local and `blob` variable) to remove the `as unknown as Uint8Array<ArrayBuffer>` cast.server/imagepro-file-process/src/routes/ai/adapters/gemini.ts (1)
155-159: Unicode characters may be unintentionally stripped.The regex
/[^\w\s\-\.\/]/guses\wwhich only matches[A-Za-z0-9_]in JavaScript. This will strip valid Unicode characters that designers might use in node names (e.g., accented characters, CJK characters). Consider whether this is intentional for Figma compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/imagepro-file-process/src/routes/ai/adapters/gemini.ts` around lines 155 - 159, sanitizeName currently strips Unicode characters because the regex uses \w which is ASCII-only; update the sanitizer to allow Unicode letters/numbers by using Unicode property escapes (e.g., \p{L}, \p{N}) with the /u flag and include allowed characters like spaces, hyphens, dots, slashes and underscores if desired; keep the .trim() and .slice(0, 80) behavior and apply the new regex in the sanitizeName function to avoid removing accented/CJK characters while still stripping truly disallowed characters.plugin/src/core/ai/section-grouper.ts (1)
116-117: Parent may not be a SceneNode.The cast
siblings[0].parent as SceneNode | nullassumes the parent is aSceneNode, but it could be aPageNodeorDocumentNode(neither extendsSceneNode). IfserializeNodeContextis called with a non-SceneNode, it may fail or produce unexpected results.🛡️ Suggested defensive check
// Serialize context: use parent node if available const parentNode = siblings[0].parent as SceneNode | null; - const contextObj = parentNode ? serializeNodeContext(parentNode) : { nodes: targetNodes }; + const contextObj = parentNode && 'type' in parentNode && parentNode.type !== 'PAGE' && parentNode.type !== 'DOCUMENT' + ? serializeNodeContext(parentNode) + : { nodes: targetNodes };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/section-grouper.ts` around lines 116 - 117, The code assumes siblings[0].parent is a SceneNode and blindly casts it before calling serializeNodeContext, which can break when parent is a PageNode/DocumentNode; update the logic to first obtain parent = siblings[0].parent, then check its runtime/type (e.g., use an existing isSceneNode guard or check parent.type === 'CANVAS'/'FRAME'/'GROUP' etc.) and only call serializeNodeContext(parent) when that check confirms a SceneNode, otherwise set contextObj to the fallback { nodes: targetNodes }; update references around parentNode, serializeNodeContext, and contextObj accordingly so non-SceneNode parents never get passed into serializeNodeContext.plugin/src/store/use-ai-store.ts (1)
59-62: Event subscription never cleaned up.The
on<ReceiveDataHandler>subscription is created during store initialization but never unsubscribed. While this is typically fine for a singleton store in a Figma plugin (the store lives for the plugin's lifetime), it's worth noting for future refactoring if the store lifecycle changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/store/use-ai-store.ts` around lines 59 - 62, The subscription created by on<ReceiveDataHandler>('RECEIVE_DATA', handleReceiveData) is never cleaned up; capture the unsubscribe (or handler token) returned by on when initializing the store and expose a cleanup/destroy method on the store that calls that unsubscribe (e.g., unsubscribeReceiveData()) so the handler is removed when the store is torn down or refactored; update any initialization code that creates the store to call this new cleanup method during plugin shutdown or when replacing the store.server/imagepro-file-process/src/utils/promise-pool.ts (1)
43-45: Consider exposing all errors via AggregateError.Currently only the first error is thrown, silently discarding subsequent failures. If multiple tasks fail, this could mask useful debugging information.
♻️ Optional: Use AggregateError for multiple failures
if (errors.length > 0) { - throw errors[0]; + throw errors.length === 1 + ? errors[0] + : new AggregateError(errors, `${errors.length} tasks failed`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/imagepro-file-process/src/utils/promise-pool.ts` around lines 43 - 45, The current code in promise-pool.ts only throws the first error (throw errors[0]) which hides other failures; replace that throw with throwing an AggregateError constructed from the collected errors (e.g., throw new AggregateError(errors, 'Multiple tasks failed')) so all errors are exposed to callers; update any type signatures or catch-site handling if necessary to accept AggregateError, and keep the existing errors array and its collection logic intact.plugin/package.json (1)
35-38: Inconsistent package manager usage between scripts.The
devscript usesbun runwhilebuildstill usespnpm run. This inconsistency could cause confusion or issues if dependencies are installed with one package manager but scripts are executed with another (different lockfile resolution, caching behavior, etc.).Consider aligning both scripts to use the same package manager, or document the intent if this is deliberate.
♻️ Suggested fix to align on bun
"scripts": { - "build": "pnpm run build:css && pnpm run build:js", + "build": "bun run build:css && bun run build:js", "build:css": "tailwindcss --input ./src/styles/input.css --output ./src/styles/output.css", "build:js": "build-figma-plugin --typecheck --minify", "dev": "bun run build:css && concurrently \"bun run watch:css\" \"bun run watch:js\"",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/package.json` around lines 35 - 38, The package.json scripts are using mixed package managers: "build" and "build:css"/"build:js" call "pnpm run" while "dev" calls "bun run"; pick one and make scripts consistent. Update the "build", "build:css", and "build:js" script commands to use the same package manager invocation as "dev" (or alternatively change "dev" to use pnpm) so all scripts ("build", "build:css", "build:js", "dev") consistently call either "bun run" or "pnpm run"; ensure you update the exact script names shown in package.json accordingly.plugin/src/pages/AIPage/index.tsx (1)
161-165: Minimize event payload: avoid emitting fullsettings(includesapiKey) to main thread.Line 164 forwards sensitive settings through
AI_RENAME_REQUEST, but this page already uses settings directly for the server call. Narrowing this payload reduces secret propagation surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/index.tsx` around lines 161 - 165, The event payload currently sends the entire settings object (including apiKey) via emit<AIRenameRequestHandler>('AI_RENAME_REQUEST', {...}), which propagates secrets to the main thread; change the emit call to only include the minimal fields needed (e.g., nodeIds: selectedNodeIds and readImage: settings.readImage or an explicit sanitizedSettings object) instead of passing settings wholesale, and keep using the existing local settings for server calls in this component (do not include apiKey or other secrets in the emitted payload).plugin/src/core/ai/rename-handler.ts (1)
10-11: Drop unusedsettingsfrom this handler contract.Line 90 adds
settings(andAISettingsimport), but Line 92 only usesnodeIdsandreadImage. Removing it here improves data minimization and keeps API key out of main-thread flow where not needed.Also applies to: 87-92
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/rename-handler.ts` around lines 10 - 11, Remove the unused settings parameter and its type import from this handler: drop the settings parameter from the handler signature that currently references AIRenameGroup and AISettings, remove the AISettings import, and update any local references so the handler only accepts/uses nodeIds and readImage; ensure AIRenameGroup usage remains correct and update any upstream call sites in this file that pass settings so they stop doing so.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/src/pages/AIPage/_components/AINodeList.tsx`:
- Line 30: The current cast "node.imageData as unknown as
Uint8Array<ArrayBuffer>" is invalid because Uint8Array is not generic; update
the conversion in AINodeList (where `url` is created) to cast `node.imageData`
to a plain Uint8Array (e.g., `as Uint8Array`) or first ensure you convert an
ArrayBuffer to a Uint8Array (e.g., `new Uint8Array(node.imageData)`), and then
pass that into Blob creation so `URL.createObjectURL` receives a proper
Uint8Array/ArrayBuffer.
In `@plugin/src/pages/AIPage/index.tsx`:
- Around line 54-57: The finalize callback currently always calls notify.success
and sets isRunning false, which wrongly reports success when node-level errors
occur; change the finalize definition (the useCallback named finalize) to accept
a boolean parameter like finalize(success: boolean, message?: string) and only
call notify.success when success is true, otherwise call notify.error (or
notify.warn) with the provided message; update the useCallback dependency list
accordingly and replace all callers that unconditionally call finalize()
(including the error handlers that currently invoke finalize on node errors) to
call finalize(false, <error message>) while successful completion sites call
finalize(true).
- Around line 74-78: The fetch to `${SERVER_URL}/api/ai/rename-batch` must be
cancellable to avoid leaving isRunning true; wrap the request with an
AbortController (create controller before calling fetch, pass controller.signal
to fetch), set a timeout (e.g., setTimeout) that calls controller.abort() after
a chosen millis, and clear that timeout when fetch resolves; in the catch block
detect abort errors and ensure you update isRunning state (and any cleanup)
accordingly; update the component logic around the existing fetch call to use
the new AbortController/timeout and to avoid leaking controllers across calls.
In `@plugin/src/styles/output.css`:
- Around line 975-1008: The generated Tailwind CSS output is tripping
stylelint's declaration-empty-line-before rule for many utility blocks (e.g.,
.border-gray-300, .border-transparent, .border-b-f-border, .bg-active-bg,
.bg-brand-bg, .bg-btn-bg-dark, .bg-gray-100, .bg-gray-200); instead of editing
the generated file, update the lint/source config: either exclude the generated
output.css from stylelint (add output.css to .stylelintignore or use stylelint
ignoreFiles), or scope-disable the declaration-empty-line-before rule for
generated files (via overrides in stylelint config matching
plugin/src/styles/output.css) or modify the generator/formatter to stop
inserting the extra empty lines so future builds do not reproduce the
violations.
---
Duplicate comments:
In `@plugin/src/core/ai/rename-handler.ts`:
- Around line 87-99: The handleAIRenameRequest function can return early (e.g.,
when nodeIds.length === 0 or targets.length === 0) or throw before the run-end
notification, leaving the UI hanging; wrap the main body of
handleAIRenameRequest (starting after extracting nodeIds/readImage and including
collectTargetNodes and subsequent logic) in a try/finally (or ensure a single
exit path) so that the terminal run-state event is always emitted in the finally
block; ensure every early return calls the same terminal notifier (the function
that informs the UI of run completion) or remove early returns and short-circuit
via the shared exit path so that UI is always notified regardless of errors or
early exits.
In `@plugin/src/pages/AIPage/index.tsx`:
- Line 71: Replace the expression-style forEach callbacks with block-style
callbacks to avoid implicit returns: where you iterate over groups (the callback
that calls setRenameStatus(t.nodeId, 'pending')) and the other groups.forEach
usage around the rename status logic, change the arrow callbacks to use braces
and an explicit statement body (e.g., groups.forEach(g => {
g.targetNodes.forEach(t => { setRenameStatus(t.nodeId, 'pending'); }); });) so
the callbacks are block callbacks rather than implicitly returning an
expression; update both occurrences that reference setRenameStatus and any
similar nested forEach usages.
- Around line 31-34: The SERVER_URL constant currently reads VITE_SERVER_URL
from process.env which is undefined in browser Vite bundles; change the
initialization of SERVER_URL (the SERVER_URL constant in AIPage/index.tsx) to
read from import.meta.env.VITE_SERVER_URL with the same localhost:3000 fallback,
e.g. use (import.meta.env.VITE_SERVER_URL || 'http://localhost:3000'), and
ensure the project has Vite env typings (ImportMetaEnv/ImportMeta) so TypeScript
recognizes import.meta.env access.
---
Nitpick comments:
In `@plugin/package.json`:
- Around line 35-38: The package.json scripts are using mixed package managers:
"build" and "build:css"/"build:js" call "pnpm run" while "dev" calls "bun run";
pick one and make scripts consistent. Update the "build", "build:css", and
"build:js" script commands to use the same package manager invocation as "dev"
(or alternatively change "dev" to use pnpm) so all scripts ("build",
"build:css", "build:js", "dev") consistently call either "bun run" or "pnpm
run"; ensure you update the exact script names shown in package.json
accordingly.
In `@plugin/src/components/ui/icon-button.tsx`:
- Line 53: The button's loading state (isLoading) isn't exposed to assistive
tech; update the IconButton component so when isLoading is true it sets
aria-busy="true" on the button (and preserve disabled behavior via
disabled={isLoading || props.disabled}), and add an aria-live region on the
spinner container (or role="status"/aria-live="polite") so screen readers
announce the loading change; update references to isLoading, the button element
in IconButton, and the spinner container rendering to include these attributes.
In `@plugin/src/core/ai/rename-handler.ts`:
- Around line 10-11: Remove the unused settings parameter and its type import
from this handler: drop the settings parameter from the handler signature that
currently references AIRenameGroup and AISettings, remove the AISettings import,
and update any local references so the handler only accepts/uses nodeIds and
readImage; ensure AIRenameGroup usage remains correct and update any upstream
call sites in this file that pass settings so they stop doing so.
In `@plugin/src/core/ai/section-grouper.ts`:
- Around line 116-117: The code assumes siblings[0].parent is a SceneNode and
blindly casts it before calling serializeNodeContext, which can break when
parent is a PageNode/DocumentNode; update the logic to first obtain parent =
siblings[0].parent, then check its runtime/type (e.g., use an existing
isSceneNode guard or check parent.type === 'CANVAS'/'FRAME'/'GROUP' etc.) and
only call serializeNodeContext(parent) when that check confirms a SceneNode,
otherwise set contextObj to the fallback { nodes: targetNodes }; update
references around parentNode, serializeNodeContext, and contextObj accordingly
so non-SceneNode parents never get passed into serializeNodeContext.
In `@plugin/src/helpers/file-operation/array-buffer.ts`:
- Line 6: The Blob construction uses an unnecessary double type assertion; in
the expression creating `blob` replace the casted value `[buffer as unknown as
Uint8Array<ArrayBuffer>]` with the already-typed `buffer` (or `[buffer]`) so you
pass the Uint8Array directly into `new Blob([...], { type: mimeType })`; update
the `blob` construction in `array-buffer.ts` (the `buffer` local and `blob`
variable) to remove the `as unknown as Uint8Array<ArrayBuffer>` cast.
In `@plugin/src/pages/AIPage/index.tsx`:
- Around line 161-165: The event payload currently sends the entire settings
object (including apiKey) via emit<AIRenameRequestHandler>('AI_RENAME_REQUEST',
{...}), which propagates secrets to the main thread; change the emit call to
only include the minimal fields needed (e.g., nodeIds: selectedNodeIds and
readImage: settings.readImage or an explicit sanitizedSettings object) instead
of passing settings wholesale, and keep using the existing local settings for
server calls in this component (do not include apiKey or other secrets in the
emitted payload).
In `@plugin/src/store/use-ai-store.ts`:
- Around line 59-62: The subscription created by
on<ReceiveDataHandler>('RECEIVE_DATA', handleReceiveData) is never cleaned up;
capture the unsubscribe (or handler token) returned by on when initializing the
store and expose a cleanup/destroy method on the store that calls that
unsubscribe (e.g., unsubscribeReceiveData()) so the handler is removed when the
store is torn down or refactored; update any initialization code that creates
the store to call this new cleanup method during plugin shutdown or when
replacing the store.
In `@server/imagepro-file-process/src/routes/ai/adapters/gemini.ts`:
- Around line 155-159: sanitizeName currently strips Unicode characters because
the regex uses \w which is ASCII-only; update the sanitizer to allow Unicode
letters/numbers by using Unicode property escapes (e.g., \p{L}, \p{N}) with the
/u flag and include allowed characters like spaces, hyphens, dots, slashes and
underscores if desired; keep the .trim() and .slice(0, 80) behavior and apply
the new regex in the sanitizeName function to avoid removing accented/CJK
characters while still stripping truly disallowed characters.
In `@server/imagepro-file-process/src/utils/promise-pool.ts`:
- Around line 43-45: The current code in promise-pool.ts only throws the first
error (throw errors[0]) which hides other failures; replace that throw with
throwing an AggregateError constructed from the collected errors (e.g., throw
new AggregateError(errors, 'Multiple tasks failed')) so all errors are exposed
to callers; update any type signatures or catch-site handling if necessary to
accept AggregateError, and keep the existing errors array and its collection
logic intact.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.DS_Storeis excluded by!**/.DS_Storeplugin/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
plugin/package.jsonplugin/src/components/SearchBox.tsxplugin/src/components/ui/icon-button.tsxplugin/src/core/ai/rename-handler.tsplugin/src/core/ai/section-grouper.tsplugin/src/helpers/file-operation/array-buffer.tsplugin/src/hooks/useStorageManager.tsplugin/src/main.tsplugin/src/pages/AIPage/_components/AINodeList.tsxplugin/src/pages/AIPage/index.tsxplugin/src/pages/HistoryPage/_components/ImageSelector/index.tsxplugin/src/store/use-ai-store.tsplugin/src/styles/output.cssplugin/src/types/ai.tsplugin/src/types/events.tsplugin/typings/types.d.tsserver/imagepro-file-process/src/routes/ai/adapters/gemini.tsserver/imagepro-file-process/src/routes/ai/index.tsserver/imagepro-file-process/src/utils/promise-pool.ts
💤 Files with no reviewable changes (2)
- plugin/src/components/SearchBox.tsx
- plugin/src/hooks/useStorageManager.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/src/types/events.ts
- plugin/src/main.ts
- server/imagepro-file-process/src/routes/ai/index.ts
Comment on lines
+54
to
+57
| const finalize = useCallback(() => { | ||
| setIsRunning(false); | ||
| notify.success('AI rename complete'); | ||
| }, [setIsRunning]); |
There was a problem hiding this comment.
finalize reports success even when node-level errors occurred.
Line 56 always shows success, but Line 128–131 reaches finalize() from error events. This gives misleading completion feedback.
🔧 Proposed fix
+ const hadErrorsRef = useRef(false);
+
const finalize = useCallback(() => {
setIsRunning(false);
- notify.success('AI rename complete');
+ if (hadErrorsRef.current) {
+ notify.error('AI rename completed with some errors');
+ } else {
+ notify.success('AI rename complete');
+ }
+ hadErrorsRef.current = false;
}, [setIsRunning]);
@@
const unsubError = on<AIRenameErrorHandler>('AI_RENAME_ERROR', ({ nodeId }) => {
+ hadErrorsRef.current = true;
setRenameStatus(nodeId, 'error');
pendingRef.current -= 1;
if (pendingRef.current <= 0) finalize();
});Also applies to: 128-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/pages/AIPage/index.tsx` around lines 54 - 57, The finalize
callback currently always calls notify.success and sets isRunning false, which
wrongly reports success when node-level errors occur; change the finalize
definition (the useCallback named finalize) to accept a boolean parameter like
finalize(success: boolean, message?: string) and only call notify.success when
success is true, otherwise call notify.error (or notify.warn) with the provided
message; update the useCallback dependency list accordingly and replace all
callers that unconditionally call finalize() (including the error handlers that
currently invoke finalize on node errors) to call finalize(false, <error
message>) while successful completion sites call finalize(true).
plugin/src/pages/AIPage/index.tsx
Outdated
Comment on lines
+74
to
+78
| const response = await fetch(`${SERVER_URL}/api/ai/rename-batch`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ groups, settings }), | ||
| }); |
There was a problem hiding this comment.
Add a timeout/abort path for the batch request.
Line 74 awaits fetch without timeout/cancellation. A stalled backend can keep isRunning stuck for the session.
🔧 Proposed fix
- const response = await fetch(`${SERVER_URL}/api/ai/rename-batch`, {
- method: 'POST',
- headers: { 'Content-Type': 'application/json' },
- body: JSON.stringify({ groups, settings }),
- });
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 15000);
+ let response: Response;
+ try {
+ response = await fetch(`${SERVER_URL}/api/ai/rename-batch`, {
+ method: 'POST',
+ headers: { 'Content-Type': 'application/json' },
+ body: JSON.stringify({ groups, settings }),
+ signal: controller.signal,
+ });
+ } finally {
+ clearTimeout(timeoutId);
+ }📝 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
| const response = await fetch(`${SERVER_URL}/api/ai/rename-batch`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ groups, settings }), | |
| }); | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), 15000); | |
| let response: Response; | |
| try { | |
| response = await fetch(`${SERVER_URL}/api/ai/rename-batch`, { | |
| method: 'POST', | |
| headers: { 'Content-Type': 'application/json' }, | |
| body: JSON.stringify({ groups, settings }), | |
| signal: controller.signal, | |
| }); | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/pages/AIPage/index.tsx` around lines 74 - 78, The fetch to
`${SERVER_URL}/api/ai/rename-batch` must be cancellable to avoid leaving
isRunning true; wrap the request with an AbortController (create controller
before calling fetch, pass controller.signal to fetch), set a timeout (e.g.,
setTimeout) that calls controller.abort() after a chosen millis, and clear that
timeout when fetch resolves; in the catch block detect abort errors and ensure
you update isRunning state (and any cleanup) accordingly; update the component
logic around the existing fetch call to use the new AbortController/timeout and
to avoid leaking controllers across calls.
Comment on lines
975
to
+1008
| .border-gray-300 { | ||
| --tw-border-opacity: 1; | ||
| border-color: rgb(209 213 219 / var(--tw-border-opacity)); | ||
| } | ||
|
|
||
| .border-gray-400 { | ||
| --tw-border-opacity: 1; | ||
| border-color: rgb(156 163 175 / var(--tw-border-opacity)); | ||
| } | ||
|
|
||
| .border-gray-900 { | ||
| --tw-border-opacity: 1; | ||
| border-color: rgb(17 24 39 / var(--tw-border-opacity)); | ||
| } | ||
|
|
||
| .border-green-800 { | ||
| --tw-border-opacity: 1; | ||
| border-color: rgb(22 101 52 / var(--tw-border-opacity)); | ||
| } | ||
|
|
||
| .border-primary-bg { | ||
| border-color: var(--figma-color-bg); | ||
| border-color: rgb(209 213 219 / var(--tw-border-opacity, 1)); | ||
| } | ||
|
|
||
| .border-transparent { | ||
| border-color: transparent; | ||
| } | ||
|
|
||
| .border-y-transparent { | ||
| border-top-color: transparent; | ||
| border-bottom-color: transparent; | ||
| } | ||
|
|
||
| .border-b-f-border { | ||
| border-bottom-color: var(--figma-color-border); | ||
| } | ||
|
|
||
| .border-t-f-border { | ||
| border-top-color: var(--figma-color-border); | ||
| } | ||
|
|
||
| .\!border-opacity-0 { | ||
| --tw-border-opacity: 0 !important; | ||
| } | ||
|
|
||
| .border-opacity-0 { | ||
| --tw-border-opacity: 0; | ||
| } | ||
|
|
||
| .border-opacity-100 { | ||
| --tw-border-opacity: 1; | ||
| } | ||
|
|
||
| .\!bg-primary-bg { | ||
| background-color: var(--figma-color-bg) !important; | ||
| } | ||
|
|
||
| .bg-active-bg { | ||
| background-color: var(--figma-color-bg-tertiary); | ||
| } | ||
|
|
||
| .bg-black { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(0 0 0 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-blue-200 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(191 219 254 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-blue-300 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(147 197 253 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-blue-400 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(96 165 250 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-blue-600 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(37 99 235 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-brand-bg { | ||
| background-color: var(--figma-color-bg-brand); | ||
| } | ||
|
|
||
| .bg-btn-bg-dark { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(80 80 80 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-danger { | ||
| background-color: var(--figma-color-bg-danger); | ||
| } | ||
|
|
||
| .bg-danger-hover { | ||
| background-color: var(--figma-color-bg-danger-hover); | ||
| } | ||
|
|
||
| .bg-emerald-400 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(52 211 153 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-gray-100 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(243 244 246 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-gray-200 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(229 231 235 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-gray-50 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(249 250 251 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-green-200 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(187 247 208 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-green-300 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(134 239 172 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-green-400 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(74 222 128 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-green-500 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(34 197 94 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-primary-bg { | ||
| background-color: var(--figma-color-bg); | ||
| } | ||
|
|
||
| .bg-red-100 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(254 226 226 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-red-200 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(254 202 202 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-red-300 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(252 165 165 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-red-400 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(248 113 113 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-red-500 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(239 68 68 / var(--tw-bg-opacity)); | ||
| } | ||
|
|
||
| .bg-scrollbar-thumb-hover { | ||
| background-color: var(--figma-color-bg-secondary); | ||
| } | ||
|
|
||
| .bg-secondary-bg { | ||
| background-color: var(--figma-color-bg-secondary); | ||
| } | ||
|
|
||
| .bg-selected-bg { | ||
| background-color: var(--figma-color-bg-selected); | ||
| background-color: rgb(80 80 80 / var(--tw-bg-opacity, 1)); | ||
| } | ||
|
|
||
| .bg-slate-100 { | ||
| .bg-gray-100 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(241 245 249 / var(--tw-bg-opacity)); | ||
| background-color: rgb(243 244 246 / var(--tw-bg-opacity, 1)); | ||
| } | ||
|
|
||
| .bg-transparent { | ||
| background-color: transparent; | ||
| .bg-gray-200 { | ||
| --tw-bg-opacity: 1; | ||
| background-color: rgb(229 231 235 / var(--tw-bg-opacity, 1)); |
There was a problem hiding this comment.
Stylelint is currently failing on declaration spacing in these utility blocks.
declaration-empty-line-before is reported at Line 977, Line 998, Line 1003, Line 1008, Line 1131, Line 1140, Line 1149, Line 1306, and Line 1346. If lint is required in CI, this blocks merge.
Given this is generated Tailwind output, prefer fixing at source (generator/lint scope) instead of hand-editing generated CSS every build.
Also applies to: 1129-1149, 1304-1307, 1344-1347
🧰 Tools
🪛 Stylelint (17.3.0)
[error] 977-977: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 998-998: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 1003-1003: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 1008-1008: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/styles/output.css` around lines 975 - 1008, The generated Tailwind
CSS output is tripping stylelint's declaration-empty-line-before rule for many
utility blocks (e.g., .border-gray-300, .border-transparent, .border-b-f-border,
.bg-active-bg, .bg-brand-bg, .bg-btn-bg-dark, .bg-gray-100, .bg-gray-200);
instead of editing the generated file, update the lint/source config: either
exclude the generated output.css from stylelint (add output.css to
.stylelintignore or use stylelint ignoreFiles), or scope-disable the
declaration-empty-line-before rule for generated files (via overrides in
stylelint config matching plugin/src/styles/output.css) or modify the
generator/formatter to stop inserting the extra empty lines so future builds do
not reproduce the violations.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugin/src/pages/AIPage/_components/AISettingsPanel.tsx (1)
103-169: Associate labels with controls to improve accessibility.Provider/Model/API Key/Naming Convention/System Prompt labels are not explicitly linked to their inputs. Add
id+htmlFor(and a real<label>for the checkbox text) for better keyboard and screen-reader behavior.♿ Suggested markup update
- <label class={labelClass}>Provider</label> + <label class={labelClass} htmlFor="ai-provider">Provider</label> <select + id="ai-provider" class={selectClass} value={settings.modelProvider} onChange={handleProviderChange} > @@ - <label class={labelClass}>Model</label> - <select class={selectClass} value={settings.model} onChange={handleModelChange}> + <label class={labelClass} htmlFor="ai-model">Model</label> + <select id="ai-model" class={selectClass} value={settings.model} onChange={handleModelChange}> @@ - <label class={labelClass}>API Key</label> + <label class={labelClass} htmlFor="ai-api-key">API Key</label> <input + id="ai-api-key" @@ - <label class={labelClass}>Naming Convention</label> - <select class={selectClass} value={settings.caseOption} onChange={handleCaseChange}> + <label class={labelClass} htmlFor="ai-case-option">Naming Convention</label> + <select id="ai-case-option" class={selectClass} value={settings.caseOption} onChange={handleCaseChange}> @@ - <div class="flex items-center gap-2"> + <div class="flex items-center gap-2"> <Checkbox value={settings.readImage} onValueChange={handleReadImageChange} /> - <span class="text-xs text-primary-text">Send screenshot to AI (slower, more accurate)</span> + <label class="text-xs text-primary-text">Send screenshot to AI (slower, more accurate)</label> </div> @@ - <label class={labelClass}>System Prompt</label> + <label class={labelClass} htmlFor="ai-system-prompt">System Prompt</label> <textarea + id="ai-system-prompt" class={cn(inputClass, 'h-24 resize-none leading-relaxed')} value={settings.systemPrompt} onInput={handleSystemPromptChange} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx` around lines 103 - 169, The labels in AISettingsPanel.tsx are not associated with their controls—update each label element for Provider, Model, API Key, Naming Convention and System Prompt to include htmlFor and add matching id attributes on the corresponding select/input/textarea elements (e.g., the Provider select used with value={settings.modelProvider} and onChange={handleProviderChange}, Model select used with settings.model and handleModelChange, API Key input used with handleApiKeyChange, Naming Convention select with handleCaseChange, and System Prompt textarea with handleSystemPromptChange). For the Read Image toggle replace the current plain span with a real <label> element tied to the Checkbox (give the Checkbox an id and use htmlFor on the label) so the checkbox text is keyboard- and screen-reader-accessible; keep existing className variables (labelClass, selectClass, inputClass) and handlers intact. Ensure ids are unique and stable (e.g., provider-select, model-select, api-key-input, case-select, system-prompt-textarea, read-image-checkbox).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx`:
- Around line 54-58: Guard accesses to MODEL_OPTIONS when reading by provider in
handleProviderChange and wherever persisted settings are loaded (e.g., code that
sets modelProvider/model from saved settings). Check that
MODEL_OPTIONS[provider] exists (and has a models array with at least one
element) before using it; if not, fall back to a safe default provider and model
(e.g., a constant DEFAULT_PROVIDER and its first model) and call setSettings
with those values. Use the same defensive check at the other site that indexed
MODEL_OPTIONS (the persisted-settings restore code) to avoid crashes on
stale/corrupted values.
- Around line 70-73: The API key is being persisted in plaintext via setSettings
(triggered in handleApiKeyChange) which calls the store's persistSettings;
change this so the raw API key is not written to clientStorage: either keep the
key in component/local session state only (store in a React useState or a
transient field and remove the apiKey property from the object passed to
setSettings/persistSettings), or if you must persist, encrypt the key before
calling persistSettings and update the store (persistSettings) to accept and
store only the encrypted value (and ensure main.ts decrypts on retrieval using a
secure key source). Update handleApiKeyChange and any callers of
setSettings/persistSettings to omit plaintext apiKey, and reference the store
functions in use-ai-store.ts and storage handling in main.ts when adding
encryption logic.
---
Nitpick comments:
In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx`:
- Around line 103-169: The labels in AISettingsPanel.tsx are not associated with
their controls—update each label element for Provider, Model, API Key, Naming
Convention and System Prompt to include htmlFor and add matching id attributes
on the corresponding select/input/textarea elements (e.g., the Provider select
used with value={settings.modelProvider} and onChange={handleProviderChange},
Model select used with settings.model and handleModelChange, API Key input used
with handleApiKeyChange, Naming Convention select with handleCaseChange, and
System Prompt textarea with handleSystemPromptChange). For the Read Image toggle
replace the current plain span with a real <label> element tied to the Checkbox
(give the Checkbox an id and use htmlFor on the label) so the checkbox text is
keyboard- and screen-reader-accessible; keep existing className variables
(labelClass, selectClass, inputClass) and handlers intact. Ensure ids are unique
and stable (e.g., provider-select, model-select, api-key-input, case-select,
system-prompt-textarea, read-image-checkbox).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugin/src/pages/AIPage/_components/AISettingsPanel.tsxplugin/src/store/use-ai-store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/src/store/use-ai-store.ts
Comment on lines
+54
to
+58
| const handleProviderChange = useCallback( | ||
| (e: Event) => { | ||
| const provider = (e.target as HTMLSelectElement).value as AIModelProvider; | ||
| const defaultModel = MODEL_OPTIONS[provider].models[0]; | ||
| setSettings({ modelProvider: provider, model: defaultModel }); |
There was a problem hiding this comment.
Guard MODEL_OPTIONS access against invalid provider values.
Line 57 and Line 122 index MODEL_OPTIONS directly. If persisted settings are stale/corrupted, this can throw and crash the panel.
🛡️ Proposed defensive fallback
const AISettingsPanel = () => {
const { settings, setSettings } = useAIStore();
+ const providerConfig = MODEL_OPTIONS[settings.modelProvider] ?? MODEL_OPTIONS.gemini;
const handleProviderChange = useCallback(
(e: Event) => {
const provider = (e.target as HTMLSelectElement).value as AIModelProvider;
- const defaultModel = MODEL_OPTIONS[provider].models[0];
+ const defaultModel = (MODEL_OPTIONS[provider] ?? MODEL_OPTIONS.gemini).models[0];
setSettings({ modelProvider: provider, model: defaultModel });
},
[setSettings]
);
@@
- <select class={selectClass} value={settings.model} onChange={handleModelChange}>
- {MODEL_OPTIONS[settings.modelProvider].models.map((m) => (
+ <select class={selectClass} value={settings.model} onChange={handleModelChange}>
+ {providerConfig.models.map((m) => (
<option key={m} value={m}>
{m}
</option>
))}
</select>Also applies to: 121-123
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[warning] 54-54: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L54
Unsafe assignment of an error typed value.
[warning] 54-54: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L54
Unsafe call of an error type typed value.
[warning] 56-56: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L56
Unsafe assignment of an error typed value.
[warning] 57-57: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L57
Computed name [provider] resolves to an error typed value.
[warning] 58-58: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L58
Unsafe assignment of an error typed value.
[warning] 58-58: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L58
Unsafe call of an error type typed value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx` around lines 54 -
58, Guard accesses to MODEL_OPTIONS when reading by provider in
handleProviderChange and wherever persisted settings are loaded (e.g., code that
sets modelProvider/model from saved settings). Check that
MODEL_OPTIONS[provider] exists (and has a models array with at least one
element) before using it; if not, fall back to a safe default provider and model
(e.g., a constant DEFAULT_PROVIDER and its first model) and call setSettings
with those values. Use the same defensive check at the other site that indexed
MODEL_OPTIONS (the persisted-settings restore code) to avoid crashes on
stale/corrupted values.
Comment on lines
+70
to
+73
| const handleApiKeyChange = useCallback( | ||
| (e: Event) => { | ||
| setSettings({ apiKey: (e.target as HTMLInputElement).value }); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the use-ai-store.ts file
echo "=== Searching for use-ai-store.ts ==="
fd 'use-ai-store' --type f
# If found, inspect it
echo ""
echo "=== Content of use-ai-store.ts ==="
fd 'use-ai-store.ts' -x cat -n
# Search for persistence patterns
echo ""
echo "=== Searching for persist/storage patterns ==="
rg -n 'persist|partialize|storage' --type=ts --type=tsx -B 2 -A 2
# Search specifically for apiKey handling
echo ""
echo "=== Searching for apiKey references ==="
rg -n 'apiKey' --type=ts --type=tsx -B 1 -A 1Repository: jacksonkasi1/ImagePro-Export
Length of output: 4778
🏁 Script executed:
#!/bin/bash
# Search for clientStorage or storage implementation
echo "=== Searching for storage/SET_DATA implementation ==="
rg -n 'SET_DATA|clientStorage|emit.*SET_DATA' -B 2 -A 2 --type=ts
# Check the types to understand AIPersistedSettings
echo ""
echo "=== Searching for type definitions ==="
fd 'ai.ts' --type f | head -5
# Look for encryption or security measures
echo ""
echo "=== Searching for encryption/security patterns ==="
rg -n 'encrypt|cipher|hash|secure|password' --type=ts -iRepository: jacksonkasi1/ImagePro-Export
Length of output: 17625
🏁 Script executed:
#!/bin/bash
# Check AISettingsPanel to verify it calls setSettings with apiKey
echo "=== Checking AISettingsPanel.tsx around line 70-73 ==="
sed -n '60,90p' plugin/src/pages/AIPage/_components/AISettingsPanel.tsx
# Look for the complete handleApiKeyChange function
echo ""
echo "=== Searching for handleApiKeyChange in AISettingsPanel ==="
rg -n 'handleApiKeyChange|setSettings.*apiKey' plugin/src/pages/AIPage/_components/AISettingsPanel.tsx -A 3 -B 1
# Check what store this component uses
echo ""
echo "=== Checking imports and store usage ==="
head -30 plugin/src/pages/AIPage/_components/AISettingsPanel.tsxRepository: jacksonkasi1/ImagePro-Export
Length of output: 2026
Remove plaintext API key persistence to Figma's clientStorage.
The setSettings call in line 72 persists the API key via the store's persistSettings function, which writes it to Figma's clientStorage in plaintext (see use-ai-store.ts line 42 and main.ts line 59). Although the code separates the API key into a distinct storage key (aiApiKey), it remains unencrypted in client storage, exposing the credential to unauthorized access.
Expected fix: Either keep the API key in-memory only (session-scoped, not persisted) or encrypt it before storage if persistence is required.
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[warning] 70-70: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L70
Unsafe assignment of an error typed value.
[warning] 70-70: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L70
Unsafe call of an error type typed value.
[warning] 72-72: plugin/src/pages/AIPage/_components/AISettingsPanel.tsx#L72
Unsafe call of an error type typed value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx` around lines 70 -
73, The API key is being persisted in plaintext via setSettings (triggered in
handleApiKeyChange) which calls the store's persistSettings; change this so the
raw API key is not written to clientStorage: either keep the key in
component/local session state only (store in a React useState or a transient
field and remove the apiKey property from the object passed to
setSettings/persistSettings), or if you must persist, encrypt the key before
calling persistSettings and update the store (persistSettings) to accept and
store only the encrypted value (and ensure main.ts decrypts on retrieval using a
secure key source). Update handleApiKeyChange and any callers of
setSettings/persistSettings to omit plaintext apiKey, and reference the store
functions in use-ai-store.ts and storage handling in main.ts when adding
encryption logic.
… animation Layout - pages/index.tsx: wrap root in flex-col h-screen so all pages fill the window - pages/AssetPage, UploadPage: adopt flex-col h-full so footer stays pinned - pages/AIPage: switch to flex-col h-full; node list is flex-1 overflow-y-auto so it scrolls independently; footer (prefix/suffix + run button) is always visible at the bottom via shrink-0 Prefix / Suffix - types/ai.ts (AIPersistedSettings): add prefix and suffix fields - store/use-ai-store.ts: default prefix='img_', suffix='' - pages/AIPage/index.tsx: add two compact inputs in the footer row; apply prefix+suffix to every suggestedName before dispatching AI_APPLY_RENAME Thumbnail skeleton - pages/AIPage/_components/AINodeList.tsx: useThumbnailUrls now tracks loadingIds; rows show animate-pulse skeleton while blob URL is being created, then swap to the real <img> — blank placeholder only for nodes with no imageData
Separate AI rename workflows for image assets and structural layers with independent state/settings and mode-aware UI flow. Improve context-aware naming by expanding section grouping and wiring dedicated image/layer node fetch events across plugin and UI.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/src/ui.tsx (1)
81-117:⚠️ Potential issue | 🟠 MajorReturn cleanup function to prevent duplicate event handlers.
Lines 82, 93, 99, and 104 register listeners with
on()from@create-figma-plugin/utilitiesbut don't capture the returned unsubscribe functions. Since the effect dependency array includesexportMode(line 114), the effect can rerun and register additional handlers without removing the previous ones, causing events to be processed multiple times. The correct pattern, as used inplugin/src/pages/AIPage/index.tsx, is to capture each unsubscribe function and call them in a cleanup return.🔧 Proposed fix
useEffect(() => { - on<FetchImageNodesHandler>('FETCH_IMAGE_NODES', (image_nodes: NodeData[]) => { + const unsubFetchImageNodes = on<FetchImageNodesHandler>('FETCH_IMAGE_NODES', (image_nodes: NodeData[]) => { setAllNodes(image_nodes); setAllNodesCount(image_nodes.length); // Don't reset AI tab's independent selection when on the AI page if (useUtilsStore.getState().currentPage !== 'ai') { setSelectedNodeIds([]); setSelectedNodesCount(0); } }); // AI Images sub-tab: recursed IMAGE-fill nodes - on<FetchAIImageNodesHandler>('FETCH_AI_IMAGE_NODES', (nodes: AINodeData[]) => { + const unsubFetchAIImageNodes = on<FetchAIImageNodesHandler>('FETCH_AI_IMAGE_NODES', (nodes: AINodeData[]) => { setAIImageNodes(nodes); setSelectedAIImageNodeIds([]); }); // AI Layers sub-tab: top-level non-image nodes - on<FetchAILayerNodesHandler>('FETCH_AI_LAYER_NODES', (nodes: AINodeData[]) => { + const unsubFetchAILayerNodes = on<FetchAILayerNodesHandler>('FETCH_AI_LAYER_NODES', (nodes: AINodeData[]) => { setAILayerNodes(nodes); setSelectedAILayerNodeIds([]); }); - on<ExportCompleteHandler>('EXPORT_COMPLETE', (data: ImageData[]) => { + const unsubExportComplete = on<ExportCompleteHandler>('EXPORT_COMPLETE', (data: ImageData[]) => { handleExportComplete({ data, setIsLoading, exportMode, exportSettings: exportSettingsRef.current, }); }); + return () => { + unsubFetchImageNodes(); + unsubFetchAIImageNodes(); + unsubFetchAILayerNodes(); + unsubExportComplete(); + }; }, [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/ui.tsx` around lines 81 - 117, The useEffect registers multiple event listeners via on<...> ('FETCH_IMAGE_NODES', 'FETCH_AI_IMAGE_NODES', 'FETCH_AI_LAYER_NODES', 'EXPORT_COMPLETE') but never captures the unsubscribe functions; modify the effect to store each returned unsubscribe (e.g., const offFetchImage = on<FetchImageNodesHandler>(...), offFetchAIImage = on<...>, etc.) and return a cleanup function that calls each unsubscribe to remove handlers when the effect reruns or unmounts (remember exportMode is in the dependency array), ensuring you still call set... handlers inside the listeners as before.
♻️ Duplicate comments (3)
plugin/src/pages/AIPage/index.tsx (1)
77-80:⚠️ Potential issue | 🟡 Minor
finalizestill reports success after node-level errors.Line 133 can call
finalize()from the error handler, but Line 79 always emits a success toast.Also applies to: 130-134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/index.tsx` around lines 77 - 80, The finalize function always shows a success toast (notify.success) even when called from error paths; remove the toast from finalize and instead call notify.success only on the successful completion path(s). Concretely, update finalize (used with setIsRunning) to only setIsRunning(false) (remove notify.success), and add explicit notify.success calls where operations complete successfully (replace calls that previously invoked finalize in success branches), while ensuring error handlers call the updated finalize without triggering success to avoid false success toasts.plugin/src/pages/AIPage/_components/AISettingsPanel.tsx (2)
70-73:⚠️ Potential issue | 🟠 MajorDo not persist raw API keys through
setSettingson every keystroke.Line 72 still sends plaintext API key into persisted store flow (
use-ai-store.ts), which is a security risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx` around lines 70 - 73, handleApiKeyChange is currently writing the raw API key into the persisted store via setSettings on every keystroke; stop persisting plaintext keys from the input handler. Keep the live key in local component state (e.g., a useState within AISettingsPanel) and remove setSettings from handleApiKeyChange, then only call setSettings (or a secure vault API) once on explicit user action (onBlur or Save) and persist a masked value or an identifier (last4/hash/token reference) instead of the raw key; reference handleApiKeyChange, setSettings and use-ai-store.ts when making this change.
54-58:⚠️ Potential issue | 🟡 MinorGuard
MODEL_OPTIONSlookups with a safe fallback provider.Line 57 and Line 122 still assume
MODEL_OPTIONS[...]always exists; stale persistedmodelProvidercan crash rendering/selection.🛡️ Proposed fix
const AISettingsPanel = ({ settings, setSettings }: AISettingsPanelProps) => { + const providerConfig = MODEL_OPTIONS[settings.modelProvider] ?? MODEL_OPTIONS.gemini; + const handleProviderChange = useCallback( (e: Event) => { const provider = (e.target as HTMLSelectElement).value as AIModelProvider; - const defaultModel = MODEL_OPTIONS[provider].models[0]; + const defaultModel = (MODEL_OPTIONS[provider] ?? MODEL_OPTIONS.gemini).models[0]; setSettings({ modelProvider: provider, model: defaultModel }); }, [setSettings] ); @@ - <select class={selectClass} value={settings.model} onChange={handleModelChange}> - {MODEL_OPTIONS[settings.modelProvider].models.map((m) => ( + <select class={selectClass} value={settings.model} onChange={handleModelChange}> + {providerConfig.models.map((m) => ( <option key={m} value={m}> {m} </option> ))} </select>Also applies to: 121-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx` around lines 54 - 58, The code assumes MODEL_OPTIONS[provider] always exists which can crash if a stale persisted modelProvider is present; update lookups in handleProviderChange and the selection/initialization logic around where modelProvider/model are read (lines around 121-123) to guard with a safe fallback provider (e.g., DEFAULT_PROVIDER or the first key of MODEL_OPTIONS). When computing defaultModel use MODEL_OPTIONS[provider] ?? MODEL_OPTIONS[fallback] and ensure a valid models array exists before selecting [0], then call setSettings with the resolved provider and model so rendering/selection never accesses undefined MODEL_OPTIONS entries.
🧹 Nitpick comments (2)
plugin/src/types/ai.ts (1)
48-63: Consider sharing types with server-side definitions.
AIRenameGroupandAIRenameResultare defined here and also inserver/imagepro-file-process/src/routes/ai/adapters/types.ts. This duplication could lead to drift. Consider extracting shared types to a common package or ensuring synchronization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/types/ai.ts` around lines 48 - 63, The AIRenameGroup and AIRenameResult type definitions are duplicated and risk drifting; replace the local declarations of AIRenameGroup and AIRenameResult with imports from a single shared source (or move them to a new shared types package) and update all references to use the shared types instead of the inline interfaces; specifically remove the interface blocks for AIRenameGroup and AIRenameResult in this file and import the canonical types so both the plugin and server use the same definitions and keep exports/shape identical to the server adapter.plugin/src/core/ai/section-grouper.ts (1)
145-147: Consider extracting the grandparent check for readability.The condition on Line 145 is dense with multiple checks. Consider extracting to a helper variable for clarity.
♻️ Suggested readability improvement
+ const hasValidGrandparent = + grandparent && + grandparent.type !== 'PAGE' && + grandparent.type !== 'DOCUMENT' && + grandparent.name && + !isGenericName(grandparent.name); + // Grandparent name for extra category signal (e.g. "article-card", "author-section") - if (grandparent && grandparent.type !== 'PAGE' && (grandparent as BaseNode).type !== 'DOCUMENT' && grandparent.name && !isGenericName(grandparent.name)) { + if (hasValidGrandparent) { ctx.sectionContext = grandparent.name; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/section-grouper.ts` around lines 145 - 147, The condition that assigns ctx.sectionContext based on grandparent is dense and hard to read; extract the combined predicate into a clearly named boolean helper (e.g., hasValidGrandparent or isDescriptiveGrandparent) that encapsulates checks on grandparent, grandparent.type !== 'PAGE', (grandparent as BaseNode).type !== 'DOCUMENT', grandparent.name, and !isGenericName(grandparent.name), then replace the inline condition with that helper so the assignment in section-grouper.ts reads clearly as: if (grandparent && hasValidGrandparent(grandparent)) { ctx.sectionContext = grandparent.name; } referencing the existing grandparent symbol, isGenericName, BaseNode cast, and ctx.sectionContext.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/src/core/ai/gemini-adapter.ts`:
- Around line 30-34: The flattened results from promisePool may not match the
input groups because promisePool can resolve out of order; modify the call in
AIRenameResult[] producer (the function containing groups.map and promisePool)
to preserve group ordering: when building tasks for promisePool (currently
mapping to () => this.renameGroup(group,...)), include each group's original
index (or wrap each task result with { index, result }) and after promisePool
completes, sort results by index and then flat the ordered results so
node-to-name mappings produced by renameGroup remain aligned with the original
groups; reference the functions/variables promisePool, renameGroup, groups, and
AIRenameResult in your change.
In `@plugin/src/core/ai/promise-pool.ts`:
- Around line 19-29: The results array is being filled in completion order via
results.push(result), breaking the promised "in order of task definition";
modify the loop in promise-pool.ts that iterates over tasks so each task
captures its definition index (e.g., use for (let i = 0; i < tasks.length; i++)
and const idx = i) and assign results[idx] = result inside the .then handler
instead of pushing; ensure results is pre-sized (or otherwise safe to assign by
index) so the final results array preserves the original task order and index
alignment with the input tasks.
In `@plugin/src/main.ts`:
- Around line 86-96: The function hasImageFill is declared but never used;
remove the unused function declaration to clean up dead code—delete the entire
hasImageFill function (the top-level function that accepts a SceneNode and
recursively checks 'fills' and 'children') and any related unused imports or
type-only references tied exclusively to it (e.g., any standalone uses of
Paint/SceneNode types only for this function) to keep the file minimal.
In `@plugin/src/pages/AIPage/index.tsx`:
- Line 87: The forEach callbacks using concise arrow bodies (e.g.,
groups.forEach((g) => g.targetNodes.forEach((t) => setRenameStatus(t.nodeId,
'pending')))) trigger the Biome useIterableCallbackReturn lint; change both
occurrences (the outer groups.forEach and the inner targetNodes.forEach usages,
also the similar callback at the other location around Line 117) to use
block-bodied arrow functions with explicit return/statement blocks so each
callback body is wrapped in { ... } and calls setRenameStatus(...) inside the
block.
- Around line 39-45: The regex leadingImgPattern is too permissive (imag?e?
matches "ima" and "imagination") and can corrupt names; update the pattern used
where leadingImgPattern is defined so it only matches the full prefixes "img" or
"image" followed by an optional separator (dash/underscore/space) and not
partial words. Replace the current /^imag?e?[\-_ ]?/i with a pattern like
/^(?:img|image)(?=$|[\-_ ])/i or /^(?:img|image)(?:[\-_ ]+)?/i so the while loop
that updates stripped/name only strips actual "img" or "image" prefixes (refer
to the leadingImgPattern variable and the loop manipulating stripped).
---
Outside diff comments:
In `@plugin/src/ui.tsx`:
- Around line 81-117: The useEffect registers multiple event listeners via
on<...> ('FETCH_IMAGE_NODES', 'FETCH_AI_IMAGE_NODES', 'FETCH_AI_LAYER_NODES',
'EXPORT_COMPLETE') but never captures the unsubscribe functions; modify the
effect to store each returned unsubscribe (e.g., const offFetchImage =
on<FetchImageNodesHandler>(...), offFetchAIImage = on<...>, etc.) and return a
cleanup function that calls each unsubscribe to remove handlers when the effect
reruns or unmounts (remember exportMode is in the dependency array), ensuring
you still call set... handlers inside the listeners as before.
---
Duplicate comments:
In `@plugin/src/pages/AIPage/_components/AISettingsPanel.tsx`:
- Around line 70-73: handleApiKeyChange is currently writing the raw API key
into the persisted store via setSettings on every keystroke; stop persisting
plaintext keys from the input handler. Keep the live key in local component
state (e.g., a useState within AISettingsPanel) and remove setSettings from
handleApiKeyChange, then only call setSettings (or a secure vault API) once on
explicit user action (onBlur or Save) and persist a masked value or an
identifier (last4/hash/token reference) instead of the raw key; reference
handleApiKeyChange, setSettings and use-ai-store.ts when making this change.
- Around line 54-58: The code assumes MODEL_OPTIONS[provider] always exists
which can crash if a stale persisted modelProvider is present; update lookups in
handleProviderChange and the selection/initialization logic around where
modelProvider/model are read (lines around 121-123) to guard with a safe
fallback provider (e.g., DEFAULT_PROVIDER or the first key of MODEL_OPTIONS).
When computing defaultModel use MODEL_OPTIONS[provider] ??
MODEL_OPTIONS[fallback] and ensure a valid models array exists before selecting
[0], then call setSettings with the resolved provider and model so
rendering/selection never accesses undefined MODEL_OPTIONS entries.
In `@plugin/src/pages/AIPage/index.tsx`:
- Around line 77-80: The finalize function always shows a success toast
(notify.success) even when called from error paths; remove the toast from
finalize and instead call notify.success only on the successful completion
path(s). Concretely, update finalize (used with setIsRunning) to only
setIsRunning(false) (remove notify.success), and add explicit notify.success
calls where operations complete successfully (replace calls that previously
invoked finalize in success branches), while ensuring error handlers call the
updated finalize without triggering success to avoid false success toasts.
---
Nitpick comments:
In `@plugin/src/core/ai/section-grouper.ts`:
- Around line 145-147: The condition that assigns ctx.sectionContext based on
grandparent is dense and hard to read; extract the combined predicate into a
clearly named boolean helper (e.g., hasValidGrandparent or
isDescriptiveGrandparent) that encapsulates checks on grandparent,
grandparent.type !== 'PAGE', (grandparent as BaseNode).type !== 'DOCUMENT',
grandparent.name, and !isGenericName(grandparent.name), then replace the inline
condition with that helper so the assignment in section-grouper.ts reads clearly
as: if (grandparent && hasValidGrandparent(grandparent)) { ctx.sectionContext =
grandparent.name; } referencing the existing grandparent symbol, isGenericName,
BaseNode cast, and ctx.sectionContext.
In `@plugin/src/types/ai.ts`:
- Around line 48-63: The AIRenameGroup and AIRenameResult type definitions are
duplicated and risk drifting; replace the local declarations of AIRenameGroup
and AIRenameResult with imports from a single shared source (or move them to a
new shared types package) and update all references to use the shared types
instead of the inline interfaces; specifically remove the interface blocks for
AIRenameGroup and AIRenameResult in this file and import the canonical types so
both the plugin and server use the same definitions and keep exports/shape
identical to the server adapter.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
plugin/src/core/ai/gemini-adapter.tsplugin/src/core/ai/promise-pool.tsplugin/src/core/ai/section-grouper.tsplugin/src/main.tsplugin/src/pages/AIPage/_components/AINodeList.tsxplugin/src/pages/AIPage/_components/AIRunButton.tsxplugin/src/pages/AIPage/_components/AISettingsPanel.tsxplugin/src/pages/AIPage/index.tsxplugin/src/pages/AssetPage/index.tsxplugin/src/pages/UploadPage/index.tsxplugin/src/pages/index.tsxplugin/src/store/use-ai-store.tsplugin/src/styles/output.cssplugin/src/types/ai.tsplugin/src/types/events.tsplugin/src/ui.tsxplugin/typings/types.d.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/src/pages/index.tsx
- plugin/src/pages/AIPage/_components/AIRunButton.tsx
- plugin/typings/types.d.ts
Comment on lines
+30
to
+34
| ): Promise<AIRenameResult[]> { | ||
| const tasks = groups.map((group) => () => this.renameGroup(group, systemPrompt, caseOption)); | ||
| const groupResults = await promisePool(tasks, concurrency); | ||
| return groupResults.flat(); | ||
| } |
There was a problem hiding this comment.
Result ordering depends on promisePool behavior.
This code calls promisePool and then flat() the results. If promisePool doesn't preserve definition order (see review comment on promise-pool.ts), the flattened results may not align with the input groups, potentially causing incorrect node-to-name mappings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/core/ai/gemini-adapter.ts` around lines 30 - 34, The flattened
results from promisePool may not match the input groups because promisePool can
resolve out of order; modify the call in AIRenameResult[] producer (the function
containing groups.map and promisePool) to preserve group ordering: when building
tasks for promisePool (currently mapping to () => this.renameGroup(group,...)),
include each group's original index (or wrap each task result with { index,
result }) and after promisePool completes, sort results by index and then flat
the ordered results so node-to-name mappings produced by renameGroup remain
aligned with the original groups; reference the functions/variables promisePool,
renameGroup, groups, and AIRenameResult in your change.
Comment on lines
+19
to
+29
| for (const task of tasks) { | ||
| const p: Promise<void> = task() | ||
| .then((result) => { | ||
| results.push(result); | ||
| }) | ||
| .catch((err) => { | ||
| errors.push(err); | ||
| }) | ||
| .finally(() => { | ||
| executing.delete(p); | ||
| }); |
There was a problem hiding this comment.
Results are not preserved in definition order as documented.
The docstring (Line 5) claims results are collected "in order of task definition," but results.push(result) on Line 22 appends in completion order. If callers rely on index alignment with input tasks, this will cause silent data corruption.
🐛 Proposed fix to preserve definition order
export async function promisePool<T>(
tasks: Array<() => Promise<T>>,
concurrency: number
): Promise<T[]> {
- const results: T[] = [];
+ const results: (T | undefined)[] = new Array(tasks.length);
const errors: unknown[] = [];
const executing = new Set<Promise<void>>();
- for (const task of tasks) {
- const p: Promise<void> = task()
+ for (let i = 0; i < tasks.length; i++) {
+ const index = i;
+ const p: Promise<void> = tasks[index]()
.then((result) => {
- results.push(result);
+ results[index] = result;
})
.catch((err) => {
errors.push(err);
})
.finally(() => {
executing.delete(p);
});
executing.add(p);
if (executing.size >= concurrency) {
await Promise.race(executing);
}
}
// Drain any remaining tasks
await Promise.all(executing);
if (errors.length > 0) {
throw errors[0];
}
- return results;
+ return results as T[];
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/core/ai/promise-pool.ts` around lines 19 - 29, The results array
is being filled in completion order via results.push(result), breaking the
promised "in order of task definition"; modify the loop in promise-pool.ts that
iterates over tasks so each task captures its definition index (e.g., use for
(let i = 0; i < tasks.length; i++) and const idx = i) and assign results[idx] =
result inside the .then handler instead of pushing; ensure results is pre-sized
(or otherwise safe to assign by index) so the final results array preserves the
original task order and index alignment with the input tasks.
Comment on lines
+86
to
+96
| function hasImageFill(node: SceneNode): boolean { | ||
| if ('fills' in node && Array.isArray(node.fills)) { | ||
| if ((node.fills as ReadonlyArray<Paint>).some((f) => f.type === 'IMAGE')) return true; | ||
| } | ||
| if ('children' in node) { | ||
| for (const child of node.children) { | ||
| if (hasImageFill(child as SceneNode)) return true; | ||
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Remove unused hasImageFill function.
The hasImageFill function is defined but never called anywhere in this file.
🧹 Proposed fix
-/**
- * Checks whether a node (or any of its descendants) contains an IMAGE fill.
- * Used by the Layers collector to exclude nodes that are already covered by
- * the Images sub-tab.
- */
-function hasImageFill(node: SceneNode): boolean {
- if ('fills' in node && Array.isArray(node.fills)) {
- if ((node.fills as ReadonlyArray<Paint>).some((f) => f.type === 'IMAGE')) return true;
- }
- if ('children' in node) {
- for (const child of node.children) {
- if (hasImageFill(child as SceneNode)) return true;
- }
- }
- return false;
-}📝 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
| function hasImageFill(node: SceneNode): boolean { | |
| if ('fills' in node && Array.isArray(node.fills)) { | |
| if ((node.fills as ReadonlyArray<Paint>).some((f) => f.type === 'IMAGE')) return true; | |
| } | |
| if ('children' in node) { | |
| for (const child of node.children) { | |
| if (hasImageFill(child as SceneNode)) return true; | |
| } | |
| } | |
| return false; | |
| } |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[warning] 86-86: plugin/src/main.ts#L86
'hasImageFill' is defined but never used.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/main.ts` around lines 86 - 96, The function hasImageFill is
declared but never used; remove the unused function declaration to clean up dead
code—delete the entire hasImageFill function (the top-level function that
accepts a SceneNode and recursively checks 'fills' and 'children') and any
related unused imports or type-only references tied exclusively to it (e.g., any
standalone uses of Paint/SceneNode types only for this function) to keep the
file minimal.
Comment on lines
+39
to
+45
| const leadingImgPattern = /^imag?e?[\-_ ]?/i; | ||
| let stripped = name; | ||
| while (leadingImgPattern.test(stripped)) { | ||
| const next = stripped.replace(leadingImgPattern, ''); | ||
| if (next === stripped) break; | ||
| stripped = next; | ||
| } |
There was a problem hiding this comment.
Fix prefix stripping regex: it misses img and can corrupt unrelated names.
Line 39 uses imag?e?, which matches ima/imag/image (not img) and can strip valid names like imagination....
🔧 Proposed fix
- const leadingImgPattern = /^imag?e?[\-_ ]?/i;
+ const leadingImgPattern = /^(?:img|image)[-_ ]?/i;📝 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
| const leadingImgPattern = /^imag?e?[\-_ ]?/i; | |
| let stripped = name; | |
| while (leadingImgPattern.test(stripped)) { | |
| const next = stripped.replace(leadingImgPattern, ''); | |
| if (next === stripped) break; | |
| stripped = next; | |
| } | |
| const leadingImgPattern = /^(?:img|image)[-_ ]?/i; | |
| let stripped = name; | |
| while (leadingImgPattern.test(stripped)) { | |
| const next = stripped.replace(leadingImgPattern, ''); | |
| if (next === stripped) break; | |
| stripped = next; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/pages/AIPage/index.tsx` around lines 39 - 45, The regex
leadingImgPattern is too permissive (imag?e? matches "ima" and "imagination")
and can corrupt names; update the pattern used where leadingImgPattern is
defined so it only matches the full prefixes "img" or "image" followed by an
optional separator (dash/underscore/space) and not partial words. Replace the
current /^imag?e?[\-_ ]?/i with a pattern like /^(?:img|image)(?=$|[\-_ ])/i or
/^(?:img|image)(?:[\-_ ]+)?/i so the while loop that updates stripped/name only
strips actual "img" or "image" prefixes (refer to the leadingImgPattern variable
and the loop manipulating stripped).
| const unsubBatchReady = on<AIBatchReadyHandler>('AI_BATCH_READY', async (groups: AIRenameGroup[]) => { | ||
| const total = groups.reduce((acc, g) => acc + g.targetNodes.length, 0); | ||
| setProgress({ done: 0, total }); | ||
| groups.forEach((g) => g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'pending'))); |
There was a problem hiding this comment.
Use block bodies in these forEach callbacks to avoid Biome useIterableCallbackReturn errors.
The concise callback form on Line 87 and Line 117 is flagged by static analysis and may fail lint gates.
🔧 Proposed fix
- groups.forEach((g) => g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'pending')));
+ groups.forEach((g) => {
+ g.targetNodes.forEach((t) => {
+ setRenameStatus(t.nodeId, 'pending');
+ });
+ });
@@
- groups.forEach((g) => g.targetNodes.forEach((t) => setRenameStatus(t.nodeId, 'error')));
+ groups.forEach((g) => {
+ g.targetNodes.forEach((t) => {
+ setRenameStatus(t.nodeId, 'error');
+ });
+ });Also applies to: 117-117
🧰 Tools
🪛 Biome (2.4.4)
[error] 87-87: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
[error] 87-87: This callback passed to forEach() iterable method should not return a value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/pages/AIPage/index.tsx` at line 87, The forEach callbacks using
concise arrow bodies (e.g., groups.forEach((g) => g.targetNodes.forEach((t) =>
setRenameStatus(t.nodeId, 'pending')))) trigger the Biome
useIterableCallbackReturn lint; change both occurrences (the outer
groups.forEach and the inner targetNodes.forEach usages, also the similar
callback at the other location around Line 117) to use block-bodied arrow
functions with explicit return/statement blocks so each callback body is wrapped
in { ... } and calls setRenameStatus(...) inside the block.
- replaced generic heading boolean with precise font size scoring - implemented progressive duplicate penalization for text scoring - refined avatar and hero inference using precise absolute & relative dimensions - enhanced gemini adapter fallback by incorporating section hierarchy - updated default system prompt for testimonials avatar scenario
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
plugin/src/core/ai/section-grouper.ts (1)
157-180: Bound nearby-text traversal to avoid heavy scans on large pages.Lines 167–180 can traverse many sibling subtrees across ancestor levels. Add a global candidate cap and short-circuit loops once reached.
⚡ Suggested refactor
+const MAX_RAW_NEARBY_CANDIDATES = 200; + function gatherScoredNearbyText(node: SceneNode, ancestors: SceneNode[]): ScoredTextItem[] { const rawCandidates: Array<{ @@ - levelNodes.forEach((levelNode, levelIndex) => { - const siblings = getSiblingNodes(levelNode); - siblings.forEach((sibling) => { - const candidates = collectTextCandidates(sibling); - candidates.forEach((candidate) => { + for (let levelIndex = 0; levelIndex < levelNodes.length; levelIndex += 1) { + const siblings = getSiblingNodes(levelNodes[levelIndex]); + for (const sibling of siblings) { + if (rawCandidates.length >= MAX_RAW_NEARBY_CANDIDATES) break; + const candidates = collectTextCandidates(sibling); + for (const candidate of candidates) { + if (rawCandidates.length >= MAX_RAW_NEARBY_CANDIDATES) break; rawCandidates.push({ text: candidate.text, fontSize: candidate.fontSize, directText: candidate.directText, sourceLevel: levelIndex, }); - }); - }); - }); + } + } + if (rawCandidates.length >= MAX_RAW_NEARBY_CANDIDATES) break; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/section-grouper.ts` around lines 157 - 180, In gatherScoredNearbyText add a global candidate cap (e.g., MAX_NEARBY_CANDIDATES) and short-circuit the nested loops that call getSiblingNodes and collectTextCandidates when rawCandidates.length reaches the cap: while iterating levelNodes (and their siblings and candidates) check the cap before pushing and break out of the inner loops and outer loop (use levelIndex/levelNodes and sibling iteration control) so traversal stops as soon as the cap is hit to avoid scanning large pages.plugin/src/core/ai/gemini-adapter.ts (4)
484-503: Same refactor applies here.This function has the same nested
forEachpattern that could be simplified withflatMap.♻️ Proposed refactor using flatMap
function extractLikelyPersonName(texts: string[]): string | undefined { - const candidates: string[] = []; - - texts.forEach((text) => { - text - .split(/\n+/) - .map((part) => part.trim()) - .filter(Boolean) - .forEach((part) => candidates.push(part)); - }); + const candidates = texts.flatMap((text) => + text.split(/\n+/).map((part) => part.trim()).filter(Boolean) + ); for (const candidate of candidates) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/gemini-adapter.ts` around lines 484 - 503, The nested forEach in extractLikelyPersonName can be simplified by building the candidates array with a single flatMap call: replace the texts.forEach(...) block with texts.flatMap(...) that splits on newlines, trims, and filters empties to produce the candidates list in one step, then keep the existing loop that normalizes (clean) and tests the regex; this removes the inner forEach nesting while preserving behavior in extractLikelyPersonName.
13-13: Consider usingunknowninstead ofanyfor better type safety.Using
unknownforces explicit type checks before accessing properties, making the code more self-documenting about handling dynamic data.♻️ Proposed type improvement
-function buildCompactContext(ctx: any): Record<string, unknown> { +function buildCompactContext(ctx: unknown): Record<string, unknown> { + if (!ctx || typeof ctx !== 'object') { + return { targetNode: {}, nearbyText: [], sectionHierarchy: [] }; + } + const c = ctx as Record<string, unknown>; const nearbyText = Array.isArray(ctx?.scoredNearbyText)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/gemini-adapter.ts` at line 13, Replace the use of any with unknown in the buildCompactContext signature and locally narrow the type before accessing properties: change function buildCompactContext(ctx: any) to buildCompactContext(ctx: unknown) and inside the function perform type guards (e.g., typeof checks, Array.isArray, and property-existence checks or a user-defined type predicate) before reading ctx fields; update any helper functions or variable declarations inside buildCompactContext that assume ctx is an object to accept the narrowed type rather than any so all property access is preceded by explicit checks.
456-482: Consider usingflatMapfor cleaner iteration.The nested
forEachwithpushworks correctly, but usingflatMapwould be more idiomatic and avoids the static analysis warning about forEach callbacks returning values.♻️ Proposed refactor using flatMap
function extractBestSubjectText(texts: string[]): string | undefined { - const candidates: string[] = []; - - texts.forEach((text) => { - text - .split(/\n+/) - .map((part) => part.trim()) - .filter(Boolean) - .forEach((part) => candidates.push(part)); - }); + const candidates = texts.flatMap((text) => + text.split(/\n+/).map((part) => part.trim()).filter(Boolean) + ); for (const candidate of candidates) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/gemini-adapter.ts` around lines 456 - 482, The nested forEach that builds candidates should be replaced with a flatMap-based construction to be more idiomatic and avoid the forEach callback return warning: inside extractBestSubjectText, replace the texts.forEach(...) push logic with const candidates = texts.flatMap(text => text.split(/\n+/).map(p => p.trim()).filter(Boolean)); then continue using candidates as before (retain the cleaning, regex checks, and return logic in extractBestSubjectText).
104-109: Consider adding a timeout to the fetch request.The
fetchcall has no timeout, which could cause the operation to hang indefinitely if the Gemini API is unresponsive. Consider usingAbortControllerwith a timeout.♻️ Proposed fix using AbortController
try { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); const response = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), + signal: controller.signal, }); + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/core/ai/gemini-adapter.ts` around lines 104 - 109, The fetch to Gemini lacks a timeout and can hang; add an AbortController, create a timer (e.g., setTimeout) to call controller.abort() after a configurable timeout, pass controller.signal into the existing fetch call (the one creating const response), and clear the timer once fetch resolves; also handle the abort error path (throw or return a clear timeout error) so callers of the enclosing function (the Gemini adapter fetch block) receive a deterministic failure instead of hanging.plugin/src/store/use-ai-store.ts (1)
128-139: Consider debouncing persistence to reduce write chatter.
SET_DATAis emitted on every settings update; typing in prompt/API key can generate many rapid writes. A short debounce around persistence would lower IPC/storage churn.Also applies to: 202-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/src/store/use-ai-store.ts` around lines 128 - 139, persistSettings and persistLayerSettings currently call emit('SET_DATA') on every update causing write churn; wrap their emit logic in a shared debounced persister (e.g., create a debounced function used by both persistSettings and persistLayerSettings) that batches calls with a short delay (200–500ms) and merges successive payloads so the last apiKey and settings win; ensure the debounced helper dispatches both handles (SETTINGS_KEY/LAYER_SETTINGS_KEY and API_KEY_KEY) correctly, provides a flush method to force immediate write (callable on unmount or explicit save), and preserves the existing separation of apiKey from persisted settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/src/core/ai/section-grouper.ts`:
- Around line 253-259: The current buildSectionHierarchy reverses and then
slices to keep the top-most 4 ancestors, which can drop the nearest parent;
change the truncation to preserve the closest ancestors instead (i.e., take the
last up to 4 items of the meaningful ancestors array rather than the
first/top-most). Update buildSectionHierarchy and the analogous truncation
around parentContext to use a "keep last N" approach (e.g., slice(-4) or
equivalent) so the actual nearest parent remains in the returned hierarchy.
In `@plugin/src/store/use-ai-store.ts`:
- Around line 202-216: When updating settings via setSettings or
setLayerSettings, the shared apiKey can become stale because each updater only
writes its own settings object; modify both setters so when partial includes an
apiKey you merge that apiKey into the other mode as well and call the
corresponding persist function for both objects. Concretely: in setSettings
(which builds next from state.settings) if partial.apiKey is present also
compute nextLayer = { ...state.layerSettings, apiKey: partial.apiKey } and call
persistLayerSettings(nextLayer) and include layerSettings: nextLayer in the
returned state; do the symmetric update in setLayerSettings (update settings +
persistSettings when partial.apiKey exists). Ensure you only synchronize the
apiKey field and still merge other fields normally.
- Around line 142-167: handleReceiveData currently trusts incoming data and
spreads it into state after casting, which will break if storage returns a
primitive or array; before calling
normalizeLoadedImageSettings/normalizeLoadedLayerSettings and before
object-spreading into settings or layerSettings (paths: handleReceiveData,
normalizeLoadedImageSettings, normalizeLoadedLayerSettings, SETTINGS_KEY,
LAYER_SETTINGS_KEY), validate that data is a plain object (e.g., typeof data ===
'object' && data !== null && !Array.isArray(data') or use an isPlainObject
helper), and bail or coerce to an empty object when the check fails so you never
spread non-object values into state or emit malformed normalized payloads.
---
Nitpick comments:
In `@plugin/src/core/ai/gemini-adapter.ts`:
- Around line 484-503: The nested forEach in extractLikelyPersonName can be
simplified by building the candidates array with a single flatMap call: replace
the texts.forEach(...) block with texts.flatMap(...) that splits on newlines,
trims, and filters empties to produce the candidates list in one step, then keep
the existing loop that normalizes (clean) and tests the regex; this removes the
inner forEach nesting while preserving behavior in extractLikelyPersonName.
- Line 13: Replace the use of any with unknown in the buildCompactContext
signature and locally narrow the type before accessing properties: change
function buildCompactContext(ctx: any) to buildCompactContext(ctx: unknown) and
inside the function perform type guards (e.g., typeof checks, Array.isArray, and
property-existence checks or a user-defined type predicate) before reading ctx
fields; update any helper functions or variable declarations inside
buildCompactContext that assume ctx is an object to accept the narrowed type
rather than any so all property access is preceded by explicit checks.
- Around line 456-482: The nested forEach that builds candidates should be
replaced with a flatMap-based construction to be more idiomatic and avoid the
forEach callback return warning: inside extractBestSubjectText, replace the
texts.forEach(...) push logic with const candidates = texts.flatMap(text =>
text.split(/\n+/).map(p => p.trim()).filter(Boolean)); then continue using
candidates as before (retain the cleaning, regex checks, and return logic in
extractBestSubjectText).
- Around line 104-109: The fetch to Gemini lacks a timeout and can hang; add an
AbortController, create a timer (e.g., setTimeout) to call controller.abort()
after a configurable timeout, pass controller.signal into the existing fetch
call (the one creating const response), and clear the timer once fetch resolves;
also handle the abort error path (throw or return a clear timeout error) so
callers of the enclosing function (the Gemini adapter fetch block) receive a
deterministic failure instead of hanging.
In `@plugin/src/core/ai/section-grouper.ts`:
- Around line 157-180: In gatherScoredNearbyText add a global candidate cap
(e.g., MAX_NEARBY_CANDIDATES) and short-circuit the nested loops that call
getSiblingNodes and collectTextCandidates when rawCandidates.length reaches the
cap: while iterating levelNodes (and their siblings and candidates) check the
cap before pushing and break out of the inner loops and outer loop (use
levelIndex/levelNodes and sibling iteration control) so traversal stops as soon
as the cap is hit to avoid scanning large pages.
In `@plugin/src/store/use-ai-store.ts`:
- Around line 128-139: persistSettings and persistLayerSettings currently call
emit('SET_DATA') on every update causing write churn; wrap their emit logic in a
shared debounced persister (e.g., create a debounced function used by both
persistSettings and persistLayerSettings) that batches calls with a short delay
(200–500ms) and merges successive payloads so the last apiKey and settings win;
ensure the debounced helper dispatches both handles
(SETTINGS_KEY/LAYER_SETTINGS_KEY and API_KEY_KEY) correctly, provides a flush
method to force immediate write (callable on unmount or explicit save), and
preserves the existing separation of apiKey from persisted settings.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugin/src/core/ai/gemini-adapter.tsplugin/src/core/ai/section-grouper.tsplugin/src/store/use-ai-store.ts
Comment on lines
+253
to
+259
| function buildSectionHierarchy(ancestors: SceneNode[]): string[] { | ||
| const meaningful = ancestors | ||
| .filter((ancestor) => ancestor.name && !isGenericName(ancestor.name)) | ||
| .map((ancestor) => ancestor.name.trim()); | ||
|
|
||
| return meaningful.reverse().slice(0, 4); | ||
| } |
There was a problem hiding this comment.
Preserve nearest ancestors when truncating section hierarchy.
Line 258 currently keeps the top-most 4 ancestors, which can drop the actual nearest parent in deep trees; then parentContext (Line 319) becomes misleading.
🔧 Suggested fix
function buildSectionHierarchy(ancestors: SceneNode[]): string[] {
- const meaningful = ancestors
+ const meaningful = ancestors
.filter((ancestor) => ancestor.name && !isGenericName(ancestor.name))
- .map((ancestor) => ancestor.name.trim());
+ .map((ancestor) => ancestor.name.trim())
+ .reverse(); // root -> nearest
- return meaningful.reverse().slice(0, 4);
+ // keep the nearest 4 while preserving root->nearest order
+ return meaningful.slice(-4);
}Also applies to: 317-321
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/core/ai/section-grouper.ts` around lines 253 - 259, The current
buildSectionHierarchy reverses and then slices to keep the top-most 4 ancestors,
which can drop the nearest parent; change the truncation to preserve the closest
ancestors instead (i.e., take the last up to 4 items of the meaningful ancestors
array rather than the first/top-most). Update buildSectionHierarchy and the
analogous truncation around parentContext to use a "keep last N" approach (e.g.,
slice(-4) or equivalent) so the actual nearest parent remains in the returned
hierarchy.
Comment on lines
+142
to
+167
| const handleReceiveData: ReceiveDataHandler['handler'] = ({ handle, data }: { handle: string; data: any }) => { | ||
| if (!data) return; | ||
| if (handle === SETTINGS_KEY) { | ||
| const normalized = normalizeLoadedImageSettings(data as Partial<AIPersistedSettings>); | ||
| set((state) => ({ | ||
| settings: { ...state.settings, ...normalized }, | ||
| })); | ||
|
|
||
| if (normalized.systemPrompt === DEFAULT_SYSTEM_PROMPT) { | ||
| emit<SetDataHandler>('SET_DATA', { handle: SETTINGS_KEY, data: normalized }); | ||
| } | ||
| } else if (handle === LAYER_SETTINGS_KEY) { | ||
| const normalized = normalizeLoadedLayerSettings(data as Partial<AIPersistedSettings>); | ||
| set((state) => ({ | ||
| layerSettings: { ...state.layerSettings, ...normalized }, | ||
| })); | ||
|
|
||
| if (normalized.systemPrompt === DEFAULT_LAYER_SYSTEM_PROMPT) { | ||
| emit<SetDataHandler>('SET_DATA', { handle: LAYER_SETTINGS_KEY, data: normalized }); | ||
| } | ||
| } else if (handle === API_KEY_KEY) { | ||
| const apiKey = (data as { apiKey: string }).apiKey ?? ''; | ||
| set((state) => ({ | ||
| settings: { ...state.settings, apiKey }, | ||
| layerSettings: { ...state.layerSettings, apiKey }, | ||
| })); |
There was a problem hiding this comment.
Guard RECEIVE_DATA payload shape before object spread.
Line 142 accepts data: any, and Line 145/154 spreads casted payloads into settings. If storage returns a malformed primitive/array, this can produce invalid state.
🛡️ Proposed hardening
+function isPlainObject(value: unknown): value is Record<string, unknown> {
+ return typeof value === 'object' && value !== null && !Array.isArray(value);
+}
+
const handleReceiveData: ReceiveDataHandler['handler'] = ({ handle, data }: { handle: string; data: any }) => {
if (!data) return;
if (handle === SETTINGS_KEY) {
- const normalized = normalizeLoadedImageSettings(data as Partial<AIPersistedSettings>);
+ if (!isPlainObject(data)) return;
+ const normalized = normalizeLoadedImageSettings(data as Partial<AIPersistedSettings>);
set((state) => ({
settings: { ...state.settings, ...normalized },
}));
@@
} else if (handle === LAYER_SETTINGS_KEY) {
- const normalized = normalizeLoadedLayerSettings(data as Partial<AIPersistedSettings>);
+ if (!isPlainObject(data)) return;
+ const normalized = normalizeLoadedLayerSettings(data as Partial<AIPersistedSettings>);
set((state) => ({
layerSettings: { ...state.layerSettings, ...normalized },
}));
@@
} else if (handle === API_KEY_KEY) {
- const apiKey = (data as { apiKey: string }).apiKey ?? '';
+ const apiKey = isPlainObject(data) && typeof data.apiKey === 'string' ? data.apiKey : '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/store/use-ai-store.ts` around lines 142 - 167, handleReceiveData
currently trusts incoming data and spreads it into state after casting, which
will break if storage returns a primitive or array; before calling
normalizeLoadedImageSettings/normalizeLoadedLayerSettings and before
object-spreading into settings or layerSettings (paths: handleReceiveData,
normalizeLoadedImageSettings, normalizeLoadedLayerSettings, SETTINGS_KEY,
LAYER_SETTINGS_KEY), validate that data is a plain object (e.g., typeof data ===
'object' && data !== null && !Array.isArray(data') or use an isPlainObject
helper), and bail or coerce to an empty object when the check fails so you never
spread non-object values into state or emit malformed normalized payloads.
Comment on lines
+202
to
+216
| setSettings: (partial: Partial<AISettings>) => | ||
| set((state) => { | ||
| const next = { ...state.settings, ...partial }; | ||
| persistSettings(next); | ||
| return { settings: next }; | ||
| }), | ||
|
|
||
| // ── Layer mode settings ────────────────────────────────────────────────── | ||
| layerSettings: { ...DEFAULT_LAYER_SETTINGS }, | ||
| setLayerSettings: (partial: Partial<AISettings>) => | ||
| set((state) => { | ||
| const next = { ...state.layerSettings, ...partial }; | ||
| persistLayerSettings(next); | ||
| return { layerSettings: next }; | ||
| }), |
There was a problem hiding this comment.
Synchronize shared apiKey across both mode settings.
apiKey is treated as shared, but Line 202-216 updates only one settings object at a time. This can leave stale apiKey in the inactive mode, and a later save from that mode can overwrite the newer key.
🔧 Proposed fix
setSettings: (partial: Partial<AISettings>) =>
set((state) => {
const next = { ...state.settings, ...partial };
+ const syncLayerSettings = Object.prototype.hasOwnProperty.call(partial, 'apiKey')
+ ? { ...state.layerSettings, apiKey: next.apiKey }
+ : state.layerSettings;
persistSettings(next);
- return { settings: next };
+ return { settings: next, layerSettings: syncLayerSettings };
}),
setLayerSettings: (partial: Partial<AISettings>) =>
set((state) => {
const next = { ...state.layerSettings, ...partial };
+ const syncImageSettings = Object.prototype.hasOwnProperty.call(partial, 'apiKey')
+ ? { ...state.settings, apiKey: next.apiKey }
+ : state.settings;
persistLayerSettings(next);
- return { layerSettings: next };
+ return { layerSettings: next, settings: syncImageSettings };
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/src/store/use-ai-store.ts` around lines 202 - 216, When updating
settings via setSettings or setLayerSettings, the shared apiKey can become stale
because each updater only writes its own settings object; modify both setters so
when partial includes an apiKey you merge that apiKey into the other mode as
well and call the corresponding persist function for both objects. Concretely:
in setSettings (which builds next from state.settings) if partial.apiKey is
present also compute nextLayer = { ...state.layerSettings, apiKey:
partial.apiKey } and call persistLayerSettings(nextLayer) and include
layerSettings: nextLayer in the returned state; do the symmetric update in
setLayerSettings (update settings + persistSettings when partial.apiKey exists).
Ensure you only synchronize the apiKey field and still merge other fields
normally.
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
Plugin changes (
plugin/src/){ value: 'ai', label: 'AI' }to tab-switchtypes/ai.ts—AIModelProvider,AIRenameStatus,AISettings,AIRenameGroup,AIRenameResult,AIState; extendedtypes/events.tswith 6 AI event handler interfacesstore/use-ai-store.ts— Zustand store tracking settings, per-node statuses, progress, isRunning, isSettingsOpencore/ai/svg-detector.ts— classifies nodes assvg_leaf | raster_leaf | sectionby detecting pure vector subtreescore/ai/section-grouper.ts— bundles ≤4 siblings sharing a parent into one group; falls back to per-node groupscore/ai/export-for-ai.ts— exports low-quality JPG (0.5x, fallback 0.25x if >150 KB) with sandbox-safe manual base64 encoder (noBuffer/atob)core/ai/rename-handler.ts— orchestrates classify→group→export (max 2 concurrent)→emit; also handlesapplyRenameandsignalRenameCompletepages/AIPage/):AIStatusBadge— idle/pending/done/error badge with pulse animationAINodeList— scrollable node list with checkbox, thumbnail, current name, per-node status badge; select-all headerAISettingsPanel— collapsible gear panel: provider/model picker, API key (password), naming convention, readImage toggle, system prompt textareaAIRunButton— disabled when no selection; shows live counter "Renaming 3/12…" while runningAIPage/index.tsx— page root wiringAI_BATCH_READY → fetch /api/ai/rename-batch → AI_APPLY_RENAME per result → AI_RENAME_PROGRESS updateshooks/useStorageManager.ts— registeredaiSettingskey to persist AI settings across plugin sessionscomponents/ui/icon-button.tsx— renamedloadingprop toisLoadingto avoid collision with Preact JSXimg.loadingattribute (was causing TS2430)Server changes (
server/imagepro-file-process/src/)routes/ai/index.ts—POST /api/ai/rename-batch: accepts{ groups, settings }, validates, delegates to adapter, returns{ renames: AIRenameResult[] }routes/ai/adapters/gemini.ts—GeminiAdapterusing@google/genaiSDK; parses JSON array response; max 5 concurrent calls via promise poolroutes/ai/adapters/openai.ts/anthropic.ts— stubs (throw "not yet implemented")routes/ai/adapters/index.ts—ModelAdapterFactory.create(provider, apiKey, model)utils/promise-pool.ts— genericpromisePool<T>(tasks, concurrency)utility/api/aiinroutes/index.tsBuild
pnpm run build— passes TypeScript typecheck + minified build cleannpx tsc --noEmit— passes with 0 errorsSummary by Sourcery
Add an AI-powered rename tab to the Figma plugin and corresponding server-side AI rename endpoint, enabling intelligent node renaming with per-node progress, settings persistence, and Gemini-based model support.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Summary by CodeRabbit
New Features
Style