test(e2e): tolerate intermittent EAI-7423 XPASS on the Strix Halo Ubuntu lane (EAI-7853) - #182
Conversation
…ntu lane (EAI-7853) The lemonade managed-serve bug EAI-7423 no longer reproduces reliably on the Strix Halo Ubuntu runner: which of its six xfail'd scenarios pass varies run to run (2, 3, and 6 XPASS across three recent runs). Because those rows are deterministic xfails, the reconciler reports stale XPASS and fails the lane on PRs that touch neither serve nor chat. Mark the EAI-7423 lemonade-linux rows `flaky`, which is what that flag is for: either outcome is tolerated while the bug stays open, matching how the EAI-7333 rows already handle an intermittent known bug. This deliberately does not claim EAI-7423 is fixed — the inference scenarios still fail on some runs, so the bug stays open and the rows stay in place. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
volen-silo
left a comment
There was a problem hiding this comment.
Reviewed the diff, the reconciler it depends on, and the CI history behind the claims. No blocking issues — the mechanism does what the PR says, the row selection is exactly right, and the fix is demonstrated on the lane it targets. Everything below is non-blocking, and most of it is about the note left behind rather than the change itself. Leaving the formal approve to a maintainer.
Verified
flakysemantics match the claim.tests/e2e-cucumber/tests/e2e.rs:905-916routes a flaky XPASS intoflaky_xpass; the exit gate at:938fires only onstale_xpassorunexpected_fail. So a flaky XPASS is logged and tolerated, exactly as described.- Row selection is exactly right. Parsed the file rather than reading the diff: 13 rows, exactly 6 carry
bug = "EAI-7423", all 6 are nowflaky = true, and the only other flaky rows are the two pre-existing EAI-7333 ones. No EAI-7423 row missed, no unrelated row touched. - The right row wins where a scenario id has two.
serve-readiness-contract(vLLM/EAI-7333 vs lemonade/EAI-7423) andchat-tool-definitions-accepted(vLLM/EAI-7223 vs lemonade/EAI-7423) both resolve to the lemonade row on Strix Ubuntu — the conditions are mutually exclusive oneffective_engine, so declaration order is inert and the newly added flags are the ones that take effect. - Schema-safe.
XfailEntryis#[serde(deny_unknown_fields)]withflakya declared#[serde(default)]field, so a typo would have failed the parse rather than silently no-op'd. The round-trip into the aggregate report is safe (skip_serializing_if+ matching#[serde(default)]). flakyhas no side effects beyond XPASS tolerance — it does not touchserve_timeout_for(),is_xfail(), scenario selection, or retry behaviour. Relaunch-on-stall isn't a confound either:serving_steps.rs:437gives xfail scenarios one attempt, and all six are xfail.- The "intermittent, not fixed" premise is evidenced on this PR's own run —
7 xfail, 3 XPASS (3 flaky, 0 stale): three of the six still failed in the same run that XPASSed the other three. - No product fix has landed that would make deletion the correct action instead — #150 and #177 are both still open (#177 draft). Every CI claim in the description checks out against the cited runs. CI on this PR is currently all-green.
1. The tracking note points at a conclusion this CI setup can't actually test
tests/e2e-cucumber/expectations.toml:52-54
The note says: "if the lemonade 11.5.1 upgrade turns out to fix EAI-7423, drop these rows outright." The only evidence for that is run 30939967076 (#177, 6/6 XPASS), and two things undercut it:
- #181 and #174 touch no lemonade, serve, lifecycle or
ci.ymlfiles, sit on unchangedmain, and show the same partial intermittency (3 and 2 XPASS) in the same 36-hour window. A version bump present in neither can't explain that. - The lemonade install is cached across jobs and the cache is version-unaware.
engines/lemonade/src/lib.rs:34still pinsLEMONADE_VERSION = "10.10.0"on this ancestry, and the extraction gate inprepare_embeddableis a pure file-existence check with no comparison against the stored manifest version:The install root lives inside the active TheRock runtime tree, which on CI is the shared, cross-job-persistentif reinstall || !lemond_path_in(&runtime_dir).is_file() {
e2e-prewarmdirectory — so once any job populatesengines/lemonadewith a workinglemond, later jobs reuse that binary regardless of the compiled version.
So run 30939967076 may not have exercised an 11.5.1 binary at all. A maintainer following this note could delete six live rows on an unfounded basis — converting real EAI-7423 failures into unexpected failures on unrelated PRs, which is the outcome the Scope boundary section says it wants to avoid.
I could not confirm which binary ran on 2026-08-04/05: serving_steps.rs prints "Preparing Lemonade embeddable {VERSION}…" only inside a failing scenario's diagnostic dump, and those scenarios passed. (The 2026-08-03 job, with all six failing, does show 10.10.0.) That observability gap is itself why the attribution can't be settled.
Suggest keeping the EAI-7853 pointer but dropping or qualifying the 11.5.1 attribution. Separately and out of scope: that gate should compare the stored manifest version against LEMONADE_VERSION, otherwise no future "did the upgrade fix it?" question is answerable from this lane.
2. This removes the only automated signal that these rows have gone obsolete
expectations.toml — the six flaky = true additions
Stale XPASS is the sole obsolescence detector in this system. Once a row is flaky its XPASS is classified FlakyXpass (crates/e2e-report/src/lib.rs:669), excluded from is_problem() (:679-684), counted as a pass in the consolidated tally, and rendered with a green ✅XPASS (flaky) (:695). Nothing tracks "this flaky row hasn't failed in N runs". The advisory xfail-hint job only nudges at authoring time when a PR's text names a ticket with a live row; it never inspects CI results.
Run 30939967076 turning the lane red is why anyone noticed the six had started passing. After this PR the identical run reports 6 XPASS (6 flaky, 0 stale) and passes silently. That's an acceptable trade for the signal-quality win, but it means the revisit trigger the PR relies on is the thing the PR removes — and combined with item 1, the trigger is both unobserved and aimed at an untestable hypothesis. Worth stating in the PR body and putting a date or an owner on the revisit.
3. The file's own schema header is now false for most of its rows
expectations.toml:13 — "a scenario that then PASSES is an XPASS (stale entry — remove it)", and the grammar block at :16-22, which lists when / bug / reason / serve_timeout_secs and never mentions flaky.
After this PR 8 of 13 rows are flaky, so that rule is wrong for the majority of the file it heads. Pre-existing — the gap dates to the commit that introduced flaky, which updated tests/e2e-cucumber/README.md:109-114 instead (the README is correct) — but this PR takes it from 2/13 to 8/13 while editing that same file two lines below. A contributor reading the header in place will delete a flaky row on its first XPASS. One grammar line for flaky plus an "unless the row is flaky" qualifier on :13 would close it.
4. The new comment block floats between two rows
expectations.toml:48-54
Every other comment in the file abuts the table it describes with no intervening blank line. This one has a blank line on both sides (:47 and :55), sitting between the EAI-7333 serve-vllm-inference row that ends at :46 and the per-row comment for serve-default-engine-working-endpoint starting at :56 — so a reader skimming top-down can reasonably attach it to the EAI-7333 row above. Deleting the blank at :55, or hoisting the note into the file header, would fix it.
Also, "every EAI-7423 lemonade-linux row below is flaky = true" asserts an invariant over six non-contiguous rows interleaved with EAI-7333/7223/7383/7384 rows, with nothing enforcing it — flaky is read per-row and never derived from bug. A seventh EAI-7423 row added later would silently not be covered and the comment would become quietly wrong.
5. Dated live-status claim with an unnamed host
expectations.toml:48 — "INTERMITTENT on this host as of 2026-08-05". This file is shared across four platforms and its other comments name hosts explicitly ("Strix Halo Ubuntu" at :56, "Strix-Windows" at :61). "this host" is resolvable only from PR context. Unlike :58's "root-caused on the box 2026-07-15" — a fixed historical fact that can't rot — a dated current-status claim goes stale with nothing to flag it.
Two small things in the PR text
- "The lane currently fails on PRs…" is true, but all three GPU lanes are
continue-on-error: trueand documented as deliberately non-blocking. This is a signal-quality fix, not an unblocking one — one clause would stop reviewers over-reading the urgency. cargo test -p e2e-cucumber --lib expectationpasses 17 tests (I re-ran it), but none of them referenceflaky, and the reconciler has no tests at all. The onlyflakyunit tests live in a crate that command doesn't run (crates/e2e-report/src/lib.rs:2290,:2380). The real evidence here is the lane result, which is strong on its own.
Adjacent, out of scope
tests/e2e-cucumber/features/model_serving.feature:49 — serve-large-model-inference (@requires-gpu @nightly, no engine pin) resolves to a real lemonade managed serve on a lemonade-default host and has no row in expectations.toml at all. If EAI-7423 bites the nightly Strix Ubuntu lane it surfaces as an unexpected failure. Arguably that's desirable; noting it only because it's the same failure mode in the same blast radius.
Not verified: whether reusing flaky for "the bug may already be fixed but we can't tell" is the right semantic versus a distinct expiry/quarantine marker — that's a design question for whoever owns the expectation system, not something I could settle from the code. Ticket contents (EAI-7423/EAI-7853) are on an external tracker I can't reach, so those claims are unchecked.
Review follow-up on the EAI-7423 flaky rows: - Document `flaky` in the file header grammar and qualify the "an XPASS is a stale entry, remove it" rule, which was false for 8 of 13 rows. - Drop the "if the lemonade 11.5.1 upgrade fixes EAI-7423, delete these rows" note. The install gate is a file-existence check with no version comparison and the install root persists across jobs, so which lemonade build a run used is not readable from this lane — the attribution was untestable and acting on it would have deleted six live rows. - Name the host (Strix Halo Ubuntu) instead of "this host", state that `flaky` is per-row and not derived from `bug`, and attach the note to the rows it describes. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
|
Thanks — this was a genuinely useful review, and item 1 caught a real hazard. Addressed in b9dca0a plus a description rewrite. 1 — 11.5.1 attribution is untestable. You're right, and I re-verified it rather than taking it on faith: Dropped the "if 11.5.1 fixes it, delete these rows" sentence entirely rather than qualifying it — a note that could lead a maintainer to delete six live rows on unfounded evidence isn't worth keeping in weakened form. The EAI-7853 pointer stays, now with the reason deletion isn't currently decidable from this lane. The version-aware gate is tracked separately; agreed it's out of scope here. 2 — this removes the only obsolescence detector. Correct, and the framing that the PR removes its own revisit trigger is the sharpest point in the review. Accepted as a trade rather than solved, but no longer implicitly: the description now has a "Tradeoff" section stating it outright, with the revisit owned by me and dated 2026-09-30 under EAI-7853. A "flaky row hasn't failed in N runs" detector is the real fix and needs to be designed, not bolted on here. 3 — header false for most rows. Fixed. 4 — floating comment block. Fixed — the blank line is gone so the note abuts the rows it describes. Also softened the invariant claim: it now says the flag is per-row and not derived from 5 — "this host". Fixed — names Strix Halo Ubuntu, and the status claim is now past-tense-dated rather than a standing assertion. PR text. Both corrected: the description now says up front that the lane is Adjacent — On the open design question — whether Not resolving your threads — leaving that to you. |
Summary
The
E2E tests (Strix Halo, Ubuntu)lane currently fails on PRs that touchneither serve nor chat — including #174 and #181 — with
0 unexpected failure(s). Nothing regressed: the reconciler is failing on stale XPASS.That lane is
continue-on-error: truelike the other two GPU lanes, so this isa signal-quality fix, not an unblocking one — a red X on unrelated PRs trains
reviewers to ignore the lane.
Six scenarios are marked expected-to-fail against EAI-7423 (lemonade managed
serve reaches ready, then shuts down ~0.08s later on gfx1151). That bug no
longer reproduces reliably on the runner, and — importantly — not consistently
either. Which scenarios pass varies run to run:
Change
Mark the six EAI-7423 lemonade-linux rows
flaky = true. That flag exists forexactly this state — "tolerates either result while the intermittent bug remains
open" — and the EAI-7333 rows already use it for the same reason on the vLLM
path.
This does not claim EAI-7423 is fixed. The inference scenarios still fail on
some runs, so the bug stays open and the rows stay in place; only the
deterministic-XPASS failure mode goes away. Removing the rows instead would
convert those runs into unexpected failures the next time the bug bites.
Tradeoff: this removes the obsolescence detector
Stale XPASS is the only automated signal that a row has outlived its bug. Once a
row is flaky its XPASS is counted as a pass and rendered
✅XPASS (flaky);nothing tracks "this row hasn't failed in N runs". Run 30939967076 turning the
lane red is precisely how anyone noticed these six had started passing — after
this PR the same run passes silently.
That is an accepted trade for the signal-quality win, but it makes the revisit a
scheduled action rather than an event: tracked in EAI-7853, owned by me, to be
re-examined by 2026-09-30.
Scope boundary
The right long-term answer may be to delete the rows outright, but only once
something establishes EAI-7423 is genuinely fixed — and that is not currently
answerable from this lane. The lemonade install gate is a file-existence check
with no comparison against the stored manifest version, and the install root
persists across jobs, so a run can reuse an older
lemondthan the compiledpin. An XPASS streak here therefore does not attribute to any particular
lemonade build. The version-aware gate is tracked separately; this PR only drops
the untestable attribution from the TOML note.
Test plan
cargo test -p e2e-cucumber --lib expectation(17 passed) and the file parsesas TOML. Note these are pre-existing resolver tests — none exercise
flaky;the
flakyreconciliation tests live incrates/e2e-report.XPASS scenarios as
flakyrather thanstale, and passes. The run also shows7 xfail, 3 XPASS (3 flaky, 0 stale)— three of the six still failed in thesame run, which is the evidence the bug is intermittent rather than fixed.
Risk
Low. Data-only change to expected-failure metadata; no product or test code. The
worst case is that a genuine EAI-7423 failure is tolerated on that lane rather
than reported — which is already true of the xfail rows themselves, since they
expect failure.