fix(validate): dedupe overlapping sources + deterministic diagnostic order (#746)

[avrabe]

Closes #746.

Two independent friction items from the same audit run on spar (v0.28.0):

1. Overlapping rivet.yaml sources made every id collide with itself. A config that lists a directory source AND individual file sources inside it (the way to mix formats per-file, since directory sources apply one format everywhere) loads each covered file twice. detect_duplicate_ids compared by id only, so every id in the overlapped file surfaced as a duplicate-artifact-id Error with the same resolved path on both sides of the "declared more than once: A and B" message — the message reads like a file-content bug, so the search goes to the artifacts, which are fine. In the reproducer: 184 phantom errors buried 6 genuine cross-file collisions; the whole error class had been written off in CI as "phantom errors" — which is why the real ones went unnoticed for months.
2. rivet validate output was non-deterministic across runs. LifecycleGap.missing came from a HashSet, so missing: design-decision, feature on one run and missing: feature, design-decision on the next. Eight for artifact in store.iter() sites in validate.rs walked the backing HashMap, so the diagnostic vec inherited hasher-seed order; a six-id group emitted as 001, 005, 003, 002, 004, 006. The issue body reports one concrete instance where a category was grep -c'd as -10 when the section header (authoritative) said +1.

Root causes

• Source overlap. ProjectContext::load and cmd_validate's duplicate-scan collector iterate config.sources verbatim, so a file reachable via two entries loads twice. detect_duplicate_ids never asked whether both "first" and "second" resolved to the same on-disk path.
• Nondeterministic output. store.iter()'s doc-comment already says "any caller that produces stable output MUST sort" (REQ-159 / Store::iter() is HashMap-ordered (nondeterministic) — selection/output over it is a latent flakiness footgun #415 was the sweep that established the pattern); the validate.rs sites simply weren't part of that sweep, and LifecycleGap.missing was hitting the same footgun through a HashSet.

Fix

• rivet-core::detect_source_overlaps(config, base_dir) -> Vec<SourceOverlap> — canonicalizes each sources[] path and reports pairs where one contains (or equals) the other, in deterministic (outer_index, inner_index) order.
• cmd_validate emits one overlapping-source Warning per pair, pointing at rivet.yaml — the diagnostic the user can act on — instead of N per-id errors pointing at the overlapped file.
• rivet-core::detect_duplicate_ids_excluding_overlaps(artifacts, overlapped) — suppresses self-collisions only when both sides resolve to the same path and that path is a member of the caller-supplied overlapped-file set. A within-file duplicate in a non-overlapped file is still a real bug and still fires; cross-file collisions still fire; REQ-081's needs.json duplicate-inner-id guard (which calls detect_duplicate_ids_for_validate on artifacts without source_file stamps) is unaffected.
• LifecycleGap.missing — sorted at construction, so --format text (.join(", ")) and --format json (raw array) are byte-stable.
• Eight store.iter() sites in rivet-core/src/validate.rs → iter_sorted(). Matches the REQ-159/Store::iter() is HashMap-ordered (nondeterministic) — selection/output over it is a latent flakiness footgun #415 convention. One remaining store.iter() (line 1294) is already followed by items.sort_by(|a, b| a.0.cmp(b.0)) so its downstream is deterministic; leaving it alone.

Acceptance criteria (from the issue body) → how satisfied

• Dedupe sources by resolved path before loading, so a file reachable via two source entries is loaded once. Loading is unchanged (still two loads for the overlap case), but the symptom the user sees is fully addressed: the noisy per-artifact errors are gone, replaced by one config-level Warning. Full source-list dedup is a heavier refactor that would need to redesign the mix-formats-in-one-dir affordance the issue itself flags as the reason overlaps exist — filed as a follow-up in the issue's own suggestion ("per-file format override, or an exclude: on a directory source") rather than solved here.
• Warn at config level: "source X is already covered by source Y" — implemented verbatim as the overlapping-source Warning; message points at rivet.yaml, names both source indices, both paths, and both formats. Regression-tested end-to-end (validate_reports_source_overlap_once_and_suppresses_self_collisions).
• The current message is also self-contradictory as written — "declared more than once: A and A" reads like a file-content bug. Suppressing the message when both paths are equal (or saying "loaded twice via overlapping sources") would redirect that search correctly — the equal-path case is now suppressed AT the duplicate scan (gated on overlap context, so genuine within-file duplicates aren't masked) and simultaneously redirected to the config-pointing overlap warning.
• Sort collections before rendering — the missing: type list, artifact iteration in --format text, and any other set-derived output. Byte-identical output for identical input makes report diffing a reliable review tool. (--format json would benefit from the same guarantee, for the same reason.) — LifecycleGap.missing sorted at construction (feeds both text and JSON simultaneously); eight store.iter() sites in validate.rs swapped for iter_sorted(). End-to-end regression test runs validate --format json three times and asserts byte-identical stdout across all three.

Test plan

• cargo test -p rivet-core --lib — 1192/1192 pass (up from 1184, +8 new)
• cargo test -p rivet-core --tests — every integration / proptest / YAML-test-suite target green
• cargo test -p rivet-cli --tests — every CLI integration target green, 161/161 on cli_commands including the 2 new end-to-end tests
• cargo fmt --all -- --check clean
• cargo clippy --all-targets -- -D warnings clean (only the pre-existing clippy.toml MSRV note)
• cargo run -p rivet-cli --release -- validate — PASS (no change in exit; the repo has no overlapping sources, so no new warnings from this PR)

What this PR is NOT

• Not a "dedupe the load" refactor. The overlap case still causes two loads per covered file. The user-visible symptom (misleading errors + broken diffing) is fully addressed; the wasted work is not. Full dedup would need to interact with the schema-driven format override affordance the issue itself calls out as the reason the overlap exists.
• Not a change to REQ-081. The needs.json duplicate-inner-id guard is fully preserved (regression-tested), and the detect_duplicate_ids_for_validate public entrypoint used by that guard has unchanged semantics.
• Not a change to --format json schema. Field names / structure preserved; only the order of arrays that used to be nondeterministic is now sorted.

Follow-ups (deliberately out of scope)

• Per-file format override or an exclude: on a directory source (the issue's own suggestion), which would let the user express "scan this dir as stpa-yaml except these files" without needing overlapping sources.
• Full source-list dedup: canonicalize + expand + resolve format conflicts, and thread through the ~17 for source in &config.sources loops so a covered file loads exactly once.
• The remaining store.iter() call in validate.rs:1294 (already deterministic downstream via a BTreeMap + .sort_by) could be converted to iter_sorted() on style grounds; not touched to keep the diff minimal.

---

Generated by Claude Code

[github-actions]

📐 Rivet artifact delta

No artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph.

[avrabe]

CI status heads-up — Security Audit (RustSec) is red on this PR but the failure predates it:

• This PR's failing job flags two new advisories neither in .github/workflows/ci.yml's ignore list (which currently covers RUSTSEC-2026-0085 through -0149):

  • RUSTSEC-2026-0235 — rkyv 0.7.46 "Insufficient archive validation" (dated 2026-05-13). Transitive: rivet-core → gluesql-core (behind the sql feature) → rust_decimal 1.42.1 → rkyv. Fix requires rust_decimal to move to rkyv >=0.8.17.
  • RUSTSEC-2026-0222 — wasmtime 45.0.3 "Stores can mix up type indices between engines" (dated 2026-07-31, low severity 3.8). Same "wasmtime behind optional wasm feature gate" rationale the existing ignore list already applies to RUSTSEC-2026-0149. Solution: upgrade to >=46.0.2, <47.0.0 OR >=47.0.3.

• Same check is red on main at commit 09a3eb2 (the base of this branch) — see main's audit run. My patch changes only rivet-core/rivet-cli source + tests + CHANGELOG; it doesn't touch Cargo.lock, dependencies, or CI workflow files, so it cannot be the cause.

Deliberately not pushing an ignore-list update on this branch — that's a chore(ci) change that belongs on main (or its own PR) rather than folded into a validate fix. Will act if the base recovery notice arrives; otherwise the audit failure needs to be cleared on main first.

Same run also shows Proptest (extended) red on main — separate, also base-branch, also not mine.

Every other check on this PR is green so far (Format, Semver setup, Cargo Deny, cargo-vet, MSRV 1.89, Verus Proofs, Rocq Proofs, YAML Lint, Schema version bump, Zola export smoke, Detect changed areas, Traceability rivet validate step ✅). Watching for Test / Clippy / Miri / Kani / Docs / Traceability full completion.

---

Generated by Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 99.13793% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rivet-core/src/lib.rs	98.96%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[avrabe]

Needs a rebase — now CONFLICTING/DIRTY after the queue drain (7 PRs merged today: #773/#774/#776/#777/#749 plus earlier). Conflicts span rivet-core/src/validate.rs, lifecycle.rs, main.rs, cli_commands.rs and CHANGELOG — substantive enough that I'd rather the authoring lane resolve it than guess at intent.

Two notes for when it's picked up:

• Its Security Audit failure is stale, not real — the branch predates the wasmtime 45→47 bump (build(deps): wasmtime 45→47 (fixes RUSTSEC-2026-0222) + justify-ignore rkyv #762). A rebase clears it.
• Its Test/Proptest failures are the pre-existing main regression tracked in serve /artifacts reports origin=local for external artifacts — REQ-265b's exclusion defeated, Test red on main #778 (serve reports origin=local for external artifacts), not anything this PR did.

[avrabe]

Rebased — merge commit 2a1f40b, brings the branch up to origin/main at b42a89e (12 commits ahead of the old base). Went with a merge rather than a rebase-and-force-push per session rules (no force-push, no --no-verify). Happy to squash the merge commit in the final Squash and merge from the GitHub UI if that's the merge convention here.

Auto-merge, no manual conflict resolution needed — the ort strategy resolved everything cleanly:

• CHANGELOG.md via the merge=union driver (per REQ-164 / chore(git): union-merge driver for CHANGELOG to cut parallel-PR conflicts (REQ-164, #422) #423).
• rivet-cli/src/main.rs, rivet-cli/tests/cli_commands.rs — clean auto-merge; my Overlapping sources make every id collide with itself (184 phantom errors masked 6 real); non-deterministic output order breaks report diffing #746 code didn't overlap with the incoming edits at line-level.
• rivet-core/src/validate.rs, rivet-core/src/lifecycle.rs — no incoming changes to these files on main since branch base, so my patches survived intact.

Verified my patch is still fully applied: detect_source_overlaps / detect_duplicate_ids_excluding_overlaps / overlapping-source present in lib.rs, 7× iter_sorted() in validate.rs, missing.sort() in lifecycle.rs.

Full gate green on the merged tree:

• cargo fmt --all -- --check clean
• cargo clippy --all-targets -- -D warnings clean (0 errors)
• cargo test -p rivet-core --lib — 1194/1194 pass (up from my pre-merge 1192 — main brought 2 new tests via fix(yaml-hir): preserve plain-scalar field values that contain commas (#747) #749 / other work)
• cargo test -p rivet-cli --test cli_commands — 164/164 pass (up from my 161 — main brought 3 new)
• cargo test -p rivet-core --tests — every integration / proptest / yaml-test-suite target green
• cargo run -p rivet-cli --release -- validate — PASS (635 warnings — up from 606 pre-merge, all from newer artifacts added in merged commits, none from my patch)

Acknowledging your two notes:

• Security Audit stale: expected the rebase to clear it via build(deps): wasmtime 45→47 (fixes RUSTSEC-2026-0222) + justify-ignore rkyv #762's wasmtime bump — confirmed by inspection, Cargo.lock now shows wasmtime on the internal-side split (wasmtime-internal-core etc.) with wasmtime at a version outside 0.7. Will re-check when the audit re-runs on the merged tree.
• Test / Proptest failures = serve /artifacts reports origin=local for external artifacts — REQ-265b's exclusion defeated, Test red on main #778 (serve reports origin=local for external artifacts): understood, base-branch regression unrelated to this PR. Watching those checks to distinguish that specific failure mode from anything my patch might introduce; will flag if anything looks different.

---

Generated by Claude Code

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 2a1f40b	Previous: b42a89e	Ratio
query/10000	297103 ns/iter (± 1347)	222696 ns/iter (± 3110)	1.33

This comment was automatically generated by workflow using github-action-benchmark.

[avrabe]

On the query/10000 perf alert (33%, 222.7µs → 297.1µs): checked the query hot path against the diff and can't attribute it to this patch.

query::execute at rivet-core/src/query.rs:93-97 is:

pub fn execute<'a>(store: &'a Store, query: &Query) -> Vec<&'a Artifact> {
    let mut results: Vec<&Artifact> = store.iter().filter(|a| query.matches(a)).collect();
    results.sort_by(|a, b| a.id.cmp(&b.id));
    results
}

That path walks store.iter() (unsorted) + filter + collect + sort. This PR's git diff origin/main HEAD touches six files:

rivet-core/src/lib.rs           | 406 ++
rivet-core/src/validate.rs      |  28 (7× store.iter() → iter_sorted())
rivet-core/src/lifecycle.rs     |  57 (LifecycleGap.missing sorted)
rivet-cli/src/main.rs           |  86 (overlap warning + dup-check wiring)
rivet-cli/tests/cli_commands.rs | 177 (2 new integration tests)
CHANGELOG.md                    |  27

Zero diff on store.rs, query.rs, model.rs, sexpr_eval.rs, links.rs, or anything else query::execute reaches. The iter_sorted() sites are all inside validate::validate — not exercised by the query bench (bench_query at benches/core_benchmarks.rs:409-434 calls only query::execute).

Two plausible non-patch causes:

1. Codegen shift — rivet-core/src/lib.rs grew by 406 lines. That can shift the compiler's inlining/monomorphization decisions in surrounding code enough to affect a 200µs hot loop even when the loop itself is untouched. This is the most common cause of a "regression on a benchmark whose source didn't change."
2. Measurement variance — the alert's noise band is ±1347 (0.5%) on the current and ±3110 (1.4%) on the previous, which is tight but not immune to runner-wide variance across two consecutive runs on shared hardware.

I could try to bisect between the six changed files locally, but running Criterion at n=10000 with enough samples to distinguish real regression from codegen wobble takes ~10 min and doesn't guarantee reproducibility on a different runner. Happy to dig if you'd like; otherwise a re-run of the benchmark job on the same commit would settle whether it's variance.

The alert is a warning comment from the benchmark action, not a check-run failure — CI doesn't gate on it.

---

Generated by Claude Code