Skip to content

feat: Phase D.26: Table autofit and column width control#104

Open
citconv-agents[bot] wants to merge 2 commits intomasterfrom
agent/issue-39
Open

feat: Phase D.26: Table autofit and column width control#104
citconv-agents[bot] wants to merge 2 commits intomasterfrom
agent/issue-39

Conversation

@citconv-agents
Copy link
Copy Markdown

@citconv-agents citconv-agents bot commented Apr 5, 2026

Summary

Implements #39

This PR was automatically generated by the Developer Agent.

Original Issue

Fix table autofit behavior and improve column width control.

Current issues

API Design

  • table.autofit_behavior — WD_TABLE_AUTOFIT enum (AUTOFIT_TO_WINDOW, AUTOFIT_TO_CONTENTS, FIXED_WIDTH)
  • column.width — Emu value, reliable set/get
  • table.preferred_width — total table width
  • table.allow_autofit — bool

XML

  • w:tblPr > w:tblW (table width, type=auto/dxa/pct)
  • w:tblPr > w:tblLayout w:type='fixed' or 'autofit'
  • w:tcPr > w:tcW (cell width)
  • w:tblPr > w:tblStyleColBandSize, w:tblStyleRowBandSize

Upstream: python-openxml#209 (13 comments), python-openxml#360 (8 comments)


Generated by Developer Agent using Claude Code

- Add WD_TABLE_AUTOFIT enum (FIXED_WIDTH, AUTOFIT_TO_CONTENTS, AUTOFIT_TO_WINDOW)
- Add table.autofit_behavior property combining tblLayout and tblW settings
- Add table.allow_autofit property (clearer name for tblLayout type)
- Add table.preferred_width property for reading/writing w:tblW
- Improve column.width setter to also update cell widths (w:tcW) for
  reliable column width control
- Register w:tblW element and add tblW child to CT_TblPr

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@citconv-agents citconv-agents bot added the agent-pr PR created by an agent label Apr 5, 2026
@citconv-agents
Copy link
Copy Markdown
Author

citconv-agents bot commented Apr 5, 2026

Security Agent Report

SECURITY_PASS

Security Review: PR #104 — Table AutoFit Behavior and Column Width Control

Branch: agent/issue-39
Reviewed: 2026-04-05
Verdict: CLEAN — no security issues found


Files Reviewed

  • src/docx/enum/table.py
  • src/docx/oxml/__init__.py
  • src/docx/oxml/table.py
  • src/docx/table.py
  • tests/oxml/test_table.py
  • tests/test_table.py

Checks Performed

XML Injection / XXE

CLEAN. All XML attribute values assigned in the changed code ("pct", "auto", "dxa", "autofit", "fixed") are hardcoded string literals, not derived from user input. No external XML parsing or entity resolution introduced.

Path Traversal

CLEAN. No file path handling added or modified.

Dependency Risks

CLEAN. No new dependencies added (requirements.txt, pyproject.toml, setup.py not modified).

Data Exposure / Unsafe File Handling

CLEAN. No I/O operations, no logging of sensitive data, no external network calls.

Secrets in Code

CLEAN. No API keys, tokens, passwords, or credentials present.

Integer Overflow / Numeric Safety

CLEAN. Width values are Python int (EMU units). Operations like tblW.w = 5000 and tblW.w = 0 are bounded, well-typed constants. No arithmetic on untrusted input.

Silent Failure on Invalid Enum Value

LOW CONCERN (informational only). The autofit_behavior setter in src/docx/table.py has no else clause — passing an unrecognized value silently does nothing. This is not a security vulnerability but could mask programming errors. Not flagged as a finding since enum membership is enforced by the type system and this matches existing patterns in the codebase.


Summary

PR #104 is a pure feature addition implementing OOXML table autofit and preferred-width controls. All XML mutations use hardcoded enum-driven literals through the existing lxml-based oxml layer. No attack surface is introduced or expanded.

@citconv-agents citconv-agents bot added the security-passed Security agent passed label Apr 5, 2026
@citconv-agents
Copy link
Copy Markdown
Author

citconv-agents bot commented Apr 5, 2026

Review Agent

REVIEW_CHANGES_NEEDED

PR #104 — Table AutoFit Behavior and Column Width Control

Overall the implementation is well-structured and tests are thorough. Two issues need addressing before merge.


Issue 1 (Moderate): allow_autofit duplicates the existing autofit property

File: src/docx/table.py:98-109

Table.allow_autofit is a new property added in this PR:

@property
def allow_autofit(self) -> bool:
    return self._tblPr.autofit

