Skip to content

fix: errors with benchmark completion in fixed schedule#585

Merged
ajcasagrande merged 2 commits intomainfrom
ajc/debug-fix-schedule
Jan 21, 2026
Merged

fix: errors with benchmark completion in fixed schedule#585
ajcasagrande merged 2 commits intomainfrom
ajc/debug-fix-schedule

Conversation

@ajcasagrande
Copy link
Copy Markdown
Contributor

@ajcasagrande ajcasagrande commented Jan 21, 2026

In order to be a small patch for release version, avoiding a larger refactor and just patching it for now

Summary by CodeRabbit

Release Notes

  • New Features

    • Fixed-schedule timing mode now automatically adjusts expected request counts and session numbers based on actual dataset metadata for more accurate scheduling.
    • Enhanced error handling with proper lifecycle event publication during failures for improved error reporting and recovery.
  • Tests

    • Added test suites validating fixed-schedule offset filtering behavior across multiple scenarios and session configurations.
    • Added tests for configuration correction under fixed-schedule timing mode with various dataset metadata conditions.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 21, 2026

Try out this PR

Quick install:

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

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@bcb51320ab84267ab2ea099e3a745ee13855cd48

Last updated for commit: bcb5132Browse code

@github-actions github-actions Bot added the fix label Jan 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 89.18919% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/aiperf/timing/phase/runner.py 84.61% 0 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 21, 2026

Walkthrough

The changes enhance error handling and lifecycle event management across the phase execution layer. PhaseRunner now respects dataset metadata when using FIXED_SCHEDULE timing mode and publishes phase lifecycle events even during error paths. Phase orchestrator adds try/finally blocks for robust cleanup. Tests validate fixed-schedule offset filtering behavior and PhaseRunner config correction logic.

Changes

Cohort / File(s) Summary
Production - Error handling & FIXED_SCHEDULE support
src/aiperf/timing/phase/runner.py, src/aiperf/timing/phase_orchestrator.py
PhaseRunner imports TimingMode and overrides expected_requests/expected_num_sessions from dataset metadata in FIXED_SCHEDULE mode; wraps execution in try/except/finally to publish lifecycle events on errors before propagating exceptions. Phase orchestrator adds try/finally to ensure cleanup and try/except around each runner.run call for explicit cancellation on failure.
Tests - Unit tests for config correction
tests/unit/timing/phase/test_runner.py
Adds TestFixedScheduleConfigCorrection class with multiple async test methods validating that FIXED_SCHEDULE mode adjusts total_expected_requests and expected_num_sessions based on dataset metadata, while REQUEST_RATE and USER_CENTRIC_RATE modes preserve configured values.
Tests - Integration tests for offset filtering
tests/component_integration/timing/test_fixed_schedule.py
Adds TestFixedScheduleOffsetFiltering suite with four test cases validating fixed-schedule offset filtering across single/multi-session traces with various offset ranges. Note: test suite is duplicated at end of file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 Hops of joy through phases new,
Lifecycle events shining true,
Fixed schedules meet their match today,
Error paths now have their say!
With metadata and cleanup care,
Our tests prove everything is fair. 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: errors with benchmark completion in fixed schedule' accurately summarizes the main changes: error handling improvements in PhaseRunner and PhaseOrchestrator for fixed schedule mode, plus test coverage for fixed-schedule offset filtering.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ajcasagrande ajcasagrande enabled auto-merge (squash) January 21, 2026 20:59
@ajcasagrande ajcasagrande merged commit dad56e9 into main Jan 21, 2026
17 checks passed
@ajcasagrande ajcasagrande deleted the ajc/debug-fix-schedule branch January 21, 2026 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants