[MohamedBassem]

No description provided.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T23:47:12.092502Z	46cb243	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[pullfrog]

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes

• Banner image optimization: new optimizeBannerImage (packages/shared-server/src/bannerImages.ts) downscales JPEG/PNG banners to WebP (max width 1200, quality 80, EXIF-rotated), returning null for gifs, webp, and multi-frame inputs so they're stored as-is.
• Crawler banner path: the banner download is split out of downloadAndStoreFile into downloadAndStoreBanner, with new fetchAsset/fetchImage helpers and storeDataUriAsset reduced to the pure decodeDataUriImage. Banners are now optimized in memory and stored via saveBufferAsset instead of the temp-file round trip.
• Dependency: adds sharp to apps/workers and @karakeep/shared-server, imported lazily. The lockfile already resolves sharp@0.35.5 (web/landing depend on it) and pnpm-workspace.yaml already allows its build script.

The data: handling removed from downloadAndStoreFile is safe: its only caller is handleAsAssetBookmark, whose URL is restricted to http/https by isAllowedBookmarkUrl, and banner data URIs are handled by downloadAndStoreBanner.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[greptile-apps]

Confidence Score: 4/5

[Medium impact] Adds image optimization for downloaded banner images.

The PR appears safe to merge, with non-blocking improvements to conversion fallback, cancellation checks, and output size.

Findings

1.  Conversion errors drop banners ▶
2.  Timed-out crawls leave files ▶
3.  Optimized banners can grow ▶

Fix with agent prompt

### Issue 1
apps/workers/workers/crawlerWorker.ts:2021-2022
`?? banner` only preserves the original when `optimizeBannerImage` returns `null`. If `sharp` throws while reading or converting a downloaded image, the outer catch drops the banner instead. Keep conversion best-effort: log the conversion error and save the original, while still letting crawl cancellation stop the job.

### Issue 2
apps/workers/workers/crawlerWorker.ts:2023-2028
A timeout during `optimizeBannerImage` does not stop conversion, so this code still writes the banner. The caller then throws before adding its database row, leaving an unused file in storage. Retries can repeat that write. Check `ctx.abortSignal` after conversion and before `saveBufferAsset`.

### Issue 3
packages/shared-server/src/bannerImages.ts:34
WebP at quality 80 is not always smaller, especially for small or heavily compressed PNGs. This always stores the converted version, so an optimization can increase storage and download size. Compare the buffer sizes and return `null` when conversion does not save bytes.

```suggestion
  if (optimized.byteLength >= image.byteLength) {
    return null;
  }
  return { image: optimized, contentType: ASSET_TYPES.IMAGE_WEBP };
```

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Downloads banner images into memory and converts JPEG/PNG banners to WebP, with a maximum width of 1200 pixels.

• Crawled JPEG and PNG banners are saved as smaller WebP images.

_{Reviews (1) · Last reviewed commit: "feat(workers): optimize downloaded banne..." · Reviewed by Greptile}

[greptile-apps]

Conversion errors drop banners

?? banner only preserves the original when optimizeBannerImage returns null. If sharp throws while reading or converting a downloaded image, the outer catch drops the banner instead. Keep conversion best-effort: log the conversion error and save the original, while still letting crawl cancellation stop the job.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/workers/workers/crawlerWorker.ts
Line: 2021-2022

Comment:
**Conversion errors drop banners**

`?? banner` only preserves the original when `optimizeBannerImage` returns `null`. If `sharp` throws while reading or converting a downloaded image, the outer catch drops the banner instead. Keep conversion best-effort: log the conversion error and save the original, while still letting crawl cancellation stop the job.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Timed-out crawls leave files

A timeout during optimizeBannerImage does not stop conversion, so this code still writes the banner. The caller then throws before adding its database row, leaving an unused file in storage. Retries can repeat that write. Check ctx.abortSignal after conversion and before saveBufferAsset.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/workers/workers/crawlerWorker.ts
Line: 2023-2028

Comment:
**Timed-out crawls leave files**

A timeout during `optimizeBannerImage` does not stop conversion, so this code still writes the banner. The caller then throws before adding its database row, leaving an unused file in storage. Retries can repeat that write. Check `ctx.abortSignal` after conversion and before `saveBufferAsset`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Optimized banners can grow

WebP at quality 80 is not always smaller, especially for small or heavily compressed PNGs. This always stores the converted version, so an optimization can increase storage and download size. Compare the buffer sizes and return null when conversion does not save bytes.

Suggested change

	return { image: optimized, contentType: ASSET_TYPES.IMAGE_WEBP };
	if (optimized.byteLength >= image.byteLength) {
	return null;
	}
	return { image: optimized, contentType: ASSET_TYPES.IMAGE_WEBP };

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/shared-server/src/bannerImages.ts
Line: 34

Comment:
**Optimized banners can grow**

WebP at quality 80 is not always smaller, especially for small or heavily compressed PNGs. This always stores the converted version, so an optimization can increase storage and download size. Compare the buffer sizes and return `null` when conversion does not save bytes.

```suggestion
  if (optimized.byteLength >= image.byteLength) {
    return null;
  }
  return { image: optimized, contentType: ASSET_TYPES.IMAGE_WEBP };
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 277fa0dd73

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Cap decoded pixels before optimizing untrusted banners

When banner caching is enabled, image comes from page-controlled metadata while fetchImage only caps compressed bytes; sharp(image) therefore relies on Sharp's roughly 268 MP default, so a compact decompression-bomb PNG can force hundreds of millions of pixels through native decoding and resizing. That work is not connected to the crawler's AbortSignal, so it can exhaust CPU/native memory and hold the crawler slot beyond the job timeout; initialize Sharp with a banner-sized limitInputPixels or reject excessive dimensions before conversion.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Fall back to the original when WebP conversion rejects

When a JPEG/PNG is valid for storage but Sharp cannot encode it—for example, a 100x20,000 PNG is left at that height by the width-only withoutEnlargement resize and exceeds WebP's 16,383-pixel dimension limit—this await rejects and the enclosing catch returns null instead of using banner. This makes a SingleFile data-URI banner disappear because its external imageUrl is deliberately not stored, whereas the previous path saved the original; catch optimization failures separately and retain the original buffer while still propagating aborts.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Stream banners that will not be transcoded

When a remote GIF or WebP (which optimizeBannerImage returns unchanged) approaches MAX_ASSET_SIZE_MB, this path retains all copied chunks and allocates another full buffer in Buffer.concat—about 100 MB at the default 50 MB limit, per concurrent crawl. The previous helper streamed these assets through a temporary file, so a large animated banner can now exhaust low-memory or multi-worker deployments for no optimization benefit; keep the streaming path for formats that bypass Sharp or impose a much smaller banner-specific memory cap.

Useful? React with 👍 / 👎.

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (277fa0d): the follow-up fix(workers): keep the original banner when optimizing fails or doesn't help commit.

• Optimization failures now fall back to the original: optimizeBannerImage wraps the sharp pipeline in try/catch, logs a warning, and returns null, so the call site's ?? banner stores the original bytes instead of dropping the banner entirely.
• Skips non-helpful WebP: the converted buffer is discarded when optimized.byteLength >= image.byteLength, preserving the smaller/original encoding.
• Abort observed before storing: downloadAndStoreBanner calls ctx.abortSignal.throwIfAborted() after optimization, so a crawl timeout that lands mid-conversion isn't silently swallowed between the fetch and saveBufferAsset.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the delta since the prior pullfrog review (19e526e): the follow-up fix(workers): cap decoded banner pixels and avoid copying banner chunks commit.

• Capped decoded banner pixels: optimizeBannerImage now builds sharp with limitInputPixels: BANNER_MAX_INPUT_PIXELS (50M), so a small compressed image that would decode into a huge pixel count fails fast and the original bytes are stored instead — a decompression-bomb guard with no data-loss downside.
• Avoided a redundant buffer copy: fetchImage pushes stream chunks through directly (typeof chunk === "string" ? Buffer.from(chunk) : chunk) instead of always copying, and Buffer.concat still copies on the final assembly, so the stored bytes are unchanged.

The prior inline concern (returning null rather than throwing on sharp failure) remains correctly implemented from 19e526e, and the new pixel cap routes through that same fallback.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes

Reviewed the delta since the prior pullfrog review (63468ba): the follow-up fix(workers): buffer banners in a resizable ArrayBuffer commit.

• Banner download now buffers in place: fetchImage grows a single resizable ArrayBuffer (maxByteLength = maxAssetSizeMb) while streaming and returns a Buffer view sharing that store, replacing the chunk-array + Buffer.concat approach that held the banner twice.
• Filesystem asset store writes the buffer directly: LocalFileSystemAssetStore.saveAsset drops the Uint8Array.from(asset) copy and hands the Buffer straight to writeFile (byte-identical output).

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

chunk is already a Uint8Array (the response body is a web ReadableStream), and Uint8Array#set accepts any ArrayLike<number>, so bytes.set(chunk, offset) writes it directly. The Buffer.from(chunk) in between allocates and copies every chunk — the extra copy this commit set out to eliminate.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46cb243055

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid cloning every downloaded banner chunk

When a remote banner is large, the new resizable-buffer path still calls Buffer.from(chunk), but fetchWithProxy uses node-fetch, whose binary response stream already yields Buffers. This allocates a second backing store for every chunk before Uint8Array#set copies it into the growing buffer; those temporary external allocations can accumulate between GCs and add a full download's worth of allocation churn, undermining the memory reduction for near-MAX_ASSET_SIZE_MB banners and concurrent crawls. Set the fetched buffer directly, converting only an actual string chunk.

Useful? React with 👍 / 👎.