diff --git a/.github/ISSUE_SPECS/724-isolate-alias-observer-test-counts-across-parallel-fixtures.md b/.github/ISSUE_SPECS/724-isolate-alias-observer-test-counts-across-parallel-fixtures.md new file mode 100644 index 00000000..00c35db4 --- /dev/null +++ b/.github/ISSUE_SPECS/724-isolate-alias-observer-test-counts-across-parallel-fixtures.md @@ -0,0 +1,115 @@ +## Problem and authority + +Delivery repair for #709/#722 after PR #723. Reviewed head +98018830ec9a8462d22dc62d2740dc45386d1aab merged as +41ef9b7667a37427e490bfe3abf3b3cebd3e118f. Required PR qualification +34489225879 passed, but main run34490147028 workspace-debug job102914615009 +failed `serialized_alias_observer_is_the_decline_boundary`: observed12 versus +expected3 callbacks. Exact log is `/tmp/rack-meter-main-debug-failure.log`. +Do not rerun away or hide the failed main receipt. + +Independent Astra XHIGH diagnosis identifies test-instrumentation interference: +`ALIAS_OBSERVATIONS` is global, the failing test resets/reads it under the old +pair-witness mutex, and the new resident-meter fixture invokes the same alias +observer concurrently without that mutex. PCM and thread-local capture checks +passed. No production duplicate-observation defect is demonstrated. Other +fader witnesses do not require expansion: these fixture owners decline before +fused-factory increments. + +User routing: Astra LOW implementation, Astra XHIGH scoping/verification. This +is a separate bounded test-isolation correction, not reopening exhausted #714 +or expanding #722's two-expression product scope. #709/#722 are passive pending +this delivery dependency; this issue occupies the active implementation slot. +At most two active slots overall. Lane B retains pin ownership. + +## Smallest closable correction and ownership + +Only `crates/builtins-compiler/src/lib.rs`, inside its existing tests module: +- Change ALIAS_OBSERVATIONS storage/accesses to thread-local Cell beside + the already-thread-local ALIAS_CAPTURE. +- Add one inline deterministic regression using actual AliasObserver calls. + +Own this numbered spec. Preserve production bytes, observer behavior, the exact +HARNESS_BLOCKS==3 assertion, all other counters, policies, dependency contracts, +pin and qualification expectations. No production repair, broad test rewrite, +new harness framework, artifact rebuild/repin or performance campaign. + +## Discriminating regression and finite gates + +Two scoped threads synchronize after both reset their count/capture and again +after making two versus five actual alias-observer calls. Each must see only +its own count and captured samples. Put count/capture assertions after the +second barrier so the negative control cannot strand the other thread. +Restoring the old shared counter must deterministically report the combined +count7 and fail the intended assertion. Use one external physical mutation; +preserve its input/diff/command/exit/output and restore exact source. + +Astra LOW implements one coherent pass and runs formatting, the new regression +and existing alias/resident selectors. Stop on the first unexpected failure; +record actual commands, environment, exits and logs externally. Pause when +focused checks pass for root exact-path checkpoint/push and GitHub sync. + +Astra XHIGH verifies the test-only scope, deterministic regression and one old- +counter negative control, then runs the full builtins-compiler library suite +with test-support once in debug and release-unwind, plus focused strict all- +target/all-feature Clippy. Reuse prior product/artifact evidence; do not launch +a repeated browser campaign. The existing five-attempt ceiling and candid +stop/rescope rules apply; do not weaken the exact callback assertion. + +## Delivery + +The current base is origin/main41ef9b76, whose post-main qualification failed; +it is not a delivered PASS baseline. Use an isolated corrective branch and +frequent exact-path checkpoints. Independent XHIGH PASS precedes one PR. +Require exact-head PR qualification and a guarded merge, then successful +qualification of the corrected main. Existing CI must reproduce the unchanged +qualified ea8f843b artifact pin. No special bypass or blind rerun of main41ef. + +Only after corrected-main PASS synchronize this issue and #709/#722 closure. +Their records must preserve the original failed main run and name this repair +and both qualification runs. #714 remains superseded without retroactive PASS. +Remove clean delivered worktrees after preserving branches/history and external +evidence. No registry publication, speedup or full-RT10 claim. + +## Status + +Independent XHIGH scope PASS established before implementation. No correction +or rerun has been performed. Earlier source/shared-artifact PASS remains earned; +#709/#722 delivery is blocked on this test-isolation correction and required CI. + +## Attempt 1 implementation checkpoint + +Astra LOW changed only the alias count storage/accesses inside the test module +and added the deterministic two-thread 2/5 observer regression. Formatting, new +regression (1), alias selector (3), and resident-meter selector (2) passed. +Actual commands, environment, exit status and logs are preserved externally in +`/tmp/issue724-attempt1-implementation`. Cargo's generated dependency ordering +change was preserved externally and restored; no dependency change is included. +These focused commands lacked --locked; independent review uses --locked. +Root audited the exact-path checkpoint. Independent XHIGH verdict and CI remain. + +## Verification attempt 1 — cache contamination; bounded attempt 2 + +The external old-global negative control discriminated correctly: both threads +saw7 rather than2/5, exit101. The verifier then incorrectly reused its target +cache for the positive checkout; Cargo ran the mutant binary without rebuilding +(0.06s). This invalid positive result is a verifier failure, not source evidence. +Both source copies were restored/verified against accepted TLS hash41265ed3; +authoritative source remains clean. Evidence and mutant binary are preserved in +`/tmp/issue724-review-rzhy86pj`. Release/Clippy/format did not start. +Root authorizes verification attempt2 with a distinct clean positive target: +repeat only invalid full-debug, then the remaining unrun gates. Reuse the earned +once-only negative control; no source change or further mutation is authorized. + +## Independent Astra XHIGH SOURCE PASS + +Accepted source5e9fb254, documentation head754726cc2addea512faf860ea51817f4ba4b81f7. +Verification attempt2 passed full locked debug and release-unwind suites (50/50 +in each), strict all-target/all-feature compiler Clippy, and formatting. The +once-only old-global physical mutation made both threads observe7, failing the +intended2/5 assertions; source restored exactly. Test-only scope, unchanged +production/lock/pin and original three-callback assertion were independently +verified. The invalid cache-contaminated positive invocation remains preserved. +Evidence: `/tmp/issue724-review-rzhy86pj/VERDICT.md`; manifest SHA-256 +30473b1dcfe3b3d661f3ca6b1711c97450947b90d2e86a1facc5a91cbab542e7. +Corrective PR and corrected-main qualification remain mandatory before closure. diff --git a/crates/builtins-compiler/src/lib.rs b/crates/builtins-compiler/src/lib.rs index 2ffc2ffb..5e7340b8 100644 --- a/crates/builtins-compiler/src/lib.rs +++ b/crates/builtins-compiler/src/lib.rs @@ -5313,15 +5313,15 @@ mod tests { nonfinite: bool, } - static ALIAS_OBSERVATIONS: AtomicUsize = AtomicUsize::new(0); static INTERVENING_OBSERVER_ERRORS: AtomicUsize = AtomicUsize::new(0); thread_local! { + static ALIAS_OBSERVATIONS: std::cell::Cell = const { std::cell::Cell::new(0) }; static ALIAS_CAPTURE: std::cell::RefCell> = const { std::cell::RefCell::new(Vec::new()) }; } struct AliasObserver; impl GraphRuntimeObserver for AliasObserver { fn observe(&mut self, block: GraphObservationBlock<'_>) -> Result<(), RenderError> { - ALIAS_OBSERVATIONS.fetch_add(1, Ordering::Relaxed); + ALIAS_OBSERVATIONS.with(|count| count.set(count.get() + 1)); ALIAS_CAPTURE.with(|capture| { let mut capture = capture.borrow_mut(); capture.extend(block.left.iter().map(|sample| sample.to_bits())); @@ -8852,6 +8852,39 @@ mod tests { assert_eq!(paired_witness.process_members, HARNESS_BLOCKS); } + #[test] + fn alias_observer_counts_and_capture_are_thread_local() { + let barrier = std::sync::Barrier::new(2); + std::thread::scope(|scope| { + for calls in [2, 5] { + let barrier = &barrier; + scope.spawn(move || { + ALIAS_OBSERVATIONS.with(|count| count.set(0)); + ALIAS_CAPTURE.with(|capture| capture.borrow_mut().clear()); + barrier.wait(); + + let left = [calls as f32]; + let right = [-(calls as f32)]; + let mut observer = AliasObserver; + let mut results = Vec::new(); + for first_sample in 0..calls { + results.push(observer.observe(GraphObservationBlock { + left: &left, + right: &right, + first_sample: first_sample as u64, + })); + } + barrier.wait(); + + assert!(results.into_iter().all(|result| result.is_ok())); + assert_eq!(ALIAS_OBSERVATIONS.with(std::cell::Cell::get), calls); + let expected = [left[0].to_bits(), right[0].to_bits()].repeat(calls); + ALIAS_CAPTURE.with(|capture| assert_eq!(*capture.borrow(), expected)); + }); + } + }); + } + #[test] fn serialized_alias_observer_is_the_decline_boundary() { let _guard = PAIR_WITNESS_LOCK @@ -8867,7 +8900,7 @@ mod tests { ); let eligible_witness = test_only_fader_matrix_witness(); test_only_reset_fader_matrix_witness(); - ALIAS_OBSERVATIONS.store(0, Ordering::Relaxed); + ALIAS_OBSERVATIONS.with(|count| count.set(0)); let (observed, _, _, _) = render_post_input_bits_with_variant( 9, Backend::Simd8, @@ -8877,7 +8910,7 @@ mod tests { ); assert_eq!(eligible, observed, "alias observer preserves PCM words"); let observed_witness = test_only_fader_matrix_witness(); - let observed_calls = ALIAS_OBSERVATIONS.load(Ordering::Relaxed); + let observed_calls = ALIAS_OBSERVATIONS.with(std::cell::Cell::get); let paired_alias = ALIAS_CAPTURE.with(|capture| capture.borrow().clone()); let (separate_observed, _, _, _) = render_post_input_bits_with_variant( 9,