feat(status): selectable template styles + DX-focused "Strata" style#1259
feat(status): selectable template styles + DX-focused "Strata" style#1259gregmagolan wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1469909f1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (data["bazel"].get("flaky_test_total", 0) or 0) > 0: | ||
| return ("⚠️", "Passed with flakes") | ||
| return ("✅", "Passed") | ||
| # Any terminal non-pass is a failure; name a timeout specifically. |
There was a problem hiding this comment.
When a task ends with status == "warning" (for example lint/format/gazelle/delivery warnings, or a Bazel run with only flaky tests), this code falls through to the failure verdict, so the Strata details heading renders ❌ Failed even though the status feature maps the same update to a neutral/warning result. Please handle warning before the failure fallback so Strata does not mislabel successful-with-warnings runs as failures.
Useful? React with 👍 / 👎.
| _style = resolve_style(ctx.args.style, ctx.args.templates or {}) | ||
| templates = {"summary": _style["summary"], "details": _style["details"]} |
There was a problem hiding this comment.
This resolved template bundle is passed later to whichever renderer_for_kind(update.kind) is selected; check_dispatch includes lint_results, format_results, gazelle_results, and delivery_results, but STRATA_DETAILS_TEMPLATE only renders the Bazel shared fields and never the non-Bazel renderers' actionable groups/file_list/delivery sections. With style = "strata", a failing or warning format/lint/gazelle/delivery check therefore loses the details users need to fix it, so either gate this template to Bazel renderers or add per-kind Strata details templates.
Useful? React with 👍 / 👎.
Sets style="strata" on GithubStatusChecks + GithubStatusComments so PR #1259's own CI renders the new DX-focused layout live on the PR comment + status checks.
gregmagolan
changed the base branch from
feat/per-failure-repro-commands
to
main
|
Temporarily retargeted the base to |
✨ Aspect Workflows🔄 Running…
🔁 Reproduce
|
gregmagolan
changed the base branch from
main
to
feat/per-failure-repro-commands
gregmagolan
force-pushed
the
feat/strata-template-style
branch
from
1621226 to
24eb13a
Compare
✅ Verified live on this repo's own CIRetargeted to GitHub status check (a real build that ran with low parallelism — note the verdict caught it and auto-expanded the perf section): GitHub PR comment (a real buildifier failure — the Fix section gave the exact reformat command): The buildifier failure it surfaced was real (my new |
CI status surfaces now support a `style` arg selecting how results render. Two styles ship built-in: - kilgore (default) — the original full report (byte-identical to before). - strata — a DX-focused layout that answers, in order: what broke → how to fix/repro it → why it was slow. Leads with a verdict heading + a compact monospace KPI strip (cache hit %, parallelism, wall + critical-path share, each with a 🟢/🟡/🔴 threshold emoji standing in for the web UI's tile color), shows failure snippets open by default with each failure's repro command beneath it, and collapses the reference sections (recoverable via the Aspect Web UI). The performance block auto-expands on a "Passed, but slow" verdict. Mechanism (lib/template_styles.axl): a style is a bundle of the three top-level Jinja2 template strings (summary/details for the check surfaces, body for the aggregated PR/MR comment) plus render knobs. resolve_style(name, overrides) layers the existing per-key `templates` arg on top of the named style, so users pick a style and replace just one slot for a custom variant. The Strata strings live in lib/strata_templates.axl; both consume the same build_details_data / _render_body data shape (no data-layer fork), with a new compute_strata_kpi() adding the KPI strip inputs. Wired into all five status features: GitHub status checks, GitHub PR comment, GitLab commit statuses, GitLab MR comment, Buildkite annotation. Tests: lib/template_styles_test.axl (20 unit tests — resolve_style selection + override layering, KPI thresholds, verdict words, slow-but-passing detection, verdict-without-metrics); Strata render passes added to the template + PR-comment snapshot suites and the inline-snippet render test in .aspect/axl.axl. Kilgore output is unchanged. Verified live on this repo's own CI (PR comment + GitHub status checks, including the "Passed, but slow" auto-expand). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
gregmagolan
force-pushed
the
feat/strata-template-style
branch
from
24eb13a to
1e51086
Compare
Flip the `style` arg default from kilgore to strata on all five status features (GitHub/GitLab status checks + comments, Buildkite annotations). kilgore stays available; strata is now what users get out of the box. `DEFAULT_STYLE` names the default in one place — both the feature `default =` literals and the `builtin_style` empty/unknown-name fallback resolve to it, so a mis-set arg renders the default rather than the other style. Docstrings + the `style` arg help updated to reflect the new default.
1 participant
The current CI status templates were a first pass that shows everything. This PR makes the template layout selectable and adds a second, DX-focused style — Strata — that answers, in priority order: what broke → how to fix/repro it → why it was slow. The existing layout is preserved as Kilgore (the default), byte-identical to today.
How it works
A style is a bundle of the three top-level Jinja2 templates a surface may need —
summary+details(check surfaces) andbody(the aggregated PR/MR comment) — selected by a newstylearg on every status feature:resolve_style(name, overrides)layers the existing per-keytemplatesarg on top of the named style, so you can pick a style and replace just one slot — that's how custom variants are expressed, no new mechanism:Wired into all five status features: GitHub status checks, GitHub PR comment, GitLab commit statuses, GitLab MR comment, Buildkite annotation.
What Strata looks like
Verdict-led, compact, aggregated PR comment:
On a check surface, the body leads with a verdict heading + a KPI strip — a fenced, monospace, threshold-emoji health line that stands in for the web UI's colored tiles (markdown has no color):
— then What broke (each failure's snippet open, with that failure's repro command right beneath it), then whole-run Fix / Reproduce, with the passed/cached rollup and the performance/invocation detail collapsed below. The performance block auto-expands on a "Passed, but slow" verdict.
Design synthesized from three independent design passes, drawing on BuildBuddy's results UI and the new Strata web UI in
silo.Changes are visible to end-users: yes
Suggested release notes
stylearg.stratais a new DX-focused layout — a verdict + a compact cache/parallelism/wall KPI strip, failure snippets shown with their repro commands inline, and reference sections collapsed. The original full report iskilgore(default, unchanged).templatesoverrides on top of anystyle.Test plan
lib/template_styles_test.axl— 20 unit tests:resolve_styleselection + override layering,compute_strata_kpithresholds (cache/parallelism/critical-path-share), verdict words, slow-but-passing detection, verdict-without-metricstest-template-snapshots,test-pr-comment-snapshots, and the inline-snippet render test in.aspect/axl.axlaspect tests axl(851),test-template-styles(20), all 7 snapshot suites,test-repro-commands/test-bazel-results/test-bazel-runner,cargo test -p aspect-cli(40)style = "strata"(in progress)