From af8fa5ae47b168386f5582ceae1d5bd181cb64c2 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 5 Sep 2026 23:11:29 -0500 Subject: [PATCH 1/3] feat!: track Docxodus v12.1.0, dropping WmlComparer for DocxDiff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docxodus v11.0.0 deleted the WmlComparer engine in favour of DocxDiff, which has been its default since v8.0.0. That removal reaches straight into this project's public API, which has exposed the upstream selector as a keyword argument since 0.3.0. BREAKING CHANGE: engine, detail_threshold and simplify_move_markup are removed from DocxodusEngine.run_redline() and raise ValueError. They are rejected rather than dropped, because dropping them would not be a breaking change but a wrong-answer one. The v12 CLI rejects --engine as an unknown flag and merely warns-and-ignores the other two, so a caller passing engine="wmlcomparer" would have silently received DocxDiff output believing they had selected something else. Unknown keyword arguments are rejected on the same reasoning: a typo used to be discarded in silence, which is indistinguishable from a setting that did nothing. The GitHub Action's `comparison` input maps onto the deleted selector and now fails the run for the same reason. XmlPowerToolsEngine wraps the *original* Open-XML-PowerTools WmlComparer in a separately published wheel. It keeps working and keeps shipping; instantiating it now warns that it is on the way out. Also fixes two bugs in BaseEngine.run_redline's temp-file handling: - #30: the output scratch file was NamedTemporaryFile(delete=False).name, which drops the only reference to the file object. Its finalizer reports a ResourceWarning on CPython 3.13+ and the descriptor stays open until collection. Now mkstemp, which is the right tool when only a path is wanted. - Path inputs were registered for deletion alongside the engine's own scratch files, so passing a path — a documented input mode that the quickstart demonstrates — deleted the caller's source documents. Only files this library creates are cleaned up now. Revision counts on the fixtures are re-pinned to 10, measured against a freshly built v12.1.0 binary. It is neither the 9 the old default reported nor the 11 the old opt-in reported: v11 and v12 changed region arrangement, surplus table cells and section defaults. Fixes #30 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014ed9PC7DDco5AFPjsZyy9F --- action.yml | 6 +- action/redline_changed.py | 18 +- .../2026-09-05-docxodus-v12-upgrade-design.md | 149 ++++++++++ docxodus | 2 +- .../core/src/python_redlines/__about__.py | 2 +- packages/core/src/python_redlines/engines.py | 134 +++++---- tests/test_action_script.py | 32 +- tests/test_docxodus_engine.py | 279 +++++++----------- tests/test_engine_contract.py | 16 +- tests/test_openxml_differ.py | 29 +- tests/test_temp_file_handling.py | 117 ++++++++ 11 files changed, 540 insertions(+), 244 deletions(-) create mode 100644 docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md create mode 100644 tests/test_temp_file_handling.py diff --git a/action.yml b/action.yml index 3a69de0..23f9e7f 100644 --- a/action.yml +++ b/action.yml @@ -47,8 +47,10 @@ inputs: default: 'docxodus' comparison: description: >- - Comparison algorithm for the docxodus engine: 'wmlcomparer' (default) or - 'docxdiff'. + REMOVED. Docxodus v11.0.0 deleted WmlComparer, so there is no longer an + algorithm to select; DocxDiff is the only one. Setting this fails the run + rather than silently comparing with something other than what was asked + for. Remove the input. required: false default: '' detect-moves: diff --git a/action/redline_changed.py b/action/redline_changed.py index 4c71b1a..33c7b80 100644 --- a/action/redline_changed.py +++ b/action/redline_changed.py @@ -31,7 +31,16 @@ # kwargs only DocxodusEngine understands; XmlPowerToolsEngine would silently # ignore them, so requesting one with engine=xmlpowertools is a config error. -DOCXODUS_ONLY_INPUTS = ('comparison', 'detect-moves') +DOCXODUS_ONLY_INPUTS = ('detect-moves',) + +# Docxodus v11.0.0 deleted WmlComparer, and with it the engine selector this +# input mapped onto. Rejected outright rather than ignored: a workflow that +# pinned 'wmlcomparer' would otherwise keep running and quietly produce +# DocxDiff output. +COMPARISON_REMOVED = ( + "Input 'comparison' is no longer supported: the comparison-engine selector was " + "removed in Docxodus v11.0.0, which deleted WmlComparer. DocxDiff is now the only " + "algorithm — remove the input.") class ConfigError(Exception): @@ -116,18 +125,15 @@ def validate(self) -> None: raise ConfigError( "Inputs 'original' and 'modified' must be provided together " "(explicit-pair mode) or both left empty (auto-detect mode).") + if self.comparison: + raise ConfigError(COMPARISON_REMOVED) if self.engine != 'docxodus': - if self.comparison: - raise ConfigError( - "Input 'comparison' is only supported by the docxodus engine.") if self.detect_moves: raise ConfigError( "Input 'detect-moves' is only supported by the docxodus engine.") def engine_kwargs(self) -> Dict: kwargs: Dict = {} - if self.comparison: - kwargs['engine'] = self.comparison if self.detect_moves: kwargs['detect_moves'] = True return kwargs diff --git a/docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md b/docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md new file mode 100644 index 0000000..d48636c --- /dev/null +++ b/docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md @@ -0,0 +1,149 @@ +# Docxodus v12.1.0 upgrade: DocxDiff becomes the only algorithm + +**Date:** 2026-09-05 +**Status:** Approved, in implementation +**Target release:** python-redlines 1.0.0 + +## Problem + +The `docxodus` submodule is pinned to v7.0.0. Upstream is at v12.1.0, and +**v11.0.0 deleted the `WmlComparer` engine outright** in favour of `DocxDiff`, +which has been the upstream default since v8.0.0 and is now mature. + +That removal reaches directly into this repository's public API. Since 0.3.0 we +have exposed the upstream selector as a keyword argument: + +```python +engine.run_redline("Reviewer", original, modified, engine="docxdiff") +``` + +`--engine=` no longer exists on the `redline` CLI. Two more flags — +`--detail-threshold=` and `--simplify-move-markup` — survive only as +warn-and-ignore stubs that upstream describes as kept "one release longer". + +## Goals + +1. Track upstream to v12.1.0. +2. Remove the dead comparison-settings surface **loudly**, never silently. +3. Fix issue #30 and the data-loss bug found beside it. +4. Ship as 1.0.0 with release notes that lead with the WmlComparer removal. + +## Non-goals + +- Retiring `XmlPowerToolsEngine` or the `python-redlines-ooxmlpowertools` + package. It wraps the *original* Open-XML-PowerTools WmlComparer, is a + separately published wheel that users have pinned, and retiring it is its own + decision with its own migration story. It gets a `DeprecationWarning` and + keeps shipping. +- Any change to the `wmlcomparer`-era *output* of already-published versions. + 0.3.0 remains on PyPI for anyone who needs that algorithm. + +## Design + +### 1. Submodule + +`docxodus` v7.0.0 → v12.1.0. Verified unchanged and therefore requiring no +build changes: `TargetFramework` is still `net10.0` and `AssemblyName` is still +`redline`, which `build_differ.py` and `BINARY_BASE_NAME` hardcode respectively. + +### 2. `DocxodusEngine` — removed settings become hard errors + +`engine`, `detail_threshold` and `simplify_move_markup` raise `ValueError` +naming the argument, the version that removed it, and what to do instead. + +**This rejection is the load-bearing part of the change.** Deleting the code +path without it would not produce a breaking change; it would produce a +*wrong-answer* change. `_build_command` only reads keys it knows about, so +`run_redline(..., engine="wmlcomparer")` would become a silently-ignored kwarg +and hand the caller DocxDiff output while they believe they selected +WmlComparer. Rejection also covers `engine="docxdiff"`: it names a selector +that no longer exists, and leaving a vestigial argument that happens to match +the surviving engine invites code that breaks confusingly later. + +A second reason the rejection cannot be skipped: v12's CLI writes a *warning to +stderr* for `--detail-threshold=` and `--simplify-move-markup`. `run_redline` +surfaces any non-empty stderr to the caller, so passing them through would +break every `assert stderr is None` in the suite. + +**Unknown keyword arguments are also rejected.** Today `detial_threshold=0.5` +is silently discarded. Hard-erroring on removed arguments while still +swallowing typos would be half a fix, and the two failures are indistinguishable +to a user. + +Retained and unchanged: `case_insensitive`, `detect_moves`, +`move_similarity_threshold`, `move_minimum_word_count`, `detect_format_changes`, +`conflate_spaces`, `date_time`. + +Note for the release notes: upstream v11 remapped `--no-detect-format-changes` +onto `DocxDiffSettings.TrackBlockFormatChanges`, so its meaning shifted from +run-level to block-level format tracking. + +### 3. `BaseEngine` — two temp-file bugs + +**Issue #30 — leaked `NamedTemporaryFile`.** +`tempfile.NamedTemporaryFile(delete=False).name` constructs the object, takes +its name and drops the only reference. The finalizer then reports +`ResourceWarning: Implicitly cleaning up ...` on CPython 3.13+, once per call, +and the descriptor stays open until collection. Under `-W error` it surfaces as +an unraisable exception attributed to whatever test was running when the +collector fired. Replaced with `tempfile.mkstemp()` + `os.close(fd)`, which is +the right tool when only a path is wanted. + +**Data loss on path inputs (found while fixing #30).** +`run_redline` appends `original_path` and `modified_path` to `temp_files` +unconditionally, and the `finally` block `os.remove`s every entry. When the +caller passes a *path* — a documented, type-hinted input mode that +`docs/quickstart.md` demonstrates — those entries are the caller's own files, +and the call deletes both source documents. Only byte inputs are safe, which is +why the GitHub Action (bytes-only) has never hit it. + +Fix: register a file for cleanup only when this library created it. + +### 4. `XmlPowerToolsEngine` + +`DeprecationWarning` on instantiation, naming DocxodusEngine as the +replacement. The class and its wheel keep working. + +### 5. GitHub Action + +The `comparison` input maps onto the deleted kwarg. It becomes a `ConfigError` +with the same reasoning as the Python surface. `action.yml`, the +`DOCXODUS_ONLY_INPUTS` validation in `action/redline_changed.py`, and +`tests/test_action_script.py` all move together. + +### 6. Revision counts are measured, not predicted + +v11 and v12 changed region arrangement, surplus table cells, section defaults +and note pruning, so the fixture count was unlikely to be either of the numbers +previously pinned (9 for wmlcomparer, 11 for docxdiff). Measured against a +freshly built v12.1.0 binary on `tests/fixtures/`, it is **10** — neither. Every +assertion and doc table is pinned to that measurement. + +The extraction cache must be cleared first: `_extraction_root()` keys on the +installed *binary package* version, which does not change when `__about__.py` +is edited, so a stale `~/.cache/python-redlines` would silently keep serving +the v7 binary. + +## Testing + +- One rejection test per removed argument, plus unknown-kwarg rejection. +- `ResourceWarning`-as-error regression test for #30. +- A test that path inputs still exist after `run_redline` returns. +- Re-pinned integration counts, measured. +- `DeprecationWarning` on `XmlPowerToolsEngine()`. +- Action config tests for the rejected `comparison` input. + +## Release + +Version 0.3.0 → **1.0.0** in `packages/core/src/python_redlines/__about__.py` +(the single source of truth all three packages read). + +Release notes lead with the removal. Wording is deliberate: *"the Docxodus +engine no longer carries WmlComparer"* — **not** *"Python-Redlines is +WmlComparer-free"*, which would be false while the `ooxmlpowertools` wheel +still ships the original. + +The notes must also state that upstream v11 dropped `PreserveInputRevisions` +from `DocxCompare.Compare`'s front door, so output shifts on revision-bearing +inputs. This project's fixtures carry no input revisions and will not catch it; +users comparing already-redlined documents will. diff --git a/docxodus b/docxodus index 47a543a..5ab5ac8 160000 --- a/docxodus +++ b/docxodus @@ -1 +1 @@ -Subproject commit 47a543a0a60a0c7be0b01dd3d1b6eec88af24d8a +Subproject commit 5ab5ac87bbc81d5348ea2f2de87f6237d479d4b2 diff --git a/packages/core/src/python_redlines/__about__.py b/packages/core/src/python_redlines/__about__.py index d769a50..f58919b 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.3.0" +__version__ = "1.0.0" diff --git a/packages/core/src/python_redlines/engines.py b/packages/core/src/python_redlines/engines.py index 898ca42..3cf3cfe 100644 --- a/packages/core/src/python_redlines/engines.py +++ b/packages/core/src/python_redlines/engines.py @@ -6,6 +6,7 @@ import subprocess import tarfile import tempfile +import warnings import zipfile from pathlib import Path from typing import Optional, Tuple, Union @@ -138,23 +139,26 @@ def run_redline(self, author_tag: str, original: Union[str, bytes, Path], modifi Runs the redline binary. The 'original' and 'modified' arguments can be either bytes or file paths (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: engine, detail_threshold, case_insensitive, detect_moves, - simplify_move_markup, move_similarity_threshold, move_minimum_word_count, - detect_format_changes, conflate_spaces, date_time. + A path the caller supplies is never deleted; only scratch files this + method creates are cleaned up. - 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. + Additional keyword arguments are passed to _build_command() for engine-specific + options. DocxodusEngine supports: case_insensitive, detect_moves, + move_similarity_threshold, move_minimum_word_count, detect_format_changes, + conflate_spaces, date_time. It raises ValueError for unrecognised settings and + for the WmlComparer-era settings removed in Docxodus v11.0.0 (engine, + detail_threshold, simplify_move_markup). XmlPowerToolsEngine ignores kwargs. """ - temp_files = [] + scratch_files = [] try: + # mkstemp, not NamedTemporaryFile: we want a path, and NamedTemporaryFile + # hands back an open file object that nothing here would close (issue #30). + handle, target_path = tempfile.mkstemp(suffix='.docx') + os.close(handle) + scratch_files.append(target_path) - target_path = tempfile.NamedTemporaryFile(delete=False).name - original_path = self._write_to_temp_file(original) if isinstance(original, bytes) else original - modified_path = self._write_to_temp_file(modified) if isinstance(modified, bytes) else modified - temp_files.extend([target_path, original_path, modified_path]) + original_path = self._as_path(original, scratch_files) + modified_path = self._as_path(modified, scratch_files) command = self._build_command(author_tag, original_path, modified_path, target_path, **kwargs) @@ -169,14 +173,27 @@ def run_redline(self, author_tag: str, original: Union[str, bytes, Path], modifi return redline_output, stdout_output, stderr_output finally: - self._cleanup_temp_files(temp_files) + self._cleanup_temp_files(scratch_files) + + def _as_path(self, document, scratch_files): + """Return a filesystem path for *document*, writing bytes out if needed. + + Only a path this method creates is appended to *scratch_files*. A path + the caller passed in is the caller's own document, and registering it + for cleanup would delete the file they asked us to compare. + """ + if isinstance(document, bytes): + path = self._write_to_temp_file(document) + scratch_files.append(path) + return path + return os.fspath(document) def _cleanup_temp_files(self, temp_files): for file_path in temp_files: try: os.remove(file_path) except OSError as e: - print(f"Error deleting temp file {file_path}: {e}") + logger.warning("Error deleting temp file %s: %s", file_path, e) def _write_to_temp_file(self, data): """ @@ -193,24 +210,49 @@ class XmlPowerToolsEngine(BaseEngine): BINARY_BASE_NAME = 'redlines' EXTRA_NAME = 'ooxmlpowertools' + def __init__(self, target_path: Optional[str] = None): + warnings.warn( + "XmlPowerToolsEngine wraps the original, unmaintained Open-XML-PowerTools " + "WmlComparer and is deprecated; it will be removed in a future major " + "release. Use DocxodusEngine, which is actively maintained and runs the " + "DocxDiff algorithm.", + DeprecationWarning, + stacklevel=2, + ) + super().__init__(target_path) + class DocxodusEngine(BaseEngine): BINARY_PACKAGE = 'python_redlines_docxodus' 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') + # Settings that died with WmlComparer in Docxodus v11.0.0. They are rejected + # rather than dropped, because dropping them would not be a breaking change + # but a wrong-answer one: the v12 CLI rejects --engine as an unknown flag, + # and merely warns-and-ignores the other two, so a caller who passed + # engine='wmlcomparer' would silently receive DocxDiff output believing they + # had selected something else. + _REMOVED_KWARGS = { + 'engine': ( + "the comparison-engine selector was removed in Docxodus v11.0.0, which " + "deleted WmlComparer. DocxDiff is now the only algorithm — drop the argument" + ), + 'detail_threshold': ( + "it tuned WmlComparer's LCS granularity and went with it in Docxodus " + "v11.0.0. DocxDiff's granularity is structural, with no equivalent knob — " + "drop the argument" + ), + 'simplify_move_markup': ( + "it worked around WmlComparer's move markup and went with it in Docxodus " + "v11.0.0. DocxDiff renders moves natively — drop the argument" + ), + } # Boolean flags (default False — presence enables) _BOOL_FLAGS = [ ('case_insensitive', '--case-insensitive'), ('detect_moves', '--detect-moves'), - ('simplify_move_markup', '--simplify-move-markup'), ] # Negatable flags (default True — --no- prefix disables) @@ -221,43 +263,31 @@ class DocxodusEngine(BaseEngine): # Value flags _VALUE_FLAGS = [ - ('detail_threshold', '--detail-threshold'), ('move_similarity_threshold', '--move-similarity-threshold'), ('move_minimum_word_count', '--move-minimum-word-count'), ('date_time', '--date-time'), ] @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 + def _supported_kwargs(cls): + """Every comparison setting this engine still understands.""" + return {name for name, _ in (*cls._BOOL_FLAGS, *cls._NEG_FLAGS, *cls._VALUE_FLAGS)} @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: - raise ValueError(f"detail_threshold must be a float between 0.0 and 1.0, got {val!r}") + # Removed settings first, so they get their specific explanation rather + # than being lumped in with typos below. + for name, reason in cls._REMOVED_KWARGS.items(): + if name in kwargs: + raise ValueError(f"{name} is no longer supported: {reason}.") + + supported = cls._supported_kwargs() + unknown = sorted(set(kwargs) - supported) + if unknown: + raise ValueError( + f"Unknown comparison setting(s): {', '.join(unknown)}. " + f"Supported settings: {', '.join(sorted(supported))}." + ) if 'move_similarity_threshold' in kwargs: val = kwargs['move_similarity_threshold'] @@ -271,14 +301,10 @@ def _validate_kwargs(cls, 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_action_script.py b/tests/test_action_script.py index 4ac2ad9..79acb4a 100644 --- a/tests/test_action_script.py +++ b/tests/test_action_script.py @@ -59,18 +59,34 @@ def test_inputs_defaults(): def test_inputs_engine_kwargs(): - inputs = ra.Inputs.from_env({ - 'INPUT_COMPARISON': 'docxdiff', - 'INPUT_DETECT_MOVES': 'true', - }) - assert inputs.engine_kwargs() == {'engine': 'docxdiff', 'detect_moves': True} + inputs = ra.Inputs.from_env({'INPUT_DETECT_MOVES': 'true'}) + assert inputs.engine_kwargs() == {'detect_moves': True} + + +@pytest.mark.parametrize('value', ['wmlcomparer', 'docxdiff']) +def test_comparison_input_is_rejected(value): + """Docxodus v11.0.0 deleted the engine selector the input mapped onto. + + Accepting it would either crash in the CLI or, worse, quietly produce + DocxDiff output for a workflow that asked for WmlComparer. + """ + with pytest.raises(ra.ConfigError, match='comparison'): + ra.Inputs.from_env({'INPUT_COMPARISON': value}) + + +def test_comparison_rejection_explains_the_removal(): + with pytest.raises(ra.ConfigError) as excinfo: + ra.Inputs.from_env({'INPUT_COMPARISON': 'wmlcomparer'}) + + message = str(excinfo.value) + assert 'v11.0.0' in message + assert 'DocxDiff' in message @pytest.mark.parametrize('env', [ {'INPUT_ENGINE': 'wordperfect'}, {'INPUT_HTML_PREVIEW': 'maybe'}, {'INPUT_ORIGINAL': 'a.docx'}, # original without modified - {'INPUT_ENGINE': 'xmlpowertools', 'INPUT_COMPARISON': 'docxdiff'}, {'INPUT_ENGINE': 'xmlpowertools', 'INPUT_DETECT_MOVES': 'true'}, {'INPUT_DETECT_MOVES': 'yes'}, # not a bool ]) @@ -268,7 +284,7 @@ def test_main_explicit_pair(tmp_path, monkeypatch): assert 'count=1\n' in text payload = [line for line in text.splitlines() if line.startswith('redlines=')][0] record = json.loads(payload[len('redlines='):])[0] - assert record['revisions'] == 9 + assert record['revisions'] == 10 redline = Path(record['redline']) assert redline.is_file() and redline.stat().st_size > 0 # absolute source paths must not escape the requested output directory @@ -299,6 +315,6 @@ def test_main_auto_detect_over_git_history(repo, tmp_path, monkeypatch): record = json.loads(payload[len('redlines='):])[0] assert record['path'] == 'contracts/agreement.docx' assert record['status'] == 'modified' - assert record['revisions'] == 9 + assert record['revisions'] == 10 assert Path(record['redline']).is_file() assert 'contracts/agreement.docx' in summary_file.read_text() diff --git a/tests/test_docxodus_engine.py b/tests/test_docxodus_engine.py index fa1c4a9..abb961a 100644 --- a/tests/test_docxodus_engine.py +++ b/tests/test_docxodus_engine.py @@ -1,9 +1,16 @@ +import io import re +import zipfile import pytest from python_redlines.engines import DocxodusEngine +# Measured against the Docxodus v12.1.0 binary on tests/fixtures/. Docxodus +# v11.0.0 removed WmlComparer, so DocxDiff is the only algorithm; the count is +# neither the 9 the old default reported nor the 11 the old opt-in reported. +EXPECTED_REVISIONS = 10 + def load_docx_bytes(file_path): with open(file_path, 'rb') as file: @@ -41,25 +48,36 @@ def test_run_docxodus_with_real_files(original_docx, modified_docx): assert "revision(s) found" in stdout -# --- Integration tests for comparison settings --- - -def test_docxodus_with_detect_moves(original_docx, modified_docx): +def test_docxodus_revision_count_is_pinned(original_docx, modified_docx): + """The regression anchor for the DocxDiff-only engine.""" engine = DocxodusEngine() redline_output, stdout, stderr = engine.run_redline( "TestAuthor", original_docx, modified_docx, - detect_moves=True, simplify_move_markup=True, ) - assert redline_output is not None - assert len(redline_output) > 0 assert stderr is None - assert "revision(s) found" in stdout + assert revision_count(stdout) == EXPECTED_REVISIONS + assert redline_output[:2] == b"PK" -def test_docxodus_with_detail_threshold(original_docx, modified_docx): +def test_docxodus_output_is_a_valid_docx_with_tracked_changes(original_docx, modified_docx): + engine = DocxodusEngine() + redline_output, _, _ = engine.run_redline("TestAuthor", original_docx, modified_docx) + + 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 " 0 @@ -92,13 +110,12 @@ def test_docxodus_with_no_format_changes(original_docx, modified_docx): def test_docxodus_with_all_options(original_docx, modified_docx): + """Every surviving setting at once, and the CLI stays quiet on stderr.""" engine = DocxodusEngine() redline_output, stdout, stderr = engine.run_redline( "TestAuthor", original_docx, modified_docx, - detail_threshold=0.3, case_insensitive=True, detect_moves=True, - simplify_move_markup=True, move_similarity_threshold=0.7, move_minimum_word_count=2, detect_format_changes=False, @@ -111,13 +128,86 @@ def test_docxodus_with_all_options(original_docx, modified_docx): assert "revision(s) found" in stdout -# --- Validation tests --- +# --- Settings removed with WmlComparer (Docxodus v11.0.0) --- +# +# These must raise rather than be dropped. Two of them are still *accepted* by +# the v12 CLI, which warns on stderr and ignores them; `engine` is rejected as +# an unknown flag. Silently discarding any of them would hand the caller +# DocxDiff output while they believe they configured something else. + +@pytest.mark.parametrize("value", ["wmlcomparer", "docxdiff", "WmlComparer", " docxdiff "]) +def test_engine_kwarg_is_rejected(value): + engine = DocxodusEngine() + with pytest.raises(ValueError, match=r"engine .*no longer"): + engine._build_command("Author", "orig", "mod", "out", engine=value) + + +def test_detail_threshold_is_rejected(): + engine = DocxodusEngine() + with pytest.raises(ValueError, match=r"detail_threshold .*no longer"): + engine._build_command("Author", "orig", "mod", "out", detail_threshold=0.5) + + +def test_simplify_move_markup_is_rejected(): + engine = DocxodusEngine() + with pytest.raises(ValueError, match=r"simplify_move_markup .*no longer"): + engine._build_command("Author", "orig", "mod", "out", simplify_move_markup=True) + -def test_docxodus_invalid_detail_threshold(): +@pytest.mark.parametrize("kwarg", ["engine", "detail_threshold", "simplify_move_markup"]) +def test_removed_kwarg_error_names_the_replacement(kwarg): + """The message has to tell the reader what to do, not just that they are wrong.""" + values = {"engine": "docxdiff", "detail_threshold": 0.5, "simplify_move_markup": True} engine = DocxodusEngine() - with pytest.raises(ValueError, match="detail_threshold must be a float between 0.0 and 1.0"): - engine._build_command("Author", "orig", "mod", "out", detail_threshold=1.5) + with pytest.raises(ValueError) as excinfo: + engine._build_command("Author", "orig", "mod", "out", **{kwarg: values[kwarg]}) + message = str(excinfo.value) + assert "v11.0.0" in message + assert "DocxDiff" in message + + +def test_removed_kwarg_is_rejected_even_when_false(): + """The check is on the keyword being present, whatever its value.""" + engine = DocxodusEngine() + with pytest.raises(ValueError, match="no longer"): + engine._build_command("Author", "orig", "mod", "out", simplify_move_markup=False) + + +def test_removed_kwarg_rejection_reaches_run_redline(original_docx, modified_docx): + """The guard is on the public call, not only on the private builder.""" + engine = DocxodusEngine() + with pytest.raises(ValueError, match="no longer"): + engine.run_redline("TestAuthor", original_docx, modified_docx, engine="docxdiff") + + +# --- Unknown settings --- + +def test_unknown_kwarg_is_rejected(): + """A typo used to be discarded in silence, which reads as 'the setting did nothing'.""" + engine = DocxodusEngine() + with pytest.raises(ValueError, match="detial_threshold"): + engine._build_command("Author", "orig", "mod", "out", detial_threshold=0.5) + + +def test_unknown_kwarg_error_lists_the_supported_settings(): + engine = DocxodusEngine() + with pytest.raises(ValueError) as excinfo: + engine._build_command("Author", "orig", "mod", "out", nonsense=True) + + message = str(excinfo.value) + assert "case_insensitive" in message + assert "detect_moves" in message + + +def test_removed_kwarg_beats_unknown_kwarg_reporting(): + """A removed setting gets its specific message, not the generic 'unknown' one.""" + engine = DocxodusEngine() + with pytest.raises(ValueError, match="no longer"): + engine._build_command("Author", "orig", "mod", "out", engine="docxdiff", nonsense=True) + + +# --- Validation of surviving settings --- def test_docxodus_invalid_move_similarity_threshold(): engine = DocxodusEngine() @@ -137,7 +227,7 @@ def test_docxodus_invalid_move_minimum_word_count_type(): engine._build_command("Author", "orig", "mod", "out", move_minimum_word_count=2.5) -# --- Unit test for _build_command flag construction --- +# --- Unit tests for _build_command flag construction --- def test_build_command_default(): engine = DocxodusEngine() @@ -149,14 +239,19 @@ def test_build_command_default(): assert len(cmd) == 5 # binary + 3 positional + --author +def test_build_command_never_emits_an_engine_flag(): + """--engine= was removed from the CLI in v11.0.0; emitting it exits 1.""" + engine = DocxodusEngine() + cmd = engine._build_command("Author", "/tmp/o.docx", "/tmp/m.docx", "/tmp/out.docx") + assert not any(str(arg).startswith("--engine") for arg in cmd) + + def test_build_command_with_all_flags(): engine = DocxodusEngine() cmd = engine._build_command( "Author", "/tmp/orig.docx", "/tmp/mod.docx", "/tmp/out.docx", - detail_threshold=0.5, case_insensitive=True, detect_moves=True, - simplify_move_markup=True, move_similarity_threshold=0.7, move_minimum_word_count=2, detect_format_changes=False, @@ -166,10 +261,8 @@ def test_build_command_with_all_flags(): assert "--author=Author" in cmd assert "--case-insensitive" in cmd assert "--detect-moves" in cmd - assert "--simplify-move-markup" in cmd assert "--no-detect-format-changes" in cmd assert "--no-conflate-spaces" in cmd - assert "--detail-threshold=0.5" in cmd assert "--move-similarity-threshold=0.7" in cmd assert "--move-minimum-word-count=2" in cmd assert "--date-time=2025-01-01T00:00:00Z" in cmd @@ -197,149 +290,3 @@ 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 " 0 assert stderr is None assert "Revisions found: 9" in stdout + + +def test_xmlpowertools_engine_is_deprecated(): + """It wraps the original, unmaintained Open-XML-PowerTools WmlComparer. + + The package keeps shipping and the class keeps working; instantiating it + has to say that it is on the way out and name what to use instead. + """ + with pytest.warns(DeprecationWarning) as record: + XmlPowerToolsEngine() + + message = str(record[0].message) + assert "DocxodusEngine" in message + + +def test_xmlpowertools_deprecation_points_at_the_caller(): + """stacklevel must blame the user's construction site, not engines.py.""" + with pytest.warns(DeprecationWarning) as record: + XmlPowerToolsEngine() + + assert record[0].filename == __file__ diff --git a/tests/test_temp_file_handling.py b/tests/test_temp_file_handling.py new file mode 100644 index 0000000..faf21e9 --- /dev/null +++ b/tests/test_temp_file_handling.py @@ -0,0 +1,117 @@ +"""Temp-file handling in BaseEngine.run_redline. + +Two bugs live in the same handful of lines, so they are tested together: + +- Issue #30: the output scratch file was created as + ``NamedTemporaryFile(delete=False).name``, which drops the only reference to + the file object. Its finalizer then reports a ResourceWarning and the + descriptor stays open until collection. +- Path inputs were registered for deletion alongside the engine's own scratch + files, so passing a path — a documented input mode — deleted the caller's + source documents. +""" +import gc +import shutil +import sys +import tempfile +import warnings + +import pytest + +from python_redlines.engines import DocxodusEngine + + +@pytest.fixture +def engine(): + return DocxodusEngine() + + +@pytest.fixture +def docs(tmp_path): + """A private copy of the fixtures, so a destructive bug cannot damage them.""" + original = tmp_path / 'original.docx' + modified = tmp_path / 'modified.docx' + shutil.copy('tests/fixtures/original.docx', original) + shutil.copy('tests/fixtures/modified.docx', modified) + return original, modified + + +def test_run_redline_closes_every_temp_file_it_opens(engine, docs, monkeypatch): + """Issue #30, in a form that is observable on every CPython version. + + The ResourceWarning the issue reports is only emitted on 3.13+, so this + asserts the underlying defect instead: a temp file object that run_redline + creates must be closed by the time the call returns. Holding a reference in + the spy is what makes the leak visible — it is precisely the reference the + buggy code drops, letting the finalizer close the file and hide the fault. + """ + original, modified = docs + created = [] + real_named_temp_file = tempfile.NamedTemporaryFile + + def spy(*args, **kwargs): + handle = real_named_temp_file(*args, **kwargs) + created.append(handle) + return handle + + monkeypatch.setattr(tempfile, 'NamedTemporaryFile', spy) + engine.run_redline('Author', original.read_bytes(), modified.read_bytes()) + + unclosed = [handle for handle in created if not handle.closed] + assert not unclosed, ( + f'run_redline abandoned {len(unclosed)} unclosed temp file object(s); ' + 'this is what raises ResourceWarning on CPython 3.13+ (issue #30)' + ) + + +@pytest.mark.skipif( + sys.version_info < (3, 13), + reason="tempfile's finalizer only emits ResourceWarning on CPython 3.13+", +) +def test_run_redline_emits_no_resource_warning(engine, docs): + """The exact symptom issue #30 reports, on the versions that can show it.""" + original, modified = docs + + with warnings.catch_warnings(): + warnings.simplefilter('error', ResourceWarning) + engine.run_redline('Author', original.read_bytes(), modified.read_bytes()) + gc.collect() + + +def test_run_redline_keeps_path_inputs_on_disk(engine, docs): + """Passing paths must not delete the caller's documents.""" + original, modified = docs + + redline_bytes, _, _ = engine.run_redline('Author', str(original), str(modified)) + + assert original.exists(), 'run_redline deleted the original document it was given' + assert modified.exists(), 'run_redline deleted the modified document it was given' + assert redline_bytes[:2] == b'PK' + + +def test_run_redline_accepts_pathlib_paths(engine, docs): + """The docstring and type hint promise pathlib.Path works, not just str.""" + original, modified = docs + + redline_bytes, _, _ = engine.run_redline('Author', original, modified) + + assert original.exists() + assert modified.exists() + assert redline_bytes[:2] == b'PK' + + +def test_run_redline_cleans_up_its_own_scratch_files(engine, docs, tmp_path, monkeypatch): + """The fix must not overshoot: engine-created temp files still get removed. + + tempfile is pointed at a private directory so the assertion sees only what + this call created, not whatever else is using the system temp directory. + """ + original, modified = docs + scratch = tmp_path / 'scratch' + scratch.mkdir() + monkeypatch.setattr(tempfile, 'tempdir', str(scratch)) + + engine.run_redline('Author', original.read_bytes(), modified.read_bytes()) + + leaked = sorted(path.name for path in scratch.iterdir()) + assert not leaked, f'run_redline left scratch files behind: {leaked}' From 5a1ce1f2de7526332d0bb02516e599892a88b000 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sat, 5 Sep 2026 23:16:02 -0500 Subject: [PATCH 2/3] docs: rewrite the engine docs for a DocxDiff-only 1.0.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The README described three ways to compare across two engine classes; two of those three are gone. It now describes two engine classes, one algorithm each, and carries an "Upgrading to 1.0.0" section covering the removed keyword arguments, the re-baselined revision count, and the input-revision policy change that this project's fixtures cannot warn anyone about. The Action self-test no longer asserts an exact revision count. That workflow installs python-redlines from PyPI, so the number it sees tracks the published version rather than the branch under test — pinning it to 10 here would fail every run until 1.0.0 ships, and pinning it to 9 would fail every run afterwards. It asserts a positive count, which is what "the action plumbing works" actually means; the exact count is pinned in tests/test_docxodus_engine.py, which runs against the working tree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014ed9PC7DDco5AFPjsZyy9F --- .github/workflows/redline-action-test.yml | 12 +- CLAUDE.md | 32 ++- README.md | 218 +++++++----------- docs/alternatives.md | 13 +- docs/index.md | 4 +- docs/quickstart.md | 18 +- .../how-to-compare-word-documents-python.md | 17 +- 7 files changed, 146 insertions(+), 168 deletions(-) diff --git a/.github/workflows/redline-action-test.yml b/.github/workflows/redline-action-test.yml index 0e41fe3..1f3efd3 100644 --- a/.github/workflows/redline-action-test.yml +++ b/.github/workflows/redline-action-test.yml @@ -44,7 +44,11 @@ jobs: import json, sys [record] = json.load(sys.stdin) assert record["status"] == "explicit", record - assert record["revisions"] == 9, record + # Not an exact count: this workflow installs python-redlines from PyPI, + # so the number tracks whatever version is published, not this branch. + # The exact count is pinned in tests/test_docxodus_engine.py, which runs + # against the working tree. + assert isinstance(record["revisions"], int) and record["revisions"] > 0, record ' test -s "redlines/tests/fixtures/modified.redline.docx" @@ -87,6 +91,10 @@ jobs: [record] = json.load(sys.stdin) assert record["path"] == "contracts/agreement.docx", record assert record["status"] == "modified", record - assert record["revisions"] == 9, record + # Not an exact count: this workflow installs python-redlines from PyPI, + # so the number tracks whatever version is published, not this branch. + # The exact count is pinned in tests/test_docxodus_engine.py, which runs + # against the working tree. + assert isinstance(record["revisions"], int) and record["revisions"] > 0, record ' test -s "redlines/contracts/agreement.redline.docx" diff --git a/CLAUDE.md b/CLAUDE.md index c0ce19a..d6f678e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,9 +7,10 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co 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) +- **XmlPowerToolsEngine** — wraps Open-XML-PowerTools WmlComparer (original engine). + Deprecated as of 1.0.0: instantiating it emits a `DeprecationWarning`. Still published. - **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. + Runs `DocxDiff`, its only algorithm — Docxodus v11.0.0 deleted `WmlComparer`. ## Monorepo structure — three published packages @@ -78,10 +79,18 @@ 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. `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. + `DocxodusEngine` overrides `_build_command()` to translate kwargs (e.g. `detect_moves`, + `case_insensitive`) into CLI flags. It raises `ValueError` for unknown kwargs and for the + three removed with WmlComparer in Docxodus v11.0.0 — `engine`, `detail_threshold`, + `simplify_move_markup` — rather than dropping them: the CLI warn-and-ignores two of them, + so passing them through would report a setting that did nothing, and silently dropping + `engine="wmlcomparer"` would return DocxDiff output to a caller who asked for something + else. `XmlPowerToolsEngine` uses the legacy 4-positional-arg format and ignores kwargs. + + `BaseEngine.run_redline` registers **only files it created** for cleanup. Adding a + caller-supplied path there deletes the user's own document (fixed in 1.0.0), and the + output scratch file uses `mkstemp`, not `NamedTemporaryFile(delete=False).name`, which + leaked an unclosed file object (issue #30). 2. **Binary packages** ship one platform archive under `src//_binaries/.tar.gz` (or `.zip` for Windows). The archive is @@ -119,10 +128,15 @@ so all three always share one version. Bump only that file. Tests live in repo-root `tests/` and must be run from the repo root (fixtures use relative paths like `tests/fixtures/original.docx`). They require all three packages installed and the binaries built for the current platform. The XmlPowerToolsEngine -integration test validates exactly 9 revisions on the fixture documents. +integration test validates exactly 9 revisions on the fixture documents; the +DocxodusEngine one validates 10. ## Stdout Format Differences - **XmlPowerToolsEngine**: `"Revisions found: 9"` -- **DocxodusEngine**, default / `engine="wmlcomparer"`: `"Redline complete: 9 revision(s) found"` -- **DocxodusEngine**, `engine="docxdiff"`: `"Redline complete: 11 revision(s) found"` +- **DocxodusEngine**: `"Redline complete: 10 revision(s) found"` + +The Docxodus count is measured, not derived — re-measure it after any submodule bump and +re-pin `EXPECTED_REVISIONS` in `tests/test_docxodus_engine.py`. Clear +`~/.cache/python-redlines` first: the extraction cache keys on the installed binary +*package* version, so a rebuilt binary at an unchanged version is otherwise ignored. diff --git a/README.md b/README.md index 5d7bf70..f76dc66 100644 --- a/README.md +++ b/README.md @@ -45,31 +45,29 @@ with open("redline.docx", "wb") as f: That's the whole thing. The rest of this README covers the other engines, comparison settings, and how the packages are built and distributed. -### 🆕 New in 0.3.0: `docxdiff`, an optional next-generation engine +### ⚠️ Breaking in 1.0.0: `WmlComparer` is gone; DocxDiff is the only algorithm -Docxodus now ships a **second comparison algorithm** alongside the classic one. `docxdiff` is a -structure-aware engine that models the document as a tree rather than a stream of runs, so it -produces finer-grained redlines and tracks structural edits — table cell and row properties, section -properties, header and footer content — that the classic algorithm reports coarsely or not at all. +Docxodus v11.0.0 **removed `WmlComparer`**, the algorithm this library ran by default for its +entire life. `DocxDiff` — introduced as an opt-in in 0.3.0, upstream's default since v8.0.0, and +now mature — replaces it. There is no flag to bring the old engine back: it no longer exists in +the binary. -**It is off by default, so nothing about your existing calls changes engines.** Opting in is one -keyword argument: +If you never passed comparison settings, **your code needs no change**. Output will differ, because +the algorithm differs; see [Upgrading to 1.0.0](#upgrading-to-100). + +Three keyword arguments were removed and now raise `ValueError`: ```python -engine.run_redline("Reviewer", original, modified, engine="docxdiff") +engine.run_redline("Reviewer", original, modified, engine="docxdiff") # ValueError +engine.run_redline("Reviewer", original, modified, detail_threshold=0.3) # ValueError +engine.run_redline("Reviewer", original, modified, simplify_move_markup=True) # ValueError ``` -**Please try it and tell us what you find.** It is new, and the two algorithms legitimately disagree -about how to describe the same edit — on this project's own test fixtures `docxdiff` reports 11 -revisions where the classic engine reports 9. Neither is wrong; they segment the same changes -differently. Before adopting it for production redlines, compare its output against your own -documents. See [Choosing an engine](#choosing-an-engine) for the trade-offs and the settings it does -not support. - -> **Note:** the default algorithm is unchanged, but the Docxodus binary behind it moved from v5.4.2 -> to v7.0.0 in this release and carries upstream `WmlComparer` fixes of its own (header references, -> table anchoring). Redline output on the default path can therefore differ from 0.2.1 independently -> of this new flag. Diff a representative document if byte-level stability matters to you. +They raise rather than being ignored on purpose. Two of them are still *accepted* by the +underlying CLI, which warns and does nothing; `--engine` is rejected outright. Silently dropping +`engine="wmlcomparer"` would have handed you DocxDiff output while you believed you had selected +something else — a wrong answer, not a breaking change. Unknown keyword arguments are rejected on +the same reasoning: a typo used to vanish in silence. ## GitHub Action @@ -123,13 +121,13 @@ You can also compare an explicit pair of files instead of auto-detecting: | `base-ref` / `head-ref` | event-derived | Commits to compare. Defaults: PR base (merge-base) → head on `pull_request`, `before` → `after` on `push`, else `HEAD~1` → `HEAD`. | | `author` | `python-redlines` | Author recorded on the tracked changes. | | `engine` | `docxodus` | `docxodus` or `xmlpowertools`. | -| `comparison` | engine default | `wmlcomparer` or `docxdiff` (docxodus engine only). | +| `comparison` | — | **Removed in 1.0.0.** Setting it fails the run; there is no longer an algorithm to select. | | `detect-moves` | `false` | Move detection (docxodus engine only). | | `output-dir` | `redlines` | Where outputs are written (mirrors the source tree). | | `html-preview` | `auto` | `auto` (render when the Docx2Html tool supports `--track-changes`, else warn and skip), `true` (require), `false` (skip — no .NET needed). | | `summary` | `true` | Write the job-summary table. | | `upload-artifact` / `artifact-name` | `true` / `docx-redlines` | Artifact upload controls. | -| `package-version` | latest | pip pin for python-redlines, e.g. `==0.3.0`. | +| `package-version` | latest | pip pin for python-redlines, e.g. `==1.0.0`. | | `docx2html-version` | latest | NuGet pin for the Docx2Html preview tool. | Outputs: `count` (redlines generated), `any-changes`, `redlines` (a JSON array of @@ -147,32 +145,31 @@ Notes: ## Comparison Engines -Python-Redlines gives you **three ways to compare**, across two engine classes. `DocxodusEngine` -carries two interchangeable algorithms in one binary; `XmlPowerToolsEngine` is a separate, legacy -package. +Python-Redlines ships **two engine classes**, each wrapping one algorithm. | # | Choice | How to select it | Algorithm | Status | |---|---|---|---|---| -| 1 | **Docxodus · `wmlcomparer`** | `DocxodusEngine()` | Modernized `WmlComparer` | ✅ **Default.** Stable, recommended | -| 2 | **Docxodus · `docxdiff`** | `DocxodusEngine()` + `engine="docxdiff"` | Structure-aware IR diff | 🆕 New in 0.3.0. Opt-in, seeking feedback | -| 3 | **Open-XML-PowerTools** | `XmlPowerToolsEngine()` | Original `WmlComparer` | 🗄️ Legacy. Upstream archived | - -Choices 1 and 3 are cousins: both descend from Microsoft's `WmlComparer`, which is why choice 1 is -named `wmlcomparer`. Choice 1 is Docxodus's actively-maintained fork of it; choice 3 is the original, -unmaintained code. Choice 2 shares nothing with either but the output format — it is a new engine. +| 1 | **Docxodus** | `DocxodusEngine()` | `DocxDiff` — structure-aware IR diff | ✅ **Default.** Actively maintained | +| 2 | **Open-XML-PowerTools** | `XmlPowerToolsEngine()` | Original `WmlComparer` | ⚠️ Deprecated. Upstream archived | **If you are unsure, use choice 1.** It is the default and requires no arguments. -### 1. `DocxodusEngine` with `wmlcomparer` — the default +Until 1.0.0 there was a third choice — Docxodus running a modernized `WmlComparer`, selected with +`engine="wmlcomparer"`. Docxodus v11.0.0 deleted it, so that choice is gone and the keyword +argument raises `ValueError`. + +### 1. `DocxodusEngine` — the default -**[Docxodus](https://github.com/JSv4/Docxodus)** is a modernized .NET 10.0 fork of Open-XML-PowerTools with -significant improvements: +**[Docxodus](https://github.com/JSv4/Docxodus)** is an actively-maintained .NET 10.0 document +toolchain. Its `DocxDiff` algorithm models the document as an intermediate representation rather +than a stream of runs, which lets it attribute a change to the exact paragraph, cell, row, or +section it touched: -- **Move detection** — identifies content that was moved rather than deleted and re-inserted +- **Structure-aware** — tracks table cell and row properties, section properties, and header and + footer content that a run-stream diff reports coarsely or not at all +- **Native move detection** — identifies content that moved rather than deleting and re-inserting it - **Format change detection** — detects changes to bold, italic, font size, and other run properties -- **Better table handling** — LCS-based row matching for large tables - **Actively maintained** — regular bug fixes and new features -- **Open XML SDK 3.x compatible** — uses the latest SDK version ```python from python_redlines import DocxodusEngine @@ -181,40 +178,24 @@ engine = DocxodusEngine() redline_bytes, stdout, stderr = engine.run_redline("AuthorName", original_bytes, modified_bytes) ``` -### 2. `DocxodusEngine` with `docxdiff` — new, opt-in +### 2. `XmlPowerToolsEngine` — deprecated -The same class and the same binary, selected per call. `docxdiff` models the document as an -intermediate representation, which lets it attribute a change to the exact paragraph, cell, row, or -section it touched. - -```python -from python_redlines import DocxodusEngine - -engine = DocxodusEngine() -redline_bytes, stdout, stderr = engine.run_redline( - "AuthorName", original_bytes, modified_bytes, engine="docxdiff", -) -``` - -Three settings do not exist for this algorithm and raise `ValueError` rather than being silently -ignored — see [Choosing an engine](#choosing-an-engine). - -### 3. `XmlPowerToolsEngine` — legacy - -Wraps the original [Open-XML-PowerTools](https://github.com/OpenXmlDev/Open-Xml-PowerTools) `WmlComparer`. This -engine remains available for backward compatibility and for users who prefer the original comparison behavior. +Wraps the original [Open-XML-PowerTools](https://github.com/OpenXmlDev/Open-Xml-PowerTools) +`WmlComparer`. Instantiating it emits a `DeprecationWarning`. ```python from python_redlines import XmlPowerToolsEngine -engine = XmlPowerToolsEngine() +engine = XmlPowerToolsEngine() # DeprecationWarning redline_bytes, stdout, stderr = engine.run_redline("AuthorName", original_bytes, modified_bytes) ``` -> **Note:** Open-XML-PowerTools was archived by Microsoft and is no longer maintained. It uses an older -> version of the Open XML SDK. While it works for many purposes, Docxodus is the recommended engine going forward. +> **Note:** Open-XML-PowerTools was archived by Microsoft and is no longer maintained. This class +> and its `python-redlines-ooxmlpowertools` wheel still ship, and still work, so that anyone who +> needs the original algorithm's output has somewhere to stand. It will be removed in a future +> major release — move to `DocxodusEngine` when you can. -All three share the same call signature — `run_redline(author, original, modified)` returning +Both share the same call signature — `run_redline(author, original, modified)` returning `(bytes, stdout, stderr)`. They differ in the class you instantiate, which keyword arguments they accept, and their stdout format (see [Stdout Differences](#stdout-differences) below). @@ -248,8 +229,7 @@ See the [Quick Start](#quick-start) above for a minimal example, or the ## Comparison Settings (DocxodusEngine only) `DocxodusEngine` supports fine-grained control over the comparison via keyword arguments to -`run_redline()`. Which arguments are available depends on the algorithm you select — the second -table below is the authoritative matrix. `XmlPowerToolsEngine` accepts none of them. +`run_redline()`. `XmlPowerToolsEngine` accepts none of them. ```python from python_redlines import DocxodusEngine @@ -258,8 +238,6 @@ engine = DocxodusEngine() redline_bytes, stdout, stderr = engine.run_redline( "Reviewer", original, modified, detect_moves=True, - simplify_move_markup=True, - detail_threshold=0.3, case_insensitive=True, ) ``` @@ -268,82 +246,58 @@ 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 | -| `simplify_move_markup` | bool | `False` | Convert moves to del/ins for Word compatibility | | `move_similarity_threshold` | float | `0.8` | Jaccard threshold for move matching (0.0–1.0) | | `move_minimum_word_count` | int | `3` | Minimum words for move detection | -| `detect_format_changes` | bool | `True` | Detect formatting-only changes | +| `detect_format_changes` | bool | `True` | Detect block-level formatting changes | | `conflate_spaces` | bool | `True` | Treat breaking/non-breaking spaces the same | | `date_time` | str | now | Custom ISO 8601 timestamp for revisions | -### Which engine accepts which setting - -| Setting | Docxodus · `wmlcomparer` | Docxodus · `docxdiff` | `XmlPowerToolsEngine` | -|---|:---:|:---:|:---:| -| `engine` | ✅ | ✅ | — ignored | -| `detail_threshold` | ✅ | ❌ `ValueError` | — ignored | -| `case_insensitive` | ✅ | ✅ | — ignored | -| `detect_moves` | ✅ | ✅ | — ignored | -| `simplify_move_markup` | ✅ | ❌ `ValueError` | — ignored | -| `move_similarity_threshold` | ✅ | ✅ | — ignored | -| `move_minimum_word_count` | ✅ | ✅ | — ignored | -| `detect_format_changes` | ✅ | ❌ `ValueError` | — ignored | -| `conflate_spaces` | ✅ | ✅ | — ignored | -| `date_time` | ✅ | ✅ | — ignored | - -**❌ `ValueError`** — `docxdiff` has no equivalent of these three settings. The underlying CLI accepts -and silently discards them, so Python rejects them up front rather than let you believe a setting took -effect when it did not. The check is on the *keyword being present*, whatever its value: pass -`detect_format_changes=True` (its default) with `engine="docxdiff"` and you still get a `ValueError`. -Drop the keyword, or use `engine="wmlcomparer"`. - -**— ignored** — `XmlPowerToolsEngine` silently discards every keyword argument, including `engine`. -This is long-standing behavior, not new. Passing `engine="docxdiff"` to it does nothing. - -> **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). +Anything else raises `ValueError`, including a misspelled setting name. Passing a setting to +`XmlPowerToolsEngine` is still silently ignored — it has never accepted any. -### Choosing an engine +### Settings removed in 1.0.0 -`DocxodusEngine` wraps two comparison algorithms in one binary, selected per call: - -```python -engine.run_redline("Reviewer", original, modified) # wmlcomparer (default) -engine.run_redline("Reviewer", original, modified, engine="docxdiff") # opt in -``` - -| | Reach for `wmlcomparer` | Reach for `docxdiff` | -|---|---|---| -| **When** | You want the long-established algorithm | You want finer-grained, structure-aware redlines | -| **Maturity** | Years of production use | New in 0.3.0 — evaluate on your documents first | -| **Granularity knob** | `detail_threshold` tunes it | Not applicable; granularity is structural | -| **Moves** | Can be lowered to del/ins via `simplify_move_markup` | Rendered natively; cannot be lowered | -| **Structural edits** | Reported coarsely | Attributed to the paragraph, cell, row, or section | - -**The two disagree about revision counts, and that is expected.** On this project's own fixtures, -`wmlcomparer` reports 9 revisions and `docxdiff` reports 11 for the same pair of documents. They -segment the same edits differently — a single reworded sentence may be one revision to one engine and -two to the other. Do not treat a changed count as a defect; do compare the rendered redline against -your own documents before switching. - -**Move markup differs.** `docxdiff` renders moves natively and rejects `simplify_move_markup`, so the -Word-compatibility mitigation 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`. - -**Feedback wanted.** `docxdiff` stays off by default precisely so that adopting 0.3.0 cannot change -your output. If you try it, please -[open an issue](https://github.com/JSv4/Python-Redlines/issues) with what you found — especially -documents where its redline reads worse than `wmlcomparer`'s. +| Setting | Why it is gone | +|---|---| +| `engine` | Selected between `wmlcomparer` and `docxdiff`. Docxodus v11.0.0 deleted `WmlComparer`, so there is nothing to select. | +| `detail_threshold` | Tuned `WmlComparer`'s LCS granularity. `DocxDiff`'s granularity is structural and has no equivalent knob. | +| `simplify_move_markup` | Worked around `WmlComparer`'s move markup. `DocxDiff` renders moves natively. | + +All three raise `ValueError` naming the removal and what to do instead. The check is on the +*keyword being present*, whatever its value: `simplify_move_markup=False` raises too. + +The 0.3.0-era warning that `detect_moves=True` needed `simplify_move_markup=True` to avoid Word's +"unreadable content" dialog applied to `WmlComparer`'s move markup. It does not apply here — +`DocxDiff` emits move markup natively and has no such mitigation, because it needs none. + +## Upgrading to 1.0.0 + +**If you called `run_redline` with no keyword arguments, nothing in your code changes.** Your +output will change, because the algorithm changed. + +1. **Remove `engine=`, `detail_threshold=` and `simplify_move_markup=`.** They raise `ValueError`. + If you were passing `engine="docxdiff"`, delete the argument — you now get it by default. +2. **Re-baseline anything that asserts on revision counts.** On this project's own fixtures the + count moved from 9 (`wmlcomparer`) and 11 (`docxdiff` in 0.3.0) to **10**. Upstream v11 and v12 + changed region arrangement, surplus table cells and section defaults, so the 0.3.0 `docxdiff` + count is not the 1.0.0 count either. +3. **Check documents that already carry tracked changes.** Docxodus v11 dropped + `PreserveInputRevisions` from `DocxCompare.Compare`'s front door to match Word's own Compare + behaviour. Revision-bearing inputs therefore produce different output than in 0.3.0. This + project's fixtures carry no input revisions and will not warn you; diff a representative + document if this is your workload. +4. **Move off `XmlPowerToolsEngine`** when you can — it now warns, and will be removed in a future + major release. +5. **In the GitHub Action, remove the `comparison:` input.** It fails the run. + +Pin `python-redlines==0.3.0` if you need the old `wmlcomparer` output while you migrate. ## Architecture Overview Both engine classes follow the same pattern: a Python wrapper class invokes a self-contained C# binary -via subprocess. `DocxodusEngine`'s two algorithms are one binary selected by a CLI flag, not two binaries. +via subprocess. The repository is a **monorepo of three separately-published packages**: @@ -388,11 +342,9 @@ The engines produce slightly different stdout messages: | Engine | Example stdout | |---|---| | `XmlPowerToolsEngine` | `Revisions found: 9` | -| `DocxodusEngine` (default / `engine="wmlcomparer"`) | `Redline complete: 9 revision(s) found` | -| `DocxodusEngine` (`engine="docxdiff"`) | `Redline complete: 11 revision(s) found` | +| `DocxodusEngine` | `Redline complete: 10 revision(s) found` | -The revision counts differ between the two Docxodus engines because the algorithms differ, -not because either is wrong. +The counts differ because the algorithms differ, not because either is wrong. ## Python-Redlines vs. Commercial Alternatives diff --git a/docs/alternatives.md b/docs/alternatives.md index fae6000..24a4903 100644 --- a/docs/alternatives.md +++ b/docs/alternatives.md @@ -66,10 +66,9 @@ processing API, including a document comparison endpoint, billed per API call. keeps document bytes in your own process the entire time — nothing to configure for data residency because nothing leaves. - **Redline fidelity.** Python-Redlines' default engine (Docxodus) supports move - detection, format-change detection, and structure-aware diffing (via the optional - `docxdiff` algorithm) — producing native Word tracked-changes output tuned - specifically for `.docx`, rather than a generic document-diff endpoint shared across - many file formats. + detection, format-change detection, and structure-aware diffing — producing native + Word tracked-changes output tuned specifically for `.docx`, rather than a generic + document-diff endpoint shared across many file formats. - **No vendor lock-in.** Cloudmersive's comparison logic is closed and proprietary. Python-Redlines is MIT-licensed open source: inspect the C# comparison engines, build them yourself, or contribute a fix upstream. @@ -98,9 +97,9 @@ launching Word, and without the TOS risk. ## High-performance, cross-platform document diffing -Python-Redlines' default engine, [Docxodus](https://github.com/JSv4/Docxodus), is a -modernized, actively-maintained .NET 10 fork of Open-XML-PowerTools' `WmlComparer` — -a high-performance document diffing engine purpose-built for cross-platform Word +Python-Redlines' default engine, [Docxodus](https://github.com/JSv4/Docxodus), is an +actively-maintained .NET 10 document toolchain whose `DocxDiff` algorithm is a +structure-aware, high-performance diffing engine purpose-built for cross-platform Word document comparison. It ships as a prebuilt, self-contained binary embedded directly in the Python wheel for Linux, macOS, and Windows (x64 and arm64), so there's no .NET SDK to install and no compilation step for end users — just `pip install diff --git a/docs/index.md b/docs/index.md index 194a0ab..7b18165 100644 --- a/docs/index.md +++ b/docs/index.md @@ -47,8 +47,8 @@ required. - **Native tracked-changes output** — the redline `.docx` opens in Word with real insertions, deletions, and moves, attributable to an author tag. - **Cross-platform, high-performance diffing engine** — the default - [Docxodus](https://github.com/JSv4/Docxodus) engine is a modernized .NET 10 - fork of Open-XML-PowerTools' `WmlComparer`, shipped as a prebuilt, self-contained + [Docxodus](https://github.com/JSv4/Docxodus) engine runs `DocxDiff`, a + structure-aware .NET 10 comparison algorithm, shipped as a prebuilt, self-contained binary embedded in the wheel for Linux, macOS, and Windows (x64/arm64) — nothing to compile. diff --git a/docs/quickstart.md b/docs/quickstart.md index 592cbb0..f14ba0c 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -2,7 +2,7 @@ `python-redlines` wraps a C# comparison engine to produce tracked-change redline `.docx` files. This guide uses `DocxodusEngine` — the default and recommended engine. -`XmlPowerToolsEngine` (legacy) shares the same call signature; the only behavioural +`XmlPowerToolsEngine` (deprecated) shares the same call signature; the only behavioural difference is that it silently ignores the keyword arguments shown in Step 4. ### Step 0: Install @@ -14,8 +14,8 @@ the engine binary is prebuilt and embedded in the wheel. pip install python-redlines[docxodus] ``` -Use `python-redlines[ooxmlpowertools]` for the legacy engine, or `python-redlines[all]` -for both. +Use `python-redlines[ooxmlpowertools]` for the deprecated engine, or +`python-redlines[all]` for both. ### Step 1: Import and Initialize the Wrapper @@ -58,19 +58,23 @@ with open('/path/to/redline_output.docx', 'wb') as f: ### Step 4: Tune the Comparison (optional, DocxodusEngine only) -`DocxodusEngine` accepts keyword arguments to control move detection, granularity, and -more. See the [main README](https://github.com/JSv4/Python-Redlines#comparison-settings-docxodusengine-only) for +`DocxodusEngine` accepts keyword arguments to control move detection, case sensitivity, +and more. See the [main README](https://github.com/JSv4/Python-Redlines#comparison-settings-docxodusengine-only) for the full table. ```python output = wrapper.run_redline( 'AuthorTag', original_bytes, modified_bytes, detect_moves=True, - simplify_move_markup=True, # required with detect_moves for Word compatibility - detail_threshold=0.3, + case_insensitive=True, ) ``` +An unrecognised keyword argument raises `ValueError`. The `engine`, `detail_threshold` +and `simplify_move_markup` arguments were removed in 1.0.0 along with the `WmlComparer` +algorithm they configured — see +[Upgrading to 1.0.0](https://github.com/JSv4/Python-Redlines#upgrading-to-100). + `XmlPowerToolsEngine` silently ignores these kwargs — switch engines if you need them. ### See also diff --git a/docs/tutorials/how-to-compare-word-documents-python.md b/docs/tutorials/how-to-compare-word-documents-python.md index 78bd594..3583c6c 100644 --- a/docs/tutorials/how-to-compare-word-documents-python.md +++ b/docs/tutorials/how-to-compare-word-documents-python.md @@ -93,26 +93,27 @@ redline_bytes, stdout, stderr = engine.run_redline("Reviewer", original_bytes, m with open("redline.docx", "wb") as f: f.write(redline_bytes) -print(stdout) # e.g. "Redline complete: 9 revision(s) found" +print(stdout) # e.g. "Redline complete: 10 revision(s) found" ``` ## Tuning the comparison -`DocxodusEngine` accepts keyword arguments for move detection, comparison -granularity, and more: +`DocxodusEngine` accepts keyword arguments for move detection, case sensitivity, +and more: ```python redline_bytes, stdout, stderr = engine.run_redline( "Reviewer", original_bytes, modified_bytes, detect_moves=True, - simplify_move_markup=True, # required alongside detect_moves for Word compatibility - detail_threshold=0.3, # lower = more detailed diff case_insensitive=True, ) ``` +An unrecognised setting raises `ValueError` rather than being ignored, so a typo +tells you about itself. + See the [comparison settings reference](https://github.com/JSv4/Python-Redlines#comparison-settings-docxodusengine-only) -for every option and which engine supports it. +for every option. ## Why not automate MS Word instead? @@ -129,7 +130,7 @@ more on this and other trade-offs (Draftable API, Cloudmersive, cloud data priva - [Quickstart guide](../quickstart.md) — the same walkthrough with more detail on engine choice -- [Comparison engines](https://github.com/JSv4/Python-Redlines#comparison-engines) — `wmlcomparer` vs - `docxdiff` vs the legacy Open-XML-PowerTools engine +- [Comparison engines](https://github.com/JSv4/Python-Redlines#comparison-engines) — Docxodus's + `DocxDiff` vs the deprecated Open-XML-PowerTools engine - [Live demo](https://redlines.opensource.legal) — try a comparison in your browser first From 4a3549a2b80795ea20c17a98842f916e148cd643 Mon Sep 17 00:00:00 2001 From: JSv4 Date: Sun, 6 Sep 2026 07:59:44 -0500 Subject: [PATCH 3/3] chore: mark 1.0.0 production-stable and flag the deprecated extra MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Development Status moves from 4 - Beta to 5 - Production/Stable, which is what shipping a 1.0.0 asserts. The `ooxmlpowertools` extra now says in the file that it installs a deprecated engine. `all` deliberately keeps installing it: the extra exists so that anyone who needs the original WmlComparer's output has somewhere to stand, and quietly changing what `all` resolves to would break existing pins to make a point. Drops the brainstorming design doc from the branch — its substance lives in the previous commit message, the PR body, and the README's "Upgrading to 1.0.0" section, and a published package's repo is not the place for a process artifact. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_014ed9PC7DDco5AFPjsZyy9F --- README.md | 4 +- .../2026-09-05-docxodus-v12-upgrade-design.md | 149 ------------------ packages/core/pyproject.toml | 5 +- 3 files changed, 6 insertions(+), 152 deletions(-) delete mode 100644 docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md diff --git a/README.md b/README.md index f76dc66..4e48886 100644 --- a/README.md +++ b/README.md @@ -211,8 +211,8 @@ Each engine ships in its own optional companion package. Install the engine(s) y as extras: ```commandline -pip install python-redlines[docxodus] # Docxodus engine -pip install python-redlines[ooxmlpowertools] # Open-XML-PowerTools engine +pip install python-redlines[docxodus] # Docxodus engine (recommended) +pip install python-redlines[ooxmlpowertools] # Open-XML-PowerTools engine (deprecated) pip install python-redlines[all] # both engine packages ``` diff --git a/docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md b/docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md deleted file mode 100644 index d48636c..0000000 --- a/docs/superpowers/specs/2026-09-05-docxodus-v12-upgrade-design.md +++ /dev/null @@ -1,149 +0,0 @@ -# Docxodus v12.1.0 upgrade: DocxDiff becomes the only algorithm - -**Date:** 2026-09-05 -**Status:** Approved, in implementation -**Target release:** python-redlines 1.0.0 - -## Problem - -The `docxodus` submodule is pinned to v7.0.0. Upstream is at v12.1.0, and -**v11.0.0 deleted the `WmlComparer` engine outright** in favour of `DocxDiff`, -which has been the upstream default since v8.0.0 and is now mature. - -That removal reaches directly into this repository's public API. Since 0.3.0 we -have exposed the upstream selector as a keyword argument: - -```python -engine.run_redline("Reviewer", original, modified, engine="docxdiff") -``` - -`--engine=` no longer exists on the `redline` CLI. Two more flags — -`--detail-threshold=` and `--simplify-move-markup` — survive only as -warn-and-ignore stubs that upstream describes as kept "one release longer". - -## Goals - -1. Track upstream to v12.1.0. -2. Remove the dead comparison-settings surface **loudly**, never silently. -3. Fix issue #30 and the data-loss bug found beside it. -4. Ship as 1.0.0 with release notes that lead with the WmlComparer removal. - -## Non-goals - -- Retiring `XmlPowerToolsEngine` or the `python-redlines-ooxmlpowertools` - package. It wraps the *original* Open-XML-PowerTools WmlComparer, is a - separately published wheel that users have pinned, and retiring it is its own - decision with its own migration story. It gets a `DeprecationWarning` and - keeps shipping. -- Any change to the `wmlcomparer`-era *output* of already-published versions. - 0.3.0 remains on PyPI for anyone who needs that algorithm. - -## Design - -### 1. Submodule - -`docxodus` v7.0.0 → v12.1.0. Verified unchanged and therefore requiring no -build changes: `TargetFramework` is still `net10.0` and `AssemblyName` is still -`redline`, which `build_differ.py` and `BINARY_BASE_NAME` hardcode respectively. - -### 2. `DocxodusEngine` — removed settings become hard errors - -`engine`, `detail_threshold` and `simplify_move_markup` raise `ValueError` -naming the argument, the version that removed it, and what to do instead. - -**This rejection is the load-bearing part of the change.** Deleting the code -path without it would not produce a breaking change; it would produce a -*wrong-answer* change. `_build_command` only reads keys it knows about, so -`run_redline(..., engine="wmlcomparer")` would become a silently-ignored kwarg -and hand the caller DocxDiff output while they believe they selected -WmlComparer. Rejection also covers `engine="docxdiff"`: it names a selector -that no longer exists, and leaving a vestigial argument that happens to match -the surviving engine invites code that breaks confusingly later. - -A second reason the rejection cannot be skipped: v12's CLI writes a *warning to -stderr* for `--detail-threshold=` and `--simplify-move-markup`. `run_redline` -surfaces any non-empty stderr to the caller, so passing them through would -break every `assert stderr is None` in the suite. - -**Unknown keyword arguments are also rejected.** Today `detial_threshold=0.5` -is silently discarded. Hard-erroring on removed arguments while still -swallowing typos would be half a fix, and the two failures are indistinguishable -to a user. - -Retained and unchanged: `case_insensitive`, `detect_moves`, -`move_similarity_threshold`, `move_minimum_word_count`, `detect_format_changes`, -`conflate_spaces`, `date_time`. - -Note for the release notes: upstream v11 remapped `--no-detect-format-changes` -onto `DocxDiffSettings.TrackBlockFormatChanges`, so its meaning shifted from -run-level to block-level format tracking. - -### 3. `BaseEngine` — two temp-file bugs - -**Issue #30 — leaked `NamedTemporaryFile`.** -`tempfile.NamedTemporaryFile(delete=False).name` constructs the object, takes -its name and drops the only reference. The finalizer then reports -`ResourceWarning: Implicitly cleaning up ...` on CPython 3.13+, once per call, -and the descriptor stays open until collection. Under `-W error` it surfaces as -an unraisable exception attributed to whatever test was running when the -collector fired. Replaced with `tempfile.mkstemp()` + `os.close(fd)`, which is -the right tool when only a path is wanted. - -**Data loss on path inputs (found while fixing #30).** -`run_redline` appends `original_path` and `modified_path` to `temp_files` -unconditionally, and the `finally` block `os.remove`s every entry. When the -caller passes a *path* — a documented, type-hinted input mode that -`docs/quickstart.md` demonstrates — those entries are the caller's own files, -and the call deletes both source documents. Only byte inputs are safe, which is -why the GitHub Action (bytes-only) has never hit it. - -Fix: register a file for cleanup only when this library created it. - -### 4. `XmlPowerToolsEngine` - -`DeprecationWarning` on instantiation, naming DocxodusEngine as the -replacement. The class and its wheel keep working. - -### 5. GitHub Action - -The `comparison` input maps onto the deleted kwarg. It becomes a `ConfigError` -with the same reasoning as the Python surface. `action.yml`, the -`DOCXODUS_ONLY_INPUTS` validation in `action/redline_changed.py`, and -`tests/test_action_script.py` all move together. - -### 6. Revision counts are measured, not predicted - -v11 and v12 changed region arrangement, surplus table cells, section defaults -and note pruning, so the fixture count was unlikely to be either of the numbers -previously pinned (9 for wmlcomparer, 11 for docxdiff). Measured against a -freshly built v12.1.0 binary on `tests/fixtures/`, it is **10** — neither. Every -assertion and doc table is pinned to that measurement. - -The extraction cache must be cleared first: `_extraction_root()` keys on the -installed *binary package* version, which does not change when `__about__.py` -is edited, so a stale `~/.cache/python-redlines` would silently keep serving -the v7 binary. - -## Testing - -- One rejection test per removed argument, plus unknown-kwarg rejection. -- `ResourceWarning`-as-error regression test for #30. -- A test that path inputs still exist after `run_redline` returns. -- Re-pinned integration counts, measured. -- `DeprecationWarning` on `XmlPowerToolsEngine()`. -- Action config tests for the rejected `comparison` input. - -## Release - -Version 0.3.0 → **1.0.0** in `packages/core/src/python_redlines/__about__.py` -(the single source of truth all three packages read). - -Release notes lead with the removal. Wording is deliberate: *"the Docxodus -engine no longer carries WmlComparer"* — **not** *"Python-Redlines is -WmlComparer-free"*, which would be false while the `ooxmlpowertools` wheel -still ships the original. - -The notes must also state that upstream v11 dropped `PreserveInputRevisions` -from `DocxCompare.Compare`'s front door, so output shifts on revision-bearing -inputs. This project's fixtures carry no input revisions and will not catch it; -users comparing already-redlined documents will. diff --git a/packages/core/pyproject.toml b/packages/core/pyproject.toml index b0ba2b7..dcc7efc 100644 --- a/packages/core/pyproject.toml +++ b/packages/core/pyproject.toml @@ -27,7 +27,7 @@ authors = [ { name = "John Scrudato IV" }, ] classifiers = [ - "Development Status :: 4 - Beta", + "Development Status :: 5 - Production/Stable", "License :: OSI Approved :: MIT License", "Programming Language :: Python", "Programming Language :: Python :: 3.9", @@ -44,6 +44,9 @@ dependencies = [ # its extra, e.g. `pip install python-redlines[docxodus]`. All three packages # are released together from the same repository on each tagged release. [project.optional-dependencies] +# Deprecated: wraps the archived Open-XML-PowerTools WmlComparer. Kept so that +# anyone who needs that algorithm's output has somewhere to stand; `all` keeps +# installing it so the extra does not change meaning under existing pins. ooxmlpowertools = ["python-redlines-ooxmlpowertools"] docxodus = ["python-redlines-docxodus"] all = ["python-redlines-ooxmlpowertools", "python-redlines-docxodus"]