Repository navigation
Record the required-floor shared-fill commit boundary - #9982
Conversation
briansrls
left a comment
There was a problem hiding this comment.
APPROVE at exact head efd95d37a87c18e3ce86806d2635b6f3bf141d88.
This is a new exact-head review. The requested 9c65a29f764e137d094a0d41d7cc9584aadc4db0 subject moved during adjudication, and its dashboard approval does not carry. I directly reviewed the current two-file subject, including the successor correction that narrows unstored accounting to this frame's remaining self work while preserving independently completed descendant-fill credits.
This PR is approved strictly as characterization, not as repair or closure of the floor flake.
The document accurately records the operative boundary: the floor enforces and receipts marginal CPU after subtracting completed shared-fill counters; CrossClaimFillGuard begins unstored, is marked stored only after a fresh CrossClaimStoreOutcome::Stored, and on drop credits a completed fill only in that arm. If cooperative preemption occurs first, this frame has no completed-fill credit, its stored descendants retain their own credits, and its remaining self work stays charged to the triggering witness. That explains the interrupted row's absent completed-fill record and the passing rows whose measured totals substantially exceed their marginal CPU without implying non-uniform enforcement.
I independently re-derived the two recorded exposure populations from the uploaded TSV artifacts. Both contain 3,444 executed rows. Run 33573600142 has median 1ms, 121 rows above 200ms, 37 above 300ms, 10 above 400ms, and maximum 455ms. Run 33578414901 has median 1ms, 115 above 200ms, 24 above 300ms, zero above 400ms, and maximum 397ms. The same exact ten identities are the top ten in both; the document correctly treats this as a stable expensive cluster while preserving the cross-head confound and refusing to read the second run's zero-over-400 count as closure.
The corrections are load-bearing and correctly placed: measured_cpu_ms is not the enforced quantity, and passing-path per-claim cost is available in the uploaded required_floor_claim_cost.tsv, not necessarily in the console log. The document does not select a repair. Its ownership conclusion is bounded to the current model: before successful publication, no durable shared identity/outcome exists for this frame, so prospective subtraction is unsupported; the separate repair subject must decide and model any different attribution law.
The doc_graph_roots enrollment is subject-grained and its dissolution requires the missing typed in-flight identity plus terminal outcome, an attribution policy consumed before the deadline, and a repair that classifies interrupted and completed runs under one rule. It does not retire merely because this document exists.
Fresh exact-head workflow run 33582269028 is still pending fabric-evidence. This is technical approval of the current content, not merge authorization before all required checks are terminal green. Any head movement or CI red requires revalidation.
|
Addressed review 58395 at b46fcbe. The derived distribution, band counts, extrema, copied TSV row, and other transcribed measurements are removed. The document now cites the existing symbol-grain authority |
briansrls
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES at exact head b46fcbef9c098ea2e85c950359cef9fa792ae447.
The repository content is accepted. Deleting the one-off exposure distribution and copied TSV values is the correct repair: gunbc.rung_drop::floor_cost_contention_verdict already owns the exposed-attention-view concept and explicitly distinguishes a measurement-selected view from the required-witness population. The findings document retains the genuinely new store/abandon mechanism, the self-time-versus-stored-descendant bound, the measured-vs-marginal correction, the artifact-vs-console correction, and the cross-head confound. I do not require the deleted numbers restored or a repair design added.
One metadata blocker remains. The live PR body still transcribes exactly the measurements the new head removed under the section 6 ruling: 214ms, 286ms / 57.2%, and the 3675ms/5ms split. That leaves the PR claiming the violation is resolved while its own durable description repeats it, and it recreates the second numerical recital beside the cited authority.
Disposition is metadata-only: update the PR body to name run 33573600142, artifact 9826348456, required_floor_claim_cost.tsv, and the producer/authority without copying their output values. The code head may remain exactly b46fcbef...; no regeneration or CI rerun is required, and exact-head CI may carry if it completes green. Return for dismissal and approval on this same SHA.
The metadata-only blocker is resolved on unchanged code head b46fcbe. The PR body now names the run, artifact, TSV producer, and existing floor-cost authority without transcribing their output values. Dismissing for exact-head approval after final live-state verification.
briansrls
left a comment
There was a problem hiding this comment.
APPROVE at exact head b46fcbef9c098ea2e85c950359cef9fa792ae447.
The metadata-only blocker from review 5085179166 is closed. The PR body now names run 33573600142, artifact 9826348456, required_floor_claim_cost.tsv, v1_compiler.cli_run::write_required_floor_claim_cost_tsv, and gunbc.rung_drop::floor_cost_contention_verdict without transcribing the removed output values. This matters under squash landing because the PR body becomes durable commit-message history.
The repository content remains accepted as characterization only. Deleting the one-off exposure distribution is the correct choice: the existing rung-drop authority owns the exposed-attention-view concept and the population-versus-measurement-view distinction. This document retains the distinct store/abandon mechanism, current-frame self time versus stored-descendant credits, marginal-versus-measured correction, artifact-versus-console correction, and cross-head confound. It proposes no repair and does not close the floor-flake subject.
Exact-head workflow run 33583865257 completed success: required-witnesses-build, required-witnesses-floor, rust-unit-tests, fabric-evidence, and the witnesses aggregate succeeded; emit-copy was skipped as designed. The head is unchanged and remains mergeable.
Composition was rechecked against live main c26de912077a702bd7d3a12c2da9fcc4fe21b8ca: since the PR merge base 8282ae4e808ae87843de702d1b00eb2277b9027c, main changed neither dag/gunbc/doc_graph_roots.dag nor docs/plans/required-floor-in-flight-shared-fill-findings.md. The #9983 shared-authority/generated-projection objection does not apply here.
Any head movement requires revalidation.
Summary
on_fill_abandonarm using the named bootstrap symbolsmeasured_cpu_msis not the enforced marginal quantity, and passing per-claim cost lives in the uploaded TSV artifact rather than the console logEvidence and authority
33553058260at2b56084270: interruption before verdict and no shared-fill record for the abandoned frame33573600142, artifact9826348456,required_floor_claim_cost.tsv, produced byv1_compiler.cli_run::write_required_floor_claim_cost_tsvgunbc.rung_drop::floor_cost_contention_verdict; this PR cites that authority rather than transcribing its measurementsScope
Characterization only. This PR does not change or propose the floor cost model. The document is registered in the typed document graph with an explicit retirement condition.
Checks
git diff --checkcargo fmt --all --check