feat: improve AI image failures and refresh progress#607
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis pull request introduces provider error handling, a safe rewrite mechanism for moderation-blocked image generation, and comprehensive progress instrumentation across the refresh pipeline. Changes include a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant OpenAI
Client->>Server: POST /api/plugin/{plugin_id}/image (with moderation-prone prompt)
Server->>OpenAI: POST /v1/images/generations
OpenAI-->>Server: 400 error with moderation_blocked code
alt safeRewriteBlockedPrompt enabled
Server->>OpenAI: POST /v1/chat/completions (rewrite prompt via gpt-3.5-nano)
OpenAI-->>Server: Rewritten, sanitized prompt
Server->>OpenAI: POST /v1/images/generations (retry with rewritten prompt)
OpenAI-->>Server: 200 OK with image data
Server-->>Client: Image successfully generated
else rewrite disabled or fails
Server-->>Client: 400 ProviderReportedPluginError with moderation message
end
sequenceDiagram
participant Client
participant Server
participant EventStream
participant Plugin/Display
Client->>Server: POST /api/plugin/{plugin_id}/image
Server-->>Client: 202 Accepted (background job)
Client->>EventStream: GET /api/progress/stream?plugin_id=X
EventStream-->>Client: SSE connection established
par Server Processing
Server->>Plugin/Display: Execute refresh task
Server->>EventStream: publish_step("Starting render attempt 1/1")
EventStream-->>Client: step event received
Server->>Plugin/Display: Render image
Server->>EventStream: publish_step("Image generated")
EventStream-->>Client: step event received
Server->>Plugin/Display: Save to display
Server->>EventStream: publish_step("Display complete")
EventStream-->>Client: step event received
and Client Polling
Client->>Server: Poll /api/plugin/{plugin_id}/image (job status)
Note over Client: Live events suppress incremental updates
Server-->>Client: Status update
end
Server->>EventStream: publish_step("done")
EventStream-->>Client: done event
Client->>EventStream: Close connection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Memory diff vs base
Largest grouped allocator deltas
Source-location detail: top 18 deltas (sampled base=500, PR=500)
JTN-610 · backend=base:memray, pr:memray · informational only, does not block merge. Hard RSS budgets are enforced separately by JTN-608. Source-location rows are sampled allocator attribution, not exact module ownership. |
There was a problem hiding this comment.
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)
src/refresh_task/display_pipeline.py (1)
225-250: 🧹 Nitpick | 🔵 TrivialMinor type hint inconsistency.
The return type
Callable[[Mapping[str, Any]], None] | NoneincludesNonebut the method always returnson_image_saved. If the| Noneis for protocol compatibility or future use, consider adding a brief comment; otherwise, the type could be simplified.♻️ Simplify return type if None is not a valid return
def _image_saved_callback( self, *, manual_request: ManualUpdateRequest | None, plugin_id: str, request_id: str | None, - ) -> Callable[[Mapping[str, Any]], None] | None: + ) -> Callable[[Mapping[str, Any]], None]: """Return a callback that reports disk-save progress and releases waiters."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/refresh_task/display_pipeline.py` around lines 225 - 250, The return type annotation for _image_saved_callback is overly permissive: change the return type from Callable[[Mapping[str, Any]], None] | None to Callable[[Mapping[str, Any]], None] (or if None must remain for compatibility, add a short comment explaining why) so that the signature matches the implementation that always returns on_image_saved; update the function signature for _image_saved_callback and ensure any callers or type stubs expect Callable[[Mapping[str, Any]], None]; reference symbols: _image_saved_callback, on_image_saved, ManualUpdateRequest, image_saved_metrics, image_saved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/plugins/ai_image/ai_image.py`:
- Around line 395-428: You extract the OpenAI payload once via
_openai_error_payload(exc) but then call _is_openai_moderation_block(exc) which
recomputes it; change the logic to reuse the already-extracted payload by
either: (a) updating _is_openai_moderation_block to accept the payload (e.g.,
_is_openai_moderation_block(payload)) and call it with the payload, or (b) add
an inline check using the payload fields directly before invoking
_safe_rewrite_openai_prompt; apply this where safe_rewrite_blocked_prompt is
checked and keep all other flows (logging via _openai_user_error_message, retry
with _safe_rewrite_openai_prompt, and raising ProviderReportedPluginError)
unchanged.
In `@src/refresh_task/task.py`:
- Around line 64-74: Replace the hardcoded _API_KEY_PLUGIN_IDS frozenset with a
dynamic computation that inspects plugin metadata at runtime: iterate the plugin
registry/collection used by the task (e.g., the same source that loads plugins)
and for each plugin check a standardized signal such as a class attribute (e.g.,
requires_api_key) or the output of generate_settings_template() for an 'api_key'
entry, collecting those plugin ids into a frozenset; update any references to
_API_KEY_PLUGIN_IDS to use this computed set so new plugins that declare the
metadata are included automatically.
In `@tests/plugins/test_ai_image.py`:
- Around line 334-378: Add a new test alongside
test_ai_image_openai_safe_rewrite_retries_moderation_block that simulates the
rewritten prompt also being rejected and asserts the code raises
ProviderReportedPluginError with the retry error details: create an AIImage
instance, patch OpenAI to return a failing images.generate twice (first
original, then rewritten) by setting mock_client.images.generate.side_effect =
[_FakeOpenAIImageError(), _FakeOpenAIImageError()] (or another simulated
provider error), stub the chat completion to return the rewrite, call
AIImage.generate_image with safeRewriteBlockedPrompt set, and assert that
ProviderReportedPluginError is raised and contains the second error's message;
reference the test function name
test_ai_image_openai_safe_rewrite_retries_moderation_block, the
AIImage.generate_image method, and ProviderReportedPluginError to locate where
to add this new test.
---
Outside diff comments:
In `@src/refresh_task/display_pipeline.py`:
- Around line 225-250: The return type annotation for _image_saved_callback is
overly permissive: change the return type from Callable[[Mapping[str, Any]],
None] | None to Callable[[Mapping[str, Any]], None] (or if None must remain for
compatibility, add a short comment explaining why) so that the signature matches
the implementation that always returns on_image_saved; update the function
signature for _image_saved_callback and ensure any callers or type stubs expect
Callable[[Mapping[str, Any]], None]; reference symbols: _image_saved_callback,
on_image_saved, ManualUpdateRequest, image_saved_metrics, image_saved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b08eee1-cfba-40ac-961e-b9b9a634a0f0
📒 Files selected for processing (11)
src/blueprints/plugin.pysrc/plugins/ai_image/ai_image.pysrc/refresh_task/display_pipeline.pysrc/refresh_task/executor.pysrc/refresh_task/task.pysrc/refresh_task/worker.pysrc/static/scripts/plugin_form.jssrc/utils/plugin_errors.pytests/plugins/test_ai_image.pytests/unit/test_refresh_task_display_pipeline.pytests/unit/test_refresh_task_executor.py
|



Summary
moderation_blockedand request ids.Base Branch Confirmation
origin/main(not a stale long-lived branch)origin/mainbefore openingParent-Fork Sync Checklist
fatihak/InkyPi, changes were cherry-picked by featureN/A: this is not an upstream sync PR.
Compatibility/Release Checklist
pytestrelevant suites pass locallysuccess:false,error,code,details,request_id)src/static/**,src/templates/**): ran browser tests (SKIP_BROWSER=0 .venv/bin/python -m pytest tests/) and all passedFrontend note: full browser suite was not run;
node --check src/static/scripts/plugin_form.jsand route/progress integration tests passed. The new AI Image setting is documented inline in the settings schema hint.Testing
python3 -m pytest tests/plugins/test_ai_image.py tests/unit/test_refresh_task_display_pipeline.py tests/unit/test_refresh_task_executor.py -qpython3 -m pytest tests/integration/test_plugin_routes.py tests/unit/test_progress_events.py tests/integration/test_observability_api.py -qpython3 -m black --check src/plugins/ai_image/ai_image.py src/utils/plugin_errors.py src/refresh_task/worker.py src/blueprints/plugin.py src/refresh_task/task.py src/refresh_task/executor.py src/refresh_task/display_pipeline.py tests/plugins/test_ai_image.py tests/unit/test_refresh_task_display_pipeline.py tests/unit/test_refresh_task_executor.pypython3 -m py_compile src/plugins/ai_image/ai_image.py src/utils/plugin_errors.py src/refresh_task/worker.py src/blueprints/plugin.py src/refresh_task/task.py src/refresh_task/executor.py src/refresh_task/display_pipeline.pynode --check src/static/scripts/plugin_form.jsgit diff --checkSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests