feat(bbox): grid-driven layout flow, retiring flood-fill #8
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/grid-bbox-flow"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Reworks the layout/OCR stage (Stage 1) into a coarse→fine grid flow that reads coordinates off a pixel-labelled grid instead of flood-fill mask measurement. One method that works identically for speech bubbles, borderless text, and sfx — and geometry is separated from transcription.
scripts/v0_bbox.py— five subcommandsgrid/viewviewwindow is the placement surface (100px snap)refine(fanout)zoomjpenters herefinal--missing; read against raw for completenessPipeline wiring
bbox-locatorreplaceslayout-verifier— it builds the region list from scratch via the grid flow rather than verifying pre-existing regions.SKILL.mdStage 1 rewritten; newbbox-agent-prompt.md(thin task template) +bbox-workflow.md(operational spec, phases 1–5).v0_annotate.pyuntouched — the typesetter still uses--flood, QC still uses--crop. Flood-fill only lost its layout-stage role.regions+layout_verified), so Stages 2–10 need no changes.Why
Flood-fill only worked on clean closed outlines — it leaked on broken bubbles and did nothing for borderless text or sfx, so the agent eyeballed anyway and burned tokens tuning seed/threshold. The grid gives the model a coordinate frame it can actually read.
Worked example — RJ379854 p24 (sfx-heavy, clean-room)
Layered validation caught 3 over-tagged motion marks (culled at refine/zoom) and 1 mislocated sfx (caught by the phase-5 gate) before any reached cleaning, where a false
sfxis destructive.page_0024.jsonis included as the example (layout not finalized — bottom-tier sfx still queued for transcription).Rationale and per-phase learnings are recorded in
LESSONS.md(2026-07-23).🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ A complete rework of the layout stage from flood-fill to a grid-driven coordinate frame! The VLM reads grid labels but can't eyeball raw pixels — that's a genuine insight, and the phased coarse→fine discipline (geometry before text, over-tag-and-cull) is beautifully reasoned. I read every line of the diff, ran every subcommand of
v0_bbox.pyend-to-end on synthetic data, and checked all twelve changed files for stale references. The architecture is clean~ ♡But fufu... I found a crash. You wouldn't leave THIS in the tool an agent drives itself, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
scripts/v0_bbox.py:148-151—cmd_viewcrashes with a raw traceback on out-of-bounds coordinates. When the requested window is fully beyond an image edge, the clampingx1 = min(img.width, x1)/y1 = min(img.height, y1)producesx1 <= x0(ory1 <= y0), andimg.crop((x0, y0, x1, y1))raisesValueError: Coordinate 'right' is less than 'left'. I verified this three ways:This will be hit in production. The workflow doc itself documents the agent misplacing boxes by 100–300px ("placing a box off the overview is how you land it 100–150px off the actual ink"), and
viewis the command that takes arbitrary typed coordinates — unlikerefine/zoomwhich derive their windows from JSON region bboxes. Every other error path in this script (loadwith no raw image,zoomwith no matching region,refinewith no matching ids, unknown subcommand) raisesSystemExitwith a clear message. This one dumps a Python traceback that an opus-tier agent can parse but shouldn't have to.Fix: guard the clamped box for zero/negative area before cropping, same style as the sibling guards:
💡 Little ideas (non-blocking)~
scripts/v0_bbox.py:167-170—cmd_gridcomputes its output path twice.render(...).save(os.path.join(outdir(base), ...))builds the path inline, thendest = os.path.join(outdir(base), ...)rebuilds the identical string for the print.outdir(base)also runs twice (idempotent viaexist_ok=True, but still). The othercmd_*functions computedestonce and reuse it — minor DRY drift. Pull the path intodestfirst, thenrender(...).save(dest).scripts/v0_bbox.py:33— hardcoded Fedora font path.FONT = "/usr/share/fonts/dejavu-sans-fonts/DejaVuSans-Bold.ttf"is Fedora-specific; thefont()fallback toImageFont.load_default()handles other environments gracefully (I verified — the script runs fine in this Debian container via the fallback). But the siblingv0_annotate.py:24usesImageFont.load_default(size=26)with aTypeErrorguard for older Pillow — a more portable pattern that doesn't need a hardcoded path at all. Consider aligning with the sibling for consistency, since the bold DejaVu is mostly cosmetic on a grid preview.✅ What I liked~
git grepforlayout-verifier/layout-agent-promptacross all tracked files returns nothing — the retirement is clean. SKILL.md Stage 1, both agent definitions, both feedback logs, and the prompt template are all consistently rewired tobbox-locator/bbox-agent-prompt/bbox-workflow. Fufu, that's how you retire something~ ♪regions+layout_verified), so Stages 2–10 need zero modifications. I verified schema.md, qc-checklist.md, and all downstream agent definitions — no coupling to the old tool. Clean seam.v0_annotate.pycorrectly untouched — it still owns--flood(typesetter) and--crop(QC). The PR body explicitly scopes the retirement to "flood-fill only lost its layout-stage role." Honest framing.sfxreaching cleaning erases real art); the phase-5 gate as a genuine backstop (not a formality) is proven by the p24 ふー♥ recovery. The LESSONS.md entry records real failures with real fixes — this is how a pipeline matures.bbox-workflow.mddiscipline rules are battle-tested. Each placement rule ("box the glyph ink, not the effect"; "one box owns one thing"; "empty box ≠ phantom — widen before dropping") is tied to a concrete mistake it already made. This is documentation that prevents recurrence, not filler.cmd_*functions, a cleanrender()that handles grid + region overlay + clipping, theload()glob-fallback for bootstrapping a new page. I ran all five subcommands plus every error path — everything that works, works correctly.Automated review by Jibril · 2026-07-23
CI/CD: absent (repo has no CI config — markdown/script PR) · Local checks: Pillow 12.3, all 5 subcommands + 6 error paths exercised on synthetic data
🔮 fufu~ Jibril re-reviewed your code!
Oh? Oh my~ scarlet pushed exactly the fix I asked for, and nothing more~ ♡ One commit, three surgical edits to
scripts/v0_bbox.py, zero scope creep. That's how you respond to a review. Let me confirm each one~Verdict: ✅ Looks good to me~ ♡
✅ All three prior findings — resolved~
cmd_viewout-of-bounds crash — FIXED & VERIFIED. The guard landed atv0_bbox.py:157-159, byte-for-byte the shape I suggested (if x1 <= x0 or y1 <= y0: raise SystemExit(...)). I reproduced the exact crash case from my last review and confirmed it now exits cleanly instead of dumping a traceback:And a valid in-bounds view still renders correctly (
view ... 100 100 400 400→ clean PNG, exit 0). The error message even echoes the post-clamp coordinates AND the image dimensions, so the agent can self-correct in one hop. Fufu, that's a good error message~ ♪cmd_gridDRY drift — FIXED.destis now computed once (v0_bbox.py:147) and reused for both.save(dest)and the print. Matches thedest-once pattern in every siblingcmd_*. Clean.Hardcoded Fedora font path — FIXED.
FONT→FONT_CANDIDATESlist with both Fedora and Debian/Ubuntu paths (v0_bbox.py:47-50), loop tries each viaImageFont.truetype, falls through toload_default(size=size)with theTypeErrorguard for older Pillow preserved. I verified the Debian path resolves here (font(20)→FreeTypeFont, not the bitmap fallback) — so the grid labels now render crisply on more distros without any hardcoding smell.✅ No regressions~
Ran the full subcommand sweep on synthetic data:
grid/view(in-bounds + both out-of-bounds axes) /final/ no-args / unknown-cmd — every path behaves correctly. The diff is +13/-7 in a single file, purely the three reviewed edits. The architectural review from9d9872bstands unchanged (clean retirement oflayout-verifier, unchanged JSON interface,v0_annotate.pyuntouched, superb workflow design).Automated review by Jibril · 2026-07-23
CI/CD: absent (repo has no CI config) · Local checks: Pillow 12.3, blocker repro + full subcommand sweep on synthetic data
Thanks jibril — that
cmd_viewcrash was a real one, and you're right that it's the reachable path:viewis the only subcommand taking arbitrary typed coordinates, and the workflow itself expects the agent to misplace boxes. Fixed in52083ea.⛔ blocking — fixed
cmd_viewnow guards the clamped window and raisesSystemExitwith a clear message (matching the sibling error paths), instead of the raw PILValueError. Verified with your exact repro:💡 non-blocking — both taken
cmd_gridnow computesdestonce and passes it torender(...).save(dest)(no more doubleoutdir/path build).font()now tries the Fedora and Debian DejaVu locations, then falls back to a sized default. Bonus: the old fallback was unsizedload_default(), so labels silently shrank to bitmap-default off Fedora; now they stay sized everywhere.Normal
grid/view/refine/zoom/finalstill run clean, syntax checks pass. Ready for another look when you have a moment~ 🔮