TRT-2846: payload-snapshot: classify flakes and informing tests separately from failures

[not-stbenjam]

Stacked on #644 — that PR is the first four commits here and should merge first. Review only classify-flakes-and-informing-tests (the last commit). Stacked rather than standalone because both touch _parse_junit_xml.

Problem

A test result is one of three things, and only one of them can fail a job and therefore reject a payload:

Category	Rule	Gates?
flake	same test, same suite, both failed and passed	no
informing	testcase carries lifecycle="informing"	no
failure	failed everywhere, no informing lifecycle	yes

The parser recognised neither of the first two. grep -ci flake payload_snapshot.py returned 0, and the testcase's lifecycle attribute was parsed and thrown away — _TestResult had no field for it. So _test_results_to_json emitted every <failure> element as a failure.

Two consequences:

1. test_failure_count overstates what could have rejected the payload.
2. Worse — a non-cause can drive regression onset. first_failed_in could be set by an informing test, sending the analysis to hunt for a culprit PR behind a test that never gated anything. That is exactly what happened in the chain analysed in payload-snapshot: fix silent data loss on unauthenticated gcloud (supersedes #641) #644, where the previous analysis chased [Feature:NetworkSegmentation] UDN onset across payloads that contained zero PRs.

Informing tests are run in the wild to stabilize them; they are not expected to gate. A missing lifecycle attribute means the test does gate — the attribute exists only to opt a test out.

Measured on real data

One conformance run's JUnit from that chain (run):

	before	after
test_failure_count	11	0
test_informing_failure_count	—	11

All 11 were UDN tests being stabilized. None could have failed the job.

And the aggregated report for 5.0.0-0.ci-2026-07-25-162741 — the payload whose misattribution started all of this:

	entries	gating	informing
aggregated-aws-ovn-upgrade-5.0-major	14	3	11

The entire blocking signal was 3 tests (the CVO invariants), sitting among 14 recorded failures.

Changes

• _TestResult carries test_lifecycle; the parser reads the attribute.
• _mark_flakes() relabels a failure as a flake when the same test also passed in the same suite. Grouping is per (suite, name) deliberately: the same monitor evaluated in openshift-tests-upgrade and openshift-tests covers two different phases, so a pass in one does not clear a failure in the other. Treating those as flakes would silently discard real upgrade-phase failures — including alert/etcdNoLeader and the CVO invariants.
• results.json records all three categories with status and test_lifecycle; nothing is hidden.
• test_failure_count counts gating results only. test_flake_count and test_informing_failure_count are separate.
• summary.json gains test_failures.informing[] and test_failures.flakes[]. No onset is tracked for either — an onset implies a culprit to find.
• Regression tracking uses gating failures only, and logs what it excluded.
• payload-analysis: never score informing failures or flakes as candidates, never derive an originating payload from them, never revert for them. Report them in their own section stating they cannot cause a rejection — with the one exception worth investigating: a test that damages the cluster it runs on.

Tests

13 new (38 total in this directory), including the cross-suite case that must not be classified as a flake, absent-lifecycle-gates, informing-flake, and mixed counting. Validated against real aggregated and non-aggregated JUnit.

Scope note: what is and isn't verified by real data

The flake convention is confirmed, not assumed. openshift/origin writes a flake as two testcases with the same name in the same testsuite — one carrying a failure, one not (pkg/test/ginkgo/junit.go:118-138):

case test.flake:
    s.NumTests++
    s.NumFailed++
    failedTestCase := &junitapi.JUnitTestCase{Name: test.name, FailureOutput: ...}
    s.TestCases = append(s.TestCases, failedTestCase)

    // also add the successful junit result:
    s.NumTests++
    successTestCase := &junitapi.JUnitTestCase{Name: test.name, ...}
    s.TestCases = append(s.TestCases, successTestCase)

_mark_flakes() matches that exactly, including the same-suite grouping — s.TestCases is one suite, so a pass in a different suite is a different test.

Note also s.NumFailed++ on the flake path: even the testsuite's own failures= attribute counts flakes as failures, so there is no pre-computed count to lean on instead.

But I have not seen the flake path fire on real data. Across the JUnit I pulled while investigating this chain:

	files	testcases	same-suite duplicates	flakes
aggregated	8	63,183	0	0
non-aggregated	1	2,238	0	0

Eight of the nine are aggregated reports, which cannot contain a flake by construction — the aggregator emits one verdict per test, so a retry appears as passed N times, failed M times in the summary text rather than as duplicate testcases. That leaves exactly one file that could have exhibited a flake, and it had none.

So: the flake code path is correct with respect to the writer that produces the input, and is covered by unit tests including the cross-suite case it must not match — but reviewers should treat it as unexercised in the wild. It changes no number in this PR's before/after tables. The informing split is what measurably changes results today.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Improvements
  • Payload snapshots now clearly separate gating failures from informing results and flakes in both reporting and regression onset logic.
  • More reliable snapshot completeness: collection errors are tracked and surfaced, with AGENTS marked when incomplete.
  • JUnit handling is hardened for missing, unavailable, corrupted, partially readable, and unparseable XML outputs.
  • Added --fail-on-incomplete to exit non-zero when unrecovered collection errors remain.
• Documentation
  • Updated rules for gating-only rejection/scoring, and clarified “informing jobs” vs “informing tests.”
• Tests
  • Added pytest coverage for JUnit classification and gating vs non-gating counting.
• Chores
  • Bumped CI plugin version to 0.0.75 in marketplace metadata.

[openshift-ci]

Hi @not-stbenjam. Thanks for your PR.

I'm waiting for a openshift-eng member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The payload snapshot now classifies JUnit outcomes as gating failures, flakes, or informing results. Regression and summary generation use gating results only, while collection errors, incomplete data, and non-gating visibility are recorded separately.

Changes

Gating-aware payload snapshots

Layer / File(s)	Summary
JUnit outcome classification and validation plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py, plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py, plugins/ci/skills/payload-snapshot/SKILL.md	JUnit lifecycle data is captured, flakes are identified, gating rules are applied, and classification behavior is covered by tests and documentation.
Regression and summary semantics plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py, plugins/ci/skills/payload-analysis/SKILL.md	Regression onset and failure counts use gating results only; informing and flake results are emitted separately for visibility and excluded from rejection or revert reasoning.
Collection integrity and stale state plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py	Artifact command failures are classified with scoped recovery tracking, incomplete JUnit results are reported, and stale regression files are removed for invalidated payloads.
Plugin metadata alignment .claude-plugin/marketplace.json, docs/index.html, plugins/ci/.claude-plugin/plugin.json	CI plugin version metadata is updated to 0.0.75.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• openshift-eng/ai-helpers#627: Adjusts payload-analysis guidance for failure classification and revert decisions.
• openshift-eng/ai-helpers#644: Updates payload snapshot JUnit collection and completeness semantics.

Suggested labels: approved, lgtm

Suggested reviewers: zaneb

Sequence Diagram(s)

sequenceDiagram
  participant JUnitArtifacts
  participant PayloadSnapshot
  participant RegressionTracker
  participant SummaryGenerator
  participant SummaryJSON
  JUnitArtifacts->>PayloadSnapshot: provide JUnit XML
  PayloadSnapshot->>PayloadSnapshot: parse lifecycle and classify outcomes
  PayloadSnapshot->>RegressionTracker: provide parsed results
  RegressionTracker->>RegressionTracker: track gating regressions only
  PayloadSnapshot->>SummaryGenerator: provide gating and non-gating results
  SummaryGenerator->>SummaryJSON: write failure lists and collection status

Loading

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
No Assumed Git Remote Names	❌ Error	The PR adds git push origin ... in evals/jira-solver/README.md with no prior remote discovery, violating the no-assumed-remote rule.	Replace the hardcoded remote with a discovered remote (e.g. git remote -v/git remote get-url) and update the example to use that variable.
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.16% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (8 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main change: separating flakes and informing tests from gating failures in payload-snapshot.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No Real People Names In Style References	✅ Passed	The last-commit diff only bumps ci version numbers; no real-person names appear in style references or examples.
Git Push Safety Rules	✅ Passed	The PR only changes payload-snapshot docs/code and version files; no git push, force-push, or protected-branch push logic appears in the touched files.
No Untrusted Mcp Servers	✅ Passed	Only plugin version bumps changed; no MCP server installs, npx mcp-server calls, or new MCP deps were added.
Ai-Helpers Overlap Detection	✅ Passed	The changed skills are complementary (snapshot vs analysis), and similarity scans found no existing ai-helper content near the 60% overlap threshold.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (3)

plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py (2)

954-973: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff

Fallback probing is serial: 3+ gcloud storage ls calls per step dir.

This path triggers exactly on jobs with huge artifact trees (many step dirs), so worst case is dozens of sequential 60s-timeout invocations. Consider probing steps through a small ThreadPoolExecutor (the error ledger is already lock-protected and scopes are thread-local, so recording stays safe — note that errors raised in worker threads won't land in the caller's scope, so mark recovery explicitly).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py` around lines
954 - 973, The artifact probing loop in the payload snapshot recovery path
performs multiple gcloud listings serially, causing slow recovery for many step
directories. Update the probing around the step_dirs loop to use a small
ThreadPoolExecutor, with each worker probing one step and returning its
discovered paths; merge results in the caller while preserving the existing
patterns and aggregator-specific recursive probe. Mark recovery explicitly and
handle worker failures in the caller because thread-raised errors do not
populate the caller’s scope.

---

1551-1569: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Also extend the generated schema list with the new fields.

The incomplete-snapshot banner is good, but the "summary.json Schema" block emitted a few lines below still lists only test_failures.blocking[] and never mentions test_failures.informing[], test_failures.flakes[], data_complete, or collection_errors[]. AGENTS.md is the first thing an agent reads, so the categories this PR introduces stay invisible there.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py` around lines
1551 - 1569, Update the summary.json Schema block generated by the surrounding
payload snapshot reporting code to include test_failures.informing[],
test_failures.flakes[], data_complete, and collection_errors[] alongside the
existing test_failures.blocking[] entry. Keep the schema output consistent with
the new summary fields introduced by the incomplete-snapshot handling.

plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py (1)

41-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

No coverage for <error> testcases.

_is_gating and _mark_flakes both treat error alongside failed, but _case can only emit <failure>. An errored=True variant would cover the error→flake relabel and error-with-lifecycle="informing" paths.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py`
around lines 41 - 50, Extend the test helper _case with an errored option that
emits an <error> testcase body, alongside the existing failed and skipped
variants. Use this helper option in tests covering _is_gating and _mark_flakes
so error cases exercise both error-to-flake relabeling and error cases with
lifecycle="informing".

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1943-1952: When collect_junit is enabled in the payload processing
flow, also invalidate the corresponding regressions.json for each payload tag
whose JUnit output is removed by _invalidate_suspect_junit. Update the logic
around _invalidate_suspect_junit so _track_regressions recomputes regressions
after re-collection, while preserving the existing carry-forward behavior when
collect_junit is disabled.
- Around line 1746-1751: The _build_failed_job_details flow must mark
junit_collection_failed when _job_junit_state reports junit_unavailable,
junit_missing, or junit_unparseable, while continuing to omit
test_failure_count. In
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py lines 1746-1751,
extend the condition accordingly. In plugins/ci/skills/payload-snapshot/SKILL.md
lines 228-258, retain the existing “Missing data is absent, never empty” bullets
because the implementation will satisfy the documented junit_collection_failed:
true contract.
- Around line 1023-1035: Replace the global before/after error-count check in
the build-log collection flow with the existing _error_scope() mechanism, so
only errors raised while _run_gcloud_bytes for this job executes determine
whether _record_collection_error("build_log_unavailable", ...) is called. Follow
the established usage in JUnitCollector.collect and preserve the existing
command, detail, stage, and job metadata.
- Around line 2515-2520: Update the no_match classification logic using
_GCLOUD_NO_MATCH_PATTERNS so only the specific “URLs matched no objects” outcome
is allowlisted; remove broad absence tokens such as “not found” and “not exist”.
Evaluate authentication, permission, and HTTP 4xx/status tokens before applying
the no-match check so those failures remain in collection_errors.

In `@plugins/ci/skills/payload-snapshot/SKILL.md`:
- Around line 211-216: Update the later “results.json (in junit/ subdirectory)”
section to state that results include failed, error, and flake categories, and
document the status and test_lifecycle fields with their supported values. Keep
its existing description of the file’s purpose and location intact.

---

Nitpick comments:
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 954-973: The artifact probing loop in the payload snapshot
recovery path performs multiple gcloud listings serially, causing slow recovery
for many step directories. Update the probing around the step_dirs loop to use a
small ThreadPoolExecutor, with each worker probing one step and returning its
discovered paths; merge results in the caller while preserving the existing
patterns and aggregator-specific recursive probe. Mark recovery explicitly and
handle worker failures in the caller because thread-raised errors do not
populate the caller’s scope.
- Around line 1551-1569: Update the summary.json Schema block generated by the
surrounding payload snapshot reporting code to include
test_failures.informing[], test_failures.flakes[], data_complete, and
collection_errors[] alongside the existing test_failures.blocking[] entry. Keep
the schema output consistent with the new summary fields introduced by the
incomplete-snapshot handling.

In `@plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py`:
- Around line 41-50: Extend the test helper _case with an errored option that
emits an <error> testcase body, alongside the existing failed and skipped
variants. Use this helper option in tests covering _is_gating and _mark_flakes
so error cases exercise both error-to-flake relabeling and error cases with
lifecycle="informing".

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 058cf7eb-af8e-4e54-b773-52a1055bb196

📥 Commits

Reviewing files that changed from the base of the PR and between f55977e and 84166aa.

📒 Files selected for processing (8)

• .claude-plugin/marketplace.json
• docs/index.html
• plugins/ci/.claude-plugin/plugin.json
• plugins/ci/skills/payload-analysis/SKILL.md
• plugins/ci/skills/payload-snapshot/SKILL.md
• plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
• plugins/ci/skills/payload-snapshot/scripts/test_collection_completeness.py
• plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py (1)

1767-1796: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Preserve suite identity when grouping non-gating tests.

found is keyed only by name, so same-named tests from different suites collapse into one informing/flake summary entry. Key by suite plus test name and emit the suite identity with the entry.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py` around lines
1767 - 1796, Update the grouping logic in the lifecycle test-processing block to
key entries by both suite identity and test name, preventing same-named tests
from different suites from merging. Preserve the existing job aggregation, and
include the suite identity in each emitted found entry alongside test_name and
jobs.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1767-1796: Update the grouping logic in the lifecycle
test-processing block to key entries by both suite identity and test name,
preventing same-named tests from different suites from merging. Preserve the
existing job aggregation, and include the suite identity in each emitted found
entry alongside test_name and jobs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e3d5e168-3b8a-4a24-a3e3-293bdfcfc872

📥 Commits

Reviewing files that changed from the base of the PR and between a929196 and 72e8ab7.

📒 Files selected for processing (1)

• plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py

[stbenjam]

/test payload-agent

[stbenjam]

/ok-to-test

[stbenjam]

/retitle TRT-2846: payload-snapshot: classify flakes and informing tests separately from failures

[openshift-ci-robot]

@not-stbenjam: This pull request references TRT-2846 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Stacked on #644 — that PR is the first four commits here and should merge first. Review only classify-flakes-and-informing-tests (the last commit). Stacked rather than standalone because both touch _parse_junit_xml.

Problem

A test result is one of three things, and only one of them can fail a job and therefore reject a payload:

Category	Rule	Gates?
flake	same test, same suite, both failed and passed	no
informing	testcase carries lifecycle="informing"	no
failure	failed everywhere, no informing lifecycle	yes

The parser recognised neither of the first two. grep -ci flake payload_snapshot.py returned 0, and the testcase's lifecycle attribute was parsed and thrown away — _TestResult had no field for it. So _test_results_to_json emitted every <failure> element as a failure.

Two consequences:

1. test_failure_count overstates what could have rejected the payload.
2. Worse — a non-cause can drive regression onset. first_failed_in could be set by an informing test, sending the analysis to hunt for a culprit PR behind a test that never gated anything. That is exactly what happened in the chain analysed in payload-snapshot: fix silent data loss on unauthenticated gcloud (supersedes #641) #644, where the previous analysis chased [Feature:NetworkSegmentation] UDN onset across payloads that contained zero PRs.

Informing tests are run in the wild to stabilize them; they are not expected to gate. A missing lifecycle attribute means the test does gate — the attribute exists only to opt a test out.

Measured on real data

One conformance run's JUnit from that chain (run):

	before	after
test_failure_count	11	0
test_informing_failure_count	—	11

All 11 were UDN tests being stabilized. None could have failed the job.

And the aggregated report for 5.0.0-0.ci-2026-07-25-162741 — the payload whose misattribution started all of this:

	entries	gating	informing
aggregated-aws-ovn-upgrade-5.0-major	14	3	11

The entire blocking signal was 3 tests (the CVO invariants), sitting among 14 recorded failures.

Changes

• _TestResult carries test_lifecycle; the parser reads the attribute.
• _mark_flakes() relabels a failure as a flake when the same test also passed in the same suite. Grouping is per (suite, name) deliberately: the same monitor evaluated in openshift-tests-upgrade and openshift-tests covers two different phases, so a pass in one does not clear a failure in the other. Treating those as flakes would silently discard real upgrade-phase failures — including alert/etcdNoLeader and the CVO invariants.
• results.json records all three categories with status and test_lifecycle; nothing is hidden.
• test_failure_count counts gating results only. test_flake_count and test_informing_failure_count are separate.
• summary.json gains test_failures.informing[] and test_failures.flakes[]. No onset is tracked for either — an onset implies a culprit to find.
• Regression tracking uses gating failures only, and logs what it excluded.
• payload-analysis: never score informing failures or flakes as candidates, never derive an originating payload from them, never revert for them. Report them in their own section stating they cannot cause a rejection — with the one exception worth investigating: a test that damages the cluster it runs on.

Tests

13 new (38 total in this directory), including the cross-suite case that must not be classified as a flake, absent-lifecycle-gates, informing-flake, and mixed counting. Validated against real aggregated and non-aggregated JUnit.

Scope note: what is and isn't verified by real data

The flake convention is confirmed, not assumed. openshift/origin writes a flake as two testcases with the same name in the same testsuite — one carrying a failure, one not (pkg/test/ginkgo/junit.go:118-138):

case test.flake:
   s.NumTests++
   s.NumFailed++
   failedTestCase := &junitapi.JUnitTestCase{Name: test.name, FailureOutput: ...}
   s.TestCases = append(s.TestCases, failedTestCase)

   // also add the successful junit result:
   s.NumTests++
   successTestCase := &junitapi.JUnitTestCase{Name: test.name, ...}
   s.TestCases = append(s.TestCases, successTestCase)

_mark_flakes() matches that exactly, including the same-suite grouping — s.TestCases is one suite, so a pass in a different suite is a different test.

Note also s.NumFailed++ on the flake path: even the testsuite's own failures= attribute counts flakes as failures, so there is no pre-computed count to lean on instead.

But I have not seen the flake path fire on real data. Across the JUnit I pulled while investigating this chain:

	files	testcases	same-suite duplicates	flakes
aggregated	8	63,183	0	0
non-aggregated	1	2,238	0	0

Eight of the nine are aggregated reports, which cannot contain a flake by construction — the aggregator emits one verdict per test, so a retry appears as passed N times, failed M times in the summary text rather than as duplicate testcases. That leaves exactly one file that could have exhibited a flake, and it had none.

So: the flake code path is correct with respect to the writer that produces the input, and is covered by unit tests including the cross-suite case it must not match — but reviewers should treat it as unexercised in the wild. It changes no number in this PR's before/after tables. The informing split is what measurably changes results today.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

• More reliable CI payload snapshots with explicit completeness reporting and richer collection error details.

• Added --fail-on-incomplete to exit non-zero when unrecovered snapshot data is missing.

• Bug Fixes

• Improved JUnit artifact discovery and recovery; prevent incomplete/unavailable data from being treated as zero results.

• Refined gating vs flake vs informing handling for failure semantics.

• Documentation

• Updated payload analysis and payload snapshot docs, including the incomplete/complete contract and schema.

• Tests

• Added pytest coverage for snapshot completeness and test classification rules.

• Chores

• Bumped CI plugin version to 0.0.73.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[stbenjam]

The agent is getting confused by the difference between informing jobs and tests, example from the payload-agent presubmit:

Informing Test Failures
These tests do not by themselves cause job failures or payload rejections. The only potential impact is if the test itself affects cluster health — for example if it breaks an operator or does something otherwise catastrophic.

37 of 73 informing jobs failed. Notable patterns:

[Feature:NetworkSegmentation] OVN UDN tests — Multiple failures across aggregated-gcp-ovn-rt-upgrade and metal-ipi-ovn-ipv4 (informing lifecycle, 0/9 in aggregated runs)
[sig-auth] Security Penetration Tests — Multiple informing failures in metal-ipi-ovn-ipv4

Please clarify the skill to include the difference.

[not-stbenjam]

Rebased onto main (resolved version bump conflicts) and added explicit disambiguation between informing jobs and informing tests to both payload-snapshot/SKILL.md and payload-analysis/SKILL.md.

The agent was conflating the two — reporting informing job failure counts under a heading about informing tests. Both SKILL files now include a table and explanation making clear these are orthogonal concepts:

• Informing job (informing_jobs.failed_jobs[]): a CI job that runs for visibility but doesn't gate
• Informing test (test_failures.informing[]): an individual test case with lifecycle="informing" — can appear inside any job

The payload-analysis SKILL.md also now explicitly says: "Never combine informing-job counts with informing-test lists."

[stbenjam]

/ok-to-test

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py`:
- Around line 1651-1661: Rename the mixed test_failures buckets in the payload
construction to distinguish scopes: use gating for rejectable test regressions,
and explicit test-lifecycle names such as informing_test_lifecycle and
flake_test_lifecycle for the non-gating buckets. Update all consumers, schema
definitions, and documentation referencing test_failures.blocking,
test_failures.informing, or test_failures.flakes to match the new keys while
preserving their existing data and behavior.

In `@plugins/ci/skills/payload-snapshot/SKILL.md`:
- Around line 202-215: Make the combined job/test gating predicate explicit in
the category table and instructions: only a failed test in a blocking job gates,
and it must not have lifecycle="informing"; informing tests never gate,
including those in blocking jobs, and missing lifecycle continues to mean the
test gates when its job is blocking. In
plugins/ci/skills/payload-snapshot/SKILL.md lines 202-215, align the categories
and missing-lifecycle rule with this predicate. In
plugins/ci/skills/payload-analysis/SKILL.md lines 159-180, exclude failed
informing tests and tests from informative failures from scoring, revert, and
rejection guidance.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: df7a4eb8-1ce7-445d-874e-2a9246ac30c0

📥 Commits

Reviewing files that changed from the base of the PR and between 72e8ab7 and 51ce340.

📒 Files selected for processing (4)

• plugins/ci/skills/payload-analysis/SKILL.md
• plugins/ci/skills/payload-snapshot/SKILL.md
• plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
• plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py

🚧 Files skipped from review as they are similar to previous changes (1)

• plugins/ci/skills/payload-snapshot/scripts/test_test_classification.py

[stbenjam]

/test payload-agent

[stbenjam]

/test eval-payload-analysis

[neisw]

/lgtm

[stbenjam]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neisw, not-stbenjam, stbenjam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• plugins/ci/OWNERS [stbenjam]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment