Skip to content

feat: classify structured artifact changes - #91

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

feat: classify structured artifact changes#91
stacknil merged 3 commits into
mainfrom
stacknil/artifact-diff-structured-summary

Conversation

@stacknil

Copy link
Copy Markdown
Owner

Summary

  • add bounded JSON and JSONL structure summaries for changed, missing, and extra artifacts
  • classify structure, schema/version, and run-manifest digest changes separately from raw content changes
  • preserve identity-first comparison so byte-identical JSON/JSONL is never parsed
  • fail closed on malformed structured input, unsafe metadata, invalid digest maps, and exceeded limits

This is layer 2 of the split replacement for #89 and is stacked on #90. The strict machine-readable report/schema remains out of scope for a separate follow-up.

Design decision

The comparator still performs normalized digest/size identity first. Only non-identical or one-sided JSON/JSONL receives a bounded deterministic summary: container, record count, sorted top-level-key union, safe schema markers, and validated run-manifest digest fields.

Main risk

Changed structured artifacts can now fail with exit 2 when JSON/JSONL or provenance fields are malformed instead of being reported as an opaque content difference. Dedicated error-path tests define that fail-closed boundary.

Compatibility impact

Additive enrichment of the stacked comparator only. Existing repository artifacts, schemas, regeneration behavior, and commands are unchanged. Human output may add stable reason labels (structure-changed, schema-version-changed, or run-manifest-digest-changed) after content-changed.

Rollback path

Revert this PR's docs, behavior, and tests commits in reverse order. #90 remains a complete human comparator with content-changed classification only.

Validation

  • .venv\Scripts\python.exe -m pytest -q tests/test_artifact_contract_diff_structured.py tests/test_artifact_contract_diff_core.py tests/test_artifact_regeneration_check.py — 28 passed, 1 skipped
  • .venv\Scripts\python.exe -m pytest -q tests/test_markdown_links.py tests/test_reviewer_docs.py — 22 passed
  • .venv\Scripts\python.exe scripts/check_release_contract.py — 29 strict artifacts matched, 6 visual artifacts regenerated, 8 schema tests passed, full suite 240 passed / 1 skipped
  • python -m py_compile src/telemetry_lab/artifact_contract_diff.py — passed
  • 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 delayed compatibility review. Relative to #90 this PR changes 3 files and adds 234 substantive behavior lines.

@stacknil

Copy link
Copy Markdown
Owner Author

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

  • Design decision: preserve identity-first comparison; summarize only changed, missing, or extra JSON/JSONL with bounded, sorted structural/provenance metadata.
  • Main risk: malformed changed structured artifacts now fail closed with exit 2 instead of appearing as opaque content changes. Dedicated invalid JSON/JSONL, unsafe marker, digest-map, and limit tests cover this boundary.
  • Compatibility impact: additive enrichment on top of feat: add bounded artifact diff core #90. Existing artifacts, schemas, regeneration behavior, and repository commands are unchanged; the human comparator may append stable semantic reason labels.
  • Rollback path: revert 2eb138a, 79adbaa, then 2324009; feat: add bounded artifact diff core #90 remains independently usable with content-only change classification.

Reviewed relative scope is 3 files and 234 substantive behavior additions. All four remote checks are green; no bot or reviewer comments are outstanding. Keeping the PR draft for delayed compatibility review and the final stacked schema projection.

@stacknil
stacknil changed the base branch from stacknil/artifact-diff-core to main August 24, 2026 09:52
@stacknil

Copy link
Copy Markdown
Owner Author

Delayed structured-compatibility review completed on 2026-08-24 against exact head 2eb138a, after retargeting this PR from the merged #90 branch to main. The relative scope remains exactly three files: structured behavior, its tests, and reviewer semantics.

The prototype review findings assigned to this layer are covered: identical JSON/JSONL is not parsed after normalized identity matches; changed/missing/extra structured artifacts are bounded; run-manifest digest scalars/maps and relative keys are validated before projection; unsafe schema markers, malformed JSON/JSONL, oversized summaries, and non-deterministic ordering fail closed or have regressions.

Fresh validation:

  • .\.venv\Scripts\python.exe -m pytest -q tests/test_artifact_contract_diff_structured.py -> 14 passed (independent compatibility matrix).

  • Core + structured + regeneration targeted matrix -> 28 passed, 1 skipped.

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

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

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

  • Design decision: classification is derived only after comparison identity differs; it explains structure/schema/provenance reasons but does not assign compatibility or acceptance policy.

  • Main risk: bounded structured parsing can still use multiple copies of up to 64 MiB input, so this remains a local reviewer tool rather than an arbitrary-scale streaming service.

  • Compatibility impact: additive structured summaries and reason labels; human exit semantics, committed artifacts, runtime dependencies, and regeneration authority are unchanged.

  • Rollback path: revert 2eb138a, 79adbaa, then 2324009; merged feat: add bounded artifact diff core #90 remains independently usable as content-only human triage.

The delayed report-behavior review requirement 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:56
@stacknil
stacknil merged commit f24caa9 into main Aug 24, 2026
4 checks passed
@stacknil
stacknil deleted the stacknil/artifact-diff-structured-summary branch August 24, 2026 09:57
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