fix(validate): salsa path silently drops adapter + external artifacts#157
Merged
fix(validate): salsa path silently drops adapter + external artifacts#157
Conversation
The default (salsa-incremental) validation path only fed YAML source
files into the salsa database — it silently dropped artifacts produced
by non-YAML adapters (aadl, reqif, needs-json, wasm) and never loaded
external-project artifacts at all. Any link whose target lived in
adapter output (e.g. a YAML artifact with `modeled-by -> AADL-Foo-Bar`
or `analyzes -> AADL-*`) or in an external project (`spar:SPAR-*`) was
flagged as a phantom "link target does not exist" broken-link
diagnostic. The only workaround was `rivet validate --direct`, which
bypasses salsa and loads everything through `ProjectContext::load`.
Fix:
* rivet-core/src/db.rs: add a new `ExtraArtifactSet` salsa input
that carries pre-parsed artifacts contributed by non-YAML adapters,
and `_with_extras` variants of `validate_all`,
`evaluate_conditional_rules`, `build_link_graph`,
`compute_coverage_tracked`, and `filter_artifact_ids` that merge
those artifacts into the store before link-graph construction. The
original (no-extras) entry points keep their exact bodies so
existing callers and memoization behaviour are preserved.
* rivet-cli/src/main.rs (run_salsa_validation): invoke
`load_artifacts` for non-YAML source formats and
`load_all_externals` for externals, feed the results through the
new `db.load_extras(..)` / `db.diagnostics_with_extras(..)` API.
Also removes an unconditional duplicate `collect_yaml_files` call
that was traversing each YAML source twice.
Regression test `salsa_path_matches_direct_on_adapter_only_targets`
pins three things at the core level:
1. Direct path resolves the modeled-by link and reports zero
broken-link diagnostics.
2. Salsa path without extras reproduces the original bug and
reports one phantom broken-link (guards the no-extras API from
silently drifting).
3. Salsa path with extras matches the direct path exactly.
On the rivet dogfood project, `rivet validate` and
`rivet validate --direct` now produce identical diagnostic counts
(6 errors, 9 warnings, 0 broken cross-refs) where the default path
previously differed.
Fixes: REQ-029, REQ-004
Verifies: REQ-004
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 21, 2026
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ 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: 5835c39 | Previous: aa706fc | Ratio |
|---|---|---|---|
store_insert/10000 |
16729930 ns/iter (± 1630152) |
13444099 ns/iter (± 1792686) |
1.24 |
link_graph_build/10000 |
36093256 ns/iter (± 3460895) |
27530692 ns/iter (± 3638629) |
1.31 |
validate/10000 |
14661858 ns/iter (± 1756518) |
10896266 ns/iter (± 1631891) |
1.35 |
traceability_matrix/1000 |
61741 ns/iter (± 495) |
45738 ns/iter (± 588) |
1.35 |
diff/10000 |
9931691 ns/iter (± 1008286) |
8034836 ns/iter (± 485698) |
1.24 |
query/10000 |
117835 ns/iter (± 728) |
90574 ns/iter (± 529) |
1.30 |
This comment was automatically generated by workflow using github-action-benchmark.
thin-vec 0.2.14 has a Double-Free / UAF in IntoIter::drop and ThinVec::clear. Pulled in transitively via salsa 0.26.0. Rivet does not directly construct or iterate thin_vec::ThinVec — the exposure is through salsa's internal data structures. Ignore in both cargo-deny and cargo-audit until either salsa bumps its thin-vec dependency or thin-vec 0.2.15 lands upstream. Trace: skip
serve_integration::start_server() previously returned as soon as TCP
accept succeeded. Under PROPTEST_CASES=1000 CI load, the socket binds
before the artifact store finishes loading, so fetch() races and hits
the server mid-load — the HTTP connection gets closed or returns an
empty response, parsed as status=0.
Seen intermittently as:
api_artifacts_unfiltered ... FAILED
assertion `left == right` failed: left=0 right=200
api_artifacts_search ... FAILED (same shape)
Fix: poll /api/v1/health until it returns HTTP/1.1 200 before declaring
the server ready. The health handler only becomes reachable after
routing is fully initialized.
Trace: skip
2 tasks
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HIGH severity: default `rivet validate` lies when adapters or externals are involved
Symptom
Users had to pass `--direct` on every invocation to get correct validation on projects with AADL or external-repo artifacts. The default (salsa-incremental) mode reported a clean PASS on inputs where `--direct` correctly reported broken-link errors. Users were being trained to not trust the default.
Root cause (worse than hypothesized)
Not a race, not stale memoization. `rivet-cli/src/main.rs:3348-3410 (run_salsa_validation)` had a `match` arm on `source.format` that:
So entire classes of artifacts were absent from the salsa store. Every link pointing at `AADL-`, `SPAR-`, etc. became a phantom `broken-link` diagnostic.
Externals had the same gap — `load_all_externals` was only invoked in `--direct` mode.
Also found and fixed: a duplicate `collect_yaml_files` call at main.rs:3377 (dead code from a refactor).
Fix
New salsa plumbing in `rivet-core/src/db.rs` (+388 LoC):
CLI dispatch in `rivet-cli/src/main.rs::run_salsa_validation`:
Evidence
On rivet's own dogfood project:
Identical counts in both paths.
Test plan
Merge order
This PR builds on main but its `main.rs` hunks interact with PR #157 (mcp-set-fields). Merge this first, then #157 can rebase with a trivial text conflict.
🤖 Generated with Claude Code