feat(slides): report resolved table size mismatches

[ethan-zhx]

Summary

Improve the Slides XML lint for table geometry. It detects table bounds outside the canvas and reports informational size mismatches when the renderer's resolved row or column totals differ from the declared table dimensions.

Changes

• Add table canvas-boundary errors for tables that overflow the slide.
• Add info output for weighted row and column layout resolution mismatches, including resolved dimensions and correction guidance.
• Add direct and command-line black-box coverage for table layout cases; remove obsolete table-size guidance.

Test Plan

• python3 -m unittest xml_text_overlap_lint_test (42 tests)
• Command-line black-box coverage verifies exact sizing, declared-size overflow, insufficient remaining space, and missing table target dimensions.

Related Issues

• None

Summary by CodeRabbit

• New Features
  • Improved table linting with new validations for tables that extend beyond slide canvas boundaries.
  • Added informational reporting for when table layout sizing resolves differently from declared dimensions, including resolved size details.
• Bug Fixes
  • Canvas boundary checks now use the resolved table geometry rather than only declared bounds.
• Documentation
  • Updated the XML format guide by removing outdated table sizing guidance.
• Tests
  • Added extensive coverage for overflow detection, resolved-size behavior, and end-to-end CLI lint output.

[coderabbitai]

📝 Walkthrough

Walkthrough

The XML linter now resolves table dimensions, detects canvas overflow and declared-size mismatches, and reports informational issue counts in JSON output. Tests cover table bounds, sizing behavior, and CLI results; table sizing guidance was removed from the format guide.

Changes

Table layout validation

Layer / File(s)	Summary
Table resolution and validation skills/lark-slides/scripts/xml_text_overlap_lint.py, skills/lark-slides/scripts/xml_text_overlap_lint_test.py, skills/lark-slides/references/xml-format-guide.md	Adds weighted table size resolution, canvas overflow detection, declared-versus-resolved size checks, and focused table validation tests; removes the prior table sizing guidance.
Summary reporting and CLI validation skills/lark-slides/scripts/xml_text_overlap_lint.py, skills/lark-slides/scripts/xml_text_overlap_lint_test.py	Adds info_count to all JSON summary paths and verifies informational table issues through an end-to-end CLI test.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant lint_xml
  participant lint_slide
  participant solve_weighted_min_layout
  participant JSONSummary
  lint_xml->>lint_slide: slide XML and canvas dimensions
  lint_slide->>solve_weighted_min_layout: table column and row sizes
  solve_weighted_min_layout-->>lint_slide: resolved sizes and totals
  lint_slide-->>lint_xml: table issues
  lint_xml->>JSONSummary: aggregate error, warning, and info counts

Loading

Possibly related PRs

• larksuite/cli#847: Earlier changes to the same XML linting pipeline and lint_slide behavior.

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: reporting resolved table size mismatches for slides.
Description check	✅ Passed	The description matches the template and covers summary, changes, test plan, and related issues.
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/slides_table_lint

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🤖 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 1002-1004: Unify fallback table ID generation between
extract_elements and this table-processing loop so every anonymous table
receives the same identifier regardless of diagnostic type. Reuse shared parsed
table records or a common fallback-ID generator, preserving explicit table IDs
and ensuring anonymous tables are numbered among all elements consistently.
- Around line 940-945: Update the table-processing flow in extract_elements and
the table canvas-validation loop to retain tables even when declared width or
height is missing, resolve one effective bounding box from the table’s columns
and rows, and use that geometry for both table_out_of_canvas overflow checks and
declared-size mismatch checks. Ensure resolved extents beyond the slide produce
table_out_of_canvas rather than only an informational mismatch.
- Around line 101-109: Update fill_last_size_gap so the computed final_sizes[-1]
is never zero or negative, while preserving the target-size sum whenever
possible. Clamp or otherwise redistribute sizes so inputs such as empty columns
resolving to a 1px target cannot produce [1, 0], and ensure the resulting
dimensions remain valid for the existing mismatch check.

🪄 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: 12e0fb3a-5788-4236-8bc2-dc6bfc68d020

📥 Commits

Reviewing files that changed from the base of the PR and between 4e0a6a9 and 8356b2a.

📒 Files selected for processing (3)

• skills/lark-slides/references/xml-format-guide.md
• skills/lark-slides/scripts/xml_text_overlap_lint.py
• skills/lark-slides/scripts/xml_text_overlap_lint_test.py

💤 Files with no reviewable changes (1)

