Skip to content

feat: add bounded artifact diff core - #90

Merged
stacknil merged 3 commits into
mainfrom
stacknil/artifact-diff-core
Aug 24, 2026
Merged

feat: add bounded artifact diff core#90
stacknil merged 3 commits into
mainfrom
stacknil/artifact-diff-core

Conversation

@stacknil

Copy link
Copy Markdown
Owner

Summary

  • add a bounded, deterministic artifact-tree comparison core
  • add a human-readable CLI with stable relative-path output and exit codes 0/1/2
  • share strict UTF-8 newline normalization with the authoritative regeneration gate
  • keep existing binary artifacts presence-only

This is layer 1 of the split replacement for #89. Structured JSON/JSONL summaries and the strict machine-readable report/schema remain out of scope for this PR and will be proposed as stacked follow-ups.

Design decision

Keep regenerate_artifacts.py --check authoritative. The new comparator only explains mismatches and never accepts generated output. Comparable text uses strict UTF-8 with CRLF and lone CR normalized to LF; binary files present in both trees are presence-only. Traversal, path, file-kind, count, and report invariants fail closed.

Main risk

The shared newline normalizer expands regeneration equivalence from CRLF-only handling to include lone CR. Dedicated parity tests lock this behavior, while the full release gate confirms all committed artifacts still match.

Compatibility impact

Additive CLI/API surface only. Existing commands, artifacts, schemas, and release compatibility labels are unchanged. There is no JSON report contract in this layer.

Rollback path

Revert the three commits in reverse order. scripts/regenerate_artifacts.py then returns to its local normalization helper, and the standalone comparator/doc route disappears without changing committed artifacts.

Validation

  • .venv\Scripts\python.exe -m pytest -q tests/test_artifact_contract_diff_core.py tests/test_artifact_regeneration_check.py — 14 passed, 1 skipped
  • .venv\Scripts\python.exe scripts/check_release_contract.py — 29 strict artifacts matched, 6 visual artifacts regenerated, 8 schema tests passed, full suite 226 passed / 1 skipped
  • independent scripts/regenerate_artifacts.py --check with a unique work directory — 29 strict artifacts matched, 6 visual artifacts regenerated
  • real-tree CLI smoke test — exit 0, 8 comparable files unchanged, 6 binaries presence-only
  • git diff --check — passed
  • privacy scan of added lines — no local usernames, OneDrive/absolute machine paths, credentials, or school identifiers

Review state

Draft for the required compatibility review and to keep the split sequence explicit before the stacked follow-ups land.

@stacknil

Copy link
Copy Markdown
Owner Author

Post-CI diff review completed against f687df6 (remote head matches the reviewed local head).

  • Design decision: keep regeneration authoritative; expose one bounded comparator core plus a human projection. Text equivalence is strict UTF-8 with CRLF/lone-CR normalization, while existing binaries remain presence-only.
  • Main risk: the shared normalizer intentionally broadens the regeneration comparison from CRLF-only to lone-CR equivalence. The parity regression and full release gate cover this boundary.
  • Compatibility impact: additive CLI/API only; no committed artifact, existing schema, existing command, or release-label contract changed. Machine-readable JSON is explicitly absent from this layer.
  • Rollback path: revert f687df6, 1ab8a7e, then 045dad7; this removes the docs, comparator, and tests and restores the prior local regeneration helper.

Reviewed scope remains 7 files with 432 substantive behavior additions (tests/docs excluded from that count). All four remote checks are green; no bot or reviewer comments are outstanding. Keeping the PR draft for the required delayed compatibility review and stacked split sequence.

@stacknil

Copy link
Copy Markdown
Owner Author

Delayed review completed on 2026-08-24 against exact head f687df6.

The earlier prototype review findings that belong to this layer are covered here: normalized identity is streamed before any later structured parsing, symlink/reparse-point roots and entries are rejected, opened handles are revalidated as regular files, special files fail closed, CLI exit 0/1/2 is regression-tested, and regeneration/triage share CRLF + lone-CR normalization.

Fresh validation:

  • .\.venv\Scripts\python.exe -m pytest -q tests/test_artifact_contract_diff_core.py tests/test_artifact_regeneration_check.py -> 14 passed, 1 skipped (POSIX-only FIFO case on Windows).

  • .\.venv\Scripts\python.exe scripts\check_release_contract.py -> 29 strict matches, 6 visual smoke artifacts, 8 schema tests, and 226 passed / 1 skipped overall.

  • git diff --check origin/main...f687df6 and changed-line privacy/secret scan passed.

  • All four remote checks remain green; no external bot/reviewer feedback is outstanding.

  • Design decision: keep the human comparator as a read-only explanation adapter; the existing regeneration command remains acceptance authority.

  • Main risk: concurrent filesystem mutation remains a local-tool TOCTOU boundary, narrowed by no-follow/reparse checks and descriptor revalidation; the tool never accepts or rewrites compared artifacts.

  • Compatibility impact: additive CLI/core plus shared lone-CR normalization; committed artifacts, schemas, runtime dependencies, and release labels are unchanged.

  • Rollback path: revert f687df6, 1ab8a7e, then 045dad7; the original regeneration gate remains intact.

The overnight traversal/security review window is satisfied. Marking this layer ready for the ordered stack merge.

@stacknil
stacknil marked this pull request as ready for review August 24, 2026 09:51
@stacknil
stacknil merged commit 6dccea9 into main Aug 24, 2026
4 checks passed
@stacknil
stacknil deleted the stacknil/artifact-diff-core branch August 24, 2026 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant