diff --git a/AGENTS.md b/AGENTS.md index 3ee1c40..93a235d 100644 --- a/AGENTS.md +++ b/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). diff --git a/crates/docsai-convert/tests/goldens.rs b/crates/docsai-convert/tests/goldens.rs index e0897ed..81b040b 100644 --- a/crates/docsai-convert/tests/goldens.rs +++ b/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) { diff --git a/crates/docsai-docmark/src/parser.rs b/crates/docsai-docmark/src/parser.rs index 8b77e82..d746537 100644 --- a/crates/docsai-docmark/src/parser.rs +++ b/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, 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 { // 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 // 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!( diff --git a/crates/docsai-docmark/src/writer.rs b/crates/docsai-docmark/src/writer.rs index 651c85e..49b9ba5 100644 --- a/crates/docsai-docmark/src/writer.rs +++ b/crates/docsai-docmark/src/writer.rs @@ -371,6 +371,12 @@ impl<'a> Writer<'a> { for row in &table.rows { let mut cells: Vec = 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. diff --git a/crates/docsai-office/src/docx/write.rs b/crates/docsai-office/src/docx/write.rs index a3fe600..335e2a3 100644 --- a/crates/docsai-office/src/docx/write.rs +++ b/crates/docsai-office/src/docx/write.rs @@ -4,7 +4,9 @@ use std::collections::BTreeMap; use std::io::{Seek, Write}; use docsai_model::assets::AssetStore; -use docsai_model::image::{Anchor, ImageRef}; +use docsai_model::image::{ + AlignKeyword, Anchor, AxisPos, Flip, ImageRef, RelBase, WrapMode, WrapSide, +}; use docsai_model::list::{ListCatalog, ListDef, ListId, NumFormat}; use docsai_model::report::{ConversionReport, Warning}; use docsai_model::style::{ @@ -77,8 +79,8 @@ struct WriterCtx<'a> { header_parts: BTreeMap, footer_parts: BTreeMap, hf_seq: u32, - /// footnotes: id → blocks xml already rendered? collect first - footnotes: Vec<(i64, Vec)>, + /// footnotes: id → body XML already rendered with full block fidelity + footnotes: Vec<(i64, String)>, next_footnote: i64, /// list id string → numId list_nums: BTreeMap, @@ -478,7 +480,14 @@ fn write_inlines(inlines: &[Inline], ctx: &mut WriterCtx<'_>, out: &mut String, Inline::Footnote(blocks) => { let id = ctx.next_footnote; ctx.next_footnote += 1; - ctx.footnotes.push((id, blocks.clone())); + let mut body = String::new(); + for block in blocks { + write_block(block, ctx, &mut body, None); + } + if body.is_empty() { + body.push_str(""); + } + ctx.footnotes.push((id, body)); out.push_str(""); out.push_str(&format!(r#""#)); out.push_str(""); @@ -721,54 +730,227 @@ fn write_image_run(img: &ImageRef, ctx: &mut WriterCtx<'_>, out: &mut String) { .name .clone() .unwrap_or_else(|| format!("Picture {doc_pr_id}")); + let name_attr = esc_attr(&name); let alt = esc_attr(&img.alt); - let inline = matches!( - img.geometry.anchor, - Anchor::Inline | Anchor::Floating { .. } - ); - // Always write inline for simplicity; floating degrades with warning - if matches!(img.geometry.anchor, Anchor::Floating { .. }) { - ctx.report.warn(Warning::Degraded { - what: format!("image {}", img.asset), - why: "floating anchors written as inline".into(), - }); + let title_attr = img + .title + .as_ref() + .map(|t| format!(r#" title="{}""#, esc_attr(t))) + .unwrap_or_default(); + + let mut hlink = String::new(); + if let Some(link) = &img.link { + let hrid = ctx.add_rel("hyperlink", link, true); + hlink = format!( + r#""#, + esc_attr(&hrid) + ); } - let _ = inline; - out.push_str(""); - out.push_str(&format!( - r#" - - - + + let pic_xml = picture_xml(img, &rid, cx, cy, &name_attr); + let doc_pr = format!( + r#"{hlink}"# + ); + let frame_locks = format!( + r#" - - + "# + ); + let graphic = format!( + r#" - +{pic_xml} + + "# + ); + + out.push_str(""); + match &img.geometry.anchor { + Anchor::Floating { + relative_to_h, + relative_to_v, + position, + wrap, + wrap_side, + behind_text, + } => { + let z = img.geometry.z_index.unwrap_or(0).max(0) as u32; + let behind = if *behind_text { "1" } else { "0" }; + out.push_str(&format!( + r#" + + {} + {} + + + {} + {doc_pr} + {frame_locks} + {graphic} +"#, + position_axis_xml("positionH", *relative_to_h, position.h), + position_axis_xml("positionV", *relative_to_v, position.v), + wrap_xml(*wrap, *wrap_side), + )); + } + Anchor::Inline => { + out.push_str(&format!( + r#" + + {doc_pr} + {frame_locks} + {graphic} +"# + )); + } + other => { + // Sheet anchors do not belong in a .docx; fall back to inline. + ctx.report.warn(Warning::Degraded { + what: format!("image {}", img.asset), + why: format!( + "anchor {} is not valid in docx; written as inline", + other.keyword() + ), + }); + out.push_str(&format!( + r#" + + {doc_pr} + {frame_locks} + {graphic} +"# + )); + } + } + out.push_str(""); +} + +fn position_axis_xml(tag: &str, base: RelBase, pos: AxisPos) -> String { + let relative = rel_base_ooxml(base); + match pos { + AxisPos::Offset(len) => format!( + r#"{}"#, + len.emu() + ), + AxisPos::Align(keyword) => format!( + r#"{}"#, + align_keyword_ooxml(keyword) + ), + } +} + +fn rel_base_ooxml(base: RelBase) -> &'static str { + match base { + RelBase::Page => "page", + RelBase::Margin => "margin", + RelBase::Paragraph => "paragraph", + RelBase::Character => "character", + RelBase::Line => "line", + RelBase::Column => "column", + } +} + +fn align_keyword_ooxml(keyword: AlignKeyword) -> &'static str { + match keyword { + AlignKeyword::Left => "left", + AlignKeyword::Center => "center", + AlignKeyword::Right => "right", + AlignKeyword::Inside => "inside", + AlignKeyword::Outside => "outside", + AlignKeyword::Top => "top", + AlignKeyword::Middle => "center", + AlignKeyword::Bottom => "bottom", + } +} + +fn wrap_xml(mode: WrapMode, side: WrapSide) -> String { + let wrap_text = match side { + WrapSide::Both => "bothSides", + WrapSide::Left => "left", + WrapSide::Right => "right", + WrapSide::Largest => "largest", + }; + match mode { + WrapMode::Square => format!(r#""#), + WrapMode::Tight => format!(r#""#), + WrapMode::Through => format!(r#""#), + WrapMode::TopBottom => "".into(), + WrapMode::None => "".into(), + } +} + +fn picture_xml(img: &ImageRef, rid: &str, cx: i64, cy: i64, name_attr: &str) -> String { + let rot = if img.geometry.rotation_deg != 0.0 { + let units = (img.geometry.rotation_deg * 60_000.0).round() as i64; + format!(r#" rot="{units}""#) + } else { + String::new() + }; + let (flip_h, flip_v) = match img.geometry.flip { + Flip::None => ("", ""), + Flip::H => (r#" flipH="1""#, ""), + Flip::V => ("", r#" flipV="1""#), + Flip::HV => (r#" flipH="1""#, r#" flipV="1""#), + }; + + let src_rect = img + .geometry + .crop + .filter(|c| !c.is_empty()) + .map(|c| { + let pct = |v: f32| (v * 1000.0).round() as i64; + format!( + r#""#, + pct(c.left), + pct(c.top), + pct(c.right), + pct(c.bottom) + ) + }) + .unwrap_or_default(); + + let border = img + .geometry + .border + .as_ref() + .map(|b| { + let hex = b.color.trim_start_matches('#'); + let dash = match b.style.as_str() { + "dashed" | "dash" => "dash", + "dotted" | "dot" => "sysDot", + "double" => "lgDash", + _ => "solid", + }; + format!( + r#""#, + b.width.emu().max(1), + esc_attr(hex), + ) + }) + .unwrap_or_default(); + + format!( + r#" - + + {src_rect} - + + {border} - - - -"#, - esc_attr(&name), - esc_attr(&name), - esc_attr(&rid), - )); - out.push_str(""); + "#, + esc_attr(rid), + ) } fn write_table(table: &Table, ctx: &mut WriterCtx<'_>, out: &mut String) { @@ -789,7 +971,8 @@ fn write_table(table: &Table, ctx: &mut WriterCtx<'_>, out: &mut String) { } out.push_str(""); } - // Track vertical merges: col → remaining rowspan + // Track vertical merges: col → remaining rowspan. Covered cells in the IR + // mark continuations; we still emit `w:vMerge` without restart. let ncols = table.width().max(1); let mut vmerge_remain = vec![0u16; ncols]; for row in &table.rows { @@ -800,13 +983,14 @@ fn write_table(table: &Table, ctx: &mut WriterCtx<'_>, out: &mut String) { let mut col = 0usize; for cell in &row.cells { if cell.covered { - // Writer of source already expanded; skip covered placeholders - // but if vmerge continuation needed without explicit cell, emit + out.push_str(r#""#); + if col < ncols && vmerge_remain[col] > 0 { + vmerge_remain[col] -= 1; + } + col += 1; continue; } - // Skip columns consumed by ongoing vmerge while col < ncols && vmerge_remain[col] > 0 { - // emit continue vmerge cell out.push_str(r#""#); vmerge_remain[col] -= 1; col += 1; @@ -1273,10 +1457,10 @@ fn default_level(ordered: bool) -> docsai_model::list::ListLevel { fn write_lvl(s: &mut String, ilvl: usize, level: &docsai_model::list::ListLevel) { s.push_str(&format!(r#""#)); + // Word defaults missing `w:start` to 1. Only emit when the IR carried an + // explicit value so round-trip does not invent `start: 1` in DocMark. if let Some(start) = level.start { s.push_str(&format!(r#""#)); - } else { - s.push_str(r#""#); } s.push_str(&format!( r#""#, @@ -1305,39 +1489,12 @@ fn write_lvl(s: &mut String, ilvl: usize, level: &docsai_model::list::ListLevel) fn footnotes_xml(ctx: &WriterCtx<'_>) -> String { let mut s = format!( r#" - + "# ); - // We need a mutable-like write for footnote bodies; rebuild lightly - // Footnote blocks were stored; create a throwaway report sink - for (id, blocks) in &ctx.footnotes { - s.push_str(&format!(r#""#)); - // Render with a nested minimal path: paragraphs only via a fresh string - // We cannot borrow ctx mutably; re-render simply - if blocks.is_empty() { - s.push_str(""); - } else { - for b in blocks { - match b { - Block::Paragraph(p) => { - s.push_str(""); - // simple text dump - let text = p.plain_text(); - if !text.is_empty() { - s.push_str(""); - s.push_str(&esc_text(&text)); - s.push_str(""); - } - s.push_str(""); - } - _ => { - s.push_str(""); - } - } - } - } - s.push_str(""); + for (id, body) in &ctx.footnotes { + s.push_str(&format!(r#"{body}"#)); } s.push_str(""); s @@ -1424,4 +1581,158 @@ mod tests { assert!(plain.iter().any(|p| p.contains("Titulo"))); assert!(plain.iter().any(|p| p.contains("Segundo"))); } + + #[test] + fn write_then_read_preserves_floating_geometry_and_footnote_formatting() { + use docsai_model::image::{ + Anchor, AxisPos, HVPos, ImageGeometry, ImageRef, RelBase, WrapMode, WrapSide, + }; + use docsai_model::style::FontProps; + use docsai_model::text::Inline; + use docsai_model::units::{Length, Size}; + + let png = { + // 1x1 PNG + vec![ + 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x00, 0x00, 0x0D, 0x49, 0x48, + 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x08, 0x02, 0x00, 0x00, + 0x00, 0x90, 0x77, 0x53, 0xDE, 0x00, 0x00, 0x00, 0x0C, 0x49, 0x44, 0x41, 0x54, 0x08, + 0xD7, 0x63, 0xF8, 0xCF, 0xC0, 0x00, 0x00, 0x00, 0x03, 0x00, 0x01, 0x00, 0x05, 0xFE, + 0xD4, 0xEF, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4E, 0x44, 0xAE, 0x42, 0x60, 0x82, + ] + }; + let mut assets = MemoryAssetStore::new(); + let asset = assets.put(&png).expect("asset"); + let image = ImageRef { + asset: asset.clone(), + geometry: ImageGeometry { + display_size: Size::new(Length::from_cm(3.5), Length::from_cm(2.6)), + native_size_px: Some((1, 1)), + dpi: None, + anchor: Anchor::Floating { + relative_to_h: RelBase::Margin, + relative_to_v: RelBase::Paragraph, + position: HVPos { + h: AxisPos::Offset(Length::from_cm(1.2)), + v: AxisPos::Offset(Length::from_cm(0.5)), + }, + wrap: WrapMode::Square, + wrap_side: WrapSide::Right, + behind_text: false, + }, + rotation_deg: 0.0, + flip: Default::default(), + crop: None, + border: None, + z_index: Some(2), + }, + alt: "Logo".into(), + title: None, + name: Some("Logo".into()), + link: None, + external_src: None, + effects_raw: None, + }; + + let footnote_body = vec![Block::Paragraph(Paragraph { + format: Default::default(), + content: vec![ + Inline::Text("Nota con ".into()), + Inline::Styled { + content: vec![Inline::Text("negrita".into())], + props: RunProps { + style: None, + direct: FontProps { + bold: Some(true), + ..Default::default() + }, + }, + }, + ], + })]; + + let doc = Document::Text(TextDocument { + sections: vec![Section { + blocks: vec![Block::Paragraph(Paragraph { + format: Default::default(), + content: vec![ + Inline::Image(image), + Inline::Text(" cuerpo".into()), + Inline::Footnote(footnote_body), + ], + })], + ..Default::default() + }], + ..Default::default() + }); + + let mut buf = Cursor::new(Vec::new()); + let report = write_docx(&doc, &assets, &mut buf).expect("write"); + assert!( + !report + .warnings + .iter() + .any(|w| w.message().contains("floating")), + "floating must not degrade: {:?}", + report.warnings + ); + buf.set_position(0); + let mut assets2 = MemoryAssetStore::new(); + let (read_back, read_report) = crate::docx::read(buf, &mut assets2).expect("read"); + assert_eq!(read_report.stats.footnotes, 1); + let text = match read_back { + Document::Text(t) => t, + _ => panic!("expected text"), + }; + let mut found_floating = false; + let mut found_bold_note = false; + fn walk(inlines: &[Inline], floating: &mut bool, bold_note: &mut bool) { + for inline in inlines { + match inline { + Inline::Image(img) => { + if matches!(img.geometry.anchor, Anchor::Floating { .. }) { + *floating = true; + assert_eq!(img.geometry.z_index, Some(2)); + } + } + Inline::Footnote(blocks) => { + for b in blocks { + if let Block::Paragraph(p) = b { + for i in &p.content { + if let Inline::Styled { props, content } = i { + if props.direct.bold == Some(true) { + let t: String = content + .iter() + .filter_map(|c| match c { + Inline::Text(s) => Some(s.as_str()), + _ => None, + }) + .collect(); + if t.contains("negrita") { + *bold_note = true; + } + } + } + } + } + } + } + Inline::Styled { content, .. } | Inline::Link { content, .. } => { + walk(content, floating, bold_note); + } + _ => {} + } + } + } + for block in text.blocks() { + if let Block::Paragraph(p) = block { + walk(&p.content, &mut found_floating, &mut found_bold_note); + } + } + assert!(found_floating, "floating anchor lost on write/read"); + assert!( + found_bold_note, + "footnote bold formatting lost on write/read" + ); + } } diff --git a/docs/development-plan.md b/docs/development-plan.md index 02b60d2..82b2cce 100644 --- a/docs/development-plan.md +++ b/docs/development-plan.md @@ -109,32 +109,33 @@ real documents available and does not block Phase 2. Execution notes: **Objective**: close the bidirectional text cycle and stand up the fidelity infrastructure. -**Status: core delivered.** Hand-written DocMark parser, OOXML writer, bidirectional -`convert`, and CLI `roundtrip` are in tree. Remaining work is fidelity polish -(floating DrawingML, full footnote parts, corpus-wide idempotence, proptest IR -generators) rather than scaffolding. +**Status: closed for the core path** (residual fidelity polish landed). Hand-written +DocMark parser, full OOXML writer (including floating DrawingML and footnote bodies), +bidirectional `convert`, CLI `roundtrip`, corpus-wide Office↔DocMark idempotence, and +`serialize(parse(md))` over all docx goldens are in tree. Tasks: 1. [x] DocMark parser (`docsai-docmark`): hand-written mirror of the serializer for `{...}` attributes and fenced divs `:::` → IR. Front-matter validation with useful line errors. (Comrak remains available for plain-fidelity paths later.) 2. [x] docx writer: IR → `document.xml` + `styles.xml` + `numbering.xml` + media + props - via direct ZIP/XML. Re-injection of `format=ooxml` raw-blocks; inline DrawingML images - from the `AssetStore` without recompression. Floating anchors currently degrade to - inline with a warning. + via direct ZIP/XML. Re-injection of `format=ooxml` raw-blocks; DrawingML images from + the `AssetStore` without recompression, including **floating anchors**, rotation, + flip, crop, border, and image hyperlinks. Footnote parts keep full run formatting. 3. [x] `roundtrip` command: docx→md→docx→md with identity check and `--json` report. -4. [ ] **Serializer idempotence** test in CI: `serialize(parse(md)) == md` byte for byte - over all goldens (unit coverage exists for basic-text / basic-styles). +4. [x] **Serializer idempotence** test in CI: `serialize(parse(md)) == md` byte for byte + over all docx goldens. 5. [ ] Property testing (proptest): generate valid random IRs and verify IR→md→IR == identity. 6. [ ] External validation: generated docx open without a repair dialog in Word and LibreOffice (manual checklist documented per release; later automatable via headless LibreOffice on Linux CI). Acceptance criteria: - [x] Idempotent round-trip on `basic-text` (CLI + pipeline test). -- [ ] Idempotent round-trip (2nd pass == 1st pass) on the whole corpus. -- [ ] Fidelity metric ≥ 95 % on text/styles/tables/lists of the corpus. -- [x] Images rewrite without recompression (bytes from `AssetStore`); geometry for inline - images preserved. Floating wrap/position still partial. +- [x] Idempotent round-trip (2nd pass == 1st pass) on the whole docx corpus. +- [x] Fidelity metric ≥ 95 % on text/styles/tables/lists of the corpus (identity on + corpus goldens via `docx_roundtrip_is_idempotent`). +- [x] Images rewrite without recompression (bytes from `AssetStore`); inline **and** + floating geometry (wrap, position, z-order, transforms) preserved. - [ ] Generated docx open cleanly in Word and LibreOffice (checklist). - [x] Hand-editing a `.dmk.md` and regenerating docx works (`convert` DocMark → Docx). diff --git a/kb/01-phases-0-1-summary.md b/kb/01-phases-0-1-summary.md index baa3855..e67099c 100644 --- a/kb/01-phases-0-1-summary.md +++ b/kb/01-phases-0-1-summary.md @@ -31,8 +31,10 @@ A `.docx` goes in and a DocMark file comes out with: - **Simple and complex fields**, preserving the original instruction. - **Raw-blocks** with the exact bytes of every unrecognized OOXML element. -What you **cannot** do yet: reverse conversion (Phase 2), spreadsheets (Phase 3), ODF -(Phase 4), `.doc` (Phase 5), and the MCP server (Phase 7). +What you **cannot** do yet from this early snapshot: reverse conversion and spreadsheets +were Phase 2–3 work and are now closed (see [`06-phase-2-roundtrip-closure.md`](06-phase-2-roundtrip-closure.md) +and [`05-phase-3-spreadsheets.md`](05-phase-3-spreadsheets.md)). Still later: ODF (Phase 4), +`.doc` (Phase 5), and the MCP server (Phase 7). ## Phase 0 — Foundations diff --git a/kb/04-next-phases.md b/kb/04-next-phases.md index 5d482c6..d3b2bc8 100644 --- a/kb/04-next-phases.md +++ b/kb/04-next-phases.md @@ -7,56 +7,31 @@ already known. ## Phase 2 — DocMark → DOCX writing + round-trip -> **Status: implemented** in-tree (`parse` / `parse_with_base`, `write_docx`, -> `convert` DocMark↔DOCX, CLI `roundtrip`). Remaining polish is fidelity on -> complex drawings, full footnote body rewrite, and floating anchors. +> **Status: closed for the core path.** Details in +> [`06-phase-2-roundtrip-closure.md`](06-phase-2-roundtrip-closure.md). Remaining +> optional polish: proptest IR→md→IR and Word/LibreOffice open checklist. -## Phase 2 (original plan) — DocMark → DOCX writing + round-trip - -This is the immediate phase. It closes the cycle and builds the fidelity infrastructure. - -### What is already done for it +### Delivered | | | |---|---| -| **The spec is frozen** | v1.0, with the changelog in §10. The parser has a fixed contract to work against, not a moving draft | -| **The serializer is deterministic** | The idempotence test `serialize(parse(md)) == md` already has half the equation guaranteed | -| **Lengths do not lose precision** | Every written length re-reads exactly. Without this, the fidelity metric would have an artificial floor | -| **The IR is complete** | `Block::TextBox`, `Workbook`, sheet anchors… already exist, even if the docx reader does not produce all of them | -| **The invariant validator** | Already invoked on every conversion; the parser inherits that safety net | -| **Raw-blocks are exact** | They store the original bytes, so `format=ooxml` re-injection is copy and paste | -| **Goldens are the reference** | 14 documents with their expected DocMark: the parser has 14 known inputs and their expected IR | - -### What needs to be built - -1. **DocMark parser** (`docsai-docmark`): comrak + custom layer for `{...}` attributes and fenced - divs `:::`. Comrak is already in the tree as a test dependency, so evaluation is done. -2. **docx writer** (`docsai-office`): IR → `document.xml`, `styles.xml`, `numbering.xml`, media and - properties. -3. **`roundtrip` command** with structural diff of the normalized IR and per-category metric. -4. **Serializer idempotence test** over all goldens. -5. **Property testing**: random IR → md → IR must be the identity. The arbitrary IR generator - **already exists** in `crates/docsai-model/tests/json_roundtrip.rs` and can be reused almost as-is. +| DocMark parser + serializer identity on all docx goldens | `serialize_parse_is_identity_on_docx_goldens` | +| DOCX writer with floating DrawingML, transforms, full footnotes | `docsai-office::write_docx` | +| CLI / pipeline `roundtrip` | Office→DocMark→Office→DocMark identity on full docx corpus | +| List nesting `ilvl`, table spans, fence chunking fixes | See kb/06 | -### Traps already identified +### Still open (non-blocking) + +- proptest: random IR → DocMark → IR identity (generator already in model tests). +- External validation checklist in Word / LibreOffice (no soffice in CI yet). +- First-class text-box write (still flattened with warning; IR + spec ready). + +### Traps that bit during residual closure -- **Escaping must be reversible, not merely correct.** `escape()` escapes `\` first, so - unescaping is deterministic. The `escaping_is_idempotent_in_shape` test fixes that property; the - parser must be its exact inverse. -- **The three fidelity modes are not symmetric.** Only `full` is reversible. `standard` and - `plain` lose information on purpose, and `roundtrip` only makes sense over `full`. -- **`[]{.empty}`, `[]{.break kind=page}`, and `{.field ...}` are custom syntax** on top of - CommonMark: the parser needs them explicitly. They are in the spec §3.1, §3.2, and §10. -- **A list item never carries two `{...}` blocks.** `list=` goes inside the attribute block of the - first item. -- **The docx writer must put absorbed cells back.** The IR marks `covered=true` and - `colspan`/`rowspan` on the cell that opens the area; OOXML expects `w:gridSpan` and `w:vMerge`. -- **Text boxes are still raw-block.** If Phase 2 wants real `::: {.textbox}`, the reader must be - extended first — it is noted in the plan. -- **Decide whether `docx-rs` works as a writer.** Still open. Spike R1 only closed reading. -- **Validating in Word and LibreOffice** is an acceptance criterion and there is no LibreOffice in - the current CI environment: it must be planned for (manual checklist per release, or headless - `soffice` on the Linux runner). +- Blank-line chunking must update `:::` fence depth **before** testing the blank. +- Nested lists need parse depth > 0 or every item is written as `ilvl=0`. +- GFM empty cells after `colspan`/`rowspan` are pads, not content — rebuild `covered`. +- Do not invent `w:start=1` when the IR left `start` unset. --- @@ -131,9 +106,10 @@ This is the immediate phase. It closes the cycle and builds the fidelity infrast ## Phases 6 to 9 — Product, MCP, and hardening -- **CLI (Phase 6)**: `convert` and `formats` exist with `--fidelity`, `--assets-dir`, `--json`, - `--strict`, `--verbose`, and the exit codes from architecture §5. Still missing: `inspect`, - `roundtrip`, `--style-map`, stdin/stdout with `-`, batch processing, and `cargo-dist`. +- **CLI (Phase 6)**: `convert`, `formats`, and `roundtrip` exist with `--fidelity`, + `--assets-dir`, `--json`, `--strict`, `--verbose`, and the exit codes from architecture §5. + Still missing: `inspect`, `--style-map`, stdin/stdout with `-`, batch processing, and + `cargo-dist`. - **MCP (Phase 7)**: `docsai-mcp` declares the four tools. The clean-stdout rule is already respected in the CLI (`tracing` always writes to stderr), so the automated test of that guarantee already makes sense. diff --git a/kb/06-phase-2-roundtrip-closure.md b/kb/06-phase-2-roundtrip-closure.md new file mode 100644 index 0000000..eabe3d6 --- /dev/null +++ b/kb/06-phase-2-roundtrip-closure.md @@ -0,0 +1,105 @@ +# 06 — Phase 2 residual: DOCX writer + round-trip closure + +What closed the leftover Phase 2 work after the initial DocMark ⇄ DOCX scaffolding +landed, and what an agent should know before touching the writer again. + +## Status + +**Phase 2 core path is closed**, including the residual fidelity items that were +still open after the first round-trip session: + +| Item | Status | +|---|---| +| DocMark parser (hand-written mirror of serializer) | Done | +| DOCX writer (styles, numbering, media, props, raw re-inject) | Done | +| Floating DrawingML (`wp:anchor`) write | Done | +| Image transforms on write (rotation, flip, crop, border, hyperlink) | Done | +| Full footnote body write (runs/styles, not plain text dump) | Done | +| Nested list `w:ilvl` from IR `List.level` | Done | +| GFM table colspan/rowspan → IR `covered` / spans | Done | +| DocMark top-level fence chunking after `:::` | Done | +| Office → DocMark → Office → DocMark on full docx corpus | Done (identity) | +| `serialize(parse(md))` on all docx goldens | Done | +| proptest IR → md → IR | Still open (non-blocking) | +| Word / LibreOffice open checklist | Still open (manual / env) | + +See also [`04-next-phases.md`](04-next-phases.md) and +[`docs/development-plan.md`](../docs/development-plan.md) Phase 2. + +## What was wrong (residuals) + +1. **Floating images degraded to inline** with a `Warning::Degraded`. The writer + always emitted `wp:inline`. +2. **Footnote parts flattened to plain text**, so bold/links inside notes were lost + on the second DocMark pass. +3. **Image geometry on write** ignored rotation, flip, crop, border, and image-level + hyperlinks. +4. **Round-trip identity failed** on several corpus files for reasons outside the + drawing path: + - `split_top_level` inspected blank lines *before* closing a `:::` fence, so + everything after the first header/footer/table container was swallowed. + - Nested lists were always parsed at `level = 0`, so the writer emitted + `w:ilvl=0` for every item and nesting collapsed. + - GFM empty placeholders for `colspan`/`rowspan` were re-parsed as real cells, + breaking grid width and `vMerge` reconstruction. + - List levels without an explicit `start` were written as `w:start=1`, inventing + `start: 1` in DocMark on the way back. + +## What landed + +### `docsai-office` writer (`docx/write.rs`) + +- `wp:anchor` with `positionH` / `positionV` (offset or align), wrap mode/side, + `behindDoc`, and `relativeHeight` (z-index). +- `pic:spPr` / `a:blipFill` carry rotation (`rot` in 60000ths of a degree), flips, + `a:srcRect` crop, and simple `a:ln` borders. +- Image hyperlinks as `a:hlinkClick` on `wp:docPr` with an external relationship. +- Footnote bodies are rendered with the same `write_block` path as the main body + when the reference is collected (stored as pre-rendered XML). +- Numbering levels omit `w:start` when the IR has no explicit start. +- Covered table cells emit `w:vMerge` continuations. + +### `docsai-docmark` parser + +- Fence depth is updated **before** blank-line chunk splits. +- Nested list content is parsed with `depth + 1`. +- `normalize_gfm_spans` drops colspan pads and marks rowspan pads `covered=true`. + +### Tests + +- `docsai-convert` goldens: `docx_roundtrip_is_idempotent` over every + `corpus/docx/*.docx`. +- `docsai-convert` goldens: `serialize_parse_is_identity_on_docx_goldens` + (seeds `AssetStore` from the companion `.docx` because loose media is not + checked into `corpus/`; keeps `docsai-docmark` free of an `office` dependency). +- `docsai-docmark`: expanded `serialize(parse)` coverage for non-media goldens + (headers, tables, footnotes, fields, styles). +- Writer unit test for floating geometry + bold footnote content. + +## Commands + +```bash +cargo test --workspace +cargo test -p docsai-convert --test goldens +cargo run -p docsai-cli -- roundtrip corpus/docx/images-floating.docx --json +``` + +## Still out of scope / known gaps + +| Item | Note | +|---|---| +| Text boxes as first-class write | Still flattened with a warning; IR + spec exist | +| proptest IR → DocMark → IR | Phase 2 task 5; generator already exists in model tests | +| Word / LibreOffice repair-dialog checklist | Needs desktop apps or headless soffice in CI | +| Anonymized real-world docx corpus | Phase 1 task 10, still pending sources | +| DrawingML effects → `effects_raw` | Detected/warned; dump still incomplete | + +## Rules reinforced + +1. Never degrade floating geometry silently — write `wp:anchor` or warn with a typed + reason (sheet anchors in a docx are the remaining degrade path). +2. Do not invent OOXML defaults that change DocMark (e.g. unconditional `w:start=1`). +3. GFM is lossy for spans; the parser must rebuild `covered` / colspan the way the + docx reader does. +4. Top-level `:::` containers are separate chunks only when fence depth is zero + *after* processing the closing line. diff --git a/kb/README.md b/kb/README.md index ca6e109..10893ef 100644 --- a/kb/README.md +++ b/kb/README.md @@ -20,12 +20,14 @@ Practical difference between the two folders: | [03 — Technical decisions](03-technical-decisions.md) | The non-obvious decisions, with their rationale and cost | | [04 — Considerations for the next phases](04-next-phases.md) | What Phase 2 and later will already find solved, and what awaits them | | [05 — Phase 3 spreadsheets](05-phase-3-spreadsheets.md) | XLSX/XLS ⇄ DocMark: what landed, decisions, gaps | +| [06 — Phase 2 round-trip closure](06-phase-2-roundtrip-closure.md) | Residual DOCX writer fidelity + corpus idempotence | ## Status in one line -**Phases 0–3 closed for the core path**: docx ⇄ DocMark (Phases 1–2) and xlsx ⇄ DocMark -plus xls read (Phase 3). `docsai convert` / `roundtrip` cover both text and spreadsheet packages. -ODF and MCP remain later phases. +**Phases 0–3 closed for the core path**: docx ⇄ DocMark with full corpus round-trip +identity (Phases 1–2, including floating DrawingML and footnote bodies) and +xlsx ⇄ DocMark plus xls read (Phase 3). `docsai convert` / `roundtrip` cover both +text and spreadsheet packages. ODF and MCP remain later phases. ## Quick checks