fix: allow silent scan submission#97
Conversation
|
@krishnendu07-code is attempting to deploy a commit to the karan3431's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Review limit reached
More reviews will be available in 34 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Warning
|
| Layer / File(s) | Summary |
|---|---|
Request infrastructure and silent option contract src/lib/api.ts |
Introduces ApiRequestOptions interface, safeFetch wrapper for centralized error handling with optional toast suppression, and rebuilds apiFetch with JSON parsing and auth headers. Response envelope types are exported explicitly, and submitScan signature expands to accept optional options that flow to safeFetch. |
API endpoint methods src/lib/api.ts |
Reformats getLatestScan, getScan, getScanHistory, getGradcam, getMarkets, and getLiveMarkets to use refactored typed helpers while maintaining endpoint paths and parameter defaults. |
Online scan error handling src/lib/api.ts |
Updates scanOnline to parse error details from response body and throw composed messages; preserves TypeError handling that returns null for network offline detection. |
Scanner page integration with silent submissions src/pages/ScannerPage.tsx |
Restructures component state, refs, and camera effects; adds {silent: true} option to offline api.submitScan call. Preserves ONNX pre-warming, frame capture, cloud-then-fallback inference logic, and progress updates. |
Scanner page UI and rendering refactoring src/pages/ScannerPage.tsx |
Reformats result display markup, inference mode badge, freshness score UI, share button handler, and error state layout. Updates class names, grid styling, and status text formatting without changing behaviors or displayed data. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
🐰 A silent scan takes flight,
No toasts to spoil the night,
Offline flows without a peep,
While UI gets a cleanup sweep,
Fresh code, fresh style, all tight! ✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 30.77% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'fix: allow silent scan submission' accurately summarizes the main change: adding silent error handling support for API requests to suppress error toasts. |
| Linked Issues check | ✅ Passed | The PR implements the primary requirement from issue #89: updating submitScan to accept a silent option and suppressing error toasts for background syncs. The bonus task (IndexedDB queue) is not implemented. |
| Out of Scope Changes check | ✅ Passed | All changes in api.ts and ScannerPage.tsx are directly related to implementing silent scan submission and refactoring related request handling. No unrelated out-of-scope changes detected. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
|
I've completed the silent submitScan fix. I have not implemented the offline queue bonus yet, since it introduces additional storage and synchronization logic. Happy to work on it in a separate PR if needed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/pages/ScannerPage.tsx (1)
58-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset failed ONNX initialization instead of leaving
getEngine()permanently stuck.If
loadModels()rejects once,engineLoadingnever goes back tofalse, so every latergetEngine()call waits forever in the polling branch and the offline scan path hangs.Suggested fix
let engineInstance: FishFreshnessInference | null = null; let engineReady = false; let engineLoading = false; +let enginePromise: Promise<FishFreshnessInference> | null = null; async function getEngine(): Promise<FishFreshnessInference> { if (engineReady && engineInstance) return engineInstance; - if (engineLoading) { - await new Promise<void>((resolve) => { - const poll = setInterval(() => { - if (engineReady) { - clearInterval(poll); - resolve(); - } - }, 100); - }); - return engineInstance!; - } - engineLoading = true; - engineInstance = new FishFreshnessInference(); - await engineInstance.loadModels(); - engineReady = true; - engineLoading = false; - return engineInstance; + if (enginePromise) return enginePromise; + + engineLoading = true; + enginePromise = (async () => { + try { + const engine = new FishFreshnessInference(); + await engine.loadModels(); + engineInstance = engine; + engineReady = true; + return engine; + } catch (error) { + engineInstance = null; + engineReady = false; + throw error; + } finally { + engineLoading = false; + enginePromise = null; + } + })(); + + return enginePromise; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ScannerPage.tsx` around lines 58 - 76, getEngine can get permanently stuck if loadModels() rejects because engineLoading stays true and pollers wait for engineReady; wrap the engine initialization in try/catch around engineInstance.loadModels() and on error reset engineLoading = false and engineInstance = null (and rethrow the error) so callers can retry, and also update the polling logic inside the branch that waits for engineReady (the setInterval that checks engineReady) to also break/resolve when engineLoading becomes false so waiting callers don't wait forever; reference getEngine, engineLoading, engineReady, engineInstance, and loadModels in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/api.ts`:
- Around line 56-76: safeFetch currently passes every Response to
handleResponse(res) which still emits toast.error for res.status >= 500, so the
silent flag only suppresses TypeError toasts; modify handleResponse to accept
the request options (or a boolean silent) and respect it when deciding to show
toasts (i.e., do not call toast.error for 5xx when silent is true), update the
signature of handleResponse (e.g., handleResponse(res, { silent?: boolean }) or
handleResponse(res, silent)) and propagate options?.silent from safeFetch into
that call, and update any other callers of handleResponse to pass the new
argument so request-level toasts are fully gated by the silent flag.
In `@src/pages/ScannerPage.tsx`:
- Around line 181-188: The code reuses a stale sessionStorage "lastScanId"
causing the share/copy action to point to an older scan; when beginning a new
scan (the block with setScanPhase("processing"), startProgress(),
setPreviewUrl(...), setInferenceMode(...)) clear or remove the sessionStorage
key for lastScanId (e.g., sessionStorage.removeItem('lastScanId') or set it to
an empty value) so it cannot be reused, and also update submitScan's error path
(the submitScan catch/failure handling) to remove/clear lastScanId on failure;
finally ensure the Grade-A share/copy logic reads the active scan id from
component state (not from sessionStorage) so it never uses a stale lastScanId.
- Around line 294-297: When captureVideoBlob(video) returns falsy you set the
error message via setError but never change the page state, so the error UI (not
rendered while status is "idle") never appears; update the failure branch in the
capture logic (the block after captureVideoBlob) to also switch the page into
the error state by calling the state updater that controls view mode (e.g.,
setStatus("error") or the equivalent setter used in this component) immediately
after setError("Failed to capture frame."), ensuring the error UI will render.
---
Outside diff comments:
In `@src/pages/ScannerPage.tsx`:
- Around line 58-76: getEngine can get permanently stuck if loadModels() rejects
because engineLoading stays true and pollers wait for engineReady; wrap the
engine initialization in try/catch around engineInstance.loadModels() and on
error reset engineLoading = false and engineInstance = null (and rethrow the
error) so callers can retry, and also update the polling logic inside the branch
that waits for engineReady (the setInterval that checks engineReady) to also
break/resolve when engineLoading becomes false so waiting callers don't wait
forever; reference getEngine, engineLoading, engineReady, engineInstance, and
loadModels in your changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cfb495cc-8d35-4f0a-be70-4d70b22bceca
📒 Files selected for processing (2)
src/lib/api.tssrc/pages/ScannerPage.tsx
|
@jpdevhub I'm glad |
2 participants
Fixes #89
Description
Added silent error handling support for background scan submission.
Changes Made
Testing
Note: Full offline scanner flow could not be tested locally because sign-in/auth was unavailable in the local environment.
Summary by CodeRabbit
Refactor
Style