• skills/lark-slides/references/xml-format-guide.md

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.96%. Comparing base (4e0a6a9) to head (ab27e2b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1928   +/-   ##
=======================================
  Coverage   74.96%   74.96%           
=======================================
  Files         892      892           
  Lines       94058    94077   +19     
=======================================
+ Hits        70506    70529   +23     
+ Misses      18139    18137    -2     
+ Partials     5413     5411    -2     

☔ 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.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

skills/lark-slides/scripts/xml_text_overlap_lint_test.py (1)

977-978: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the return code before parsing stdout.

If the subprocess crashes (e.g., due to a syntax error or a missing import), stdout will likely be empty. Attempting to parse it first will raise a JSONDecodeError, burying the actual crash details from stderr. Swapping these two lines ensures that if the process fails, the test reports the actual error from completed.stderr.

♻️ Proposed refactor

-                    result = json.loads(completed.stdout)
-                    self.assertEqual(completed.returncode, 0, completed.stderr)
+                    self.assertEqual(completed.returncode, 0, completed.stderr)
+                    result = json.loads(completed.stdout)

🤖 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_test.py` around lines 977 -
978, In the subprocess test around completed, move the assertEqual check for
completed.returncode before json.loads(completed.stdout). Preserve the existing
stderr failure detail so subprocess errors are reported directly instead of
being masked by JSON parsing.

🤖 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 1012-1014: Bound the `<col span>` value in the column-size
expansion logic before `sizes.extend` allocates entries. Define or reuse the
table-column limit, validate `span_count` against it, and emit the existing lint
error through the surrounding lint-reporting path before allocation; preserve
normal expansion for valid spans.
- Around line 598-601: Update the element-content extraction in the shape/table
matching logic to detect when the matched opening element is self-closing and
skip searching for a closing tag in that case. Ensure self-closing tables
receive empty content, while non-self-closing elements continue using their
matching closing tag and do not consume sibling content.

---

Nitpick comments:
In `@skills/lark-slides/scripts/xml_text_overlap_lint_test.py`:
- Around line 977-978: In the subprocess test around completed, move the
assertEqual check for completed.returncode before json.loads(completed.stdout).
Preserve the existing stderr failure detail so subprocess errors are reported
directly instead of being masked by JSON parsing.

🪄 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: dc1cee95-08c1-4852-84e4-36a1f0d58a11

📥 Commits

Reviewing files that changed from the base of the PR and between 8356b2a and ab27e2b.

📒 Files selected for processing (3)

• skills/lark-slides/references/xml-format-guide.md
• skills/lark-slides/scripts/xml_text_overlap_lint.py
• skills/lark-slides/scripts/xml_text_overlap_lint_test.py

💤 Files with no reviewable changes (1)

• skills/lark-slides/references/xml-format-guide.md

[fangshuyu-768]

Thanks for the table-geometry lint improvements. I verified the current PR head (ab27e2b) locally: the declared test command passes (python3 -m unittest xml_text_overlap_lint_test, 47 tests), and CI is green. I still see a few issues worth fixing before merge:

1. extract_table_column_sizes expands <col span> directly into a Python list with no upper bound. A very large but well-formed span, e.g. span="10000000", can make the lint process hang or run out of memory before it has a chance to report anything. Please validate/bound span_count before sizes.extend(...), ideally using a table column limit and emitting a lint error instead of allocating unbounded entries.

2. extract_elements only keeps a table when all of x, y, resolved width, and resolved height are present. Because table width, height, and colgroup are optional, this can skip real overflow cases. For example, a table with rows but no declared width/colgroup can still resolve height and overflow the bottom edge, but it is dropped because width is None; similarly, a table with columns but no rows/height can overflow horizontally and be dropped. The canvas check should either resolve one effective bbox with sensible defaults, or validate each axis independently when only one dimension is known.

3. The table content extraction uses slide_xml.find("</table>", match.end()) even for self-closing <table .../> tags. If a self-closing table is followed by another table, the first table can accidentally consume the second table’s content and report false table_resolved_size_mismatch diagnostics. Please detect self-closing opening tags and treat their content as empty, or switch this portion to parsed XML traversal.

4. Minor test maintainability: in the CLI black-box test, assert completed.returncode before json.loads(completed.stdout). If the subprocess crashes, this preserves the useful stderr instead of masking it with a JSON decode error.

Separately, my read of the new sizing behavior is: table width/height are treated as target minimum dimensions; filled column/row sizes are preserved; empty columns/rows share remaining space; when all child sizes are filled and the target is larger, sizes are scaled by their existing weights; when the target is too small, filled sizes/defaults win and the resolved total can exceed the declared table size. The resolved bbox is then used for table_out_of_canvas, while declared-vs-resolved differences are reported as info.