diff --git a/crates/aionui-ai-agent/src/session_agent.rs b/crates/aionui-ai-agent/src/session_agent.rs index d40517355..e88acc324 100644 --- a/crates/aionui-ai-agent/src/session_agent.rs +++ b/crates/aionui-ai-agent/src/session_agent.rs @@ -187,6 +187,27 @@ struct CatalogPreload { current_mode: Option, } +/// Per-model reasoning efforts out of the persisted `available_models` column. +/// +/// The catalog is written by `catalog_partial_from_caps` as +/// `{available_models:[{id,label,reasoning_efforts?}]}`. Rows written before +/// that field existed simply have none, so a stale catalog degrades to "no +/// effort axis" rather than failing to parse. +fn efforts_for_model(available_models: Option<&serde_json::Value>, model_id: &str) -> Vec { + let Some(list) = available_models + .and_then(|v| v.get("available_models")) + .and_then(|v| v.as_array()) + else { + return Vec::new(); + }; + list.iter() + .find(|entry| entry.get("id").and_then(|v| v.as_str()) == Some(model_id)) + .and_then(|entry| entry.get("reasoning_efforts")) + .and_then(|v| v.as_array()) + .map(|efforts| efforts.iter().filter_map(|e| e.as_str().map(str::to_owned)).collect()) + .unwrap_or_default() +} + impl CatalogPreload { /// Parse the persisted handshake's `available_models` / `available_modes` /// columns into the live-capabilities shape. Reuses the ACP path's @@ -210,7 +231,13 @@ impl CatalogPreload { id: m.model_id.to_string(), name: m.name.clone(), description: m.description.clone(), - reasoning_efforts: Vec::new(), + // Read straight from the stored JSON: the ACP + // `SessionModelState` this comes from has no effort + // axis, so the parser cannot carry it through. + reasoning_efforts: efforts_for_model( + handshake.available_models.as_ref(), + &m.model_id.to_string(), + ), }) .collect::>(); let current = state.current_model_id.to_string(); @@ -1234,6 +1261,32 @@ pub struct SessionBuildInputs<'a> { /// /// Aliases are normalized to the backend-native id here, so callers compare /// against catalog values rather than whatever spelling reached the API. +/// The reasoning effort a session should open with: what `set_config_option` +/// persisted, else the create-time seed. +/// +/// Mirrors the `mode`/`model` precedence — snapshot over seed — and is a free +/// function so the precedence is testable without standing up a backend. +/// +/// NOT claude-scoped. It was, on the grounds that "codex effort rides +/// collaborationMode via SetMode"; measured false — codex accepts +/// `SetConfigOption{effort|reasoning_effort|thought_level}` and writes +/// `thread/settings/update {"effort":…}`. The gate meant a codex conversation +/// persisted its effort and lost it on every rebuild. +pub(crate) fn resolved_effort( + config: &aionui_api_types::AcpBuildExtra, + session_snapshot: Option<&PersistedSessionState>, +) -> Option { + session_snapshot + .and_then(|s| { + s.config_selections + .iter() + .find(|(k, _)| k.as_str() == EFFORT_CONFIG_KEY) + .map(|(_, v)| v.as_str().to_owned()) + }) + .or_else(|| config.thought_level.clone()) + .filter(|value| !value.is_empty()) +} + pub(crate) fn resolved_session_mode( config: &AcpBuildExtra, session_snapshot: Option<&PersistedSessionState>, @@ -1566,20 +1619,22 @@ pub async fn build_session_instance( // #4 — the persisted reasoning-effort level (claude only). There is no spawn-time // effort flag (effort rides a post-open control_request, NOT `--`args like // model/mode), so it cannot go into `SessionConfig`; instead we re-apply it AFTER - // open. codex effort is not a standalone selection (it rides collaborationMode via - // SetMode), so this is claude-scoped. Read from the snapshot's config_selections - // (the map `set_config_option` persisted under EFFORT_CONFIG_KEY). - let persisted_effort = (backend_label == "claude") - .then(|| { - session_snapshot.and_then(|s| { - s.config_selections - .iter() - .find(|(k, _)| k.as_str() == EFFORT_CONFIG_KEY) - .map(|(_, v)| v.as_str().to_owned()) - }) - }) - .flatten() - .filter(|s| !s.is_empty()); + // open. Snapshot first (what `set_config_option` persisted under + // EFFORT_CONFIG_KEY), then the create-time seed — the same precedence + // `mode` and `model` above already use. + // + // NOT claude-scoped. It was, on the grounds that "codex effort rides + // collaborationMode via SetMode" — measured false: codex accepts + // `SetConfigOption{effort|reasoning_effort|thought_level}` and writes + // `thread/settings/update {"effort":"high"}` (codex_conn.rs, and confirmed + // live through the HTTP config-options endpoint). The gate meant a codex + // conversation persisted its effort and then silently lost it on every + // rebuild. + // + // The seed leg closes the other half: `extra.thought_level` has been + // carried from the new-conversation screen all along and nothing ever read + // it, so an effort chosen before the first turn was dropped. + let persisted_effort = resolved_effort(config, session_snapshot); // DEV (`--dump-prompts`): dump the resolved SessionConfig BEFORE it moves // into open_session. Best-effort — a failure only warns, never fails open. @@ -1913,6 +1968,36 @@ fn catalog_partial_from_caps(caps: &aionui_session::Capabilities) -> Option>(), })); } + // Reasoning-effort ("thought level") select. The new-conversation screen + // builds its effort picker from `config_options` ALONE — + // `buildAgentRuntimeThoughtLevelOption` looks up category `thought_level` + // and, unlike mode, has no fallback to a top-level column. Without this the + // control is simply absent exactly where the choice is made. + // + // Offered for any backend whose models advertise efforts: claude and codex + // both do, and both accept `SetConfigOption{reasoning_effort}` (verified + // live). A backend with no effort axis — agy folds it into the model id — + // yields an empty list and no option, rather than a dead control. + let efforts = resolve_current_model_efforts(&caps.available_models, caps.current_model.as_deref()); + if !efforts.is_empty() { + config_options.push(serde_json::json!({ + "id": "reasoning_effort", + "category": "thought_level", + "type": "select", + // Deliberately NOT `caps.current_effort`. That field means "the + // level THIS session last set" — claude remembers it because the + // CLI never echoes effort back (capability.rs), and codex does not + // track it at all. This catalog is agent-level and shared by every + // conversation, so writing a session's level here would make one + // conversation's choice the default for all of them. The picker + // offers the levels; the chosen one travels per-conversation via + // `extra.thought_level`. + "currentValue": serde_json::Value::Null, + "options": efforts.iter().map(|e| serde_json::json!({ + "value": e, "name": e, + })).collect::>(), + })); + } let available_commands = if caps.slash_commands.is_empty() { None } else { @@ -1947,9 +2032,20 @@ fn catalog_partial_from_caps(caps: &aionui_session::Capabilities) -> Option>(), + "available_models": caps.available_models.iter().map(|m| { + let mut entry = serde_json::json!({ "id": m.id, "label": m.name }); + // Per-model reasoning efforts (claude's `supportedEffortLevels`). + // Dropped here until now, so the persisted catalog carried only + // {id,label} and the thought-level picker could not be rebuilt + // from it — see `CatalogPreload::from_handshake`. Omitted rather + // than written empty for models that have none, keeping the + // column byte-identical for every backend that has no effort + // axis (agy folds effort into the model id; codex has none). + if !m.reasoning_efforts.is_empty() { + entry["reasoning_efforts"] = serde_json::json!(m.reasoning_efforts); + } + entry + }).collect::>(), "current_model_id": caps.current_model, }) }); @@ -3830,6 +3926,66 @@ mod build_mapping_tests { use crate::shared_kernel::{ModeId, ModelId}; use aionui_session::SessionSpec; + fn snapshot_with_effort(effort: &str) -> PersistedSessionState { + let mut s = PersistedSessionState::default(); + s.config_selections.insert( + crate::shared_kernel::ConfigKey::new(EFFORT_CONFIG_KEY), + crate::shared_kernel::ConfigValue::new(effort), + ); + s + } + + fn extra_with_thought_level(level: Option<&str>) -> aionui_api_types::AcpBuildExtra { + aionui_api_types::AcpBuildExtra { + backend: Some("codex".into()), + thought_level: level.map(str::to_owned), + ..Default::default() + } + } + + #[test] + fn a_codex_session_restores_its_persisted_effort() { + // Was gated on `backend_label == "claude"` because "codex effort rides + // collaborationMode via SetMode". Measured false: codex accepts + // SetConfigOption{effort} and writes thread/settings/update. The gate + // meant codex persisted an effort and lost it on every rebuild. + assert_eq!( + resolved_effort(&extra_with_thought_level(None), Some(&snapshot_with_effort("high"))), + Some("high".to_owned()) + ); + } + + #[test] + fn the_create_time_effort_seed_is_applied() { + // `extra.thought_level` has been carried from the new-conversation + // screen all along and nothing read it, so an effort chosen before the + // first turn was silently dropped. + assert_eq!( + resolved_effort(&extra_with_thought_level(Some("xhigh")), None), + Some("xhigh".to_owned()) + ); + } + + #[test] + fn a_persisted_effort_outranks_the_seed() { + // Same precedence as mode/model: what the user changed mid-session wins + // over what they picked when creating it. + assert_eq!( + resolved_effort( + &extra_with_thought_level(Some("low")), + Some(&snapshot_with_effort("max")) + ), + Some("max".to_owned()) + ); + } + + #[test] + fn no_effort_anywhere_stays_none() { + assert_eq!(resolved_effort(&extra_with_thought_level(None), None), None); + // An empty seed is not a choice. + assert_eq!(resolved_effort(&extra_with_thought_level(Some("")), None), None); + } + fn snapshot(mode: Option<&str>, model: Option<&str>) -> PersistedSessionState { PersistedSessionState { current_mode_id: mode.map(ModeId::new), @@ -7921,6 +8077,143 @@ mod force_kill_tests { } } +#[cfg(test)] +mod cold_start_effort_tests { + //! A resumed conversation rebuilds its pickers from the persisted catalog + //! before the live handshake lands. That catalog carried only `{id,label}`, + //! so the thought-level group was missing until the user left the + //! conversation and came back — nothing re-publishes config options when + //! the handshake arrives. Present since #609 moved claude/codex onto the + //! direct-CLI path. + use super::*; + + fn handshake_with(models: serde_json::Value) -> aionui_api_types::AgentHandshake { + aionui_api_types::AgentHandshake { + available_models: Some(models), + ..Default::default() + } + } + + #[test] + fn a_persisted_catalog_round_trips_per_model_efforts() { + let caps = aionui_session::Capabilities { + available_models: vec![aionui_session::ModelInfo { + id: "opus".into(), + name: "Opus".into(), + description: None, + reasoning_efforts: vec!["low".into(), "high".into()], + }], + current_model: Some("opus".into()), + ..Default::default() + }; + let partial = catalog_partial_from_caps(&caps).expect("catalog"); + let preload = CatalogPreload::from_handshake(&partial); + + assert_eq!( + preload.available_models[0].reasoning_efforts, + vec!["low".to_owned(), "high".to_owned()], + "efforts must survive the write/read round trip, or a resumed \ + conversation opens without a thought-level picker" + ); + // The whole point: this is what feeds the picker. + assert_eq!( + resolve_current_model_efforts(&preload.available_models, Some("opus")), + vec!["low".to_owned(), "high".to_owned()] + ); + } + + #[test] + fn the_catalog_offers_an_effort_option() { + // The new-conversation screen builds its effort picker from + // `config_options` alone — `buildAgentRuntimeThoughtLevelOption` has no + // fallback to a top-level column the way mode does, so without this the + // control is absent exactly where the choice is made. + let caps = aionui_session::Capabilities { + available_models: vec![aionui_session::ModelInfo { + id: "gpt-5.6-sol".into(), + name: "GPT-5.6-Sol".into(), + description: None, + reasoning_efforts: vec!["low".into(), "high".into()], + }], + current_model: Some("gpt-5.6-sol".into()), + current_effort: Some("high".into()), + ..Default::default() + }; + let partial = catalog_partial_from_caps(&caps).expect("catalog"); + let effort = partial + .config_options + .as_ref() + .and_then(|v| v.as_array()) + .and_then(|a| { + a.iter() + .find(|o| o.get("category").and_then(|v| v.as_str()) == Some("thought_level")) + }) + .expect("an effort option must be offered") + .clone(); + assert_eq!(effort["id"], "reasoning_effort"); + // Agent-level catalog: a session's current level must not leak into it + // as everyone's default. + assert!(effort["currentValue"].is_null(), "{effort}"); + assert_eq!(effort["options"].as_array().unwrap().len(), 2); + } + + #[test] + fn a_backend_with_no_effort_axis_offers_no_option() { + // agy folds effort into the model id; an empty select renders a dead + // control. + let caps = aionui_session::Capabilities { + available_models: vec![aionui_session::ModelInfo { + id: "gemini-3.6-flash-low".into(), + name: "gemini-3.6-flash-low".into(), + description: None, + reasoning_efforts: Vec::new(), + }], + ..Default::default() + }; + let partial = catalog_partial_from_caps(&caps).expect("catalog"); + let has_effort = partial + .config_options + .as_ref() + .and_then(|v| v.as_array()) + .map(|a| { + a.iter() + .any(|o| o.get("category").and_then(|v| v.as_str()) == Some("thought_level")) + }) + .unwrap_or(false); + assert!(!has_effort, "{:?}", partial.config_options); + } + + #[test] + fn a_model_without_efforts_writes_no_key() { + // agy folds effort into the model id and codex has no effort axis; + // writing an empty array for them would change the stored column for + // every backend to fix one. + let caps = aionui_session::Capabilities { + available_models: vec![aionui_session::ModelInfo { + id: "gemini-3.6-flash-low".into(), + name: "gemini-3.6-flash-low".into(), + description: None, + reasoning_efforts: Vec::new(), + }], + ..Default::default() + }; + let partial = catalog_partial_from_caps(&caps).expect("catalog"); + let entry = &partial.available_models.as_ref().unwrap()["available_models"][0]; + assert!(entry.get("reasoning_efforts").is_none(), "{entry}"); + } + + #[test] + fn a_catalog_written_before_this_field_still_loads() { + // Every row already in the database predates the field. + let preload = CatalogPreload::from_handshake(&handshake_with(serde_json::json!({ + "available_models": [{"id": "opus", "label": "Opus"}], + "current_model_id": "opus", + }))); + assert_eq!(preload.available_models.len(), 1); + assert!(preload.available_models[0].reasoning_efforts.is_empty()); + } +} + #[cfg(test)] mod catalog_writeback_tests { //! When a backend discovers models LATE. agy runs `agy models` off the open diff --git a/crates/aionui-session/src/backend/antigravity/translate.rs b/crates/aionui-session/src/backend/antigravity/translate.rs index 6f1e873a6..df093db33 100644 --- a/crates/aionui-session/src/backend/antigravity/translate.rs +++ b/crates/aionui-session/src/backend/antigravity/translate.rs @@ -172,8 +172,28 @@ impl Translator { } } + /// Stable id for the step's tool card. + /// + /// NAMESPACED by agy's conversation id, because `messages.id` is unique + /// across the whole database while `step_index` only counts within one agy + /// conversation. Bare `step-` collided the moment a second conversation + /// reached the same index — observed 52 times in one log + /// ("Duplicate record: Message with id 'step-6' already exists outside the + /// requested conversation"), which silently dropped the tool card for every + /// conversation after the first to use that number. + /// + /// Falls back to the bare form only before `init` has bound a session id, + /// where there is nothing to namespace with and no second conversation to + /// collide against yet. + fn step_id(&self, step_index: u64) -> String { + match self.backend_session_id.as_deref() { + Some(session) => format!("{session}-step-{step_index}"), + None => format!("step-{step_index}"), + } + } + fn translate_step(&mut self, su: AgyStepUpdate) -> Vec { - let id = format!("step-{}", su.step_index); + let id = self.step_id(su.step_index); let mut out = Vec::new(); match su.step_type { @@ -566,6 +586,33 @@ mod tests { const SUBAGENT_ACTIVE: &str = r#"{"event":"step_update","step_update":{"step_index":3,"state":"ACTIVE","step_type":"subagent","subagent_info":{"subagents":[{"type_name":"research","role":"File Counter","initial_prompt":"Count the files.","conversation_id":"108d172f","log_uri":"file:///brain/log"}]}}}"#; const SUBAGENT_DONE: &str = r#"{"event":"step_update","step_update":{"step_index":3,"state":"DONE","step_type":"subagent","duration_seconds":0.06,"subagent_info":{"subagents":[{"type_name":"research","role":"File Counter","initial_prompt":"Count the files.","conversation_id":"108d172f","log_uri":"file:///brain/log"}]}}}"#; + #[test] + fn tool_ids_do_not_collide_across_conversations() { + // `messages.id` is unique across the whole database, but `step_index` + // only counts within one agy conversation. A bare `step-` therefore + // collided as soon as a second conversation reached the same index: + // "Duplicate record: Message with id 'step-6' already exists outside + // the requested conversation" — 52 of them in one log, each a tool card + // silently dropped. Every existing test drove a single Translator, so + // none of them could see it. + let tool_step = r#"{"event":"step_update","step_update":{"step_index":3,"state":"ACTIVE","step_type":"tool","tool_name":"run_command","tool_info":{"name":"run_command","parameters":{}}}}"#; + let id_in = |conv: &str| { + let init = format!(r#"{{"event":"init","conversation_id":"{conv}","init":{{"cwd":"/w"}}}}"#); + let (_, evs) = tr(&[&init, tool_step]); + evs.iter() + .find_map(|e| match e { + SessionEvent::ToolCall { tool_use_id, .. } => Some(tool_use_id.clone()), + _ => None, + }) + .expect("tool call") + }; + assert_ne!( + id_in("conv-a"), + id_in("conv-b"), + "the same step index in two conversations must not produce the same message id" + ); + } + #[test] fn a_subagent_dispatch_is_visible_as_a_tool_call() { // Before this arm the step fell to `Unknown` and the dispatch was