feat(timing): warmup progress heartbeat and empty-drain guard

[ajcasagrande]

Summary

Carve from the agentx branch: two small, independent improvements to PhaseRunner.

1. Env-gated warmup progress heartbeat (INFO)

Headless/non-interactive runs previously had no observable signal during warmup — progress messages are only consumed by interactive UIs. _progress_report_loop now emits a throttled INFO heartbeat during the WARMUP phase:

Phase warmup progress | returned=32/50 | sent=40 | in_flight=8 | errors=1 | elapsed=12.3s

• New env knob: AIPERF_SERVICE_WARMUP_PROGRESS_LOG_INTERVAL (Field on _ServiceSettings, default 30.0s, 0 disables), per the repo rule that tunables live in environment.py.
• docs/environment-variables.md regenerated via make generate-env-vars-docs.
• Formatting lives in a small PhaseRunner._format_warmup_progress static helper; target falls back from final_requests_sent to total_expected_requests, and omits the /target suffix when neither is known.

2. Empty-drain guard in _wait_for_returning_complete

Fixes a hang edge on the timeout → cancel_all_credits() → drain path: if all credits have already returned by the time need is recomputed (need <= 0), nothing will ever set all_credits_returned_event, so the runner waited out the full CANCEL_DRAIN_TIMEOUT and then force-released "stuck" slots that were never stuck. The guard sets the event immediately when need <= 0.

Revert-verify evidence (tests run against reverted src): the drain test logs Need 0 more credits. followed by Timeout waiting 10.0s for cancelled credits to return. ... Forcing phase completion. — exactly the edge this fixes.

Pin discipline (revert-verified)

git apply -R of the src hunks → all 4 new tests FAIL:

FAILED ...::TestWarmupProgressHeartbeat::test_format_warmup_progress_includes_returned_target_and_elapsed
FAILED ...::TestWarmupProgressHeartbeat::test_format_warmup_progress_without_target
FAILED ...::TestWarmupProgressHeartbeat::test_warmup_log_interval_env_field_exists
FAILED ...::TestEdgeCases::test_timeout_skips_cancel_drain_when_all_credits_returned

Re-applied → 4 passed.

Testing

• uv run pytest -n auto tests/unit/timing/phase/test_runner.py tests/unit/timing/ — 771 passed (no pre-existing failures observed)
• pytest --collect-only tests/unit/timing/ clean (771/778 collected, 7 deselected by markers)
• ruff format --check / ruff check clean on all changed files
• make check-ruff-baselined — 0 new; make check-ergonomics — 0 new

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added configurable warmup progress heartbeat logging during warmup, exposed via AIPERF_SERVICE_WARMUP_PROGRESS_LOG_INTERVAL (default 30s), including an option to disable with 0.
  • Enhanced warmup progress messages with returned/completed/cancelled counts and elapsed time.
• Bug Fixes
  • Improved timeout behavior to skip the cancelled-credit drain wait when all credits are already returned, avoiding unnecessary delays.
• Documentation
  • Documented the new warmup progress heartbeat environment variable, including defaults and valid range.

[github-actions]

Try out this PR

Quick install:

pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@9272320798cbfe0e726f0f9330ff2c7f76811877

Recommended with virtual environment (using uv):

uv venv --python 3.12 && source .venv/bin/activate
uv pip install --upgrade --force-reinstall git+https://github.com/ai-dynamo/aiperf.git@9272320798cbfe0e726f0f9330ff2c7f76811877

---

Last updated for commit: 9272320 • Browse code

[github-actions]

Fern Docs Preview: https://nvidia-preview-022a6bc3-4275-4f28-8927-b21bbefe7a56.docs.buildwithfern.com/aiperf/dev

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b1a38079-9712-4e8b-92c2-0b5fd5162f44

📥 Commits

Reviewing files that changed from the base of the PR and between 4fa9369 and 9272320.

📒 Files selected for processing (4)

• docs/environment-variables.md
• src/aiperf/common/environment.py
• src/aiperf/timing/phase/runner.py
• tests/unit/timing/phase/test_runner.py

✅ Files skipped from review due to trivial changes (1)

• docs/environment-variables.md

🚧 Files skipped from review as they are similar to previous changes (3)

• src/aiperf/common/environment.py
• tests/unit/timing/phase/test_runner.py
• src/aiperf/timing/phase/runner.py

---

Walkthrough

Adds a configurable warmup progress log interval, emits throttled warmup heartbeat logs in PhaseRunner, and changes the return-wait timeout path to skip unnecessary drain waiting when no credits remain outstanding.

Changes

Warmup heartbeat and drain timeout fix

Layer / File(s)	Summary
Config setting and documentation src/aiperf/common/environment.py, docs/environment-variables.md	Adds WARMUP_PROGRESS_LOG_INTERVAL to service environment settings and documents the default, bounds, and 0-disables behavior.
Warmup heartbeat logging src/aiperf/timing/phase/runner.py, tests/unit/timing/phase/test_runner.py	Adds warmup progress formatting and throttled INFO heartbeat emission during CreditPhase.WARMUP; tests cover message formatting and the new config field.
Cancel-drain timeout short-circuit src/aiperf/timing/phase/runner.py, tests/unit/timing/phase/test_runner.py	Short-circuits the post-timeout cancel drain wait when remaining credit need is <= 0; regression test verifies the event is set and stuck-slot release is skipped.

Estimated code review effort: 2 (Simple) | ~15 minutes

Poem

A rabbit tapped a logging drum,
While warmup credits started to hum.
If nothing’s left to return,
No drain wait makes it burn—
Just hop ahead, and on we come.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the two main timing changes: warmup heartbeat logging and the empty-drain guard.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/aiperf/timing/phase/runner.py (1)

791-809: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Throttled warmup heartbeat logic looks correct.

Gating on phase == CreditPhase.WARMUP, warmup_log_interval > 0, and now >= next_warmup_log_at correctly implements throttling and the 0-disables semantics from the new setting.

No test in this PR exercises _progress_report_loop's actual gating/throttle behavior (phase check, interval comparison, next_warmup_log_at advancement) — only _format_warmup_progress and the env field are unit-tested directly. Consider a test that drives a couple of loop iterations with a controlled time.monotonic() to confirm the heartbeat fires at most once per interval and is suppressed when WARMUP_PROGRESS_LOG_INTERVAL=0 or phase != WARMUP.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/aiperf/timing/phase/runner.py` around lines 791 - 809, Add tests for
_progress_report_loop in runner.py that exercise the real throttling behavior,
since the current coverage only hits _format_warmup_progress and the env field.
Mock time.monotonic() and drive multiple loop iterations through
_progress_report_loop to verify the warmup heartbeat is emitted at most once per
warmup_log_interval, and that it is suppressed when
Environment.SERVICE.WARMUP_PROGRESS_LOG_INTERVAL is 0 or when self._config.phase
is not CreditPhase.WARMUP. Use the existing _progress, _phase_publisher, and
self.info call sites to assert the gating and next_warmup_log_at advancement
logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/aiperf/timing/phase/runner.py`:
- Around line 791-809: Add tests for _progress_report_loop in runner.py that
exercise the real throttling behavior, since the current coverage only hits
_format_warmup_progress and the env field. Mock time.monotonic() and drive
multiple loop iterations through _progress_report_loop to verify the warmup
heartbeat is emitted at most once per warmup_log_interval, and that it is
suppressed when Environment.SERVICE.WARMUP_PROGRESS_LOG_INTERVAL is 0 or when
self._config.phase is not CreditPhase.WARMUP. Use the existing _progress,
_phase_publisher, and self.info call sites to assert the gating and
next_warmup_log_at advancement logic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 64491c7f-79a7-41df-b06d-fda46adb4355

📥 Commits

Reviewing files that changed from the base of the PR and between 0314074 and 4fa9369.

📒 Files selected for processing (4)

• docs/environment-variables.md
• src/aiperf/common/environment.py
• src/aiperf/timing/phase/runner.py
• tests/unit/timing/phase/test_runner.py

[codecov]

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/aiperf/timing/phase/runner.py	81.25%	2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!