Skip to content

Code Review and Debugging Session#2

Merged
improdead merged 8 commits intomainfrom
claude/work-in-progress-013mdrk378SdVQKANMYL3Kp8
Nov 15, 2025
Merged

Code Review and Debugging Session#2
improdead merged 8 commits intomainfrom
claude/work-in-progress-013mdrk378SdVQKANMYL3Kp8

Conversation

@improdead
Copy link
Owner

@improdead improdead commented Nov 15, 2025

PR Type

Enhancement, Bug fix


Description

  • Add OpenAI support alongside Gemini with provider selection via environment variables

  • Implement dual-provider architecture for story planning and image generation

  • Add @Injectable() decorators to NestJS services for proper dependency injection

  • Downgrade React from 19.1.1 to 18.3.1 for stability and disable strict mode

  • Add Playwright testing infrastructure with browser configuration and test suites

  • Improve error handling with proper type checking for Error instances

  • Add comprehensive documentation covering architecture, setup, and usage

  • Fix EventSource cleanup in React components to prevent memory leaks


Diagram Walkthrough

flowchart LR
  A["Story Input"] --> B["Planner Service"]
  B --> C{"Provider Selection"}
  C -->|OpenAI| D["GPT-5-Mini"]
  C -->|Gemini| E["Gemini 2.5 Flash"]
  D --> F["Episode Outline"]
  E --> F
  F --> G["Renderer Service"]
  G --> H{"Provider Selection"}
  H -->|OpenAI| I["GPT-Image-1"]
  H -->|Gemini| J["Gemini Image Preview"]
  I --> K["Manga Pages"]
  J --> K
  K --> L["Storage & Database"]
Loading

File Walkthrough

Relevant files
Enhancement
2 files
planner.service.ts
Add OpenAI GPT-5-Mini support with dual-provider routing 
+128/-7 
renderer.service.ts
Add OpenAI GPT-Image-1 support with provider routing         
+152/-13
Configuration changes
2 files
config.ts
Update renderer config for dual-provider support                 
+6/-2     
next.config.js
Disable React strict mode for stability                                   
+1/-1     
Bug fix
8 files
episodes.service.ts
Add @Injectable decorator and fix renderer model selection
+7/-5     
events.service.ts
Add @Injectable decorator to EventsService                             
+1/-0     
tts.service.ts
Add @Injectable decorator and improve error handling         
+5/-4     
prisma.service.ts
Add @Injectable decorator to PrismaService                             
+1/-0     
queue.service.ts
Add @Injectable decorator to QueueService                               
+1/-0     
storage.service.ts
Add @Injectable decorator to StorageService                           
+1/-0     
index.tsx
Fix EventSource cleanup and update UI badges to OpenAI     
+37/-6   
[id].tsx
Fix EventSource cleanup and prevent memory leaks                 
+22/-3   
Error handling
1 files
pages.controller.ts
Improve error handling with proper Error type checking     
+1/-1     
Documentation
2 files
.env.example
Document OpenAI and Gemini provider configuration options
+21/-4   
DOCUMENTATION.md
Add comprehensive documentation for architecture and setup
+859/-0 
Dependencies
2 files
package.json
Add OpenAI SDK and multer types dependencies                         
+2/-0     
package.json
Downgrade React to 18.3.1 and add Playwright testing         
+5/-3     
Tests
4 files
playwright.config.ts
Add Playwright test configuration with Chrome browser       
+39/-0   
app.spec.ts
Add Playwright test suite for application debugging           
+83/-0   
debug.spec.ts
Add error capture test for debugging browser crashes         
+67/-0   
firefox-debug.spec.ts
Add Firefox browser debugging test                                             
+33/-0   

Summary by CodeRabbit

  • New Features

    • Option to use either OpenAI or Gemini for planning and image/character generation.
    • End-to-end test setup with sample Playwright tests.
  • Bug Fixes

    • Hardened error messages across services.
    • Improved SSE/EventSource lifecycle with cleanup and timeouts to avoid stuck streams.
  • Documentation

    • Added comprehensive DOCUMENTATION covering architecture, setup, and usage.
  • Chores

    • Updated dependencies, package/config versions, and .gitignore entries.

- React 19 is not fully stable and was causing browser crashes
- Next.js 15.5.2 officially supports React 18
- Disabled React strict mode temporarily during debugging
- Added Playwright test configuration for automated testing

This resolves compatibility issues between React 19 and Next.js 15.
- Added playwright.config.ts with browser configuration
- Created test suite for debugging application issues
- Updated .gitignore to exclude .next build artifacts and test results
Major Changes:
- Added OpenAI SDK integration alongside existing Gemini support
- Story Planning: Now uses GPT-4o (default) or Gemini 2.5 Flash
- Image Generation: Now uses DALL-E 3 (default) or Gemini Image Preview
- Provider selection via environment variables (PLANNER_PROVIDER, RENDERER_PROVIDER)

Backend Updates:
- renderer.service.ts: Added OpenAI DALL-E 3 integration with dual-provider support
- planner.service.ts: Added OpenAI GPT-4o integration with dual-provider support
- config.ts: Updated to support both OpenAI and Gemini providers
- .env.example: Comprehensive documentation for both AI providers

Frontend Updates:
- index.tsx: Updated UI to show "GPT-4o" and "DALL-E 3" badges
- Replaced references to "Gemini 2.5" and "Nano Banana" with OpenAI equivalents

Environment Variables:
- OPENAI_API_KEY: OpenAI API key
- OPENAI_PLANNER_MODEL: Story planning model (default: gpt-4o)
- OPENAI_IMAGE_MODEL: Image generation model (default: dall-e-3)
- PLANNER_PROVIDER: 'openai' or 'gemini' (default: openai)
- RENDERER_PROVIDER: 'openai' or 'gemini' (default: openai)

The system now defaults to OpenAI but maintains full backward compatibility with Gemini.
…08-07

- Changed image generation model from dall-e-3 to gpt-image-1
- Changed story planning model from gpt-4o to gpt-5-mini-2025-08-07
- Updated .env.example with correct model names
- Updated frontend badges to show 'GPT-5 Mini' and 'GPT-Image-1'
- Updated renderer and planner service default configurations
- Complete system architecture overview
- Detailed explanation of how MangaFusion works end-to-end
- AI models configuration and usage (OpenAI GPT-5-Mini, GPT-Image-1)
- Setup and installation instructions
- Configuration reference for all environment variables
- Usage guide with step-by-step workflow
- Complete API reference
- Technical implementation details
- Troubleshooting guide
- Development notes and project structure

This documentation covers everything from setup to production deployment.
- Fix EventSource memory leaks in pages/index.tsx and pages/episodes/[id].tsx
  - Added proper cleanup with useRef pattern
  - Close EventSource on component unmount and error events
  - Prevent multiple simultaneous EventSource connections

- Add missing @Injectable decorators to all 8 backend services
  - PlannerService, RendererService, StorageService, TTSService
  - EventsService, QueueService, PrismaService, EpisodesService
  - Enables proper dependency injection in NestJS

- Fix TypeScript build errors in backend
  - Install missing @types/multer package
  - Fix renderer.imageModel references (use provider-specific model)
  - Add explicit type annotations for Prisma query results
  - Fix null vs undefined in PanelDialogue character field
  - Add proper error type checking (unknown to Error)
  - Add optional chaining for response.data array access

All backend TypeScript errors resolved - build now succeeds.
@coderabbitai
Copy link

coderabbitai bot commented Nov 15, 2025

Walkthrough

Adds multi-provider AI support (OpenAI + Gemini) for planning and rendering, introduces Playwright E2E tests and config, hardens SSE/error handling and filename sanitization, applies NestJS @Injectable() to multiple services, updates renderer config and backend dependencies, adds DOCUMENTATION.md and .gitignore entries. No public API signature changes.

Changes

Cohort / File(s) Summary
Repository config
\.gitignore, next.config.js
Ignore .next, test-results, playwright-report; set reactStrictMode to false.
Root packages
package.json
Downgrade react/react-dom to 18.3.1, add Playwright dev deps, move prisma to devDependencies.
Documentation
DOCUMENTATION.md
New comprehensive project documentation covering architecture, AI flows, setup, API, testing, and deployment.
Backend env & deps
backend/.env.example, backend/package.json
Add planner/renderer provider/model defaults; add openai SDK and @types/multer.
Planner service
backend/src/planner/planner.service.ts
Add provider-based routing: generateOutlineOpenAI + generateOutlineGemini, separate clients, JSON extraction helper, and API key gating.
Renderer config & service
backend/src/renderer/config.ts, backend/src/renderer/renderer.service.ts
Replace single imageModel with provider/openaiModel/geminiModel; runtime routing between OpenAI/Gemini for page and character generation, new client getters, API key fields, filename sanitization, storage upload and fallback handling.
Episodes adjustments
backend/src/episodes/episodes.service.ts
Derive and persist rendererModel from renderer config for DB and in-memory episode objects; minor typing and stub outline tweaks.
DI decorators added
backend/src/events/events.service.ts, backend/src/prisma/prisma.service.ts, backend/src/queue/queue.service.ts, backend/src/storage/storage.service.ts, backend/src/tts/tts.service.ts
Added @Injectable() to services; TTS service also hardens error message formatting.
Pages controller
backend/src/pages/pages.controller.ts
Safer error handling in catch blocks using instanceof Error before accessing error.message.
Frontend SSE lifecycle & UI text
pages/index.tsx, pages/episodes/[id].tsx
Add eventSourceRef and timer refs, ensure single active EventSource, close/clear on error/completion/unmount, add timeout fallback; small UI text updates.
Playwright & tests
playwright.config.ts, tests/app.spec.ts, tests/debug.spec.ts, tests/firefox-debug.spec.ts
Add Playwright config and E2E tests covering page load, JS errors, network failures and debug logging.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant PlannerService
    participant OpenAI
    participant GeminiAPI as Gemini
    Note over PlannerService: generateOutline(seed)
    Client->>PlannerService: generateOutline(seed)
    PlannerService->>PlannerService: read config.provider
    alt provider == "openai"
        PlannerService->>PlannerService: generateOutlineOpenAI(seed)
        PlannerService->>OpenAI: send prompt + schema
        OpenAI->>PlannerService: response
        PlannerService->>PlannerService: extractJson() -> PlannerOutput
    else provider == "gemini"
        PlannerService->>PlannerService: generateOutlineGemini(seed)
        PlannerService->>Gemini: send prompt
        Gemini->>PlannerService: response
        PlannerService->>PlannerService: extractJson() -> PlannerOutput
    end
    PlannerService->>Client: return PlannerOutput
Loading
sequenceDiagram
    participant Client
    participant RendererService
    participant Config
    participant OpenAI
    participant GeminiAPI as Gemini
    participant Storage
    Note over RendererService: generatePage(request, seed)
    Client->>RendererService: generatePage(request, seed)
    RendererService->>Config: read renderer.provider
    alt provider == "openai"
        RendererService->>RendererService: generatePageOpenAI(...)
        RendererService->>OpenAI: image generation request
        OpenAI->>RendererService: image (b64 or url)
    else provider == "gemini"
        RendererService->>RendererService: generatePageGemini(...)
        RendererService->>Gemini: image generation request
        Gemini->>RendererService: image (b64 or url)
    end
    opt storage enabled
        RendererService->>Storage: upload image
        Storage->>RendererService: public URL
    end
    RendererService->>Client: return { imageUrl, seed }
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Areas to focus review on:

  • Planner: OpenAI prompt/schema correctness, JSON extraction robustness, API key gating logic.
  • Renderer: provider routing, binary/base64 vs URL handling, storage upload and filename sanitization, fallback generation paths.
  • Episodes service: correct propagation of rendererModel into DB and in-memory structures.
  • Frontend SSE: lifecycle handling (timeouts, error/reconnect behavior) and cleanup.
  • Playwright: CI flags, webServer integration and test stability.

Poem

🐰 I thumped my foot and scribbled lore,

Two engines hum and paint once more.
Pages render, tests take flight—
I nibble bugs with pure delight.
Hooray, the comic garden grows! 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Code Review and Debugging Session' is generic and vague, failing to convey the specific nature of changes in this comprehensive PR that adds OpenAI support, fixes dependency injection, updates testing infrastructure, and improves error handling. Replace with a more descriptive title that highlights the main change, such as 'Add OpenAI provider support with multi-provider routing for planning and rendering' or 'Implement dual-provider architecture for story planning and image generation'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/work-in-progress-013mdrk378SdVQKANMYL3Kp8

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.

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 15, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 3c3f0a7)

Below is a summary of compliance checks for this PR:

Security Compliance
Path traversal in keys

Description: User-controlled episodeTitle is interpolated into the storage object key without full
sanitization, which can enable path traversal or key injection in object storage backends
(e.g., including slashes or Unicode separators) allowing writes outside the intended
prefix; sanitize by restricting to a safe whitelist and stripping path separators.
renderer.service.ts [95-106]

Referred Code
let finalImageUrl: string;
const padded = String(request.pageNumber).padStart(2, '0');
const filename = `episodes/${request.episodeTitle.replace(/[^a-zA-Z0-9]/g, '_')}/page_${padded}_${seed}.png`;

if (this.storage.enabled) {
    finalImageUrl = await this.storage.uploadImage(imageBuffer, filename, 'image/png');
    console.log(`Image uploaded to storage: ${finalImageUrl}`);
} else {
    console.warn('Storage not configured, using fallback placeholder');
    const shortBeat = encodeURIComponent(request.outline.beat.slice(0, 40));
    finalImageUrl = `https://placehold.co/1024x1536/FFA500/000000?text=STORAGE+DISABLED%0APAGE+${padded}%0A${shortBeat}`;
}
Sensitive data exposure

Description: On OpenAI image generation failure, the service returns a URL containing encoded page beat
text in the query string, potentially reflecting sensitive/planned story content to
third-party placeholder service placehold.co; this leaks internal data to external
domains.
renderer.service.ts [116-121]

Referred Code
const padded = String(request.pageNumber).padStart(2, '0');
const shortBeat = encodeURIComponent(request.outline.beat.slice(0, 40));
const fallbackUrl = `https://placehold.co/1024x1536/FF6B6B/FFFFFF?text=OPENAI+ERROR%0APAGE+${padded}%0A${shortBeat}`;

console.log(`Using error fallback: ${fallbackUrl}`);
return { imageUrl: fallbackUrl, seed };
Incomplete filename sanitization

Description: Character generation fallback and normal paths upload filenames derived from assetFilename
and episodeTitle; while assetFilename is sanitized, episodeTitle only replaces
non-alphanumerics but leaves underscores for folder separation and assumes forward slashes
won’t appear—ensure complete removal of all path separators and control chars to prevent
object key manipulation across storage providers.
renderer.service.ts [373-416]

Referred Code
].join('\n');

try {
    const response = await this.openaiClient.images.generate({
        model: this.config.openaiModel,
        prompt: prompt.slice(0, 32000), // gpt-image-1 supports 32k chars; DALL-E 3 is 4k
        n: 1,
        size: '1024x1792',
        quality: 'standard',
        style: 'natural',
        response_format: 'b64_json', // Request base64 for consistent handling
    });

    let imageBuffer: Buffer;
    const imageData = response.data?.[0];

    if (!imageData) {
        const url = `https://placehold.co/768x1024/222/EEE?text=${encodeURIComponent(request.name)}`;
        return { imageUrl: url };
    }



 ... (clipped 23 lines)
Verbose error logging

Description: The code logs "Calling OpenAI ..." and may log provider/model usage; while not directly
logging secrets, subsequent errors from the SDK could include request identifiers or
fragments—avoid logging full error objects from third‑party SDKs in production to prevent
inadvertent leakage of tokens or prompts.
planner.service.ts [108-121]

Referred Code
try {
  console.log(`Calling OpenAI ${this.openaiModel} for manga planning...`);
  const response = await this.openaiClient.chat.completions.create({
    model: this.openaiModel,
    messages: [
      { role: 'system', content: system },
      { role: 'user', content: user }
    ],
    temperature: 0.7,
    response_format: { type: 'json_object' },
  });

  const text = response.choices[0]?.message?.content;
  if (!text) throw new Error('No response from OpenAI');
Error detail leakage

Description: API error responses echo error.message back to clients, which can disclose internal error
details and stack information depending on thrown errors; return generic messages and map
internal errors to safe client-facing strings.
pages.controller.ts [40-42]

Referred Code
  console.error('TTS generation failed:', error);
  return { error: error instanceof Error ? error.message : 'TTS generation failed' };
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Auditing: New critical actions like AI image generation, storage uploads, and provider selection
occur without added structured audit logs including user ID, timestamp, action, and
outcome.

Referred Code
console.log(`Generating image for page ${request.pageNumber} with OpenAI ${this.config.openaiModel}`);
console.log(`Prompt: ${prompt.slice(0, 200)}...`);

// Note: gpt-image-1 supports up to 32k characters; DALL-E 3 has 4k limit
// We'll generate based on text prompt only (no image editing support)
const response = await this.openaiClient.images.generate({
    model: this.config.openaiModel,
    prompt: prompt.slice(0, 32000), // gpt-image-1 supports 32k chars; DALL-E 3 is 4k
    n: 1,
    size: '1024x1792', // Closest to 1024x1536 manga ratio
    quality: 'hd',
    style: 'natural', // More suitable for manga than 'vivid'
    response_format: 'b64_json', // Request base64 for consistent handling
});

console.log('OpenAI call completed, processing response...');

let imageBuffer: Buffer;
const imageData = response.data?.[0];

if (!imageData) {


 ... (clipped 43 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Limited Context: Error handling wraps provider failures but often rethrows without contextual metadata
(e.g., seed ID/episode ID), which may hinder debugging and omits null/shape validation
beyond basic checks.

Referred Code
try {
  console.log(`Calling OpenAI ${this.openaiModel} for manga planning...`);
  const response = await this.openaiClient.chat.completions.create({
    model: this.openaiModel,
    messages: [
      { role: 'system', content: system },
      { role: 'user', content: user }
    ],
    temperature: 0.7,
    response_format: { type: 'json_object' },
  });

  const text = response.choices[0]?.message?.content;
  if (!text) throw new Error('No response from OpenAI');

  const json = this.extractJson(text);
  // Basic validation
  if (!json || !json.pages || !Array.isArray(json.pages) || json.pages.length !== 10) {
    throw new Error('Planner returned invalid JSON shape');
  }
  return json as PlannerOutput;


 ... (clipped 5 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Error Exposure: The API returns raw error messages to clients (error.message) which may expose internal
details; safer generic messages should be used while logging specifics internally.

Referred Code
  console.error('TTS generation failed:', error);
  return { error: error instanceof Error ? error.message : 'TTS generation failed' };
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Verbose Logging: Console logs print full prompts and provider/model info which could include sensitive
story inputs or user-provided data without redaction or structured logging.

Referred Code
console.log(`Generating image for page ${request.pageNumber} with OpenAI ${this.config.openaiModel}`);
console.log(`Prompt: ${prompt.slice(0, 200)}...`);

// Note: gpt-image-1 supports up to 32k characters; DALL-E 3 has 4k limit
// We'll generate based on text prompt only (no image editing support)
const response = await this.openaiClient.images.generate({
    model: this.config.openaiModel,
    prompt: prompt.slice(0, 32000), // gpt-image-1 supports 32k chars; DALL-E 3 is 4k
    n: 1,
    size: '1024x1792', // Closest to 1024x1536 manga ratio
    quality: 'hd',
    style: 'natural', // More suitable for manga than 'vivid'
    response_format: 'b64_json', // Request base64 for consistent handling
});

console.log('OpenAI call completed, processing response...');

let imageBuffer: Buffer;
const imageData = response.data?.[0];

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Sanitization Added: While filenames are sanitized for character assets, episode titles are only
alphanumeric-replaced and other inputs (prompts, URLs) flow to external APIs without
explicit validation or length limits.

Referred Code
let finalImageUrl: string;
const padded = String(request.pageNumber).padStart(2, '0');
const filename = `episodes/${request.episodeTitle.replace(/[^a-zA-Z0-9]/g, '_')}/page_${padded}_${seed}.png`;

if (this.storage.enabled) {
    finalImageUrl = await this.storage.uploadImage(imageBuffer, filename, 'image/png');
    console.log(`Image uploaded to storage: ${finalImageUrl}`);
} else {
    console.warn('Storage not configured, using fallback placeholder');
    const shortBeat = encodeURIComponent(request.outline.beat.slice(0, 40));
    finalImageUrl = `https://placehold.co/1024x1536/FFA500/000000?text=STORAGE+DISABLED%0APAGE+${padded}%0A${shortBeat}`;
}

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 1c3d104
Security Compliance
Third-party URL exposure

Description: The OpenAI image generation path can return a temporary provider URL directly to clients
when storage is disabled (finalImageUrl = imageUrl), which may expose short-lived,
access-controlled URLs and leak provider infrastructure details; ensure images are always
proxied or uploaded to trusted storage before returning URLs.
renderer.service.ts [87-96]

Referred Code
const padded = String(request.pageNumber).padStart(2, '0');
const filename = `episodes/${request.episodeTitle.replace(/[^a-zA-Z0-9]/g, '_')}/page_${padded}_${seed}.png`;

if (this.storage.enabled) {
    finalImageUrl = await this.storage.uploadImage(imageBuffer, filename, 'image/png');
    console.log(`Image uploaded to storage: ${finalImageUrl}`);
} else {
    console.warn('Storage not configured, using OpenAI temporary URL');
    finalImageUrl = imageUrl;
}
Information disclosure

Description: Marketing copy hardcodes specific model names (e.g., "GPT-5 Mini", "GPT-Image-1") into the
UI, potentially disclosing internal provider choices and encouraging user assumptions
about capabilities; consider avoiding precise model naming in public UI to reduce
information disclosure risk.
index.tsx [381-402]

Referred Code
    <svg className="w-7 h-7" fill="currentColor" viewBox="0 0 24 24">
      <path d="M14 2H6a2 2 0 0 0-2 2v16a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V8l-6-6z"/>
      <polyline points="14,2 14,8 20,8"/>
      <line x1="16" y1="13" x2="8" y2="13"/>
      <line x1="16" y1="17" x2="8" y2="17"/>
      <polyline points="10,9 9,9 8,9"/>
    </svg>
  ),
  badge: 'GPT-5 Mini',
}, {
  title: 'Visual Generation',
  desc: 'OpenAI GPT-Image-1 creates stunning B&W manga artwork with perfect character consistency across all pages.',
  color: 'from-blue-500 to-cyan-500',
  icon: (
    <svg className="w-7 h-7" fill="currentColor" viewBox="0 0 24 24">
      <rect x="3" y="3" width="18" height="18" rx="2" ry="2"/>
      <circle cx="8.5" cy="8.5" r="1.5"/>
      <polyline points="21,15 16,10 5,21"/>
    </svg>
  ),
  badge: 'GPT-Image-1',


 ... (clipped 1 lines)
Verbose logging exposure

Description: Planner logs include explicit model identifiers and operation context (e.g., "Calling
OpenAI ... for manga planning...") which, if logs are exposed or collected centrally, may
reveal provider choices and operational details; sanitize or lower verbosity in production
logs and avoid logging secrets or overly specific identifiers.
planner.service.ts [109-132]

Referred Code
  console.log(`Calling OpenAI ${this.openaiModel} for manga planning...`);
  const response = await this.openaiClient.chat.completions.create({
    model: this.openaiModel,
    messages: [
      { role: 'system', content: system },
      { role: 'user', content: user }
    ],
    temperature: 0.7,
    response_format: { type: 'json_object' },
  });

  const text = response.choices[0]?.message?.content;
  if (!text) throw new Error('No response from OpenAI');

  const json = this.extractJson(text);
  // Basic validation
  if (!json || !json.pages || !Array.isArray(json.pages) || json.pages.length !== 10) {
    throw new Error('Planner returned invalid JSON shape');
  }
  return json as PlannerOutput;
} catch (error) {


 ... (clipped 3 lines)
Prompt logging leakage

Description: Renderer logs prompt previews ("Prompt: ...") which may include user-provided content and
internal prompt structure; logging prompts risks sensitive data leakage to log stores—mask
or truncate user content and disable in production.
renderer.service.ts [58-73]

Referred Code
console.log(`Generating image for page ${request.pageNumber} with OpenAI DALL-E 3`);
console.log(`Prompt: ${prompt.slice(0, 200)}...`);

// Note: DALL-E 3 doesn't support image editing or reference images in the same way
// We'll generate based on text prompt only
const response = await this.openaiClient.images.generate({
    model: this.config.openaiModel,
    prompt: prompt.slice(0, 4000), // DALL-E has a 4000 char limit
    n: 1,
    size: '1024x1792', // Closest to 1024x1536 manga ratio
    quality: 'hd',
    style: 'natural', // More suitable for manga than 'vivid'
});

console.log('OpenAI call completed, processing response...');
Sensitive URL logging

Description: Playwright tests log full failed request URLs and statuses to stdout, which can leak
internal endpoints or tokens embedded in query strings when test logs are shared; redact
query parameters or avoid logging full URLs.
app.spec.ts [61-81]

Referred Code
page.on('response', response => {
  if (!response.ok()) {
    failedRequests.push({
      url: response.url(),
      status: response.status()
    });
    console.log(`Failed request: ${response.url()} - Status: ${response.status()}`);
  }
});

await page.goto('/');
await page.waitForLoadState('networkidle');

if (failedRequests.length > 0) {
  console.log('\n=== FAILED REQUESTS ===');
  failedRequests.forEach((req, index) => {
    console.log(`${index + 1}. ${req.url} - Status: ${req.status}`);
  });
  console.log('======================\n');
}
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit logs: New critical actions like AI image generation requests and storage uploads add only
console logs without structured audit entries capturing user ID, timestamp, action, and
outcome.

Referred Code
console.log(`Generating image for page ${request.pageNumber} with OpenAI DALL-E 3`);
console.log(`Prompt: ${prompt.slice(0, 200)}...`);

// Note: DALL-E 3 doesn't support image editing or reference images in the same way
// We'll generate based on text prompt only
const response = await this.openaiClient.images.generate({
    model: this.config.openaiModel,
    prompt: prompt.slice(0, 4000), // DALL-E has a 4000 char limit
    n: 1,
    size: '1024x1792', // Closest to 1024x1536 manga ratio
    quality: 'hd',
    style: 'natural', // More suitable for manga than 'vivid'
});

console.log('OpenAI call completed, processing response...');

const imageUrl = response.data?.[0]?.url;
if (!imageUrl) {
    throw new Error('No image URL returned from OpenAI');
}



 ... (clipped 21 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Generic catch blocks: Catch blocks rethrow with generic messages and console errors without structured logging
or handling of null/empty responses, potentially losing context for production debugging.

Referred Code
  const filename = `tts/${Date.now()}_${Math.random().toString(36).slice(2)}.mp3`;

  // Upload to storage
  const audioUrl = await this.storage.uploadAudio(buffer, filename, 'audio/mpeg');

  return { audioUrl };
} catch (error) {
  console.error('TTS generation failed:', error);
  throw new Error(`Failed to generate speech: ${error instanceof Error ? error.message : String(error)}`);
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Error exposure: User-facing endpoint now returns raw error.message when Error instance, which may expose
internal details instead of a generic message.

Referred Code
  console.error('TTS generation failed:', error);
  return { error: error instanceof Error ? error.message : 'TTS generation failed' };
}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured logs: Added extensive console logging and printing of page body and errors in tests without
structured format and potential to capture sensitive data during runs.

Referred Code
  // Take a screenshot for debugging
  await page.screenshot({ path: 'tests/screenshots/homepage.png', fullPage: true });

  // Check if page loaded without errors
  const title = await page.title();
  console.log('Page title:', title);

  // Log any console errors
  page.on('console', msg => {
    if (msg.type() === 'error') {
      console.log('Browser console error:', msg.text());
    }
  });

  // Check for any error messages in the page
  const bodyText = await page.textContent('body');
  console.log('Page contains:', bodyText?.substring(0, 500));
});

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-code-review
Copy link

qodo-code-review bot commented Nov 15, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Abstract AI provider logic

Refactor the AI provider logic in PlannerService and RendererService to use an
abstraction, such as the Strategy pattern. This involves moving the OpenAI and
Gemini-specific code from large if/else blocks into separate classes that
implement a common interface, which will reduce duplication and improve
extensibility.

Examples:

backend/src/planner/planner.service.ts [25-30]
    if (this.provider === 'openai') {
      return this.generateOutlineOpenAI(seed);
    } else {
      return this.generateOutlineGemini(seed);
    }
  }
backend/src/renderer/renderer.service.ts [43-48]
        if (this.config.provider === 'openai') {
            return this.generatePageOpenAI(request, seed);
        } else {
            return this.generatePageGemini(request, seed);
        }
    }

Solution Walkthrough:

Before:

// backend/src/planner/planner.service.ts
export class PlannerService {
  private readonly provider = process.env.PLANNER_PROVIDER || 'openai';

  async generateOutline(seed: EpisodeSeed): Promise<PlannerOutput> {
    if (this.provider === 'openai') {
      return this.generateOutlineOpenAI(seed);
    } else {
      return this.generateOutlineGemini(seed);
    }
  }

  private async generateOutlineOpenAI(seed: EpisodeSeed): Promise<PlannerOutput> {
    // ... OpenAI specific implementation with duplicated logic ...
  }

  private async generateOutlineGemini(seed: EpisodeSeed): Promise<PlannerOutput> {
    // ... Gemini specific implementation with duplicated logic ...
  }
}

After:

// backend/src/planner/planner.provider.ts
interface IPlannerProvider {
  generateOutline(seed: EpisodeSeed): Promise<PlannerOutput>;
}

// backend/src/planner/openai.planner.ts
class OpenAIPlanner implements IPlannerProvider {
  async generateOutline(seed: EpisodeSeed) { /* ... OpenAI logic ... */ }
}

// backend/src/planner/planner.service.ts
export class PlannerService {
  constructor(private readonly plannerProvider: IPlannerProvider) {}

  async generateOutline(seed: EpisodeSeed): Promise<PlannerOutput> {
    return this.plannerProvider.generateOutline(seed);
  }
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a major architectural issue of code duplication and poor extensibility in PlannerService and RendererService, which is central to the PR's goal of adding multi-provider support.

High
Possible issue
Prevent race condition with fallback
Suggestion Impact:The commit introduces a ref to store the fallback timeout, clears it when planning completes, and also clears it on unmount and before creating new EventSources, preventing double invocation of continueAfterPlanning.

code diff:

@@ -21,6 +21,7 @@
   const [heroTitle, setHeroTitle] = useState('Create Your AI Manga');
   const [heroSubtitle, setHeroSubtitle] = useState('Transform your ideas into stunning manga pages with AI-powered storytelling and image generation');
   const eventSourceRef = useRef<EventSource | null>(null);
+  const planningTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
 
   useEffect(() => {
     fetch(`${API_BASE}/health`).then((r) => (r.ok ? r.json() : Promise.reject())).then(() => setApiUp(true)).catch(() => setApiUp(false));
@@ -48,12 +49,16 @@
       });
   }, []);
 
-  // Cleanup EventSource on unmount
+  // Cleanup EventSource and timeout on unmount
   useEffect(() => {
     return () => {
       if (eventSourceRef.current) {
         eventSourceRef.current.close();
         eventSourceRef.current = null;
+      }
+      if (planningTimeoutRef.current) {
+        clearTimeout(planningTimeoutRef.current);
+        planningTimeoutRef.current = null;
       }
     };
   }, []);
@@ -88,9 +93,13 @@
       }
       const episodeId = planJson.episodeId as string;
 
-      // Close any existing EventSource before creating a new one
+      // Close any existing EventSource and timeout before creating new ones
       if (eventSourceRef.current) {
         eventSourceRef.current.close();
+      }
+      if (planningTimeoutRef.current) {
+        clearTimeout(planningTimeoutRef.current);
+        planningTimeoutRef.current = null;
       }
 
       // Listen for planning status updates
@@ -108,6 +117,10 @@
               eventSourceRef.current.close();
               eventSourceRef.current = null;
             }
+            if (planningTimeoutRef.current) {
+              clearTimeout(planningTimeoutRef.current);
+              planningTimeoutRef.current = null;
+            }
             // Continue with the rest of the process
             continueAfterPlanning(episodeId);
           }
@@ -123,13 +136,14 @@
         }
       };
 
-      // Fallback in case SSE doesn't work
-      setTimeout(() => {
+      // Fallback in case SSE doesn't work (only runs if planning hasn't completed)
+      planningTimeoutRef.current = setTimeout(() => {
         if (eventSourceRef.current) {
           eventSourceRef.current.close();
           eventSourceRef.current = null;
         }
         continueAfterPlanning(episodeId);
+        planningTimeoutRef.current = null;
       }, 10000);
 
     } catch (err: any) {

Fix a race condition by clearing the setTimeout fallback timer when the
planning_complete SSE event is received. This prevents the continueAfterPlanning
function from being called twice.

pages/index.tsx [126-133]

 // Fallback in case SSE doesn't work
-setTimeout(() => {
+const fallbackTimeout = setTimeout(() => {
   if (eventSourceRef.current) {
     eventSourceRef.current.close();
     eventSourceRef.current = null;
   }
-  continueAfterPlanning(episodeId);
+  // Check if we haven't already proceeded
+  if (!r.asPath.startsWith('/episodes/')) {
+    continueAfterPlanning(episodeId);
+  }
 }, 10000);
 
+eventSource.onmessage = (event) => {
+  try {
+    const data = JSON.parse(event.data);
+    if (data.type === 'planning_started' || data.type === 'planning_progress') {
+      setPlanningStatus(data.message || 'Planning in progress...');
+    } else if (data.type === 'planning_complete') {
+      clearTimeout(fallbackTimeout); // Clear the fallback timer
+      setPlanningStatus(data.message || 'Planning complete!');
+      if (eventSourceRef.current) {
+        eventSourceRef.current.close();
+        eventSourceRef.current = null;
+      }
+      // Continue with the rest of the process
+      continueAfterPlanning(episodeId);
+    }
+  } catch (e) {
+    // ignore parsing errors
+  }
+};
+

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a race condition where continueAfterPlanning could be called twice, and proposes a valid fix, which significantly improves the application's robustness.

Medium
  • Update

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: 3

Caution

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

⚠️ Outside diff range comments (1)
backend/src/renderer/renderer.service.ts (1)

115-271: Seed parameter likely ineffective for image generation; consider graceful fileData fallback

The review raises concerns about non-deterministic rendering and strict error handling, but the underlying issues differ slightly from the suggestions:

  1. The seed field is documented in Vertex AI's generationConfig schema, but it's primarily supported for text generation. All observable examples of gemini-2.5-flash-image generation show responses extracted from inlineData (base64), not fileData. Passing seed to the image model's generationConfig may not improve reproducibility for this particular model—verify if gemini-2.5-flash-image-preview even honors the seed parameter for image generation before adding it.

  2. Generated image content consistently returns as inlineData, not fileData. However, rather than throwing an error if fileData ever appears, consider logging a warning and continuing to extract from it gracefully, since you may want to support that format in the future.

  3. The padStart inconsistency is valid: the general error fallback uses padStart(4, '0') while character- and style-image fallbacks use 2 digits. Standardize to 2 for consistency.

🧹 Nitpick comments (13)
backend/src/pages/pages.controller.ts (1)

60-62: Apply consistent type-safe error handling pattern.

Now that line 41 uses the type-safe error instanceof Error ? error.message : 'default' pattern, consider applying the same approach to all other catch blocks in this file for consistency.

Apply this pattern to standardize error handling:

   } catch (e: any) {
-    return { error: e?.message || String(e) };
+    return { error: e instanceof Error ? e.message : 'Operation failed' };
   }

Customize the fallback message for each endpoint as appropriate (e.g., 'Failed to save overlays', 'Failed to regenerate page', etc.).

Also applies to: 72-74, 82-84, 92-94, 102-104, 112-114, 122-124

DOCUMENTATION.md (1)

1-857: Consider addressing markdown linting issues for improved documentation quality.

Static analysis identified several minor markdown linting issues:

  • Bare URLs that should be formatted as links (lines 370, 405-407, 468, 853)
  • Fenced code blocks without language specifiers (lines 41, 104, 158, 173, 202, 787)
  • Using emphasis instead of proper headings (lines 388, 395, 859)
  • Redundant "PNG image" phrasing (line 265)

These are optional improvements for documentation polish.

tests/firefox-debug.spec.ts (1)

1-33: Consider improving test assertions and avoiding anti-patterns.

This test has several issues that limit its effectiveness:

  1. No assertions: The test only logs output but doesn't assert expected behavior. Consider adding expect() assertions.
  2. Arbitrary timeout: Line 19 uses waitForTimeout(3000) which is an anti-pattern. Use proper wait conditions like waitForSelector() or waitForLoadState().
  3. Hardcoded URL: Line 18 hardcodes 'http://localhost:3000'. Use baseURL from Playwright config or the page fixture.
  4. Manual browser launch: Lines 4-5 manually launch Firefox instead of using Playwright's test fixtures with browser configuration.

Example improvement:

-import { test, expect, firefox } from '@playwright/test';
+import { test, expect } from '@playwright/test';

-test('test with firefox', async () => {
-  const browser = await firefox.launch();
-  const page = await browser.newPage();
+test('test with firefox', async ({ page }) => {
   const errors: string[] = [];
   page.on('pageerror', error => {
     errors.push(error.message);
     console.log('PAGE ERROR:', error.message);
   });

   page.on('console', msg => {
     console.log(`CONSOLE [${msg.type()}]:`, msg.text());
   });

   try {
-    await page.goto('http://localhost:3000', { waitUntil: 'domcontentloaded', timeout: 30000 });
-    await page.waitForTimeout(3000);
+    await page.goto('/', { waitUntil: 'domcontentloaded' });
+    await page.waitForLoadState('networkidle');

     const title = await page.title();
     console.log('Page title:', title);
+    expect(title).toBeTruthy();

     const content = await page.textContent('h1');
     console.log('H1 content:', content);
+    expect(content).toBeTruthy();

+    expect(errors).toHaveLength(0);
     console.log('✅ Firefox test passed!');
   } catch (error) {
     console.log('❌ Firefox test failed:', error);
+    throw error;
-  } finally {
-    await browser.close();
   }
 });

Configure Firefox in playwright.config.ts:

projects: [
  {
    name: 'firefox',
    use: { ...devices['Desktop Firefox'] },
  },
]
tests/app.spec.ts (3)

4-27: Consider adding assertions and fixing event listener timing.

Issues with this test:

  1. Console listener attached too late: Lines 18-22 attach the console listener AFTER navigation (line 5), so errors during page load won't be captured. Move listener setup before page.goto().
  2. No assertions: The test logs output but doesn't assert expected behavior. Add assertions for title, body content, or absence of errors.
  3. Screenshot directory: Line 11 assumes 'tests/screenshots/' exists. Consider using a relative path or creating the directory.
 test('should load the home page', async ({ page }) => {
+  // Set up console listener before navigation
+  page.on('console', msg => {
+    if (msg.type() === 'error') {
+      console.log('Browser console error:', msg.text());
+    }
+  });
+
   await page.goto('/');

   // Wait for the page to be fully loaded
   await page.waitForLoadState('networkidle');

   // Take a screenshot for debugging
-  await page.screenshot({ path: 'tests/screenshots/homepage.png', fullPage: true });
+  await page.screenshot({ path: 'homepage.png', fullPage: true });

   // Check if page loaded without errors
   const title = await page.title();
   console.log('Page title:', title);
+  expect(title).toContain('MangaFusion');

-  // Log any console errors
-  page.on('console', msg => {
-    if (msg.type() === 'error') {
-      console.log('Browser console error:', msg.text());
-    }
-  });

   // Check for any error messages in the page
   const bodyText = await page.textContent('body');
   console.log('Page contains:', bodyText?.substring(0, 500));
+  expect(bodyText).toBeTruthy();
 });

29-57: Add assertions to make error detection test meaningful.

The test collects errors but doesn't assert on them. Consider adding an assertion to fail the test if errors are found (or explicitly document that this is a monitoring test).

   // Log all errors found
   if (errors.length > 0) {
     console.log('\n=== ERRORS FOUND ===');
     errors.forEach((error, index) => {
       console.log(`${index + 1}. ${error}`);
     });
     console.log('===================\n');
   } else {
     console.log('No errors found!');
   }
+
+  // Assert no errors found (or document this as monitoring only)
+  expect(errors).toHaveLength(0);
 });

59-82: Add assertion for failed network requests.

Similar to the error detection test, this test logs failed requests but doesn't assert on them.

   if (failedRequests.length > 0) {
     console.log('\n=== FAILED REQUESTS ===');
     failedRequests.forEach((req, index) => {
       console.log(`${index + 1}. ${req.url} - Status: ${req.status}`);
     });
     console.log('======================\n');
   }
+
+  // Filter out expected failures (like 404s for favicons) if any
+  const unexpectedFailures = failedRequests.filter(
+    req => !req.url.includes('favicon') && req.status !== 404
+  );
+  expect(unexpectedFailures).toHaveLength(0);
 });
tests/debug.spec.ts (1)

1-67: Consider improving test with assertions and proper wait conditions.

Similar to the other test files, this debug test has opportunities for improvement:

  1. No assertions: The test collects and logs errors but doesn't fail if errors are found.
  2. Arbitrary timeout: Line 38 uses waitForTimeout(2000). Use proper wait conditions.
  3. Hardcoded URL: Line 32 hardcodes the URL. Use baseURL from config or page fixtures.
  4. Error swallowing: The try/catch (lines 31-50) logs the error but doesn't re-throw it or assert.
 test('capture errors before crash', async ({ page }) => {
   const errors: string[] = [];
   const consoleMessages: Array<{ type: string; text: string }> = [];

   // Capture page errors
   page.on('pageerror', error => {
     const errorMsg = `PAGE ERROR: ${error.message}\n${error.stack}`;
     errors.push(errorMsg);
     console.log(errorMsg);
   });

   // Capture console messages
   page.on('console', msg => {
     const msgText = `CONSOLE [${msg.type()}]: ${msg.text()}`;
     consoleMessages.push({ type: msg.type(), text: msg.text() });
     console.log(msgText);
   });

   // Capture failed requests
   page.on('response', response => {
     if (!response.ok()) {
       const failMsg = `FAILED REQUEST: ${response.url()} - Status: ${response.status()}`;
       console.log(failMsg);
       errors.push(failMsg);
     }
   });

-  // Try to navigate without waiting for networkidle
-  try {
-    await page.goto('http://localhost:3000', {
-      waitUntil: 'domcontentloaded',
-      timeout: 30000
-    });
+  await page.goto('/', {
+    waitUntil: 'domcontentloaded',
+  });

-    // Wait a bit to see if errors appear
-    await page.waitForTimeout(2000);
+  // Wait for React to render
+  await page.waitForSelector('#__next', { state: 'attached' });

-    // Try to get page content
-    const title = await page.title();
-    console.log('Page title:', title);
+  // Try to get page content
+  const title = await page.title();
+  console.log('Page title:', title);
+  expect(title).toBeTruthy();

-    // Check if React rendered
-    const reactRoot = await page.$('#__next');
-    console.log('React root found:', reactRoot !== null);
-
-  } catch (error) {
-    console.log('Navigation error:', error);
-  }
+  // Check if React rendered
+  const reactRoot = await page.$('#__next');
+  console.log('React root found:', reactRoot !== null);
+  expect(reactRoot).not.toBeNull();

   // Print summary
   console.log('\n=== ERROR SUMMARY ===');
   console.log(`Total errors: ${errors.length}`);
   console.log(`Total console messages: ${consoleMessages.length}`);

   if (errors.length > 0) {
     console.log('\nErrors:');
     errors.forEach((err, i) => console.log(`${i + 1}. ${err}`));
   }

   if (consoleMessages.length > 0) {
     console.log('\nConsole messages:');
     consoleMessages.forEach((msg, i) => console.log(`${i + 1}. [${msg.type}] ${msg.text}`));
   }
   console.log('====================\n');
+
+  // Assert no critical errors
+  expect(errors).toHaveLength(0);
 });
backend/src/episodes/episodes.service.ts (1)

402-446: Narration panels use character: undefined while other paths use null

In stubOutline, the first narration panel is now emitted as:

dialogues.push({
  panel_number: p,
  character: undefined,
  text: `The city never sleeps...`,
  type: 'narration' as const,
});

While parseDialogueLines uses character: null for narration lines that don’t match the name: text pattern. Functionally this is fine (callers likely treat falsy/non-string characters the same), but for consistency it may be cleaner to standardize on one convention (e.g., always null for narration) across all planner/outline sources.

If you want to align them, changing undefined to null here is enough.

backend/src/planner/planner.service.ts (1)

3-22: Planner multi-provider routing is well-structured; consider normalizing provider casing

The refactor cleanly introduces:

  • Per-provider configuration (geminiApiKey, openaiApiKey, geminiModel, openaiModel) and a provider flag.
  • Provider-based routing in generateOutline to generateOutlineOpenAI vs generateOutlineGemini.
  • Provider-specific implementations that both:
    • Build the same schema/prompt,
    • Call their respective SDKs,
    • Run through a shared extractJson helper and enforce 10-page output.

That keeps the public API unchanged while adding OpenAI support and better error reporting.

One minor robustness tweak: provider is currently read as-is:

private readonly provider = process.env.PLANNER_PROVIDER || 'openai';

if (this.provider === 'openai') {  } else {  }

If someone sets PLANNER_PROVIDER=OpenAI or adds whitespace, this will silently route to the Gemini branch and then fail on missing GEMINI_API_KEY. You could defensively normalize:

private readonly provider = (process.env.PLANNER_PROVIDER || 'openai').toLowerCase();

and optionally validate against an allowed set (openai | gemini) to give clearer errors when misconfigured.

Also applies to: 24-31, 32-133, 135-221, 223-247

backend/src/renderer/renderer.service.ts (4)

1-37: DI wiring and client getters look consistent with planner service

The Injectable decoration, API key fields, and geminiClient / openaiClient getters are clean and consistent with the planner side. If you find yourself adding more services that need these clients, consider a shared helper/service to centralize client construction and API key checks, but this is fine as-is.


40-48: Provider routing silently treats any non‑openai value as Gemini

generatePage falls back to generatePageGemini for any config.provider other than the string 'openai'. If RENDERER_PROVIDER is mis-typed (e.g. 'opena1'), requests will quietly hit Gemini instead of failing fast.

Consider making the routing explicit and throwing on unknown providers, e.g.:

-        if (this.config.provider === 'openai') {
-            return this.generatePageOpenAI(request, seed);
-        } else {
-            return this.generatePageGemini(request, seed);
-        }
+        if (this.config.provider === 'openai') {
+            return this.generatePageOpenAI(request, seed);
+        }
+        if (this.config.provider === 'gemini') {
+            return this.generatePageGemini(request, seed);
+        }
+        throw new Error(`Unsupported renderer provider: ${this.config.provider}`);

343-349: Character routing mirrors page routing

The generateCharacter router cleanly mirrors the page routing logic, so the provider choice is consistent across both APIs. Once you decide how strict you want to be about unknown provider values in generatePage, consider applying the same pattern here for symmetry.


399-413: Gemini character flow: consider data URL trade-offs for storage-disabled behavior

The Gemini character flow is well-structured and consistent with the page path. Two considerations:

  • When this.storage.enabled is false, you return a placeholder instead of the actual generated image (unlike the OpenAI character path, which returns the OpenAI URL). Data URLs are acceptable for small, one-off images but increase request size, prevent caching, and don't scale well. For local/dev use, a data URL works; for production, the recommended approach for ephemeral images is uploading to Cloud Storage and returning a time-limited signed URL instead.
  • The inlineData parsing logic is nearly identical to the page path; if you introduce support for fileData or other formats later, consider extracting a shared helper to keep the two in sync.

If you choose to use data URLs for dev scenarios, the suggested change above remains valid; just be aware of the trade-offs when persistent storage is unavailable in production.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3def476 and 1c3d104.

⛔ Files ignored due to path filters (2)
  • backend/package-lock.json is excluded by !**/package-lock.json
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (22)
  • .gitignore (1 hunks)
  • DOCUMENTATION.md (1 hunks)
  • backend/.env.example (1 hunks)
  • backend/package.json (1 hunks)
  • backend/src/episodes/episodes.service.ts (7 hunks)
  • backend/src/events/events.service.ts (1 hunks)
  • backend/src/pages/pages.controller.ts (1 hunks)
  • backend/src/planner/planner.service.ts (2 hunks)
  • backend/src/prisma/prisma.service.ts (1 hunks)
  • backend/src/queue/queue.service.ts (1 hunks)
  • backend/src/renderer/config.ts (1 hunks)
  • backend/src/renderer/renderer.service.ts (4 hunks)
  • backend/src/storage/storage.service.ts (1 hunks)
  • backend/src/tts/tts.service.ts (5 hunks)
  • next.config.js (1 hunks)
  • package.json (1 hunks)
  • pages/episodes/[id].tsx (3 hunks)
  • pages/index.tsx (7 hunks)
  • playwright.config.ts (1 hunks)
  • tests/app.spec.ts (1 hunks)
  • tests/debug.spec.ts (1 hunks)
  • tests/firefox-debug.spec.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
backend/src/storage/storage.service.ts (2)
backend/src/renderer/renderer.service.ts (1)
  • Injectable (21-445)
backend/src/tts/tts.service.ts (1)
  • Injectable (10-193)
backend/src/planner/planner.service.ts (3)
backend/src/episodes/episodes.service.ts (1)
  • Injectable (10-632)
backend/src/renderer/renderer.service.ts (1)
  • Injectable (21-445)
backend/src/episodes/types.ts (2)
  • EpisodeSeed (48-56)
  • PlannerOutput (42-46)
backend/src/renderer/renderer.service.ts (1)
backend/src/renderer/config.ts (1)
  • getRendererConfig (1-10)
🪛 dotenv-linter (4.0.0)
backend/.env.example

[warning] 18-18: [UnorderedKey] The OPENAI_IMAGE_MODEL key should go before the OPENAI_PLANNER_MODEL key

(UnorderedKey)

🪛 LanguageTool
DOCUMENTATION.md

[style] ~265-~265: This phrase is redundant (‘G’ stands for ‘graphic’). Use simply “PNG”.
Context: ... specifications - Output: 1024x1792 PNG image - Quality: HD for pages, Standard f...

(ACRONYM_TAUTOLOGY)

🪛 markdownlint-cli2 (0.18.1)
DOCUMENTATION.md

41-41: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


104-104: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


158-158: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


173-173: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


202-202: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


370-370: Bare URL used

(MD034, no-bare-urls)


388-388: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


395-395: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


405-405: Bare URL used

(MD034, no-bare-urls)


406-406: Bare URL used

(MD034, no-bare-urls)


407-407: Bare URL used

(MD034, no-bare-urls)


468-468: Bare URL used

(MD034, no-bare-urls)


787-787: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


853-853: Bare URL used

(MD034, no-bare-urls)


859-859: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (21)
backend/src/pages/pages.controller.ts (1)

39-42: LGTM: Type-safe error handling applied.

The instanceof Error check before accessing error.message is a best practice that ensures type safety and provides a clear fallback message.

backend/src/queue/queue.service.ts (1)

4-4: LGTM! Proper NestJS DI integration.

The @Injectable() decorator correctly enables dependency injection for QueueService, aligning with NestJS best practices and the DI pattern applied across backend services in this PR.

backend/src/prisma/prisma.service.ts (1)

4-4: LGTM! Enables DI for PrismaService.

The @Injectable() decorator is required since PrismaService is injected into EpisodesService and other services. This change properly registers the service with NestJS's DI container.

backend/src/storage/storage.service.ts (1)

4-4: LGTM! Required for dependency injection.

The @Injectable() decorator is necessary since StorageService is injected into RendererService and TTSService constructors. This properly registers the service with NestJS's DI system.

backend/src/events/events.service.ts (1)

13-13: LGTM! Enables DI for EventsService.

The @Injectable() decorator is required since EventsService is injected into EpisodesService for emitting episode and page lifecycle events. This change enables proper DI resolution.

backend/src/tts/tts.service.ts (2)

10-10: LGTM! Adds DI support.

The @Injectable() decorator enables TTSService to be properly registered with NestJS's dependency injection container, aligning with the DI pattern applied across backend services in this PR.


64-64: Excellent type-safe error handling!

The error handling improvements correctly use error instanceof Error ? error.message : String(error) to safely format error messages. This prevents "[object Object]" messages and gracefully handles non-Error throws.

Also applies to: 137-137, 161-161, 190-190

next.config.js (1)

3-3: Verify the necessity of disabling React Strict Mode.

The codebase shows proper EventSource cleanup patterns. EventSource connections in pages/index.tsx and pages/episodes/[id].tsx are correctly closed in useEffect cleanup functions. The useEffect patterns appear sound and should work correctly with Strict Mode enabled.

Disabling Strict Mode is generally not recommended unless there's a specific, documented reason. Consider:

  1. Re-enabling reactStrictMode: true and testing locally to confirm there are no actual issues
  2. If issues arise, fix the specific problems rather than disabling the check
  3. Document the reason if disabling is truly necessary

If this was disabled due to observed issues during testing, please share those details so the underlying problems can be addressed properly.

.gitignore (1)

6-8: LGTM!

Appropriate ignore patterns for Next.js build output and Playwright test artifacts.

package.json (2)

29-29: LGTM! Playwright test infrastructure added.

The addition of Playwright dependencies supports the new E2E testing infrastructure introduced in this PR.

Also applies to: 35-35


23-24: React downgrade from 19.1.1 to 18.3.1 is safe and intentional.

Verification confirms:

  1. Intentional downgrade: Git history shows commit "Fix: Downgrade React from 19.1.1 to 18.3.1 for stability"
  2. No React 19 features in use: Codebase contains no React 19–specific hooks (useOptimistic, useFormStatus, useActionState), React.use() API, or 'use' directives
  3. Full compatibility: Project uses Pages Router only (no App Router configuration), which is fully backward compatible with React 18 in Next.js 15; @types/react is correctly aligned to ^18.2.66

All concerns raised have been verified as non-issues. The downgrade is safe to proceed.

pages/episodes/[id].tsx (3)

42-42: Excellent EventSource lifecycle management!

The addition of eventSourceRef and proper cleanup before creating a new EventSource prevents memory leaks and multiple concurrent connections. This matches the best practice pattern for managing Server-Sent Events.

Also applies to: 101-107


129-135: LGTM! Proper error handling to prevent reconnection attempts.

Closing the EventSource on error prevents automatic reconnection attempts, which is the correct behavior for this use case.


137-142: LGTM! Proper cleanup on unmount.

The cleanup function ensures the EventSource is closed and the ref is cleared when the component unmounts or the effect re-runs, preventing resource leaks.

backend/package.json (1)

23-23: OpenAI package version is current and secure—no action needed.

The latest stable version of the openai package is 6.9.0, which matches the version specified in package.json. No known security vulnerabilities were found for this package.

playwright.config.ts (1)

1-39: Playwright setup looks solid

Config values (testDir, baseURL, CI-specific retries/workers, webServer, Chromium project) are coherent and should work well for local + CI E2E runs.

pages/index.tsx (1)

375-402: Feature badges and copy updates

The updated badges/descriptions (GPT-5 Mini / GPT-Image-1) are consistent and purely presentational; no behavioral impact from these changes.

backend/.env.example (1)

4-27: Provider and model env docs align with backend config

The added PLANNER_PROVIDER/RENDERER_PROVIDER and model envs (OPENAI_PLANNER_MODEL, OPENAI_IMAGE_MODEL, PLANNER_MODEL, RENDERER_IMAGE_MODEL) match how PlannerService and getRendererConfig() read configuration, so this example will help users wire providers correctly.

backend/src/episodes/episodes.service.ts (2)

10-23: EpisodesService DI + rendererModel propagation look consistent

  • Adding @Injectable() (Line 10) correctly registers EpisodesService for NestJS DI given the injected services in the constructor.
  • planEpisode now derives rendererModel from getRendererConfig() and persists it both in Prisma (Line 52) and the in-memory Episode (Lines 77–78, 102). getEpisode mirrors this via rendererModel: e.rendererModel ?? undefined (Line 310).

This keeps the episode’s renderer model visible to callers without changing how RendererService itself selects a provider, and doesn’t introduce behavioral regressions.

Also applies to: 41-53, 64-80, 95-103, 310-319


295-307: Overlays normalization for DB pages is reasonable

The new mapping for overlays:

overlays: Array.isArray((p as any).overlays)
  ? (p as any).overlays
  : (p as any).overlays?.items ?? undefined

handles both legacy “array-only” and newer { items, dialogueOverride } shapes cleanly, so consumers always see either an array or undefined. This is a pragmatic compatibility shim given the evolving overlays schema.

backend/src/renderer/config.ts (1)

1-10: Renderer config matches env example and renderer usage

The struct returned by getRendererConfig() (provider + separate OpenAI/Gemini model names) lines up with .env.example and the new provider-routing in RendererService. Defaults (openai, gpt-image-1, gemini-2.5-flash-image-preview) are sensible.

Comment on lines +254 to +255
- **Model**: `gpt-5-mini-2025-08-07`
- **Purpose**: Generate 10-page manga outlines
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

🧩 Analysis chain

Verify the AI model identifiers are correct and available.

The documentation references several AI models:

  • gpt-5-mini-2025-08-07 (line 254)
  • gpt-image-1 (line 262)
  • gemini-2.5-flash (line 285)
  • gemini-2.5-flash-image-preview (line 286)

Since my knowledge is from March 2025 and this PR is from November 2025, please verify these model names are correct and currently available from the respective providers.

Also applies to: 262-263, 285-286


🌐 Web query:

What are the current OpenAI GPT models available for text generation in November 2025?

💡 Result:

Short answer — as of November 15, 2025, OpenAI’s main GPT-family text-generation models available via the API (and in ChatGPT/Products) include:

  • GPT-5 series: gpt-5 (and smaller variants gpt-5‑mini, gpt-5‑nano) and the new GPT‑5.1 family (gpt-5.1, gpt-5.1-chat-latest, gpt-5.1-codex, etc.). [1][2]
  • GPT-4.1 family: gpt-4.1 and its smaller variants (gpt-4.1-mini, gpt-4.1-nano). [2][5]
  • GPT-4o family: gpt-4o (and snapshots/variants such as gpt-4o‑mini, audio/realtime/search preview variants used in products). [5][6]
  • GPT-4 / GPT-4-turbo (legacy / API snapshots still available in the API). [6]
  • GPT-3.5 family: gpt-3.5-turbo (and 16k / other snapshot variants). [6]
  • “o” series (OpenAI O models): o3 (and o3-pro), o4‑mini / o4‑mini variants, o1 / o1‑mini — lower-latency / cost-optimized options used for many text tasks. [5][7]
  • Open-weight releases for self‑hosting: gpt-oss-120b and gpt-oss-20b (text-only open‑weight models). [4]

Sources: OpenAI product announcements and pricing/release notes (GPT‑5/GPT‑5.1, model/pricing pages, and ChatGPT release notes). [1][2][4][5]

If you want, I can produce the exact API model IDs and example API calls for any specific model you plan to use.


🌐 Web query:

What are the current OpenAI image generation models available in November 2025?

💡 Result:

As of November 15, 2025, OpenAI’s publicly available image-generation models include:

  1. DALL·E 3 — current flagship DALL·E model (API + ChatGPT). [1][3]
  2. DALL·E 2 — older DALL·E generation still available for backward compatibility. [1][4]
  3. GPT Image / GPT-4o (omnimodal) image generation — image generation integrated into ChatGPT and offered via the GPT Image API (autoregressive, multimodal image capability). [2][5]

Sources: OpenAI DALL·E 3 API and docs; OpenAI release notes about image generation in GPT-4o; reporting on ChatGPT image/GPT Image rollout. [1][3][2][5]


🌐 Web query:

What are the current Google Gemini models available in November 2025?

💡 Result:

As of November 15, 2025, the Gemini lineup in active use (cloud/consumer/Vertex AI) includes multiple families and variants. Major available models are:

  • Gemini 2.5 family: Gemini 2.5 Pro (most capable), Gemini 2.5 Flash, Gemini 2.5 Flash‑Lite (and image/video variants). [1][2]
  • Gemini 2.0 family: Gemini 2.0 Flash (default/fast), Gemini 2.0 Flash‑Lite, and Gemini 2.0 Pro / experimental “thinking” variants (still offered in some surfaces). [1][3]
  • Earlier 1.5 family still present in some places: Gemini 1.5 Pro and Gemini 1.5 Flash/Nano (on‑device/lightweight) though superseded by 2.x for most new features. [3][4]

Sources: news coverage and model listings from Google/press summaries. [1][2][3][4]

References

  1. The Verge — coverage of Gemini 2.5 / 2.0 updates.
  2. TechRadar — Oracle/Google Cloud Gemini offering (lists 2.5 availability).
  3. Tom’s Guide — Gemini model-family overview (1.0 → 2.5).
  4. Wikipedia — Gemini (language model) version table and status.

[/run_scripts]


🌐 Web query:

Is gpt-image-1 a valid OpenAI model identifier in November 2025?

💡 Result:

Yes — as of November 15, 2025, gpt-image-1 is a valid OpenAI model identifier (OpenAI's image-generation model). [1][2]

Sources:
[1] OpenAI — Introducing our latest image generation model in the API (Apr 23, 2025).
[2] TechCrunch — "OpenAI makes its upgraded image generator available to developers" (Apr 23, 2025).


🌐 Web query:

What is the exact model identifier for Gemini 2.5 Flash image generation?

💡 Result:

The model identifier is gemini-2.5-flash-image (preview alias: gemini-2.5-flash-image-preview). [1][2]

Sources:
[1] Google AI (Gemini models) — model code: gemini-2.5-flash-image.
[2] Firebase / Vertex AI docs — lists gemini-2.5-flash-image and gemini-2.5-flash-image-preview.


🌐 Web query:

Do OpenAI GPT models use date suffixes like 2025-08-07 in their API model identifiers?

💡 Result:

Short answer: No — OpenAI’s official API model identifiers do not use date suffixes. Official docs and pages list models with names like gpt-4o, gpt-5, gpt-5-mini, gpt-4o-mini, etc., not with YYYY‑MM‑DD suffixes. [1][2]

Why you might see date-suffixed names: some third-party directories or gateway services (not OpenAI) sometimes append dates to a model alias (e.g., "gpt-5-mini-2025-08-07") for their own indexing or release-date tagging — those are provider-specific labels, not the official OpenAI model IDs. [3][4]

Sources:

  • OpenAI GPT-5 System Card / model naming examples. [1]
  • OpenAI API/pricing and model pages showing model IDs (gpt-5, gpt-5-mini, gpt-4o, etc.). [2]
  • Third-party listings that append dates to model aliases (example: Glama). [3]
  • Community model index showing date-tagged entries (models.dev). [4]

Remove the date suffix from the gpt-5-mini model identifier.

The model identifier on line 254 should be gpt-5-mini, not gpt-5-mini-2025-08-07. OpenAI's official API model identifiers do not use date suffixes; date-suffixed names are provider-specific labels used by third-party services, not official OpenAI model IDs.

The other models verified correctly: gpt-image-1 is a valid OpenAI image-generation model as of November 2025, and gemini-2.5-flash-image-preview is a valid preview alias for Google's Gemini 2.5 Flash image model.

🤖 Prompt for AI Agents
In DOCUMENTATION.md around lines 254 to 255 the model identifier is incorrectly
listed as "gpt-5-mini-2025-08-07"; replace that string with the official OpenAI
model id "gpt-5-mini" (remove the date suffix) so the document uses the correct
vendor-neutral model identifier.

Critical Fixes:
- Fix OpenAI image generation to handle b64_json response format
  - Update generatePageOpenAI to accept both b64_json and url formats
  - Update generateCharacterOpenAI to handle both response formats
  - Add response_format: 'b64_json' parameter for consistent behavior
  - Fix log message to use actual model name instead of hardcoded "DALL-E 3"

- Sanitize assetFilename to prevent path traversal attacks
  - Add sanitization in generateCharacterOpenAI (renderer.service.ts:405)
  - Add sanitization in generateCharacterGemini (renderer.service.ts:457)
  - Prevent malicious filenames like "../../../etc/passwd"

- Fix SSE fallback timeout to prevent double execution and memory leaks
  - Add planningTimeoutRef to track timeout in pages/index.tsx
  - Clear timeout when planning completes successfully
  - Clear timeout on component unmount to prevent post-unmount state updates
  - Prevents continueAfterPlanning from running twice for same episode

- Update OpenAI model identifier to correct API format
  - Change from 'gpt-5-mini-2025-08-07' to 'gpt-5-mini'
  - OpenAI API doesn't support date-suffixed model names
  - Updated in backend/.env.example and planner.service.ts

All changes verified - backend builds successfully with zero TypeScript errors.
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: 1

Caution

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

⚠️ Outside diff range comments (2)
pages/index.tsx (1)

30-50: Add cleanup to prevent post-unmount state updates.

The fetch lacks an abort mechanism, so if the component unmounts before the fetch completes, setHeroTitle and setHeroSubtitle will trigger React warnings.

Apply this diff to add a mounted flag:

 useEffect(() => {
+  let mounted = true;
   // Load content from words.md
   fetch('/words.md')
     .then(response => response.text())
     .then(content => {
+      if (!mounted) return;
       const lines = content.split('\n').filter(line => line.trim());
       if (lines.length > 0) {
         // Extract title from first line (remove # and trim)
         const title = lines[0].replace(/^#+\s*/, '').trim();
         setHeroTitle(title);

         // Extract subtitle from second line if it exists
         if (lines.length > 1) {
           setHeroSubtitle(lines[1].trim());
         }
       }
     })
     .catch(error => {
       console.log('Could not load words.md, using default text');
     });
+  return () => { mounted = false; };
 }, []);
backend/src/renderer/renderer.service.ts (1)

275-275: Inconsistent zero-padding for page numbers.

Line 275 uses padStart(4, '0') while the rest of the file (lines 96, 116, 245, 268) uses padStart(2, '0'). This inconsistency could cause confusion when debugging or searching logs.

Apply this diff:

-            const padded = String(request.pageNumber).padStart(4, '0');
+            const padded = String(request.pageNumber).padStart(2, '0');
🧹 Nitpick comments (6)
pages/index.tsx (1)

96-103: Set ref to null after closing for consistency.

For consistency with the other cleanup sites (lines 118, 135, 143), set eventSourceRef.current = null after closing it.

Apply this diff:

 // Close any existing EventSource and timeout before creating new ones
 if (eventSourceRef.current) {
   eventSourceRef.current.close();
+  eventSourceRef.current = null;
 }
 if (planningTimeoutRef.current) {
   clearTimeout(planningTimeoutRef.current);
   planningTimeoutRef.current = null;
 }
backend/src/planner/planner.service.ts (3)

14-22: Consider caching client instances to avoid repeated instantiation.

Both geminiClient and openaiClient getters create new client instances on every access. If these getters are called frequently (e.g., multiple outline generations in quick succession), this could add unnecessary overhead.

Apply this refactor to cache client instances:

+  private _geminiClient?: GoogleGenerativeAI;
+  private _openaiClient?: OpenAI;
+
   private get geminiClient() {
     if (!this.geminiApiKey) throw new Error('GEMINI_API_KEY not set');
-    return new GoogleGenerativeAI(this.geminiApiKey);
+    if (!this._geminiClient) {
+      this._geminiClient = new GoogleGenerativeAI(this.geminiApiKey);
+    }
+    return this._geminiClient;
   }

   private get openaiClient() {
     if (!this.openaiApiKey) throw new Error('OPENAI_API_KEY not set');
-    return new OpenAI({ apiKey: this.openaiApiKey });
+    if (!this._openaiClient) {
+      this._openaiClient = new OpenAI({ apiKey: this.openaiApiKey });
+    }
+    return this._openaiClient;
   }

32-33: Redundant API key validation.

Line 33 validates OPENAI_API_KEY explicitly, but the openaiClient getter (line 110) already throws the same error if the key is missing. This validation is redundant and can be removed.

Apply this diff:

   private async generateOutlineOpenAI(seed: EpisodeSeed): Promise<PlannerOutput> {
-    if (!this.openaiApiKey) throw new Error('Planner unavailable: OPENAI_API_KEY not set');
-
     const system = [

135-137: Redundant API key validation.

Similar to the OpenAI path, line 136 validates GEMINI_API_KEY explicitly, but the geminiClient getter (line 211) already performs this check. This validation is redundant.

Apply this diff:

   private async generateOutlineGemini(seed: EpisodeSeed): Promise<PlannerOutput> {
-    if (!this.geminiApiKey) throw new Error('Planner unavailable: GEMINI_API_KEY not set');
-
     const system = [
backend/src/renderer/renderer.service.ts (2)

99-106: Consider returning data URL when storage is disabled and b64_json is available.

When storage is disabled (lines 103-106), the code returns a placeholder URL even though imageBuffer contains the actual generated image from b64_json. This discards the generated image unnecessarily.

Apply this diff to preserve the generated image as a data URL when storage is unavailable:

             if (this.storage.enabled) {
                 finalImageUrl = await this.storage.uploadImage(imageBuffer, filename, 'image/png');
                 console.log(`Image uploaded to storage: ${finalImageUrl}`);
             } else {
-                console.warn('Storage not configured, using fallback placeholder');
+                console.warn('Storage not configured, returning data URL');
                 const shortBeat = encodeURIComponent(request.outline.beat.slice(0, 40));
-                finalImageUrl = `https://placehold.co/1024x1536/FFA500/000000?text=STORAGE+DISABLED%0APAGE+${padded}%0A${shortBeat}`;
+                const base64Image = imageBuffer.toString('base64');
+                finalImageUrl = `data:image/png;base64,${base64Image}`;
             }

343-344: Image generation models typically struggle with rendering readable text.

Lines 343-344 instruct the model to "INCLUDE speech bubbles with readable text" and specify formatting. However, most current image generation models (including DALL-E 3, gpt-image-1, and Gemini image models) have difficulty rendering accurate, readable text within images. This requirement may result in gibberish or poorly formed text in speech bubbles.

Consider either:

  1. Documenting this as a known limitation
  2. Implementing text overlay in post-processing rather than relying on AI-generated text
  3. Adjusting expectations in the prompt to accept stylized/approximated text
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c3d104 and 78f26ef.

📒 Files selected for processing (4)
  • backend/.env.example (1 hunks)
  • backend/src/planner/planner.service.ts (2 hunks)
  • backend/src/renderer/renderer.service.ts (5 hunks)
  • pages/index.tsx (7 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/planner/planner.service.ts (1)
backend/src/episodes/types.ts (2)
  • EpisodeSeed (48-56)
  • PlannerOutput (42-46)
backend/src/renderer/renderer.service.ts (4)
backend/src/planner/planner.service.ts (1)
  • Injectable (6-247)
backend/src/storage/storage.service.ts (1)
  • Injectable (4-201)
backend/src/episodes/episodes.service.ts (1)
  • Injectable (10-632)
backend/src/renderer/config.ts (1)
  • getRendererConfig (1-10)
🪛 dotenv-linter (4.0.0)
backend/.env.example

[warning] 18-18: [UnorderedKey] The OPENAI_IMAGE_MODEL key should go before the OPENAI_PLANNER_MODEL key

(UnorderedKey)

🔇 Additional comments (6)
pages/index.tsx (2)

2-2: Excellent EventSource and timeout cleanup implementation!

The ref-based tracking and cleanup properly addresses the concerns from the previous review about duplicate continueAfterPlanning calls and post-unmount state updates:

  • clearTimeout(planningTimeoutRef.current) on planning_complete ensures the fallback doesn't run if SSE succeeds
  • Closing the EventSource before the timeout fires prevents late-arriving planning_complete events
  • The cleanup effect guarantees both resources are released on unmount

The flow is now safe: either SSE completes and clears the timeout, or the timeout fires and closes SSE, but never both.

Also applies to: 23-24, 52-64, 96-103, 107-107, 116-123, 132-137, 139-147


403-403: LGTM! UI updates reflect the new OpenAI provider integration.

The badge and description changes accurately reflect the dual-provider architecture introduced in this PR, highlighting GPT-5 Mini for planning and GPT-Image-1 for image generation.

Also applies to: 406-406, 415-415

backend/src/planner/planner.service.ts (1)

223-246: LGTM! Robust JSON extraction with appropriate fallbacks.

The multi-stage parsing approach (direct parse → bracket extraction → code fence) handles common AI response patterns effectively. The silent catch blocks are appropriate here since each failure triggers the next fallback strategy.

backend/src/renderer/renderer.service.ts (3)

405-407: LGTM! Filename sanitization prevents path traversal.

The sanitization of assetFilename using replace(/[^a-zA-Z0-9._-]/g, '_') properly prevents path traversal attacks and ensures safe storage paths. This addresses the concern from the previous review.


457-459: LGTM! Consistent filename sanitization.

The Gemini character generation path also properly sanitizes assetFilename, maintaining consistency with the OpenAI path and preventing path traversal vulnerabilities.


67-67: OpenAI DALL-E 3 limitation, not an implementation error.

DALL-E 3 only supports three sizes: 1024x1024, 1792x1024, or 1024x1792. The intended 1024x1536 (2:3 ratio) is unsupported. The code correctly uses 1024x1792 as the closest available alternative for the vertical manga format.

The misalignment between the API size (1024x1792, which is 16:9) and the prompts/specs (which reference 1024x1536, which is 2:3) creates a documentation/expectation gap: the generated image dimensions won't match what the prompts request. Consider updating the prompts to align expectations with what DALL-E 3 can actually produce, or clarifying that 1024x1792 was selected as the closest practical alternative.

- Increase prompt limit from 4,000 to 32,000 characters for gpt-image-1
- gpt-image-1 supports up to 32k chars vs DALL-E 3's 4k limit
- Prevents unnecessary truncation of detailed manga generation prompts
- Applied to both generatePageOpenAI and generateCharacterOpenAI methods
- Updated comments to clarify model-specific limits
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: 0

♻️ Duplicate comments (1)
backend/src/renderer/renderer.service.ts (1)

50-123: OpenAI page generation path looks solid and fixes the earlier b64/url issue

This implementation now correctly requests b64_json, handles both b64_json and url, uploads the resulting Buffer to storage, and has clear error placeholders, so the previous problem where gpt-image-1 produced no usable URL is resolved. As a follow-up, you might want a small helper that derives the prompt.slice(...) limit from this.config.openaiModel so switching to a model with a smaller window does not unexpectedly start failing.

🧹 Nitpick comments (4)
backend/src/renderer/renderer.service.ts (4)

3-37: Client initialization could be memoized

geminiClient and openaiClient currently construct new SDK clients on every access. This is acceptable but you might want to memoize them in private fields (lazy-init on first use) so you can reuse connections and centralize options like timeouts or custom transports.


39-48: Prefer nullish coalescing for seed handling

If 0 is ever treated as a valid seed, const seed = request.seed || ... will override it with a random value. Using const seed = request.seed ?? Math.floor(Math.random() * 1_000_000); would avoid that surprise.


361-422: Consider logging OpenAI character generation failures and revisiting storage-disabled behavior

This path correctly supports both b64_json and url responses and sanitizes assetFilename, but the catch block silently returns a placeholder without logging the underlying error, which will make production debugging harder; adding a console.error or Nest logger call there would help. Also, when storage is disabled you currently discard a successfully generated image and return a placeholder—if you ever want character previews to work without Supabase, you might later prefer returning a data URL derived from the Buffer instead.


405-408: Filename sanitization is a good hardening; guard against empty results

The new assetFilename sanitization for both OpenAI and Gemini character flows nicely closes off path traversal and odd keys. One small edge case: if the original filename is empty or composed only of stripped characters, sanitizedFilename becomes an empty string and you end up uploading to the characters/ prefix itself; consider falling back to a generated default (e.g., based on request.name plus an extension) when sanitizedFilename.length === 0.

Also applies to: 457-459

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78f26ef and 3c3f0a7.

📒 Files selected for processing (1)
  • backend/src/renderer/renderer.service.ts (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/renderer/renderer.service.ts (4)
backend/src/planner/planner.service.ts (1)
  • Injectable (6-247)
backend/src/episodes/episodes.service.ts (1)
  • Injectable (10-632)
backend/src/storage/storage.service.ts (1)
  • Injectable (4-201)
backend/src/renderer/config.ts (1)
  • getRendererConfig (1-10)
🔇 Additional comments (3)
backend/src/renderer/renderer.service.ts (3)

125-145: Gemini provider wiring is consistent with renderer config

Using this.config.geminiModel in both the log message and getGenerativeModel keeps runtime behavior aligned with configuration, and the explicit GEMINI_API_KEY guard matches the OpenAI path. This looks correct.


353-359: Character provider routing mirrors page routing as expected

The provider switch in generateCharacter mirrors generatePage and keeps the public API unchanged while enabling OpenAI-based character rendering, which aligns with how EpisodesService calls this service.


424-472: Gemini character generation path matches the OpenAI flow and storage contract

The Gemini character path reuses the same prompt shape, extracts inline image data correctly, applies the same sanitized filename convention, and respects this.storage.enabled with reasonable placeholders. This keeps behavior consistent across providers.

@improdead improdead merged commit 6ce0f8b into main Nov 15, 2025
2 checks passed
improdead pushed a commit that referenced this pull request Nov 17, 2025
Implemented 4 critical improvements from code review:

1. Singleton Redis client (#2): Refactored QueueEventsBridgeService to use
   a singleton Redis publisher pattern, preventing connection overhead from
   creating new connections for every worker event emission.

2. Defensive null checks (#3): Added null checks in worker after Supabase
   storage upload and getPublicUrl calls to prevent runtime errors when
   storage operations return no data.

3. Character job error handling (#4): Enhanced character job processing to
   emit character_done and character_failed events for real-time updates,
   matching the consistency of page job event handling.

4. Parallel export downloads (#6): Refactored PDF export to download all
   page images in parallel using Promise.all(), improving performance from
   ~30s to ~5s for 10-page episodes (5-10x speedup).

All changes tested with successful TypeScript build.
@coderabbitai coderabbitai bot mentioned this pull request Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants