feat(validate): warn when prose names an artifact id without a typed link (#207)#234
Conversation
AI Code Review for PR #234pulseengine/rivet: Verdict: 💬 Comment Summary: The code snippet provided is a Rust implementation for validating artifacts in a project using a typed graph. The Findings (1):
Generated by a local AI model and post-validated against a strict JSON contract. Each finding includes the verbatim line being criticised — verify by reading the file at the cited location. Reviewed at |
…ed link Closes #207. Add a structural-validation pass that scans each artifact's `description` (and every string-typed value in its `fields` map) for tokens matching `\b[A-Z][A-Z0-9]*-[0-9]+\b`. When a match resolves to an artifact in the corpus and the mentioning artifact has no typed link to it, emit a Warning-severity diagnostic (`prose-mention-without-typed-link`). Suppression cases (matching the issue body): * the mention is the artifact's own id (self-reference), * the mentioned id does not resolve in the corpus (broken refs are a separate concern), * the artifact already has any typed link to that id. Per-(artifact, mentioned-id) dedup mirrors the unknown-link-type pass's policy so repeated mentions of the same id yield one warning. The regex is compiled once via `LazyLock<Regex>` (mirroring `rivet-core/src/markdown.rs`). Six unit tests cover every Tests-bullet from #207 plus dedup: * warn fires on bare mention, * typed link suppresses warn, * self-id mention suppresses warn, * unresolved id suppresses warn, * string-typed `fields` value is scanned, * three mentions of one id dedupe to one warning. Note on escalation: the issue mentions a hypothetical `rivet validate --strict` to escalate to Error. rivet already exposes `rivet validate --fail-on warning` which fails the run on any warning; that subsumes the hypothetical flag without a new surface. A literal severity-changing `--strict` is left for a follow-up if wanted. Implements: REQ-004
380e8fc to
86c2b02
Compare
📐 Rivet artifact deltaNo artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph. |
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: 86c2b02 | Previous: 05c9400 | Ratio |
|---|---|---|---|
link_graph_build/10000 |
28575716 ns/iter (± 943005) |
23408112 ns/iter (± 311314) |
1.22 |
validate/10000 |
14694884 ns/iter (± 257550) |
10596288 ns/iter (± 370139) |
1.39 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…246) Workspace, vscode-rivet, and npm root package versions bumped to 0.7.0. Platform packages stay on the release-npm.yml override path. What's in 0.7.0: - feat(schema): rivet schema migrate Phase 2 (#242) — full git-rebase conflict-resolution UX. Conflict markers in YAML, --continue, --skip, --edit. New MigrationConflict invariant in rivet docs check. - feat(docs-check): subcommand-coverage gate (#241) — walks the live clap CLI tree and asserts each path has an embedded docs topic. Default warn-only; --strict makes it enforcing. - feat(validate): prose-mention-without-typed-link warning (#234, closes #207). - feat(schemas): vv-coverage repo-status type (#232, partial #188). - feat(mutants): canonical cargo-mutants template (#229, closes #185). - docs(pre-commit): canonical 21-hook template (#222, closes #186). - fix(ci): Release workflow now idempotent on existing tag (#244). Known issue: v0.5.0 / v0.5.1 / v0.6.0 release pages have no binary assets attached because the workflow's Create Release step failed on each (race with manual gh release create). The fix in #244 lands in this release; v0.7.0 onward is unaffected. Older releases need a manual gh release upload to backfill. Verified: cargo check, cargo clippy --workspace -- -D warnings, cargo test -p rivet-cli, rivet docs check (clean), rivet docs check --coverage reports 48/81 paths covered (warn-only). Trace: skip
Closes #207.
Summary
rivet validatenow scans each artifact'sdescription(and every string-typed value in itsfieldsmap) for tokens matching\b[A-Z][A-Z0-9]*-[0-9]+\b. When such a mention resolves to an artifact in the corpus and the mentioning artifact has no typed link to it, validate emits aWarningdiagnostic (prose-mention-without-typed-link).This is the discipline analogue of Karpathy's
[[wikilinks]]for the typed-graph world — see the issue body's link to the companion blog post.Acceptance-criteria mapping (from #207)
The issue body's "Implementation sketch" enumerates testable bullets; mapped 1:1 here:
rivet-core/src/validate.rsprose-mention-without-typed-link, severityWarning, emitted from a new pass-10 invalidate_structural\b[A-Z][A-Z0-9]*-[0-9]+\b(cached statically)static ID_MENTION_RE: LazyLock<Regex>at module scope, mirroring the pattern inrivet-core/src/markdown.rsdescription+ every value infieldsthat is a stringif let Some(desc) = &artifact.description { scan(desc); }plus iteration overartifact.fields.values()filteringvalue.as_str()if mentioned == artifact.id { continue; }Store::get(mentioned_id)resolves; if yes and the artifact has no typed link to it, emitstore.contains(mentioned)AND!linked_targets.contains(mentioned)(BTreeSet of all link targets, regardless of link type)fieldsvalue is scanned; repeated mentions dedupe to one warning)On
--strictThe issue mentions a hypothetical
rivet validate --strictto escalate this warning to an error. rivet already exposesrivet validate --fail-on warning(parsed inrivet-cli/src/main.rs::parse_fail_on), which fails the run on any warning; that subsumes the hypothetical flag without adding a new CLI surface. A literal severity-changing--strictis left for a follow-up if a separate warning-stays-warn-but-rule-becomes-error semantic is wanted.Verification
cargo test -p rivet-core --lib— 822 passed, 0 failed (6 new tests for this rule, all green).cargo test -p rivet-core --tests— all integration test binaries pass.cargo clippy -p rivet-core --all-targets -- -D warnings— no new warnings (only the pre-existing MSRV-config mismatch warning).rustfmt --check rivet-core/src/validate.rs— clean../target/release/rivet validateon this repo:main:FAIL (6 errors, 10 warnings, 0 broken cross-refs)FAIL (6 errors, 68 warnings, 0 broken cross-refs)[TEST-027] prose mentions 'FEAT-061' but no typed link to it). They are not regressions; default--fail-on erroris unaffected, so CI/release/compliance/pre-commit flows that rely on the default exit-code threshold see no behaviour change. A follow-up clean-up PR can decide which mentions deserve typed links and which should be reworded.Tests added (all under
rivet-core/src/validate.rs::tests, annotated// rivet: verifies REQ-004)prose_mention_warns_when_no_typed_link— positive case: warn fires.prose_mention_suppressed_when_typed_link_present— typed link to mentioned id suppresses.prose_mention_suppressed_when_self_reference— self-id mention suppressed.prose_mention_suppressed_when_id_does_not_resolve— unresolved id suppressed (broken-ref handling is a separate rule).prose_mention_scans_string_field_values— string-typedfieldsvalues are scanned, not onlydescription.prose_mention_dedupes_per_id_per_artifact— three mentions of one id yield one warning.Test plan
cargo fmt --checkon the changed file is cleancargo test -p rivet-coregreen (lib + integration)cargo clippy -p rivet-core --all-targets -- -D warningscleanrivet validateexit code unchanged on default--fail-on error# rivet: ignore prose-mentionmechanismTrailers
Per
CLAUDE.md:Validation change →
REQ-004.Generated by Claude Code — issue-triage agent run 2026-04-28.
Generated by Claude Code