Skip to content

fix(validate): salsa path evaluates status-gate validation-rules (REQ-146, #355)#393

Merged
avrabe merged 1 commit into
mainfrom
fix/salsa-validate-status-gates
Jun 2, 2026
Merged

fix(validate): salsa path evaluates status-gate validation-rules (REQ-146, #355)#393
avrabe merged 1 commit into
mainfrom
fix/salsa-validate-status-gates

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented Jun 2, 2026

Addresses #355 Finding 3 ("rivet validate and rivet check gaps-json disagree").

Root cause (verified)

rivet validate runs the incremental salsa path (db::validate_all), which evaluated structural rules (phases 1-7) and conditional rules (phase 8) — but never evaluated the status-gate validation-rules (phase 9: the (implies …) V-model promotion gates). The direct validate::validate* path — used by rivet check gaps-json and rivet validate --directdid. So the default rivet validate silently PASSED a project with a status-gate violation that gaps-json / --direct FAILED. That's both the reported cross-command disagreement and a soundness gap (the default surface under-reports).

Reproduction (local)

A project on common + aspice with an approved sys-verification that verifies a draft system-req (violates V-sys-verification-needs-approved-req):

command before
rivet validate (salsa) FAIL (1 error) — gate missing
rivet validate --direct FAIL (2 errors) — gate present

After the fix both report 2 errors including the gate.

Fix

db::validate_all and db::validate_all_with_extras now call validate::evaluate_validation_rules against the same materialized store/schema/graph they already build, so structural + conditional + status-gate rules are all evaluated on every surface.

Verification

  • rivet's own corpus: rivet validate and --direct both PASS, unchanged.
  • New regression test db::tests::validation_rules_evaluated_in_validate_all (salsa path surfaces the gate).
  • rivet-core db (24) + validation_rule (3) tests pass; clippy --all-targets + fmt clean; rivet validate + rivet docs check PASS.

Note: this is the engine-parity half of #355. Findings 1-2 (no canonical status enum; the gate hardcoding literal approved with no ordered lifecycle) remain a maintainer design decision and are untouched here.

Fixes: REQ-146

🤖 Generated with Claude Code

…-146, #355)

`rivet validate` runs the incremental salsa path (`db::validate_all`),
which evaluated structural rules (phases 1-7) and conditional rules
(phase 8) but NOT the status-gate `validation-rules` (phase 9 — the
`(implies …)` V-model promotion gates). The direct `validate::validate*`
path (used by `rivet check gaps-json` and `rivet validate --direct`) DID
evaluate them. So the default `rivet validate` silently PASSED a project
with a status-gate violation that gaps-json / --direct correctly FAILED
— the divergence reported in #355 Finding 3, and a soundness gap since
the default validation surface under-reported.

Fix: `db::validate_all` and `validate_all_with_extras` now also call
`validate::evaluate_validation_rules` against the same materialized
store/schema/graph, so every validation surface agrees.

Reproduced + verified: a project with an aspice `validation-rules` gate
(an approved sys-verification verifying a draft system-req) previously
gave salsa `FAIL (1 error)` vs direct `FAIL (2 errors)`; now both report
2 including the gate. rivet's own corpus still PASS (181→ unchanged) on
both paths. New regression test
`db::tests::validation_rules_evaluated_in_validate_all`; rivet-core db
(24) + validation_rule (3) tests pass; clippy --all-targets + fmt clean;
rivet validate + docs check PASS.

Fixes: REQ-146
Refs: REQ-029, REQ-004

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

📐 Rivet artifact delta

Change Count
Added 1
Removed 0
Modified 0
Downstream impacted (depth ≤ 5) 0

Graph

graph LR
  REQ_146["REQ-146"]:::added
  classDef added fill:#d4edda,stroke:#28a745,color:#155724
  classDef removed fill:#f8d7da,stroke:#dc3545,color:#721c24
  classDef modified fill:#fff3cd,stroke:#ffc107,color:#856404
  classDef overflow fill:#e2e3e5,stroke:#6c757d,color:#495057,stroke-dasharray: 3 3
Loading
Added
  • REQ-146

📎 Full HTML dashboard attached as workflow artifact rivet-delta-pr-393download from the workflow run.

Posted by rivet-delta workflow. The graph shows only changed artifacts; open the HTML dashboard (above) for full context.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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: 7849e7c Previous: a37bda6 Ratio
store_insert/10000 15794735 ns/iter (± 1499225) 12398686 ns/iter (± 372468) 1.27
link_graph_build/10000 35848484 ns/iter (± 4597669) 24086899 ns/iter (± 1136601) 1.49
validate/10000 16103470 ns/iter (± 2506107) 12815374 ns/iter (± 582007) 1.26

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

@avrabe avrabe merged commit 7a41f0d into main Jun 2, 2026
19 of 38 checks passed
@avrabe avrabe deleted the fix/salsa-validate-status-gates branch June 2, 2026 09:43
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 98.57143% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rivet-core/src/db.rs 98.46% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

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