fix(mythos): 4 one-liner silent-accept bugs (validate/coverage/yaml_hir/formats)#168
Merged
fix(mythos): 4 one-liner silent-accept bugs (validate/coverage/yaml_hir/formats)#168
Conversation
…ntics
Before this fix, `validate::validate` and `coverage::compute_coverage`
gave contradictory answers on the same traceability rule + artifact
data:
- `TraceabilityRule.target_types` / `.from_types` are `#[serde(default)]`
so they deserialize to an empty `Vec<String>` when omitted.
- `validate::validate` treated an empty list as "match nothing"
(rule.target_types.contains(&t) is false for every t), producing a
false-positive violation.
- `coverage::compute_coverage` treated an empty list as "match any"
(`if target_types.is_empty() { true } else { ... }`), reporting the
same artifact as fully covered.
Result: `rivet validate` said "1 error" while `rivet coverage` said
"1/1 (100%)" on the same inputs. Discovered by the Mythos pass.
Unify on the "match any" convention (also used by LinkFieldDef checks
at validate.rs ~L310) so both tools agree. Adds two regression tests
that pin validate and coverage must never contradict each other on
empty `target-types` or `from-types`.
Fixes: REQ-004
Verifies: REQ-010
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`compute_coverage` counted a link from an artifact to itself (e.g. `DD-001 → DD-001` via `satisfies`) as a valid rule satisfaction. That meant an author blocked from finding upstream trace could pass CI by writing a single self-referential line — the author's own DD closed the loop. Add `l.target != *id` (forward) and `bl.source != *id` (backlink) to the filter chain inside `compute_coverage`. Two regression tests — one per direction — pin the expected behaviour. Discovered by the Mythos pass; empirically reproducible. Fixes: REQ-004 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hantom link
When a schema-shorthand-link field is written with a null-like YAML
value (e.g. `losses: null`, `losses: ~`, `losses: ""`), the extractor
was creating a `Link { target: "null" }` / `"~"` / `""` instead of
treating it as "no link." These phantom links silently pollute the
trace graph, and the YAML footgun fuzzer surfaced the behaviour as a
confirmed bug.
Add an `is_null_or_empty_scalar` helper and guard the shorthand link
emission on it. Also skip empty-string entries inside list shorthand
values (`losses: [L-1, ""]`). Three regression tests cover the null,
tilde, and empty-string cases.
Implements: REQ-028
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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: b83abaa | Previous: 60d728a | Ratio |
|---|---|---|---|
store_lookup/1000 |
24966 ns/iter (± 380) |
19280 ns/iter (± 48) |
1.29 |
diff/100 |
66952 ns/iter (± 2804) |
53071 ns/iter (± 128) |
1.26 |
diff/1000 |
712509 ns/iter (± 8299) |
584274 ns/iter (± 2706) |
1.22 |
query/100 |
748 ns/iter (± 11) |
619 ns/iter (± 1) |
1.21 |
query/1000 |
6577 ns/iter (± 43) |
5174 ns/iter (± 14) |
1.27 |
This comment was automatically generated by workflow using github-action-benchmark.
…ly dropping Without `#[serde(deny_unknown_fields)]` on `GenericFile`, a file that has a valid `artifacts:` key plus a typo-ed companion key like `artifact:` (singular) or `Artifacts:` (wrong case) deserialized successfully and silently dropped every artifact under the typo'd key. The fuzzer (test/yaml-cli-fuzzers branch) confirmed the resulting trace-graph hole. Add `deny_unknown_fields` so unknown top-level keys surface as `serde_yaml::Error`s. `parse_generic_yaml` already bubbles these up as `Error::Yaml`, and the diagnostics pipeline in db.rs (`collect_parse_errors`) converts them to `Severity::Error` diagnostics — so the typo is now user-visible rather than silent. Two regression tests pin the new behaviour for `artifact:` (singular) and `Artifacts:` (capitalised) companion keys. Fixes: REQ-004 Verifies: REQ-010 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
avrabe
force-pushed
the
fix/mythos-quickfixes
branch
from
3570696 to
b83abaa
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This was referenced Apr 22, 2026
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.
Four confirmed silent-accept bugs, one PR
All four were found in the Mythos discovery pass and empirically reproduced by the YAML footgun fuzzer (branch `test/yaml-cli-fuzzers` / PR #160). Each fix is ≤10 LoC of code + 2-3 regression tests.
Fixes
1. `validate.rs` + `coverage.rs` — empty `from_types`/`target_types` semantics unified
Same rule + same data used to produce contradictory reports: `rivet validate` said "1 error", `rivet coverage` said "1/1 (100% covered)". `TraceabilityRule.{from,target}_types: []` meant "match nothing" to validate and "match any" to coverage.
Fix: align both on "match any" (the existing majority convention — `coverage.rs` uses it twice, `validate.rs:310` uses it for `LinkFieldDef` checks). Only the two `TraceabilityRule` branches dissented. Zero `[]` occurrences in any `schemas/*.yaml`, so no user-visible regression.
2. `coverage.rs` — self-satisfying links no longer count
`DD-001 → satisfies → DD-001` used to count as coverage. Author blocked from finding a real upstream could close the loop on their own DD and pass CI.
Fix: `l.target != *id` in forward filter, `bl.source != *id` in backlink filter. One-liner each.
Note: the original task brief said `l.source != id`, but `links_from(id)` always returns links where `l.source == id`, so the semantically correct self-link guard is `l.target != id`. Agent caught this.
3. `yaml_hir.rs` — null/~/empty shorthand no longer emits phantom link
`controller: null` used to produce `Link { target: "null" }` which `rivet validate` then flagged as a confusing "broken-link targets 'null'" error. Three empirical reproductions in the fuzzer (null, tilde, empty string).
Fix: new `is_null_or_empty_scalar` helper guards shorthand-link emission in `extract_section_item`. Also skips empty strings inside list shorthand (`losses: [L-1, ""]`).
4. `formats/generic.rs` — `artifact:` typo no longer silently invisible
`GenericFile` lacked `#[serde(deny_unknown_fields)]`. User typed `artifact:` (singular) or `Artifacts:` (capitalized) → deserializer returned `Ok(GenericFile { artifacts: vec![] })` with zero warnings. Whole file invisible to the trace graph. Fuzzer confirmed.
Fix: add `#[serde(deny_unknown_fields)]`. Note: intentionally did not touch `import_generic_directory`'s `log::warn! + drop` path because that function serves mixed-format directories (STPA/cybersecurity/safety-case). End-to-end the typo now surfaces via the salsa `collect_parse_errors` → `Severity::Error` diagnostic pipeline.
Test plan
Commit-level trailers
Each commit has its own `Fixes: REQ-NNN` trailer (mix of REQ-004 validation engine, REQ-010 schema, REQ-028 parser) — see individual commits.
Not fixed here (remaining Mythos items for future PRs)
🤖 Generated with Claude Code