feat(office): close DOCX round-trip residuals and document Phase 2 closure

AGENTS.md

@@ -14,8 +14,10 @@ minimal format loss. It is invoked as a CLI or as an **MCP server over stdio**.
 has the seven crates, the IR (`docsai-model`), the `.docx` and `.xlsx` readers
 **and writers** plus `.xls` read (`docsai-office`), DocMark serialize **and parse**
 for text and workbooks (`docsai-docmark`), orchestration (`docsai-convert`), and
-the CLI with `convert`, `formats` and `roundtrip`. `docsai-odf` and `docsai-mcp`
-remain skeletons that only fix the dependency rules.
+the CLI with `convert`, `formats` and `roundtrip`. The DOCX writer covers floating
+DrawingML, image transforms, and full footnote bodies; the full docx corpus
+round-trips with DocMark identity. `docsai-odf` and `docsai-mcp` remain skeletons
+that only fix the dependency rules.
 
 Next up is **Phase 4**: ODF (`.odt` / `.ods` ⇄ DocMark).
 

crates/docsai-convert/tests/goldens.rs

@@ -292,6 +292,64 @@ fn xlsx_roundtrip_is_idempotent() {
     }
 }
 
+/// Office → DocMark → Office → DocMark idempotence for the docx corpus
+/// (Phase 2 residual: floating images, footnotes, lists, tables, headers).
+#[test]
+fn docx_roundtrip_is_idempotent() {
+    for document in documents("docx", "docx") {
+        let outcome = docsai_convert::roundtrip_file(
+            &document,
+            None,
+            &docsai_convert::ConvertOptions::default(),
+        )
+        .unwrap_or_else(|e| panic!("{}: {e}", document.display()));
+        assert!(
+            outcome.identical,
+            "{} second DocMark differs from the first:\n--- first ---\n{}\n--- second ---\n{}",
+            document.display(),
+            outcome.first_markdown,
+            outcome.second_markdown
+        );
+    }
+}
+
+/// Phase 2: `serialize(parse(md)) == md` over every docx golden.
+///
+/// Image goldens reference `assets/img-*.png` that only exist inside the
+/// companion `.docx`; the store is seeded from that package first so asset
+/// ids stay stable without checking loose media into the corpus.
+#[test]
+fn serialize_parse_is_identity_on_docx_goldens() {
+    for document in documents("docx", "docx") {
+        let golden = golden_path(&document);
+        let md = std::fs::read_to_string(&golden)
+            .unwrap_or_else(|e| panic!("{}: {e}", golden.display()));
+        let mut assets = MemoryAssetStore::new();
+        let file = std::fs::File::open(&document)
+            .unwrap_or_else(|e| panic!("{}: {e}", document.display()));
+        let _ = docsai_office::read_docx(file, &mut assets)
+            .unwrap_or_else(|e| panic!("{}: {e}", document.display()));
+        let base = golden.parent();
+        let (doc, _) = docsai_docmark::parse_with_base(&md, base, &mut assets)
+            .unwrap_or_else(|e| panic!("{}: {e}", golden.display()));
+        let (out, _) = docsai_docmark::serialize(
+            &doc,
+            &assets,
+            &Options {
+                fidelity: Fidelity::Full,
+                assets_dir: "assets".into(),
+                source_format: Format::Docx,
+            },
+        );
+        assert_eq!(
+            out,
+            md,
+            "{}: serialize(parse(md)) must equal md",
+            golden.display()
+        );
+    }
+}
+
 /// Writes a synthetic document of roughly 50 pages: paragraphs, headings and
 /// tables, all of them things the reader must handle at speed.
 fn build_large_docx(path: &Path, paragraphs: usize) {

crates/docsai-docmark/src/parser.rs

@@ -613,6 +613,7 @@ impl<'a> BodyParser<'a> {
                 }
             }
         }
+        normalize_gfm_spans(&mut table);
         Ok(table)
     }
 
