fix: use eval config model in job id#738
Conversation
xeophon
requested review from
JannikSt,
JohannesHa,
burnpiro,
d42me,
kcoopermiller and
willccbb
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1f95f43ac
ℹ️ 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".
| if job_target is None: | ||
| job_target = Path(environment).stem | ||
| if config_envs and _parse_value_option(passthrough_args, "--model", "-m") is None: | ||
| model = toml.load(environment).get("model") or model |
There was a problem hiding this comment.
For config-driven runs where the TOML has at least one [[eval]] entry and no CLI --model, this assigns any truthy model value directly into model; if the user has a malformed but parseable config such as model = 123 or a table, _build_job_id() immediately calls .replace() on a non-string and the CLI crashes before verifiers can report the configuration error. Please only accept a non-empty string here, otherwise fall back or raise a clear typer.Exit validation message.
Useful? React with 👍 / 👎.
1 participant
Summary
when you don't specify the model name in the CLI (but rather in the TOML), the prime CLI used the DEFAULT_MODEL (gpt-4.1 mini) instead of the TOML value. this fixes it
Note
Low Risk
Small, localized change to job id metadata for config evals; no auth, billing, or inference routing changes.
Overview
For config-driven eval runs (TOML with
[[eval]]entries), theX-PI-Job-Idheader now uses the top-levelmodelfrom that config when building the job id, instead of always using the default/CLI-resolved model from_add_default_inference_and_key_args.An explicit
--model/-mon the original passthrough args still wins; the TOMLmodelis only applied when that override is absent.Reviewed by Cursor Bugbot for commit d1f95f4. Bugbot is set up for automated code reviews on this repo. Configure here.