feat(v0.9.0): test-plan schema + branch-integrity gate + oracle JSON gate (PR #1) - #120
Conversation
…gate (PR #1) Guided Fault Finding cycle, slice 1 of 5 (Tier A). - docs/testplans.md: the [[step]] schema contract (measurement verbs, on_pass/on_fail/next branches, conclusion nodes, per-step source rule). - community/testplans/README.md: author-facing quick reference. - src-tauri/src/community.rs (test-only): shipped_testplans_branch_integrity (resolvable branches, reachable conclusion, sourced steps, dtc==filename, acyclic BFS) + shipped_oracle_json_parses (survey finding #3: oracle JSON was ungated). - CHANGELOG.md. Gate proven with a temporary broken fixture (dangling branch + unsourced step both caught), then removed. cargo test 102 passed; lint-toml clean. No production code changed.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cef35042c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for target in [&step.on_pass, &step.on_fail, &step.next].into_iter().flatten() { | ||
| if visited.insert(target.as_str()) { | ||
| queue.push_back(target.as_str()); |
There was a problem hiding this comment.
Reject cyclic test-plan branches
When a plan has a reachable cycle and any reachable conclusion (for example s1.next=s2, s2.next=s1, and s2.on_fail=s3 where s3 concludes), this loop silently drops the second visit because visited.insert returns false. Since all branch targets were already proven to exist, visited.len() <= plan.step.len() can never trip, so the new CI gate accepts cycles despite the documented acyclic rule and can let data through that later traversal code may loop on.
Useful? React with 👍 / 👎.
| [meta] | ||
| title = "VANOS intake solenoid fault" # human label for the UI | ||
| engine_family = "n55" # OPTIONAL hint; not a gate target | ||
| [suppressed] # OPTIONAL — see "Honesty" below |
There was a problem hiding this comment.
Nest suppressed under meta in the schema example
When a contributor copies this top-level schema, [suppressed] creates a top-level table, but the gate only recognizes plan.meta.suppressed and the README example uses [meta.suppressed]. A bodyless placeholder written from this example will not be skipped and will fail as a non-suppressed plan with no steps, so the documented header should match the gated table name.
Useful? React with 👍 / 👎.
| serde_json::from_str::<serde_json::Value>(&content) | ||
| .unwrap_or_else(|e| panic!("shipped Oracle JSON must parse: {path:?}: {e}")); |
There was a problem hiding this comment.
Validate Oracle JSON against the loader shape
For a syntactically valid JSON file that is not the HashMap<String, OracleEntry> object oracle::load expects, such as an array or an entry with a wrongly typed field, this test passes because every JSON value deserializes into serde_json::Value; startup will still log the loader error and skip the file. Since this is the new CI gate for shipped Oracle data, it should deserialize into the same map shape (or a mirror of it) rather than only checking JSON syntax.
Useful? React with 👍 / 👎.
| assert!( | ||
| !s.source.trim().is_empty(), |
There was a problem hiding this comment.
Verify sources resolve inside the repo
For a step with source = "https://..." or source = "missing.toml", this assertion passes because it only checks that the string is non-empty. The schema and README promise an in-repo citation for every step; without resolving the path and rejecting absolute or parent-directory escapes, CI can accept plans whose provenance link is unusable in the later walkthrough UI.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| #[derive(Deserialize)] | ||
| struct TestPlanStep { |
There was a problem hiding this comment.
Reject malformed measurement blocks
For any plan containing a measurement table, serde ignores its contents here because TestPlanStep has no measurement field and unknown TOML keys are allowed by default. A typo such as kind = "uds" or a manual check without question can therefore pass the new schema gate even though the documented UI contract cannot execute it.
Useful? React with 👍 / 👎.
| assert!( | ||
| reachable_conclusion, | ||
| "{path:?}: no conclusion node reachable from s1 (rule 2)" |
There was a problem hiding this comment.
Require every branch path to conclude
When one reachable branch dead-ends at a non-conclusion step while a different branch reaches a conclusion, this single boolean still passes the plan. That lets CI accept a walkthrough that can strand the UI on inputs that take the dead-end branch, because the gate only proves some conclusion is reachable from s1, not that every terminal path ends in one.
Useful? React with 👍 / 👎.
| assert_eq!( | ||
| plan.dtc.to_uppercase(), | ||
| stem.to_uppercase(), |
There was a problem hiding this comment.
Enforce uppercase DTC filenames exactly
Because both sides are uppercased before comparison, a plan committed as community/testplans/2a82.toml with dtc = "2a82" passes even though the documented read paths produce uppercase protocol::Dtc.code values and the schema requires uppercase filenames. On a case-sensitive install this can leave a valid-looking shipped plan that a later lookup by 2A82.toml will not find.
Useful? React with 👍 / 👎.
| #[serde(default)] | ||
| #[allow(dead_code)] | ||
| title: String, |
There was a problem hiding this comment.
Validate required test-plan titles
meta.title is marked required in the schema, but this test-only shape defaults it and no assertion ever checks it. A non-suppressed plan without a title therefore passes CI and later UI code has no reliable walkthrough header to display, defeating the gate's purpose as the contract for authored plans.
Useful? React with 👍 / 👎.
ROADMAP v0.9.0 'Guided Fault Finding' moves Planned -> In progress. PRs #1/#2/#3/#4 (#120/#121/#122/#123) marked Done; PR #5 (validation harness + contribution path) remains Ready/Not yet dispatched. Footer refreshed to 2026-07-19. Doc-only, no code change. Tier A per CLAUDE.md. Co-authored-by: hermes-agent <ohgeeceee@users.noreply.github.com>
v0.9.0 "Guided Fault Finding" — slice 1 of 5 (Tier A)
First slice of the guided-fault-finding cycle (
docs/v0.9.0_plan.md). Installs the schema + CI gates every later slice builds on — no production code changes, so it lands autonomously perCLAUDE.mdTier A.What's here
docs/testplans.md— the[[step]]schema contract: step ids,instruction,measurement(adid:live-data poll with expected range, or amanualyes/no observation),on_pass/on_fail/nextbranch targets,conclusionnodes, and the mandatory per-stepsourcecitation rule (in-repo grounding only — no forum paste, no invented procedure).community/testplans/README.md— author-facing quick reference mirroring the schema.src-tauri/src/community.rs(test-only) — two new gates:shipped_testplans_branch_integrity: everyon_pass/on_fail/nextresolves to a step in the same file; ≥1conclusionreachable froms1(BFS); every step names a non-emptysource; top-leveldtcequals the filename stem; reachable graph is acyclic (BFS bounded by step count).shipped_oracle_json_parses: gatescommunity/oracle/*.json— survey finding chore(deps): bump toml from 0.8.2 to 0.8.23 in /src-tauri in the cargo-minor-patch group #3 (a broken Oracle file previously failed only as a startupeprintln!CI never saw).CHANGELOG.md— two bullets.Verification
9999.toml): first caught an unsourced step, then a danglingon_failtarget — both failcargo test. Fixture removed before commit.cargo test— 102 passed (lib) + async-command allowlist test green (no command added this slice) + all other targets green.node scripts/lint-toml.js— clean, 24 files.Protected paths
None.
community.rschange is confined to#[cfg(test)].