diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c67d875..aed54e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,7 +31,7 @@ jobs: - name: Set up .NET uses: actions/setup-dotnet@v4 with: - dotnet-version: '8.0.x' + dotnet-version: '10.0.x' - name: Determine runtime identifier id: rid @@ -83,7 +83,7 @@ jobs: python-version: '3.11' - uses: actions/setup-dotnet@v4 with: - dotnet-version: '8.0.x' + dotnet-version: '10.0.x' - run: pip install build hatchling twine - name: Build per-platform engine wheels shell: bash diff --git a/.github/workflows/python-publish.yml b/.github/workflows/python-publish.yml index ff781d7..8c9480e 100644 --- a/.github/workflows/python-publish.yml +++ b/.github/workflows/python-publish.yml @@ -46,7 +46,7 @@ jobs: python-version: '3.11' - uses: actions/setup-dotnet@v4 with: - dotnet-version: '8.0.x' + dotnet-version: '10.0.x' - run: pip install build hatchling - name: Build per-platform engine wheels shell: bash diff --git a/CLAUDE.md b/CLAUDE.md index ac58114..c5927b2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,11 +4,12 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Overview -Python-Redlines generates `.docx` redline/tracked-changes documents by comparing two Word files. A pure-Python wrapper drives compiled C# (.NET 8) engine binaries; the Python layer handles platform detection, binary extraction, temp file management, and subprocess execution. +Python-Redlines generates `.docx` redline/tracked-changes documents by comparing two Word files. A pure-Python wrapper drives compiled C# (.NET 10) engine binaries; the Python layer handles platform detection, binary extraction, temp file management, and subprocess execution. Two comparison engines are available: - **XmlPowerToolsEngine** — wraps Open-XML-PowerTools WmlComparer (original engine) -- **DocxodusEngine** — wraps Docxodus, a modernized .NET 8.0 fork with better move detection +- **DocxodusEngine** — wraps Docxodus, a modernized .NET 10.0 fork with better move detection. + Takes `engine="wmlcomparer"` (default) or `engine="docxdiff"` to pick the comparison algorithm. ## Monorepo structure — three published packages @@ -34,7 +35,7 @@ shared pytest/coverage config. # Initialize the Docxodus submodule (required before building its engine) git submodule update --init --recursive -# Build engine binaries for one or more platforms (requires .NET 8.0 SDK). +# Build engine binaries for one or more platforms (requires .NET 10.0 SDK). # RIDs: linux-x64 linux-arm64 win-x64 win-arm64 osx-x64 osx-arm64 python build_differ.py linux-x64 python build_differ.py --all @@ -63,9 +64,10 @@ python -m build --wheel packages/docxodus # needs an archive in _binaries/ package is missing, with the `pip install` command to fix it. Both engines expose `run_redline(author_tag, original, modified, **kwargs)`. - `DocxodusEngine` overrides `_build_command()` to translate kwargs (e.g. `detect_moves`, - `detail_threshold`) into CLI flags. `XmlPowerToolsEngine` uses the legacy - 4-positional-arg format and ignores kwargs. + `DocxodusEngine` overrides `_build_command()` to translate kwargs (e.g. `engine`, + `detect_moves`, `detail_threshold`) into CLI flags, and raises `ValueError` when a + WmlComparer-only kwarg is combined with `engine="docxdiff"`. `XmlPowerToolsEngine` uses + the legacy 4-positional-arg format and ignores kwargs. 2. **Binary packages** ship one platform archive under `src//_binaries/.tar.gz` (or `.zip` for Windows). The archive is @@ -79,6 +81,10 @@ python -m build --wheel packages/docxodus # needs an archive in _binaries/ `build_differ.py` compiles an engine for a given RID with `dotnet publish` and writes a single flat archive into the corresponding binary package's `_binaries/`. + The two engines target different frameworks — `csproj` is `net8.0`, `docxodus/tools/redline` + is `net10.0` — so `build_differ.py`'s `ENGINES` list carries a per-engine `tfm`. The .NET 10 + SDK builds both; `global.json` pins it. + ## Build & release flow - A binary-package wheel must contain **exactly one** platform archive. Each @@ -104,4 +110,5 @@ integration test validates exactly 9 revisions on the fixture documents. ## Stdout Format Differences - **XmlPowerToolsEngine**: `"Revisions found: 9"` -- **DocxodusEngine**: `"Redline complete: 9 revision(s) found"` +- **DocxodusEngine**, default / `engine="wmlcomparer"`: `"Redline complete: 9 revision(s) found"` +- **DocxodusEngine**, `engine="docxdiff"`: `"Redline complete: 11 revision(s) found"` diff --git a/README.md b/README.md index eec3ff7..c3897a0 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ on it freely: two documents in, one redline out. ## Quick Start The **default engine is [Docxodus](https://github.com/JSv4/Docxodus)** — a modernized, -actively-maintained .NET 8 comparison engine (detailed below). Install it and you're +actively-maintained .NET 10 comparison engine (detailed below). Install it and you're running; the engine binary is prebuilt and embedded in the wheel, so there is **no .NET SDK to install and nothing to compile**: @@ -45,7 +45,7 @@ recommended choice; `XmlPowerToolsEngine` remains available as a legacy option. ### `DocxodusEngine` — Default (Recommended) -**[Docxodus](https://github.com/JSv4/Docxodus)** is a modernized .NET 8.0 fork of Open-XML-PowerTools with +**[Docxodus](https://github.com/JSv4/Docxodus)** is a modernized .NET 10.0 fork of Open-XML-PowerTools with significant improvements: - **Move detection** — identifies content that was moved rather than deleted and re-inserted @@ -125,6 +125,7 @@ redline_bytes, stdout, stderr = engine.run_redline( | Setting | Type | Default | Description | |---|---|---|---| +| `engine` | str | `"wmlcomparer"` | Comparison algorithm: `"wmlcomparer"` or `"docxdiff"` | | `detail_threshold` | float | 0.0 | Comparison granularity (0.0–1.0, lower = more detailed) | | `case_insensitive` | bool | False | Ignore case differences | | `detect_moves` | bool | False | Enable move detection | @@ -135,13 +136,35 @@ redline_bytes, stdout, stderr = engine.run_redline( | `conflate_spaces` | bool | True | Treat breaking/non-breaking spaces the same | | `date_time` | str | now | Custom ISO 8601 timestamp for revisions | -> **Warning:** Move detection can cause Word to display "unreadable content" warnings due to a known +> **Warning:** (WmlComparer only) Move detection can cause Word to display "unreadable content" warnings due to a known > ID collision bug. When using `detect_moves=True`, always set `simplify_move_markup=True` as well. > This converts move markup to regular del/ins (loses green move styling but ensures Word compatibility). > **Note:** These settings are only available on `DocxodusEngine`. `XmlPowerToolsEngine` ignores > extra keyword arguments. +### Choosing an engine + +`DocxodusEngine` wraps two comparison algorithms in one binary. `wmlcomparer` is the default +and is the lineage inherited from Open-XML-PowerTools. `docxdiff` is Docxodus's newer +structure-aware IR engine, which produces finer-grained markup — on the same pair of documents +it reports 11 revisions where `wmlcomparer` reports 9. + +```python +engine.run_redline("Reviewer", original, modified, engine="docxdiff") +``` + +`docxdiff` does not implement `detail_threshold`, `simplify_move_markup`, or +`detect_format_changes`. Passing any of them alongside `engine="docxdiff"` raises `ValueError` +rather than silently ignoring them. It does honour `detect_moves`, `case_insensitive`, +`conflate_spaces`, `move_similarity_threshold`, `move_minimum_word_count`, and `date_time`. + +Move markup differs between the two. `docxdiff` renders moves natively and rejects +`simplify_move_markup`, so the Word-compatibility mitigation described in the warning above is +unavailable there; whether Word's ID-collision warning affects DocxDiff's native move markup is +untested. If you need moves lowered to plain del/ins for maximum Word compatibility, use +`engine="wmlcomparer"` with `simplify_move_markup=True`. + ## Architecture Overview Both engines follow the same pattern: a Python wrapper class invokes a self-contained C# binary via subprocess. @@ -184,19 +207,25 @@ extracts the platform archive once into the user cache directory, and runs it. ### Stdout Differences -The two engines produce slightly different stdout messages: +The engines produce slightly different stdout messages: | Engine | Example stdout | |---|---| | `XmlPowerToolsEngine` | `Revisions found: 9` | -| `DocxodusEngine` | `Redline complete: 9 revision(s) found` | +| `DocxodusEngine` (default / `engine="wmlcomparer"`) | `Redline complete: 9 revision(s) found` | +| `DocxodusEngine` (`engine="docxdiff"`) | `Redline complete: 11 revision(s) found` | + +The revision counts differ between the two Docxodus engines because the algorithms differ, +not because either is wrong. ## Development ### Prerequisites - Python 3.9+ -- .NET 8.0 SDK (only for building the engine binaries locally) +- .NET 10.0 SDK (only for building the engine binaries locally). Install with + `apt install dotnet-sdk-10.0`, or `curl -sSL https://dot.net/v1/dotnet-install.sh | bash -s -- --channel 10.0`. + A .NET 8 SDK can no longer build the Docxodus engine, which targets `net10.0`. ### Setup diff --git a/build_differ.py b/build_differ.py index 17c5315..89bacd3 100644 --- a/build_differ.py +++ b/build_differ.py @@ -25,6 +25,7 @@ "name": "ooxmlpowertools", "csproj": os.path.join("csproj"), "csproj_file": os.path.join("csproj", "redlines.csproj"), + "tfm": "net8.0", "binaries_dir": os.path.join( "packages", "ooxmlpowertools", "src", "python_redlines_ooxmlpowertools", "_binaries", @@ -34,6 +35,7 @@ "name": "docxodus", "csproj": os.path.join("docxodus", "tools", "redline"), "csproj_file": os.path.join("docxodus", "tools", "redline", "redline.csproj"), + "tfm": "net10.0", "binaries_dir": os.path.join( "packages", "docxodus", "src", "python_redlines_docxodus", "_binaries", @@ -88,7 +90,7 @@ def build_engine_for_rid(engine, rid): print(f"[{engine['name']}] Building {rid} ...") run_command(f"dotnet publish {csproj} -c Release -r {rid} --self-contained") - publish_dir = os.path.join(csproj, "bin", "Release", "net8.0", rid, "publish") + publish_dir = os.path.join(csproj, "bin", "Release", engine["tfm"], rid, "publish") if not os.path.isdir(publish_dir): raise RuntimeError(f"Expected publish output not found: {publish_dir}") diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 175beeb..9688807 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -3,7 +3,9 @@ ## Prerequisites - Python 3.9 or higher -- .NET 8.0 SDK (to compile the C# engine binaries) +- .NET 10.0 SDK (to compile the C# engine binaries). The Docxodus engine targets + `net10.0`; the Open-XML-PowerTools engine still targets `net8.0` and the .NET 10 SDK + builds both. - `git` (the Docxodus engine is a submodule) ## Repository layout diff --git a/docxodus b/docxodus index d0601cd..47a543a 160000 --- a/docxodus +++ b/docxodus @@ -1 +1 @@ -Subproject commit d0601cd1b96eb13fc807d3b9bd907b690e112268 +Subproject commit 47a543a0a60a0c7be0b01dd3d1b6eec88af24d8a diff --git a/global.json b/global.json index 3fea262..ade3450 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "8.0.0", + "version": "10.0.0", "rollForward": "latestFeature" } } diff --git a/packages/core/README.md b/packages/core/README.md index f75be0b..d2f1771 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -28,6 +28,9 @@ redline_bytes, stdout, stderr = engine.run_redline( ) ``` +`DocxodusEngine` accepts `engine="wmlcomparer"` (default) or `engine="docxdiff"` to select the +comparison algorithm. See the [project README](https://github.com/JSv4/Python-Redlines#choosing-an-engine). + If an engine's companion package is not installed, instantiating the engine raises `EngineNotInstalledError` with the `pip install` command to fix it. diff --git a/packages/core/src/python_redlines/__about__.py b/packages/core/src/python_redlines/__about__.py index a478188..d769a50 100644 --- a/packages/core/src/python_redlines/__about__.py +++ b/packages/core/src/python_redlines/__about__.py @@ -1,4 +1,4 @@ # SPDX-FileCopyrightText: 2024-present U.N. Owen # # SPDX-License-Identifier: MIT -__version__ = "0.2.1" +__version__ = "0.3.0" diff --git a/packages/core/src/python_redlines/engines.py b/packages/core/src/python_redlines/engines.py index 63e07af..898ca42 100644 --- a/packages/core/src/python_redlines/engines.py +++ b/packages/core/src/python_redlines/engines.py @@ -139,9 +139,14 @@ def run_redline(self, author_tag: str, original: Union[str, bytes, Path], modifi (as ``str`` or ``pathlib.Path``). Returns the redline output as bytes. Additional keyword arguments are passed to _build_command() for engine-specific options. - DocxodusEngine supports: detail_threshold, case_insensitive, detect_moves, + DocxodusEngine supports: engine, detail_threshold, case_insensitive, detect_moves, simplify_move_markup, move_similarity_threshold, move_minimum_word_count, detect_format_changes, conflate_spaces, date_time. + + DocxodusEngine's engine kwarg selects the comparison algorithm: 'wmlcomparer' + (the default) or 'docxdiff'. The docxdiff engine ignores detail_threshold, + simplify_move_markup, and detect_format_changes, so passing them alongside + engine='docxdiff' raises ValueError rather than silently changing nothing. """ temp_files = [] try: @@ -194,6 +199,13 @@ class DocxodusEngine(BaseEngine): BINARY_BASE_NAME = 'redline' EXTRA_NAME = 'docxodus' + # Comparison engines accepted by the redline CLI's --engine flag. + ENGINES = ('wmlcomparer', 'docxdiff') + + # DocxCompare.ToDocxDiffSettings drops these on the docxdiff branch, and the CLI + # accepts them there without complaint, so reject them before we shell out. + _WMLCOMPARER_ONLY = ('detail_threshold', 'simplify_move_markup', 'detect_format_changes') + # Boolean flags (default False — presence enables) _BOOL_FLAGS = [ ('case_insensitive', '--case-insensitive'), @@ -215,8 +227,33 @@ class DocxodusEngine(BaseEngine): ('date_time', '--date-time'), ] - @staticmethod - def _validate_kwargs(kwargs): + @classmethod + def _normalize_engine(cls, kwargs): + """The chosen engine, lowercased and stripped, or None if the caller didn't pick one.""" + if 'engine' not in kwargs: + return None + + engine = kwargs['engine'] + if not isinstance(engine, str): + raise ValueError(f"engine must be a string, got {engine!r}") + + normalized = engine.strip().lower() + if normalized not in cls.ENGINES: + raise ValueError( + f"engine must be one of {', '.join(cls.ENGINES)}, got {engine!r}" + ) + return normalized + + @classmethod + def _validate_kwargs(cls, kwargs): + if cls._normalize_engine(kwargs) == 'docxdiff': + for name in cls._WMLCOMPARER_ONLY: + if name in kwargs: + raise ValueError( + f"{name} is not supported by the 'docxdiff' engine " + f"(WmlComparer-only). Remove it or use engine='wmlcomparer'." + ) + if 'detail_threshold' in kwargs: val = kwargs['detail_threshold'] if not isinstance(val, (int, float)) or val < 0.0 or val > 1.0: @@ -234,10 +271,14 @@ def _validate_kwargs(kwargs): def _build_command(self, author_tag, original_path, modified_path, target_path, **kwargs): self._validate_kwargs(kwargs) + engine = self._normalize_engine(kwargs) cmd = [self.extracted_binaries_path, original_path, modified_path, target_path, f'--author={author_tag}'] + if engine is not None: + cmd.append(f'--engine={engine}') + for kwarg, flag in self._BOOL_FLAGS: if kwargs.get(kwarg): cmd.append(flag) diff --git a/tests/test_docxodus_engine.py b/tests/test_docxodus_engine.py index 5036772..fa1c4a9 100644 --- a/tests/test_docxodus_engine.py +++ b/tests/test_docxodus_engine.py @@ -1,3 +1,5 @@ +import re + import pytest from python_redlines.engines import DocxodusEngine @@ -18,6 +20,13 @@ def modified_docx(): return load_docx_bytes('tests/fixtures/modified.docx') +def revision_count(stdout): + """The integer revision count from a Docxodus 'Redline complete: N revision(s) found' line.""" + match = re.search(r"Redline complete: (\d+) revision\(s\) found", stdout) + assert match, f"no revision count in stdout: {stdout!r}" + return int(match.group(1)) + + def test_run_docxodus_with_real_files(original_docx, modified_docx): wrapper = DocxodusEngine() @@ -188,3 +197,149 @@ def test_build_command_negatable_true_not_added(): ) assert "--no-detect-format-changes" not in cmd assert "--no-conflate-spaces" not in cmd + + +# --- Engine selection (Docxodus v7.0.0 --engine flag) --- + +def test_build_command_engine_omitted_by_default(): + """No engine= kwarg means no --engine flag: the argv stays as it was pre-v7.""" + engine = DocxodusEngine() + cmd = engine._build_command("Author", "/tmp/o.docx", "/tmp/m.docx", "/tmp/out.docx") + assert not any(arg.startswith("--engine") for arg in cmd) + + +def test_build_command_engine_docxdiff(): + engine = DocxodusEngine() + cmd = engine._build_command( + "Author", "/tmp/o.docx", "/tmp/m.docx", "/tmp/out.docx", engine="docxdiff", + ) + assert "--engine=docxdiff" in cmd + + +def test_build_command_engine_explicit_wmlcomparer(): + engine = DocxodusEngine() + cmd = engine._build_command( + "Author", "/tmp/o.docx", "/tmp/m.docx", "/tmp/out.docx", engine="wmlcomparer", + ) + assert "--engine=wmlcomparer" in cmd + + +def test_build_command_engine_is_normalized(): + engine = DocxodusEngine() + cmd = engine._build_command( + "Author", "/tmp/o.docx", "/tmp/m.docx", "/tmp/out.docx", engine=" DocxDiff ", + ) + assert "--engine=docxdiff" in cmd + + +def test_build_command_unknown_engine(): + engine = DocxodusEngine() + with pytest.raises(ValueError, match="engine must be one of"): + engine._build_command("Author", "orig", "mod", "out", engine="bogus") + + +def test_build_command_non_string_engine(): + engine = DocxodusEngine() + with pytest.raises(ValueError, match="engine must be a string"): + engine._build_command("Author", "orig", "mod", "out", engine=1) + + +@pytest.mark.parametrize("kwarg, value", [ + ("detail_threshold", 0.5), + ("detail_threshold", 0.0), + ("simplify_move_markup", True), + ("simplify_move_markup", False), + ("detect_format_changes", True), + ("detect_format_changes", False), +]) +def test_docxdiff_rejects_wmlcomparer_only_kwargs(kwarg, value): + """docxdiff silently ignores these in C#; reject on key presence, whatever the value.""" + engine = DocxodusEngine() + expected = f"{kwarg} is not supported by the 'docxdiff' engine" + with pytest.raises(ValueError, match=expected): + engine._build_command("Author", "orig", "mod", "out", engine="docxdiff", **{kwarg: value}) + + +def test_wmlcomparer_still_allows_its_own_kwargs(): + engine = DocxodusEngine() + cmd = engine._build_command( + "Author", "orig", "mod", "out", + engine="wmlcomparer", detail_threshold=0.5, simplify_move_markup=True, + ) + assert "--engine=wmlcomparer" in cmd + assert "--detail-threshold=0.5" in cmd + assert "--simplify-move-markup" in cmd + + +def test_docxdiff_allows_the_kwargs_it_honours(): + engine = DocxodusEngine() + cmd = engine._build_command( + "Author", "orig", "mod", "out", + engine="docxdiff", detect_moves=True, case_insensitive=True, + conflate_spaces=False, move_similarity_threshold=0.7, move_minimum_word_count=2, + ) + assert "--engine=docxdiff" in cmd + assert "--detect-moves" in cmd + assert "--case-insensitive" in cmd + assert "--no-conflate-spaces" in cmd + assert "--move-similarity-threshold=0.7" in cmd + assert "--move-minimum-word-count=2" in cmd + + +def test_docxdiff_engine_check_precedes_range_check(): + """engine='docxdiff' + an out-of-range detail_threshold reports the engine problem.""" + engine = DocxodusEngine() + with pytest.raises(ValueError, match="not supported by the 'docxdiff' engine"): + engine._build_command("Author", "orig", "mod", "out", engine="docxdiff", detail_threshold=1.5) + + +# --- Engine selection, end to end --- + +def test_docxodus_default_engine_is_wmlcomparer(original_docx, modified_docx): + """The default path is the regression anchor: 9 revisions, exactly as before v7.""" + engine = DocxodusEngine() + redline_output, stdout, stderr = engine.run_redline( + "TestAuthor", original_docx, modified_docx, + ) + assert stderr is None + assert "Redline complete: 9 revision(s) found" in stdout + assert redline_output[:2] == b"PK" + + +def test_docxodus_docxdiff_engine(original_docx, modified_docx): + """docxdiff is a different algorithm and finds a different number of revisions.""" + engine = DocxodusEngine() + redline_output, stdout, stderr = engine.run_redline( + "TestAuthor", original_docx, modified_docx, engine="docxdiff", + ) + assert stderr is None + assert "Redline complete: 11 revision(s) found" in stdout + assert redline_output[:2] == b"PK" + + +def test_docxodus_explicit_wmlcomparer_matches_default(original_docx, modified_docx): + engine = DocxodusEngine() + _, default_stdout, _ = engine.run_redline("TestAuthor", original_docx, modified_docx) + _, explicit_stdout, _ = engine.run_redline( + "TestAuthor", original_docx, modified_docx, engine="wmlcomparer", + ) + default_count = revision_count(default_stdout) + explicit_count = revision_count(explicit_stdout) + assert explicit_count == default_count + assert default_count == 9 + + +def test_docxdiff_output_is_a_valid_docx_with_tracked_changes(original_docx, modified_docx): + import io + import zipfile + + engine = DocxodusEngine() + redline_output, _, _ = engine.run_redline( + "TestAuthor", original_docx, modified_docx, engine="docxdiff", + ) + with zipfile.ZipFile(io.BytesIO(redline_output)) as archive: + assert archive.testzip() is None + document_xml = archive.read("word/document.xml").decode("utf-8") + + assert "