Refactor HTML/JSON report generation + fix revision information#109
Merged
AlexJones0 merged 3 commits intolowRISC:masterfrom Mar 10, 2026
Merged
Refactor HTML/JSON report generation + fix revision information#109AlexJones0 merged 3 commits intolowRISC:masterfrom
AlexJones0 merged 3 commits intolowRISC:masterfrom
Conversation
AlexJones0
force-pushed
the
report_refactor
branch
4 times, most recently
from
8d8181e to
62e625f
Compare
AlexJones0
force-pushed
the
report_refactor
branch
2 times, most recently
from
7af363a to
d3b115b
Compare
machshev
reviewed
Mar 10, 2026
src/dvsim/sim/report.py
Outdated
| gen_block_report(results=flow_result, path=path, version=summary.version) | ||
| for renderer in (JsonReportRenderer(), HtmlReportRenderer()): | ||
| report_files = renderer.render(summary) | ||
| write_report(report_files, path) |
Collaborator
There was a problem hiding this comment.
Is it possible to write to disk earlier for the individual files?
At the moment it looks like if there is an error in the template rendering (which can happen if certain values are optional), then we crash without saving any data at all.
Contributor
Author
There was a problem hiding this comment.
I've made this change now in my last two force pushes. I've retained the write_report interface, but each report renderer now takes an optional outdir argument of type Path | None. If that outdir is given, it writes generated artifacts to that path whilst generating. This way we have incremental writes - if templating fails halfway through we will still get some data.
For Markdown (which I will add later), I will just not provide an outdir and so this behaviour will not occur, since we just have the singular markdown output anyhow.
machshev
approved these changes
Mar 10, 2026
Collaborator
machshev
left a comment
machshev
left a comment
There was a problem hiding this comment.
Thanks Alex! Nice improvements.
AlexJones0
force-pushed
the
report_refactor
branch
2 times, most recently
from
cc52d8c to
f2ef0ca
Compare
Refactor the current report handling to more clearly abstract the details and interface of rendering HTML/JSON reports to artifacts and then emitting those artifacts. We still take an optional output directory to allow writing to disk whilst emitting, to account for the fact that an error might occur during templating, so that we still get partial results which might be of value. Otherwise we decouple the two. This will then be more extendible to adding Markdown report formatting, which will be much more involved than the HTML. It also easily allows us to add methods to emit the Markdown report to the CLI, instead of just writing it to disk. Now that we still generate a "summary" even if we are just generating a non-primary config, this results in a cleaner abstraction here. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This is still a bit messy, because we do not have ideal abstractions in place for the cfgs/flows. It might also be nice to separate out the revision info from the IP Meta even more in the future - some of the abstractions here still feel a bit arbitrary. This corrects a few issues with regards to current git commit / revision handling: - Report the short commit hash returned by git, not just an arbitrarily truncated hash. - The deploy commit is now listed correctly, instead of picking up the sim_cfg "revision" string which is generally not the same as the commit. - While the commit hash, branch and URL are used alternatively, the overriding "revision" string from the expanded SimCfg is now also picked up into IP metadata right now. This will need to be surfaced for Markdown reporting later. It currently isn't used in the HTML report, but probably should be later. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
report_refactor
branch
from
f2ef0ca to
63e86d1
Compare
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
2 participants
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.
This PR is the third of a series of 4 PRs which aim to fix issues in HTML report generation and add back Markdown CLI reports to DVSim.
This PR refactors and abstracts the HTML/JSON report generation logic in a way that will make it easier to introduce the Markdown CLI reporting in a follow-up PR. It also fixes the storage of revision information (commit, branch, URL) in the stored metadata to better capture the values used in the original DVSim / testplans. Specifically, it distinguishes between the commit (parsed via
gitpython) and revision (optional user override for reporting) more clearly, fixing some of the deploy object output, and more clearly defines the shortened commit sha viagitpython.It is recommended to review commit-by-commit, checking the commit messages for more info. See also this comment on the previous PR for information about revision handling.