feat: score the vPlan from test results as well as coverage - #243
Open
martin-velay wants to merge 5 commits into
Open
feat: score the vPlan from test results as well as coverage#243martin-velay wants to merge 5 commits into
martin-velay wants to merge 5 commits into
Conversation
CI checkouts leave the access token in the remote url, like https://oauth2:<token>@host/org/repo. We record that url in the run metadata and print it in the JSON and HTML reports, and those get archived, so the token leaks with them. ssh urls keep their user. git@ is part of the address, so dropping it would give a url that no longer reaches the remote. Passwords are stripped whatever the scheme. AI-assisted (Claude Code) - reviewed and approved by author Signed-off-by: martin-velay <mvelay@lowrisc.org>
The status-change callback carries no reason, so an observer cannot tell a failed job from one the scheduler cancelled before dispatching it. The new callback gets the reason and fires once the status is settled. The existing callback is untouched, since its only consumer is the status printer and that has no use for the reason. Flows opt in by overriding FlowCfg.on_job_completed, which does nothing by default. AI-assisted (Claude Code) - reviewed and approved by author Signed-off-by: martin-velay <mvelay@lowrisc.org>
A cov_vplan job runs dvplan over the coverage report and a dv_evidence.json this flow writes, so a nightly says how much of the verification plan is met and not just how much of the design was covered. Nothing runs unless the sim cfg names a vplan. The evidence comes from the scheduler's completion hook as each run finishes, so it covers jobs cancelled before dispatch too. A test the plan asked for and the regression never ran then shows up as a hole instead of going missing. Both sources go to one dvplan invocation, because dvplan writes an item off as unmeasurable when nothing it was given can measure it, and that sticks in the annotated file. Runs without --cov score from the evidence alone. Deploy gains a log_path property for the path it already built inline, which is what the LSF launcher reaches for. AI-assisted (Claude Code) - reviewed and approved by author Signed-off-by: martin-velay <mvelay@lowrisc.org>
martin-velay
changed the title
Review fixes for the vPlan back-annotation series. The plan could not be scored in the regression it most needs to describe. `needs_all_dependencies_passing` had two states and this job needs a third, so it becomes `DependencyPolicy`. `ALL_PASSING` is the default and `ANY_PASSING` is what CovMerge always did, so neither changes behaviour. CovVPlan takes `ALWAYS` and runs once its dependencies are terminal, whatever they concluded. Under either existing policy a regression where nothing passed was killed rather than scored, and with --cov the job has a single dependency, so anything that stopped the coverage report also stopped the plan. dvsim now defines the evidence format rather than deferring to dvplan. doc/dv_evidence.md specifies it and the pydantic models are normative. dvsim produces the file and is the public repo, so a consumer can be written against a spec rather than against whichever tool was built first. Whether dvplan is installed is decided by the job's own script, so it reads the PATH of the machine the job lands on rather than that of the host dvsim was launched from, which on a compute farm need not be the same. A `dvplan_inspect` pattern matching nothing is now a config error. The command is built while the jobs are, so it stops the run in seconds rather than failing inside dvplan once the regression has already gone. The vPlan report page is linked only once it exists, since a killed job or a machine without dvplan otherwise left a dead link in the HTML report. AI-assisted (Claude Code) — reviewed and approved by author Signed-off-by: martin-velay <mvelay@lowrisc.org>
martin-velay
force-pushed
the
testcase_dvplan
branch
from
August 18, 2026 14:29
af5f48e to
258e394
Compare
The page claimed the pydantic models were the normative definition of the whole format. They are not: they forbid the inspection key, so they reject a file DVPlan accepts. That is correct for a writer, but it makes the claim wrong. AI-assisted (Claude Code) — reviewed and approved by author Signed-off-by: martin-velay <mvelay@lowrisc.org>
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.
Linked to https://github.com/lowRISC/dvplan/pull/117
What this does
The
cov_vplanjob already annotated a vPlan from the coverage database. It now alsoannotates from the regression's test results, so a nightly says how much of the
verification plan is met and not only how much of the design was covered. A test the
plan asked for and the regression never ran shows as a hole rather than going missing.
dvsim writes those results to
dv_evidence.jsonand passes it to dvplan alongside thecoverage report. It also passes through a path to hand-written inspection records if the
cfg names one.
Nothing runs unless the sim cfg sets
vplan.Three decisions worth reviewing
The evidence comes from a new scheduler callback, not from the end-of-run results.
Everything it records is already in
run_results, but that only exists once the schedulerhas finished, and
cov_vplanis a scheduled job that needs the file before then. Thealternative is to stop scheduling it and run it at
gen_resultstime, which would dropthe callback, the collector and the
FlowCfghook. I kept it scheduled so the step keepsits row in the status table and can fail like any other job.
needs_all_dependencies_passingbecameDependencyPolicy. The bool had two statesand this job needs a third.
ALL_PASSINGis the default andANY_PASSINGis whatCovMergealways did, so neither changes.CovVPlantakesALWAYS, because a regressionwhere nothing passed is exactly the case the plan has to describe, and under either
existing policy it was killed instead of scored: with
--covit has one dependency, soanything that stopped the coverage report also stopped the plan.
dvsim defines the evidence format, not dvplan.
doc/dv_evidence.mdspecifies it andthe pydantic models are normative. dvsim is what produces the file and is the public repo,
so a consumer can be written against a spec rather than against whichever tool was built
first. dvplan is one consumer.
Dependencies and degradation
Needs the linked dvplan PR first: it defines the
dv_evidencesource name and themulti-source behaviour. Whether dvplan is installed is decided by the job's own script on
the machine it runs on, so a checkout without it warns and passes.
The credential fix rides along
git_https_url_with_commitreturns theoriginremote verbatim today, so a CI checkoutwith
https://oauth2:<token>@host/org/repoalready publishes the token in the JSON andHTML reports. This PR adds another archived artefact carrying the same URL, so the fix is
first in the series. It reverts cleanly on its own.
Behaviour change
A cfg that sets
vplanand runs without--covpreviously got no vPlan job. It now getsone, scored from test results alone.
Verified
ruff format --checkandruff check --config=ruff-ci.tomlclean,pytest --strict396passed,
license_check.pyclean,pyright182 errors unchanged, every commit testedstandalone. Run end to end against a real hmac regression in an OpenTitan tree.