Skip to content

Accept provider total token usage in review gate#840

Merged
bingran-you merged 1 commit into
mainfrom
bry/review-total-token-usage
Jun 27, 2026
Merged

Accept provider total token usage in review gate#840
bingran-you merged 1 commit into
mainfrom
bry/review-total-token-usage

Conversation

@bingran-you

Copy link
Copy Markdown
Collaborator

Summary

  • update benchflow-experiment-review so training-ready results.jsonl rows can satisfy token usage with provider total_tokens when a provider does not expose input/output split
  • keep the gate fail-closed for rows with no token usage, malformed Prime-RL rows, missing trajectories, or missing tool definitions
  • add regression coverage for provider-total-only rows

Validation

  • uv run --extra dev --locked python -m pytest tests/test_benchflow_experiment_review_validator.py -q
  • uv run --extra dev --locked ruff check .agents/skills/benchflow-experiment-review/scripts/validate_run_artifacts.py tests/test_benchflow_experiment_review_validator.py
  • uv run --extra dev --locked ruff format --check .agents/skills/benchflow-experiment-review/scripts/validate_run_artifacts.py tests/test_benchflow_experiment_review_validator.py

Context

The MobileLite Daytona canary on BenchFlow #828 produced Prime-RL-valid results.jsonl rows and bench train validate passed, but Azure gpt-5.4-mini only exposed provider total_tokens. The review gate should accept that provider-total shape instead of requiring a split that the provider did not return.

@bingran-you
bingran-you temporarily deployed to pypi-internal-preview June 27, 2026 22:30 — with GitHub Actions Inactive

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b3bc39895

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".



def test_validator_accepts_provider_total_only_token_usage(tmp_path: Path) -> None:
"""Some providers expose only total tokens; Prime-RL can still render the row."""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Name the guarded PR in this regression test

The repo-level AGENTS.md convention requires regression tests to name the PR or commit they guard, but this new regression test only describes the provider behavior. Since this is explicitly added as regression coverage for the token-usage gate, please include the guarded PR/commit in the docstring so future maintainers can trace why the case exists.

Useful? React with 👍 / 👎.

@bingran-you
bingran-you merged commit bdf797d into main Jun 27, 2026
9 checks passed
@bingran-you
bingran-you deleted the bry/review-total-token-usage branch June 27, 2026 22:39
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.

1 participant