fix(slides): detect text and line boundary violations#2050
Open
BD-ZERO wants to merge 1 commit into
Open
Conversation
- flag lines crossing text bounds - flag text touching or exceeding container boundaries - use estimated glyph bounds to avoid text-box false positives - add regression tests and validation guidance
github-actions
Bot
added
the
size/L
📝 WalkthroughWalkthroughThe slide XML lint tool adds warnings for lines crossing text and text visually overflowing candidate containers. Both detectors are integrated into per-slide issue collection, covered by geometry tests, registered in rule metadata, and documented in the validation checklist. ChangesSlide geometry linting
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 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 1652-1655: Update the visual line filtering logic in the
overlap-lint collection to retain diagonal lines instead of excluding them based
on zero width or height. Use segment-versus-visual-text-bounds intersection when
evaluating line/text overlap, preserving the existing negative-test behavior
that rejects filled-AABB false positives while warning on actual diagonal
crossings.
- Around line 1656-1668: Update line_crosses_text in
skills/lark-slides/scripts/xml_text_overlap_lint.py (1656-1668) to preserve XML
draw order, skip lines rendered behind text, and intersect each line with
estimate_text_visual_bbox(text) instead of the full text box. In
skills/lark-slides/scripts/xml_text_overlap_lint_test.py (625-645), place the
warning line above the text and add negative cases for background lines and
glyph-whitespace-only intersections. Update
skills/lark-slides/references/validation-checklist.md (67-67) to describe
estimated glyph-bound intersections.
🪄 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: 3db67f80-905b-4c80-80c6-104df22cb7d5
📒 Files selected for processing (3)
skills/lark-slides/references/validation-checklist.mdskills/lark-slides/scripts/xml_text_overlap_lint.pyskills/lark-slides/scripts/xml_text_overlap_lint_test.py
Comment on lines
+1652
to
+1655
| if element["kind"] == "line" | ||
| and is_visually_rendered(element) | ||
| and (element["width"] == 0 or element["height"] == 0) | ||
| ] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Detect diagonal line crossings instead of excluding them.
Lines 1652–1655 discard every diagonal <line>, so a diagonal connector that actually crosses text can never warn. Use segment-vs-visual-text-bounds intersection; that avoids the filled-AABB false positive covered by the existing negative test while detecting real crossings.
🤖 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 1652 -
1655, Update the visual line filtering logic in the overlap-lint collection to
retain diagonal lines instead of excluding them based on zero width or height.
Use segment-versus-visual-text-bounds intersection when evaluating line/text
overlap, preserving the existing negative-test behavior that rejects filled-AABB
false positives while warning on actual diagonal crossings.
Comment on lines
+1656
to
+1668
| texts = [ | ||
| element | ||
| for element in elements | ||
| if is_text_element(element) | ||
| and is_visually_rendered(element) | ||
| and has_text_content(element) | ||
| and not is_decorative_text(element) | ||
| ] | ||
| for line in lines: | ||
| stroke_bbox = line_stroke_bbox(line) | ||
| for text in texts: | ||
| width = intersection_width(stroke_bbox, text) | ||
| height = intersection_height(stroke_bbox, text) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use glyph bounds and XML draw order for line_crosses_text.
The detector intersects every line with the full declared text box and never checks stacking order. This warns for background lines rendered beneath text and for lines passing only through unused text-box whitespace, contrary to the glyph-bound/z-order objective.
skills/lark-slides/scripts/xml_text_overlap_lint.py#L1656-L1668: preserve XML order for lines, skip lines behind text, and intersectestimate_text_visual_bbox(text)rather thantext.skills/lark-slides/scripts/xml_text_overlap_lint_test.py#L625-L645: put the warning line above the text in draw order; add negative coverage for background lines and glyph-whitespace intersections.skills/lark-slides/references/validation-checklist.md#L67-L67: describe intersection with estimated glyph bounds rather than declared text-box bounds.
Based on supplied PR objectives: the detector must use estimated glyph bounds and z-order.
📍 Affects 3 files
skills/lark-slides/scripts/xml_text_overlap_lint.py#L1656-L1668(this comment)skills/lark-slides/scripts/xml_text_overlap_lint_test.py#L625-L645skills/lark-slides/references/validation-checklist.md#L67-L67
🤖 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 1656 -
1668, Update line_crosses_text in
skills/lark-slides/scripts/xml_text_overlap_lint.py (1656-1668) to preserve XML
draw order, skip lines rendered behind text, and intersect each line with
estimate_text_visual_bbox(text) instead of the full text box. In
skills/lark-slides/scripts/xml_text_overlap_lint_test.py (625-645), place the
warning line above the text and add negative cases for background lines and
glyph-whitespace-only intersections. Update
skills/lark-slides/references/validation-checklist.md (67-67) to describe
estimated glyph-bound intersections.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@aa9c288b8ddc834bd2958a7a6d12f5d363b33da3🧩 Skill updatenpx skills add larksuite/cli#fix/slides-lint-container-boundaries -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2050 +/- ##
=======================================
Coverage 75.14% 75.14%
=======================================
Files 911 911
Lines 96322 96322
=======================================
+ Hits 72381 72385 +4
+ Misses 18372 18370 -2
+ Partials 5569 5567 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Strengthen Slides XML linting for text boundary violations. Detect lines crossing text and text touching or extending beyond its container bottom edge, without adding a 2 px overflow buffer.
Changes
line_crosses_textwarning when a line segment intersects a text box.text_outside_containerwarning when the estimated visual text bottom touches or exceeds the container bottom.Test Plan
python3 -m unittest -v xml_text_overlap_lint_test.py— 97 tests passedmake unit-testoverflow = 0) is reportedLowWsCcaclZEP9dmgYjc3tv1nncH0M7sBDHJl4bFFd215wcDfJxnZbPF7esWwISlxhkwdxqEocMKf1nlcRelated Issues