fix(cl2k): extract mixed white+coloured titles in subject mode - #477
Conversation
Subject mode's backdrop palette was poisoned by the title's own colour: distressed grunge spray bleeds title colour into the ring just outside the brush, so letter bodies keyed as backdrop (moth-eaten coloured words, white words hollowed to outlines). Mixed titles had no working mode at all - the white key drops coloured words and the colour key dropped white ones. - _drop_bleed: near-ring palette clusters must have colour support in a far ring (40-80px out). Spray hugs the glyphs; real backdrop continues outward. Fail-safe: keeps all clusters when the far ring is too small or nothing validates. - _white_union_alpha: subject mode unions in the brightness key for the white words, guarded by border-spill rejection (bright content flowing across the brush border is backdrop) and a coverage cap (a union over 60% of the brush is a pale field, rejected wholesale). Both guards fail toward the previous behaviour, never past it.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughLogo extraction now filters backdrop bleed in Lab space and preserves white and pale portions of mixed titles. Text-zone filtering, updated instructions, and expanded tests cover spill rejection, fail-safe behavior, and mask tightening. ChangesLogo extraction
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
backend/util/cl2k/logo_extract.py (1)
212-220: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce helper documentation to behavior and operational gotchas.
Both docstrings contain extended implementation rationale. Keep each to one or two lines that state behavior and failure handling.
backend/util/cl2k/logo_extract.py#L212-L220: reduce_drop_bleeddocumentation to unsupported near-ring cluster removal and its fallback.backend/util/cl2k/logo_extract.py#L279-L295: reduce_white_union_alphadocumentation to white-alpha union plus spill and coverage guards.As per path instructions, comments must be “navigational/instructional only (1-2 line what/gotcha), no why/history essays; match existing density.”
🤖 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 `@backend/util/cl2k/logo_extract.py` around lines 212 - 220, The docstring for _drop_bleed in backend/util/cl2k/logo_extract.py:212-220 should be reduced to one or two lines describing removal of near-ring clusters lacking far-ring support and the fallback that preserves all clusters when validation is insufficient. Also reduce the _white_union_alpha docstring at backend/util/cl2k/logo_extract.py:279-295 to one or two lines describing white-alpha union behavior and its spill and coverage guards; remove extended rationale and history from both.Source: Path instructions
🤖 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 `@backend/util/cl2k/logo_extract.py`:
- Around line 310-316: Remove the fixed _SPILL_MAX_ITERS bound from the spill
propagation loop in extract_subject_logo, and continue expanding spill until
grown and spill have equal coverage. If a bounded loop is necessary, derive its
limit from mask.shape rather than a constant; also add a regression test using a
large mask to verify connected backdrop spill is fully removed.
---
Nitpick comments:
In `@backend/util/cl2k/logo_extract.py`:
- Around line 212-220: The docstring for _drop_bleed in
backend/util/cl2k/logo_extract.py:212-220 should be reduced to one or two lines
describing removal of near-ring clusters lacking far-ring support and the
fallback that preserves all clusters when validation is insufficient. Also
reduce the _white_union_alpha docstring at
backend/util/cl2k/logo_extract.py:279-295 to one or two lines describing
white-alpha union behavior and its spill and coverage guards; remove extended
rationale and history from both.
🪄 Autofix
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
Run ID: 4a7fe2df-2e7d-4a8d-9095-9d6aa587d9c1
📒 Files selected for processing (2)
backend/util/cl2k/logo_extract.pytests/test_cl2k_logo_extract.py
Extends the mixed-title extraction fix across the remaining gaps: - tighten_text_mask's _outside_background gets the same far-ring bleed guard as extraction, so title spray outside the erase block no longer aborts mask tightening (shared _far_ring_palette helper). - The spill flood's iteration cap is now derived from the mask size - a fixed 64-pass cap left connected backdrop deeper than ~256px alive inside a tall brush. Far-ring validation also requires the far ring to be at least half the near ring's size: a brush close to the frame edge leaves a one-sided far ring that wrongly condemned colours legitimately present near the other sides. - _anchor_rescue_alpha: a fat vivid word could pull the Otsu band-fit high enough that a pale muted word landed under lo and vanished (below the white-key floor, so the union could not catch it either). Substantial brushed clusters clearly separated from the backdrop palette now get their own key band, clamped to 0.6x their backdrop distance, with the same border-spill and coverage guards. - UI: the mode is labelled 'Coloured / mixed title' and the brush hint no longer warns users away from brushing the whole title. Validated against real posters (Kevin Bridges, Squid Game, Life and Dirt): tight-brush Squid Game went from losing every white letter to the complete title; Life and Dirt is pixel-identical; ring sampling is ~7x faster with _dilate replacing MaxFilter.
Content the colour key can't tell from title - a smudge of scene art, cars, a football - keys in whenever it sits far from the ring palette. _text_zone_filter keeps only keyed components connected to a detected text line (dilated for shapes, underlines and plates); isolated scene junk drops. Fail-safe on every edge: no detector, no boxes, or a keep under half the keyed area leaves the alpha untouched. Also trims the _drop_bleed/_white_union_alpha/_anchor_rescue_alpha docstrings to the 1-2 line house style. Validated on the same real posters: Kevin Bridges loses the football and city fragments; Squid Game with per-word brush strokes loses the doorway smear; Life and Dirt drops a stray streak and keeps the full title.
|
Round 2/3 summary for the incremental review: 4882d84 extends the far-ring bleed guard to the erase path (_outside_background), derives the spill-flood bound from the mask size (per the inline finding), distrusts one-sided far rings near frame edges, and adds per-anchor rescue for pale words the band-fit drops. 73bf492 adds a detector-gated zone filter that drops keyed scene junk not connected to any detected text line (fail-safe when the detector is absent or misses the wordmark), and trims the helper docstrings flagged in the nitpick to house style. All changes validated against real posters (Kevin Bridges, Squid Game, Life and Dirt) plus 11 new tests; backend suite 1640 green. |
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/test_cl2k_logo_extract.py (2)
337-337: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the gap probe to the centre of the gap.
Strokes occupy x 90..102 and x 135..147, so the gap is x 103..134.
growismax(2, round(0.004 * 500)), which is 2, so the dilated stroke edge reaches x 133. The probe at x=130 clears it by 3 px. A probe at x=118 sits at the gap centre and keeps the assertion meaningful ifgrowchanges.🧪 Proposed change
- assert m.getpixel((130, 150)) < 60 # gap between strokes -> keep + assert m.getpixel((118, 150)) < 60 # gap between strokes -> keep🤖 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 `@tests/test_cl2k_logo_extract.py` at line 337, Update the pixel probe in the gap assertion of the logo extraction test to use x=118, the center of the x=103..134 gap, while preserving the existing y-coordinate and threshold.
237-243: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive the brush size from the poster instead of repeating
(400, 300).
_junk_posterowns the image size. Lines 255, 266 and 284 repeat(400, 300)literally. If_junk_posterchanges size, those three tests fail with a mask-size mismatch rather than a clear signal. Return the image and readimg.size, astest_subject_rescues_a_pale_word_next_to_a_vivid_onedoes at line 226.♻️ Proposed refactor
- out = extract_subject_logo(_jpeg(_junk_poster()), _brush((400, 300), (40, 70, 360, 270))) + img = _junk_poster() + out = extract_subject_logo(_jpeg(img), _brush(img.size, (40, 70, 360, 270)))Also applies to: 255-255
🤖 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 `@tests/test_cl2k_logo_extract.py` around lines 237 - 243, Update _junk_poster and the tests that use it so the brush size is derived from the returned image’s img.size rather than repeating (400, 300). Return the image from _junk_poster in the same manner as test_subject_rescues_a_pale_word_next_to_a_vivid_one, and use that size for the affected brush constructions.backend/util/cl2k/logo_extract.py (3)
316-319: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
_border_spillto reflect its second, non-border use.
_text_zone_filtercalls_border_spill(keyed, keyed & zone)at line 394 to grow a KEEP region from detected text lines. That call has nothing to do with a brush border or with spill. The function is a generic geodesic flood ofseedthroughadd. A name such as_geodesic_floodand a docstring that states the generic contract would prevent a reader from assuming a border relationship at the_text_zone_filtercall site.🤖 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 `@backend/util/cl2k/logo_extract.py` around lines 316 - 319, Rename _border_spill to a generic name such as _geodesic_flood, update its docstring to describe flooding seed through add without border-specific terminology, and update every call site including _text_zone_filter. Preserve the existing flood behavior and parameters.
355-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
33.0tolerance cap into a module constant.Line 355 hardcodes
33.0as the tolerance cap._detect_anchorsapplies the same cap at line 659 through itscolor_tolparameter, whose default is also33.0. The docstring at line 333 states that this band matches_detect_anchors. If thecolor_toldefault changes, the two paths drift and the docstring becomes wrong, with no test that detects it.♻️ Proposed refactor
+# Cap on any per-anchor colour key band (see _detect_anchors, _anchor_rescue_alpha). +_COLOR_TOL_MAX = 33.0- tol = min(33.0, max(_BG_SAME, 0.6 * d)) + tol = min(_COLOR_TOL_MAX, max(_BG_SAME, 0.6 * d))Then use
_COLOR_TOL_MAXas thecolor_toldefault intighten_text_mask.🤖 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 `@backend/util/cl2k/logo_extract.py` at line 355, Define a module-level _COLOR_TOL_MAX constant with value 33.0, replace the hardcoded cap in the tolerance calculation near _detect_anchors, and use the same constant as the color_tol default in tighten_text_mask so both paths and the documented matching band remain synchronized.
221-227: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the 40 px dilation in
_far_ring_palette. Compute the 80 px dilation from the 40 px result. This preserves the square-dilation result and reduces each call from 484 to 324 full-array passes on images up to_MAX_SIDE(3000 px).🤖 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 `@backend/util/cl2k/logo_extract.py` around lines 221 - 227, Update _far_ring_palette to compute the 40 px dilation once, reuse it when deriving the 80 px dilation, and preserve the existing far-mask, filtering, clustering, and return behavior while avoiding a second independent 40 px dilation pass.
🤖 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 `@tests/test_cl2k_logo_extract.py`:
- Around line 271-286: Update
test_subject_zone_filter_distrusts_a_partial_detection so the keyed title
content consists of disconnected bars or pieces, with the mocked detection
covering only a minority piece. Position them far enough apart to remain
disconnected after _dilate(keep, 2) and the 12-pixel zone dilation, ensuring
_text_zone_filter’s _ZONE_MIN_KEEP early return is exercised.
---
Nitpick comments:
In `@backend/util/cl2k/logo_extract.py`:
- Around line 316-319: Rename _border_spill to a generic name such as
_geodesic_flood, update its docstring to describe flooding seed through add
without border-specific terminology, and update every call site including
_text_zone_filter. Preserve the existing flood behavior and parameters.
- Line 355: Define a module-level _COLOR_TOL_MAX constant with value 33.0,
replace the hardcoded cap in the tolerance calculation near _detect_anchors, and
use the same constant as the color_tol default in tighten_text_mask so both
paths and the documented matching band remain synchronized.
- Around line 221-227: Update _far_ring_palette to compute the 40 px dilation
once, reuse it when deriving the 80 px dilation, and preserve the existing
far-mask, filtering, clustering, and return behavior while avoiding a second
independent 40 px dilation pass.
In `@tests/test_cl2k_logo_extract.py`:
- Line 337: Update the pixel probe in the gap assertion of the logo extraction
test to use x=118, the center of the x=103..134 gap, while preserving the
existing y-coordinate and threshold.
- Around line 237-243: Update _junk_poster and the tests that use it so the
brush size is derived from the returned image’s img.size rather than repeating
(400, 300). Return the image from _junk_poster in the same manner as
test_subject_rescues_a_pale_word_next_to_a_vivid_one, and use that size for the
affected brush constructions.
🪄 Autofix
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
Run ID: 2d10e7d2-b6e0-4420-9492-23c02b4fb13b
📒 Files selected for processing (3)
backend/util/cl2k/logo_extract.pyfrontend/src/pages/poster/Cl2kMakerPage.jsxtests/test_cl2k_logo_extract.py
…view nitpicks The tall-title fixture was one connected rectangle, so the flood kept everything and the _ZONE_MIN_KEEP early-return never fired - the test passed on connectivity, not the guard. Disconnected bars with the box over the minority piece now gate it (verified: disabling the guard fails the test). Nitpicks from the same review: _border_spill renamed _geodesic_flood (its zone-filter use has nothing to do with borders), the 33.0 anchor band cap extracted to _COLOR_TOL_MAX and shared with tighten_text_mask, _far_ring_palette reuses the 40px dilation, junk-poster tests derive the brush size from the image, and the tighten gap probe moved to the gap centre.
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
|
1 participant
Problem
Subject-mode ("Coloured title") extraction fails on titles that mix white and coloured words, and on distressed titles generally. Repro: TMDB poster
w1280/3oAVM6LUT75uKalleQlBySABsKR.jpg(Kevin Bridges: In Search of the Beautiful Game — white words + yellow GAME):_background_colorsputs a yellow cluster in the backdrop palette and the letter bodies key as backdrop. Knife-edge sensitive: moving the brush ~10px flips between clean and ruined.Fix
Two changes in
backend/util/cl2k/logo_extract.py, both confined to subject mode:_drop_bleed— near-ring palette clusters must have colour support in a far ring 40–80px out (ΔE ≤ 14). Spray hugs the glyphs; real backdrop continues outward. On the repro the yellow bleed cluster fails by ΔE 42 while genuine backdrop validates within 14. Fail-safe: keeps all clusters when the far ring is too small or nothing validates._white_union_alpha— subject mode unions in the same min-channel brightness keyextract_title_logouses, so the white words of a mixed title extract in the same pass (colours stay original; the downstream whiten still two-tones). Two guards keep pale backdrop out:Rejected approaches, for context: a min-fraction gate on ring clusters (the bleed reaches 17–24% of the ring — not a sliver), erosion-based cluster solidity (k-means dithers textured backdrop into speckle while grunge blobs are chunky — the signal inverts), and per-pixel ΔE suppression of the union (80% of legitimate white letters sit within ΔE 8 of a genuine white backdrop cluster). The discriminators that survive testing are geometric: far-ring support and border connectivity.
Multicolour titles (white + red + yellow + green) work by the same mechanism — the colour key accepts any hue far from the backdrop palette — and are covered by a new test. Still out of scope: title colour sitting on near-identical backdrop colour, which is what diff-mode (AI erase) is for.
Verification
tests/test_cl2k_logo_extract.py.ruff check .clean.Summary by CodeRabbit
Bug Fixes
Documentation
Tests