From 0e696d317ce2f3057f7f2960c07a1b4709ac47c8 Mon Sep 17 00:00:00 2001 From: Chris Murphy Date: Sun, 30 Aug 2026 10:35:12 +0800 Subject: [PATCH] Apply agent profile reasoning/temperature frontmatter (GH #828) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reasoning and temperature frontmatter keys were parsed into AgentDef but never read. Wire them into the main loop: - /agent applies the profile's reasoning via session.effort_override (the same mechanism /effort uses, so rebuilds keep it sticky), capturing the pre-profile override in ContextFiles.effort_before_agent — the effort sibling of route_before_agent. /agent off restores it. - The profile wins over a live /effort override at activation; a later /effort overrides it; /agent off restores the pre-profile value, mirroring how the model restore discards a mid-profile /model. - An unrecognised reasoning value warns and leaves effort untouched; the switch proceeds (fail-soft, matching /effort's unknown-level handling). - temperature is consulted from the active agent layer at agent build time (profile > CLI > config), so /agent's rebuild applies it and /agent off's rebuild falls back automatically. Same clamp/warn as before. - Subagent path unchanged (still model + prompt only); docs updated: the key table now lists all seven /effort levels and the new semantics. Verified end-to-end with the wire dump: turn under the profile flips to model=claude-haiku-4-5 reasoning=false, /agent off returns to claude-sonnet-5 reasoning=true. --- docs/agents.md | 7 +- src/agent/builder/agent_inner.rs | 14 +- src/agent/builder/reminder_tests.rs | 4 + src/context/mod.rs | 10 ++ src/provider/mod_tests.rs | 2 + src/ui/slash/cmd/agent/clear.rs | 28 ++++ src/ui/slash/cmd/agent/switch.rs | 190 ++++++++++++++++++++++++++++ 7 files changed, 249 insertions(+), 6 deletions(-) diff --git a/docs/agents.md b/docs/agents.md index f42766418..167316dc9 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -51,8 +51,8 @@ All frontmatter keys are optional: | `model` | A `providers` alias **or** a model name. Resolved to a model string for the current client (see *Model routing* below). Omit to keep the current model. | | `deny_tools` | Tools to deny while this profile is active (e.g. `[bash, write, edit, apply_patch]`). | | `allow_tools` | The complement: deny every built-in **not** listed. `deny_tools` wins if both are given. | -| `reasoning` | Reasoning-effort hint (`low` / `medium` / `high`). | -| `temperature` | Sampling temperature. | +| `reasoning` | Reasoning effort applied on activation (`off` / `minimal` / `low` / `medium` / `high` / `xhigh` / `max` — the same levels as `/effort`). Wins over any live `/effort` override at activation; a later `/effort` overrides it; `/agent off` restores the pre-profile effort. An unrecognised value warns and leaves the session's effort unchanged. | +| `temperature` | Sampling temperature while the profile is active (clamped to `0.0..=2.0`). Wins over `--temperature` and the config default; `/agent off` falls back to those. | | `description` | One-line summary shown in `/agents`. | | `subagent_tools` | Opt this profile's `task(agent=…)` subagent into tools: `readonly` (read-only tool universe), `readwrite` (readonly + write/edit/bash — can edit the repo), or `toolless` (the default one-shot). See *Tooled subagents* below. | | `subagent_max_turns` | Cap the tooled subagent's loop (default `25`). | @@ -155,7 +155,8 @@ Profiles are resolved into subagent routes once at startup. By default subagents are **tool-less** (a one-shot query — a profile's `deny_tools`/`allow_tools` doesn't apply, since the subagent has no tools), and the profile's `reasoning`/`temperature` aren't applied on the subagent -path — only the model and system prompt are. Routing `/plan` phases to named +path — only the model and system prompt are (the MAIN loop applies both when +the profile is activated with `/agent `; see the key table above). Routing `/plan` phases to named profiles, and cross-provider client switching, remain follow-ups. ### Tooled subagents (opt-in) diff --git a/src/agent/builder/agent_inner.rs b/src/agent/builder/agent_inner.rs index 87f8d0158..f836881b8 100644 --- a/src/agent/builder/agent_inner.rs +++ b/src/agent/builder/agent_inner.rs @@ -316,9 +316,17 @@ pub async fn build_agent_inner( let max_turns = cli.resolve_max_agent_turns(cfg); builder = builder.default_max_turns(max_turns); - // Temperature: CLI > config > unset. Previously only `cli.temperature` - // was checked, so users couldn't set a default in config.json. - if let Some(temp) = cli.resolve_temperature(cfg) { + // Temperature: active agent profile > CLI > config > unset. Previously + // only `cli.temperature` was checked, so users couldn't set a default in + // config.json. The profile tier (GH #828) reads the `/agent` layer's + // `temperature` frontmatter — parsed since the key was introduced but + // never consumed. Consulting the layer HERE (rather than a runtime + // setter, which `AnyAgent` has none of for temperature) means `/agent + // `'s rebuild picks it up and `/agent off`'s rebuild, with the + // layer cleared, falls straight back to CLI/config — no capture/restore + // needed. A profile omitting the key changes nothing. + let profile_temp = context.agent_layer.as_ref().and_then(|d| d.temperature); + if let Some(temp) = profile_temp.or_else(|| cli.resolve_temperature(cfg)) { let clamped = temp.clamp(0.0, 2.0); if (clamped - temp).abs() > f64::EPSILON { // Warn ONCE per process if the user's value was clamped diff --git a/src/agent/builder/reminder_tests.rs b/src/agent/builder/reminder_tests.rs index 796a76c1d..1bbce5cb6 100644 --- a/src/agent/builder/reminder_tests.rs +++ b/src/agent/builder/reminder_tests.rs @@ -539,6 +539,7 @@ async fn build_agent_inner_emits_assembled_preamble() { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, }; // Real openai client/model — never called (no network until // first request). The builder only inspects type bounds and @@ -687,6 +688,7 @@ async fn preamble_lists_global_tier_skills() { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, }; let client = openai::Client::new("test-key").expect("openai client builds"); let model = client.completion_model("gpt-4o"); @@ -768,6 +770,7 @@ async fn no_skills_suppresses_the_preamble_catalog() { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, }; let client = openai::Client::new("test-key").expect("openai client builds"); let model = client.completion_model("gpt-4o"); @@ -816,6 +819,7 @@ async fn steering_fragment_tracks_active_model_not_cli() { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, }; let client = openai::Client::new("test-key").expect("openai client builds"); diff --git a/src/context/mod.rs b/src/context/mod.rs index c41a1830e..4d8e8266f 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -60,6 +60,14 @@ pub struct ContextFiles { /// leave the pre-agent model pointed at the profile's provider /// (dirge-fhr5). `None` when no agent is active. pub route_before_agent: Option, + /// The `session.effort_override` in force when an agent profile first + /// applied its `reasoning` frontmatter (GH #828), so `/agent off` can + /// restore it — the effort sibling of `route_before_agent`. Outer + /// `None` = nothing captured (no active profile has applied + /// `reasoning`); `Some(None)` = there was no override before the + /// profile (restore falls back to the provider config default on + /// rebuild). + pub effort_before_agent: Option>, } /// The `/prompt`-selected layer: a named mode with a body and its own @@ -191,6 +199,7 @@ pub fn load(no_context_files: bool) -> ContextFiles { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, } } @@ -306,6 +315,7 @@ mod composition_tests { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, } } diff --git a/src/provider/mod_tests.rs b/src/provider/mod_tests.rs index b0dc3442b..d8c9d53a8 100644 --- a/src/provider/mod_tests.rs +++ b/src/provider/mod_tests.rs @@ -1702,6 +1702,7 @@ async fn cerebras_identity_survives_client_model_and_agent_construction() { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, }; let agent = build_agent( model, @@ -1768,6 +1769,7 @@ async fn provider_effort_config_seeds_agent_reasoning() { prompt_layer: None, agent_layer: None, route_before_agent: None, + effort_before_agent: None, }; let agent = build_agent( model, diff --git a/src/ui/slash/cmd/agent/clear.rs b/src/ui/slash/cmd/agent/clear.rs index 64e09de71..0676a44e6 100644 --- a/src/ui/slash/cmd/agent/clear.rs +++ b/src/ui/slash/cmd/agent/clear.rs @@ -33,6 +33,20 @@ pub(crate) async fn cmd_agent_clear(ctx: &mut SlashCtx<'_>) -> anyhow::Result<() )); } + // Restore the pre-profile effort override captured when a profile's + // `reasoning` frontmatter was applied (GH #828) — the effort sibling of + // the route restore above, and deliberately independent of it: a refused + // model restore must not leave the profile's reasoning behind. Restoring + // the pre-profile value also discards any `/effort` issued WHILE the + // profile was active, exactly as the route restore discards a mid-profile + // `/model`. Done before `rebuild_agent` so the rebuild installs the + // restored override (or, for `Some(None)`, re-seeds the provider config + // default) on the live agent. + restore_profile_reasoning( + &mut ctx.session.effort_override, + &mut ctx.context.effort_before_agent, + ); + rebuild_agent(ctx).await; if let Some(err) = restore_error { @@ -47,3 +61,17 @@ pub(crate) async fn cmd_agent_clear(ctx: &mut SlashCtx<'_>) -> anyhow::Result<() ctx.renderer.write_line(&msg, c_agent())?; Ok(()) } + +/// Put `session.effort_override` back to the value captured when an agent +/// profile first applied its `reasoning` frontmatter (GH #828). A no-op +/// when nothing was captured (no active profile ever set `reasoning`), so a +/// profile without the key leaves the session's effort untouched in both +/// directions. +pub(crate) fn restore_profile_reasoning( + effort_override: &mut Option, + effort_before_agent: &mut Option>, +) { + if let Some(prior) = effort_before_agent.take() { + *effort_override = prior; + } +} diff --git a/src/ui/slash/cmd/agent/switch.rs b/src/ui/slash/cmd/agent/switch.rs index eb373d45d..a95d2c695 100644 --- a/src/ui/slash/cmd/agent/switch.rs +++ b/src/ui/slash/cmd/agent/switch.rs @@ -58,6 +58,33 @@ pub(crate) async fn cmd_agent_switch(ctx: &mut SlashCtx<'_>, arg: &str) -> anyho } } + // Apply the profile's `reasoning` frontmatter (GH #828) — parsed since + // the key was introduced but never consumed. It layers exactly like the + // model: the profile's level wins at activation (over any live `/effort` + // override), a later `/effort` overrides it as it would any current + // level, and `/agent off` restores what was captured here. Applied even + // when the model route was refused above — the profile stays active in + // that case, so its reasoning does too. Written to + // `session.effort_override` (not `set_reasoning` directly) so the + // `rebuild_agent` below installs it on the live agent and every later + // rebuild keeps it sticky, the same way `/effort` survives `/model`. + let mut applied_effort = None; + if let Some(raw) = def.reasoning.as_deref() { + match apply_profile_reasoning( + raw, + &mut ctx.session.effort_override, + &mut ctx.context.effort_before_agent, + ) { + Ok(level) => applied_effort = Some(level), + // Fail soft: warn and leave the session's effort alone, the same + // way `/effort` treats an unknown level. Never abort the switch — + // the rest of the profile is still valid. + Err(msg) => ctx + .renderer + .write_line(&format!("agent '{}': {msg}", def.name), c_error())?, + } + } + rebuild_agent(ctx).await; let mut summary = format!("active agent: {}", def.name); @@ -67,6 +94,169 @@ pub(crate) async fn cmd_agent_switch(ctx: &mut SlashCtx<'_>, arg: &str) -> anyho if let Some(alias) = &switched_to { summary.push_str(&format!(" · {alias}")); } + if let Some(level) = applied_effort { + summary.push_str(&format!(" · effort {}", level.effort_label())); + } ctx.renderer.write_line(&summary, c_agent())?; Ok(()) } + +/// Apply an agent profile's `reasoning` frontmatter value to the session's +/// effort override (GH #828), capturing the pre-profile override on the +/// FIRST profile application so `/agent off` can restore it. The capture +/// guard is `effort_before_agent`, not the agent layer: profile A without a +/// `reasoning` key followed by profile B with one must capture at B, and +/// A-with-B-with must keep A's capture (the pre-agent value) — mirroring +/// how `route_before_agent` holds the pre-agent route across profile hops. +/// +/// Returns the level applied, or `Err` with a warning message (worded like +/// `/effort`'s unknown-level error) when `raw` is not a recognised level — +/// in which case NOTHING is touched: no capture, no override change. +pub(crate) fn apply_profile_reasoning( + raw: &str, + effort_override: &mut Option, + effort_before_agent: &mut Option>, +) -> Result { + use crate::agent::agent_loop::types::ThinkingLevel; + let Some(level) = ThinkingLevel::from_effort_str(raw) else { + return Err(format!( + "unknown reasoning `{}` — expected off/minimal/low/medium/high/xhigh/max; \ + leaving effort unchanged", + raw.trim(), + )); + }; + if effort_before_agent.is_none() { + *effort_before_agent = Some(*effort_override); + } + *effort_override = Some(level); + Ok(level) +} + +#[cfg(test)] +mod tests { + use super::super::clear::restore_profile_reasoning; + use super::apply_profile_reasoning; + use crate::agent::agent_loop::types::ThinkingLevel; + + // GH #828: a profile's `reasoning` is applied on activation and the + // pre-profile state (no override) is captured for `/agent off`. + #[test] + fn profile_reasoning_is_applied_and_prior_state_captured() { + let mut over = None; + let mut before = None; + let applied = apply_profile_reasoning("low", &mut over, &mut before); + assert_eq!(applied, Ok(ThinkingLevel::Low)); + assert_eq!(over, Some(ThinkingLevel::Low)); + assert_eq!(before, Some(None), "must capture 'no prior override'"); + } + + // Precedence at activation: the profile wins over a live `/effort` + // override, the same way its model wins over a `/model` choice — and + // the displaced override is what `/agent off` will restore. + #[test] + fn profile_reasoning_wins_over_live_effort_override_at_activation() { + let mut over = Some(ThinkingLevel::Max); + let mut before = None; + let applied = apply_profile_reasoning("low", &mut over, &mut before); + assert_eq!(applied, Ok(ThinkingLevel::Low)); + assert_eq!(over, Some(ThinkingLevel::Low)); + assert_eq!(before, Some(Some(ThinkingLevel::Max))); + } + + // `/agent off` restores the pre-profile override. + #[test] + fn restore_returns_the_pre_profile_override() { + let mut over = Some(ThinkingLevel::High); + let mut before = None; + apply_profile_reasoning("off", &mut over, &mut before).unwrap(); + restore_profile_reasoning(&mut over, &mut before); + assert_eq!(over, Some(ThinkingLevel::High)); + assert_eq!(before, None, "capture must be consumed"); + } + + // `/agent off` after a profile applied over NO prior override restores + // "no override" (the rebuild then re-seeds the provider config default). + #[test] + fn restore_returns_no_override_when_there_was_none_before() { + let mut over = None; + let mut before = None; + apply_profile_reasoning("medium", &mut over, &mut before).unwrap(); + restore_profile_reasoning(&mut over, &mut before); + assert_eq!(over, None); + assert_eq!(before, None); + } + + // A `/effort` issued WHILE the profile is active is discarded by + // `/agent off` in favour of the pre-profile value — mirroring how the + // route restore discards a mid-profile `/model`. + #[test] + fn restore_discards_a_mid_profile_effort_change() { + let mut over = Some(ThinkingLevel::Medium); + let mut before = None; + apply_profile_reasoning("low", &mut over, &mut before).unwrap(); + over = Some(ThinkingLevel::Xhigh); // user ran `/effort xhigh` mid-profile + restore_profile_reasoning(&mut over, &mut before); + assert_eq!(over, Some(ThinkingLevel::Medium)); + } + + // A profile that omits `reasoning` never calls apply — so on `/agent + // off` there is no capture, and restore must change NOTHING (a + // key-less profile leaves effort alone in both directions). + #[test] + fn restore_without_a_capture_is_a_no_op() { + let mut over = Some(ThinkingLevel::Max); + let mut before = None; + restore_profile_reasoning(&mut over, &mut before); + assert_eq!(over, Some(ThinkingLevel::Max)); + } + + // An invalid value fails soft: warn (the Err), touch nothing, never + // abort the switch. + #[test] + fn invalid_reasoning_value_changes_nothing() { + let mut over = Some(ThinkingLevel::High); + let mut before = None; + let res = apply_profile_reasoning("turbo", &mut over, &mut before); + assert!(res.is_err()); + assert!(res.unwrap_err().contains("unknown reasoning `turbo`")); + assert_eq!(over, Some(ThinkingLevel::High), "override untouched"); + assert_eq!(before, None, "no capture on failure"); + } + + // Hopping profile A -> profile B keeps A's capture: the value `/agent + // off` restores is the PRE-AGENT one, exactly as `route_before_agent` + // holds the pre-agent route across profile hops. + #[test] + fn profile_hop_keeps_the_pre_agent_capture() { + let mut over = Some(ThinkingLevel::Minimal); + let mut before = None; + apply_profile_reasoning("high", &mut over, &mut before).unwrap(); + apply_profile_reasoning("max", &mut over, &mut before).unwrap(); + assert_eq!(over, Some(ThinkingLevel::Max)); + assert_eq!(before, Some(Some(ThinkingLevel::Minimal))); + restore_profile_reasoning(&mut over, &mut before); + assert_eq!(over, Some(ThinkingLevel::Minimal)); + } + + // All seven `/effort` levels are accepted — the profile key must never + // diverge from `/effort`'s vocabulary (they share the parser). + #[test] + fn all_seven_effort_levels_parse() { + for (raw, want) in [ + ("off", ThinkingLevel::Off), + ("minimal", ThinkingLevel::Minimal), + ("low", ThinkingLevel::Low), + ("medium", ThinkingLevel::Medium), + ("high", ThinkingLevel::High), + ("xhigh", ThinkingLevel::Xhigh), + ("max", ThinkingLevel::Max), + ] { + let mut over = None; + let mut before = None; + assert_eq!( + apply_profile_reasoning(raw, &mut over, &mut before), + Ok(want) + ); + } + } +}