feat: DOCX output (GJ-1) — IR to OOXML writer - #44
Conversation
- hwp-convert::docx: OPC packaging ([Content_Types], rels, document, styles, numbering, footnotes/endnotes, media, core props) - paragraphs with Heading1-6 styles, run properties (font, size, color, shade, letter-spacing, super/subscript, b/i/u/strike) - para alignment/line-spacing/indent/margins with the doubled hwp5 unit conversions (twips = value/10, line = value/10 or pct x 240/100) - tables: occupancy grid with gridSpan (colspan) and vMerge (rowspan), nested tables preserved - images as word/media + inline drawings (EMU extent), hyperlinks as external rels + w:hyperlink - numbering.xml from numbering_levels/bullet_chars (template ^N to %N, Korean formats to koreanCounting/koreanDigital) - footnotes/endnotes with separator entries and references - equations fall back to script text (OMML out of scope) - CLI: ConvertFormat::Docx + .docx inference + target extension (MCP hwp_convert gets it via execute) - tests: structure/marks, span+vMerge, notes/link/list, image extent; CLI test with zip structure asserts + textutil text round-trip
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf487ced49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| r.push_str("<w:vertAlign w:val=\"subscript\"/>"); | ||
| } | ||
| // 크기: base_size(1/100pt) × rel_sizes(%) → half-points. | ||
| let hp = (i64::from(s.base_size) * i64::from(s.rel_sizes[0]) / 50).max(2); |
There was a problem hiding this comment.
For every run with a normal character shape, this conversion is off by a factor of 100: a 10 pt shape (base_size=1000, rel_sizes[0]=100) becomes w:sz=2000, which Word interprets as 1000 pt rather than 10 pt. Divide by 5000 when converting hundredths of a point times a percentage to half-points; otherwise essentially all exported body text is enormously oversized.
Useful? React with 👍 / 👎.
| if covered[r][c] { | ||
| blocks.push_str(&format!( | ||
| "<w:tc><w:tcPr><w:tcW w:w=\"{col_twips}\" w:type=\"dxa\"/><w:vMerge/></w:tcPr><w:p/></w:tc>" | ||
| )); |
There was a problem hiding this comment.
When a cell has col_span > 1, its covered columns reach this branch and each is emitted as another <w:tc> with <w:vMerge/>. For example, a three-column row whose first cell spans two columns is serialized as a two-column gridSpan cell plus two additional cells, exceeding the declared grid and corrupting the merged layout. Horizontal coverage must be skipped, while vertical continuations need a single cell with the original gridSpan where applicable.
Useful? React with 👍 / 👎.
| let num_id = | ||
| u32::from(ps.numbering_id) + 1 + if ht == 3 { BULLET_NUM_BASE } else { 0 }; |
There was a problem hiding this comment.
For every bullet paragraph (head_type == 3), the extra + 1 makes definition index 0 reference w:numId=129, while numbering_xml emits that bullet as w:numId=128. The relationship therefore points to a nonexistent numbering instance, so bullet markers can disappear or trigger document repair; bullet IDs should use BULLET_NUM_BASE + numbering_id.
Useful? React with 👍 / 👎.
| if let Some(eq) = &g.equation { | ||
| for c in eq.script.chars() { | ||
| push_escaped(out, c); |
There was a problem hiding this comment.
When an equation control is encountered, the caller has already closed the current run, but this writes the script directly beneath <w:p>. WordprocessingML paragraph text must be contained in elements such as <w:r><w:t>…</w:t></w:r>, so documents containing equations can lose the fallback text or be repaired when opened.
Useful? React with 👍 / 👎.
| let page = doc | ||
| .sections | ||
| .iter() | ||
| .flat_map(|s| &s.paragraphs) | ||
| .flat_map(|p| &p.controls) | ||
| .find_map(|c| match c { | ||
| Control::SectionDef(sd) => sd.page.as_ref(), | ||
| _ => None, | ||
| }); |
There was a problem hiding this comment.
For a document with multiple sections, all paragraphs are flattened into one body and find_map selects only the first SectionDef, despite the function claiming to use the last one. Consequently later section margins, paper sizes, and orientations are discarded and the first section's setup is applied globally; each section needs a boundary sectPr, with the final section's properties used for the body-level terminator.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements GJ-1 DOCX export by adding an IR → OOXML (DOCX) writer in hwp-convert, wiring it into hwp convert --to docx (including extension inference), and updating roadmap/docs/changelog to reflect the new capability.
Changes:
- Add
hwp-convert::docxwriter that packages OOXML parts into an OPC ZIP (.docx), including paragraphs/styles, tables, images, hyperlinks, lists, notes, and section page setup. - Expose
ConvertFormat::Docxin the CLI, add.docxextension inference, and add a CLI integration test for DOCX structure (plus optional macOStextutilsmoke). - Update generated CLI reference docs and roadmap documents (EN/KR) plus
CHANGELOG.mdto document DOCX export.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/manual/cli-reference.md | Generated CLI docs updated to include docx in convert --to. |
| docs/manual/cli-reference.ko.md | Korean generated CLI docs updated to include docx in convert --to. |
| docs/design/12-feature-gaps.md | Roadmap updated to mark GJ-1 DOCX output as resolved (EN). |
| docs/design/12-feature-gaps.ko.md | Roadmap updated to mark GJ-1 DOCX output as resolved (KR). |
| crates/hwp-convert/src/lib.rs | Exposes new docx module and to_docx API. |
| crates/hwp-convert/src/docx.rs | New DOCX writer implementation + unit tests. |
| crates/hwp-cli/src/cli.rs | Adds ConvertFormat::Docx to CLI surface. |
| crates/hwp-cli/src/commands/convert.rs | Writes DOCX output and infers .docx format from extension. |
| crates/hwp-cli/tests/cli.rs | Adds end-to-end CLI test to validate DOCX packaging and text extraction (macOS-only). |
| CHANGELOG.md | Documents the new DOCX export feature in Unreleased. |
Suppressed comments (3)
crates/hwp-convert/src/docx.rs:420
- The table merge algorithm marks every covered grid slot as
coveredand always emits a<w:tc>with<w:vMerge/>for it. This does not match OOXML semantics forgridSpan(horizontal merges): covered columns should typically be omitted (or represented withw:hMerge, notw:vMerge), and vertical-merge continuation cells should only be emitted once per spanned region (e.g., at the origin column, potentially with the samegridSpan). As-is,col_spanmerges can turn into extra cells/incorrect vertical merges in Word.
if covered[r][c] {
blocks.push_str(&format!(
"<w:tc><w:tcPr><w:tcW w:w=\"{col_twips}\" w:type=\"dxa\"/><w:vMerge/></w:tcPr><w:p/></w:tc>"
));
crates/hwp-convert/src/docx.rs:690
sect_prclaims to use the lastSectionDefpage settings, but the iterator usesfind_map, which returns the first match. In multi-section documents this will apply the wrong page size/margins/orientation.
/// 마지막 구역 정의의 페이지 설정 → sectPr (twips).
fn sect_pr(doc: &Document) -> String {
let page = doc
.sections
.iter()
.flat_map(|s| &s.paragraphs)
.flat_map(|p| &p.controls)
.find_map(|c| match c {
Control::SectionDef(sd) => sd.page.as_ref(),
_ => None,
});
crates/hwp-convert/src/docx.rs:237
- Control characters are currently emitted in a way that can produce invalid WordprocessingML:
HYPHEN/spaces become raw character data (not inside<w:t>), and a paragraph starting withLINE_BREAK/TABcan emit<w:br/>/<w:tab/>before any<w:r>is opened. Word expects these to be inside a run and text to be inside<w:t>.
HwpChar::CharCtrl(code) => {
flush_text!();
match *code {
ctrl_char::LINE_BREAK => out.push_str("<w:br/>"),
ctrl_char::HYPHEN => out.push('-'),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| that the specification is public). The former largest demand, **GJ-1** DOCX output, was resolved on 2026-08-01 (export only; input remains L-tier) — next large items are GA-2 and the GM-3/4/8 family | ||
| (DOCX output, open territory in OSS). |
|
|
||
| use hwp_model::{CharShape, Control, Document, HwpChar, NumFmt, Paragraph, ctrl_char}; | ||
|
|
||
| /// DOCX의 모든 문서를 ZIP(OPC)로 직렬화한다. |
| #[test] | ||
| fn 문서_구조와_본문() { | ||
| let doc = crate::from_markdown::from_markdown( |
| // hwp→docx — OPC 파트와 본문이 살아 있어야 한다 (GJ-1). | ||
| let src = fixture("samples/report-tables.hwpx"); |
entelecheia
left a comment
entelecheia
left a comment
There was a problem hiding this comment.
Reviewed the generated OOXML by running the branch and inspecting word/document.xml / word/numbering.xml. The wiring (CLI format, rels, content types, part layout) is sound, but five defects in the emitted XML are reproducible and would show up the moment the file is opened in Word. The unit tests pass because they only assert substring presence, never element order, cardinality or numeric values.
Blocking
1. Every run's font size is 100x too large - docx.rs:608
base_size is pt x 100 and rel_sizes is a percentage, so half-points are base_size * rel / 5000, not / 50. A 10pt body run emits <w:sz w:val="2000"/> (= 1000pt); Word's maximum is 1638 half-points.
Repro: hwp new -o t.hwp --from t.md && hwp convert t.hwp --to docx -o t.docx -> <w:sz w:val="2000"/> on every run. (The docDefaults sz=20 is correct, which is why extracted text still looks fine.)
2. Bullet lists reference an undefined numId - docx.rs:147 vs numbering_xml
ppr() computes numbering_id + 1 + BULLET_NUM_BASE for head_type == 3, but numbering_xml defines bullet w:num ids as BULLET_NUM_BASE + i (no +1).
Repro (- alpha / - beta): document.xml references numId 129, numbering.xml defines 1 and 128. The bullets are dropped.
3. Horizontally covered cells emit a phantom <w:tc> - docx.rs:415-425
covered does not distinguish colspan coverage from rowspan coverage, so a slot covered by gridSpan still emits a <w:tc> with <w:vMerge/>. The row then occupies more grid columns than <w:tblGrid> declares, and the phantom cell starts a vMerge with no restart above it.
Repro (<td colspan=2> + <td rowspan=2> over a 3-column grid): row 0 emits 3 <w:tc> for 4 grid columns - gridSpan(2) + vMerge + vMerge-restart. Colspan coverage should emit nothing; rowspan coverage should emit the continuation cell (and carry the merged cell's gridSpan).
4. <w:pPr> violates the CT_PPr schema - duplicate <w:spacing> and wrong child order - docx.rs:163-199
Line spacing and before/after spacing are emitted as two separate <w:spacing> elements (maxOccurs is 1, so the second one wins and line spacing is lost), and <w:jc> is emitted before <w:spacing>/<w:ind> while the schema sequence is pStyle, numPr, ..., spacing, ind, ..., jc.
Observed: <w:pPr><w:jc w:val="both"/><w:spacing w:line="480" w:lineRule="auto"/><w:ind w:left="20"/><w:spacing w:before="10" w:after="10"/></w:pPr>. Merge the attributes into one <w:spacing> and emit children in schema order.
5. Bare character data outside a run - docx.rs:238-241 (HYPHEN, NB_SPACE, FW_SPACE) and the equation fallback at docx.rs:352
These push raw text into out after flush_text!()/close_run(), so the characters land directly under <w:p> (or inside <w:r> but outside <w:t>), which the content model does not allow. The hyphen also ends up before the run instead of at its character position.
Observed for a paragraph "a-b": <w:p><w:pPr>...</w:pPr>-<w:r>...<w:t>ab</w:t></w:r></w:p>. The equation fallback has the same shape and is worse, since equation scripts are long. Route all of these through text_buf (or emit a dedicated <w:r><w:t> for them).
Non-blocking
- Empty paragraphs are dropped (
docx.rs:110). Blank paragraphs carry vertical spacing in HWP documents; consider emitting<w:p>with the pPr when the paragraph exists but has no inline content. numPris emitted wheneverhead_typeis 2/3, butnumbering.xmlis only written whennumbering_levels/bullet_charsare non-empty - a document with the former but not the latter gets a dangling numId.let _ = rel;in the hyperlink-close branch (docx.rs:246) is dead; takelink_openwithout binding.- Bullet
lvlTextinterpolates the raw char without escaping, and pinsrFontsto Symbol even for Hangul bullets. - The PR description is in Korean; per the language policy merged in #43, PR text, commit messages and release notes are English only.
Tests
Suggest raising the assertions to catch this class of bug: exact w:sz for a known point size, numId cross-checked against the ids defined in numbering.xml, <w:tc> count per row versus <w:gridCol> count, and one assertion that no <w:pPr> contains two <w:spacing> elements. The textutil smoke test cannot see any of these. P3 verification in Word is still pending and should catch 1/3/4/5 immediately.
- font size was emitted at 100x (base*rel/50): correct half-points conversion is base*rel/5000 (10pt body -> w:sz 20) - horizontally covered (colspan) grid slots must not emit <w:tc> at all; only vertical (rowspan) coverage emits vMerge, carrying the origin's gridSpan — coverage is now tracked as (from_above, span) - bullet numPr referenced numId 129 while numbering.xml emitted 128: bullets use BULLET_NUM_BASE + numbering_id, numbers index+1 - equation fallback script is wrapped in <w:r><w:t> (bare text under <w:p> is invalid WordprocessingML) - sectPr is now emitted at each section boundary (previous section in pPr) with the body terminator from the last section - regression tests: sz=20 and no 2000, first-row origin count, bullet numId 128 round-trip, equation run wrapper - new file's comments and test names translated to English per the comment convention; 12-feature-gaps summary contradiction and the ko typo fixed
Review resolution (b9a3d7c)All 10 comments addressed; P1 — Scale font sizes down to OOXML half-points P1 — Skip horizontally covered table grid positions P1 — Reference the emitted bullet numbering IDs P2 — Wrap equation fallback text in an OOXML run P2 — Emit section properties at each section boundary Copilot nits
|
Summary
로드맵 최대 수요 항목 GJ-1(DOCX 출력)을 구현한다.
convert --to docx가 IR에서 OOXML을 직접 합성한다(외부 라이브러리 없음, odt.rs 패턴). 출력 전용 — DOCX 입력은 L급 별도 항목으로 남는다.매핑
CLI:
ConvertFormat::Docx+.docx추론 +--out-dir확장자. MCPhwp_convert는 execute 경유로 자동 지원.Verification
scripts/check.sh그린 (fmt / clippy -D warnings / test / structured-corpus)<w:t>누락으로 텍스트가 비던 치명 결함을 textutil 스모크로 잡아 수정~/Documents/hwp-verification/P3_DOCX출력.docx·P3_표모음.docx— Microsoft Word에서 열어 확인 예정Docs