Skip to content

fix(cli): fail benchmarks with no completed renders - #5303

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/benchmark-all-failed-exit
Oct 24, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/benchmark-all-failed-exit

Conversation

@user-github-me

Copy link
Copy Markdown
Contributor

What

Return exit code 1 when hyperframes benchmark completes no renders across all configurations and attempts.

Why

With a corrupt local audio source, every preset fails during render preparation. The command reports five failures and no runs, but exits 0, so automation treats the unusable benchmark as successful.

Related work

Self-contained CLI diagnostic correction; no matching benchmark exit-status issue or overlapping implementation found in current source and PR searches. Existing worker-budget and AWS planning work does not change benchmark command results.

How

Record a failure through the shared command-result API after all configurations finish and before either output branch. The existing JSON report and human table remain available. A completed render still makes the benchmark successful, including partial failures and sub-millisecond runs.

Test plan

  • Four all-failure regressions fail on unchanged main, with six success controls passing. All 55 benchmark, command-result, doctor, and batch-render tests pass with the fix.
  • Run source and built CLI processes on a 64×64, 0.2-second composition with a corrupt WAV. Both JSON and human modes exit 1; the JSON report preserves all five configurations, failure counts, empty runs, and null averages. Unchanged main exits 0 for the same input.
  • Run both CLI processes on a healthy composition: all five configurations render and the process exits 0. Successful renders emit existing browser diagnostic logs ahead of the JSON report; the report was parsed separately for validation.
  • Both actual compositions pass static lint. The healthy composition passes browser check with no findings; the deliberately corrupt audio produces the expected unreadable-media warning during browser check and is rejected during render preparation.
  • Full workspace build, CLI rebuild/typecheck, root lint, formatting, test reachability, comment checks, and normal commit hooks pass.
  • Comments follow CONTRIBUTING.md.

To reproduce, put a corrupt local WAV in an otherwise valid composition and run hyperframes benchmark <project> --runs 1 --json. Every result has failures: 1 and runs: []; this change also makes the process exit 1.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Confirmed main's benchmark.ts records per-config failures but never sets a non-zero result, so an all-failure run exits 0. The fix calls the shared setCommandExitCode(1) only when every config has zero completed runs, and it does so before either output branch, so the JSON report and human table are still printed. Keying on runs.length instead of avgTime is the right call, because avgTime drops 0 ms runs and would wrongly fail a sub-millisecond success; the test pins that case. The tests mock the producer and go through runCommand and consumeCommandResult, covering both output modes, 1 and 2 runs per config, partial success, and full success. Need: yes · Size: ok · Reuse: uses the existing commandResult API · Simpler: already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 9e533f0 Oct 24, 2026
81 checks passed
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