add option snapshots and adapt GUI orchestration pickers onto them#13714
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
0e8169a to
a1da335
Compare
22bc7be to
97993fd
Compare
|
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 extracts orchestration configuration option-list construction into frontend-neutral snapshots and rewires the GUI picker population paths to render from those snapshots. I reviewed the attached diff, PR description, and spec context; no approved spec context was available, and I found no security findings.
Concerns
- The PR changes user-facing GUI picker rendering and selection behavior, but the description does not include screenshots or a screen recording demonstrating the pickers working end to end. For this user-facing change, please include screenshots or a short recording covering the affected orchestration pickers so reviewers can verify the intended parity.
Verdict
Found: 0 critical, 0 important, 1 suggestions
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
6a11bfe to
bf39ff7
Compare
23b2eb1 to
932a2bf
Compare
Co-Authored-By: Oz <oz-agent@warp.dev>
Option-taking from_run_agents_fields (churn all callers), extract oz_model_menu_items helper, clearer module/fn docs, drop export comments. Co-Authored-By: Oz <oz-agent@warp.dev>
bf39ff7 to
9f1f151
Compare
Drop selection-lookup and enum-mapping tests; keep ordering, filtering, dedupe, stale-cache, and don't-clobber-selection policy tests. Co-Authored-By: Oz <oz-agent@warp.dev>
9f1f151 to
8ecadc5
Compare
moirahuang
left a comment
moirahuang
left a comment
There was a problem hiding this comment.
i know not all of the comments are bc of your newly added code! but they came up when i used /readout so i figured i'd flag. going to approve to unblock though
| known_slugs.push(slug); | ||
| } | ||
| if let Some(slug) = recent_host.filter(|s| !s.trim().is_empty()) { | ||
| if !known_slugs |
There was a problem hiding this comment.
i don't fully understand this logic here. it seems like we're saying if the slug isn't known we label it as recent?
There was a problem hiding this comment.
recent_host is already sourced from the persisted last_selected_host; this condition isn’t inferring recency from an unknown slug. It only checks whether that recent slug was already added as the default, warp, or a connected host. If so, the earlier classification wins; otherwise we add the separate recent row.
Agreed though that known_slugs makes this unclear — I'll rename to added_slugs and add a short comment before this block.
…arpdotdev#13707/warpdotdev#13714) Upstream sync (merge f71f181) extracted orchestration UI config into app/src/ai/orchestration/ with raw English literals — our menu_label wraps regressed in 4 files. Restore ~14 sites. Keys exist in en.yml. - snapshots.rs: 4 const->fn (DEFAULT_MODEL_LABEL, AUTH_SECRET_INHERIT_LABEL, CUSTOM_HOST_LABEL, AUTH_SECRETS_LOAD_FAILED_MESSAGE) + 5 inline wraps - providers.rs: ORCHESTRATION_ENV_NONE_LABEL const->pub fn (snake_case, lint-clean) - validation.rs: 3 inline fn-return wraps (.to_string() preserved) - config_state.rs: 1 inline method-return wrap (Option<&'static str>, no .to_string()) - mod.rs + tui_export.rs: re-exports updated (const->fn rename) - orchestration_controls.rs + snapshots_tests.rs: consumer call-sites updated - en/ru.yml: 3 new keys (custom_host, no_harnesses_available, no_models_available) Russian: custom_host=Свой хост… (custom_inference precedent ru.yml:1513), no_harnesses_available=Среды исполнения недоступны (no_tools_available precedent), no_models_available=Модели недоступны. Plans: 28ae0e1e + 215807cc (2 plan-review cycles — P 🔴=0; 2 code-review cycles — 🔴=0 🟡=0 🔵=2 cosmetic dismissed). Gate: cargo check 0 NEW warnings (5 baseline mcp/), cargo test -p i18n 12/12, orchestration tests 34/34, rustfmt clean, en/ru parity 23/23. Deferred: host_picker.rs:54 separate CUSTOM_HOST_LABEL (different module, micro-plan TBD).
2 participants
Description
Adds
app/src/ai/orchestration/snapshots.rs: one plain-dataOptionSnapshotbuilder per orchestration configuration field (location, harness, model, API key, host, environment). A snapshot carries rows, ordering, badges, disabled reasons, load status, footer affordances, and the current selection — with no GUI types — so the GUI pickers and the TUI configuration pages (upstack) render the exact same option lists and cannot drift.The GUI's picker-population helpers in
orchestration_controls.rsare rewritten to consume the snapshots instead of querying catalogs inline:populate_harness_picker,populate_model_picker_for_harness,populate_environment_picker,populate_host_picker,populate_auth_secret_picker_for_harness, andsync_picker_selectionsnow map snapshot rows to menu items and only keep genuinely GUI concerns (icons, brand colors, rich Oz model rows viaavailable_model_menu_items, dropdown trigger labels). No behavior change intended.Review guide
New Code
app/src/ai/orchestration/snapshots.rs— the snapshot types and the per-field builders. Each*_snapshot(state, ctx)reads the live catalogs and delegates to a purebuild_*_snapshotcore (inputs injected for testability). The row inclusion/ordering/disabled-reason policy here was lifted from the old picker-population code as is, so no need to read too carefully.app/src/ai/blocklist/inline_action/orchestration_controls.rs— the picker rewrites. Again, pretty much pure moves, so no need to read super carefully.Testing
./script/runAgain, no behavior change
Agent Mode