feat: refresh buttons, bubble controls, inner boundary + per-bubble studio actions - #7
Merged
Conversation
…tudio actions
Features bundled in this PR:
1. Portal refresh buttons (Jobs + Library pages) — Re-detect, Re-translate,
Re-render toolbar actions with loading spinners and error banners.
2. Bubble inner-boundary refinement — server-side crop erosion via
BubbleDetectionService; frontend Pad ± stepper stored in localStorage.
3. Extension push — completed job result image pushed to extension via
OcrRoutes; background.ts / content.ts updated to receive and display it.
4. Per-bubble studio controls (re-OCR, re-translate, re-inpaint, re-patch)
— TypesettingService: WhiteFillBubble, RenderOneBubble, optional
fontFamily/fontSizeOverride on BubbleTranslation
— PageTranslationService: ReocrBubbleAsync, RetranslateBubbleAsync,
ReinpaintBubbleAsync, RepatchBubbleAsync
— PortalRoutes: 4 new POST endpoints per bubble; UpdateBubbleRequest
extended with font fields
— EF migration AddBubbleFontSettings (FontFamily TEXT, FontSizeOverride INTEGER)
— BubbleEditor: Render Style section (font family + size), Bubble Actions
2×2 grid with per-button loading states
— StudioPage: per-bubble handlers wired up; result URL cache-busted via
version signal after reinpaint/repatch
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds Jobs and Library refresh controls, refined bubble detection, persistent bubble rendering settings with per-bubble actions, database support for font overrides, and tracked OCR job result delivery from the server to the browser extension. ChangesFeature updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExtensionBackground
participant OcrRoutes
participant PageTranslationWorker
participant ExtensionContent
ExtensionBackground->>OcrRoutes: POST /ocr with track_job
OcrRoutes->>PageTranslationWorker: enqueue translation job
ExtensionBackground->>OcrRoutes: poll job status
OcrRoutes-->>ExtensionBackground: return completed result image
ExtensionBackground->>ExtensionContent: send job-result-ready data URL
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/src/Services/PageTranslationService.cs (1)
240-257: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSerialize per-job result-image mutations.
Concurrent rerender/reinpaint/repatch requests can read the same
result.png, modify different copies, and let the last write silently discard the other update. Guard each job’s full read-modify-write sequence with a shared keyed async lock.Also applies to: 347-356, 383-391
🤖 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 `@server/src/Services/PageTranslationService.cs` around lines 240 - 257, The result-image read/render/write sequence in the surrounding PageTranslationService method must be serialized per job to prevent concurrent updates from overwriting one another. Add or reuse a shared keyed async lock keyed by jobId, and acquire it before reading originalPath through writing resultPath; apply the same guard to the related rerender/reinpaint/repatch sequences, releasing it reliably after each operation.server/ClientApp/src/pages/StudioPage.tsx (1)
222-234: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
handleRerenderdoesn't bust the result image cache.Re-render regenerates the result file at the same URL, but
resultImageVersionis only incremented by reinpaint/repatch, so the canvas keeps showing the stale cached image until another per-bubble action runs.🐛 Proposed fix
await rerenderJob(params.id, bubblePadding()); await pollUntilDone(); await refetchBubbles(); + setResultImageVersion((v) => v + 1);🤖 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 `@server/ClientApp/src/pages/StudioPage.tsx` around lines 222 - 234, Update handleRerender to increment resultImageVersion after the rerender completes and before refetching or displaying the updated bubbles, ensuring the regenerated result image URL bypasses the browser cache. Leave the existing error and loading-state handling unchanged.
🧹 Nitpick comments (3)
server/src/Routes/OcrRoutes.cs (1)
86-113: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueJob status/result endpoints are unauthenticated and live in the OCR route group.
Two notes:
GET /jobs/{id}/statusand/jobs/{id}/result-imageexpose translated page images to any origin (default CORS, no auth) keyed only by an opaque id — confirm that matches the intended threat model for this server.- These are a distinct endpoint group; consider a dedicated route extension class per the route-organization guideline.
As per coding guidelines, "Register services and API routes through
ServiceExtensions.cs, using one route extension class per endpoint group undersrc/Routes/."🤖 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 `@server/src/Routes/OcrRoutes.cs` around lines 86 - 113, Review the unauthenticated access model for the /jobs/{id}/status and /jobs/{id}/result-image endpoints and add the required authorization or access restrictions if opaque job IDs are not sufficient for the intended threat model. Move these endpoints out of the OCR route registration into a dedicated route extension class under Routes, and register that group through ServiceExtensions.cs while preserving their existing behavior.Source: Coding guidelines
server/ClientApp/src/components/BubbleCanvas.tsx (1)
427-430: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResize handles no longer sit on the drawn rect when
bubblePadding > 0.
svgRect()is inset bypad, but the handle positions (Line 499) still derive from the un-paddedrect(), so handles detach from the visible outline. Consider deriving handle positions from the same padded rect for visual consistency (drag math can keep using the true bounds).🤖 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 `@server/ClientApp/src/components/BubbleCanvas.tsx` around lines 427 - 430, Update the handle-position calculations near the handle rendering in BubbleCanvas so they use the same padded SVG rectangle produced by svgRect(), rather than the unpadded rect() bounds. Keep the true bounds used by drag calculations unchanged, and ensure handles remain aligned with the visible outline when bubblePadding is positive.extension/src/content.ts (1)
57-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDrop the cast and use
jobIdto avoid attaching stale results.The
ToContentMsgunion already narrows onmsg.type, somsg as JobResultReadyMsgis unnecessary and hides future type drift. Also,appendJobImageappends unconditionally: if the user re-scanned since the job started, the image lands on a panel for a different selection, and a repeat message would append a second image. Passingmsg.jobIdand replacing any existing.socr-job-image(or ignoring results for a superseded scan) makes this deterministic.♻️ Suggested change
- else if (msg.type === "job-result-ready") appendJobImage((msg as JobResultReadyMsg).resultImageDataUrl); + else if (msg.type === "job-result-ready") appendJobImage(msg.resultImageDataUrl);function appendJobImage(resultImageDataUrl: string): void { if (!resultPanelEl) return; const inner = resultPanelEl.querySelector<HTMLElement>(".socr-panel-inner"); if (!inner) return; - const section = document.createElement("div"); + inner.querySelector(".socr-job-image")?.remove(); + const section = document.createElement("div");The
JobResultReadyMsgimport can then be dropped if unused elsewhere.Also applies to: 311-322
🤖 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 `@extension/src/content.ts` at line 57, Update the “job-result-ready” handling in the content message flow to use the narrowed message directly, remove the unnecessary JobResultReadyMsg cast and import if unused, and pass msg.jobId into appendJobImage. Make appendJobImage replace an existing .socr-job-image or ignore results whose jobId no longer matches the active scan, preventing stale or duplicate images.Source: Linters/SAST tools
🤖 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 `@docs/feature-plan-next.md`:
- Around line 121-130: Update both fenced code blocks in the job events
documentation near the GET endpoint and emitted event examples to specify the
text language, changing each opening fence to ```text while preserving all
example content.
In `@extension/src/background.ts`:
- Around line 202-235: Replace the in-memory timer loop in pollJobResult with
persisted pending-job state in session storage, including jobId, serverUrl,
tabId, and the existing deadline/scope, and schedule recurring chrome.alarms
polling so MV3 worker termination can resume it. Add alarm startup handling that
reloads pending jobs and invokes pollJobResult, remove the recursive sleep-based
scheduling, clear persisted state and the alarm on completion or timeout, and
tolerate a small bounded number of non-OK responses or thrown network errors
before aborting.
In `@server/ClientApp/src/components/BubbleEditor.tsx`:
- Around line 267-284: Clamp the parsed value in the fontSizeOverride input
handlers to the declared 0–72 range before updating state or calling
props.onUpdate. Update the onInput and onChange logic in the visible input so
typed values such as negatives or values above 72 are normalized and only the
clamped value is persisted.
In `@server/ClientApp/src/pages/JobsListPage.tsx`:
- Around line 170-176: Add explicit accessible names to both icon-only refresh
buttons: in server/ClientApp/src/pages/JobsListPage.tsx lines 170-176, add
aria-label="Refresh jobs"; in server/ClientApp/src/pages/LibraryPage.tsx lines
183-190, add aria-label="Refresh library".
- Around line 170-176: Update the refresh button in JobsListPage so it is
disabled whenever jobs.loading is true, while preserving the existing refetch
handler and loading spinner behavior. Match the disabled-button handling used by
LibraryPage.
In `@server/ClientApp/src/pages/StudioPage.tsx`:
- Around line 66-68: Update the bubblePadding signal initialization to validate
the parsed localStorage value and fall back to a finite numeric default when
parsing yields NaN or another invalid value. Keep the existing
"studio-bubble-padding" storage key and ensure the resulting signal value
remains safe for BubbleCanvas attributes and rerender/reinpaint/repatch request
payloads.
In `@server/src/Routes/OcrRoutes.cs`:
- Around line 61-81: Replace the fire-and-forget Task.Run block in the tracked
OCR path with an InferenceQueue job handled by InferenceWorker, ensuring the
translation pipeline executes under the queue’s bounded concurrency. Have the
route enqueue the job and await its completion, while preserving captured job
data and MarkJobFailedAsync error handling; do not run TranslatePageAsync
directly from the route.
In `@server/src/Services/BubbleDetectionService.cs`:
- Line 172: Reorder the candidate-processing flow in BubbleDetectionService so
raw BubbleBox candidates undergo non-maximum suppression before
RefineToInnerBoundary allocates grids and performs flood-fill refinement. Apply
this to all noted candidate paths, then refine only surviving boxes and preserve
any existing post-refinement suppression if required.
---
Outside diff comments:
In `@server/ClientApp/src/pages/StudioPage.tsx`:
- Around line 222-234: Update handleRerender to increment resultImageVersion
after the rerender completes and before refetching or displaying the updated
bubbles, ensuring the regenerated result image URL bypasses the browser cache.
Leave the existing error and loading-state handling unchanged.
In `@server/src/Services/PageTranslationService.cs`:
- Around line 240-257: The result-image read/render/write sequence in the
surrounding PageTranslationService method must be serialized per job to prevent
concurrent updates from overwriting one another. Add or reuse a shared keyed
async lock keyed by jobId, and acquire it before reading originalPath through
writing resultPath; apply the same guard to the related
rerender/reinpaint/repatch sequences, releasing it reliably after each
operation.
---
Nitpick comments:
In `@extension/src/content.ts`:
- Line 57: Update the “job-result-ready” handling in the content message flow to
use the narrowed message directly, remove the unnecessary JobResultReadyMsg cast
and import if unused, and pass msg.jobId into appendJobImage. Make
appendJobImage replace an existing .socr-job-image or ignore results whose jobId
no longer matches the active scan, preventing stale or duplicate images.
In `@server/ClientApp/src/components/BubbleCanvas.tsx`:
- Around line 427-430: Update the handle-position calculations near the handle
rendering in BubbleCanvas so they use the same padded SVG rectangle produced by
svgRect(), rather than the unpadded rect() bounds. Keep the true bounds used by
drag calculations unchanged, and ensure handles remain aligned with the visible
outline when bubblePadding is positive.
In `@server/src/Routes/OcrRoutes.cs`:
- Around line 86-113: Review the unauthenticated access model for the
/jobs/{id}/status and /jobs/{id}/result-image endpoints and add the required
authorization or access restrictions if opaque job IDs are not sufficient for
the intended threat model. Move these endpoints out of the OCR route
registration into a dedicated route extension class under Routes, and register
that group through ServiceExtensions.cs while preserving their existing
behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bb76e9e6-3db9-4c7a-8c88-c140dafd145d
⛔ Files ignored due to path filters (2)
server/wwwroot/js/app.cssis excluded by!server/wwwroot/js/app.cssserver/wwwroot/js/app.jsis excluded by!server/wwwroot/js/app.js
📒 Files selected for processing (25)
docs/feature-plan-next.mdextension/package.jsonextension/src/background.tsextension/src/content.tsextension/src/types.tsextension/static/content.cssextension/static/manifest.jsonserver/ClientApp/src/api.tsserver/ClientApp/src/components/BubbleCanvas.tsxserver/ClientApp/src/components/BubbleEditor.tsxserver/ClientApp/src/pages/JobsListPage.tsxserver/ClientApp/src/pages/LibraryPage.tsxserver/ClientApp/src/pages/StudioPage.tsxserver/ClientApp/src/types.tsserver/Migrations/20260727000001_AddBubbleFontSettings.Designer.csserver/Migrations/20260727000001_AddBubbleFontSettings.csserver/Migrations/AppDbContextModelSnapshot.csserver/Models/RequestModels.csserver/Models/ResponseModels.csserver/src/Data/AppDbContext.csserver/src/Routes/OcrRoutes.csserver/src/Routes/PortalRoutes.csserver/src/Services/BubbleDetectionService.csserver/src/Services/PageTranslationService.csserver/src/Services/TypesettingService.cs
- PageTranslationService: add per-job SemaphoreSlim keyed lock (GetImageLock) to serialize concurrent rerender/reinpaint/repatch read-modify-write operations on result.png - StudioPage: increment resultImageVersion after handleRerender so full re-render also cache-busts the canvas result image URL - BubbleCanvas: resize handles now derive positions from svgRect() (padded visual rect in SVG coords) instead of rect() + toSvg(), so handles stay aligned with the visible outline when bubblePadding > 0 - BubbleEditor: clamp fontSizeOverride to 0-72 in both onInput and onChange handlers to prevent out-of-range values being persisted - content.ts: remove redundant JobResultReadyMsg cast (type is already narrowed by the if-check); appendJobImage now removes any existing .socr-job-image before inserting, preventing duplicate result panels Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
- StudioPage: NaN guard for bubblePadding localStorage init - JobsListPage: aria-label + disabled state on refresh button - LibraryPage: aria-label on refresh button - docs: add language specifiers to bare fenced code blocks - BubbleDetectionService: run NMS before RefineToInnerBoundary - background.ts: replace in-memory poll loop with chrome.alarms - OcrRoutes: replace Task.Run fire-and-forget with PageTranslationQueue - Add PageTranslationQueue + PageTranslationWorker BackgroundService - manifest.json: add alarms permission Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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 (4)
server/src/Services/BubbleDetectionService.cs (2)
297-323: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBound and cancel the flood fill.
RefineToInnerBoundaryallocatesbool[w,h]and visits every pixel inside each surviving box; large decoded pages can make bubble detection CPU- and memory-intensive. Add an image/box-area cap with a safe fallback, and pass the existing task cancellation token throughDetect/RefineToInnerBoundaryso cancellation can be respected during refinement.🤖 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 `@server/src/Services/BubbleDetectionService.cs` around lines 297 - 323, Bound the flood fill in RefineToInnerBoundary with an image/box-area threshold and return the existing safe fallback when the cap is exceeded, avoiding unbounded bool[w,h] allocation or traversal. Thread the existing cancellation token from Detect into RefineToInnerBoundary and check it during initialization and the queue-processing loop, propagating cancellation promptly without changing normal refinement behavior.
325-331: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the inclusive flood-fill bounds.
maxX/maxYtrack inclusive pixel coordinates, whileBubbleBox.Widthis the number of pixels to crop. After the 2 px inset,maxX - minX/maxY - minYmakes every refined box one pixel narrower in each dimension, so compensate the width/height for the inclusive end bounds.Proposed fix
- return new BubbleBox(minX, minY, maxX - minX, maxY - minY, box.Confidence); + return new BubbleBox(minX, minY, maxX - minX + 1, maxY - minY + 1, box.Confidence);🤖 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 `@server/src/Services/BubbleDetectionService.cs` around lines 325 - 331, Update the refined BubbleBox construction after the EdgeInset bounds validation to account for inclusive maxX/maxY coordinates: calculate width and height with the inclusive endpoint adjustment so the cropped dimensions retain both boundary pixels, while leaving the fallback return box unchanged.server/src/Routes/OcrRoutes.cs (2)
104-110: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
JpegToPngsilently returns non-PNG bytes on decode failure.When
SKBitmap.Decodefails, the method returns the original JPEG bytes but the caller (Line 65) still labels themPngBytesand forwards them intoPageTranslationItem. Downstream,PageTranslationService.TranslatePageAsync/TypesettingService.RenderTranslationsname the parameterimagePng, implying a PNG is expected — passing mismatched-format bytes through the pipeline risks silent corruption or an obscure failure instead of a clear, attributable one.🐛 Proposed fix
private static byte[] JpegToPng(byte[] jpeg) { using var bmp = SKBitmap.Decode(jpeg); - if (bmp is null) return jpeg; + if (bmp is null) throw new InvalidOperationException("Failed to decode image for page-translation job"); using var imgData = bmp.Encode(SKEncodedImageFormat.Png, 100); return imgData.ToArray(); }🤖 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 `@server/src/Routes/OcrRoutes.cs` around lines 104 - 110, Update JpegToPng so SKBitmap.Decode failure does not return the original JPEG bytes; instead, fail explicitly with an attributable exception or propagate a clear conversion failure. Preserve the existing PNG encoding path for successfully decoded bitmaps and ensure the caller does not forward non-PNG data as PngBytes.
60-67: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winIsolate the job-tracking enqueue from the primary OCR response.
The
translationQueue.Writer.WriteAsync(..., ct)call is inline in the request path, unguarded by a try/catch, and the channel is bounded (capacity 32,FullMode = WaitperPageTranslationQueue.cs). Two consequences:
- If the write throws (client cancels
ct, or the channel is somehow closed), the whole/ocrresponse fails with a 500 even though the OCR result was already successfully computed — a secondary, best-effort feature (extension result push) taking down the primary response.- If the channel is full (worker still draining a prior full OCR→translate→typeset pipeline), this await blocks the response until a slot frees, coupling ordinary OCR latency to an unrelated background feature's backpressure.
Wrap the enqueue in a try/catch (returning
jobId = nullon failure) and considerTryWritewith a fallback instead of blocking indefinitely on a full channel.🛡️ Proposed fix
string? jobId = null; if (req.TrackJob == true && boot.IsReady) { jobId = Guid.NewGuid().ToString("N"); var pngBytes = JpegToPng(imageBytes); - await translationQueue.Writer.WriteAsync(new PageTranslationItem(jobId, pngBytes), ct); + try + { + if (!translationQueue.Writer.TryWrite(new PageTranslationItem(jobId, pngBytes))) + jobId = null; // queue full — skip tracking rather than blocking the response + } + catch (Exception ex) + { + logger.LogWarning(ex, "Failed to enqueue page-translation job {JobId}", jobId); + jobId = null; + } }🤖 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 `@server/src/Routes/OcrRoutes.cs` around lines 60 - 67, Isolate the optional enqueue in the OCR route so translation-queue failures cannot fail or delay the primary response. Update the job-tracking block around translationQueue.Writer and PageTranslationItem to use a non-blocking enqueue such as TryWrite, catch enqueue exceptions including cancellation or a closed channel, and reset jobId to null whenever enqueue does not succeed.
🤖 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 `@extension/src/background.ts`:
- Around line 231-283: Add a failure message type alongside JobResultReadyMsg,
then update handlePollAlarm to send a job-result-error notification via
sendToTab with the job identifier before clearing session state on deadline
expiry, server-reported status === "error", and reaching MAX_CONSECUTIVE_ERRORS.
Preserve the existing cleanup and return behavior for each terminal path.
---
Outside diff comments:
In `@server/src/Routes/OcrRoutes.cs`:
- Around line 104-110: Update JpegToPng so SKBitmap.Decode failure does not
return the original JPEG bytes; instead, fail explicitly with an attributable
exception or propagate a clear conversion failure. Preserve the existing PNG
encoding path for successfully decoded bitmaps and ensure the caller does not
forward non-PNG data as PngBytes.
- Around line 60-67: Isolate the optional enqueue in the OCR route so
translation-queue failures cannot fail or delay the primary response. Update the
job-tracking block around translationQueue.Writer and PageTranslationItem to use
a non-blocking enqueue such as TryWrite, catch enqueue exceptions including
cancellation or a closed channel, and reset jobId to null whenever enqueue does
not succeed.
In `@server/src/Services/BubbleDetectionService.cs`:
- Around line 297-323: Bound the flood fill in RefineToInnerBoundary with an
image/box-area threshold and return the existing safe fallback when the cap is
exceeded, avoiding unbounded bool[w,h] allocation or traversal. Thread the
existing cancellation token from Detect into RefineToInnerBoundary and check it
during initialization and the queue-processing loop, propagating cancellation
promptly without changing normal refinement behavior.
- Around line 325-331: Update the refined BubbleBox construction after the
EdgeInset bounds validation to account for inclusive maxX/maxY coordinates:
calculate width and height with the inclusive endpoint adjustment so the cropped
dimensions retain both boundary pixels, while leaving the fallback return box
unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7eb9c551-8c8e-4968-a4ee-35b7df9acf5c
⛔ Files ignored due to path filters (1)
server/wwwroot/js/app.jsis excluded by!server/wwwroot/js/app.js
📒 Files selected for processing (12)
docs/feature-plan-next.mdextension/package.jsonextension/src/background.tsextension/static/manifest.jsonserver/ClientApp/src/pages/JobsListPage.tsxserver/ClientApp/src/pages/LibraryPage.tsxserver/ClientApp/src/pages/StudioPage.tsxserver/src/Routes/OcrRoutes.csserver/src/ServiceExtensions.csserver/src/Services/BubbleDetectionService.csserver/src/Workers/PageTranslationQueue.csserver/src/Workers/PageTranslationWorker.cs
🚧 Files skipped from review as they are similar to previous changes (6)
- extension/package.json
- server/ClientApp/src/pages/JobsListPage.tsx
- server/ClientApp/src/pages/LibraryPage.tsx
- extension/static/manifest.json
- docs/feature-plan-next.md
- server/ClientApp/src/pages/StudioPage.tsx
- extension: add JobResultErrorMsg to ToContentMsg union; handlePollAlarm sends job-result-error on timeout, server-error, and network-error paths - server/OcrRoutes: throw InvalidOperationException on JpegToPng decode failure instead of silently returning corrupt JPEG bytes - server/OcrRoutes: replace blocking WriteAsync with non-blocking TryWrite for the page-translation job queue; catch enqueue failures so a full or closed queue never delays or fails the primary OCR response - server/BubbleDetectionService: add CancellationToken to Detect / DetectRtDetr / DetectYolo / RefineToInnerBoundary; add MaxBfsArea cap (1 M pixels) to RefineToInnerBoundary to prevent unbounded allocation; fix inclusive pixel-count return (maxX - minX + 1, maxY - minY + 1) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
This was referenced Jul 30, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BubbleDetectionService; frontend Pad ± stepper persisted inlocalStorageOcrRoutes;background.ts/content.tsupdated to receive and display itChanges
Server (C#)
TypesettingService:WhiteFillBubble,RenderOneBubble, optionalfontFamily/fontSizeOverrideonBubbleTranslationPageTranslationService:ReocrBubbleAsync,RetranslateBubbleAsync,ReinpaintBubbleAsync,RepatchBubbleAsync; font settings threaded throughRerenderAsyncPortalRoutes: 4 new POST endpoints per bubble (/reocr,/retranslate,/reinpaint,/repatch);UpdateBubbleRequest+ PUT handler extended with font fieldsOcrRoutes: push result image to extension on job completion20260727000001_AddBubbleFontSettings—FontFamily TEXT,FontSizeOverride INTEGERnullable columns onPageTranslationLogsFrontend (SolidJS / Vite)
BubbleEditor: Render Style section (font family dropdown, font size0 = auto-fit); Bubble Actions 2×2 grid with individual spinners and inline error bannerStudioPage: per-bubble handlers;resultUrl()cache-busts result image after reinpaint/repatchJobsListPage/LibraryPage: Re-detect, Re-translate, Re-render toolbar buttonsapi.ts+types.ts:fontFamily/fontSizeOverrideonTranslationBubble; 4 new bubble action functionsExtension (TypeScript)
background.ts/content.ts: receive and display pushed result image from serverTest plan
dotnet ef database update)🤖 Generated with Claude Code
Summary by CodeRabbit