Split reconciler spec into per-concern files

TODO/11-split-reconciler-spec.md

@@ -1,55 +1,119 @@
-# 13 — Split `reconciler_spec.rb` (1980 lines) into per-concern files
+# 13 — Split `reconciler_spec.rb` into per-concern files
 
 **Priority:** Medium (spec organization)
-**Files:** Split `spec/uniword/docx/reconciler_spec.rb` into
-`spec/uniword/docx/reconciler/` directory
+**Files:** `spec/uniword/docx/reconciler_spec.rb` →
+`spec/uniword/docx/reconciler/`
 
-## Problem
+## Status of the original note
+
+Numbers are stale and have grown. It said 1980 lines and 12 top-level
+describes; actual today is **2054 lines and 21 describes**. That growth
+is the argument for doing this now.
+
+`spec/uniword/docx/reconciler/` **already exists** with three files
+(`fix_codes_spec.rb`, `helpers_spec.rb`, `referential_integrity_spec.rb`),
+so this extends an established layout rather than inventing one.
 
-`reconciler_spec.rb` is 1980 lines with 12 top-level `describe` blocks
-covering: footnotes, endnotes, profiles, content types, package rels,
-document rels, referential integrity (4 sub-sections), numbering,
-tables, value preservation, headers/footers, and clear_stored_namespace_plans.
+## Problem
 
-This is a monolith. Slow to navigate, slow to run individually, hard
-to see what's tested and what's not.
+A 2054-line monolith with 21 top-level describes. Slow to navigate, slow
+to run a single concern, hard to see what is covered.
 
 ## Fix
 
-Split into per-concern files under `spec/uniword/docx/reconciler/`:
-
-```
-spec/uniword/docx/reconciler/
-├── footnotes_spec.rb
-├── endnotes_spec.rb
-├── settings_spec.rb
-├── content_types_spec.rb
-├── package_rels_spec.rb
-├── document_rels_spec.rb
-├── numbering_spec.rb
-├── tables_spec.rb
-├── headers_footers_spec.rb
-├── note_references_spec.rb
-├── style_references_spec.rb
-├── hyperlink_references_spec.rb
-├── clear_namespace_plans_spec.rb
-└── value_preservation_spec.rb
-```
-
-Each file has its own `RSpec.describe Uniword::Docx::Reconciler do`
-block with only the relevant `let`s and helpers.
-
-### Verification
-
-- All examples preserved (no test deleted, only moved)
-- Each file runs independently: `rspec spec/uniword/docx/reconciler/footnotes_spec.rb`
-- Combined example count matches the original 101
-
-### Migration notes
-
-- The shared `let(:settings_class) { ... }` etc. either move to a
-  `spec/support/reconciler_helpers.rb` file or get duplicated in each
-  spec file (preferred for clarity — small duplication beats hidden
-  shared state).
-- Keep the integration smoke-test in `reconciler_spec.rb` (one example
-  that exercises the full reconcile pipeline end-to-end).
+Split along `lib/uniword/docx/reconciler/`'s actual module boundaries, so
+each spec file has an obvious owner. The original note proposed filenames
+invented from describe titles; those titles mislead in several places.
+
+Ownership below was checked against the implementing method, not guessed
+from the title.
+
+| lib module | spec file | absorbs |
+| --- | --- | --- |
+| `notes.rb` | `notes_spec.rb` | footnotes, endnotes, note reference validation (R10), note definition integrity (R15/R16), notes reorder |
+| `tables.rb` | `tables_spec.rb` | table reconciliation, table gridAfter |
+| `body.rb` | `body_spec.rb` | headers/footers, existing value preservation |
+| `parts.rb` | `parts_spec.rb` | numbering reconciliation (`Parts#reconcile_numbering`, `parts.rb:152`) |
+| `package_structure.rb` | `package_structure_spec.rb` | Group 3 package consistency |
+| `referential_integrity.rb` | *(existing file)* | most of "referential integrity" (see below), style reference/inheritance/run-and-table integrity, hyperlink references, paraId/rId uniqueness, numbering body reference integrity (`referential_integrity.rb:176`) |
+| root orchestration | `reconciler_spec.rb` *(what remains)* | `clear_stored_namespace_plans`, profile-dependent reconciliation |
+
+Three corrections, because the describe titles actively mislead:
+
+- **"numbering body reference integrity" is not `body.rb`.** It is
+  `ReferentialIntegrity#reconcile_numbering_body_references`.
+- **"numbering reconciliation" is not cross-cutting.** It is
+  `Parts#reconcile_numbering`.
+- **`fix.rb` owns nothing here.** It only defines the `Fix` value
+  object, so there is no `fix_spec.rb` in this split.
+
+**No `theme_spec.rb`.** There is no top-level theme describe to absorb;
+the only theme behavior sits inside the profile describe.
+
+**"profile-dependent reconciliation" stays at root.** It spans `Parts`,
+`Body`, `Theme` and the top-level orchestration, so no single module owns
+it. Splitting it by owner would shred one coherent end-to-end test into
+fragments that individually prove nothing. Keep it as the orchestration
+smoke test, which is what the original note also wanted.
+
+**"referential integrity" does not move as one block.** Its first
+examples ("creates missing footnote definition for dangling reference"
+and the endnote equivalent) go through `Notes#reconcile_note_references`
+(`notes.rb:12`), not `ReferentialIntegrity`. Those belong in
+`notes_spec.rb`; the rest moves to `referential_integrity_spec.rb`. This
+is the one describe that must be split by example rather than moved
+intact, so read each example's call path before moving it.
+
+## How to sequence the work
+
+There is no useful "move the setup first" step. Concern-local setup cannot move
+before its describe moves, and relocating the top-level aliases into nested
+blocks inside the monolith would be review-only churn that changes no behaviour.
+Two earlier revisions of this note proposed one; drop it.
+
+Instead, each step moves one concern **and** carries whatever setup that concern
+needs.
+
+Measured, so the sizing is real: the top level has **8 `let`s** (class aliases)
+and **one** helper, `build_package` (line 16). `build_package_with_headers` is
+**nested inside** the headers/footers describe at line 2016, not shared —
+rubocop's "13 memoized helpers" warning refers to that nested group, not the top
+level.
+
+Prefer duplicating a one-line class alias in each destination file over creating
+a shared fixture file. Small duplication beats hidden shared state, which is
+what the original note preferred too.
+
+The one describe needing care is "referential integrity" — see above; it splits
+by example rather than moving intact, so read each example's call path first.
+
+## Verification
+
+A spec split has a specific failure mode: the tests still pass but no
+longer test the same thing. Guard against it explicitly.
+
+- **Example count must be conserved.** Record
+  `bundle exec rspec spec/uniword/docx/ --dry-run` before and after; the
+  totals must match exactly. A drop means a describe was orphaned.
+- **Each moved file must still be able to fail.** For each new file,
+  break the relevant reconciler module once and confirm it goes red.
+- Each file runs independently:
+  `bundle exec rspec spec/uniword/docx/reconciler/notes_spec.rb`
+- `bundle exec rspec spec/uniword/docx/` green.
+- `bundle exec rubocop spec/uniword/docx/`
+
+## Out of scope
+
+- Rewriting any assertion. This is a move, not a rewrite. If a spec looks
+  wrong, note it — fixing it here makes the conservation check
+  meaningless.
+- The pre-existing rubocop offences in the moved code (long lines,
+  memoized helper counts, `build_package_with_headers` ABC size). They
+  travel as-is; cleaning them up would hide the move in unrelated churn.
+- Splitting any other monolithic spec file.
+
+## Sequencing
+
+None. An earlier revision claimed this should follow TODO/02 to avoid rebase
+pain. There is no overlap: TODO/02 touches accessibility, MHTML and math specs,
+none of which is `reconciler_spec.rb`. Either order works.

spec/uniword/docx/reconciler/body_spec.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+require "uniword/docx"
+
+RSpec.describe Uniword::Docx::Reconciler do
+  describe "existing value preservation (||= pattern)" do
+    let(:profile) { Uniword::Docx::Profile.load(:word_2024_en) }
+
+    it "preserves pre-existing rsidR on paragraphs" do
+      package = Uniword::Docx::Package.new
+      package.document = Uniword::Wordprocessingml::DocumentRoot.new
+      para = Uniword::Wordprocessingml::Paragraph.new(
+        rsid_r: "AABBCCDD",
+        rsid_r_default: "11223344",
+        para_id: "DEADBEEF",
+        text_id: "CAFEBABE",
+      )
+      run = Uniword::Wordprocessingml::Run.new
+      run.text = Uniword::Wordprocessingml::Text.new(value: "Hello")
+      para.runs << run
+      package.document.body.paragraphs << para
+
+      described_class.new(package, profile: profile).reconcile
+
+      expect(para.rsid_r).to eq("AABBCCDD")
+      expect(para.rsid_r_default).to eq("11223344")
+      expect(para.para_id).to eq("DEADBEEF")
+      expect(para.text_id).to eq("CAFEBABE")
+    end
+
+    it "preserves pre-existing section properties rsidR" do
+      package = Uniword::Docx::Package.new
+      package.document = Uniword::Wordprocessingml::DocumentRoot.new
+      para = Uniword::Wordprocessingml::Paragraph.new
+      run = Uniword::Wordprocessingml::Run.new
+      run.text = Uniword::Wordprocessingml::Text.new(value: "Hello")
+      para.runs << run
+      package.document.body.paragraphs << para
+      package.document.body.section_properties =
+        Uniword::Wordprocessingml::SectionProperties.new(
+          rsid_r: "FFEEDDCC",
+          page_size: Uniword::Wordprocessingml::PageSize.new(
+            width: 12_240, height: 15_840
+          ),
+        )
+
+      described_class.new(package, profile: profile).reconcile
+
+      expect(package.document.body.section_properties.rsid_r).to eq("FFEEDDCC")
+    end
+  end
+
+  describe "headers/footers reconciliation" do
+    let(:header_class) { Uniword::Wordprocessingml::Header }
+    let(:run_class) { Uniword::Wordprocessingml::Run }
+    let(:run_props_class) { Uniword::Wordprocessingml::RunProperties }
+    let(:text_class) { Uniword::Wordprocessingml::Text }
+
+    def build_package_with_headers
+      package = Uniword::Docx::Package.new
+      header = header_class.new
+      para = Uniword::Wordprocessingml::Paragraph.new
+      props = Uniword::Wordprocessingml::ParagraphProperties.new
+      props.alignment = Uniword::Properties::Alignment.new(val: "right")
+      para.properties = props
+
+      run = run_class.new
+      run.properties = run_props_class.new
+      para.runs << run
+
+      header.paragraphs << para
+      package.document = Uniword::Wordprocessingml::DocumentRoot.new
+      package.document.headers = { "default" => header }
+      package
+    end
+
+    it "strips empty runs from header paragraphs" do
+      package = build_package_with_headers
+      described_class.new(package).reconcile
+
+      header = package.document.headers["default"]
+      expect(header.paragraphs.first.runs).to be_empty
+    end
+
+    it "preserves runs that have text content" do
+      package = build_package_with_headers
+      run = run_class.new
+      run.text = text_class.new(content: "Header text")
+      package.document.headers["default"].paragraphs.first.runs << run
+
+      described_class.new(package).reconcile
+
+      header = package.document.headers["default"]
+      expect(header.paragraphs.first.runs.size).to eq(1)
+    end
+  end
+end