Add pencil sketch image transformation tool#6
Open
ASP-31 wants to merge 11 commits into
Open
Conversation
|
@ASP-31 is attempting to deploy a commit to the Azmil's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a new "Pencil Sketch" feature: a backend Express POST /api/images/sketch route that accepts multipart uploads and returns a sketchified PNG via Sharp, and a frontend tool page for upload, preview, processing, and download. The router is mounted at /api/images (registered after server listen). Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Client as Browser/React
participant Backend as Express Route
participant Sharp as Sharp Library
User->>Client: Select & upload image
Client->>Client: Show local preview
User->>Client: Click "Apply Sketch"
Client->>Backend: POST /api/images/sketch (multipart/form-data)
activate Backend
Backend->>Backend: multer in-memory validation
Backend->>Sharp: Run pipeline (grayscale, blur, invert, blend, normalize, sharpen)
Sharp-->>Backend: PNG buffer
Backend->>Client: 200 OK (image/png blob)
deactivate Backend
Client->>Client: Display result and enable "Download"
User->>Client: Click "Download"
Client->>User: Save pencil-sketch.png
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/app/tools/sketch/page.tsx (1)
15-17: Revoke stale object URLs.Both preview and result URLs are leaked right now. After a few large images, this page will keep extra blob data alive until refresh.
Suggested fix
"use client"; -import { useState } from "react"; +import { useEffect, useState } from "react"; @@ const [preview, setPreview] = useState<string | null>(null); const [result, setResult] = useState<string | null>(null); const [loading, setLoading] = useState(false); + + useEffect(() => { + return () => { + if (preview) URL.revokeObjectURL(preview); + if (result) URL.revokeObjectURL(result); + }; + }, [preview, result]); @@ - setPreview(URL.createObjectURL(uploaded)); + setPreview((prev) => { + if (prev) URL.revokeObjectURL(prev); + return URL.createObjectURL(uploaded); + }); setResult(null);Also applies to: 33-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/tools/sketch/page.tsx` around lines 15 - 17, The preview/result object URLs are leaking because URL.createObjectURL(uploaded) is called without revoking previous blobs; update the logic around setFile/setPreview/setResult so you revoke any existing preview and result URLs before replacing them (use URL.revokeObjectURL(previousPreview) and URL.revokeObjectURL(previousResult>) and also revoke in the component cleanup (useEffect return) to ensure no stale blob URLs remain; locate the code paths that call setPreview(URL.createObjectURL(...)) and the similar URLs at the later block (the lines that setPreview/setResult around the upload handling) and add revocation before assigning and on unmount.backend/src/server.ts (1)
3-8: Register this router inbackend/src/app.tsinstead.Right now
/api/images/sketchonly exists when the process boots throughbackend/src/server.ts; anything importingappdirectly gets a different app shape. Keeping route mounts in one place avoids that split.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/server.ts` around lines 3 - 8, The router mount for imageSketch is currently in server boot code (app.use("/api/images", imageSketch) next to app.listen) which causes different app shapes for consumers; remove that mount from the server file and instead import and register the router inside the module that constructs and exports the Express app (where the Express "app" is created, e.g., in app.ts), by adding import imageSketch and app.use("/api/images", imageSketch) there before exporting the app so any module importing the app sees the same routes; leave app.listen only in server code that imports the already-configured app.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/routes/imageSketch.ts`:
- Around line 6-8: The route creates a local Multer instance (upload = multer({
storage: multer.memoryStorage() })) which omits the 5MB cap and can exhaust
process memory; update the POST /sketch handler to use the shared upload
middleware from upload.middleware.ts or recreate the Multer instance with
limits.fileSize set (e.g., 5 * 1024 * 1024) and keep upload.single("image") in
router.post("/sketch", ...), so memory-backed uploads are capped before Sharp
runs.
In `@frontend/app/tools/sketch/page.tsx`:
- Around line 20-37: applySketch currently POSTs to a hardcoded
"http://localhost:5000/api/images/sketch" and treats any response as an image
blob; update applySketch to use a configurable API base (e.g., read from an
env/client config) instead of the hardcoded localhost, immediately check res.ok
after fetch and throw or handle non-OK responses so errors don't return blobs,
ensure setLoading(false) is called in all exit paths (on success, non-OK
responses, and network exceptions), and surface or log the error before
returning so callers can react; key symbols to change: applySketch, fetch call,
res.ok check, setLoading, setResult.
---
Nitpick comments:
In `@backend/src/server.ts`:
- Around line 3-8: The router mount for imageSketch is currently in server boot
code (app.use("/api/images", imageSketch) next to app.listen) which causes
different app shapes for consumers; remove that mount from the server file and
instead import and register the router inside the module that constructs and
exports the Express app (where the Express "app" is created, e.g., in app.ts),
by adding import imageSketch and app.use("/api/images", imageSketch) there
before exporting the app so any module importing the app sees the same routes;
leave app.listen only in server code that imports the already-configured app.
In `@frontend/app/tools/sketch/page.tsx`:
- Around line 15-17: The preview/result object URLs are leaking because
URL.createObjectURL(uploaded) is called without revoking previous blobs; update
the logic around setFile/setPreview/setResult so you revoke any existing preview
and result URLs before replacing them (use URL.revokeObjectURL(previousPreview)
and URL.revokeObjectURL(previousResult>) and also revoke in the component
cleanup (useEffect return) to ensure no stale blob URLs remain; locate the code
paths that call setPreview(URL.createObjectURL(...)) and the similar URLs at the
later block (the lines that setPreview/setResult around the upload handling) and
add revocation before assigning and on unmount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d98830c0-943f-44e6-80ad-81842a31e50d
⛔ Files ignored due to path filters (3)
backend/package-lock.jsonis excluded by!**/package-lock.jsonfrontend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
backend/src/routes/imageSketch.tsbackend/src/server.tsfrontend/app/tools/page.tsxfrontend/app/tools/sketch/page.tsx
Comment on lines
+6
to
+8
| const upload = multer({ storage: multer.memoryStorage() }); | ||
|
|
||
| router.post("/sketch", upload.single("image"), async (req, res) => { |
There was a problem hiding this comment.
Restore the upload size limit on this memory-backed route.
This local Multer instance drops the 5 MB cap used elsewhere, so one oversized upload can blow up process memory before Sharp ever runs. Reuse the shared middleware from backend/src/middleware/upload.middleware.ts or add an explicit limits.fileSize here.
Suggested fix
import express from "express";
-import multer from "multer";
import sharp from "sharp";
+import { upload } from "../middleware/upload.middleware";
const router = express.Router();
-const upload = multer({ storage: multer.memoryStorage() });
router.post("/sketch", upload.single("image"), async (req, res) => {📝 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 upload = multer({ storage: multer.memoryStorage() }); | |
| router.post("/sketch", upload.single("image"), async (req, res) => { | |
| import express from "express"; | |
| import sharp from "sharp"; | |
| import { upload } from "../middleware/upload.middleware"; | |
| const router = express.Router(); | |
| router.post("/sketch", upload.single("image"), async (req, res) => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/routes/imageSketch.ts` around lines 6 - 8, The route creates a
local Multer instance (upload = multer({ storage: multer.memoryStorage() }))
which omits the 5MB cap and can exhaust process memory; update the POST /sketch
handler to use the shared upload middleware from upload.middleware.ts or
recreate the Multer instance with limits.fileSize set (e.g., 5 * 1024 * 1024)
and keep upload.single("image") in router.post("/sketch", ...), so memory-backed
uploads are capped before Sharp runs.
Comment on lines
+20
to
+37
| const applySketch = async () => { | ||
| if (!file) return; | ||
|
|
||
| setLoading(true); | ||
|
|
||
| const formData = new FormData(); | ||
| formData.append("image", file); | ||
|
|
||
| const res = await fetch("http://localhost:5000/api/images/sketch", { | ||
| method: "POST", | ||
| body: formData | ||
| }); | ||
|
|
||
| const blob = await res.blob(); | ||
| const url = URL.createObjectURL(blob); | ||
|
|
||
| setResult(url); | ||
| setLoading(false); |
There was a problem hiding this comment.
Make the API target configurable and fail fast on non-image responses.
This fetch is hardcoded to the browser's localhost, so the feature breaks outside local dev. It also treats 4xx/5xx payloads as successful image blobs and can leave loading stuck on network failures.
Suggested fix
export default function SketchTool() {
+ const API_URL =
+ process.env.NEXT_PUBLIC_API_URL || "http://localhost:5000";
const [file, setFile] = useState<File | null>(null);
const [preview, setPreview] = useState<string | null>(null);
const [result, setResult] = useState<string | null>(null);
const [loading, setLoading] = useState(false);
@@
const applySketch = async () => {
if (!file) return;
setLoading(true);
const formData = new FormData();
formData.append("image", file);
- const res = await fetch("http://localhost:5000/api/images/sketch", {
- method: "POST",
- body: formData
- });
-
- const blob = await res.blob();
- const url = URL.createObjectURL(blob);
-
- setResult(url);
- setLoading(false);
+ try {
+ const res = await fetch(`${API_URL}/api/images/sketch`, {
+ method: "POST",
+ body: formData,
+ });
+
+ if (!res.ok) {
+ throw new Error(await res.text());
+ }
+
+ const blob = await res.blob();
+ setResult((prev) => {
+ if (prev) URL.revokeObjectURL(prev);
+ return URL.createObjectURL(blob);
+ });
+ } finally {
+ setLoading(false);
+ }
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/app/tools/sketch/page.tsx` around lines 20 - 37, applySketch
currently POSTs to a hardcoded "http://localhost:5000/api/images/sketch" and
treats any response as an image blob; update applySketch to use a configurable
API base (e.g., read from an env/client config) instead of the hardcoded
localhost, immediately check res.ok after fetch and throw or handle non-OK
responses so errors don't return blobs, ensure setLoading(false) is called in
all exit paths (on success, non-OK responses, and network exceptions), and
surface or log the error before returning so callers can react; key symbols to
change: applySketch, fetch call, res.ok check, setLoading, setResult.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/app/tools/page.tsx (1)
84-87: Use a sketch-specific icon instead of QrCode for the Pencil Sketch card.Lines 77 and 84 both use the
QrCodeicon, but "Pencil Sketch" needs visual distinction from "Image → QR" to be scannable in the grid. Replace withPencil(available in lucide-react):Suggested change
import { Crop, Image as ImageIcon, ImageOff, Layers, Maximize2, - Minimize2, Palette, QrCode, + Minimize2, Palette, Pencil, QrCode, ShieldOff, SlidersHorizontal, Type, Wand2 } from "lucide-react"; @@ { - icon: <QrCode className="w-6 h-6" />, + icon: <Pencil className="w-6 h-6" />, color: "rgba(163,163,163,1)", label: "Pencil Sketch", href: "/tools/sketch" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/tools/page.tsx` around lines 84 - 87, Replace the QrCode icon used for the card whose label is "Pencil Sketch" by swapping the icon reference in its object: change icon: <QrCode .../> to icon: <Pencil className="w-6 h-6" /> and add Pencil to the lucide-react imports at the top of the file (keeping the existing className and props unchanged) so the "Pencil Sketch" card uses the Pencil icon instead of QrCode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/app/tools/page.tsx`:
- Around line 84-87: Replace the QrCode icon used for the card whose label is
"Pencil Sketch" by swapping the icon reference in its object: change icon:
<QrCode .../> to icon: <Pencil className="w-6 h-6" /> and add Pencil to the
lucide-react imports at the top of the file (keeping the existing className and
props unchanged) so the "Pencil Sketch" card uses the Pencil icon instead of
QrCode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4d67bae-65c7-4826-955e-5fb1c2e3bb20
📒 Files selected for processing (1)
frontend/app/tools/page.tsx
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
2 participants
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 by CodeRabbit