fix(slides): enhance text overflow and occlusion detection in xml lint - #2152
Open
ethan-zhx wants to merge 18 commits into
Open
fix(slides): enhance text overflow and occlusion detection in xml lint#2152ethan-zhx wants to merge 18 commits into
ethan-zhx wants to merge 18 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe slide XML overlap linter adds text measurement, wrapping, overflow, occlusion, container, rotation, stacking-order, and issue-deduplication logic. Regression tests cover Unicode sizing, spacing, auto-fit growth, geometry, z-order, and error severity. ChangesSlide overlap and overflow linting
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@5a1214760e8537da8a8aa300f6196a0f4b04c488🧩 Skill updatenpx skills add larksuite/cli#fix/text_over_flow -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/lark-slides/scripts/xml_text_overlap_lint.py (1)
1954-1972: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAnchor the unclamped auto-fit box to the authored top.
shape-auto-fitnow keepsraw_heighteven when it exceedscontent_height. The vertical anchoring below still runs. With the defaultverticalAlignof"middle"(set inextract_elements),(content_height - visual_height) / 2is negative, so the box is shifted up by half the excess. With"bottom"it is shifted up by the full excess.The renderer grows a
shape-auto-fitbox downward from the authored top, as the docstring ondetect_auto_fit_growth_collisionsstates. The current geometry therefore places the grown region too high:
detect_auto_fit_growth_collisionsmeasuresglyph["y"] + glyph["height"] - authored_bottom, which reports about half of the real downward growth for middle-aligned runs.- The box also extends above the authored top, which can create overlap reports against content sitting above the run.
Skip the vertical re-anchoring when the estimated height exceeds the content box.
🐛 Suggested fix
y = element["y"] + padding_top - if element.get("verticalAlign") == "middle": - y += (content_height - visual_height) / 2 - elif element.get("verticalAlign") == "bottom": - y += content_height - visual_height + # A grown shape-auto-fit box extends downward from the authored top, so alignment + # offsets only apply while the estimated block still fits the content box. + if visual_height <= content_height: + if element.get("verticalAlign") == "middle": + y += (content_height - visual_height) / 2 + elif element.get("verticalAlign") == "bottom": + y += content_height - visual_height🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 1954 - 1972, Update the vertical alignment logic in the text geometry calculation to skip middle/bottom re-anchoring when shape-auto-fit uses a visual_height larger than content_height. Keep the authored top as the y origin for the grown box, while preserving existing vertical alignment behavior for non-grown boxes and auto-fit cases that do not exceed the content height.
🧹 Nitpick comments (3)
skills/lark-slides/scripts/xml_text_overlap_lint.py (3)
1204-1213: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePrecompute the glyph boxes once per text element.
estimate_text_visual_bboxruns per-character width estimation and wrapped line counting. This loop calls it once for every (shape, text) pair, anddetect_auto_fit_growth_collisionsrepeats the same pattern forother_bbox. Build the glyph boxes once before the shape loop.♻️ Suggested change
+ glyph_boxes = { + element["id"]: estimate_text_visual_bbox(element) for element in text_elements + } for shape in covering_shapes: for text_element in text_elements: if not is_drawn_in_front_of(shape, text_element): continue - glyph = estimate_text_visual_bbox(text_element) + glyph = glyph_boxes[text_element["id"]] if glyph is None: continue🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 1204 - 1213, Precompute each text element’s glyph bounding box once before iterating through covering_shapes, then reuse the cached result when checking shape overlap. Apply the same caching approach to the repeated other_bbox estimation in detect_auto_fit_growth_collisions, while preserving the existing handling for None boxes and overlap thresholds.
1102-1102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an area constant for the area gate.
Line 1089 compares
growth(pixels) againstCONTAINER_OVERFLOW_MIN_PX, which is correct. Line 1102 comparesintersection_area(...)(square pixels) against the same constant. The units differ. Both constants are 4.0 today, so behavior is unchanged, but a future tuning of the linear slack would silently move the area gate.♻️ Suggested change
- if intersection_area(grown_region, other_bbox) <= CONTAINER_OVERFLOW_MIN_PX: + if intersection_area(grown_region, other_bbox) <= SHAPE_TEXT_OCCLUSION_MIN_AREA: continue🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` at line 1102, Update the area comparison in the overlap-checking logic around intersection_area to use a dedicated area-threshold constant rather than CONTAINER_OVERFLOW_MIN_PX. Keep CONTAINER_OVERFLOW_MIN_PX for the growth pixel comparison and initialize the new area constant to preserve the current behavior.
1016-1050: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared occluder-scan helper.
detect_chart_text_occlusionsduplicatesdetect_table_text_occlusionsexcept for the element kind, issue code, message, and hint. A single parameterized helper keeps the two detectors in sync when the text filter changes.♻️ Suggested consolidation
def detect_element_text_occlusions( elements: list[dict[str, Any]], kind: str, code: str, noun: str, hint: str ) -> list[dict[str, Any]]: issues: list[dict[str, Any]] = [] text_elements = [ element for element in elements if is_text_element(element) and has_text_content(element) and not is_ghost_text(element) ] occluders = [element for element in elements if element["kind"] == kind and element["alpha"] > 0] for text_element in text_elements: if is_decorative_text(text_element): continue glyph_bbox = estimate_text_visual_bbox(text_element) if glyph_bbox is None: continue for occluder in occluders: if not intersects(occluder, glyph_bbox): continue issues.append({ "level": "error", "code": code, "elements": [occluder["id"], text_element["id"]], "message": f'text shape {text_element["id"]} overlaps {noun} {occluder["id"]}', "hint": hint, }) return issues🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 1016 - 1050, Extract the shared scanning logic from detect_table_text_occlusions and detect_chart_text_occlusions into a parameterized detect_element_text_occlusions helper. Pass the occluder kind, issue code, noun, and hint for each detector, while preserving the existing filtering, intersection checks, and issue output.
🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 1152-1163: Update the container-overflow handling around the
overflow gate to align with the documented scope of CONTAINER_OVERFLOW_MIN_PX:
either apply that tolerance to this authored text-frame check, or revise the
constant’s comment to explicitly state that it only gates
detect_auto_fit_growth_collisions. Preserve the existing tests’ behavior that
reports a 4px frame overhang.
- Around line 2602-2611: Introduce one element-id-keyed glyph-bbox cache scoped
to lint_slide and pass it through the detectors, so each element’s
estimate_text_visual_bbox result is reused. In
skills/lark-slides/scripts/xml_text_overlap_lint.py#L2602-L2611, resolve left
and right glyph boxes from the shared cache before should_flag_overlap instead
of re-estimating per pair. In
skills/lark-slides/scripts/xml_text_overlap_lint.py#L1204-L1213, update
detect_shape_text_occlusions and detect_auto_fit_growth_collisions to read
cached boxes for each shape/text pair and other_bbox, preserving existing
detector behavior.
- Around line 1836-1850: Update the width-wrap filtering loop around the
existing single-line and short-label checks to skip any element recognized by
is_vertical_text, including all accepted vertical-run values. Keep vertical text
out of the width-based overflow calculation while preserving existing filtering
for horizontal text.
- Around line 2516-2525: Update the crossing-box adjustment around the
glyph_bbox handling to skip the vertical re-anchoring when the text run is
rotated, preserving the rotated bounds returned by estimate_text_visual_bbox.
Keep the existing unrotated vertical-align calculations for non-rotated text,
and ensure rotated runs do not mix rotated x/width with recomputed unrotated
y/height.
---
Outside diff comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 1954-1972: Update the vertical alignment logic in the text
geometry calculation to skip middle/bottom re-anchoring when shape-auto-fit uses
a visual_height larger than content_height. Keep the authored top as the y
origin for the grown box, while preserving existing vertical alignment behavior
for non-grown boxes and auto-fit cases that do not exceed the content height.
---
Nitpick comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 1204-1213: Precompute each text element’s glyph bounding box once
before iterating through covering_shapes, then reuse the cached result when
checking shape overlap. Apply the same caching approach to the repeated
other_bbox estimation in detect_auto_fit_growth_collisions, while preserving the
existing handling for None boxes and overlap thresholds.
- Line 1102: Update the area comparison in the overlap-checking logic around
intersection_area to use a dedicated area-threshold constant rather than
CONTAINER_OVERFLOW_MIN_PX. Keep CONTAINER_OVERFLOW_MIN_PX for the growth pixel
comparison and initialize the new area constant to preserve the current
behavior.
- Around line 1016-1050: Extract the shared scanning logic from
detect_table_text_occlusions and detect_chart_text_occlusions into a
parameterized detect_element_text_occlusions helper. Pass the occluder kind,
issue code, noun, and hint for each detector, while preserving the existing
filtering, intersection checks, and issue output.
🪄 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: 239a59f1-92ef-45f9-980b-00a67f3d8699
📒 Files selected for processing (2)
skills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2152 +/- ##
==========================================
+ Coverage 75.70% 75.72% +0.01%
==========================================
Files 944 944
Lines 100288 100355 +67
==========================================
+ Hits 75926 75994 +68
Misses 18565 18565
+ Partials 5797 5796 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ethan-zhx
force-pushed
the
fix/text_over_flow
branch
from
August 3, 2026 12:19
fec80e5 to
ad0651a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
skills/lark-slides/scripts/xml_text_overlap_lint.py (2)
980-1050: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract a shared occluder-versus-text detector.
detect_table_text_occlusionsanddetect_chart_text_occlusionsdiffer only in the element kind, the issue code, the message, and the hint. The filtering, glyph-box estimation, and intersection logic are identical. A future change to the shared logic must be applied twice.♻️ Proposed consolidation
+def detect_element_text_occlusions( + elements: list[dict[str, Any]], kind: str, code: str, hint: str +) -> list[dict[str, Any]]: + issues: list[dict[str, Any]] = [] + text_elements = [ + element + for element in elements + if is_text_element(element) and has_text_content(element) and not is_ghost_text(element) + ] + occluders = [element for element in elements if element["kind"] == kind and element["alpha"] > 0] + for text_element in text_elements: + if is_decorative_text(text_element): + continue + glyph_bbox = estimate_text_visual_bbox(text_element) + if glyph_bbox is None: + continue + for occluder in occluders: + if not intersects(occluder, glyph_bbox): + continue + issues.append({ + "level": "error", + "code": code, + "elements": [occluder["id"], text_element["id"]], + "message": f'text shape {text_element["id"]} overlaps {kind} {occluder["id"]}', + "hint": hint, + }) + return issues
detect_table_text_occlusionsanddetect_chart_text_occlusionsthen become thin wrappers that keep their docstrings and pass the kind, code, and hint.🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 980 - 1050, Extract the duplicated filtering, glyph-bounding-box estimation, and intersection logic from detect_table_text_occlusions and detect_chart_text_occlusions into a shared helper that accepts the occluder kind, issue code, message context, and hint. Convert both existing functions into thin wrappers that preserve their docstrings and pass their table- or chart-specific values while retaining the current issue structure and behavior.
1102-1103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an area constant for the area gate.
Line 1102 compares an intersection area in px² against
CONTAINER_OVERFLOW_MIN_PX, which lines 76-79 document as linear pixels of slack. The neighbouring detector already has an area-scoped constant with the same value,SHAPE_TEXT_OCCLUSION_MIN_AREA. Reuse it here so the units match the comparison.♻️ Proposed change
- if intersection_area(grown_region, other_bbox) <= CONTAINER_OVERFLOW_MIN_PX: + if intersection_area(grown_region, other_bbox) <= SHAPE_TEXT_OCCLUSION_MIN_AREA: continue🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 1102 - 1103, Update the area threshold in the intersection check around intersection_area(grown_region, other_bbox) to use SHAPE_TEXT_OCCLUSION_MIN_AREA instead of CONTAINER_OVERFLOW_MIN_PX, preserving the existing comparison and control flow.
🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint_test.py`:
- Around line 1177-1179: Update the comment above the assertions in
test_lint_xml_reports_wrap_false_text_wider_than_box to remove the claim that
wrap="false" opts a run out; state instead that no-wrap-label is not flagged
because its estimated width exceeds the heuristic risk band but remains within
the exact available-width tolerance, while comfortable fits normally.
---
Nitpick comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 980-1050: Extract the duplicated filtering, glyph-bounding-box
estimation, and intersection logic from detect_table_text_occlusions and
detect_chart_text_occlusions into a shared helper that accepts the occluder
kind, issue code, message context, and hint. Convert both existing functions
into thin wrappers that preserve their docstrings and pass their table- or
chart-specific values while retaining the current issue structure and behavior.
- Around line 1102-1103: Update the area threshold in the intersection check
around intersection_area(grown_region, other_bbox) to use
SHAPE_TEXT_OCCLUSION_MIN_AREA instead of CONTAINER_OVERFLOW_MIN_PX, preserving
the existing comparison and control flow.
🪄 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: d41aff56-d416-4ade-a24b-28ac38aa7fc5
📒 Files selected for processing (2)
skills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
ethan-zhx
changed the title
BD-ZERO
reviewed
Aug 3, 2026
BD-ZERO
reviewed
Aug 3, 2026
1 task
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 1033-1041: Update the innerRadius handling in the visible
chart-radius calculation to enforce the documented 0..1 fraction range: reject
or clamp values above 1 before multiplying by pie_radius, while preserving the
existing behavior for missing or non-positive values. Ensure the resulting hole
radius never exceeds the pie radius used by bbox_within_circle.
🪄 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: 6329e495-306f-4fe3-991f-b8eef8c3ba2d
📒 Files selected for processing (2)
skills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
The order-based skip (`image.order <= text.order`) allowed images that appear before text in XML to silently cover text glyphs. Remove it so any geometric overlap is reported regardless of XML element order. Also update the error hint to no longer suggest reordering XML as a fix, since that no longer works. Add a regression test verifying the new behavior.
The height-only check (text_may_overflow_shape) misses shapes that wrap because their box is too narrow, not too short. Added a new width-axis detector that: - Flags single-line short labels/metrics whose estimated width exceeds the content box (0.85 risk band for latin runs, 1.18 tolerance for plain metrics, exact fit for pure CJK). - Works independently of autoFit (shape-auto-fit only grows height). - Preserves internal whitespace (e.g. "autofix 87%") so spaces are not collapsed away. - Deduplicates with the height check so the same shape is not double-reported under the shared text_may_overflow_shape code. The two axes share code="text_may_overflow_shape" and are distinguished by overflow_axis="height"|"width". Regression test covers all three real false-negative cases (bMP/bMp/bMm) plus negative controls.
Fix missed overflow and occlusion cases in xml_text_overlap_lint: CJK ambiguous-width and percent glyph width estimation, chart-vs-text occlusion, full-canvas background-image exemption, and severity masking in the width/height overflow dedupe. Consolidate the scattered raw paint-order comparisons into is_drawn_behind / is_drawn_in_front_of so stacking direction is decided in one place, with contract tests that turn red if the fixes are reverted.
A ring/donut chart (chartPlot type="pie" with chartSectors innerRadius > 0) has an empty center hole. A text shape placed there (e.g. "70%+ / API收入占比") occludes nothing — it's the classic KPI-donut design. Skip the report when the glyph box is fully contained in the hole circle. - compute_donut_center_hole: parse chartPlot type and chartSectors innerRadius - bbox_within_circle: require full containment (all 4 corners) so text grazing the colored ring is still flagged - detect_chart_text_occlusions: skip charts with a donut_hole when the glyph box fits inside it Verified against real slide I9ddsIq8AljNVqdbkIEcLlzznhb page 6: 2 expected chart_covers_text (text on ring) reported, center text suppressed.
A short single-line label ("60%+") that overflows vertically only because
it is too wide for one line and passively wrapped is now reported on the
width axis. wrap defaults to true, so suggesting wrap="true" or raising
shape.height cannot un-wrap it -- the actionable fix is to widen the box or
shrink the font. Genuine multi-line prose overflow keeps the height hint.
Also pair the width-wrap suggestion's wrap="false" with widening the shape
(wrap="false" alone clips text past a still-narrow box), and clarify the
height hint that autoFit shrinks the font while wrap is already the default.
Parse shape fill visibility according to the Slides XML schema instead of only reading fillColor. Treat missing fill as unfilled, empty fill as the schema default, and support fillImg/fillPattern alpha defaults so visible non-color fills can still occlude text. Add regression coverage for missing fill, empty fill, image fill, pattern fill, and transparent fill variants.
ethan-zhx
force-pushed
the
fix/text_over_flow
branch
from
August 4, 2026 12:31
54ca7d3 to
5d603d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
skills/lark-slides/scripts/xml_text_overlap_lint.py (2)
1159-1172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCompare the overlap area against an area constant.
Line 1159 compares a length (
growth, px) againstCONTAINER_OVERFLOW_MIN_PX. Line 1172 compares an area (px²) against the same constant. The two gates now share one number with two different units. If someone retunesCONTAINER_OVERFLOW_MIN_PXfor the growth threshold, the area gate changes silently.Use a dedicated area threshold for line 1172, for example the existing
SHAPE_TEXT_OCCLUSION_MIN_AREA.♻️ Proposed change
- if intersection_area(grown_region, other_bbox) <= CONTAINER_OVERFLOW_MIN_PX: + if intersection_area(grown_region, other_bbox) <= SHAPE_TEXT_OCCLUSION_MIN_AREA: continue🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 1159 - 1172, Use a dedicated area threshold for the intersection check in the overflow-detection loop around grown_region and intersection_area, replacing CONTAINER_OVERFLOW_MIN_PX with the existing SHAPE_TEXT_OCCLUSION_MIN_AREA while leaving the growth-length threshold unchanged.
1391-1407: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
is_full_canvas_background_imageduplicatesis_canvas_sized_background_shape.Both functions compute the same value: the intersection with the canvas divided by the canvas area, compared against
FULL_CANVAS_BACKGROUND_COVERAGE_RATIO(lines 1186-1194). Only the docstrings and parameter names differ. Keep one predicate, for examplecovers_full_canvas(element, slide_width, slide_height), and call it from both sites so the two rules cannot drift apart.🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py` around lines 1391 - 1407, Consolidate the duplicated full-canvas coverage predicate by introducing or reusing a shared helper such as covers_full_canvas(element, slide_width, slide_height). Replace the logic in both is_full_canvas_background_image and is_canvas_sized_background_shape with calls to that helper, preserving the existing canvas-area validation and FULL_CANVAS_BACKGROUND_COVERAGE_RATIO threshold.
🤖 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 `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 1876-1887: Update short_line_passive_wrap_width to return None for
elements where is_vertical_text is true, matching the existing guard in
detect_text_may_wrap_shapes. Keep vertical runs out of passive-wrap
reclassification so they are not assigned overflow_axis "width" or a shape-width
remediation hint.
---
Nitpick comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint.py`:
- Around line 1159-1172: Use a dedicated area threshold for the intersection
check in the overflow-detection loop around grown_region and intersection_area,
replacing CONTAINER_OVERFLOW_MIN_PX with the existing
SHAPE_TEXT_OCCLUSION_MIN_AREA while leaving the growth-length threshold
unchanged.
- Around line 1391-1407: Consolidate the duplicated full-canvas coverage
predicate by introducing or reusing a shared helper such as
covers_full_canvas(element, slide_width, slide_height). Replace the logic in
both is_full_canvas_background_image and is_canvas_sized_background_shape with
calls to that helper, preserving the existing canvas-area validation and
FULL_CANVAS_BACKGROUND_COVERAGE_RATIO threshold.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 92243a19-56cd-4294-8942-5d28ef3ae9b6
📒 Files selected for processing (2)
skills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
… owner The container overflow detectors select the text owner by maximum intersection area, which lets a full-slide background win over the actual card and suppress text_overflows_container reports. Extract choose_text_container_owner as a shared helper that downgrades canvas-sized backgrounds when a more specific container exists. Apply it to both detect_text_container_overflow and detect_shape_container_overlaps. Add a regression test for the background + card + overflowing text scenario.
…trics near font wrap boundary
The static width estimator uses fixed digit/punctuation coefficients, so
"60%+" at 44pt in a 113px box reports ~110px on every font family. Some
fonts (Arial, Verdana, Garamond, Cambria) wrap the operator onto a second
line, but the existing SINGLE_LINE_METRIC_WIDTH_RATIO (1.18) gate is too
permissive for this subset and produces a false negative.
Add a `is_short_percent_symbol_metric_text` predicate that matches
compact percentage-symbol runs (e.g. "60%+", "+99%"), and route them to a
conservative 0.95 risk band threshold so they are flagged even when the
estimate is below the available width. Plain numeric metrics with CJK
units ("4.16万亿", "1,380") keep the wider 1.18 tolerance.
Add a regression test covering Arial / Verdana / Garamond / Cambria with
44pt "60%+" in a 113px box, asserting width-axis overflow with
estimated_width < available_width. (225 tests pass.)
compute_donut_center_hole accepts any positive innerRadius, but the docstring documents it as a 0..1 fraction of the pie radius. A deck writing innerRadius="55" (meaning 55%) produces a hole radius of 55 * pie_radius, so bbox_within_circle returns true for every text box over that chart and chart_covers_text is silently suppressed for the entire chart — a false negative in a mandatory gate. Reject values > 1 so the hole never exceeds the pie. innerRadius="55" now returns None (no donut center exemption), preserving the existing behavior for valid fractions like 0.55.
Vertical runs (vert, vert270, word-art-vert, word-art-vert-rtl, ea-vert) lay out along the height axis, so measuring estimated line width against element["width"] produces false positives. Skip them in detect_text_may_wrap_shapes, matching the pattern already used by detect_image_text_occlusions. Add regression test covering all five vertical text variants.
…tion A shape with rotation sweeps a different footprint than its authored bbox, but detect_shape_text_occlusions compared the shape unrotated bbox against the already-rotated glyph bbox, missing occlusions where rotation moves the shape into the text area. Rotate the shape bbox before the intersection test, matching how the glyph bbox is already rotated. Includes a regression test: a tall bar rotated 90 degrees beside the text is now correctly flagged as covering it.
…wrap check The wrap="false" branch ran before the is_vertical_text guard, so vertical text with wrap="false" entered detect_wrap_false_width_overflow, which measured horizontal glyph advance width against box width — meaningless for vertically-stacked text — and falsely reported overflow_axis: "width". Move is_vertical_text before the wrap="false" branch so vertical text always skips the horizontal width check. Add a test case with vert="vert" + wrap="false" to pin the fix.
ethan-zhx
force-pushed
the
fix/text_over_flow
branch
from
August 4, 2026 14:19
5d603d1 to
5a12147
Compare
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
3 participants
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
增强 Slides XML Lint 工具
xml_text_overlap_lint.py的文本溢出与遮挡检测能力，覆盖文本被线条/图片/形状/表格/图表遮挡、宽度触发换行、容器溢出以及 auto-fit 增长碰撞等场景；同时补齐 donut 图表、非纯色形状填充、容器归属、短百分号指标、非法innerRadius、竖排文本等真实 deck 边界，降低必拦 gate 的漏报和误报。变更文件（2 个文件，+2348 / -128）
skills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py新增/强化检测能力
文本-线条重叠检测 (
line_covers_text)：使用 Liang-Barsky 线段相交算法检测<line>与文本字形的重叠，支持水平、垂直及斜线。通过边缘侵蚀（max(fontSize * 0.12, 2px)）规避边框擦过误报，border_alpha < 0.08的不可见线条自动跳过。宽度触发的文本换行检测 (
text_may_overflow_shape，新增overflow_axis="width")：原检测仅覆盖高度溢出，新增宽度轴检测。当单行短标签/指标的估计宽度超出内容框或接近字体换行边界时，报告潜在 Skia 渲染换行；对 Latin、CJK、普通数值指标和短百分号符号指标使用不同风险带，竖排文本不参与宽度轴判断。形状遮挡文本检测 (
shape_covers_text)：当非文本<shape>的可见填充区域覆盖文本字形时报警，排除细线/分隔线（短边 ≤ 8px）。填充可见性按 Slides XML schema 解析，覆盖空 fill 默认值、fillImg、fillPattern、透明 fill 和缺失 fill 等情况。容器溢出检测 (
text_overflows_container)：检测文本排版框是否超出其背景容器（卡片/色块），通过 z-order 和最大交集面积自动识别文本归属容器；当全画布背景与局部卡片同时存在时，优先选择更具体的容器，避免背景图/背景色块劫持 owner。表格/图表遮挡文本检测 (
table_covers_text/chart_covers_text)：当自由文本与<table>或<chart>的绘制区域重叠时报警，无论 z-order 如何。对 donut/ring chart 中完全落在中心空洞内的 KPI 文本做豁免，同时拒绝超出0..1范围的非法innerRadius，避免整张图表被错误豁免。Auto-fit 增长碰撞检测 (
bbox_overlap)：检测shape-auto-fit文本因换行向下增长后与下方文本的碰撞。Bug 修复与边界收敛
image.order <= text.order的逻辑允许 XML 中先出现的图片静默遮挡文本，现已移除，任何几何重叠均报警。60%+）、被动换行导致的高度溢出重分类，以及 width/height 溢出去重中的严重级别屏蔽。is_drawn_behind/is_drawn_in_front_of，单一决策点，附带契约测试。Commits
Summary by CodeRabbit