Add model reasoning level for codex.#11101
Conversation
liliwilson
force-pushed
the
lili/remote-1706-model-choice-for-codex-improvments
branch
from
e86ccec to
70713a2
Compare
| } | ||
|
|
||
| #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub struct HarnessModelConfig { |
There was a problem hiding this comment.
Added this struct, even though it's slightly diff than the serde shape we get from the API, because it makes it easier to reason about state in the driver code
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR threads third-party harness model config and Codex reasoning level through task snapshots, harness startup, GraphQL harness availability, settings persistence, and the V2 cloud model selector UI.
Concerns
⚠️ [IMPORTANT] This changes user-facing model selector behavior, but the PR description does not include an accessible screenshot or screen recording; the Loom line is empty and does not link to media. For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.- 💡 [SUGGESTION] Existing saved third-party harness model selections were stored as string values, so the new persisted object shape should accept or migrate legacy values to avoid silently dropping users' saved model choices.
Verdict
Found: 0 critical, 1 important, 1 suggestion
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| }, | ||
| last_selected_harness_model: LastSelectedHarnessModel { | ||
| type: HashMap<String, String>, | ||
| type: HashMap<String, HarnessModelSelection>, |
There was a problem hiding this comment.
💡 [SUGGESTION] Existing last_selected_harness_model entries are stored as HashMap<String, String> in private prefs. Changing the value type without legacy parsing means users with saved selections fall back to the default map on next load, so add a migration/deserializer that accepts old string values.
There was a problem hiding this comment.
I thought about this, but seeing as it hasn't hit prod yet I don't think this is worth doing. The new HarnessModelSelection struct also adds a layer of indirection so we can more easily add fields to this later without needing a legacy handling story
abhishekp106
left a comment
abhishekp106
left a comment
There was a problem hiding this comment.
thanks for grabbing this!
| } | ||
|
|
||
| #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] | ||
| pub struct HarnessModelConfig { |
liliwilson
force-pushed
the
lili/remote-1706-model-choice-for-codex-improvments
branch
from
d7f1049 to
50de452
Compare
liliwilson
deleted the
lili/remote-1706-model-choice-for-codex-improvments
branch
2 participants
Description
This PR adds a few improvements to the model selector for third-party cloud agents:
model_reasoning_effortin the codex configdefaultoption is always accessible via the dropdownTesting
Tested locally with different harnesses and model selection choices.
./script/runScreenshots / Videos
Loom attached to the associated server PR: https://github.com/warpdotdev/warp-server/pull/11224
Agent Mode