@@ -670,17 +671,48 @@ impl<'a> BodyParser<'a> {
             let rest = strip_list_indent(&rest, item.marker_len);
             blocks.push(Block::Paragraph(para));
             if !rest.trim().is_empty() {
-                // May contain nested list and/or more paragraphs
-                let nested = self.parse_normal_blocks(&rest, line)?;
-                // If first nested is list, keep tight
+                // Nested lists must carry depth+1 so the docx writer emits the
+                // correct `w:ilvl` (and DocMark nesting survives Office round-trip).
+                let nested = self.parse_blocks_at_list_depth(&rest, line, depth + 1)?;
                 blocks.extend(nested);
             }
-            // Remove empty trailing?
             list.items.push(ListItem { blocks });
         }
         Ok(list)
     }
 
+    /// Like [`parse_normal_blocks`], but nested lists inherit `list_depth`.
+    fn parse_blocks_at_list_depth(
+        &mut self,
+        text: &str,
+        start_line: usize,
+        list_depth: u8,
+    ) -> Result<Vec<Block>, ParseError> {
+        let chunks = split_top_level(text);
+        let mut out = Vec::new();
+        let mut line = start_line;
+        for chunk in chunks {
+            let chunk_line = line;
+            line += chunk.bytes().filter(|b| *b == b'\n').count() + 2;
+            if chunk.trim().is_empty() {
+                continue;
+            }
+            let trimmed = chunk.trim_start_matches('\n');
+            if looks_like_list(trimmed) {
+                let list = self.parse_list(trimmed, chunk_line, list_depth)?;
+                self.report.stats.lists += 1;
+                out.push(Block::List(list));
+                continue;
+            }
+            if let Some(ParsedBlock::Normal(b)) =
+                self.parse_container_or_block(chunk, chunk_line)?
+            {
+                out.push(b);
+            }
+        }
+        Ok(out)
+    }
+
     fn parse_paragraph_block(&mut self, text: &str) -> Result<Paragraph, ParseError> {
         // Hard breaks: line ending with two spaces
         let mut joined = String::new();
@@ -1551,6 +1583,100 @@ fn parse_raw_block(attrs: &Attrs, body: &str) -> Result<RawFragment, ParseError>
 // Chunk / list / table utilities
 // ---------------------------------------------------------------------------
 
+fn cell_is_visually_empty(cell: &TableCell) -> bool {
+    cell.blocks.is_empty()
+        || matches!(
+            cell.blocks.as_slice(),
+            [Block::Paragraph(p)] if p.is_empty()
+        )
+}
+
+/// GFM cannot omit absorbed cells, so the serializer pads colspan/rowspan with
+/// empty placeholders. Rebuild the IR shape the docx reader uses: real cells
+/// only, with `covered=true` markers for rowspan continuations.
+fn normalize_gfm_spans(table: &mut Table) {
+    // Drop empty placeholders that only exist to keep a colspan rectangular.
+    for row in &mut table.rows {
+        let mut out = Vec::with_capacity(row.cells.len());
+        let mut absorb = 0u16;
+        for cell in std::mem::take(&mut row.cells) {
+            if absorb > 0 {
+                if cell_is_visually_empty(&cell) && cell.colspan <= 1 && cell.rowspan <= 1 {
+                    absorb -= 1;
+                    continue;
+                }
+                absorb = 0;
+            }
+            if cell.colspan > 1 {
+                absorb = cell.colspan.saturating_sub(1);
+            }
+            out.push(cell);
+        }
+        row.cells = out;
+    }
+
+    // Mark empty cells that sit under an active rowspan as covered, matching
+    // the docx reader. Walk grid columns so multi-colspan parents occupy the
+    // right slots.
+    let ncols = table.width().max(1);
+    let mut remain = vec![0u16; ncols];
+    for row in &mut table.rows {
+        let mut col = 0usize;
+        let mut out = Vec::with_capacity(row.cells.len() + 2);
+        let pending = std::mem::take(&mut row.cells);
+        let mut iter = pending.into_iter();
+        while col < ncols {
+            if remain[col] > 0 {
+                // Prefer converting an empty GFM pad into a covered marker.
+                match iter.next() {
+                    Some(cell)
+                        if cell_is_visually_empty(&cell)
+                            && cell.colspan <= 1
+                            && cell.rowspan <= 1 =>
+                    {
+                        out.push(TableCell {
+                            covered: true,
+                            ..Default::default()
+                        });
+                    }
+                    Some(cell) => {
+                        // Content where a continuation was expected: keep it,
+                        // and still clear the remaining rowspan for this slot.
+                        out.push(cell);
+                    }
+                    None => {
+                        out.push(TableCell {
+                            covered: true,
+                            ..Default::default()
+                        });
+                    }
+                }
+                remain[col] -= 1;
+                col += 1;
+                continue;
+            }
+
+            let Some(cell) = iter.next() else {
+                break;
+            };
+            let span = cell.colspan.max(1) as usize;
+            if cell.rowspan > 1 {
+                for slot in remain
+                    .iter_mut()
+                    .skip(col)
+                    .take(span.min(ncols.saturating_sub(col)))
+                {
+                    *slot = cell.rowspan - 1;
+                }
+            }
+            out.push(cell);
+            col += span;
+        }
+        out.extend(iter);
+        row.cells = out;
+    }
+}
+
 fn split_top_level(text: &str) -> Vec<&str> {
     let mut chunks = Vec::new();
     let mut start = 0usize;
@@ -1559,24 +1685,10 @@ fn split_top_level(text: &str) -> Vec<&str> {
     let mut fence_depth = 0i32;
     while i < bytes.len() {
         if bytes[i] == b'\n' {
-            // Check blank line
-            let mut j = i + 1;
-            while j < bytes.len() && bytes[j] == b'\r' {
-                j += 1;
-            }
-            if j < bytes.len() && bytes[j] == b'\n' {
-                // blank line at j
-                if fence_depth == 0 {
-                    let chunk = &text[start..i];
-                    if !chunk.trim().is_empty() {
-                        chunks.push(chunk);
-                    }
-                    start = j + 1;
-                    i = j + 1;
-                    continue;
-                }
-            }
-            // track fences on this line
+            // Account for fences on the line that just ended *before* deciding
+            // whether a following blank line is a top-level chunk boundary.
+            // Otherwise a closing `:::` still looks "open" when the blank that
+            // follows it is inspected, and the rest of the body is swallowed.
             let line_start = text[..i].rfind('\n').map(|p| p + 1).unwrap_or(0);
             let line = &text[line_start..i];
             let t = line.trim();
@@ -1588,6 +1700,20 @@ fn split_top_level(text: &str) -> Vec<&str> {
                     fence_depth += 1;
                 }
             }
+
+            let mut j = i + 1;
+            while j < bytes.len() && bytes[j] == b'\r' {
+                j += 1;
+            }
+            if j < bytes.len() && bytes[j] == b'\n' && fence_depth == 0 {
+                let chunk = &text[start..i];
+                if !chunk.trim().is_empty() {
+                    chunks.push(chunk);
+                }
+                start = j + 1;
+                i = j + 1;
+                continue;
+            }
         }
         i += 1;
     }
@@ -1943,6 +2069,21 @@ mod tests {
         round_trip_golden("nested-lists");
     }
 
+    #[test]
+    fn round_trip_headers_footers_and_tables() {
+        // Residual Phase 2 fixtures that need no on-disk media.
+        for name in [
+            "headers-footers",
+            "table-simple",
+            "table-merged",
+            "footnotes",
+            "custom-styles",
+            "fields-raw",
+        ] {
+            round_trip_golden(name);
+        }
+    }
+
     #[test]
     fn parses_nested_lists_golden() {
         let path = concat!(

crates/docsai-docmark/src/writer.rs

@@ -371,6 +371,12 @@ impl<'a> Writer<'a> {
         for row in &table.rows {
             let mut cells: Vec<String> = Vec::new();
             for cell in &row.cells {
+                if cell.covered {
+                    // Occupies one grid slot under a rowspan above; keep the
+                    // GFM rectangle rectangular without re-emitting content.
+                    cells.push(String::new());
+                    continue;
+                }
                 cells.push(self.render_table_cell(cell));
                 // A horizontal span leaves the absorbed columns empty so that
                 // the GFM grid stays rectangular.