From 42adf10106cd122620f24f8619ac2656a95c1f62 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 14 Jun 2026 09:46:21 +0000 Subject: [PATCH 1/2] docs: fix stale task count, dangling refs, and Python version drift - tasks.md: correct the summary count (20 done / 14 remaining, was 8/26) - quickstart.md: repoint the plan reference to specs/ and add the real docs/onboarding.md entry point (was dangling); 3.11 -> 3.9.x - plan.md / research.md: devcontainer pins the Python feature to 3.9.4 (was described as a "3.11 image"); rejected-alternative mentions left intact - add docs/onboarding.md (satisfies SC-007) and wire it into the MkDocs nav --- docs/onboarding.md | 47 +++++++++++++++++++ mkdocs.yml | 1 + specs/001-codespace-xml-scaffold/plan.md | 2 +- .../001-codespace-xml-scaffold/quickstart.md | 5 +- specs/001-codespace-xml-scaffold/research.md | 2 +- specs/001-codespace-xml-scaffold/tasks.md | 2 +- 6 files changed, 54 insertions(+), 5 deletions(-) create mode 100644 docs/onboarding.md diff --git a/docs/onboarding.md b/docs/onboarding.md new file mode 100644 index 0000000..eaf2066 --- /dev/null +++ b/docs/onboarding.md @@ -0,0 +1,47 @@ +# Onboarding + +> **Start point for a new contributor.** This single page gets you to a green build and +> shows you where everything lives. For the guided, explained walkthrough, follow the +> [Start here tutorial](tutorials/01-start-here.md); for a command-by-command validation +> run, see the [quickstart](https://github.com/deepbluecltd/python-party/blob/main/specs/001-codespace-xml-scaffold/quickstart.md). + +## Reach green + +**GitHub Codespace (primary):** on the repository page, **Code → Codespaces → Create +codespace**. Provisioning runs `make bootstrap` for you (installs the toolchain on Python +3.9.4 and generates the models). When the terminal is ready: + +```bash +make verify # lint + tests — confirm the environment is green +make pipeline # produce build/acoustic_dataset.xml (schema-valid, round-trip-equal) +``` + +**Local fallback:** you need Python 3.9.x and `make`, then run `make bootstrap` yourself +before the same `make verify` / `make pipeline`. Both paths reach the *same* green state +([ADR 0006](decisions/0006-codespaces-with-local-fallback.md)). Run `make help` to see every target. + +## Where things live + +| You're looking for… | It's here | +|---|---| +| The contract (the XSD) — **swap location** for the real schema | `schema/acoustic_dataset.xsd` ([how-to](how-to/swap-in-the-real-schema.md)) | +| The scientific seams (named, testable calc functions) | `src/acoustic_dataset/acoustics/` | +| The **one** place calc output becomes typed objects | `src/acoustic_dataset/mapping.py` | +| Generated models (never hand-edited — regenerate) | `src/acoustic_dataset/models/` | +| Example calculation input (placeholder) | `examples/calculation_input.json` | +| Tests (unit / integration / golden) | `tests/` | +| The plan & design artifacts | `specs/001-codespace-xml-scaffold/` (`spec.md`, `plan.md`, `tasks.md`) | +| Why each choice was made | [Decision records](decisions/index.md) | + +## Build the mental model + +Read these, in order — they're short: + +1. [Schema as the contract](concepts/schema-as-contract.md) — the idea everything follows from. +2. [The two verification gates](concepts/two-verification-gates.md) — why schema-valid isn't the same as correct. +3. [Pipeline data flow](concepts/pipeline-data-flow.md) — how input becomes validated XML. + +!!! tip "What's done, what's next" + The environment and the end-to-end pipeline are in place. Remaining pipeline work + (migration-safety `compare`, generated schema docs/ERD, the distribution bundle) is + tracked in `specs/001-codespace-xml-scaffold/tasks.md`. diff --git a/mkdocs.yml b/mkdocs.yml index 6b3a113..4ffd2b6 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -41,6 +41,7 @@ markdown_extensions: nav: - Home: index.md + - Onboarding: onboarding.md - Tutorials: - Start here (zero to green): tutorials/01-start-here.md - How-to guides: diff --git a/specs/001-codespace-xml-scaffold/plan.md b/specs/001-codespace-xml-scaffold/plan.md index 290921d..be31201 100644 --- a/specs/001-codespace-xml-scaffold/plan.md +++ b/specs/001-codespace-xml-scaffold/plan.md @@ -83,7 +83,7 @@ specs/001-codespace-xml-scaffold/ ```text .devcontainer/ -└── devcontainer.json # Codespaces: Python 3.11 image, post-create bootstrap, extensions +└── devcontainer.json # Codespaces: Python 3.9.4 (devcontainer feature), post-create bootstrap, extensions .github/ └── workflows/ diff --git a/specs/001-codespace-xml-scaffold/quickstart.md b/specs/001-codespace-xml-scaffold/quickstart.md index 697f485..2020457 100644 --- a/specs/001-codespace-xml-scaffold/quickstart.md +++ b/specs/001-codespace-xml-scaffold/quickstart.md @@ -6,7 +6,7 @@ detail lives in `tasks.md` and the code itself. Commands map to the contract in ## Prerequisites -- A GitHub account (for the Codespaces path), **or** Python 3.11 + `make` locally (fallback path). +- A GitHub account (for the Codespaces path), **or** Python 3.9.x + `make` locally (fallback path). - No other local tooling required — provisioning installs everything. ## Path A — GitHub Codespaces (primary) @@ -81,6 +81,7 @@ make bundle - The contract: `schema/acoustic_dataset.xsd` (PLACEHOLDER — swap here). - The one place logic lives: `src/acoustic_dataset/mapping.py`. - Scientific seams: `src/acoustic_dataset/acoustics/`. -- The source plan: `docs/delivery-plan.md`; onboarding: `docs/onboarding.md`. +- The source plan & design artifacts: `specs/001-codespace-xml-scaffold/` (`spec.md`, `plan.md`). +- Onboarding entry point: `docs/onboarding.md` (also the tutorial `docs/tutorials/01-start-here.md`). *(SC-007: a second contributor should reach green and locate these using only `docs/onboarding.md`.)* diff --git a/specs/001-codespace-xml-scaffold/research.md b/specs/001-codespace-xml-scaffold/research.md index 5b87454..48ece0a 100644 --- a/specs/001-codespace-xml-scaffold/research.md +++ b/specs/001-codespace-xml-scaffold/research.md @@ -49,7 +49,7 @@ decisions, each with rationale and rejected alternatives, so the plan is grounde ## D5 — Codespaces environment: devcontainer -- **Decision**: A `.devcontainer/devcontainer.json` on the Python 3.11 image with a `postCreateCommand` +- **Decision**: A `.devcontainer/devcontainer.json` pinning the Python feature to 3.9.4 with a `postCreateCommand` that runs `make bootstrap` (install deps, generate models). Document an identical local path. - **Rationale**: FR-001/FR-002 one-click provisioning with no manual steps; FR-004 portability via the same Make targets locally. Keeps the environment definition declarative and version-controlled. diff --git a/specs/001-codespace-xml-scaffold/tasks.md b/specs/001-codespace-xml-scaffold/tasks.md index aea18bf..ce693b6 100644 --- a/specs/001-codespace-xml-scaffold/tasks.md +++ b/specs/001-codespace-xml-scaffold/tasks.md @@ -191,4 +191,4 @@ placeholder once it arrives. - `[P]` = different files, no dependencies. `[Story]` ties a task to a user story for traceability. - Generated artifacts (`src/acoustic_dataset/models/`, `docs/reference/schema/`) are **never** hand-edited — regenerate (ADR 0008). - Commit after each task or logical group; keep `make verify` green. -- Total: 34 tasks — 8 already complete (`[X]`), 26 remaining (the pipeline). +- Total: 34 tasks — 20 already complete (`[X]`), 14 remaining (the pipeline). From 3850464199ac8b16a93c4424695e923d2f5978d7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 14 Jun 2026 09:57:29 +0000 Subject: [PATCH 2/2] feat(compare): implement US3 migration-safety comparison (T021-T023) Catch schema-valid-but-different output against a known-good reference (FR-015, SC-005). - compare.py: canonicalising comparison. Equality key uses C14N with rewrite_prefixes (prefix/attribute/whitespace/comment-insensitive); the diff renderer strips namespaces for a readable, prefix-independent unified diff. Pure functions shared by CLI and tests. - cli.py: implement `compare ` (exit 0 = clean match, 1 = difference or IO/parse error) with required typed positionals. - Makefile: wire `make compare` with overridable COMPARE_GENERATED / COMPARE_REFERENCE. - examples/reference/trial_known_good.xml: shipped placeholder known-good reference, serialised with a ds: prefix to demonstrate cosmetic-difference tolerance. - tests/integration/test_compare.py + reference_path fixture: cosmetic-ignored (attr order, whitespace, prefix, comments), schema-valid-but-different surfaced, and CLI exit codes. - docs: mark compare done (tasks.md, commands.md, onboarding.md); add how-to guide + nav. --- Makefile | 7 +- .../run-the-migration-safety-comparison.md | 61 ++ docs/onboarding.md | 6 +- docs/reference/commands.md | 6 +- examples/reference/trial_known_good.xml | 572 ++++++++++++++++++ mkdocs.yml | 1 + specs/001-codespace-xml-scaffold/tasks.md | 21 +- src/acoustic_dataset/cli.py | 32 +- src/acoustic_dataset/compare.py | 101 ++++ tests/conftest.py | 5 + tests/integration/test_compare.py | 106 ++++ 11 files changed, 896 insertions(+), 22 deletions(-) create mode 100644 docs/how-to/run-the-migration-safety-comparison.md create mode 100644 examples/reference/trial_known_good.xml create mode 100644 src/acoustic_dataset/compare.py create mode 100644 tests/integration/test_compare.py diff --git a/Makefile b/Makefile index d9ffeff..3f69bb5 100644 --- a/Makefile +++ b/Makefile @@ -36,8 +36,11 @@ gen-schema-docs: ## Generate schema reference pages + Mermaid ERD from the enric pipeline: ## End-to-end: map example input -> objects -> XML -> validate -> round-trip. python -m acoustic_dataset.cli pipeline -compare: ## Migration-safety diff of generated output vs a known-good reference. (Phase 1 task) - @echo "Not yet implemented — tracked in specs/001-codespace-xml-scaffold/tasks.md"; exit 1 +COMPARE_GENERATED ?= build/acoustic_dataset.xml +COMPARE_REFERENCE ?= examples/reference/trial_known_good.xml + +compare: ## Migration-safety diff: $(COMPARE_GENERATED) vs $(COMPARE_REFERENCE) (override the vars). + python -m acoustic_dataset.cli compare "$(COMPARE_GENERATED)" "$(COMPARE_REFERENCE)" bundle: ## Produce distribution bundle (data + schema + generated models). (Phase 1 task) @echo "Not yet implemented — tracked in specs/001-codespace-xml-scaffold/tasks.md"; exit 1 diff --git a/docs/how-to/run-the-migration-safety-comparison.md b/docs/how-to/run-the-migration-safety-comparison.md new file mode 100644 index 0000000..ba49157 --- /dev/null +++ b/docs/how-to/run-the-migration-safety-comparison.md @@ -0,0 +1,61 @@ +# How to run the migration-safety comparison + +> **How-to** — check freshly generated XML against a trusted known-good file and catch the +> dangerous *schema-valid-but-different* case. + +Schema validity proves the output fits the contract; it does **not** prove it says the same +thing as a file a consumer already depends on. The `compare` command closes that gap +([ADR 0004](../decisions/0004-two-gate-verification.md), FR-015). + +## Steps + +1. **Get a known-good reference.** Drop a trusted prior-process file (e.g. one of the + consumer's trial outputs) into `examples/reference/`. A shipped placeholder, + `examples/reference/trial_known_good.xml`, is there as a template. + +2. **Produce fresh output.** + ```bash + make pipeline # writes build/acoustic_dataset.xml + ``` + +3. **Compare.** + ```bash + make compare # build/acoustic_dataset.xml vs examples/reference/trial_known_good.xml + ``` + Point it at other files by overriding the variables: + ```bash + make compare COMPARE_GENERATED=build/acoustic_dataset.xml \ + COMPARE_REFERENCE=examples/reference/your-trial-file.xml + ``` + Or call the CLI directly: + ```bash + python -m acoustic_dataset.cli compare + ``` + +## How to read the result + +- **Clean match** → prints `match: …` and exits `0`. The documents are identical once + cosmetic noise is removed. +- **Meaningful difference** → prints a unified diff (reference → generated) and exits non-zero, + so it fails CI and scripts rather than passing silently. + +## What counts as cosmetic (ignored) vs meaningful (caught) + +The comparison canonicalises both sides first, so these **never** cause a failure: + +| Ignored (cosmetic) | Example | +|---|---| +| Attribute order | `a="1" b="2"` vs `b="2" a="1"` | +| Whitespace / indentation | pretty-printed vs single-line | +| Namespace **prefix** | default `xmlns=…` vs `ds:` prefix (same URI) | +| Comments | a `` banner | + +Anything that changes the **data** — an element value, a missing or extra element, a different +structure — is reported as a difference. For instance, a radiated-noise `Level` of `134.000` +that becomes `144.000` is still inside the schema's decibel band (so it stays schema-valid) but +is surfaced as a one-line diff. + +!!! note "Why prefixes and comments are ignored" + A reference file from another tool may be serialised in a different style. The equality + decision is fully namespace-aware; only the *prefix label* and documentation comments are + treated as presentation, so style differences don't drown out the differences that matter. diff --git a/docs/onboarding.md b/docs/onboarding.md index eaf2066..2dafc60 100644 --- a/docs/onboarding.md +++ b/docs/onboarding.md @@ -42,6 +42,6 @@ Read these, in order — they're short: 3. [Pipeline data flow](concepts/pipeline-data-flow.md) — how input becomes validated XML. !!! tip "What's done, what's next" - The environment and the end-to-end pipeline are in place. Remaining pipeline work - (migration-safety `compare`, generated schema docs/ERD, the distribution bundle) is - tracked in `specs/001-codespace-xml-scaffold/tasks.md`. + The environment, the end-to-end pipeline, and the migration-safety `compare` are in + place. Remaining pipeline work (generated schema docs/ERD and the distribution bundle) + is tracked in `specs/001-codespace-xml-scaffold/tasks.md`. diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 5575f9f..dfe2d37 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -12,10 +12,10 @@ | `make verify` | Lint (`ruff`) + tests (`pytest`) | ✅ | | `make docs` | Build static HTML site (`mkdocs build --strict`) → `site/` | ✅ | | `make docs-serve` | Live-preview docs at | ✅ | -| `make generate` | Regenerate models from `schema/*.xsd` via `xsdata` | ⏳ Phase 1 task | +| `make generate` | Regenerate models from `schema/*.xsd` via `xsdata` | ✅ | | `make gen-schema-docs` | Generate schema reference + Mermaid ERD from the enriched XSD | ⏳ Phase 1 task | -| `make pipeline` | End-to-end: map → serialise → validate → round-trip | ⏳ Phase 1 task | -| `make compare` | Migration-safety diff vs a known-good reference | ⏳ Phase 1 task | +| `make pipeline` | End-to-end: map → serialise → validate → round-trip | ✅ | +| `make compare` | Migration-safety diff vs a known-good reference | ✅ | | `make bundle` | Distribution bundle (data + schema + models) | ⏳ Phase 1 task | ⏳ targets currently print a pointer to `specs/001-codespace-xml-scaffold/tasks.md` and exit diff --git a/examples/reference/trial_known_good.xml b/examples/reference/trial_known_good.xml new file mode 100644 index 0000000..cfb190d --- /dev/null +++ b/examples/reference/trial_known_good.xml @@ -0,0 +1,572 @@ + + + + 0.2.0-placeholder + Illustrative platform (placeholder) + 2026-06-14T00:00:00Z + + 7.500 + 95.000 + 2400.000 + 1998 + + + + 50.000 + + + 0.000 + 134.000 + + + 30.000 + 134.804 + + + 60.000 + 137.000 + + + 90.000 + 140.000 + + + 120.000 + 143.000 + + + 150.000 + 145.196 + + + 180.000 + 146.000 + + + 210.000 + 145.196 + + + 240.000 + 143.000 + + + 270.000 + 140.000 + + + 300.000 + 137.000 + + + 330.000 + 134.804 + + + + + 100.000 + + + 0.000 + 129.000 + + + 30.000 + 129.804 + + + 60.000 + 132.000 + + + 90.000 + 135.000 + + + 120.000 + 138.000 + + + 150.000 + 140.196 + + + 180.000 + 141.000 + + + 210.000 + 140.196 + + + 240.000 + 138.000 + + + 270.000 + 135.000 + + + 300.000 + 132.000 + + + 330.000 + 129.804 + + + + + 200.000 + + + 0.000 + 124.000 + + + 30.000 + 124.804 + + + 60.000 + 127.000 + + + 90.000 + 130.000 + + + 120.000 + 133.000 + + + 150.000 + 135.196 + + + 180.000 + 136.000 + + + 210.000 + 135.196 + + + 240.000 + 133.000 + + + 270.000 + 130.000 + + + 300.000 + 127.000 + + + 330.000 + 124.804 + + + + + 400.000 + + + 0.000 + 119.000 + + + 30.000 + 119.804 + + + 60.000 + 122.000 + + + 90.000 + 125.000 + + + 120.000 + 128.000 + + + 150.000 + 130.196 + + + 180.000 + 131.000 + + + 210.000 + 130.196 + + + 240.000 + 128.000 + + + 270.000 + 125.000 + + + 300.000 + 122.000 + + + 330.000 + 119.804 + + + + + 800.000 + + + 0.000 + 114.000 + + + 30.000 + 114.804 + + + 60.000 + 117.000 + + + 90.000 + 120.000 + + + 120.000 + 123.000 + + + 150.000 + 125.196 + + + 180.000 + 126.000 + + + 210.000 + 125.196 + + + 240.000 + 123.000 + + + 270.000 + 120.000 + + + 300.000 + 117.000 + + + 330.000 + 114.804 + + + + + 1600.000 + + + 0.000 + 109.000 + + + 30.000 + 109.804 + + + 60.000 + 112.000 + + + 90.000 + 115.000 + + + 120.000 + 118.000 + + + 150.000 + 120.196 + + + 180.000 + 121.000 + + + 210.000 + 120.196 + + + 240.000 + 118.000 + + + 270.000 + 115.000 + + + 300.000 + 112.000 + + + 330.000 + 109.804 + + + + + 3200.000 + + + 0.000 + 104.000 + + + 30.000 + 104.804 + + + 60.000 + 107.000 + + + 90.000 + 110.000 + + + 120.000 + 113.000 + + + 150.000 + 115.196 + + + 180.000 + 116.000 + + + 210.000 + 115.196 + + + 240.000 + 113.000 + + + 270.000 + 110.000 + + + 300.000 + 107.000 + + + 330.000 + 104.804 + + + + + 6400.000 + + + 0.000 + 99.000 + + + 30.000 + 99.804 + + + 60.000 + 102.000 + + + 90.000 + 105.000 + + + 120.000 + 108.000 + + + 150.000 + 110.196 + + + 180.000 + 111.000 + + + 210.000 + 110.196 + + + 240.000 + 108.000 + + + 270.000 + 105.000 + + + 300.000 + 102.000 + + + 330.000 + 99.804 + + + + + 12800.000 + + + 0.000 + 94.000 + + + 30.000 + 94.804 + + + 60.000 + 97.000 + + + 90.000 + 100.000 + + + 120.000 + 103.000 + + + 150.000 + 105.196 + + + 180.000 + 106.000 + + + 210.000 + 105.196 + + + 240.000 + 103.000 + + + 270.000 + 100.000 + + + 300.000 + 97.000 + + + 330.000 + 94.804 + + + + + 25600.000 + + + 0.000 + 89.000 + + + 30.000 + 89.804 + + + 60.000 + 92.000 + + + 90.000 + 95.000 + + + 120.000 + 98.000 + + + 150.000 + 100.196 + + + 180.000 + 101.000 + + + 210.000 + 100.196 + + + 240.000 + 98.000 + + + 270.000 + 95.000 + + + 300.000 + 92.000 + + + 330.000 + 89.804 + + + + + + + AS-900 Echo + DeepBlue Sonics + 6000.000 + 215.000 + 15.000 + 0.500 + 118850.223 + + + PA-110 Flank Array + DeepBlue Sonics + 1500.000 + 18.000 + 10.000 + 1.500 + + + PA-220 Towed Array + Marine Acoustics Ltd + 300.000 + 22.000 + 8.000 + 2.000 + + + diff --git a/mkdocs.yml b/mkdocs.yml index 4ffd2b6..c87b251 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -48,6 +48,7 @@ nav: - Use the Codespace: how-to/use-the-codespace.md - Build the docs site: how-to/build-the-docs-site.md - Swap in the real schema: how-to/swap-in-the-real-schema.md + - Run the migration-safety comparison: how-to/run-the-migration-safety-comparison.md - Add a decision record: how-to/add-a-decision-record.md - Concepts: - Schema as the contract: concepts/schema-as-contract.md diff --git a/specs/001-codespace-xml-scaffold/tasks.md b/specs/001-codespace-xml-scaffold/tasks.md index ce693b6..85ea759 100644 --- a/specs/001-codespace-xml-scaffold/tasks.md +++ b/specs/001-codespace-xml-scaffold/tasks.md @@ -16,9 +16,12 @@ round-trip gate (FR-009, FR-010, FR-014), so test tasks are first-class here. > **Status note**: Phase 1 (Setup) and User Story 1 (zero-to-green) landed first. Phase 2 > (Foundational) and **User Story 2 — the schema-driven pipeline MVP** are now implemented: > enriched placeholder XSD, xsdata generation (3.9-safe, idempotent), the single mapping, the -> two structural gates, `make generate` / `make pipeline`, and the golden-file tests. The -> **remaining work is US3 (compare), US5 (generated schema docs/ERD) and US4 (bundle + drift -> gate)**; their `compare`/`bundle` CLI subcommands exist but exit non-zero as not-yet-implemented. +> two structural gates, `make generate` / `make pipeline`, and the golden-file tests. +> **User Story 3 — the migration-safety `compare`** is now implemented too: a canonicalising +> comparison (prefix/attribute/whitespace/comment-insensitive), `make compare`, a shipped +> known-good reference fixture, and its tests. The **remaining work is US5 (generated schema +> docs/ERD) and US4 (bundle + drift gate)**; the `bundle` CLI subcommand exists but exits +> non-zero as not-yet-implemented. ## Format: `[ID] [P?] [Story] Description` @@ -84,17 +87,17 @@ round-trip gate (FR-009, FR-010, FR-014), so test tasks are first-class here. --- -## Phase 5: User Story 3 - Migration-safety comparison (Priority: P2) +## Phase 5: User Story 3 - Migration-safety comparison (Priority: P2) — ✅ COMPLETE **Goal**: Diff generated output against a known-good reference; surface *schema-valid-but-different*. **Independent Test**: Identical output → clean match (exit 0); schema-valid-but-different → diff + non-zero exit. -- [ ] T021 [US3] Implement canonicalise (sort attributes, normalise whitespace/namespaces) + diff in `src/acoustic_dataset/compare.py` -- [ ] T022 [US3] Wire `compare` subcommand + `make compare` in `src/acoustic_dataset/cli.py` (depends on T021) -- [ ] T023 [P] [US3] Reference fixture in `examples/reference/` + tests (clean match, meaningful diff, cosmetic-only ignored) in `tests/integration/test_compare.py` +- [X] T021 [US3] Implement canonicalise (sort attributes, normalise whitespace/namespaces) + diff in `src/acoustic_dataset/compare.py` +- [X] T022 [US3] Wire `compare` subcommand + `make compare` in `src/acoustic_dataset/cli.py` (depends on T021) +- [X] T023 [P] [US3] Reference fixture in `examples/reference/` + tests (clean match, meaningful diff, cosmetic-only ignored) in `tests/integration/test_compare.py` -**Checkpoint**: `make compare` catches a schema-valid-but-different file (SC-005). +**Checkpoint**: `make compare` catches a schema-valid-but-different file (SC-005). ✅ --- @@ -191,4 +194,4 @@ placeholder once it arrives. - `[P]` = different files, no dependencies. `[Story]` ties a task to a user story for traceability. - Generated artifacts (`src/acoustic_dataset/models/`, `docs/reference/schema/`) are **never** hand-edited — regenerate (ADR 0008). - Commit after each task or logical group; keep `make verify` green. -- Total: 34 tasks — 20 already complete (`[X]`), 14 remaining (the pipeline). +- Total: 34 tasks — 23 already complete (`[X]`), 11 remaining (the pipeline). diff --git a/src/acoustic_dataset/cli.py b/src/acoustic_dataset/cli.py index 3d57f2a..7d06dae 100644 --- a/src/acoustic_dataset/cli.py +++ b/src/acoustic_dataset/cli.py @@ -79,12 +79,32 @@ def cmd_validate(args: argparse.Namespace) -> int: def cmd_compare(args: argparse.Namespace) -> int: + """Migration-safety diff: catch schema-valid-but-different output vs a reference.""" + from lxml import etree + + from acoustic_dataset import compare as compare_mod + + generated, reference = args.generated, args.reference + for path in (generated, reference): + if not path.is_file(): + print(f"error: file not found: {path}", file=sys.stderr) + return 1 + + try: + result = compare_mod.compare(generated, reference) + except etree.XMLSyntaxError as exc: + print(f"error: could not parse XML: {exc}", file=sys.stderr) + return 1 + + if result.equal: + print(f"match: {generated} is canonically identical to {reference}") + return 0 print( - "compare is not yet implemented (User Story 3 — see " - "specs/001-codespace-xml-scaffold/tasks.md).", + f"different: {generated} is schema-shaped but differs from {reference}", file=sys.stderr, ) - return _NOT_IMPLEMENTED + print(result.diff, file=sys.stderr) + return 1 def cmd_bundle(args: argparse.Namespace) -> int: @@ -119,8 +139,10 @@ def build_parser() -> argparse.ArgumentParser: p_val.set_defaults(func=cmd_validate) p_cmp = sub.add_parser("compare", help="Migration-safety diff vs a reference (US3).") - p_cmp.add_argument("generated", nargs="?") - p_cmp.add_argument("reference", nargs="?") + p_cmp.add_argument("generated", type=Path, help="The freshly generated XML to check.") + p_cmp.add_argument( + "reference", type=Path, help="The known-good reference XML to compare against." + ) p_cmp.set_defaults(func=cmd_compare) p_bun = sub.add_parser("bundle", help="Distribution bundle: data + schema + models (US4).") diff --git a/src/acoustic_dataset/compare.py b/src/acoustic_dataset/compare.py new file mode 100644 index 0000000..88af67a --- /dev/null +++ b/src/acoustic_dataset/compare.py @@ -0,0 +1,101 @@ +"""Migration-safety comparison: catch *schema-valid-but-different* output (US3, FR-015). + +Schema validity proves a document fits the contract; it does **not** prove the document says +the same thing as a trusted prior-process file (e.g. a consumer's trial output). This module +canonicalises both sides — sorting attributes, normalising namespace prefixes, and stripping +cosmetic whitespace — so that only *meaningful* differences survive, then reports a +human-readable unified diff. + +Why two canonicalisations (contracts/cli-commands.md §compare): + +* The **equality key** uses C14N with ``rewrite_prefixes=True`` so a file that uses a namespace + prefix compares equal to one using the default namespace. lxml emits an invalid empty-prefix + declaration for unprefixed attributes under that flag, so the key is only ever *string* + compared — never re-parsed. +* The **diff rendering** uses plain C14N (valid, re-parseable XML with attributes already + sorted and cosmetic whitespace removed), pretty-printed so the diff is line-oriented and + readable rather than one long line. + +Both are pure functions over XML text, so the CLI and tests share them. +""" + +from __future__ import annotations + +import difflib +from dataclasses import dataclass +from pathlib import Path + +from lxml import etree + + +@dataclass +class CompareResult: + """Outcome of a migration-safety comparison.""" + + equal: bool + diff: str = "" # empty when equal; a unified diff (reference -> generated) otherwise + + @property + def ok(self) -> bool: + return self.equal + + +def _read(xml: str | Path) -> str: + if isinstance(xml, Path): + return xml.read_text(encoding="utf-8") + return xml + + +def _canonical(xml_text: str) -> str: + """Prefix-, attribute- and whitespace-independent canonical form (the equality key). + + Comments are dropped: they are documentation, not data, so a banner on a reference file + must not register as a migration difference. + """ + return etree.canonicalize( + xml_text, strip_text=True, rewrite_prefixes=True, with_comments=False + ) + + +def _pretty_lines(xml_text: str) -> list[str]: + """Attribute-sorted, whitespace-normalised, indented lines for a readable unified diff. + + Namespace prefixes are stripped for display so a prefix-only difference (e.g. a ``ds:`` + reference vs default-namespace output) does not swamp the diff — without it, every line + would read as changed. The equality verdict is decided separately and stays fully + namespace-aware (:func:`_canonical`); this rendering is only a human aid. + """ + canon = etree.canonicalize(xml_text, strip_text=True, with_comments=False) + tree = etree.fromstring(canon.encode("utf-8")) + for el in tree.iter(): + if isinstance(el.tag, str) and "}" in el.tag: + el.tag = el.tag.split("}", 1)[1] + etree.cleanup_namespaces(tree) + return etree.tostring(tree, pretty_print=True, encoding="unicode").splitlines() + + +def _label(xml: str | Path, fallback: str) -> str: + return xml.name if isinstance(xml, Path) else fallback + + +def compare(generated: str | Path, reference: str | Path) -> CompareResult: + """Compare two XML documents, ignoring cosmetic differences only. + + Returns ``equal=True`` when the documents are canonically identical (a clean match); + otherwise ``equal=False`` with a unified diff of the normalised forms, oriented as + reference -> generated so additions are what the new output introduced. + """ + gen_text = _read(generated) + ref_text = _read(reference) + if _canonical(gen_text) == _canonical(ref_text): + return CompareResult(equal=True) + diff = "\n".join( + difflib.unified_diff( + _pretty_lines(ref_text), + _pretty_lines(gen_text), + fromfile=f"reference:{_label(reference, 'reference')}", + tofile=f"generated:{_label(generated, 'generated')}", + lineterm="", + ) + ) + return CompareResult(equal=False, diff=diff) diff --git a/tests/conftest.py b/tests/conftest.py index 436a227..3cabd84 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,3 +27,8 @@ def input_path() -> Path: @pytest.fixture(scope="session") def golden_path() -> Path: return _REPO_ROOT / "tests" / "golden" / "acoustic_dataset.xml" + + +@pytest.fixture(scope="session") +def reference_path() -> Path: + return _REPO_ROOT / "examples" / "reference" / "trial_known_good.xml" diff --git a/tests/integration/test_compare.py b/tests/integration/test_compare.py new file mode 100644 index 0000000..4ad9d37 --- /dev/null +++ b/tests/integration/test_compare.py @@ -0,0 +1,106 @@ +"""Migration-safety comparison tests (T023, US3 / FR-015, SC-005). + +These encode the contract that a *meaningful* difference is surfaced while cosmetic +differences (attribute order, whitespace, namespace prefix, comments) are ignored — so the +comparison catches the dangerous "schema-valid but different" case without crying wolf. +""" + +from __future__ import annotations + +from acoustic_dataset import compare, validate +from acoustic_dataset.cli import main + +# --- cosmetic differences MUST be ignored (no false positives) --------------------------- + + +def test_identical_documents_match(golden_path): + result = compare.compare(golden_path, golden_path) + assert result.equal + assert result.diff == "" + + +def test_whitespace_and_indentation_are_cosmetic(golden_path): + text = golden_path.read_text(encoding="utf-8") + squashed = "".join(line.strip() for line in text.splitlines()) # strip all indentation + assert squashed != text + assert compare.compare(squashed, text).equal + + +def test_namespace_prefix_is_cosmetic(golden_path, reference_path): + # The shipped reference uses a ``ds:`` prefix; the golden uses the default namespace. + assert "xmlns:ds" in reference_path.read_text(encoding="utf-8") + assert compare.compare(golden_path, reference_path).equal + + +def test_attribute_order_is_cosmetic(): + a = '' + b = '' + assert compare.compare(a, b).equal + + +def test_comment_banner_is_cosmetic(golden_path): + text = golden_path.read_text(encoding="utf-8") + with_banner = text.replace("\n schema-valid... + different = text.replace("134.000", "144.000", 1) + assert different != text + assert validate.schema_errors(different, schema_path) == [] # ...yet schema-valid + + result = compare.compare(text, different) + assert not result.equal # ...and the difference is caught, not silently passed + assert "134.000" in result.diff + assert "144.000" in result.diff + + +def test_diff_is_oriented_reference_to_generated(golden_path): + text = golden_path.read_text(encoding="utf-8") + generated = text.replace("134.000", "144.000", 1) + diff = compare.compare(generated, text).diff + # reference (134.000) removed, generated (144.000) added + assert "- 134.000" in diff + assert "+ 144.000" in diff + + +# --- CLI exit-code contract (contracts/cli-commands.md §compare) -------------------------- + + +def test_cli_clean_match_exits_zero(golden_path, reference_path, capsys): + rc = main(["compare", str(golden_path), str(reference_path)]) + assert rc == 0 + assert "match" in capsys.readouterr().out + + +def test_cli_meaningful_difference_exits_nonzero(tmp_path, golden_path, capsys): + different = tmp_path / "generated.xml" + different.write_text( + golden_path.read_text(encoding="utf-8").replace( + "134.000", "144.000", 1 + ), + encoding="utf-8", + ) + rc = main(["compare", str(different), str(golden_path)]) + assert rc == 1 + err = capsys.readouterr().err + assert "different" in err + assert "144.000" in err # the diff is printed for the human + + +def test_cli_missing_file_exits_nonzero(tmp_path, golden_path, capsys): + rc = main(["compare", str(tmp_path / "nope.xml"), str(golden_path)]) + assert rc == 1 + assert "not found" in capsys.readouterr().err