test(engine): confirm pegging alternate/routing output is env-order-unstable (E2 finding)#17
Merged
Merged
Conversation
…nstable (E2 finding) E2 pegging-coverage investigation (engine review H4). Outcome: the 3 smoke-only pegging tests (pegging_4/5/7, the alternate/routing models) CANNOT be byte-exact golden as-is - their pegging-report ordering is environment-dependent, not just a missing baseline. Documented with the verified root cause; the original maintainers' "platform-sensitive" call is confirmed and strengthened. Verified (containerized ubuntu:24.04 + the GitHub runner): - deterministic WITHIN an environment (30x identical), but - the operationplans() iteration order varies ACROSS environments - Docker Release vs Debug+ASan, and the GitHub ubuntu-24.04 runner, all differ (same content, reordered blocks); PYTHONHASHSEED-independent; single-threaded (loglevel>0 forces one thread, so not a thread race). - An attempt to capture golden .expect for pegging_4/5 passed in two Docker builds but FAILED on the GitHub runner - proving the instability is environment-wide, not config-specific. Change: tighten the smoke-only rationale comment in each pegging_4/5/7.xml with this evidence + the fix path (a deterministic tiebreaker in the pegging iterator/output - an engine change), and record the finding + blocked status in MODERNIZATION_PLAN E2. No flaky golden tests are added.
dnplkndll
force-pushed
the
e2-pegging-golden-coverage
branch
from
95a811b to
cd94dbc
Compare
dnplkndll
changed the title
Self-review of this branch: the verified pegging-ordering finding was duplicated verbatim (9 lines) across all three pegging_*.xml. Centralize it. - engine-review-E1.md H4: record the confirmed E2 finding in the canonical findings doc - deterministic per-environment but reorders across Docker Release/Debug + the GitHub runner; single-threaded; PYTHONHASHSEED-independent; the Docker-pass/GitHub-fail proof; and the two fix options (engine-side stable sort vs test-side content-keyed sort in the output block). - pegging_4/5/7.xml: replace the duplicated 9-line rationale with a 3-line smoke-only note that points to engine-review-E1.md (H4). - MODERNIZATION_PLAN E2: tighten the entry, reference the review doc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome (corrected after CI)
The E2 pegging-coverage investigation (engine review H4). I attempted to convert the 3 smoke-only pegging tests (pegging_4/5/7) to golden, CI proved they can't be as-is, and this PR ships the rigorous verified finding instead of flaky tests.
What I found
The pegging-report ordering for these alternate/routing models is environment-dependent, not just a missing baseline:
ubuntu-24.04runner all produce different orderings (identical content, reordered blocks);How CI proved it
I captured golden
.expectfor pegging_4/5 and verified byte-stability across two Docker builds (Release + Debug+ASan) and repeatedruntest.pyruns — passed locally. Pushed → the GitHub runner build failed on exactly pegging_4 + pegging_5 (Difference in output 1). That cross-environment failure is the proof: the instability is environment-wide, validating (and strengthening) the maintainers' original "platform-sensitive" comment. I reverted the.expect.What ships
pegging_4/5/7.xmlwith the verified evidence + the fix path.MODERNIZATION_PLANE2..gitignore: add.DS_Store(hygiene — it had snuck into the build context).The real fix (follow-up)
Byte-exact golden coverage for these (and a future >=3-level BOM + cycle case) needs a deterministic tiebreaker in the pegging iterator / output — an engine change with cross-golden-output implications, so it is scoped as its own task, not smuggled into a test PR.
No UI surface, so no screenshots. Per convention, not auto-merging.
Self-review pass (3rd commit)
Bugbot-style review of this branch found the finding was duplicated verbatim (9 lines) in all 3
pegging_*.xml. Fixed:engine-review-E1.mdH4 (the canonical findings doc), incl. both fix options (engine-side stable sort vs test-side content-keyed output sort).pegging_*.xmlnow carries a 3-line smoke-only note pointing to it..DS_Storeis tracked anywhere (the.gitignoreadd is sufficient).