@allow_autofit.setter
def allow_autofit(self, value: bool):
    self._tblPr.autofit = value

This is identical in every way to the already-existing Table.autofit property at lines 84-96. The codebase now has two properties with different names doing the same thing. This creates API confusion for users and should not be merged as-is.

Options:

  • If allow_autofit is the preferred name going forward, deprecate autofit rather than silently keeping both.
  • If autofit is the canonical name, remove allow_autofit entirely.
  • The accompanying tests (it_knows_its_allow_autofit_setting, it_can_change_its_allow_autofit_setting) are also redundant with existing autofit tests.

Issue 2 (Minor): autofit_behavior setter leaves stale tblW when switching to FIXED_WIDTH

File: src/docx/table.py:130-131

if value == WD_TABLE_AUTOFIT.FIXED_WIDTH:
    tblPr.autofit = False

When switching from AUTOFIT_TO_WINDOW (which writes tblW type="pct" w="5000") to FIXED_WIDTH, the tblW child element is left in the XML unchanged. The resulting XML contains both tblLayout type="fixed" and tblW type="pct", which is inconsistent.

While Word may ignore tblW when layout is fixed, this orphaned element can confuse other OOXML consumers (e.g. LibreOffice, document converters).

Fix: Clear tblW when switching to FIXED_WIDTH:

if value == WD_TABLE_AUTOFIT.FIXED_WIDTH:
    tblPr.autofit = False
    tblPr._remove_tblW()

A test covering this transition (e.g. starting from AUTOFIT_TO_WINDOW XML and asserting tblW is absent after setting FIXED_WIDTH) would be appropriate.


What's Good

  • CT_TblPr.tblW element registration and ZeroOrOne declaration at the correct schema position (_tag_seq[7:]) is correct.
  • CT_TblPr.preferred_width getter/setter delegation to CT_TblWidth.width is clean.
  • _Column._update_cell_widths correctly skips cells with grid_span > 1 (merged cells) and handles the tblGrid is None edge case.
  • WD_TABLE_AUTOFIT correctly uses BaseEnum (not BaseXmlEnum) since the enum values don't map directly to XML attributes.
  • Test coverage for CT_TblPr.preferred_width including the element ordering test (it_inserts_tblW_in_the_right_position) is thorough.
  • register_element_cls("w:tblW", CT_TblWidth) in oxml/__init__.py fills a pre-existing gap.

@citconv-agents citconv-agents bot added the review-changes-needed Review agent requested changes label Apr 5, 2026
Remove duplicate `allow_autofit` property (identical to existing `autofit`)
and clear stale `tblW` element when switching to FIXED_WIDTH behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@citconv-agents citconv-agents bot removed security-passed Security agent passed review-changes-needed Review agent requested changes labels Apr 5, 2026
@citconv-agents
Copy link
Copy Markdown
Author

citconv-agents bot commented Apr 5, 2026

Security Agent Report

SECURITY_PASS

Security Review — PR #104 (Table AutoFit / Preferred Width)

Branch: agent/issue-39
Date: 2026-04-05
Reviewer: Security Agent


Summary

No security issues found. All changes are internal library extensions that operate on typed values within the existing XML abstraction layer.


Files Reviewed

File Change Type
docs/dev/analysis/features/table/table-props.rst Docs only — property rename
src/docx/enum/table.py New enum WD_TABLE_AUTOFIT
src/docx/oxml/__init__.py Element class registration
src/docx/oxml/table.py preferred_width property on CT_TblPr
src/docx/table.py autofit_behavior, preferred_width properties; _update_cell_widths
tests/oxml/test_table.py Unit tests
tests/test_table.py Unit tests

Checks Performed

XML Injection / XXE

Result: CLEAN

All values written to XML attributes are strongly typed (Length / WD_TABLE_AUTOFIT enum / Python int). No raw user-supplied strings are concatenated into XML. The tblW.type, tblW.w, and tblW.width assignments in table.py and oxml/table.py use either hardcoded literals ("pct", "auto", "dxa", 5000, 0) or values that pass through the existing Length type system. The framework's BaseOxmlElement handles serialization safely via lxml — no manual XML string construction is present.

Path Traversal

Result: CLEAN

No file paths are constructed or manipulated in any of the changed files.

New Dependencies

Result: CLEAN

No new third-party dependencies are introduced. All imports are from existing internal modules (docx.enum.table, docx.oxml.table, docx.shared).

Secrets / Hardcoded Credentials

Result: CLEAN

No API keys, tokens, passwords, or other credentials present.

Data Exposure / Unsafe File Handling

Result: CLEAN

No file I/O, network calls, or sensitive data handling introduced.

Logic / DoS Concerns

Result: CLEAN

The _update_cell_widths method iterates over table rows with a ValueError guard on tc_at_grid_offset. The loop is bounded by the number of rows in the table (user-controlled document data, not externally injected). No unbounded recursion or allocation.


Conclusion

The PR adds well-scoped table width and autofit behavior control. All new XML attribute writes use the existing type-safe oxml abstraction layer. No security-relevant issues were identified.

@citconv-agents citconv-agents bot added the security-passed Security agent passed label Apr 5, 2026
@citconv-agents
Copy link
Copy Markdown
Author

citconv-agents bot commented Apr 5, 2026

Review Agent

REVIEW_CHANGES_NEEDED

PR #104 — Table AutoFit Behavior and Preferred Width

Summary

The PR adds Table.autofit_behavior (using new WD_TABLE_AUTOFIT enum), Table.preferred_width, and updates _Column.width to sync cell widths. The overall direction is sound and the feature is well-tested. One real architectural issue requires a fix; the rest are notes.


Issue 1 — Calling a private CT_ method from the proxy layer (Required Change)

src/docx/table.py, line 119

if value == WD_TABLE_AUTOFIT.FIXED_WIDTH:
    tblPr.autofit = False
    tblPr._remove_tblW()   # <-- private method called from proxy layer

The project's three-layer pattern keeps _remove_* methods internal to the oxml layer. The FIXED_WIDTH case already has a public path: CT_TblPr.preferred_width = None calls _remove_tblW() internally. Use that instead:

if value == WD_TABLE_AUTOFIT.FIXED_WIDTH:
    tblPr.autofit = False
    tblPr.preferred_width = None

Issue 2 — autofit_behavior setter logic belongs in CT_TblPr (Required Change)

src/docx/table.py, lines 120–129

The AUTOFIT_TO_WINDOW and AUTOFIT_TO_CONTENTS cases reach through to tblPr.get_or_add_tblW() and then set .type and .w directly from the proxy layer:

elif value == WD_TABLE_AUTOFIT.AUTOFIT_TO_WINDOW:
    tblPr.autofit = True
    tblW = tblPr.get_or_add_tblW()   # oxml element
    tblW.type = "pct"                  # raw XML attr set from Table
    tblW.w = 5000

All other Table setters (alignment, autofit, style) delegate the full mutation to a property on CT_TblPr. The autofit_behavior setter should follow the same pattern — move this logic into CT_TblPr.autofit_behavior (getter + setter) and have Table.autofit_behavior simply delegate to it, consistent with Table.alignment, Table.autofit, etc.


Minor Notes (no change required)

_update_cell_widths redundant parent traversal (src/docx/table.py, line 493)

After confirming tblGrid = self._gridCol.getparent() is not None, the method then retrieves the table via self.table._tbl (going through the Python proxy chain) rather than tblGrid.getparent(). Both arrive at the same CT_Tbl element; it's redundant but harmless.

Missing test case for autofit_behavior getter with dxa-type tblW + autofit layout

When a real document has <w:tblW w:w="4320" w:type="dxa"/> alongside <w:tblLayout w:type="autofit"/>, the getter returns AUTOFIT_TO_CONTENTS (since type ≠ "pct"). This edge case is not exercised by the parametrized test matrix but is implicitly handled correctly by the current logic.

WD_TABLE_AUTOFIT docstring example (src/docx/enum/table.py, line 243)

The example in the class docstring uses table.autofit_behavior = WD_TABLE_AUTOFIT.AUTOFIT_TO_WINDOW, but the description says "Specifies the AutoFit behavior for a table" which is fine. The from docx.enum.table import WD_TABLE_AUTOFIT import in the example is missing, unlike comparable enums in this file (e.g. WD_TABLE_DIRECTION). Not a bug, but worth aligning with the file's own convention.


What's Good

  • CT_TblWidth now registered for w:tblW (was only registered for w:tcW).
  • tblW successor index (_tag_seq[7:]) is correct per schema ordering.
  • preferred_width properly delegates to CT_TblWidth.width, which already handles the dxa/non-dxa distinction.
  • Test coverage is thorough: parametrized round-trip tests for getter and setter, schema-position test for tblW insertion, and a concrete integration test for cell-width sync.
  • _update_cell_widths correctly handles merged cells (skips cells with grid_span != 1 and catches ValueError from tc_at_grid_offset).

@citconv-agents citconv-agents bot added the review-changes-needed Review agent requested changes label Apr 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-pr PR created by an agent review-changes-needed Review agent requested changes security-passed Security agent passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant