Skip to content

feat(eval): add GPT-5.6 Luna and Terra models - #44

Closed
vincentkoc wants to merge 1 commit into
fix/native-gpt55-highfrom
bench/gpt56-variants
Closed

feat(eval): add GPT-5.6 Luna and Terra models#44
vincentkoc wants to merge 1 commit into
fix/native-gpt55-highfrom
bench/gpt56-variants

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

What does this PR do?

Adds GPT-5.6 Luna and Terra to the native ShellBench model catalog and applies configured reasoning effort to all OpenAI model specs.

Why?

The GPT-5.6 variant benchmark needs exact provider IDs and explicit high reasoning metadata instead of ad hoc model overrides. This PR is stacked on #43 because it extends that native runner work.

Changes

  • add gpt56-luna mapped to gpt-5.6-luna
  • add gpt56-terra mapped to gpt-5.6-terra
  • apply proxy reasoning effort to OpenAI provider models generically
  • update the expected native matrix size to 96 runs
  • document the new model support in the changelog

Tests

  • python -m pytest -q passes locally
  • 54 focused native runner tests pass
  • python -m ruff check clawbench app.py scripts tests passes locally

Full six-run AWS evidence will be published in a separate ShellBench/public-tasks results PR.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 28, 2026
@clawsweeper

clawsweeper Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 28, 2026, 1:04 AM ET / 05:04 UTC.

ClawSweeper review

What this changes

This PR adds GPT-5.6 Luna and Terra to ShellBench’s native evaluation catalog, gives every OpenAI proxy entry configured reasoning effort, and updates the native matrix expectations from 72 to 96 runs.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep this PR open. It is a member-authored draft stacked on an open native-runner PR, and the provided evidence does not establish that current main already contains the requested catalog entries or that the expanded default benchmark matrix has received final maintainer approval.

Priority: P2
Reviewed head: 96b6c9363d76fdbded679287bcc5ac0bd2042449
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The patch is focused and has passing focused checks, with final confidence appropriately depending on the stacked runner work and live provider validation.
Proof confidence 🌊 off-meta tidepool Not applicable: This member-authored PR is not subject to the external-contributor proof gate; however, a real native run remains the most useful merge-time confirmation for the new provider IDs and expanded matrix.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This member-authored PR is not subject to the external-contributor proof gate; however, a real native run remains the most useful merge-time confirmation for the new provider IDs and expanded matrix.
Evidence reviewed 4 items Proposed catalog and proxy change: The PR adds gpt56-luna and gpt56-terra to the native model catalog, and changes proxy configuration from a two-slug condition to all OpenAI provider models.
Matrix expansion is intentional in the branch: The proposed tests update both planned-run and run-index expectations from 72 to 96, so this is an operator-visible default benchmark expansion rather than only an alias addition.
Focused validation reported: The PR body reports 54 focused native-runner tests and Ruff passing; GitHub context also reports successful Python 3.11 and 3.12 test-suite checks on the current head.
Findings None None.
Security None None.

How this fits together

ShellBench’s native evaluator takes model catalog entries and harness definitions to generate benchmark runs. It writes a LiteLLM proxy configuration for each provider model, then the matrix planner schedules the resulting runs and records their metadata.

flowchart LR
  A[Native model catalog] --> B[Proxy configuration]
  C[Reasoning-effort setting] --> B
  B --> D[LiteLLM provider requests]
  A --> E[Matrix planner]
  F[Harness definitions] --> E
  E --> G[96 scheduled native runs]
  D --> H[Evaluation results and manifests]
  G --> H
Loading

Decision needed

Question Recommendation
Should GPT-5.6 Luna and Terra become part of ShellBench’s default native evaluation matrix, increasing a full run from 72 to 96 scheduled runs? Approve the expanded default matrix: Merge after the stacked native-runner work lands and a real native run confirms both provider IDs and the configured reasoning effort.

Why: The code change is narrow, but adding models changes the default benchmark scope, provider-call volume, and required availability assumptions; that is a maintainer-owned evaluation-policy choice.

Before merge

  • Resolve merge risk (P1) - Adding two catalog entries raises the standard native matrix from 72 to 96 runs, increasing provider calls and requiring the announced model IDs to be available to operators using the default full matrix.
  • Resolve merge risk (P1) - The branch depends on the unmerged behavior in fix(eval): stabilize GPT-5.5 native harness runs #43, so its final merge result and operational proof should be reviewed after that dependency is settled.
  • Complete next step (P2) - This needs maintainer confirmation of default benchmark scope and merge ordering with the stacked native-runner PR, not an automated repair.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Native matrix size 72 → 96 runs Two added models expand the default evaluation schedule by 24 runs across the existing harness and repetition matrix.
Patch scope 5 files, 24 added, 4 removed The implementation is focused on catalog metadata, proxy generation, and corresponding expectations.

Merge-risk options

Maintainer options:

  1. Confirm the default-matrix expansion before merge (recommended)
    Verify both GPT-5.6 provider IDs in a real native run after the stacked dependency lands, then merge if the 96-run default is intentional.
  2. Make the variants opt-in
    Preserve the existing default run scope while exposing Luna and Terra through an explicit model-selection path.
  3. Pause the catalog addition
    Leave the variants out until maintainers decide that their benchmark value justifies the additional default provider calls.

Technical review

Best possible solution:

Land the native-runner foundation first, then confirm the two provider IDs in a real native run and merge this focused catalog expansion only if maintainers intend the 96-run default matrix.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a defect reproduction: this PR proposes new catalog coverage rather than reporting broken existing behavior. The relevant verification is a real native run resolving both added provider IDs with the generated proxy configuration.

Is this the best way to solve the issue?

Unclear. The catalog and generic OpenAI reasoning-effort rule are narrow, but whether these variants should alter the default 72-run matrix requires maintainer confirmation and live provider validation after the stacked dependency lands.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 28ce18e5dfc8.

Labels

Label changes:

  • add P2: This is a bounded evaluation-catalog feature with a concrete operator-cost and default-scope decision, but no demonstrated active user outage.
  • add merge-risk: 🚨 compatibility: Changing the catalog’s default matrix from 72 to 96 runs can alter existing operators’ provider usage, cost, and availability expectations after upgrade.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This member-authored PR is not subject to the external-contributor proof gate; however, a real native run remains the most useful merge-time confirmation for the new provider IDs and expanded matrix.

Label justifications:

  • P2: This is a bounded evaluation-catalog feature with a concrete operator-cost and default-scope decision, but no demonstrated active user outage.
  • merge-risk: 🚨 compatibility: Changing the catalog’s default matrix from 72 to 96 runs can alter existing operators’ provider usage, cost, and availability expectations after upgrade.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This member-authored PR is not subject to the external-contributor proof gate; however, a real native run remains the most useful merge-time confirmation for the new provider IDs and expanded matrix.

Evidence

What I checked:

Likely related people:

  • vincentkoc: Authored this branch and the related open native-runner work that this PR explicitly extends; no separate current-main ownership trail was available in the supplied review evidence. (role: stacked native-evaluation contributor; confidence: low; commits: 96b6c9363d76, 822e8f9198b3; files: scripts/native_eval/models.py, scripts/native_eval/proxy.py, tests/test_native_eval_proxy.py)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@vincentkoc

Copy link
Copy Markdown
Member Author

Superseded by #46, which replayed and landed this branch’s still-unmerged commits after the parent squash made the protected stack impossible to update in place.

@vincentkoc vincentkoc closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant