fix(rivet): stop double-loading safety/stpa and de-duplicate RENDER-REQ (#360) - #368
fix(rivet): stop double-loading safety/stpa and de-duplicate RENDER-REQ (#360)#368avrabe wants to merge 2 commits into
Conversation
Rivet verification gate✅ 20/20 passed
Filter: Failed artifacts(none) Updated automatically by |
Correction — two of this PR's claims were wrong, found by cold-context adversarial verificationI ran a fresh-context agent against this PR's own claims with no inherited framing. It refuted two, and I re-derived both myself before acting. Pushed 1. "The directory scan alone loses nothing" — FALSE, and it was committed into
|
format: |
rivet get VAL-010 --format json | jq -r '.fields["test-name"]' |
|---|---|
generic-yaml |
tests::boolean_matches_aadl_boolean, tests::boolean_mismatches_aadl_string |
stpa-yaml |
tests::boolean_matches_aadl_boolean |
test-name is evidence — the pointer from a verification artifact to the tests discharging it. The trade is still worth taking (5 truncated pointers against 190 phantom duplicate-id errors), but it is a trade, not a free win, and the committed comment now says so. Filed upstream as pulseengine/rivet#747.
2. "The repo cannot widen allowed-values locally" — FALSE
I claimed the residual 276 errors were unfixable here because common@0.3.0 is embedded in the binary. rivet's own output says otherwise, verbatim:
common 0.3.0 embedded (compiled into rivet — changes on upgrade; vendor under schemas/ to pin)
Measured:
$ cp <rivet>/schemas/common.yaml schemas/common.yaml # version "0.3.0" — identical to embedded
$ rivet validate | tail -2
Result: FAIL (276 errors, 629 warnings, 0 broken cross-refs)
Schemas: common@0.3.0 (on-disk), ... # override live
$ # append passing / not-implemented / planned / partial to status allowed-values
$ rivet validate | tail -2 ; echo exit=$?
Result: PASS (629 warnings)
exit=0
276 → 0. My claim inferred "only sysml2 can be on-disk" from the observation "only sysml2 is on-disk" — an inference reported as a measurement. That's the same one-variable-check failure this repo keeps re-learning.
Consequently these two statements in the PR body are withdrawn: "ci.yml's pin to rivet-cli v0.4.3 is load-bearing for the green check" and "measured as not viable." The pin is load-bearing today, but for a fixable reason.
…and the remedy should still be declined — now on the merits
Now that the option is known to exist, #371 declines it deliberately rather than on a false constraint. Widening the vocabulary on a vendored fork of the org-shared common schema would silence a correct diagnostic: rivet already ships verification-verdict — "Pass/fail verdict for a single verification measure in an execution run" — and spar is recording verdicts in the lifecycle status field of type: feature ("a user-visible capability"). The 276 errors are a real modeling defect, not a vocabulary gap. Softening the ledger to green it is the textbook anti-pattern.
Smaller corrections, for the record
- "rivet kept one copy and dropped the other with no diagnostic" — inaccurate at 0.28.0, which names both files and the winner (
— the second definition silently overwrites the first). True only of the pinned v0.4.3. - "Which one won was load order, not a decision" — load order is a decision: it's the
sources:order inrivet.yaml, andStore::upsertis documented last-write-wins. Fully deterministic. Required rivet gate cannot detect duplicate artifact ids — 6 real ones on main, masked by 184 self-inflicted phantoms #360's "looks hash-ordered" is refuted; I softened but inherited it. - The repo-grep anecdote is overstated.
zoom-lowalso appears intests/playwright/rendering.spec.ts:145-147— an executable assertion, not a fixture — andsemantic zoomappears inartifacts/verification.yaml:474, the same spec, and a docs plan. The rhetorical point stands; the grep offered as evidence for it does not. - Adjacent defect this PR canonises without checking: keeping the STPA copies makes
RENDER-REQ-006 verified-by: [VAL-R5]authoritative, and VAL-R5's evidence isrun: cargo test -p spar-wasm -- semantic_zoom— which matches zero tests repo-wide.playwrightappears in no workflow. The status is correct against etch source; its verification pointer is dead. Tracking separately.
The core of the PR is unaffected — the duplicate-id class still goes 190 → 6 → 0, and that measurement was independently confirmed exact at all four rows.
…EQ (#360) `rivet validate` reported 466 errors on a clean tree. 190 of them were phantom: every id in four STPA files collided with ITSELF, producing "declared more than once" messages with the same path on both sides. Cause: rivet.yaml listed `safety/stpa` as a directory scan AND listed four files inside it individually (to force `format: generic-yaml`). Each file was loaded twice, so every id it declared was declared twice. Listing a directory and its own contents is not additive — it is duplicative, and the diagnostic could not say so because both copies had identical provenance. Removing the four explicit entries is safe, and the artifact set was compared rather than assumed: 883 ids and 1195 links, byte-identical before and after via `rivet list --format json`. The format change loses nothing; stpa-yaml is a superset of what generic-yaml extracted from these files. That left 6 real duplicates: RENDER-REQ-001..006 were defined in BOTH artifacts/requirements.yaml and safety/stpa/rendering-analysis.yaml. rivet kept one copy and dropped the other with no diagnostic at all, and which one won was load order rather than a decision. The stubs are deleted, not the canonical copy, because only the STPA file carries the hazards/satisfies/verified-by links that make these part of the STPA V. The stubs carried none. The stubs were also stale in BOTH directions, which is why this was checked against source instead of picking the newer-looking file: - RENDER-REQ-006 claimed `partial`; it is `implemented`. etch/src/html.rs emits `svg.zoom-low` / `svg.zoom-overview` rules and asserts a `wheel` handler (html.rs:65-68, :131). - RENDER-REQ-003 claimed `implemented`; it is `partial`. `minimap` is still `/// Show minimap (Phase 3b - reserved)`. Verified at the pinned etch rev 4c06709, not by grepping this repo. A repo-scoped grep says "zoom-low appears only in a test fixture", which reads exactly like "not implemented" when the implementation lives in a git dependency. That nearly produced a false finding in the other direction. Also normalises the status vocabulary: `pass` -> `passing` (5) and `pending` -> `planned` (4), collapsing singletons into the dominant spelling. MEASUREMENTS, and one correction to the issue --------------------------------------------- 466 errors -> 282 (drop the source overlap: 190 duplicates -> 6) -> 276 (delete the 6 RENDER-REQ stubs: 6 -> 0) -> 276 (normalise the vocabulary: NO CHANGE) Duplicate-id errors are now 0. But #360 predicted that with the overlap and the vocabulary both fixed "the residual error count should be near zero", and that is FALSE. The normalisation cannot reduce the count, because its targets are not in the allowed set either: allowed: draft, proposed, approved, implemented, verified, released, accepted, deprecated, rejected `passing` and `planned` are both absent, so `pass` -> `passing` moves 5 errors between buckets (172 -> 177) and `pending` -> `planned` moves 4 (36 -> 40). Net zero, as measured. It is still worth doing: it leaves 4 non-conforming values to resolve instead of 6. It is just not an error fix, and recording it as one would have overstated what this change buys. The residual 276 are entirely status-vocabulary skew: 177 `passing`, 51 `not-implemented`, 40 `planned`, 8 `partial`. These are test/verification states, not approval-lifecycle states, and `common@0.3.0` is embedded in the rivet binary -- schemas/ only overrides sysml2 -- so the repo cannot widen them locally. This does NOT unblock lifting the v0.4.3 pin in ci.yml; see #360 for the measurement. The pin is load-bearing for the green check, which is a fact worth stating plainly rather than leaving implicit in a comment. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… test-names The equivalence check this PR cited was measured on `rivet list --format json` WITHOUT `--full`, which emits only the summary view (`--help` states it omits `description`, `tags`, and `fields`). That view is structurally incapable of detecting field-level loss — precisely the class of change a `format:` switch produces. "Byte-identical" was true and meaningless. Re-measured with `--full` on base vs. this branch: +86 artifacts GAIN `mitigates` and `traces-to` -5 artifacts LOSE half of `test-name` The five are VAL-010, VAL-011, VAL-014, VAL-015, VAL-021 — exactly the five whose on-disk `test-name` is a comma-separated pair. The stpa-yaml parser keeps only the first element: on disk: tests::boolean_matches_aadl_boolean, tests::boolean_mismatches_aadl_string rivet: tests::boolean_matches_aadl_boolean A/B on identical bytes with only `format:` differing confirms the parser is the cause: generic-yaml round-trips the full string. The trade is still worth taking — five truncated evidence pointers against 190 phantom duplicate-id errors — but it is a trade, not a free win, and the comment now says so. Filed upstream as pulseengine/rivet#747; revert the trade if that lands a fix. No behaviour change: `rivet validate` is FAIL (276 errors, 629 warnings) before and after this commit. Comment-only. Found by cold-context adversarial verification of this PR's own claims. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cce6fbd to
3c9ea9d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Superseded by #387, merged as This branch was not merged directly — the six guardrail PRs were rebuilt onto one batch branch ( Verified on Closing as superseded, not as unmerged work. Reopen if anything here is missing from |
Warning
Two claims below were refuted by cold-context verification and are withdrawn. See the correction comment and #371.
rivet list --format jsonwithout--full, a view that omitsfieldsentirely. Re-measured: 5 artifacts (VAL-010/011/014/015/021) lose half theirtest-name. Fixed incce6fbd; filed upstream as stpa-yaml parser silently truncates comma-containing scalars at the first element (test-name evidence loss) rivet#747.allowed-valueslocally" and its corollaries "the v0.4.3 pin is load-bearing" / "measured as not viable" — false. Vendoringschemas/common.yaml(v0.3.0, same as embedded) and widening the vocabulary takesrivet validate276 → 0, exit 0. The remedy is still declined, but on the merits (Verification verdicts are recorded as feature.status — the root cause of all 276 rivet validate errors #371), not on a constraint that doesn't exist.The duplicate-id result — 190 → 6 → 0 — is unaffected and was independently confirmed exact at all four measurement rows.
Closes #360.
rivet validatereported 466 errors on a clean tree. This takes it to 276,eliminates the duplicate-id class entirely, and — where the issue's own plan was
wrong — says so with the measurement rather than quietly dropping that step.
The 190 phantom duplicates
rivet.yamllistedsafety/stpaas a directory scan and listed four filesinside it individually, to force
format: generic-yamlon them. Each of thosefiles was therefore loaded twice, and every id in them was "declared more than
once" — colliding with itself, with the same path on both sides of the
message.
Listing a directory and its own contents isn't additive, it's duplicative, and
the diagnostic couldn't hint at that because both copies had identical
provenance.
Equivalence was measured, not assumed. Dropping the explicit entries changes
those files' parse format, so before/after
rivet list --format jsonwascompared: 883 ids and 1195 links, byte-identical.
stpa-yamlis a supersetof what
generic-yamlwas extracting here.The 6 real ones
RENDER-REQ-001..006were defined in bothartifacts/requirements.yamlandsafety/stpa/rendering-analysis.yaml. rivet kept one copy and dropped the otherwith no diagnostic, and which one won was load order, not a decision.
I deleted the
requirements.yamlstubs and kept the STPA copy, because only theSTPA file carries the
hazards/satisfies/verified-bylinks that make thesepart of the STPA V. The stubs carried none.
The stubs were stale in both directions, which is why I checked against
source instead of keeping whichever file looked fresher:
implementedpartialminimapis/// Show minimap (Phase 3b — reserved)partialimplementedetch/src/html.rs:65-68emitssvg.zoom-low/svg.zoom-overview;:131asserts awheelhandlerVerified at the pinned etch rev
4c06709, not by grepping this repo — andthat distinction nearly cost me a false finding. A repo-scoped grep shows
zoom-lowonly in a checked-in playwright fixture andsemantic zoomonly in aREADME bullet, which reads exactly like "asserted against a hand-authored
artifact, never implemented." It reads that way because the implementation lives
in a git dependency. "Not in this repo" and "not implemented" are
indistinguishable to grep.
Where the issue was wrong
#360 predicted that with the overlap and the vocabulary both fixed, "the
residual 0.28.0 error count should be near zero." That is false, and this PR
measures it rather than asserting it:
pass/pendingThe normalisation cannot reduce the count, because its targets aren't in the
allowed set either:
Neither
passingnorplannedis there. Sopass→passingmoves 5 errorsbetween buckets (172 → 177) and
pending→plannedmoves 4 (36 → 40). Netzero, exactly as measured.
It's still worth doing — it leaves 4 non-conforming values to resolve instead
of 6 — but it is not an error fix, and booking it as one would have overstated
what this change buys.
Why the v0.4.3 pin still can't be lifted
The residual 276 is entirely status-vocabulary skew: 177
passing, 51not-implemented, 40planned, 8partial. These are test/verificationstates, not approval-lifecycle states — a VAL record's "passing" is about whether
the test passes right now, which is a genuinely different axis from where an
artifact sits in an approval flow.
rivet names the two options itself: fix the artifacts, or add the values to the
statusbase-field'sallowed-values. The repo can't take the second one —common@0.3.0is embedded in the rivet binary, andschemas/only overridessysml2.So
ci.yml's pin torivet-cli v0.4.3is load-bearing for the green check,not incidental. Lifting it turns a required context red with 276 errors that are
real under the current schema. The existing comment there calls a newer rivet's
findings "phantom errors"; for the duplicate-id class that was true and this PR
fixes it, but for the status class it isn't — those are genuine
non-conformances against
common@0.3.0that the pin is holding back.Resolving them is an upstream rivet conversation (does the lifecycle admit
test-execution states?), not something to force in this PR.
What this does not do
safety/stpa/rendering-analysis.yaml. The canonical statusesthere were independently confirmed correct against etch, so there was nothing
to change.
🤖 Generated with Claude Code