feat: preserve typed work-batch results during admission stop - #177
Conversation
Add an explicit expected-result adapter over the existing owned-worker scheduler. Preserve per-index original success/error/exception and not-started state; failures outrank concurrent cancellation and block the caller's successor gate. Existing scheduler, compiler and CLI callers stay unchanged; no process kill token or lease authority. Extend the existing scheduler executable with 46 deterministic checks and add six fixed real MSVC two-worker cases using actual compiler recipes, preserved diagnostics and fixture-only link/commit barriers. VERSION5.5.0 and historical runtime debts unchanged.
Real two-worker result propagation verified from original artifactsExact head 6/6 cases pass on the first attempt. Original toolchain is MSVC14.51.36231/Hostx64-x64,Windows10.0.26100,PowerShell7.6.5,runner image20260824.214.3. Read-only audit checked all18original compile/link/run result files and36stdout/stderr files, six native reports, exact plan/source identity, two unique retained compiler identities per case, and actual scheduler counts/flags.
Each stopped case has the third slot explicitly not_started, no third result/diagnostic and no successor files/calls. Both complete cases actually link and run successfully and create the fixture-only sentinel. Failure-cancel cases keep All six cases witness two live real cl.exe handles with exact compiler image checking before the stop decision and retain their creation identities; both observed compilers have exited when the joined result is examined. All native compiler results have cancelled=false, and the forwarding runner rejects a stoppable ProcessSpec. This is not an in-flight RPC or shared-service quiescence proof. The deliberately trailing static_assert is not a PDB/C1041 historical reproduction. Independently compared12complete/cancel/failure source pairs (only the declared trailing static_assert removed for comparison) and12corresponding planned-recipe argv lists, normalizing only case directories. Fixed12000-function active inputs unchanged. All18planned argv records retain effective /Zi, /FS, /bigobj and correct /MTd or /MT, and native reports confirm actual compiler PDB creation. The existing default /Z7 precedes /Zi; the record is not falsely described as lacking /Z7 or as executing /Z7 instead. These are recipe-derived argv records, not separate OS command-line telemetry. This closes the six-case real batch gate only. Full native Debug/Release and independent ABBA/default/private reviews remain required before merge. The successor sentinel is a fixture control, not actual production cache publication. |
Iviesever
left a comment
There was a problem hiding this comment.
Final review — accept opt-in typed batch outcomes; production pipeline integration remains separate
Implementation self-review with separate read-only source reconstruction, artifact inspection and paired-statistics replay; not an independent human approval.
Exact base: 8e4c9eef2ab0479c43e0448421baea53ac699362.
Reviewed head: 2088e6afe0e6473aa88b96b14ce2d0ccdc1f69de.
Candidate tree: 8e84f920b1f3f57b64a651a34ffa5112a619f9bb.
Native test-merge: 5c466aff88d4ab1fdc478560665273a7ee03d61f, identical tree and exactly those base/head parents.
Complete seven-file diff reviewed: 821 additions/1 deletion;409base files,407byte-identical,2modified and5new,414candidate files. Canonical source archives reconstruct both trees and the exact patch. This is a new public product header plus tests, not a test-only change. No existing product .cpp, scheduler implementation, compiler, ProcessSpec, production caller, manifest,79-executable native inventory, benchmark, CLI/freshness or VERSION change.
API review and bounded guarantee
run_work_batch preallocates one index-owned variant slot and composes the accepted owned-worker scheduler; it does not implement a second scheduling loop. Each entered callback stores its original expected result or exception_ptr. A returned error, callback exception, setup exception, scheduler error or inconsistent/incomplete report cannot become all_succeeded. Original failure is considered before external admission-stop status, retaining successful siblings and explicitly unentered slots.
Nothrow-movable expected payloads prevent a secondary storage move from replacing the original result. When no external stoppable token exists, a private stop source deliberately selects the existing RAII worker path; the legacy nonstoppable scheduler and current callers are unchanged. Callback exceptions are saved and rethrown so scheduler metadata survives too. Setup/dispatch exception_ptr is separate from the scheduler's typed error. Allocation is O(requested items); its failure is retained without launching work. The report cannot be read concurrently while workers write it; external/detached work and shared-service writers are outside its completion contract.
The predecessor gate does not establish freshness, project commit authority, process-tree quiescence or safe write-lease transfer. No existing production pipeline uses it yet. Partially launched OS-worker startup failure is not fault-injected here; allocation failure and source-level RAII inspection are not that runtime evidence.
Actual current-head validation —15 triggered workflows completed
- Native Debug #731/34426468999: actual Debug MQB/library build, all four paired shard jobs, ambient/include/runtime/parameter, freshness/side-output checks and final gate passed. Inspected actual compile/link/run/PASS for
bounded_work_batch_cases 46 checks passedin job102714136189, subshard3/8. - Native Release #619/34426468958: Stage0, all four Release shard jobs, full self-host, runtime-only staging and validation of the exact packaged artifact passed. The same46checks actually ran in job102714039788, subshard3/8. publish-release skipped.
- Typed MSVC Batch #1/34426468957: actual PowerShell parsing and native fixture build, followed by six fixed cases on a separate VM, passed.
- Default Endpoint #23/34426468875 and **Private Ownership #25/34426468949:**70/70 and42/42complete, all required positive controls passed;150/150ownership-record expectations match.
- Performance #590/34426468972: actual exact candidate/base checkout, both Release builds, instrumentation verification, all19x4ABBA and upload passed.
- Documentation#180, Cross-Stage#208 and the seven triggered incremental/PCH/compdb/build-plan/module-inspection workflow statuses completed successfully. Their green statuses are not presented as a separate full raw-artifact audit.
Accepted File Event/PDB Open investigation workflows were not triggered by this diff. Their source and dependent product paths remain unchanged; they are not labeled rerun/passed on this head. No post-submission source correction, same-head native/CI/benchmark retry or sampled-until-green sequence occurred.
Real two-worker outcomes and successor gates
Full six-case original-result and executable-provenance inspection.
| Configuration/mode | Typed outcome | Original compiler exits | Entered items | Link/run/fixture commit |
|---|---|---|---|---|
| Debug complete | succeeded | 0,0,0 | 3 | 1/1/1 |
| Debug cancel | cancelled | 0,0 | 2 | 0/0/0 |
| Debug failure-cancel | failed | 0,2 (C2338) | 2 | 0/0/0 |
| Release complete | succeeded | 0,0,0 | 3 | 1/1/1 |
| Release cancel | cancelled | 0,0 | 2 | 0/0/0 |
| Release failure-cancel | failed | 0,2 (C2338) | 2 | 0/0/0 |
Read all18original compiler/link/run result files and36stdout/stderr files. Both failure-cancel cases preserve CompilerErrorCode::compilation_failed, original nonzero ProcessResult and exact static_assert marker while admission_stop_observed is true. Successful sibling results survive. All four stopped cases keep item2 not_started, no third result/diagnostic, no link/run call and no output executable or fixture commit sentinel. Both success cases actually link/run and commit the fixture sentinel; the gate is not validated merely by suppressing everything.
All six cases retain two distinct observed live cl.exe handles, expected image and process creation identities before stop, and observe completion before accepting the joined report. The forwarding runner rejects any stoppable ProcessSpec; native results all say cancelled=false. This is not an in-flight RPC or all-service-writer completion claim. No historical C1041 reproduction is claimed for the trailing static_assert.
The twelve source comparisons and twelve planned-argv comparisons differ only by the declared injection and case directories. All18planned recipes retain effective /Zi,/FS,/bigobj, case PDB and correct /MTd or /MT; actual PDB creation is checked. The builder's original /Z7 precedes /Zi and is not falsely described as absent. These are recipe-derived argv, not independent OS command telemetry. Actual MSVC14.51.36231/Hostx64-x64,Windows10.0.26100,PowerShell7.6.5,runner image20260824.214.3.
The link/run/sentinel barrier is fixture-only. production_pipeline_integrated=false and safe_to_transfer_write_lease=false remain explicit. Production target/cache integration is the next separate step, not retrospectively claimed here.
Original ownership controls and negative evidence
Default70 retains all14single-worker scheduler and14direct-drain references. Independently compared56source files and70argv records across those pairs after only directory normalization, and inspected native seven-check helper/refusal outputs.150synthetic expectations match113accepted/37rejected with exact CRLF collector hash. Default and batch input executable bytes separately hash-match their own source/provenance records.
Original default retains four adverse managed-ending B failures, all A-started normal endings: Zi Release, PCH Release, Modules Debug and Modules Release, with C1090/original B0+B1 exit2. Failed link/run remain unattempted and recovery0 does not replace them. Private B builds happen to succeed, but across both matrices all28A-started managed endings still kill the original service. Neither this API nor the current absence of private B failures authorizes that policy.
Audited715default/417private tool results and1430/834diagnostic files. B-resource identity is observed16/70default and0/42private; absent/empty snapshots are not a no-writer certificate.912/540binary-hash inventory entries are post-case records; generated binary bodies are excluded from diagnostic ZIPs and these are not failure-instant snapshots.
Independent unchanged ABBA — full19scenarios
Artifact 10133051028,35715bytes,SHA256 a8d83eb9450d09165635e88edfc6c43c1c97c50b82ef8fb930825b205cec2057. Recomputed all76pairs, per-pair arithmetic, paired medians/MAD/nearest-rankP95, marginal medians and AB/BA/AB/BA order. All72instrumented complete counters/breakdowns/compile-link-archive hit-miss identities match. Four timings-off external counter sets are unavailable, not zeros. Only the final row uses external elapsed; other rows use mqb.timings. Four-pairP95 is the maximum, not a population-tail estimate.
| Scenario | Paired median Δms | Paired median Δ% | MAD ms | Paired P95 Δms | Slower |
|---|---|---|---|---|---|
| cold | −129.045 | −6.60 | 206.180 | +124.703 | 2/4 |
| no-op | −0.271 | −6.22 | 0.118 | −0.063 | 0/4 |
| single-tu | −235.862 | −44.41 | 224.361 | +1.272 | 1/4 |
| public-header | −7.300 | −5.33 | 6.289 | +2.576 | 1/4 |
| build-run | −16.266 | −32.67 | 17.957 | +2.799 | 2/4 |
| link-only | −10.823 | −13.74 | 19.686 | +283.626 | 1/4 |
| target-scale-cold | −1190.258 | −20.63 | 995.507 | −188.847 | 0/4 |
| target-scale-no-op | −4.467 | −12.34 | 2.085 | −1.579 | 0/4 |
| target-scale-no-op-auto | −2.353 | −6.96 | 3.424 | +7.665 | 1/4 |
| target-scale-no-op-j1 | +1.926 | +4.58 | 3.937 | +6.457 | 2/4 |
| target-scale-common-header-no-op | −1.495 | −3.88 | 2.322 | +0.869 | 2/4 |
| target-scale-single-tu | −9.350 | −3.38 | 26.987 | +20.974 | 2/4 |
| discovery-cold | −182.035 | −9.89 | 152.197 | +100.618 | 1/4 |
| discovery-no-op | −0.254 | −3.36 | 0.083 | −0.143 | 0/4 |
| discovery-header | −1.477 | −0.95 | 29.683 | +48.324 | 2/4 |
| modules-cold | −182.484 | −9.74 | 153.639 | +28.883 | 1/4 |
| modules-no-op | −0.096 | −1.99 | 0.904 | +2.172 | 1/4 |
| timings-enabled-no-op | +0.030 | +0.85 | 0.036 | +0.087 | 3/4 |
| timings-disabled-no-op (external) | +0.339 | +3.16 | 0.788 | +1.363 | 2/4 |
Unrounded external paired median +0.3388ms/+3.1622469% does not cross the predeclared BOTH>1ms AND>10% regression gate. No speedup/zero-overhead claim: the new public header is not used by the benchmarked production callers. Preserve adverse link-only+283.626ms,scale-j1 median+1.926ms and all other adverse/favorable samples. Favorable scale-cold is not an attributed optimization. No noise-based deletion, AA subtraction or closure of historical #170/C1041/cold-tail debt. Comparison JSON does not archive an OS trace or actual performance binary pair.
Artifact integrity, decision and handoff
Eight current core ZIPs were CRC/SHA-checked, with exact source archives, patch replay and read-only reproductions. New batch10133041291 SHA256 57c2794b7ea7047acd577da21aded5691556e1dc53b570f39f6545add275de94; default10133146874 86df6504515a8cbbd9b24b06a0fc827b88146786160b9cedea21f861386c3eed; private10133080655 ee58c8933b18a41d24f049b2e8167f893ed1f1d981f5add6f81d056cd27883ea. Build, both exact-input ZIPs, ownership contract and ABBA are also retained. Actual batch probe hash is 972a81c58ded844748b300f6bfabfb95b7cf3982af12aacc98a5ca5e6a0cc016; observer inputs are not the performance pair.
Local GCC14.2/C++23 strict+ASan/UBSan executed the actual46checks and full scheduler test with platform declaration/backend shims, and rejected the cancellation-before-failure mutation. Initial local stub exception-specification/Win32-declaration issues remain logged; no local MSVC/PowerShell/ETW execution is claimed. Windows native compile/run logs supply that separate evidence. Only explicitly labeled native log excerpts, not the complete CI logs, are retained in this delivery.
Accept ordinary expected-head merge of 2088e6afe0e6473aa88b96b14ce2d0ccdc1f69de, subject to unchanged main and protection. This accepts an explicit public result adapter plus its native two-worker proof, not production pipeline adoption. Keep VERSION5.5.0, #172 held, M1b incomplete and historical runtime debts open. No process-token propagation, writer lease, build/run/session/residency or formal release authority is included.
Next separate small PR: adopt typed results at an explicit compile-wave/target boundary, preserving current no-token hot path, inspection/execution evidence and freshness semantics. Original compile failures must block downstream target link/cache publication; unentered work cannot be treated as a successful prerequisite. Test actual production-boundary behavior rather than relabeling this fixture sentinel as cache integration. Partial worker-start failure, shared-service ownership and project write transactions remain separate dependent gates.
Actions retention30days; delivery contains the eight core artifacts, not every artifact from all15workflows. Title/ready-triggered skipped performance jobs do not replace actual#590; new post-merge push CI is not claimed completed by this review.
Final decision: accept opt-in typed batch outcomes; production integration stays separate
Final exact-source review, real two-worker results, full19-rowABBA and acceptance decision
Accept ordinary expected-head merge. This is a new public product interface with actual native validation, not merely test-only code. Existing product callers and CLI behavior remain unchanged. Historical C1041/performance debts, #172 HOLD and M1b's remaining gates are not resolved by this decision.
Exact source
8e4c9eef2ab0479c43e0448421baea53ac699362.2088e6afe0e6473aa88b96b14ce2d0ccdc1f69de.8e84f920b1f3f57b64a651a34ffa5112a619f9bb.5c466aff88d4ab1fdc478560665273a7ee03d61f, exactly those base/head parents.Product result contract
run_work_batchaccepts callbacks returning nothrow-movablestd::expected<T,E>by value, and composes the existing owned-worker scheduler. Preallocated per-index slots keep not-entered/original expected/original exception_ptr, plus separate scheduling result and setup/dispatch exception. Successful sibling payloads survive another callback's failure. Original returned errors and exceptions take precedence over concurrent external admission cancellation; pending work never becomes successful.When no external stoppable token is supplied, a private stop source selects the existing RAII/owned-worker path; the unchanged legacy scheduler is not silently rewritten. No token is passed to work or ProcessSpec. Expected payload storage cannot introduce a throwing move; allocation can fail and is retained. The API is not a second scheduler and launches no process itself.
all_succeeded()is a necessary predecessor-result gate, not freshness, commit authority, process/service-writer quiescence or safe project write-lease transfer. No production pipeline calls it yet. Partially launched OS-worker startup failure has not been fault-injected; setup allocation failure/source RAII reasoning are not that runtime coverage.Completed current-head gates
The accepted File Event/PDB Open research workflows were not triggered by this diff and are not presented as rerun/passed this head. Their code/dependent product paths remain unchanged. No post-submission source revision or same-head native/CI/benchmark retry occurred.
Actual multiworker success/failure/cancellation
Six-case original-result and executable-provenance audit
Debug and Release each run the fixed complete/cancel/failure-cancel sequence. Complete returns succeeded with original exits0,0,0 and actual link/run/fixture-commit. Cancel returns cancelled with original exits0,0, two entered items and no successor. Failure-cancel returns failed, with original exits0,2/C2338 and the injected static_assert marker, even while admission_stop_observed=true; no successor is invoked. The third slot remains not_started for all four stopped cases. Original failure is not rewritten as cancellation or recovery success.
All6cases retain two real live cl.exe handles/image/creation identities before the stop decision and observe their completion before accepting the batch. The forwarding runner rejects terminating tokens and every native compiler result has cancelled=false. Two12000-function active inputs, one pending input, /FS/effective /Zi,/bigobj and /MTd or /MT remain fixed. The original recipe's /Z7 precedes /Zi; actual PDB creation is checked. Twelve input comparisons and twelve planned-argv comparisons normalize only case paths and the declared trailing failure marker. These are recipe-derived argv, not independent OS command telemetry.
All18original compile/link/run results and36stdout/stderr files inspected. Positive cases really link/run; stopped cases have no link/run call, output executable or sentinel. That sentinel is fixture-only, not production cache publication.
production_pipeline_integrated=falseandsafe_to_transfer_write_lease=falseremain explicit. The injected static_assert is not historical PDB/C1041 reproduction or an RPC-completion test.Existing errors and performance stay visible
Default70 preserves all14single-worker scheduler controls and14direct references;126source/argv comparisons across those pairs and actual helper/refusal outputs pass. Four intentionally adverse default B failures remain (Zi Release,PCH Release,Modules Debug,Modules Release under A-started normal), C1090/original exit2, unattempted link/run and separate recovery0. Private B happens to pass, but all28A-started managed endings still kill original services. Default715/private417toolresults and1430/834diagnostics checked. Resource identities16/70and0/42 are not writer-quiescence evidence.
Independent19x4ABBA preserves all72instrumented complete counters/breakdowns/compile-link-archive hit-miss identities; four external timings-off counter sets unavailable, not zeros. External no-op paired median +0.3388ms/+3.1622469%, maximum adverse+1.363ms,2/4slower; unchanged BOTH>1ms AND>10% median gate not crossed. Preserve link-only+283.626ms,scale-j1+1.926ms median and all other adverse/favorable samples. No speedup/zero-overhead claim, noise deletion, AA subtraction or historical debt closure. Only external row uses timings-off elapsed; four-pairP95 is a maximum, not a population-tail estimate. Comparison JSON does not archive the benchmark binary pair or OS trace.
Original predeclared gates and next boundary
Full exact-source native Debug/all Release/selfhost/package,46checks in actual Debug/Release,six real two-worker outcome/successor controls,original70/42positive controls and150record expectations, plus unchanged19x4ABBA and its original conjunction were required before acceptance. Those gates now pass; none was waived or weakened. Untriggered historical research is not relabeled current validation. Temporaryperf title changed only after actual#590finished; later skippedperf jobs are not new measurements.
Next separate small PR: adopt typed outcomes at an explicit compile-wave/target boundary without changing the existing no-token hot path or weakening inspection/execution evidence and freshness semantics. Original failures must block downstream target link/cache publication and unentered tasks cannot satisfy prerequisites. Test real production-boundary behavior rather than claiming this fixture commit is production integration. No additional observer expansion, CLI cancel switch, shared-service termination policy, project lease/transaction/build-run/session/residency or formal release is introduced here.
Eight immutable coreZIPs, completebase/headsourcearchives,patch,manifests and executedread-onlyaudits are retained in delivery; not every Actionsartifact. Retention30days; generatedcompilerbinarybodies are excludedfromdiagnosticZIPs, separateinputs contain actual observer executables and are not the ABBApair. Finitepost-caseinventories are not failure-instantsnapshots. LocalGCC/ASan/UBSan with platformshims and declaration-onlyWin32syntaxchecks are not localMSVC/PowerShell/ETWexecution; original localsetupfailures remain logged. Native log excerpts are explicitly excerpts, not full archivedCIlogs. No current failed check or original failure was relabeled successful.