subagents: add effort param, default to medium, document in skill
This commit is contained in:
parent
95898338fc
commit
77c3c656b2
3 changed files with 104 additions and 12 deletions
|
|
@ -24,8 +24,8 @@ Your container's `subagent` MCP server (`hive-subagent-daemon`) runs this
|
||||||
skill's recipe for you:
|
skill's recipe for you:
|
||||||
|
|
||||||
```
|
```
|
||||||
start(name, prompt_file, model?, trigger?)
|
start(name, prompt_file, model?, effort?, trigger?)
|
||||||
continue(name, prompt, model?)
|
continue(name, prompt, model?, effort?)
|
||||||
status(name)
|
status(name)
|
||||||
interrupt(name, force?)
|
interrupt(name, force?)
|
||||||
```
|
```
|
||||||
|
|
@ -41,6 +41,37 @@ Model choice, prompt hygiene, splitting big batches, verify-then-report —
|
||||||
everything else in this skill — applies exactly the same whether you're
|
everything else in this skill — applies exactly the same whether you're
|
||||||
calling the tool or thinking through the recipe by hand.
|
calling the tool or thinking through the recipe by hand.
|
||||||
|
|
||||||
|
## Effort
|
||||||
|
|
||||||
|
An omitted `effort` defaults to `medium` here — cheaper than claude's own
|
||||||
|
model default (`high` on most models), a deliberate cost-conscious choice
|
||||||
|
for subagent work specifically, same spirit as "cheaper-than-you" model
|
||||||
|
choice above. Raise it explicitly when the task is complex enough to
|
||||||
|
actually need deeper reasoning, not as a reflex.
|
||||||
|
|
||||||
|
Anthropic's own levels (per current Claude Code docs — names/availability
|
||||||
|
are model-dependent, check before relying on an exact list): `low`,
|
||||||
|
`medium`, `high`, `xhigh`, `max`. Each trades token spend for capability:
|
||||||
|
|
||||||
|
- **`low`** — short, scoped, latency-sensitive tasks that aren't
|
||||||
|
intelligence-sensitive.
|
||||||
|
- **`medium`** — cost-sensitive work that can trade off some intelligence.
|
||||||
|
This skill's default for subagent work.
|
||||||
|
- **`high`** — balances token usage and intelligence; Anthropic's own
|
||||||
|
recommended default for most _interactive_ coding tasks (not what this
|
||||||
|
skill defaults subagents to — see above).
|
||||||
|
- **`xhigh`** — deeper reasoning at higher token spend.
|
||||||
|
- **`max`** — demanding tasks only; diminishing returns and overthinking
|
||||||
|
are a real risk, per Anthropic's own guidance — don't reach for it as a
|
||||||
|
default.
|
||||||
|
|
||||||
|
Anthropic's guidance: treat effort as a general preference, not a
|
||||||
|
task-by-task dial — raise it if a subagent keeps skipping files, not
|
||||||
|
running tests, or not double-checking its own work; lower it for routine
|
||||||
|
work where quality hasn't suffered. Changing effort between turns on the
|
||||||
|
_same_ session invalidates prompt caching, so pick a level for the whole
|
||||||
|
session rather than flipping it turn-to-turn.
|
||||||
|
|
||||||
## Prompt hygiene - this is where batches succeed or fail
|
## Prompt hygiene - this is where batches succeed or fail
|
||||||
|
|
||||||
- **Concrete constants, not "figure it out":** exact ids, field names,
|
- **Concrete constants, not "figure it out":** exact ids, field names,
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,15 @@ struct StartArgs {
|
||||||
/// guidance still applies here.
|
/// guidance still applies here.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
/// Which reasoning effort level the subagent's own session runs at
|
||||||
|
/// (`--effort`). Omit to default to `medium` — a deliberate hive
|
||||||
|
/// policy for subagent work, not claude's own default (`high` on most
|
||||||
|
/// models). Independent of `model`, so a cheap model at high effort or
|
||||||
|
/// an expensive one at low effort are both valid combinations, not
|
||||||
|
/// just the two extremes. See the `base:claude-subagents` skill for
|
||||||
|
/// Anthropic's own guidance on choosing between levels.
|
||||||
|
#[serde(default)]
|
||||||
|
effort: Option<String>,
|
||||||
/// Path to a file holding the subagent's actual task instructions. A
|
/// Path to a file holding the subagent's actual task instructions. A
|
||||||
/// file, not an inline string, so a large recipe can't blow past a
|
/// file, not an inline string, so a large recipe can't blow past a
|
||||||
/// shell argument length limit.
|
/// shell argument length limit.
|
||||||
|
|
@ -64,6 +73,11 @@ struct ContinueArgs {
|
||||||
/// default — this does not have to match whatever model `start` used.
|
/// default — this does not have to match whatever model `start` used.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
/// Which reasoning effort level this turn runs at. Omit to default to
|
||||||
|
/// `medium` (this daemon's own default, not claude's) — this does not
|
||||||
|
/// have to match whatever effort `start` (or a prior `continue`) used.
|
||||||
|
#[serde(default)]
|
||||||
|
effort: Option<String>,
|
||||||
/// Omit to reuse whatever `dir` `start` (or a prior `continue`) used for
|
/// Omit to reuse whatever `dir` `start` (or a prior `continue`) used for
|
||||||
/// this name — the daemon remembers it. Only pass this to point the
|
/// this name — the daemon remembers it. Only pass this to point the
|
||||||
/// session at a *different* directory than last time.
|
/// session at a *different* directory than last time.
|
||||||
|
|
@ -116,6 +130,7 @@ impl SubagentMcp {
|
||||||
&self.state,
|
&self.state,
|
||||||
&args.name,
|
&args.name,
|
||||||
args.model,
|
args.model,
|
||||||
|
args.effort,
|
||||||
&args.prompt_file,
|
&args.prompt_file,
|
||||||
args.trigger,
|
args.trigger,
|
||||||
args.dir.as_deref(),
|
args.dir.as_deref(),
|
||||||
|
|
@ -141,6 +156,7 @@ impl SubagentMcp {
|
||||||
&args.name,
|
&args.name,
|
||||||
args.prompt,
|
args.prompt,
|
||||||
args.model,
|
args.model,
|
||||||
|
args.effort,
|
||||||
args.dir.as_deref(),
|
args.dir.as_deref(),
|
||||||
) {
|
) {
|
||||||
Ok(msg) => msg,
|
Ok(msg) => msg,
|
||||||
|
|
|
||||||
|
|
@ -289,7 +289,14 @@ fn subagent_otel_attrs(name: &str) -> String {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Build the `Config` one subagent turn runs against. `prompt_file`, when
|
/// Build the `Config` one subagent turn runs against. `model` maps straight
|
||||||
|
/// onto `Config::model` — `--model`, omitted when `None` so claude falls
|
||||||
|
/// back to its own default. `effort` does not: an omitted `effort` defaults
|
||||||
|
/// to `"medium"` here rather than falling through to claude's own default
|
||||||
|
/// (`high` on most models) — a deliberate hive policy for subagent work
|
||||||
|
/// specifically, not a reflection of Anthropic's own recommendation, on the
|
||||||
|
/// same "cheaper than you" cost-consciousness the `base:claude-subagents`
|
||||||
|
/// skill already asks of `model`. `prompt_file`, when
|
||||||
/// given, becomes `--append-system-prompt-file` — the subagent's task
|
/// given, becomes `--append-system-prompt-file` — the subagent's task
|
||||||
/// instructions. `dir`, when given, becomes `Config::cwd` (e.g. a worktree
|
/// instructions. `dir`, when given, becomes `Config::cwd` (e.g. a worktree
|
||||||
/// the caller already prepared); `None` inherits this daemon's own working
|
/// the caller already prepared); `None` inherits this daemon's own working
|
||||||
|
|
@ -305,6 +312,7 @@ fn subagent_otel_attrs(name: &str) -> String {
|
||||||
fn build_config(
|
fn build_config(
|
||||||
name: &str,
|
name: &str,
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
effort: Option<String>,
|
||||||
prompt_file: Option<&str>,
|
prompt_file: Option<&str>,
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
) -> Config {
|
) -> Config {
|
||||||
|
|
@ -315,6 +323,7 @@ fn build_config(
|
||||||
}
|
}
|
||||||
Config {
|
Config {
|
||||||
model,
|
model,
|
||||||
|
effort: Some(effort.unwrap_or_else(|| "medium".to_owned())),
|
||||||
cwd: dir.map(PathBuf::from),
|
cwd: dir.map(PathBuf::from),
|
||||||
mcp_config: crate::mcp_config::build(),
|
mcp_config: crate::mcp_config::build(),
|
||||||
strict_mcp_config: true,
|
strict_mcp_config: true,
|
||||||
|
|
@ -354,6 +363,7 @@ pub fn start(
|
||||||
state: &Arc<State>,
|
state: &Arc<State>,
|
||||||
name: &str,
|
name: &str,
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
effort: Option<String>,
|
||||||
prompt_file: &str,
|
prompt_file: &str,
|
||||||
trigger: String,
|
trigger: String,
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
|
|
@ -365,7 +375,15 @@ pub fn start(
|
||||||
// Only commit the remembered `dir` now that `reserve` has actually
|
// Only commit the remembered `dir` now that `reserve` has actually
|
||||||
// claimed `name` — see `resolve_dir`'s doc for why the order matters.
|
// claimed `name` — see `resolve_dir`'s doc for why the order matters.
|
||||||
let dir = state.resolve_dir(name, dir);
|
let dir = state.resolve_dir(name, dir);
|
||||||
let result = start_reserved(state, name, model, prompt_file, trigger, dir.as_deref());
|
let result = start_reserved(
|
||||||
|
state,
|
||||||
|
name,
|
||||||
|
model,
|
||||||
|
effort,
|
||||||
|
prompt_file,
|
||||||
|
trigger,
|
||||||
|
dir.as_deref(),
|
||||||
|
);
|
||||||
if result.is_err() {
|
if result.is_err() {
|
||||||
state.release_reservation(name);
|
state.release_reservation(name);
|
||||||
}
|
}
|
||||||
|
|
@ -380,11 +398,12 @@ fn start_reserved(
|
||||||
state: &Arc<State>,
|
state: &Arc<State>,
|
||||||
name: &str,
|
name: &str,
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
effort: Option<String>,
|
||||||
prompt_file: &str,
|
prompt_file: &str,
|
||||||
trigger: String,
|
trigger: String,
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
) -> anyhow::Result<String> {
|
) -> anyhow::Result<String> {
|
||||||
let config = build_config(name, model, Some(prompt_file), dir);
|
let config = build_config(name, model, effort, Some(prompt_file), dir);
|
||||||
let store = build_store(&config)?;
|
let store = build_store(&config)?;
|
||||||
if store.find_by_title(name).is_some() {
|
if store.find_by_title(name).is_some() {
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
|
|
@ -424,6 +443,7 @@ pub fn continue_(
|
||||||
name: &str,
|
name: &str,
|
||||||
prompt: String,
|
prompt: String,
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
effort: Option<String>,
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
) -> anyhow::Result<String> {
|
) -> anyhow::Result<String> {
|
||||||
validate_name(name)?;
|
validate_name(name)?;
|
||||||
|
|
@ -438,7 +458,7 @@ pub fn continue_(
|
||||||
// Only commit the remembered `dir` now that `reserve` has actually
|
// Only commit the remembered `dir` now that `reserve` has actually
|
||||||
// claimed `name` — see `resolve_dir`'s doc for why the order matters.
|
// claimed `name` — see `resolve_dir`'s doc for why the order matters.
|
||||||
let dir = state.resolve_dir(name, dir);
|
let dir = state.resolve_dir(name, dir);
|
||||||
let result = continue_reserved(state, name, prompt, model, dir.as_deref());
|
let result = continue_reserved(state, name, prompt, model, effort, dir.as_deref());
|
||||||
if result.is_err() {
|
if result.is_err() {
|
||||||
state.release_reservation(name);
|
state.release_reservation(name);
|
||||||
}
|
}
|
||||||
|
|
@ -467,9 +487,10 @@ fn continue_reserved(
|
||||||
name: &str,
|
name: &str,
|
||||||
prompt: String,
|
prompt: String,
|
||||||
model: Option<String>,
|
model: Option<String>,
|
||||||
|
effort: Option<String>,
|
||||||
dir: Option<&str>,
|
dir: Option<&str>,
|
||||||
) -> anyhow::Result<String> {
|
) -> anyhow::Result<String> {
|
||||||
let config = build_config(name, model, None, dir);
|
let config = build_config(name, model, effort, None, dir);
|
||||||
let store = build_store(&config)?;
|
let store = build_store(&config)?;
|
||||||
if store.find_by_title(name).is_none() {
|
if store.find_by_title(name).is_none() {
|
||||||
anyhow::bail!(
|
anyhow::bail!(
|
||||||
|
|
@ -561,7 +582,7 @@ pub fn status(state: &State, name: &str, dir: Option<&str>) -> anyhow::Result<St
|
||||||
// those states answer on their own, and the store read is the only
|
// those states answer on their own, and the store read is the only
|
||||||
// expensive part of this call.
|
// expensive part of this call.
|
||||||
let session_exists = if occupancy.is_none() && killed.is_none() {
|
let session_exists = if occupancy.is_none() && killed.is_none() {
|
||||||
let config = build_config(name, None, None, dir.as_deref());
|
let config = build_config(name, None, None, None, dir.as_deref());
|
||||||
build_store(&config)?.find_by_title(name).is_some()
|
build_store(&config)?.find_by_title(name).is_some()
|
||||||
} else {
|
} else {
|
||||||
false
|
false
|
||||||
|
|
@ -775,14 +796,14 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn build_config_only_appends_system_prompt_when_given() {
|
fn build_config_only_appends_system_prompt_when_given() {
|
||||||
let with = build_config("n", None, Some("/tmp/p.md"), None);
|
let with = build_config("n", None, None, Some("/tmp/p.md"), None);
|
||||||
assert!(
|
assert!(
|
||||||
with.extra_args
|
with.extra_args
|
||||||
.contains(&"--append-system-prompt-file".to_owned())
|
.contains(&"--append-system-prompt-file".to_owned())
|
||||||
);
|
);
|
||||||
assert!(with.extra_args.contains(&"/tmp/p.md".to_owned()));
|
assert!(with.extra_args.contains(&"/tmp/p.md".to_owned()));
|
||||||
|
|
||||||
let without = build_config("n", None, None, None);
|
let without = build_config("n", None, None, None, None);
|
||||||
assert!(
|
assert!(
|
||||||
!without
|
!without
|
||||||
.extra_args
|
.extra_args
|
||||||
|
|
@ -865,6 +886,7 @@ mod tests {
|
||||||
&state,
|
&state,
|
||||||
"dup",
|
"dup",
|
||||||
None,
|
None,
|
||||||
|
None,
|
||||||
"/tmp/prompt.md",
|
"/tmp/prompt.md",
|
||||||
"trigger".to_owned(),
|
"trigger".to_owned(),
|
||||||
Some("/tmp/rejected"),
|
Some("/tmp/rejected"),
|
||||||
|
|
@ -1054,10 +1076,33 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn build_config_sets_cwd_only_when_a_dir_is_given() {
|
fn build_config_sets_cwd_only_when_a_dir_is_given() {
|
||||||
let with = build_config("n", None, None, Some("/tmp/some-worktree"));
|
let with = build_config("n", None, None, None, Some("/tmp/some-worktree"));
|
||||||
assert_eq!(with.cwd, Some(PathBuf::from("/tmp/some-worktree")));
|
assert_eq!(with.cwd, Some(PathBuf::from("/tmp/some-worktree")));
|
||||||
|
|
||||||
let without = build_config("n", None, None, None);
|
let without = build_config("n", None, None, None, None);
|
||||||
assert_eq!(without.cwd, None);
|
assert_eq!(without.cwd, None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn build_config_sets_effort_alongside_model() {
|
||||||
|
let config = build_config(
|
||||||
|
"n",
|
||||||
|
Some("opus".to_owned()),
|
||||||
|
Some("high".to_owned()),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
);
|
||||||
|
assert_eq!(config.model, Some("opus".to_owned()));
|
||||||
|
assert_eq!(config.effort, Some("high".to_owned()));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn build_config_defaults_effort_to_medium_when_omitted() {
|
||||||
|
// Unlike `model`, an omitted `effort` does not fall through to
|
||||||
|
// claude's own default (`high` on most models) — this daemon picks
|
||||||
|
// `medium` itself, a deliberate cost-conscious choice for subagent
|
||||||
|
// work.
|
||||||
|
let config = build_config("n", None, None, None, None);
|
||||||
|
assert_eq!(config.effort, Some("medium".to_owned()));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue