Skip to content

Improve failure bucket presentation in block reports#187

Open
AlexJones0 wants to merge 3 commits intolowRISC:masterfrom
AlexJones0:report_bucket_sorting
Open

Improve failure bucket presentation in block reports#187
AlexJones0 wants to merge 3 commits intolowRISC:masterfrom
AlexJones0:report_bucket_sorting

Conversation

@AlexJones0
Copy link
Copy Markdown
Contributor

Sort the buckets so the most common failure mode appears first, and also display some metadata listing the total number of failures in each bucket. This should make the generated block report more useful for finding important issues.

@AlexJones0 AlexJones0 force-pushed the report_bucket_sorting branch from 67238c7 to 8ea415b Compare April 30, 2026 14:18
</thead>
<tbody>
{% for msg, job_list in failed_jobs.buckets.items() %}
{% for msg, job_list in failed_jobs.buckets.items() | sort(attribute='1|length', reverse=True) %}
Copy link
Copy Markdown
Contributor

@rswarbrick rswarbrick May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is neat: I didn't know anything about Jinja before today!

How does the attribute = '1|length' bit work? I don't think I can find any examples like this when searching online.

Copy link
Copy Markdown
Contributor Author

@AlexJones0 AlexJones0 May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out!

Jinja2 does support a dictsort, but it doesn't seem to offer any way that I can tell to sort over the length of the values specifically.

Instead, when we call .items(), we are then iterating over the dictionary as a tuple of (key, value) pairs. Jinja2 supports the use of integer indexes (e.g. 1) to sort over attributes of tuples. This works because for attribute accesses, Jinja2 just follows the rule of regular __getitem__ indexing. So it relies on the fact that you can essentially do e.g. next(iter(example_dict.items()))[1] in Python to get the value. The | length mapping is then used to basically just call len() on the result.

However, it turns out that actually this was not working! I had (incorrectly) assumed these can be chained together as in regular Jinja2 expressions, but it turns out the syntax in attribute here when sorting is not rich enough to express this sort of behaviour. And unfortunately it seems I was not rigorous enough in my testing, as the two cases I used to test already had the failures sorted, meaning I didn't catch it. I had assumed from the working test number annotations that things were working properly... apologies for that! I'm also not sure why Jinja2 isn't throwing an error in this case - it's obviously still interpreting it in some meaningful way.

I did come up with a solution to fix this in pure Jinja2, if you are interested:

{# We must sort within a namespace, as Jinja2's assignment scoping behaviour #}
{# prevents us from assigning within a loop. #}
{# See: https://jinja.palletsprojects.com/en/stable/templates/#assignments #}
{% set buckets = namespace(with_amounts=[]) %}
{% for k, v in failed_jobs.buckets.items() %}
    {% set buckets.with_amounts = buckets.with_amounts + [(v|length, k, v)] %}
{% endfor %}
{% for num_jobs, msg, job_list in buckets.with_amounts | sort(attribute=0, reverse=true) %}

However I think that this kind of Jinja2 logic is more complex than just pre-sorting the dictionary in Python before rendering the template, so I'm opting to switch to that solution instead.

AlexJones0 added 3 commits May 1, 2026 11:46
It makes sense that we probably want to see the most common failure
bucket first. Let's sort these in the generated HTML report (like we
already do for the CLI markdown reports) to make this more useful.

It turns out that Jinja2 does not make it easy to do this, so pre-sort
the list in Python before passing it to the template renderer.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This is kind of confusing, we have a line number `job.line` and also
different lines in the context log. Let's just rename this variable to
make the code cleaner.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
It would be useful to know from a glance how many test runs are in each
failing bucket. Let's add it as a bit of metadata at the top alongside
the failure message/category.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
@AlexJones0 AlexJones0 force-pushed the report_bucket_sorting branch from 8ea415b to 0a8fbbc Compare May 1, 2026 10:49
@AlexJones0 AlexJones0 dismissed hcallahan-lowrisc’s stale review May 1, 2026 10:50

Logic has changed since it wasn't actually working properly.

@AlexJones0 AlexJones0 requested a review from rswarbrick May 1, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants