Skip to content

FIX: stop flaky wall-clock margin in hierarchy budget-exhaustion test - #3680

Open
Pushpak Siva Sai (Pushpak731) wants to merge 1 commit into
microsoft:mainfrom
Pushpak731:fix/3671-flaky-timing-assertion
Open

Pushpak Siva Sai (Pushpak731) wants to merge 1 commit into
microsoft:mainfrom
Pushpak731:fix/3671-flaky-timing-assertion

Conversation

@Pushpak731

Copy link
Copy Markdown
Contributor

Description

test_hierarchy_traversal_halts_immediately_on_budget_exhaustion asserted the bounded traversal completes in under 0.05s of wall clock. That margin has no tolerance for runner contention and has flapped on main repeatedly (#3671):

  • main run 41014283717 (ubuntu 3.13): failed with Expected <0.05s, got 1.7288s
  • main run 41012598348: identical commit passed
  • PR run 40810357178 (macOS 3.12): failed at 0.0539s — an 8% overshoot — while passing on every other OS/Python combination in the same run

Why the bound can't be saved by choosing a better number

I measured the two regimes the test tries to separate, on the same 1,000-root fixture the test builds:

Work Wall clock
bounded traversal (max_nodes=1, current behavior) ~6 ms
full build of all 1,000 nodes (max_nodes=1000) ~20 ms

Even building every node costs ~20 ms, while contended runners have measured 1.7 s for the same call — the noise is ~85x the full-work cost. At this scale no wall-clock bound can both tolerate CI scheduling noise and catch a quadratic regression, so the timing assertion cannot be the regression guard.

The fix

  • The halt stays proven structurally, exactly as before: the projection contains exactly one node (node_count == 1), is flagged truncated, and keeps the exact aggregate (overall_counts.completed == 1000). A traversal that ignored the budget cannot produce that projection.
  • The wall-clock assert becomes a generous 5s ceiling that keeps some teeth — a hung or catastrophically regressed traversal fails in seconds with a clear message instead of waiting for the job to time out — while sitting ~3x above the worst contention ever observed, so it cannot flake.

Validation

  • Full file locally: tests/unit/backend/test_scenario_hierarchy_parity.py — 84 passed.
  • ruff check / ruff format --check clean.

Closes #3671

The 0.05s assertion had no tolerance for runner contention and flapped
on main (1.73s on a contended ubuntu runner in run 41014283717, an 8%
overshoot on macOS in run 40810357178). The halt itself is already
proven structurally — exact node count of 1, the truncated flag, and the
exact aggregate — and wall clock cannot guard the timing part at this
scale: the bounded traversal takes ~6ms and a full build of all 1,000
nodes only ~20ms, so contention noise (~1.7s observed) is roughly 85x
the full-work cost and no tight bound separates 'immediate' from
'quadratic'. Replace it with a generous 5s ceiling that still fails fast
on a hung traversal instead of waiting for the job to time out.

Closes microsoft#3671.
Copilot AI balanced review requested due to automatic review settings October 30, 2026 17:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

This branch has not been deployed

No deployments
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.

Flaky timing assertion in test_hierarchy_traversal_halts_immediately_on_budget_exhaustion fails on main CI

3 participants