Repository navigation
Keep US staging telemetry on for every build - #1082
PavelMakarchuk wants to merge 3 commits into
Conversation
- Exact-count builds stage under their own run prefix and never move latest_staging.json or runs.json, so they no longer need --no-staging (#578 forbids automatic pointer moves, not visibility). The ladder launcher stops passing --no-staging. - The release uploads from a background thread, so a slow or unreachable Hub never stalls the build; the terminal upload is awaited for at most 120 s. Uploads snapshot files under the write lock. - The default upload interval is 300 s, as for UK builds: each cycle is up to six single-file Hub commits against roughly 128 per hour per repository. - SIGTERM raises BuildTerminatedError, so a supervisor or budget stop marks the staging run failed instead of leaving it running. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A US build whose Hugging Face credential cannot write the staging repository is never refused. It checks at the start, prints how to upload the run later, records delivery_check in the run manifest, keeps its telemetry local and builds as usual. tools/restage_us_staging_run.py uploads a finished run folder from any machine with a token; it adds the run to runs.json and never moves latest_staging.json. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis
left a comment
There was a problem hiding this comment.
Review of 09b817007. Two reviewers worked independently, one on the signal handling and telemetry and one on tests and scope, and I checked each finding below against the code at this head. CI is green and the branch merges cleanly with main at 07bbb656b.
The SIGTERM handling works. main() calls fail(), re-raises, exits 143 and restores the previous handler, and a subprocess probe confirmed it. Exact-k live uploads leave latest_staging.json and runs.json alone. A missing token warns and the build continues. The problems are in the paths around delivery and recovery. I'd fix the first three before merging, because each makes the dashboard or the publisher guard wrong in exactly the way this PR sets out to prevent.
Fix before merge
-
A failure after
complete()leaves the remote runpassed.- Where:
tools/build_us_fiscal_refresh_release.py:15826andpackages/microcosm-build/src/microcosm/build/staging.py:401. - What happens:
- The builder calls
telemetry.complete()and only then runs_copy_base_h5_for_local_audit, which can fail (disk full, for example). complete()→_finish_uploads()sets_uploads_closing, so the worker exits after its terminal cycle.- A later
fail()from the newmain()handler writesfailedlocally and sets_upload_requested, but no worker is left to upload it. - So the dashboard says
passedfor a build that failed. A repro ofcomplete(); fail(OSError(28, ...))gave localfailedand remotepassed.
- The builder calls
- The complete-then-copy order predates this PR; the background worker is what makes the later failure unreachable.
- Fix: call
complete()after the last fallible step, and letfail()upload on its own when the worker has already stopped.
- Where:
-
The build manifest's delivery count is taken before uploads finish.
- Where:
tools/build_us_fiscal_refresh_release.py:15802, then:15826, andpackages/microcosm-data/src/microcosm/data/publish_cli.py:53. - What happens:
staging=_staging_manifest_block(telemetry)readsuploads_succeededwhile the background worker may still be mid-cycle. The terminal uploads, including the build manifest itself, come after.- The publisher treats
not staging.get("uploads_succeeded")as undelivered and refuses without--allow-missing-staging. - In a repro with the first upload held: stored count 0, final count 6, and the publisher guard refused.
- With synchronous uploads on main the count was current when the manifest was written; moving uploads to the background is what opened this.
- Fix: write the count after a bounded drain, then upload the corrected manifest. Restaging should be able to repair it as well.
- Where:
-
The recommended recovery command breaks exact-k isolation.
- Where:
staging.py_check_write_access(around line 267) andtools/restage_us_staging_run.py:47. - What happens:
- The no-token warning prints
restage_us_staging_run.py --run-dir …without--no-index. run_manifest.jsondoesn't recordupdate_pointers, sorestage_rundefaults toupdate_index=Trueand upsertsruns.json.- Running that command on an exact-k run (
update_pointers=False) wroteruns.json, which is the thing README:57-58 says exact-k never does.
- The no-token warning prints
- Fix: persist the pointer policy in the run manifest, honor it in
restage_run, and print--no-indexin the hint for exact-k runs.
- Where:
Should fix
-
The credential check runs on the build thread.
- Where:
staging.py:224. __init__calls_check_write_access()→credential_can_write()→whoami()before the worker starts and beforeprogress.jsonexists._ACTIVE_TELEMETRYis assigned only after the constructor returns, so a SIGTERM during the check cannot mark the run failed.- A fake
whoami()that blocked on an Event left construction blocked with no progress file, which contradicts README:55-56 ("never stalls the build"). - Fix: write local telemetry first and run the check in the worker under a deadline.
- Where:
-
Restaging silently skips artifacts attached with
copy=False.- Where:
staging.py:107and:597, and the builder at:15821. attach_artifactrecords onlysource.name.restage_runlooks forrun_dir / name, andbuild_manifest.jsonandrelease_manifest.jsonlive inrelease_dir, so they're skipped without a message even though the run manifest advertises their staging paths.- Fix: copy attachments into the run folder, or record a resolvable path, and report any declared artifact that's missing.
- Where:
-
The background-upload test can pass without the terminal bytes ever uploading.
- Where:
packages/microcosm-build/tests/engine_free/shared/test_staging.py:26and:307. - The fake API stores paths and the assertions re-read snapshot files that later cycles overwrite.
- A mutation that suppressed every upload after the first still passed.
- Fix: capture the bytes inside
upload_file()and assert the transmitted terminal content.
- Where:
-
The SIGTERM test signals the pytest process itself.
- Where:
packages/microcosm-build/tests/engine_free/us/test_us_fiscal_refresh_builder.py:13456. - It catches regressions, but if the handler is removed the runner is killed (exit -15) instead of reporting a failure.
- Fix: run the body in a subprocess and assert its exit status and the recorded
failedtelemetry.
- Where:
-
README.
- Line 65 puts prose on the same line as the closing fence, so the code block never closes.
- Line 423 still says the exact-k launcher "forces
--no-staging", which this PR removes intools/build_us_exact_k_ladder_release.py:441.
Not covered here
- SIGKILL. An in-process handler can't catch it, and the docstring says so. On DISK_FLOOR, Route A's supervisor SIGKILLs the build and records the failure on its own side, so staging stays at its last uploaded state. That's acceptable as long as it's documented.
- #933 (exact-k staging telemetry) has no assignee, and nothing in it depends on this PR.
On the Route A side, the driver PR that drops --no-staging and gives the release a token at exec time is in progress. It will turn staging on for the next Route A attempt whether or not this merges first.
|
Checked against main (84486d0) on 2026-10-15 for the release-contract milestone, #1285 (docs/release-contract.md, C3). From 2026-11-06 a national-default release may not opt out of staging, and staging version 1 ( |
Why
US builds have uploaded nothing to
policyengine/populace-us-stagingsince 2026-07-02, so the staging dashboard cannot show a running US build. Two things keep US staging off:--exact-krequires--no-staging(ExactKPointerSuppressionError), and the exact-k ladder launcher hard-codes it.experiments/build_j_recert/buildj_*.shpass--no-staging. July's Builds I–P havestaging: nullin their manifests.This PR makes staging cheap and safe enough to leave on for every build, so nobody has to remember to enable it.
What changes
Exact-count builds stage without moving pointers.
update_pointers=False,StagingTelemetrywrites only underruns/<run_id>/. It never toucheslatest_staging.jsonorruns.json, and never reads the index.update_pointers=Falsewhenever--exact-kis set.test_staging_credentials_cannot_enable_pointer_writesnow asserts that a credentialed exact-k run uploads only under its own prefix.Uploads never block the build.
background_uploads=True, which the release now always sets, uploads run on a worker thread.complete()andfail()wait for the terminal upload for at mostfinal_upload_timeout_seconds, default 120 s, then return with a warning; local files are always complete.The default
--staging-upload-interval-secondsis now 300 s, up from 30 s.uk_runtime/rowwise_staging.pyuses 300 s.SIGTERM records a failure.
mainturns SIGTERM intoBuildTerminatedError, which exits with code 143.except BaseExceptionpath then marks the staging run failed instead of leaving itrunningforever.A build without a write token is never refused.
credential_can_write().delivery_checkin the run manifest, keeps its telemetry local and builds as usual.tools/restage_us_staging_run.py --run-dir <run folder>uploads a finished local run from any machine with a token.runs.json.latest_staging.json.--no-indexleavesruns.jsonalone, for exact-count runs.Not in this PR
populace-us-staging, androute_a.shshould not pass--no-staging. Without a token, uploads self-disable after three failures, local telemetry is still written, and publish asks for--allow-missing-stagingas today.Testing
--no-stagingand stages pointer-free;packages/microcosm-build/tests/engine_free/shared/test_staging.pypackages/microcosm-build/tests/engine_free/us/test_us_exact_k_ladder_launcher.pypackages/microcosm-build/tests/engine_free/us/test_us_fiscal_refresh_builder.pypackages/microcosm-data/tests/engine_free/shared/test_publish_guard.pyruff check .andruff format --checkpass.Context: the staging dashboard's new Build progress tab reads this telemetry (PolicyEngine/calibration-diagnostics#197).
🤖 Generated with Claude Code