Repository navigation
Fix split view errors and speed up rotation and text search - #12
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes update EPUB PNG encoding settings, image rotation pixel copying, split-mode selection, and OCR search processing. ChangesEPUB PNG Encoding
Image Rotation
Split Mode
OCR Scanning
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The rendering optimizations preserve image handling, split view displays failed book opens, and search processing remains bounded. No actionable merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| // Book text needs no OCR: read it here in batches instead of one message | ||
| // per page, and yield between batches so input and progress stay live. | ||
| next := msg.page + 1 | ||
| if batch == 64 || !p.book.IsTextPage(next) { |
There was a problem hiding this comment.
🟡 Medium ui/ui.go:1394
updateOCR processes 65 contiguous text pages before yielding, so a 65th large page can keep input and progress blocked beyond the intended 64-page responsiveness bound. Because the current message is processed before checking batch == 64, change the threshold to 63.
| if batch == 64 || !p.book.IsTextPage(next) { | |
| if batch == 63 || !p.book.IsTextPage(next) { |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @internal/ui/ui.go around line 1394:
`updateOCR` processes 65 contiguous text pages before yielding, so a 65th large page can keep input and progress blocked beyond the intended 64-page responsiveness bound. Because the current message is processed before checking `batch == 64`, change the threshold to `63`.
Evidence trail:
Reviewed commit 582df77: `internal/ui/ui.go:1369-1398`, especially lines 1391-1397, shows the zero-based batch loop, current-message processing before the `batch == 64` check, and synchronous text-page loading. `internal/ui/ui.go:1329-1357` shows `ocrPage` returning text-page messages and the OCR command chain.
There was a problem hiding this comment.
Fixed in 3f0b979: the counter now starts at 1 and counts pages handled, so an update yields after the 64th page.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Book text needs no OCR: read it here in batches instead of one message | ||
| // per page, and yield between batches so input and progress stay live. | ||
| next := msg.page + 1 | ||
| if batch == 64 || !p.book.IsTextPage(next) { |
There was a problem hiding this comment.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/ui/ui.go
Line: 1394
Comment:
**Search batches can delay input** A text-heavy book can make one update extract and search 65 pages before the reader handles another key or redraws progress. The fixed page count does not limit how long that work takes when pages contain substantial text. Consider yielding after a time or work budget so a dense batch does not stall the interface.
**Knowledge Base Used:**
- [Terminal UI navigation](https://app.greptile.com/looneytunes/-/custom-context/knowledge-base/hawkff/cbzr/-/docs/terminal-ui-navigation.md)
- [OCR integration](https://app.greptile.com/looneytunes/-/custom-context/knowledge-base/hawkff/cbzr/-/docs/ocr-integration.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Page geometry bounds the work here. A laid-out text page is 900×1200 px at 32 px, about 25 lines of 60 characters, so 64 pages is roughly 150 KB of string scanning per update, well under a millisecond. A clock read per page would add cost without a measurable gain. 3f0b979 corrects the count to 64.
There was a problem hiding this comment.
That’s a fair correction. With batch starting at 1 and yielding when it reaches 64, each update handles at most 64 pages (not 65). Given the fixed 900×1200 layout and the resulting bounded text volume, the batch is small enough that a per-page clock check would add overhead without addressing a practical stall. I’m withdrawing the “Search batches can delay input” finding; no time-based yield is needed here.
Show the open error of a second command-line book instead of dropping it. Two paths now always start in split view, so the pane holds the error when its book fails to open.
Rotate pages through RGBA pixel rows instead of per-pixel At and Set calls, which boxed a color for every pixel. Encode laid-out text pages at best speed, matching the other PNG encoders in the pipeline. Scan book text pages in batches of 64 inside one update instead of one message per page, so a text search over a long book finishes in a few updates while input and progress stay live.
Add a rotation test covering all three quarter turns, offset bounds, the RGBA fast path on a sub-image, and a round trip.
Summary by CodeRabbit
Note
Fix split-view opening errors and speed up rotation and text search
ui.Newnow decides split-pane mode by the number of command-line paths, not by whether the second book opened, so a failed second book still shows its error in its pane (ui.go)render.rotatenow uses direct RGBA pixel-buffer copies instead of per-pixel color-interface reads; non-RGBA images are converted once, and quarter-turn mappings are unchanged (transform.go)ui.Model.updateOCRprocesses consecutive text pages in batches of up to 64 per update, requests OCR for non-text pages, and yields between batches while keeping hit tracking and navigation behaviorpng.BestSpeedfor faster serialization (epub_render.go)ui.Newfor that caseMacroscope summarized 3f0b979.
The PR appears safe to merge, though dense text batches can still delay input briefly.
Findings
Fix with agent prompt
Summary
This PR keeps the error pane visible when a second command-line book fails to open, speeds up image rotation and EPUB PNG encoding, and scans text pages in batches during search. The latest changes make batches contain 64 pages and add a test for search progress across batch and image-page boundaries.
Reviews (2) · Last reviewed commit: "Yield text search after 64 pages and tes..."