Skip to content

fix(slides): detect image text occlusion#1984

Merged
ethan-zhx merged 1 commit into
mainfrom
feat/detect_image_conver_text
Jul 21, 2026
Merged

fix(slides): detect image text occlusion#1984
ethan-zhx merged 1 commit into
mainfrom
feat/detect_image_conver_text

Conversation

@ethan-zhx

@ethan-zhx ethan-zhx commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Improve Slides XML linting to detect image layers that may occlude text content.

Changes

  • Add image-over-text occlusion detection based on XML layer order and visual bounding boxes.
  • Report horizontal image/text occlusion as an error.
  • Report vertical-text/image overlap as an informational issue requiring rendered-slide inspection.
  • Account for text padding, alignment, wrapping, vertical alignment, rotation direction, transparency, and inline span font sizes when estimating text bounds.
  • Reduce false positives for wrapped text with sufficient horizontal spacing.
  • Update regression tests for image occlusion and text overflow behavior.
  • Remove outdated whiteboard-related test cases that no longer match the current lint scope.

Testing

  • Added and updated unit test coverage in xml_text_overlap_lint_test.py.
  • Full test suite not run for this change.

Summary by CodeRabbit

  • New Features

    • Added detection for images that may obscure text, including special handling for vertical text.
    • Improved text layout analysis using font size, padding, and horizontal and vertical alignment.
  • Bug Fixes

    • Reduced false warnings for text overflow when wrapping is enabled and text boxes are separated by a gap.
    • Improved accuracy of overlap and occlusion diagnostics.
  • Tests

    • Added coverage for image overlap with vertical text.
    • Updated overflow expectations and removed obsolete scenarios.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 21, 2026
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The XML overlap lint now models inline font sizes, text padding and alignment, image transparency, and image-text occlusion. Overflow behavior and regression tests were updated, while several whiteboard-specific tests and fixtures were removed.

Changes

Text overlap linting

Layer / File(s) Summary
Text extraction and visual bounds
skills/lark-slides/scripts/xml_text_overlap_lint.py, skills/lark-slides/scripts/xml_text_overlap_lint_test.py
Paragraph font sizes derive from inline spans, text bounds account for padding and alignment, and wrapped overflow detection is refined. Related fixtures and whiteboard tests are updated or removed.
Image occlusion diagnostics
skills/lark-slides/scripts/xml_text_overlap_lint.py, skills/lark-slides/scripts/xml_text_overlap_lint_test.py
Image alpha and text orientation are extracted, image intersections produce vertical-text info diagnostics or non-vertical-text error diagnostics, and the new behavior is tested.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SlideXML
  participant lint_slide
  participant OcclusionCheck
  participant Issues
  SlideXML->>lint_slide: extract ordered shapes and images
  lint_slide->>OcclusionCheck: analyze image-text intersections
  OcclusionCheck->>Issues: report occlusion diagnostics
  Issues-->>lint_slide: return slide issues
Loading

Possibly related PRs

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the main change: detecting image/text occlusion in Slides.
Description check ✅ Passed The description covers the summary, main changes, and testing, with only the related issues section omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/detect_image_conver_text

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 694-696: Update estimate_text_line_count_for_text to calculate
usable width by subtracting element["paddingLeft"] and element["paddingRight"]
from element["width"], clamping the result to at least 1, and use that width
when computing wrapped line counts.
🪄 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

Run ID: 2a237418-f60f-4f1c-8d57-5a30238ef98f

📥 Commits

Reviewing files that changed from the base of the PR and between 6675e3c and 3906e50.

📒 Files selected for processing (2)
  • skills/lark-slides/scripts/xml_text_overlap_lint.py
  • skills/lark-slides/scripts/xml_text_overlap_lint_test.py

Comment thread skills/lark-slides/scripts/xml_text_overlap_lint.py
@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3906e50ad1014fb06c7f1091faa0875a13dc3033

🧩 Skill update

npx skills add larksuite/cli#feat/detect_image_conver_text -y -g

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.05%. Comparing base (6675e3c) to head (3906e50).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1984   +/-   ##
=======================================
  Coverage   75.05%   75.05%           
=======================================
  Files         901      901           
  Lines       95391    95391           
=======================================
  Hits        71591    71591           
  Misses      18307    18307           
  Partials     5493     5493           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ethan-zhx
ethan-zhx merged commit 680501c into main Jul 21, 2026
32 of 44 checks passed
@ethan-zhx
ethan-zhx deleted the feat/detect_image_conver_text branch July 21, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants