Add a pure RegressionReport service - #307
Conversation
aliceinwire
commented
Aug 14, 2026
aliceinwire
commented
- Classifies regressions, fixes, unstable results, persistent failures, new results, and missing results.
- Preserves duplicate executions using occurrence indexes rather than collapsing them into dictionary entries.
- Uses origin, platform, architecture, compiler, configuration, path, and result type as the comparison identity.
- Incorporates Dashboard tree-report classifications for history-aware regression, fixed, and unstable detection.
- Classifies regressions, fixes, unstable results, persistent failures, new results, and missing results. - Preserves duplicate executions using occurrence indexes rather than collapsing them into dictionary entries. - Uses origin, platform, architecture, compiler, configuration, path, and result type as the comparison identity. - Incorporates Dashboard tree-report classifications for history-aware regression, fixed, and unstable detection. Signed-off-by: Arisu Tachibana <arisu.tachibana@miraclelinux.com>
| base_results, head_results = checkout(base), checkout(head) | ||
| # tree-report supplies history-aware unstable/regression decisions. If | ||
| # HEAD is not the newest checkout the raw transition remains useful. | ||
| history = self.get_tree_report(origin, branch, giturl) |
There was a problem hiding this comment.
so get_tree_report() uses default max_age_in_hours=None/min_age_in_hours=None. For me I get a error 400, can we pass a default in here (or real ints)
There was a problem hiding this comment.
Also probably worth wrapping this in a try/except: and set report.incomplete = True on failure. So history stays optional as the comment intends
| base_results, head_results = checkout(base), checkout(head) | ||
| # tree-report supplies history-aware unstable/regression decisions. If | ||
| # HEAD is not the newest checkout the raw transition remains useful. | ||
| history = self.get_tree_report(origin, branch, giturl) |
There was a problem hiding this comment.
Also probably worth wrapping this in a try/except: and set report.incomplete = True on failure. So history stays optional as the comment intends
| new = str(after.get("status") or "UNKNOWN").upper() | ||
| if key in history["unstable"]: | ||
| return "unstable" | ||
| if key in history["regression"] or (old == "PASS" and new in FAIL_STATUSES): |
There was a problem hiding this comment.
so PASS→PASS and even FAIL→PASS (a fix) get classified as regression. History should only disambiguate maybe require new in FAIL_STATUSES before the history key wins. Same shape just below for fixed.
| category = cls._classify(before, after, key, history) | ||
| if category is None: | ||
| continue | ||
| chosen = after or before |
There was a problem hiding this comment.
this crashes for me on a falsy head entry (an empty {} execution becomes None -> AttributeError). after if after is not None else before fixes it.
| old[_key(item, kind)].append(item) | ||
| for item in head_results.get(kind + "s", []): | ||
| new[_key(item, kind)].append(item) | ||
| for key in sorted(set(old) | set(new)): |
There was a problem hiding this comment.
Duplicate pairing is positional, so it's order-dependent: [PASS, FAIL] vs [FAIL, PASS] gives regression+fixed, while [PASS, FAIL] vs [PASS, FAIL] gives persistent_fail. mainline these dupes are usually the same board in different labs, and lab isn't in the identity, so sorting each bucket (or adding lab) would make it deterministic
| return tuple(result_identity(item, kind).values()) | ||
|
|
||
|
|
||
| def _tree_report_keys(tree_report, category): |
There was a problem hiding this comment.
The history identity built here ends up with kind="test" and origin="unknown", so it can never match boot or build results (and likely not tests either). Dropping kind and filling origin from the tree report would let history actually apply
| ) | ||
| if report["incomplete"]: | ||
| raise click.exceptions.Exit(2) | ||
| policies = {value.strip() for value in fail_on.split(",")} |
There was a problem hiding this comment.
--fail-on isn't validated. Could we check the parsed set against the category names and UsageError on unknowns?
| if bool(base) != bool(head): | ||
| raise click.UsageError("provide both --base and --head, or neither") | ||
| if not base: | ||
| giturl, branch, latest = set_giturl_branch_commit( |
There was a problem hiding this comment.
This auto base/head resolution raises click.Abort. Not a KciDevError, we should also catch click.Abort
| click.echo(json.dumps({"error": str(exc), "incomplete": True})) | ||
| raise click.exceptions.Exit(2) from exc | ||
| if json_output: | ||
| # This is deliberately the only stdout write in JSON mode. |
There was a problem hiding this comment.
Your comment here "only stdout write in JSON mode". Threading error_verbose=False through get_build_issues/get_boot_issues (or redirecting to stderr) would fix it.
| report = RegressionReport.compare( | ||
| base, head, base_results, head_results, history | ||
| ) | ||
| if include_issues: |
There was a problem hiding this comment.
This issues one request per regression and per persistent_fail; on a big commit that's thousands of sequential calls. Worth capping it or making include_issues opt-in.
| @@ -0,0 +1,88 @@ | |||
| import json | |||
There was a problem hiding this comment.
Could we add coverage for the history branches, the gate command (exit codes and --fail-on parsing), and duplicate count mismatches (base 2 / head 1)? Those paths hold most of the issues above and are currently untested.
2 participants
