Skip to content

[Bugfix][Benchmark] Make judge frame-overflow recovery explicit - #7387

Open
LOGO127 wants to merge 5 commits into
vllm-project:mainfrom
LOGO127:fix/duplex-judge-token-budget-0911
Open

LOGO127 wants to merge 5 commits into
vllm-project:mainfrom
LOGO127:fix/duplex-judge-token-budget-0911

Conversation

@LOGO127

@LOGO127 LOGO127 commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Purpose and contributor intent

Make judge image-frame context-overflow behavior explicit and auditable. The highest-leverage variable is whether frame reduction is explicitly enabled. The default image-frame policy is error; reduce opts into bounded retries on recognized HTTP 400/500 context errors, selecting uniformly from the original timeline. Evaluation users need to distinguish failure recovery from protocol-equivalent benchmark results: reduced frames can change scores.

The existing upstream video_url→sampled-frame fallback is retained. If those sampled frames also overflow, reduction requires explicit --judge-frame-overflow reduce. Unrelated errors, single-frame overflow and incomplete judge responses still fail. Upstream's single corrective request for a scoreless reply is retained; a still-invalid corrective reply fails rather than creating a score.

Current integration

This update reconciles upstream 7eb377d8d8a8e696ce5c6af79541f780dcd92932, including #9260's corrective judge request. The format-correction followup argument now passes through temporal, frame-sample content, direct-video content and video fallback. A correction keeps the accepted frame count. Further context overflow can reduce that count only under the existing opt-in policy, sampling the original timeline. Reduction metadata accumulates across both attempts.

Ground-truth reference normalization, raw judge-reply traces, format-retry markers, score validation, prior-score quarantine and atomic score writes remain intact. HTTP overflow rejections are logged, not represented as judge-reply trace rows. There is no proactive token-fit, equivalent-score, GPU or performance claim.

  • Publication HEAD: e516da99ada8c858208dd78ee7a7ea2ea9635057.
  • Candidate tree: 9d16ac3dd075d41afb463eefec2f15222ed75613.
  • Intended parents: prior PR head 2a9d3abcb1d67e3e5f226fd6760c618f3446cac4 and main 7eb377d8d8a8e696ce5c6af79541f780dcd92932.
  • Normal two-parent integration preserving history; no force push.
  • Two manually changed files: evaluator and frame-budget regressions. The feature delta against new main remains five files. Other upstream changes are inherited, not newly authored work.

Validation

Fresh native CPU validation of the candidate: 435 passed, 33 warnings, no skipped/deselected tests, covering all duplex benchmark and patch tests plus the CPU CLI test module. Tests run with normal package initialization and repository conftests. Eight new overlap regressions additionally fail as expected on a separate mechanical-conflict-resolution control, exposing the missing followup support. Those expected failures are not counted as passing candidate tests.

The overlap checks cover temporal/content/video-fallback corrective requests, another overflow during correction, direct video correction, default-policy failure, frame identity/selection and accumulated metadata. Existing failed-score quarantine coverage now includes upstream's one corrective request and its failed-retry trace.

Runtime: Python 3.12.14, official vLLM 0.32.0+cpu and PyTorch 2.13.0+cpu, Intel OpenMP preloaded, CPU affinity and threads bounded at two, writable offline Hugging Face caches. The review packet includes the exact dependency freeze, commands and logs. Codex executed these tests; they are not claimed as a personal manual rerun by the contributor.

taskset -c 0,1 timeout 900 python -m pytest \
  tests/benchmarks/duplex tests/benchmarks/patch \
  tests/e2e/features/fullduplex/test_omni_duplex_eval_cli.py -q

All 13 applicable validation hooks plus the informational suggestion hook passed through the unchanged pre-commit configuration. This includes pinned Ruff, formatting, mypy 3.10, typos, markdownlint, SPDX, test marks, forbidden imports and CUDA-call checks. The aggregate launcher could not install inapplicable actionlint because this host's /usr/bin/go is not a Go toolchain; its failure log is retained. No applicable validation hook was skipped. The initial mypy conditional-call inference error was corrected with an explicit callable annotation; both the hooks and complete native selection were rerun successfully on the final tree.

The previous 232-pass report and hosted green checks applied to 2a9d3ab, not this new merge. New hosted checks must be verified on its published SHA. No real judge model, full dataset, GPU, full hosted CI, performance or score-equivalence validation is claimed. Maintainers control further CI admission and repository merge requirements.

Contribution

Codex assisted conflict reconciliation, regression coverage, tests and this description. Independent AI source review is not a human reviewer approval. The contributor personally reviewed and understood this exact integration and validation evidence, accepted DCO and authorized its normal publication and this body update. No self-approval or repository merge is part of this update.

Retry Omni-DuplexEval frame-sample content judging with uniformly subsampled frames only when the judge explicitly rejects the multimodal prompt for exceeding max_model_len. Preserve unrelated HTTP failures and record the initial/final frame counts plus retry count.

AI-assisted: developed and reviewed with ChatGPT; submitter must add DCO sign-off before publication.
Signed-off-by: luozijian <luozijian0924@gmail.com>
@hsliuustc0106 hsliuustc0106 added bug Something isn't working benchmark/profiler/metrics/logger Codes related to benchmarks, profiler, metrics and logger system labels Sep 10, 2026
@hsliuustc0106

Copy link
Copy Markdown
Collaborator

This PR touches tests/e2e/, vllm_omni/benchmarks/ (2 files). Based on CODEOWNERS coverage of the changed files, the most-related reviewers appear to be:

@alex-jw-brooks @Bounty-hunter @yenuo26

Could one of you take a look when you get a chance? Thanks!

Cover temporal and content image requests with the same bounded recovery path. Keep failure as the default policy; expose opt-in frame reduction, narrow HTTP error classification, and record changes in score and summary metadata. Add typed CPU regressions, real localhost HTTP fixtures, CLI policy coverage, and documentation.

AI assistance: ChatGPT assisted implementation, test design, and review. Local validation includes 34 CPU benchmark tests and 6 CLI-leaf tests with an explicit import adapter; no real model or GPU scoring claim. This follow-up is local and requires human review and DCO sign-off before publication.

Signed-off-by: luozijian <luozijian0924@gmail.com>
@LOGO127 LOGO127 changed the title [Bugfix][Benchmark] Retry frame-sample content after context overflow [Bugfix][Benchmark] Make judge frame-overflow recovery explicit Sep 11, 2026
@LOGO127

LOGO127 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Contributor-side self-review (AI-assisted) for signed follow-up 623fbbb1:

  • Checked both temporal and content frame-sample paths, including the default fail-without-reduction policy. Reduction is explicit opt-in; selecting fewer frames is not claimed to preserve benchmark scores.
  • Checked selection from the original timeline, endpoints/middle frame, the one-frame termination condition, and propagation of unrelated HTTP/transport errors. Score and summary metadata expose reductions; the content video-url path is unchanged.
  • Reran the signed tree: 34 core benchmark tests and 6 CLI-leaf tests passed, plus applicable pre-commit checks. The CLI-leaf test uses the documented import adapter; no full top-level CLI, actual judge-model/GPU or complete-dataset validation is claimed. The production HTTP client is tested against a deterministic localhost fixture.

The new commit is a fast-forward follow-up; prior signed history is unchanged. The PR body and title now describe the revised policy, and the PR remains Draft pending review of that policy and the new checks.

AI assistance: ChatGPT assisted with implementation, tests, and these review notes.

Integrate reviewed main 423f343 while
preserving both original PR commits and the upstream PyAV media migration.
Keep the default error policy, explicit reduce opt-in, bounded retries,
and existing score/summary metadata. Preserve OSError/ValueError frame
extraction behavior and add compatibility coverage.

This tree matches the reviewed refresh candidate
3514482.

Validation: 92 source-level CPU tests pass on the restored reviewed tree.
The source harness bypasses package initialization and conftest, with the
real vLLM v0.30.0 CLI types module. Normal collection remains blocked by
missing torch/vLLM; no GPU, real judge-model, or full-package validation
is claimed. AI assistance: ChatGPT assisted implementation, tests,
validation, and this submission.

Signed-off-by: luozijian <luozijian0924@gmail.com>
…ent main

Merge fixed main 2f15526, preserving the original branch history. Retain upstream judge traces, strict score validation, prior-score quarantine, reference normalization and video_url fallback; image-frame reduction remains explicit opt-in. Cover fallback/reduction composition and schema failure after recovery.

Validated tree 9afcbdd: 232 native CPU tests passed, zero skipped, with official vLLM 0.32.0+cpu; all 13 applicable changed-file static hooks passed. No GPU, live judge-model, complete upstream CI, score-equivalence or performance claim. Codex assisted integration, tests, validation and drafting. The contributor reviewed the delivered exact candidate, confirmed understanding and DCO, and authorized this signed merge and normal publication.

Signed-off-by: luozijian <luozijian0924@gmail.com>

LOGO127 commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor Author

The branch is refreshed at 2a9d3abcb1d67e3e5f226fd6760c618f3446cac4, integrating main 2f155262 without rewriting history. The resolution preserves upstream response tracing, score validation, reference normalization and the existing video_url fallback. Frame reduction remains explicit opt-in; recovered scores are not claimed equivalent to the original evaluation.

The exact published tree passed 232 native CPU tests (0 skipped) and all 13 applicable changed-file static hooks locally. Normal package initialization and repository conftests ran. The updated description contains commands, environment and limitations; this is not GPU, live-judge or full hosted-CI validation.

DCO, pre-commit, both wheel builds, Analyze/CodeQL and Read the Docs have now succeeded on this head. Following contributor authorization and the full readiness check, this PR is ready for review. Maintainer review and any further standard CI admission remain separate. Feedback on the explicit recovery boundary would be welcome.

Codex assisted with the refresh and verification. I reviewed the candidate and authorized its DCO sign-off and publication.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-01T03:43:09.631328Z
We couldn’t complete this review. Comment @codex review to try again.
2a9d3ab Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@vllm-omni-review-bot

Copy link
Copy Markdown

This PR appears to belong to: docs/design/module/benchmarking.md.

Module owners: @alex-jw-brooks @Bounty-hunter @y-null

Routing: @alex-jw-brooks via module of the changed files, CODEOWNERS; @Bounty-hunter via module of the changed files, CODEOWNERS; @y-null via model owner

@LOGO127, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

…ction

Integrate main without rewriting prior history. Forward the format follow-up,
retain the accepted frame budget, and keep cumulative reduction metadata.
Add regressions for temporal, content and video fallback composition.

Signed-off-by: LOGO127 <luozijian0924@gmail.com>
@vllm-omni-review-bot

Copy link
Copy Markdown

Omni ReviewBot triage note

Automated triage of commit e516da99ada8 produced:

  • Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot routing record

Assigned Strict on claude-code (mimo-v2.6-pro) under experiment fleet-strict-claudecode-mimo26pro-100-c10-20261015.

@vllm-omni-review-bot

vllm-omni-review-bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Omni ReviewBot review status

Review for commit e516da99ada8c858208dd78ee7a7ea2ea9635057 ended as timeout (Strict attempt outlived its budget; no fallback: the routing table has no fallback arm).

@vllm-omni-review-bot

Copy link
Copy Markdown
Omni ReviewBot routing record

Assigned Strict on zcode (GLM-5.3-Flash) under experiment fleet-strict-glm-qwen38flash-5050-c4-20261105.

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

Labels

benchmark/profiler/metrics/logger Codes related to benchmarks, profiler, metrics and logger system bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants