Skip to content

fix(openrouter-images): modalities 404, extractImages, SVG/WebP ext, …#44

Open
janboshoff75 wants to merge 3 commits into
OpenRouterTeam:mainfrom
janboshoff75:fix/openrouter-images-recraft
Open

fix(openrouter-images): modalities 404, extractImages, SVG/WebP ext, …#44
janboshoff75 wants to merge 3 commits into
OpenRouterTeam:mainfrom
janboshoff75:fix/openrouter-images-recraft

Conversation

@janboshoff75
Copy link
Copy Markdown

…rgb_colors flags, retry

  • postChatCompletion: retry up to 3× on 429/5xx with 2 s base delay
  • extractImages(): handles 4 response shapes (message.images, Responses API output[], data[], content[] image_url parts)
  • saveImage(): async, fetches HTTP/HTTPS URLs, derives file extension from MIME type (SVG, WebP, PNG, JPG, GIF)
  • defaultOutputPath(): returns stem without extension so saveImage can append the correct one
  • pathWithMimeExt(): helper to replace/add MIME-derived extension
  • parseRgbTriplet / parseRgbColors: parse "r,g,b" and "r,g,b;r,g,b"
  • generate.ts + edit.ts: add --rgb-colors, --background-rgb-color, --strength flags; conditionally send modalities only for google/* models (fixes 404 on Recraft and other providers)

…rgb_colors flags, retry

- postChatCompletion: retry up to 3× on 429/5xx with 2 s base delay
- extractImages(): handles 4 response shapes (message.images, Responses
  API output[], data[], content[] image_url parts)
- saveImage(): async, fetches HTTP/HTTPS URLs, derives file extension
  from MIME type (SVG, WebP, PNG, JPG, GIF)
- defaultOutputPath(): returns stem without extension so saveImage can
  append the correct one
- pathWithMimeExt(): helper to replace/add MIME-derived extension
- parseRgbTriplet / parseRgbColors: parse "r,g,b" and "r,g,b;r,g,b"
- generate.ts + edit.ts: add --rgb-colors, --background-rgb-color,
  --strength flags; conditionally send modalities only for google/*
  models (fixes 404 on Recraft and other providers)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Copy link
Copy Markdown

@perry-the-pr-reviewer perry-the-pr-reviewer Bot left a comment

Choose a reason for hiding this comment

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

Perry's Review

Adds retry logic, multi-shape image extraction, async saveImage with HTTP download, and new RGB and strength flags for Recraft — solid improvements overall, but the path-stem extraction has a correctness bug present in all three changed files.

Verdict: 🔁 Needs changes

Details

CI: perry/review in_progress (no other checks present)

Findings (see inline comments for full context):

  • 🔴 lib.ts:156pathWithMimeExt splits on dots in .. directory segments, outputs wrong path
  • 🔴 generate.ts:85 — same lastIndexOf(".") bug in multi-image output path loop
  • 🔴 edit.ts:98 — same lastIndexOf(".") bug in multi-image output path loop
  • 🟡 lib.ts:59 — retry logic ignores Retry-After header on 429; OpenRouter sends this header and fixed 2s delay may under-wait
  • nit: SKILL.md docs still show the old --output default and do not mention that the extension is now MIME-derived and will override any caller-supplied suffix

Codex: skipped (medium tier)

Research: RFC 9110 §10.2.3 and RFC 6585 — 429 responses SHOULD include Retry-After. OpenRouter rate-limit docs confirm the header is returned. Node.js path.extname() docs — the function considers only the last path segment's basename and correctly returns "" for paths like the .. case; this is already imported in lib.ts but unused in pathWithMimeExt.

Test coverage: no tests for the new library functions; extractImages, parseRgbTriplet, parseRgbColors, and pathWithMimeExt are all pure functions that would be easy to unit test — the path bug in particular would have been caught by a test using a relative parent-directory path as input.

Unresolved threads: none

Tier: medium (271 LoC)

Comment thread skills/openrouter-images/scripts/lib.ts Outdated
Comment thread skills/openrouter-images/scripts/generate.ts Outdated
Comment thread skills/openrouter-images/scripts/edit.ts Outdated
Comment thread skills/openrouter-images/scripts/lib.ts
- pathWithMimeExt: use extname() from node:path instead of
  lastIndexOf('.') so dots in parent dir segments (e.g. '..') are
  not mistaken for file extensions
- generate.ts, edit.ts multi-image loop: same extname() fix so
  '../images/out' produces '../images/out-1' not '.-1'
- postChatCompletion: honour Retry-After header on 429 responses;
  fall back to BASE_DELAY_MS * attempt when header absent

Co-Authored-By: Paperclip <noreply@paperclip.ing>
…, clarify MIME-ext behavior

- Change --output default from image-YYYYMMDD-HHmmss.png to stem only;
  extension is now auto-derived from model MIME type (fixes outdated docs)
- Add --rgb-colors, --background-rgb-color, --strength flags to options
  tables for both generate.ts and edit.ts (Recraft models)
- Add "Output extension" note explaining MIME-type derivation behavior
- Correct API Response Shapes section: uses chat/completions not /responses;
  document all four extractImages() response shapes
- Add example CLI invocations using the new flags

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@perry-the-pr-reviewer perry-the-pr-reviewer Bot dismissed their stale review May 29, 2026 22:37

Superseded by updated Perry review

Copy link
Copy Markdown
Contributor

@perry-the-pr-maintainer perry-the-pr-maintainer Bot left a comment

Choose a reason for hiding this comment

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

Perry's Review

Fixes multi-model image generation in the openrouter-images skill: adds Recraft support (rgb-colors, strength flags), corrects modalities routing (Google-only), unifies response extraction across four API shapes, and derives file extensions from MIME type rather than hard-coded PNG.

Verdict: ✅ LGTM

Details

CI: no checks besides perry/review — ✅

Findings (see inline comments for full context):

  • 🟡 lib.ts:110b64_json responses always assumed PNG; non-PNG bytes saved with wrong extension (suggestion)
  • nit: switch-case 429 message says "wait and try again" but then calls process.exit(1) on the final attempt — slightly misleading on exhausted retries. Low impact.

Codex (HEAVY_SECONDARY_MODEL): skipped (medium tier)

Research: skipped (medium tier)

Security: no concerns — API key handling unchanged; debug logging on no-image path is stderr-only, acceptable for a CLI tool. No secrets committed.

Test coverage: no tests exist for this skill; none added. The PR is a bug-fix and feature addition for a CLI tool — absence of tests is pre-existing.

Unresolved threads: 3 outdated (prior blockers fixed), 1 active (Retry-After suggestion — implemented in this PR).

Scope: first review (full)
Tier: medium (321 LoC)

if (Array.isArray(json.data) && json.data.length > 0) {
return json.data
.map((d: any) => {
if (d.b64_json) return `data:image/png;base64,${d.b64_json}`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[suggestion] lib.ts:110b64_json responses always get data:image/png prefix, so files with non-PNG content are saved with .png extension

Suggested change
if (d.b64_json) return `data:image/png;base64,${d.b64_json}`;
if (d.b64_json) return `data:image/png;base64,${d.b64_json}`; // DALL-E/OpenRouter-native b64_json is always PNG
Details

Why: saveImage derives the file extension from the MIME type in the data: URL. By hardcoding data:image/png;base64,..., any model returning WebP or JPEG via b64_json will produce a file with .png extension even though the bytes are a different format. DALL-E currently returns PNG so this is benign today, but it is a silent assumption worth documenting.

Fix: The suggestion above adds a comment to make the assumption explicit. Alternatively, if OpenRouter ever surfaces a MIME hint alongside b64_json, use it: data:${d.mime ?? "image/png"};base64,....

Reviewed at fb1d9b0

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