Skip to content

Conversation

@thegovind
Copy link

@thegovind thegovind commented Dec 19, 2025

Summary

Adds support for Microsoft Foundry (formerly Azure AI Foundry) text-to-image models, enabling users to generate images using:

  • FLUX.2 Pro by Black Forest Labs
  • GPT-Image-1.5 with configurable quality settings

New Features

Microsoft Foundry Models

  • FLUX.2 Pro: High-quality image generation with configurable output sizes (1024×1024, 1024×768, 768×1024, 1536×1024, 1024×1536)
  • GPT-Image-1.5: GPT-based generation with quality controls (low/medium/high) and flexible sizing

Image Cropper/Grid Splitter

  • New modal for splitting generated images into grids (useful for contact sheets)
  • Freeform crop regions support
  • Export options: download individual files, ZIP archive, or add as new nodes

Other Improvements

  • Reusable download utilities for images
  • Improved output node with crop/split functionality

Configuration

Users configure their Microsoft Foundry endpoints via environment variables:

# FLUX.2 Pro
AZURE_API_KEY=your_api_key
AZURE_FLUX_ENDPOINT=https://your-resource.openai.azure.com/providers/blackforestlabs/v1/flux-2-pro?api-version=preview

# GPT Image  
AZURE_GPT_IMAGE_API_KEY=your_api_key
AZURE_GPT_IMAGE_ENDPOINT=https://your-resource.openai.azure.com/openai/v1/images/generations

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **New Features**
  * Multi-provider AI image generation: Added Azure FLUX and Azure GPT Image support alongside existing Gemini integration.
  * Image cropping modal with grid-based splitting and freeform drawing modes; extract, download, or export regions individually or as ZIP files.
  * Node Groups and Auto-Save features.

* **Documentation**
  * Enhanced setup guides with Azure provider configuration and endpoint instructions.
  * Added Supported Models and Image Generation reference tables.

* **Chores**
  * Added jszip dependency for ZIP export functionality.

<sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

- Implemented CropperModal with grid and freeform cropping modes.
- Integrated grid detection and image extraction features.
- Added state management using Zustand for cropper functionalities.
- Created utility functions for downloading images and handling ZIP downloads.
- Enhanced user experience with keyboard shortcuts and responsive design.
@coderabbitai
Copy link

coderabbitai bot commented Dec 19, 2025

Walkthrough

The pull request adds Azure AI image generation support (FLUX and GPT Image), introduces a comprehensive image cropping and grid-splitting modal, extends the data model with Azure-specific parameters, provides multi-format image download utilities, updates environment configuration examples, and significantly expands documentation with model tables and setup guidance.

Changes

Cohort / File(s) Summary
Configuration & Documentation
.env.local.example, CLAUDE.md, README.md
Added environment template with API keys for Gemini, OpenAI, and Azure services. Updated documentation with supported models tables (image and text generation), Azure setup instructions, endpoint configuration details, and expanded getting started guides with pnpm-based workflows.
Dependencies
package.json
Added jszip ^3.10.1 for ZIP file creation in image batch downloads.
Type System
src/types/index.ts
Extended ModelType to include azure-flux-pro and azure-gpt-image. Added new Azure-specific types: AzureFluxSize, AzureGptImageSize, AzureGptImageQuality. Extended NanoBananaNodeData and GenerateRequest interfaces with size, gptImageSize, and gptImageQuality fields.
State Management
src/store/cropperStore.ts, src/store/workflowStore.ts
Introduced new Zustand-based cropperStore managing modal state, grid detection, freeform cropping regions, and extraction results with support for both grid and freeform modes. Extended workflowStore to propagate new Azure image parameters through node data and generation payloads.
API Integration
src/app/api/generate/route.ts
Added Azure endpoint dispatching for FLUX and GPT Image models with fallback to Gemini. Integrated environment-based endpoint and key configuration. Added aspect ratio to size mapping for both Azure providers. Implemented handlers for Azure FLUX and GPT Image image extraction and response formatting with enhanced error handling.
Image Utilities
src/utils/downloadImage.ts
Created utility module supporting single/batch image downloads, ZIP packaging (with JSZip dynamic import), and configurable filename/format options.
UI Components - Image Operations
src/components/CropperModal.tsx
Introduced fully-featured interactive cropping component with two modes: grid-based splitting (with auto-detection and preset suggestions) and freeform region drawing. Supports keyboard interactions, region manipulation, and multiple export pathways (individual download, ZIP, workflow node creation, history logging).
Component Updates
src/components/GlobalImageHistory.tsx, src/components/nodes/ImageInputNode.tsx, src/components/nodes/OutputNode.tsx, src/components/nodes/NanoBananaNode.tsx, src/app/page.tsx
Integrated cropper modal and download capabilities into image workflows. Added hover overlays and context menus for quick actions. Extended NanoBananaNode with Azure model support, size/quality selectors, and crop/download buttons. Wired cropper and download handlers across history, input, output, and generation nodes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

  • src/components/CropperModal.tsx — Complex interactive component with grid detection logic, region drawing/manipulation state machine, canvas-based image extraction, and multiple export workflows; requires careful review of coordinate transforms, region normalization, and cross-origin image handling.
  • src/app/api/generate/route.ts — Azure endpoint integration with conditional routing, error handling across multiple providers, and parameter mapping (aspect ratio to size conversions); verify all fallback chains and Azure API contract compliance.
  • src/store/cropperStore.ts — New Zustand store with extraction helpers; verify canvas operations handle base64 and URL-based images, region geometry normalization (10x10 minimum), and state consistency across mode switches.
  • Integration points — Verify cropper modal wiring across NanoBananaNode, OutputNode, ImageInputNode, and GlobalImageHistory; check that state management (cropperStore, workflowStore) interactions don't cause unintended state pollution or re-renders.

Poem

🐰✨ From Azure clouds to grids so fine,
Cropping regions, one by one align!
ZIP them up and split with glee,
Image magic wild and free!
Configuration set, endpoints humming—
Feature-packed, our workflows are blooming! 🌱🎨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding Microsoft Foundry text-to-image support with two new models (FLUX.2 Pro and GPT Image 1.5), which is the primary feature across the entire changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/app/api/generate/route.ts (1)

280-281: requestId is shadowed in catch block, breaking error logging.

The outer requestId from line 42 is inaccessible in the catch block because a new requestId is declared on line 281. All error logs will show 'unknown' instead of the actual request ID.

🔎 Proposed fix
   } catch (error) {
-    const requestId = 'unknown'; // Fallback if we don't have it in scope
     console.error(`[API:${requestId}] ❌❌❌ EXCEPTION CAUGHT IN API ROUTE ❌❌❌`);

The outer requestId declared at line 42 is already in scope for the catch block - no need to redeclare it.

🧹 Nitpick comments (15)
.env.local.example (1)

1-30: Good template structure with clear documentation.

The environment variable template is well-organized with helpful section headers and guidance comments. Consider adding a trailing newline at the end of the file for POSIX compliance.

🔎 Suggested fix for trailing newline
 AZURE_GPT_IMAGE_API_KEY=your_azure_gpt_image_api_key_here
 AZURE_GPT_IMAGE_ENDPOINT=https://your-resource.openai.azure.com/openai/v1/images/generations
+
src/components/GlobalImageHistory.tsx (1)

242-244: Context menu may overflow viewport near screen edges.

The context menu is positioned at clientX/clientY without boundary checks. If a user right-clicks near the right or bottom edge, the menu could render partially off-screen.

🔎 Suggested viewport-aware positioning
+          const menuWidth = 160; // min-w-[160px]
+          const menuHeight = 80; // approximate height for 2 items
+          const adjustedX = Math.min(contextMenu.x, window.innerWidth - menuWidth - 8);
+          const adjustedY = Math.min(contextMenu.y, window.innerHeight - menuHeight - 8);
           <div
             className="fixed z-[301] bg-neutral-800 border border-neutral-700 rounded-lg shadow-xl py-1 min-w-[160px]"
-            style={{ left: contextMenu.x, top: contextMenu.y }}
+            style={{ left: adjustedX, top: adjustedY }}
           >
src/utils/downloadImage.ts (1)

64-68: Consider defensive handling for malformed data URLs.

If dataUrl doesn't contain a comma (malformed), split(",")[1] returns undefined, which could cause issues when adding to the ZIP.

🔎 Suggested defensive check
     // Add each image to the zip
     dataUrls.forEach((dataUrl, index) => {
       // Extract base64 data from data URL
-      const base64Data = dataUrl.split(",")[1];
-      zip.file(`image-${index + 1}.png`, base64Data, { base64: true });
+      const base64Data = dataUrl.split(",")[1];
+      if (base64Data) {
+        zip.file(`image-${index + 1}.png`, base64Data, { base64: true });
+      }
     });
README.md (1)

39-46: Consider updating the placeholder repository URL.

The git clone URL uses your-username as a placeholder. If this is the actual repository, consider updating to the correct URL for easier copy-paste.

🔎 Suggested fix
 # Clone the repository
-git clone https://github.com/your-username/node-banana.git
+git clone https://github.com/shrimbly/node-banana.git
 cd node-banana
src/app/api/generate/route.ts (5)

14-17: Hardcoded fallback URLs expose internal endpoint patterns.

The default fallback URLs reveal internal Azure endpoint structures. While these won't work without valid API keys, they leak naming conventions. Consider using empty strings or throwing a configuration error when endpoints are not set.

🔎 Proposed fix
-const AZURE_FLUX_ENDPOINT = process.env.AZURE_FLUX_ENDPOINT || "https://your-resource.openai.azure.com/providers/blackforestlabs/v1/flux-2-pro?api-version=preview";
-const AZURE_GPT_IMAGE_ENDPOINT = process.env.AZURE_GPT_IMAGE_ENDPOINT || "https://your-resource.openai.azure.com/openai/v1/images/generations";
+const AZURE_FLUX_ENDPOINT = process.env.AZURE_FLUX_ENDPOINT || "";
+const AZURE_GPT_IMAGE_ENDPOINT = process.env.AZURE_GPT_IMAGE_ENDPOINT || "";

409-421: External API call lacks timeout configuration.

The fetch to Azure FLUX endpoint has no timeout. If the Azure service hangs, this request will block until the 5-minute maxDuration expires. Consider using AbortController to set a reasonable timeout (e.g., 120 seconds).

🔎 Proposed fix
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), 120000); // 2 min timeout
+
     const response = await fetch(AZURE_FLUX_ENDPOINT, {
       method: "POST",
       headers: {
         "Content-Type": "application/json",
         "api-key": azureApiKey,
       },
       body: JSON.stringify({
         prompt,
         size: imageSize,
         n: 1,
         model: "FLUX.2-pro",
       }),
+      signal: controller.signal,
     });
+
+    clearTimeout(timeoutId);

464-476: Secondary fetch for URL-based response lacks timeout and validation.

When Azure returns a URL instead of base64, the code fetches from that URL without timeout or domain validation. Consider adding a timeout and optionally validating the URL is from an expected Azure domain.

🔎 Proposed fix
     if (data.data && data.data.length > 0 && data.data[0].url) {
       console.log(`[API:${requestId}] Found URL in response, fetching image...`);
       const imageUrl = data.data[0].url;
-      const imageResponse = await fetch(imageUrl);
+      
+      // Add timeout for image fetch
+      const imgController = new AbortController();
+      const imgTimeoutId = setTimeout(() => imgController.abort(), 30000);
+      const imageResponse = await fetch(imageUrl, { signal: imgController.signal });
+      clearTimeout(imgTimeoutId);
+      
       const imageBuffer = await imageResponse.arrayBuffer();

552-567: Azure GPT Image fetch also lacks timeout (same issue as FLUX handler).

Apply the same AbortController pattern here for consistency and reliability.


362-498: Consider extracting shared logic between Azure handlers.

Both handleAzureFluxGeneration and handleAzureGptImageGeneration share similar patterns: API key validation, prompt validation, fetch with error handling, and response parsing (b64_json vs URL). A helper function could reduce duplication, though current structure is acceptable for clarity.

Also applies to: 500-644

src/store/cropperStore.ts (3)

105-115: Minor: Set population can be simplified.

The for-loop to populate indices works, but Array.from or spread could be more concise.

🔎 Proposed fix
   setGridResult: (result) => {
     set({ gridResult: result });
     // Auto-select all cells when grid is set
     if (result) {
-      const allCells = new Set<number>();
-      for (let i = 0; i < result.cells.length; i++) {
-        allCells.add(i);
-      }
+      const allCells = new Set(Array.from({ length: result.cells.length }, (_, i) => i));
       set({ selectedCells: allCells });
     }
   },

150-155: ID collision risk with Date.now() on rapid interactions.

Using Date.now() for region IDs could theoretically collide if two regions are created within the same millisecond. Consider using crypto.randomUUID() for guaranteed uniqueness.

🔎 Proposed fix
   startRegion: (x, y) => {
-    const id = `region-${Date.now()}`;
+    const id = `region-${crypto.randomUUID()}`;
     set({
       currentRegion: { id, x, y, width: 0, height: 0 },
     });
   },

287-297: Sequential extraction is memory-safe but slower.

The sequential for...of loop processes regions one at a time. For better performance with many regions, consider Promise.all with optional chunking to balance speed and memory.

🔎 Proposed fix for parallel extraction
 export async function extractRegions(
   imageDataUrl: string,
   regions: (CropRegion | GridCell)[]
 ): Promise<string[]> {
-  const results: string[] = [];
-  for (const region of regions) {
-    const extracted = await extractRegion(imageDataUrl, region);
-    results.push(extracted);
-  }
-  return results;
+  return Promise.all(regions.map(region => extractRegion(imageDataUrl, region)));
 }
src/components/CropperModal.tsx (3)

392-398: Direct store mutation bypasses action pattern.

The onDragEnd handler directly calls useCropperStore.getState() and setState(), bypassing the store's action pattern. Consider adding an updateRegionPosition action to the store for consistency.

🔎 Proposed fix - add action to store

In cropperStore.ts, add:

updateRegionPosition: (id: string, x: number, y: number) => {
  set((state) => ({
    cropRegions: state.cropRegions.map((r) =>
      r.id === id ? { ...r, x, y } : r
    ),
  }));
},

Then in CropperModal.tsx:

           onDragEnd={(e) => {
-            const { cropRegions } = useCropperStore.getState();
-            const updated = cropRegions.map((r) =>
-              r.id === region.id ? { ...r, x: e.target.x(), y: e.target.y() } : r
-            );
-            useCropperStore.setState({ cropRegions: updated });
+            updateRegionPosition(region.id, e.target.x(), e.target.y());
           }}

633-636: Stage dimensions not reactive to container resize.

containerRef.current?.clientWidth is read once during render. If the window/container resizes, the Stage won't update. Consider adding a ResizeObserver to track container dimensions.

🔎 Proposed fix
// Add state for container dimensions
const [containerSize, setContainerSize] = useState({ width: 800, height: 600 });

// Add ResizeObserver effect
useEffect(() => {
  const container = containerRef.current;
  if (!container) return;
  
  const observer = new ResizeObserver((entries) => {
    const { width, height } = entries[0].contentRect;
    setContainerSize({ width, height });
  });
  
  observer.observe(container);
  return () => observer.disconnect();
}, []);

// Use in Stage
<Stage
  width={containerSize.width}
  height={containerSize.height}
  // ...
/>

426-427: Modal lacks accessibility attributes.

The modal overlay should have role="dialog", aria-modal="true", and aria-labelledby pointing to a title. Focus should be trapped within the modal when open.

🔎 Proposed fix
   return (
-    <div className="fixed inset-0 z-[100] bg-neutral-950 flex flex-col">
+    <div
+      className="fixed inset-0 z-[100] bg-neutral-950 flex flex-col"
+      role="dialog"
+      aria-modal="true"
+      aria-labelledby="cropper-modal-title"
+    >
       {/* Top Bar */}
       <div className="h-14 bg-neutral-900 flex items-center justify-between px-4 border-b border-neutral-800">
+        <h2 id="cropper-modal-title" className="sr-only">Image Cropper</h2>
         <div className="flex items-center gap-4">
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1f5c73 and 5363c26.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (15)
  • .env.local.example (1 hunks)
  • CLAUDE.md (1 hunks)
  • README.md (2 hunks)
  • package.json (1 hunks)
  • src/app/api/generate/route.ts (4 hunks)
  • src/app/page.tsx (2 hunks)
  • src/components/CropperModal.tsx (1 hunks)
  • src/components/GlobalImageHistory.tsx (8 hunks)
  • src/components/nodes/ImageInputNode.tsx (3 hunks)
  • src/components/nodes/NanoBananaNode.tsx (6 hunks)
  • src/components/nodes/OutputNode.tsx (2 hunks)
  • src/store/cropperStore.ts (1 hunks)
  • src/store/workflowStore.ts (3 hunks)
  • src/types/index.ts (3 hunks)
  • src/utils/downloadImage.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/app/page.tsx (1)
src/components/CropperModal.tsx (1)
  • CropperModal (34-753)
src/components/nodes/ImageInputNode.tsx (3)
src/types/index.ts (1)
  • ImageInputNodeData (49-53)
src/store/cropperStore.ts (1)
  • useCropperStore (79-232)
src/utils/downloadImage.ts (1)
  • downloadImage (15-30)
src/store/cropperStore.ts (1)
src/utils/gridSplitter.ts (2)
  • GridDetectionResult (18-23)
  • GridCell (11-16)
src/components/CropperModal.tsx (3)
src/store/cropperStore.ts (1)
  • extractRegions (287-297)
src/utils/gridSplitter.ts (5)
  • getGridCandidates (165-217)
  • detectGrid (51-84)
  • detectGridWithDimensions (121-141)
  • GridCell (11-16)
  • splitImage (403-454)
src/utils/downloadImage.ts (3)
  • downloadImages (36-48)
  • downloadImagesAsZip (54-87)
  • downloadImage (15-30)
src/components/GlobalImageHistory.tsx (3)
src/types/index.ts (1)
  • ImageHistoryItem (119-126)
src/store/cropperStore.ts (1)
  • useCropperStore (79-232)
src/utils/downloadImage.ts (1)
  • downloadImage (15-30)
src/app/api/generate/route.ts (1)
src/types/index.ts (5)
  • AzureFluxSize (22-22)
  • AzureGptImageSize (25-25)
  • GenerateRequest (187-197)
  • GenerateResponse (199-203)
  • AzureGptImageQuality (28-28)
🪛 dotenv-linter (4.0.0)
.env.local.example

[warning] 30-30: [EndingBlankLine] No blank line at the end of the file

(EndingBlankLine)

🪛 LanguageTool
README.md

[style] ~171-~171: Consider using a less common alternative to make your writing sound more unique and professional.
Context: ...ontributing Contributions are welcome! Please feel free to submit a Pull Request. ## License MIT...

(FEEL_FREE_TO_STYLE_ME)

🔇 Additional comments (34)
CLAUDE.md (1)

3-29: Well-structured documentation for the new model integrations.

The model table, environment variables, and API endpoint documentation are clear and comprehensive. The distinction between authentication methods (api-key header for FLUX vs Authorization: Bearer for GPT Image) is important and correctly documented.

src/app/page.tsx (1)

9-9: Clean integration of the CropperModal component.

The CropperModal is correctly placed as a sibling to the existing AnnotationModal, following the same store-driven modal pattern where visibility is controlled via useCropperStore.

Also applies to: 30-30

src/components/nodes/OutputNode.tsx (2)

6-7: Good refactoring with centralized download utility and cropper integration.

The changes correctly:

  • Replace manual download logic with the reusable downloadImage utility
  • Add cropper integration via useCropperStore.openModal
  • Include proper dependencies in useCallback hooks

Also applies to: 15-27


62-67: Consistent button styling for the new crop action.

The "Split Grid / Crop" button follows the existing button pattern with appropriate contrast (darker background) to differentiate it from the primary Download action.

src/components/nodes/ImageInputNode.tsx (2)

83-93: Well-implemented download and cropper handlers.

The handleDownload function smartly uses the original filename when available, falling back to a timestamped default. Both callbacks have correct dependency arrays.


112-141: Intuitive action overlay with clear visual hierarchy.

The hover-to-reveal action buttons provide a clean UX with appropriate color coding:

  • Blue for crop (creative action)
  • Green for download (positive action)
  • Red for remove (destructive action)

The opacity-0 group-hover:opacity-100 transition is smooth and doesn't interfere with the image preview.

src/components/nodes/NanoBananaNode.tsx (5)

17-46: Clean organization of Azure model configuration constants.

The size and quality options are well-structured with typed values and display labels. The MODELS array extension maintains consistency with existing model definitions.


83-118: Consistent handler implementations for Azure-specific options.

The size and quality change handlers follow the same pattern as existing handlers. The download and cropper handlers are consistent with other node implementations.


128-130: Good use of derived boolean flags for readability.

The isAzureFlux, isAzureGptImage, and isGeminiModel flags simplify the conditional rendering logic throughout the component.


198-215: Consistent action buttons in the image preview area.

The crop and download buttons follow the same styling pattern as the existing regenerate and clear buttons, maintaining visual consistency across the node UI.


314-357: Default values are consistent between component and store.

The inline defaults in the select elements match the store initialization in workflowStore.ts: size, gptImageSize, and gptImageQuality all use "1024x1024", "1024x1024", and "medium" respectively, which align with the createDefaultNodeData function for nanoBanana nodes.

package.json (1)

16-16: Appropriate dependency addition for ZIP export functionality.

jszip version 3.10.1 is the latest version, and the library is a well-established choice for creating and editing ZIP files with JavaScript. The dependency is correctly placed in dependencies for runtime usage. No direct vulnerabilities have been found for this version.

src/store/workflowStore.ts (3)

155-160: LGTM - Azure model defaults properly initialized.

The new Azure-specific fields (size, gptImageSize, gptImageQuality) are correctly initialized with sensible defaults that align with the type definitions in src/types/index.ts.


778-781: LGTM - Azure parameters correctly propagated to API.

The request payload properly includes the new Azure-specific fields, ensuring the API receives the necessary parameters for FLUX and GPT Image model configurations.


1015-1018: LGTM - Regeneration path consistent with execute path.

The regenerateNode function correctly includes the same Azure parameters as executeWorkflow, maintaining consistency across both execution paths.

src/types/index.ts (3)

19-28: LGTM - Well-structured Azure model types.

The new type definitions for Azure FLUX and GPT Image models are clean and well-documented. The separation of size types for each model (AzureFluxSize vs AzureGptImageSize) is appropriate since they have different valid options.


137-142: LGTM - Node data properly extended.

The NanoBananaNodeData interface correctly includes the new Azure-specific fields as required (non-optional), which aligns with the default initialization in workflowStore.ts.


194-197: LGTM - API request type correctly uses optional fields.

The GenerateRequest appropriately marks the Azure-specific fields as optional, allowing clients to omit them when using non-Azure models.

src/components/GlobalImageHistory.tsx (4)

6-7: LGTM - Clean integration of cropper and download utilities.

The new imports properly integrate the cropper store and download utility, enabling the new image actions from the history sidebar.


115-125: LGTM - Proper Escape key handling priority.

The updated handler correctly closes the context menu first before closing the sidebar, providing intuitive keyboard navigation.


199-219: LGTM - Well-designed hover overlay with quick actions.

The hover overlay implementation is clean with proper event propagation handling (e.stopPropagation()), accessible titles, and intuitive iconography.


363-373: LGTM - Download and cropper handlers properly implemented.

The handlers correctly use the imported utilities and manage UI state appropriately by closing both sidebar and fan views after opening the cropper.

src/utils/downloadImage.ts (3)

15-30: LGTM - Clean download implementation.

The anchor element approach is the standard pattern for programmatic downloads. Proper cleanup by removing the link after click.


36-48: LGTM - Staggered downloads to avoid browser blocking.

The 100ms stagger between downloads is a reasonable approach to prevent browsers from blocking multiple rapid download requests.


74-81: LGTM - Proper blob URL cleanup.

Good practice to call URL.revokeObjectURL(url) after the download to prevent memory leaks.

README.md (3)

67-76: LGTM - Clear Azure endpoint configuration examples.

The endpoint format examples with descriptive placeholders (your-resource) make it easy for users to understand what values to substitute. The note about only needing to configure keys for models in use is helpful.


99-105: LGTM - Comprehensive model documentation.

The supported models table with provider information helps users understand their options at a glance.


150-167: LGTM - Helpful Microsoft Foundry setup guide.

The step-by-step instructions for deploying Azure models are clear and actionable, with proper links and endpoint configuration examples.

src/store/cropperStore.ts (3)

63-77: LGTM!

Initial state is well-structured with appropriate defaults. Type assertions are correctly applied where needed.


82-99: LGTM!

State reset on openModal prevents stale data from previous sessions. The closeModal intentionally preserves state for potential re-open scenarios.


237-282: LGTM with minor note on canvas cleanup.

The canvas-based extraction is correct. Browser will garbage collect the created elements, but for heavy usage with large images, explicit cleanup (canvas.width = 0) could help release memory sooner.

src/components/CropperModal.tsx (3)

34-62: LGTM!

Clean destructuring from the store with all necessary state and actions. Good separation of concerns.


132-155: LGTM!

Keyboard shortcuts are properly implemented with cleanup. Dependencies array correctly includes all used values.


745-750: LGTM!

Click-outside-to-close pattern is correctly implemented. The overlay sits behind the modal but captures clicks outside the export menu.

Comment on lines +83 to +117
useEffect(() => {
if (sourceImage && isOpen) {
const img = new window.Image();
img.crossOrigin = "anonymous";
img.onload = () => {
setImage(img);
setImageDimensions({ width: img.width, height: img.height });

if (containerRef.current) {
const containerWidth = containerRef.current.clientWidth - 48;
const containerHeight = containerRef.current.clientHeight - 48;
const scaleX = containerWidth / img.width;
const scaleY = containerHeight / img.height;
const newScale = Math.min(scaleX, scaleY, 1);
setScale(newScale);
setStageSize({ width: img.width, height: img.height });
setPosition({
x: (containerWidth - img.width * newScale) / 2 + 24,
y: (containerHeight - img.height * newScale) / 2 + 24,
});
}

// Get grid candidates for suggestions
const candidates = getGridCandidates(img.width, img.height);
setSuggestedGrids(
candidates.slice(0, 5).map((c) => ({
rows: c.rows,
cols: c.cols,
score: c.score,
}))
);
};
img.src = sourceImage;
}
}, [sourceImage, isOpen, setImageDimensions]);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing cleanup in image loading effect.

If sourceImage or isOpen changes while an image is loading, the previous load continues and may call setImage with stale data. Add cleanup to prevent race conditions.

🔎 Proposed fix
   useEffect(() => {
     if (sourceImage && isOpen) {
+      let cancelled = false;
       const img = new window.Image();
       img.crossOrigin = "anonymous";
       img.onload = () => {
+        if (cancelled) return;
         setImage(img);
         setImageDimensions({ width: img.width, height: img.height });
         // ... rest of onload logic
       };
       img.src = sourceImage;
+      return () => {
+        cancelled = true;
+      };
     }
   }, [sourceImage, isOpen, setImageDimensions]);
🤖 Prompt for AI Agents
In src/components/CropperModal.tsx around lines 83 to 117, the useEffect that
creates and loads a new Image lacks cleanup so a previous image's onload can
fire after sourceImage/isOpen change causing stale state updates; fix by
capturing the created img in a local variable, set img.onload and img.onerror
handlers, and return a cleanup function that nulls those handlers (img.onload =
img.onerror = null) and clears img.src = '' (to abort loading) and optionally
skip state updates after cleanup via a local cancelled flag so
setImage/setImageDimensions/setScale/etc. only run when not cancelled.

Comment on lines +274 to +291
case "add-nodes":
// Create ImageInput nodes for each extracted image
imagesToExtract.forEach((img, index) => {
const nodeId = addNode("imageInput", {
x: 100 + index * 50,
y: 100 + index * 50,
});
// Load the image to get dimensions
const tempImg = new Image();
tempImg.onload = () => {
updateNodeData(nodeId, {
image: img,
filename: `cropped-${index + 1}.png`,
dimensions: { width: tempImg.width, height: tempImg.height },
});
};
tempImg.src = img;
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Image loading in forEach doesn't await, potential race condition.

When adding nodes, tempImg.onload is async but forEach continues immediately. If closeModal() is called before images load, updateNodeData may fail or update wrong state. Consider using Promise.all with proper image loading.

🔎 Proposed fix
       case "add-nodes":
         // Create ImageInput nodes for each extracted image
-        imagesToExtract.forEach((img, index) => {
-          const nodeId = addNode("imageInput", {
-            x: 100 + index * 50,
-            y: 100 + index * 50,
-          });
-          // Load the image to get dimensions
-          const tempImg = new Image();
-          tempImg.onload = () => {
-            updateNodeData(nodeId, {
-              image: img,
-              filename: `cropped-${index + 1}.png`,
-              dimensions: { width: tempImg.width, height: tempImg.height },
-            });
-          };
-          tempImg.src = img;
-        });
-        closeModal();
+        await Promise.all(
+          imagesToExtract.map((img, index) => {
+            return new Promise<void>((resolve) => {
+              const nodeId = addNode("imageInput", {
+                x: 100 + index * 50,
+                y: 100 + index * 50,
+              });
+              const tempImg = new Image();
+              tempImg.onload = () => {
+                updateNodeData(nodeId, {
+                  image: img,
+                  filename: `cropped-${index + 1}.png`,
+                  dimensions: { width: tempImg.width, height: tempImg.height },
+                });
+                resolve();
+              };
+              tempImg.onerror = () => resolve(); // Don't block on error
+              tempImg.src = img;
+            });
+          })
+        );
+        closeModal();
         break;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/components/CropperModal.tsx around lines 274 to 291, the code creates
Image objects inside a forEach and relies on their onload callbacks without
awaiting them, causing race conditions if the modal closes or state changes
before images finish loading; refactor to map imagesToExtract to an array of
Promises that create each Image, set onload to resolve with the loaded
dimensions (and onerror to reject), call addNode synchronously to get nodeId for
each image, then await Promise.all on those load promises and call
updateNodeData for each resolved result (or handle failures) before proceeding
(e.g., before closeModal()) so updates happen deterministically.

@thegovind
Copy link
Author

@shrimbly - Let me know how religiously you follow these PR Agents. Happy to address what makes sense to get this merged into main

@shrimbly
Copy link
Owner

Hey @thegovind - thanks for your contribution here. I missed the notification about your PR.

Seems like we have like minds, since I already made a few improvements that are similar to your work here.

  • Image cropper, I think this should be resolved now with the new image grid splitter node.
  • Download utils, I took a different approach where all generated images are automatically saved locally as they're generated.

Not sure we need these features now that there are equivalent ones in the repo. Let me know what you think

For Microsoft foundry, I'm only passingly familiar with that. Are there benefits of using foundry over something like Replicate, OpenRouter etc? I am planning to consolidate providers and support more models, but haven't made firm decisions yet.

FYI I'd prefer to stick with npm for this project over pnpm, just personal preference.

Thanks again for the contributions here!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants