Skip to content

fix(validate)!: one stable envelope, records separated from schema health - #124

Merged
pablontiv merged 3 commits into
masterfrom
pablontiv/w5-issue68-json-envelope
Aug 6, 2026
Merged

fix(validate)!: one stable envelope, records separated from schema health#124
pablontiv merged 3 commits into
masterfrom
pablontiv/w5-issue68-json-envelope

Conversation

@pablontiv

Copy link
Copy Markdown
Owner

Related issue

Closes #68 — the validate surface of it. Defects 6 and 8–15 live in fix, new,
schema apply and query, which this branch does not touch.

What was wrong

# Defect Evidence before
1 .stem health and directory verdicts folded into results and summary.total 3-document corpus reported total: 5, paths ["sub/.stem","sub/.stem","a.md","b.md","c.md"], while query --count said 3
1b Those pseudo-records survived --where --where "estado == 'Pending'" still returned both sub/.stem entries
2 Empty corpus reported green total: 1, valid: 1 — the stem-files-exist entry — exit 0
3 yaml-valid and stem-files-exist unreachable scan failed first, Phase-1 diagnostics discarded, no JSON at all and exit 1
Envelope shape varied by invocation bare rootline/validate for one file, -batch for several, zero bytes for --staged with an empty index
5 nested-root-marker authored info, shipped warn failed --strict with no way to suppress a supported configuration
4 monotonic-violations mislabelled 4 of 5 categories rendered (type change: ...); min_children/max_children both rendered as field structural

The envelope

Every invocation — one file, several files, --all, --staged on an empty index, and the
scan-failure path — emits rootline/validate-batch version 2:

{
  "version": 2,
  "kind": "rootline/validate-batch",
  "results":        [ /* documents */ ],
  "structural":     [ /* directories, trailing-slash paths */ ],
  "stem_health":    [ { "path": "sub/.stem", "check": "scope-match", "field": "",
                        "severity": "warn", "message": "..." } ],
  "drift_warnings": [ ],
  "notices":        [ { "severity": "warn", "code": "no_records", "message": "..." } ],
  "summary": { "total": 3, "valid": 3, "invalid": 0,
               "errors_count": 0, "warnings_count": 0, "drift_warnings_count": 0,
               "structural_errors_count": 0, "structural_warnings_count": 0,
               "stem_health_errors_count": 0, "stem_health_warnings_count": 2,
               "stem_health_info_count": 0 }
}

Three properties are the contract:

  1. All six keys always present, empty collections as []. Nothing appears or vanishes
    with a flag.
  2. Disjoint populations, each counted on its own axis. summary.total is a record
    count and agrees with query --count, tree --field root.total and stats --field total.
  3. Splitting changed where a verdict is reported, never whether it counts. Exit is
    non-zero on an invalid record, a structural error, a stem-health error, or an error
    notice; --strict adds warnings on all four axes; info never fails.

notices[] is the extension point: run-level diagnostics keyed by a stable code
(scan_failed, schema_resolution_failed, stem_health_unavailable, no_records).

What #63 still has to do — and does NOT have to reshape

#63 (--output never validated; unknown-field warning never fires) can land entirely on
top of this shape:

  • --output validation is a rejected invocation, not a corpus finding. Validate the
    flag value before any work and return a plain CLI error (no envelope, exit non-zero) —
    the same convention --outbound-type requires --has-outbound already follows. No
    envelope change.
  • Unknown-field warning (a --where naming a field no record carries) is run-level, so
    it becomes one more notices[] entry — suggested code: "unknown_field", severity
    warn, message carrying the fuzzy "did you mean?" hint from
    internal/query.CheckFieldNames. Add a code, not a key. Under --strict it fails,
    consistent with every other warn-level notice.
  • rules.ValidationEnvelopeInput is a struct precisely so a further collection can join
    without touching any call site.

The only thing #63 must not do is reintroduce a key that is conditionally absent.

Verification

  • just test (go test ./... -race) — all packages pass
  • just check (gofmt + golangci-lint + build) — 0 issues
  • just coverage-checkTOTAL 89.5%, every package above the 85% floor
    (cmd/rootline 86.2%, internal/rules 90.4%)

Strict TDD: every defect got a failing test first. New files
internal/rules/envelope_test.go, internal/rules/stemhealth_monotonic_message_test.go,
cmd/rootline/validate_envelope_test.go. Existing tests that encoded the old behaviour
(TestValidateStagedNoFiles asserting empty output, TestValidateAll_StemHealthChecks
asserting health inside results) were rewritten to assert the fixed contract.

Reproduced each defect against a built binary before and after — including the two health
checks that previously emitted no JSON.

Breaking change

Title carries ! deliberately: the release workflow reads the squashed PR title, and an
unmarked breaking change would ship as a patch. Upgrade table is in docs/validate.md;
CHANGELOG.md, CLAUDE.md and .claude/skills/rootline/ref-validate.md are updated.

Note for the maintainer

The structural[] split was not in the issue text. It surfaced while verifying the claim
that summary.total now equals the record count: with structural: rules declared,
directory pseudo-records ("/", "sub/") inflated total the same way .stem entries
did. Left in results[] it would have falsified the contract this PR documents, so it is
fixed here rather than deferred. It carries its own regression test asserting a structural
violation still exits 1.

…alth

`validate --all` folded `.stem` health diagnostics and directory structural
verdicts into the same `results` slice and `summary.total` as documents, so
`total` was not a record count: a three-document corpus with two health findings
reported 5 while `query --count`, `tree --field root.total` and `stats --field
total` all reported 3 on the same path. Those pseudo-records also survived a
`--where` filter they carry no frontmatter to match, and an emptied or renamed
path reported `total: 1, valid: 1` — the `stem-files-exist` entry — which a CI
gate read as green.

Two of the twelve health checks were unreachable through the command. Stem
health ran first, then `index.Scan` failed and every Phase-1 diagnostic was
discarded: `yaml-valid` and `stem-files-exist` are exactly the checks that fire
when the scan cannot succeed, so a consumer asking for JSON got a raw Go error
on stderr and no JSON at all.

The envelope also changed shape by invocation: a bare `rootline/validate` object
for one file, `rootline/validate-batch` for several, and zero bytes for
`--staged` with an empty index — breaking the pre-commit hook idiom
`rootline validate --staged | jq -e '.summary.invalid == 0'`.

Now every invocation emits `rootline/validate-batch` version 2 with six
always-present keys: `results` (documents), `structural` (directories),
`stem_health` (`.stem` files, with `error`/`warn`/`info` severity), `notices`
(run-level, keyed by a stable `code`), `drift_warnings`, and `summary`. The
populations are disjoint and counted on their own axes; splitting them changed
where a verdict is reported, never whether it counts, so an error on any axis
still exits 1.

Also fixed, in the same surface:

- `nested-root-marker` is delivered at `info` as authored. The severity mapper
  handled only `pass` and `fail`, promoting it to a warning that failed
  `--strict` with no way to suppress a supported configuration.
- `monotonic-violations` names the category it detected. Type widening, required
  loosening, severity loosening and structural loosening all rendered as
  `(type change: ...)`, and structural paths were truncated to the field
  `structural`, making `min_children` and `max_children` indistinguishable.

BREAKING CHANGE: `validate` emits `rootline/validate-batch` version 2 for every
invocation. Read a single-file verdict as `.results[0]`; `--field valid` becomes
`--field "results[].valid"`. `.stem` findings move from `results[]` to
`stem_health[]`, directory verdicts to `structural[]`, and `summary.total` is now
a record count. See the upgrade table in docs/validate.md.

Closes #68
Resolves the CHANGELOG.md conflict in the Unreleased/Changed section.
Both sides were additive: the validate v2 envelope entries from this
branch and the stacked-PR CI trigger entry from master are kept.

All other paths auto-merged: master's atomic-write, analyze-determinism
and graph-cycle work does not touch the validate envelope surface.
`getStagedFiles` runs `git diff --cached` with the process environment,
which is correct: a pre-commit hook wants the index git handed it. But
git exports GIT_DIR and GIT_INDEX_FILE into every hook it runs, so the
suite executed from `.githooks/pre-push` read the outer repository's
index instead of the fixture's and reported zero staged files.
TestGetStagedFiltersMarkdown and TestValidateStaged failed on push while
passing under a plain `just test` — reproducible on master as well.

Issue #121 cleared the environment for the fixture's own git writes; the
production reader was still inheriting it. `makeStagedRepo` now unsets
the repo-scoping variables for the duration of the test and restores
them afterwards, and gitenv exports the denylist as `ScopingVars` so the
cleared-subprocess path and the cleared-process path cannot drift apart.
@pablontiv
pablontiv merged commit a66d056 into master Aug 6, 2026
11 checks passed
@pablontiv
pablontiv deleted the pablontiv/w5-issue68-json-envelope branch August 6, 2026 12:40
pablontiv added a commit that referenced this pull request Aug 6, 2026
Master carries #124's validate envelope after its squash-merge, so this
branch's copies of those hunks are already upstream and merged clean; the
net diff against master is now this PR's own change only.

Resolved on the merits:
- CHANGELOG.md, CLAUDE.md, docs/graph.md — additive on both sides; both
  entries kept. Adds the BREAKING CHANGELOG entry this branch never had,
  since its previous CHANGELOG hunks all belonged to #124.
- cmd/rootline/staged_test.go — kept master's
  TestGetStagedFilesIgnoresAmbientGitScope alongside this branch's
  envelope assertions in TestValidateStagedNoFiles.
pablontiv added a commit that referenced this pull request Aug 6, 2026
Reconcile this branch with master after its former parent (#127, the
--output format contract) was squash-merged, alongside the validate
envelope (#124) and the repair-surface contract (#133).

All conflicts were additive: CHANGELOG.md, CLAUDE.md, docs/graph.md and
.claude/skills/rootline/ref-query.md keep both sides, and
cmd/rootline/staged_test.go keeps master's new
TestGetStagedFilesIgnoresAmbientGitScope next to the existing empty-index
assertions. The resulting diff against master is exactly this PR's own
change: field-name validation for --where (warning) and --sort (error).
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.

JSON contract hygiene: validate --all conflates .stem health with records, two health checks never reach the report, and envelopes change shape by flag

1 participant