Skip to content

Keep US staging telemetry on for every build - #1082

Draft
PavelMakarchuk wants to merge 3 commits into
mainfrom
staging-on-by-default
Draft

PavelMakarchuk wants to merge 3 commits into
mainfrom
staging-on-by-default

Conversation

@PavelMakarchuk

@PavelMakarchuk PavelMakarchuk commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Handoff (Pavel, 2026-10-01). For @anth-volk to own or fold into #933, which this partly implements: exact-count builds stage without moving pointers. #933 also asks for versioned (v2) staging and requested and realized counts, which this does not do. @MaxGhenis: the SIGTERM handler touches the release tool's main. CI is green. Please review before merge, ideally between Route A attempts.

Why

US builds have uploaded nothing to policyengine/populace-us-staging since 2026-07-02, so the staging dashboard cannot show a running US build. Two things keep US staging off:

  • Exact-count builds: --exact-k requires --no-staging (ExactKPointerSuppressionError), and the exact-k ladder launcher hard-codes it.
  • Recent release launches: builds such as experiments/build_j_recert/buildj_*.sh pass --no-staging. July's Builds I–P have staging: null in 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.

  • Uploads never block the build.

    • With background_uploads=True, which the release now always sets, uploads run on a worker thread.
    • Each cycle uploads snapshots taken under the write lock, so a half-written file is never shipped.
    • complete() and fail() wait for the terminal upload for at most final_upload_timeout_seconds, default 120 s, then return with a warning; local files are always complete.
    • Synchronous mode remains the default for other callers.
  • The default --staging-upload-interval-seconds is now 300 s, up from 30 s.

    • Each cycle is up to six single-file Hub commits, and the Hub allows roughly 128 commits per hour per repository. This is the same reason uk_runtime/rowwise_staging.py uses 300 s.
    • Stage failures, attachments and the final state still upload immediately.
  • SIGTERM records a failure.

    • main turns SIGTERM into BuildTerminatedError, which exits with code 143.
    • The existing except BaseException path then marks the staging run failed instead of leaving it running forever.
    • This covers supervisor stops and Modal budget stops. SIGKILL and OOM kills remain out of reach.
    • The previous handler is restored on exit.
  • A build without a write token is never refused.

    • At the start, the release checks the ambient credential with credential_can_write().
    • With no token, or one that can't write, the build prints a warning with the exact re-stage command, records delivery_check in the run manifest, keeps its telemetry local and builds as usual.
    • A check that cannot reach the Hub changes nothing: uploads stay best-effort.
  • tools/restage_us_staging_run.py --run-dir <run folder> uploads a finished local run from any machine with a token.

    • It sends the run files and the artifacts the manifest lists, and upserts the run into runs.json.
    • It never moves latest_staging.json.
    • --no-index leaves runs.json alone, for exact-count runs.

Not in this PR

  • One-time operations: the build machine needs a Hugging Face write token for populace-us-staging, and route_a.sh should not pass --no-staging. Without a token, uploads self-disable after three failures, local telemetry is still written, and publish asks for --allow-missing-staging as today.
  • Richer telemetry, as a follow-up: stage end with CPU and RSS, a heartbeat, batch progress inside target compilation, and failure classes.

Testing

  • New tests:
    • background uploads do not block on a stuck Hub; the terminal upload is awaited and is consistent; the wait is bounded;
    • pointer-free runs upload only under their prefix; exact-k parses without --no-staging and stages pointer-free;
    • SIGTERM marks the run failed and restores the handler;
    • a read-only credential builds on and stays local with a recorded reason, and a write credential uploads;
    • re-staging uploads a local run, upserts the index, and never moves the pointer.
  • Run locally on the stacked branch, which includes this one: 398 passed, 8 skipped.
    • packages/microcosm-build/tests/engine_free/shared/test_staging.py
    • packages/microcosm-build/tests/engine_free/us/test_us_exact_k_ladder_launcher.py
    • packages/microcosm-build/tests/engine_free/us/test_us_fiscal_refresh_builder.py
    • packages/microcosm-data/tests/engine_free/shared/test_publish_guard.py
  • ruff check . and ruff format --check pass.

Context: the staging dashboard's new Build progress tab reads this telemetry (PolicyEngine/calibration-diagnostics#197).

🤖 Generated with Claude Code

PavelMakarchuk and others added 2 commits October 1, 2026 13:19
- 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 MaxGhenis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. A failure after complete() leaves the remote run passed.

    • Where: tools/build_us_fiscal_refresh_release.py:15826 and packages/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 new main() handler writes failed locally and sets _upload_requested, but no worker is left to upload it.
      • So the dashboard says passed for a build that failed. A repro of complete(); fail(OSError(28, ...)) gave local failed and remote passed.
    • 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 let fail() upload on its own when the worker has already stopped.
  2. The build manifest's delivery count is taken before uploads finish.

    • Where: tools/build_us_fiscal_refresh_release.py:15802, then :15826, and packages/microcosm-data/src/microcosm/data/publish_cli.py:53.
    • What happens:
      • staging=_staging_manifest_block(telemetry) reads uploads_succeeded while 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.
  3. The recommended recovery command breaks exact-k isolation.

    • Where: staging.py _check_write_access (around line 267) and tools/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.json doesn't record update_pointers, so restage_run defaults to update_index=True and upserts runs.json.
      • Running that command on an exact-k run (update_pointers=False) wrote runs.json, which is the thing README:57-58 says exact-k never does.
    • Fix: persist the pointer policy in the run manifest, honor it in restage_run, and print --no-index in the hint for exact-k runs.

Should fix

  1. 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 before progress.json exists.
    • _ACTIVE_TELEMETRY is 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.
  2. Restaging silently skips artifacts attached with copy=False.

    • Where: staging.py:107 and :597, and the builder at :15821.
    • attach_artifact records only source.name. restage_run looks for run_dir / name, and build_manifest.json and release_manifest.json live in release_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.
  3. The background-upload test can pass without the terminal bytes ever uploading.

    • Where: packages/microcosm-build/tests/engine_free/shared/test_staging.py:26 and :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.
  4. 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 failed telemetry.
  5. 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 in tools/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.

@MaxGhenis

Copy link
Copy Markdown
Contributor

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 (staging.py) is deleted at M2 once the US writes version 3. So "US staging on for every build" should land as staging version 3 for the fiscal-refresh tool, which is #933. I'd re-scope this PR to that, or close it into #933. Your call, Pavel.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants