From c89db9dac6a58da1cd3f063cf45cf5f1ad1cf240 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Thu, 20 Aug 2026 16:38:57 -0700 Subject: [PATCH 1/8] chore: fix test --- tests/datasets-fixtures/snapshots-create/fixture.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/datasets-fixtures/snapshots-create/fixture.json b/tests/datasets-fixtures/snapshots-create/fixture.json index a519fea0..6c67196e 100644 --- a/tests/datasets-fixtures/snapshots-create/fixture.json +++ b/tests/datasets-fixtures/snapshots-create/fixture.json @@ -126,7 +126,7 @@ "baseline" ], "expect_success": false, - "stdout_contains": [ + "stderr_contains": [ "snapshot delete requires --force in non-interactive mode" ] }, @@ -169,7 +169,7 @@ "snapshot-source" ], "expect_success": false, - "stdout_contains": [ + "stderr_contains": [ "dataset delete requires --force in non-interactive mode" ] }, From e1269a92e6973ef3a3a1eecccd6eb074a3d1021f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Thu, 13 Aug 2026 18:36:16 -0700 Subject: [PATCH 2/8] feat(functions): update functions tools and scorers --- README.md | 15 +- src/functions/api.rs | 14 + src/functions/create.rs | 2 +- src/functions/mod.rs | 47 +- src/functions/prompt_patch.rs | 53 +++ src/functions/update.rs | 806 ++++++++++++++++++++++++++++++++++ src/scorers.rs | 60 ++- tests/cli.rs | 15 + 8 files changed, 989 insertions(+), 23 deletions(-) create mode 100644 src/functions/prompt_patch.rs create mode 100644 src/functions/update.rs diff --git a/README.md b/README.md index bae04b0e..78755829 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,9 @@ Remove-Item -Recurse -Force (Join-Path $env:APPDATA "bt") -ErrorAction SilentlyC | `bt projects` | Manage projects (list, create, view, delete) | | `bt datasets` | Manage remote datasets (list, create, update, view, delete) | | `bt prompts` | Manage prompts (list, view, delete) | -| `bt scorers` | Manage scorers (list, create, view, invoke, delete) | +| `bt functions` | Manage functions (list, view, invoke, update, push, pull, delete) | +| `bt tools` | Manage tools (list, view, invoke, update, delete) | +| `bt scorers` | Manage scorers (list, create, view, invoke, update, delete) | | `bt sync` | Synchronize project logs between Braintrust and local NDJSON files | | `bt update` | Update bt in-place | @@ -175,6 +177,17 @@ Use `--if-exists error|ignore|replace` to control slug conflicts. Text and struc Before writing a scorer, `bt` sends the complete candidate definition to Braintrust for validation. The backend applies the same model-parameter and replacement checks as the write and returns structured issues with normalization suggestions when available. +Update only the fields you specify, or use `--patch` for fields without dedicated flags: + +```bash +bt scorers update helpfulness --messages @messages.json +bt scorers update helpfulness --model gpt-5.4-nano +bt functions update my-function --description "Updated" +bt tools update my-tool --patch @tool-patch.json +``` + +The API replaces `prompt_data` rather than merging it, so `bt` reads the current definition and sends a materialized replacement with your changes. A concurrent edit can therefore be overwritten. + For TypeScript and Python code scorers, use the Braintrust SDK and `bt functions push`. ## `bt eval` diff --git a/src/functions/api.rs b/src/functions/api.rs index 0e526cfd..c300d860 100644 --- a/src/functions/api.rs +++ b/src/functions/api.rs @@ -252,6 +252,20 @@ pub async fn delete_function(client: &ApiClient, function_id: &str) -> Result<() client.delete(&path).await } +/// Partially update a function (scorer/tool/prompt/...) by id. +/// +/// Top-level fields are patched, but object-valued fields such as `prompt_data` +/// are replaced wholesale. Callers updating `prompt_data` must materialize the +/// complete value before sending the request. +pub async fn patch_function( + client: &ApiClient, + function_id: &str, + body: &serde_json::Value, +) -> Result { + let path = format!("/v1/function/{}", encode(function_id)); + client.patch(&path, body).await +} + pub async fn list_functions_page( client: &ApiClient, query: &FunctionListQuery, diff --git a/src/functions/create.rs b/src/functions/create.rs index e9f3576e..77dff0eb 100644 --- a/src/functions/create.rs +++ b/src/functions/create.rs @@ -173,7 +173,7 @@ pub(crate) async fn run(ctx: &ResolvedContext, args: &CreateArgs, json_output: b Ok(()) } -fn report_validation_issues(report: &api::FunctionValidationReport) -> Result<()> { +pub(super) fn report_validation_issues(report: &api::FunctionValidationReport) -> Result<()> { let mut blocking = Vec::new(); for result in &report.results { for issue in &result.issues { diff --git a/src/functions/mod.rs b/src/functions/mod.rs index 03c5301a..aa445272 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -18,9 +18,11 @@ mod delete; mod invoke; mod list; pub(crate) mod prompt_config; +pub(crate) mod prompt_patch; mod pull; mod push; pub(crate) mod report; +mod update; mod view; use api::Function; @@ -173,8 +175,7 @@ Examples: bt tools view my-tool bt tools view fn_123 bt tools view --id fn_123 - bt scorers list - bt scorers delete my-scorer + bt tools update my-tool --patch @tool-patch.json ")] pub struct FunctionArgs { #[command(subcommand)] @@ -191,6 +192,8 @@ pub(crate) enum FunctionCommands { Delete(DeleteArgs), /// Invoke by slug Invoke(invoke::InvokeArgs), + /// Update a function in place (prompt configuration, metadata, or arbitrary patch) + Update(Box), } #[derive(Debug, Clone, Args)] @@ -223,6 +226,8 @@ enum FunctionsCommands { Delete(FunctionsDeleteArgs), /// Invoke a function Invoke(FunctionsInvokeArgs), + /// Update a function in place (prompt configuration, metadata, or arbitrary patch) + Update(Box), /// Push local function definitions Push(PushArgs), /// Pull remote function definitions @@ -272,6 +277,15 @@ struct FunctionsInvokeArgs { function_type: Option, } +#[derive(Debug, Clone, Args)] +struct FunctionsUpdateArgs { + #[command(flatten)] + inner: update::UpdateArgs, + /// Filter by function type (for interactive selection) + #[arg(long = "type", short = 't', value_enum)] + function_type: Option, +} + #[derive(Debug, Clone, Args)] pub(crate) struct PushArgs { /// File or directory path(s) to scan for function definitions. @@ -657,6 +671,7 @@ pub(crate) async fn run_typed_command( None | Some(FunctionCommands::List) => list::run(&ctx, base.json, ft).await, Some(FunctionCommands::Delete(d)) => delete::run(&ctx, d.slug(), d.force, ft).await, Some(FunctionCommands::Invoke(i)) => invoke::run(&ctx, &i, base.json, ft).await, + Some(FunctionCommands::Update(u)) => update::run(&ctx, &u, base.json, ft).await, Some(FunctionCommands::View(_)) => { unreachable!("handled before context resolution") } @@ -720,6 +735,9 @@ pub async fn run(base: BaseArgs, args: FunctionsArgs) -> Result<()> { Some(FunctionsCommands::Invoke(i)) => { invoke::run(&ctx, &i.inner, base.json, i.function_type.or(function_type)).await } + Some(FunctionsCommands::Update(u)) => { + update::run(&ctx, &u.inner, base.json, u.function_type.or(function_type)).await + } Some(FunctionsCommands::Push(_)) | Some(FunctionsCommands::Pull(_)) | Some(FunctionsCommands::View(_)) => { @@ -1201,6 +1219,31 @@ mod tests { ) } + #[test] + fn typed_function_update_command_is_not_read_only() { + let _guard = test_lock(); + let parsed = FunctionArgsHarness::try_parse_from(["bt-tools", "update", "my-tool", "-y"]) + .expect("parse"); + assert!(!function_command_is_read_only(parsed.args.command.as_ref())); + } + + #[test] + fn functions_update_command_is_not_read_only() { + let _guard = test_lock(); + let parsed = FunctionsArgsHarness::try_parse_from([ + "bt-functions", + "update", + "my-fn", + "--description", + "x", + "-y", + ]) + .expect("parse"); + assert!(!functions_command_is_read_only( + parsed.args.command.as_ref() + )); + } + #[test] fn typed_function_commands_map_to_expected_auth_mode() { let _guard = test_lock(); diff --git a/src/functions/prompt_patch.rs b/src/functions/prompt_patch.rs new file mode 100644 index 00000000..3a3ba7af --- /dev/null +++ b/src/functions/prompt_patch.rs @@ -0,0 +1,53 @@ +use serde_json::Value; + +use crate::utils::merge_json_objects; + +/// Replace a partial `prompt_data` patch with the full merged object. +/// +/// The function and prompt PATCH endpoints merge top-level fields but replace +/// `prompt_data` wholesale. Materializing it before the request preserves fields +/// that the user did not change. +pub(crate) fn materialize_prompt_data_patch( + patch: &mut Value, + existing_prompt_data: Option<&Value>, +) { + let Some(patch_prompt_data) = patch.get("prompt_data").and_then(Value::as_object).cloned() + else { + return; + }; + let mut merged = existing_prompt_data + .and_then(Value::as_object) + .cloned() + .unwrap_or_default(); + merge_json_objects(&mut merged, &patch_prompt_data); + patch["prompt_data"] = Value::Object(merged); +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn materializes_complete_prompt_data_for_patch() { + let existing = json!({ + "prompt": {"type": "chat", "messages": []}, + "parser": {"type": "llm_classifier"}, + "options": {"model": "test-model", "params": {"temperature": 0.5}} + }); + let mut patch = json!({ + "prompt_data": {"options": {"params": {"temperature": 0.2}}} + }); + + materialize_prompt_data_patch(&mut patch, Some(&existing)); + + assert_eq!(patch["prompt_data"]["prompt"], existing["prompt"]); + assert_eq!(patch["prompt_data"]["parser"], existing["parser"]); + assert_eq!(patch["prompt_data"]["options"]["model"], "test-model"); + assert_eq!( + patch["prompt_data"]["options"]["params"]["temperature"], + 0.2 + ); + } +} diff --git a/src/functions/update.rs b/src/functions/update.rs new file mode 100644 index 00000000..08f4db4b --- /dev/null +++ b/src/functions/update.rs @@ -0,0 +1,806 @@ +use anyhow::{anyhow, bail, Context, Result}; +use clap::{builder::BoolishValueParser, Args}; +use dialoguer::Confirm; +use serde_json::{json, Map, Value}; + +use crate::{ + error::user_error, + ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, + utils::{merge_json_objects, read_text_source, read_yaml_object_source}, +}; + +use super::{ + api, create::report_validation_issues, label, label_plural, select_function_interactive, +}; +use super::{ + prompt_config::{ + parse_choice_scores_source, parse_classifications_source, validate_unit_interval, + PromptConfigArgs, + }, + prompt_patch::materialize_prompt_data_patch, + FunctionTypeFilter, ResolvedContext, +}; + +/// Update a function's prompt configuration or metadata in place. +/// +/// This wraps `PATCH /v1/function/{id}`. The endpoint replaces `prompt_data` +/// wholesale, so the command reads the current definition and materializes a +/// complete replacement while changing only the fields requested by the user. +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt scorers update my-scorer --messages @messages.json + bt scorers update my-scorer --model gpt-5.4-nano --reasoning-effort none --temperature 0.1 + bt scorers update my-scorer --template-format jinja --pass-threshold 0.7 + bt scorers update my-scorer --classifications '[\"safe\",\"unsafe\"]' + bt scorers update my-scorer --metadata @metadata.yaml + bt scorers update my-scorer --description \"Helpfulness judge\" + bt scorers update my-scorer --patch '{\"prompt_data\":{\"options\":{\"model\":\"gpt-5.4-nano\"}}}' + bt scorers update --id fn_123 --patch @scorer-patch.json + bt tools update my-tool --patch @tool-patch.json +")] +pub struct UpdateArgs { + #[command(flatten)] + slug: super::SlugArgs, + + /// Function id (alternative to slug). Auto-detected for `fn_`/`func_` prefixes. + #[arg(long = "id")] + id: Option, + + /// Replacement chat messages source: inline JSON, @PATH to read from a + /// file, or - for stdin. + #[arg(long, value_name = "SOURCE")] + messages: Option, + + /// Update the model used by an LLM scorer/prompt. + #[arg(long, short = 'm', value_name = "MODEL")] + model: Option, + + #[command(flatten)] + prompt_config: PromptConfigArgs, + + /// Replace choice-to-score mappings for score output. Accepts inline JSON, + /// @PATH to read from a file, or - for stdin. + #[arg(long, value_name = "SOURCE", conflicts_with = "classifications")] + choice_scores: Option, + + /// Replace labels for classification output. Accepts an inline JSON array, + /// @PATH to read from a file, or - for stdin. + #[arg(long, value_name = "SOURCE", conflicts_with = "choice_scores")] + classifications: Option, + + /// Update chain-of-thought reasoning. Pass --use-cot=false to disable it. + #[arg( + long, + num_args = 0..=1, + default_missing_value = "true", + value_parser = BoolishValueParser::new() + )] + use_cot: Option, + + /// Update whether a classifier may return no matching classification. + #[arg( + long, + num_args = 0..=1, + default_missing_value = "true", + value_parser = BoolishValueParser::new() + )] + allow_no_match: Option, + + /// Update the score threshold for passing, between 0 and 1. + #[arg(long, value_name = "NUMBER", conflicts_with = "classifications")] + pass_threshold: Option, + + /// Deep-merge metadata from inline YAML, @PATH, or stdin (-). + #[arg(long, value_name = "SOURCE")] + metadata: Option, + + /// Update the function description. + #[arg(long, short = 'd', value_name = "TEXT")] + description: Option, + + /// Arbitrary JSON object deep-merged into the function. Accepts inline + /// JSON, @PATH to read JSON from a file, or - for stdin. + #[arg(long, value_name = "SOURCE")] + patch: Option, + + /// Skip the confirmation prompt. + #[arg(long, short = 'y')] + yes: bool, +} + +impl UpdateArgs { + /// Flags that only make sense for LLM scorers and classifiers. + /// + /// Returns the flag names that were set so callers can reject them on other + /// function kinds (for example tools) with an actionable message. + fn scorer_output_flags(&self) -> Vec<&'static str> { + let mut flags = Vec::new(); + if self.choice_scores.is_some() { + flags.push("--choice-scores"); + } + if self.classifications.is_some() { + flags.push("--classifications"); + } + if self.allow_no_match.is_some() { + flags.push("--allow-no-match"); + } + if self.use_cot.is_some() { + flags.push("--use-cot"); + } + if self.pass_threshold.is_some() { + flags.push("--pass-threshold"); + } + flags + } + + fn selector(&self) -> Result> { + match ( + self.id.as_deref(), + self.slug.slug_positional(), + self.slug.slug_flag(), + ) { + (Some(_), Some(_), _) | (Some(_), _, Some(_)) => { + bail!("use either --id or a slug, not both") + } + (Some(id), None, None) => Ok(UpdateSelector::Id(id)), + (None, Some(positional), None) if super::is_likely_function_id(positional) => { + Ok(UpdateSelector::Id(positional)) + } + (None, positional, flag) => Ok(UpdateSelector::Slug(positional.or(flag))), + } + } +} + +#[derive(Debug)] +enum UpdateSelector<'a> { + Id(&'a str), + Slug(Option<&'a str>), +} + +fn validation_candidate(function: &api::Function, patch: &Value) -> Value { + let mut candidate = json!({ + "project_id": function.project_id, + "name": function.name, + "slug": function.slug, + }); + let object = candidate + .as_object_mut() + .expect("validation candidate is an object"); + + for (key, value) in [ + ( + "description", + function.description.as_ref().map(|v| json!(v)), + ), + ( + "function_type", + function.function_type.as_ref().map(|v| json!(v)), + ), + ("prompt_data", function.prompt_data.clone()), + ("function_data", function.function_data.clone()), + ("tags", function.tags.as_ref().map(|v| json!(v))), + ("metadata", function.metadata.clone()), + ] { + if let Some(value) = value { + object.insert(key.to_string(), value); + } + } + + // PATCH replaces each top-level value. `prompt_data` has already been + // materialized into a complete value before this helper is called. + for (key, value) in patch + .as_object() + .expect("function update patch is an object") + { + object.insert(key.clone(), value.clone()); + } + candidate +} + +pub async fn run( + ctx: &ResolvedContext, + args: &UpdateArgs, + json_output: bool, + ft: Option, +) -> Result<()> { + let mut body = build_patch_body(args)?; + + let function = resolve_target_function(ctx, args, ft).await?; + + // LLM scorer/classifier output flags only apply to prompt-based scorers and + // classifiers. Reject them on other function kinds (for example tools) so an + // unrelated function is not silently patched with a parser it cannot use. + let is_scorer_like = matches!( + function.function_type.as_deref(), + Some("scorer") | Some("classifier") + ); + let scorer_flags = args.scorer_output_flags(); + if !scorer_flags.is_empty() && !is_scorer_like { + bail!( + "{} apply to LLM scorers and classifiers, not {} '{}'. \ + Run `bt scorers update` on a scorer instead.", + scorer_flags.join(", "), + label(ft), + function.name, + ); + } + + // Mirrors `create`, where --allow-no-match requires --classifications: a + // score parser would never consult it. + let produces_classifications = + args.classifications.is_some() || function.function_type.as_deref() == Some("classifier"); + if args.allow_no_match.is_some() && !produces_classifications { + bail!( + "--allow-no-match applies to classification output, but '{}' produces scores. \ + Pass --classifications to switch it to labels.", + function.name, + ); + } + + materialize_prompt_data_patch(&mut body, function.prompt_data.as_ref()); + + // Last of the up-front checks because it hits the network. Validate the + // complete candidate rather than the partial PATCH body so the backend can + // apply the same model-parameter checks as it does for scorer creation. + let candidate = validation_candidate(&function, &body); + let validation = with_spinner( + "Validating function...", + api::validate_functions(&ctx.client, std::slice::from_ref(&candidate)), + ) + .await?; + report_validation_issues(&validation).map_err(user_error)?; + + // Switching output mode updates function_type, but materialization merges + // the parser and does not drop the previous mode's keys. Warn so the + // user can review or recreate for a clean switch. + if !crate::ui::is_quiet() { + match function.function_type.as_deref() { + Some("classifier") if args.choice_scores.is_some() => print_command_status( + CommandStatus::Warning, + "Switching to score output; previous classification labels may remain in the definition. Review with `bt scorers view`.", + ), + Some("scorer") if args.classifications.is_some() => print_command_status( + CommandStatus::Warning, + "Switching to classification output; previous choice scores may remain in the definition. Review with `bt scorers view`.", + ), + _ => {} + } + } + + if !args.yes && is_interactive() { + let confirm = Confirm::new() + .with_prompt(format!( + "Update {} '{}' in {}?", + label(ft), + function.name, + ctx.project.name + )) + .default(false) + .interact()?; + if !confirm { + return Ok(()); + } + } + + let updated = match with_spinner( + &format!("Updating {}...", label(ft)), + api::patch_function(&ctx.client, &function.id, &body), + ) + .await + { + Ok(value) => { + print_command_status( + CommandStatus::Success, + &format!("Updated '{}'", function.name), + ); + value + } + Err(error) => { + print_command_status( + CommandStatus::Error, + &format!("Failed to update '{}'", function.name), + ); + return Err(error); + } + }; + + if json_output { + println!("{}", serde_json::to_string(&updated)?); + } else if !crate::ui::is_quiet() { + eprintln!( + "Run `bt {} view {}` to inspect the updated definition.", + label_plural(ft), + function.slug + ); + } + + Ok(()) +} + +async fn resolve_target_function( + ctx: &ResolvedContext, + args: &UpdateArgs, + ft: Option, +) -> Result { + let project_id = &ctx.project.id; + match args.selector()? { + UpdateSelector::Id(id) => api::get_function_by_id(&ctx.client, id, None) + .await? + .ok_or_else(|| anyhow!("{} with id '{id}' not found", label(ft))), + UpdateSelector::Slug(Some(slug)) => { + api::get_function_by_slug(&ctx.client, project_id, slug, None) + .await? + .ok_or_else(|| anyhow!("{} with slug '{slug}' not found", label(ft))) + } + UpdateSelector::Slug(None) => { + if !is_interactive() { + bail!( + "{} slug or --id required. Use: bt {} update [--patch ...]", + label(ft), + label_plural(ft), + ); + } + Ok(select_function_interactive(&ctx.client, project_id, ft).await?) + } + } +} + +fn build_patch_body(args: &UpdateArgs) -> Result { + let mut patch: Map = Map::new(); + + if let Some(description) = args.description.as_deref() { + patch.insert( + "description".to_string(), + Value::String(description.to_string()), + ); + } + + let metadata = resolve_metadata(args)?; + if !metadata.is_empty() { + patch.insert("metadata".to_string(), Value::Object(metadata)); + } + + if let Some(messages) = resolve_messages(args)? { + let prompt_data_patch = json!({ + "prompt_data": { + "prompt": { "type": "chat", "messages": messages }, + }, + }); + merge_json_objects( + &mut patch, + prompt_data_patch + .as_object() + .expect("prompt data patch is an object"), + ); + } + + let parser_patch = resolve_parser_patch(args)?; + if let Some((function_type, parser)) = parser_patch { + if let Some(function_type) = function_type { + patch.insert( + "function_type".to_string(), + Value::String(function_type.to_string()), + ); + } + let prompt_data_patch = json!({ "prompt_data": { "parser": parser } }); + merge_json_objects( + &mut patch, + prompt_data_patch + .as_object() + .expect("prompt data patch is an object"), + ); + } + + let prompt_config = args + .prompt_config + .build_prompt_data_patch(args.model.as_deref())?; + if !prompt_config.is_empty() { + let prompt_data_patch = json!({ "prompt_data": prompt_config }); + merge_json_objects( + &mut patch, + prompt_data_patch + .as_object() + .expect("prompt data patch is an object"), + ); + } + + let extra = resolve_extra_patch(args)?; + if let Some(extra_obj) = extra { + merge_json_objects(&mut patch, &extra_obj); + } + + if patch.is_empty() { + bail!("no updates requested. Pass an update flag; see `bt scorers update --help`"); + } + + Ok(Value::Object(patch)) +} + +fn resolve_metadata(args: &UpdateArgs) -> Result> { + if args.classifications.is_some() && args.pass_threshold.is_some() { + bail!("--pass-threshold applies to score output and cannot be used with --classifications"); + } + + let mut metadata = match args.metadata.as_deref() { + Some(source) => read_yaml_object_source(source, "function metadata")?, + None => Map::new(), + }; + if let Some(pass_threshold) = args.pass_threshold { + validate_unit_interval(pass_threshold, "--pass-threshold")?; + metadata.insert("__pass_threshold".to_string(), json!(pass_threshold)); + } + Ok(metadata) +} + +fn resolve_parser_patch(args: &UpdateArgs) -> Result, Value)>> { + if args.choice_scores.is_some() && args.allow_no_match.is_some() { + bail!("--allow-no-match applies to classification output, not --choice-scores"); + } + + let mut parser = Map::new(); + let mut function_type = None; + + if let Some(source) = args.choice_scores.as_deref() { + parser.insert( + "type".to_string(), + Value::String("llm_classifier".to_string()), + ); + parser.insert( + "choice_scores".to_string(), + Value::Object(parse_choice_scores_source(source)?), + ); + function_type = Some("scorer"); + } + if let Some(source) = args.classifications.as_deref() { + parser.insert( + "type".to_string(), + Value::String("llm_classifier".to_string()), + ); + parser.insert( + "choice".to_string(), + Value::Array(parse_classifications_source(source)?), + ); + function_type = Some("classifier"); + } + if let Some(use_cot) = args.use_cot { + parser.insert("use_cot".to_string(), Value::Bool(use_cot)); + } + if let Some(allow_no_match) = args.allow_no_match { + parser.insert("allow_no_match".to_string(), Value::Bool(allow_no_match)); + } + + if parser.is_empty() { + Ok(None) + } else { + Ok(Some((function_type, Value::Object(parser)))) + } +} + +fn resolve_messages(args: &UpdateArgs) -> Result> { + match args.messages.as_deref() { + Some(source) => { + let raw = read_text_source(source, "messages")?; + let parsed: Value = serde_json::from_str(&raw).context("invalid JSON in --messages")?; + match parsed { + Value::Array(_) => Ok(Some(parsed)), + _ => bail!("--messages must be a JSON array of chat messages"), + } + } + None => Ok(None), + } +} + +fn resolve_extra_patch(args: &UpdateArgs) -> Result>> { + let Some(source) = args.patch.as_deref() else { + return Ok(None); + }; + let raw = read_text_source(source, "patch")?; + parse_patch_object(&raw).map(Some) +} + +fn parse_patch_object(raw: &str) -> Result> { + let value: Value = serde_json::from_str(raw).context("invalid JSON in --patch")?; + match value { + Value::Object(map) => Ok(map), + _ => bail!("--patch must be a JSON object"), + } +} + +#[cfg(test)] +mod tests { + use clap::Parser; + + use super::*; + + #[derive(Debug, Parser)] + struct UpdateArgsHarness { + #[command(flatten)] + args: UpdateArgs, + } + + fn args(model: Option<&str>, description: Option<&str>) -> UpdateArgs { + UpdateArgs { + slug: super::super::SlugArgs { + slug_positional: Some("test-slug".to_string()), + slug_flag: None, + }, + id: None, + messages: None, + model: model.map(ToOwned::to_owned), + prompt_config: PromptConfigArgs::default(), + choice_scores: None, + classifications: None, + use_cot: None, + allow_no_match: None, + pass_threshold: None, + metadata: None, + description: description.map(ToOwned::to_owned), + patch: None, + yes: true, + } + } + + #[test] + fn validation_candidate_applies_top_level_patch_to_existing_function() { + let function = api::Function { + id: "fn_test_update".to_string(), + name: "Test scorer".to_string(), + slug: "test-scorer".to_string(), + project_id: "test-project".to_string(), + description: Some("Old description".to_string()), + function_type: Some("scorer".to_string()), + prompt_data: Some(json!({"options": {"model": "test-model"}})), + function_data: Some(json!({"type": "prompt"})), + tags: Some(vec!["test-tag".to_string()]), + metadata: Some(json!({"owner": "test-user"})), + created: None, + _xact_id: None, + }; + let patch = json!({ + "description": "New description", + "prompt_data": {"options": {"model": "test-model-2"}} + }); + + let candidate = validation_candidate(&function, &patch); + + assert_eq!(candidate["project_id"], "test-project"); + assert_eq!(candidate["description"], "New description"); + assert_eq!(candidate["prompt_data"], patch["prompt_data"]); + assert_eq!(candidate["function_data"], function.function_data.unwrap()); + assert!(candidate.get("id").is_none()); + } + + #[test] + fn scorer_output_flags_reported_only_when_set() { + let base = args(None, None); + assert!(base.scorer_output_flags().is_empty()); + + let mut scored = args(None, None); + scored.choice_scores = Some(r#"{"pass":1}"#.to_string()); + scored.pass_threshold = Some(0.5); + assert_eq!( + scored.scorer_output_flags(), + vec!["--choice-scores", "--pass-threshold"] + ); + + let mut labeled = args(None, None); + labeled.classifications = Some(r#"["a"]"#.to_string()); + labeled.allow_no_match = Some(true); + labeled.use_cot = Some(false); + assert_eq!( + labeled.scorer_output_flags(), + vec!["--classifications", "--allow-no-match", "--use-cot"] + ); + } + + #[test] + fn build_patch_body_messages_writes_chat_block() { + let mut args = args(None, None); + args.messages = Some(r#"[{"role":"user","content":"hi"}]"#.to_string()); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["prompt"]["type"], + serde_json::json!("chat") + ); + assert_eq!( + body["prompt_data"]["prompt"]["messages"], + serde_json::json!([{"role":"user","content":"hi"}]) + ); + } + + #[test] + fn build_patch_body_model_merges_into_prompt_data() { + let args = args(Some("gpt-4o-mini"), None); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["options"]["model"], + serde_json::json!("gpt-4o-mini") + ); + } + + #[test] + fn build_patch_body_messages_and_model_combine() { + let mut args = args(Some("gpt-4o-mini"), None); + args.messages = Some(r#"[{"role":"user","content":"Grade it."}]"#.to_string()); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["prompt"]["messages"], + json!([{"role": "user", "content": "Grade it."}]) + ); + assert_eq!( + body["prompt_data"]["options"]["model"], + serde_json::json!("gpt-4o-mini") + ); + } + + #[test] + fn build_patch_body_updates_all_llm_configuration() { + let parsed = UpdateArgsHarness::try_parse_from([ + "test", + "test-scorer", + "--model", + "gpt-test", + "--temperature", + "0.2", + "--max-tokens", + "128", + "--top-p", + "0.9", + "--frequency-penalty", + "0.5", + "--presence-penalty", + "0.25", + "--stop-sequence", + "END", + "--tool-choice", + "test_tool", + "--reasoning-effort", + "low", + "--verbosity", + "high", + "--template-format", + "none", + "--use-cot=false", + ]) + .expect("parse update"); + + let body = build_patch_body(&parsed.args).expect("patch body"); + let params = &body["prompt_data"]["options"]["params"]; + assert_eq!(body["prompt_data"]["options"]["model"], "gpt-test"); + assert_eq!(params["temperature"], 0.2); + assert_eq!(params["max_tokens"], 128); + assert_eq!(params["top_p"], 0.9); + assert_eq!(params["frequency_penalty"], 0.5); + assert_eq!(params["presence_penalty"], 0.25); + assert_eq!(params["stop"], json!(["END"])); + assert_eq!( + params["tool_choice"], + json!({"type": "function", "function": {"name": "test_tool"}}) + ); + assert_eq!(params["reasoning_effort"], "low"); + assert_eq!(params["verbosity"], "high"); + assert_eq!(body["prompt_data"]["template_format"], "none"); + assert_eq!(body["prompt_data"]["parser"]["use_cot"], false); + } + + #[test] + fn build_patch_body_switches_to_classification_output() { + let mut args = args(None, None); + args.classifications = Some(r#"["safe","unsafe"]"#.to_string()); + args.allow_no_match = Some(true); + args.metadata = Some("owner: test-team".to_string()); + + let body = build_patch_body(&args).expect("patch body"); + assert_eq!(body["function_type"], "classifier"); + assert_eq!( + body["prompt_data"]["parser"]["choice"], + json!(["safe", "unsafe"]) + ); + assert_eq!(body["prompt_data"]["parser"]["allow_no_match"], true); + assert_eq!(body["metadata"]["owner"], "test-team"); + } + + #[test] + fn build_patch_body_updates_scores_and_pass_threshold() { + let mut args = args(None, None); + args.choice_scores = Some(r#"{"pass":1,"fail":0}"#.to_string()); + args.pass_threshold = Some(0.8); + + let body = build_patch_body(&args).expect("patch body"); + assert_eq!(body["function_type"], "scorer"); + assert_eq!( + body["prompt_data"]["parser"]["choice_scores"], + json!({"pass": 1, "fail": 0}) + ); + assert_eq!(body["metadata"]["__pass_threshold"], 0.8); + } + + #[test] + fn build_patch_body_description_is_top_level() { + let args = args(None, Some("Helpfulness judge")); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!(body["description"], serde_json::json!("Helpfulness judge")); + } + + #[test] + fn build_patch_body_reads_at_prefixed_messages_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("messages.json"); + std::fs::write(&path, r#"[{"role":"user","content":"Grade from a file."}]"#) + .expect("write messages"); + let source = format!("@{}", path.display()); + + let mut args = args(None, None); + args.messages = Some(source); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["prompt"]["messages"], + json!([{"role": "user", "content": "Grade from a file."}]) + ); + } + + #[test] + fn build_patch_body_reads_at_prefixed_patch_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("patch.json"); + std::fs::write(&path, r#"{"description":"From a file"}"#).expect("write patch"); + let source = format!("@{}", path.display()); + + let mut args = args(None, None); + args.patch = Some(source); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!(body["description"], "From a file"); + } + + #[test] + fn build_patch_body_rejects_empty_update() { + let args = args(None, None); + let err = build_patch_body(&args).expect_err("should reject empty"); + assert!(err.to_string().contains("no updates requested")); + } + + #[test] + fn build_patch_body_extra_patch_merges_into_prompt_data() { + let mut args = args(None, None); + args.patch = Some(r#"{"prompt_data":{"parser":{"type":"llm_classifier","use_cot":true,"choice_scores":{"A":1.0,"B":0.0}}}}"#.to_string()); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["parser"]["choice_scores"], + serde_json::json!({"A": 1.0, "B": 0.0}) + ); + } + + #[test] + fn parse_patch_object_rejects_non_object() { + let err = parse_patch_object("[1,2,3]").expect_err("should reject"); + assert!(err.to_string().contains("JSON object")); + } + + #[test] + fn merge_objects_deep_merges_nested_maps() { + let mut target = serde_json::json!({ + "prompt_data": { "options": { "model": "gpt-4o" } } + }) + .as_object() + .expect("object") + .clone(); + let source = serde_json::json!({ + "prompt_data": { "options": { "temperature": 0 } } + }) + .as_object() + .expect("object") + .clone(); + + merge_json_objects(&mut target, &source); + + assert_eq!( + target["prompt_data"]["options"]["model"], + serde_json::json!("gpt-4o") + ); + assert_eq!( + target["prompt_data"]["options"]["temperature"], + serde_json::json!(0) + ); + } +} diff --git a/src/scorers.rs b/src/scorers.rs index d2485a6e..e9b6a167 100644 --- a/src/scorers.rs +++ b/src/scorers.rs @@ -11,6 +11,7 @@ Examples: bt scorers view my-scorer bt scorers create \"Helpfulness\" --model gpt-5.4-nano --messages @messages.json \\ --choice-scores '{\"A\":1,\"B\":0}' + bt scorers update my-scorer --messages @messages.json bt scorers delete my-scorer TypeScript and Python code scorers: @@ -47,7 +48,6 @@ mod tests { use clap::Parser; use super::*; - use crate::args::CLIArgs; #[derive(Debug, Parser)] struct ScorersArgsHarness { @@ -55,24 +55,6 @@ mod tests { args: ScorersArgs, } - #[test] - fn invoke_accepts_global_json_flag() { - #[derive(Debug, Parser)] - struct Harness { - #[command(flatten)] - command: CLIArgs, - } - - let parsed = Harness::try_parse_from(["bt-scorers", "invoke", "test-scorer", "--json"]) - .expect("parse scorer invoke with global JSON output"); - - assert!(parsed.command.base.json); - assert!(matches!( - parsed.command.args.command, - Some(ScorersCommands::Function(FunctionCommands::Invoke(_))) - )); - } - #[test] fn parses_create_scorer() { let parsed = ScorersArgsHarness::try_parse_from([ @@ -96,4 +78,44 @@ mod tests { Some(ScorersCommands::Create(_)) )); } + + #[test] + fn parses_create_classifier() { + let parsed = ScorersArgsHarness::try_parse_from([ + "bt-scorers", + "create", + "Test classifier", + "--model", + "gpt-test", + "--messages", + r#"[{"role":"user","content":"Classify {{output}}"}]"#, + "--classifications", + r#"["safe","unsafe"]"#, + "--allow-no-match", + ]) + .expect("parse create classifier"); + + assert!(matches!( + parsed.args.command, + Some(ScorersCommands::Create(_)) + )); + } + + #[test] + fn still_parses_shared_scorer_commands() { + let parsed = ScorersArgsHarness::try_parse_from([ + "bt-scorers", + "update", + "test-scorer", + "--model", + "gpt-test", + "--yes", + ]) + .expect("parse update"); + + assert!(matches!( + parsed.args.command, + Some(ScorersCommands::Function(FunctionCommands::Update(_))) + )); + } } diff --git a/tests/cli.rs b/tests/cli.rs index 5430134a..aa833077 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1227,6 +1227,21 @@ fn scorers_create_help_includes_llm_judge_configuration() { .stdout(predicate::str::contains("bt functions push scorer.py")); } +#[test] +fn scorer_update_help_is_conflict_free() { + bt_command() + .args(["scorers", "update", "--help"]) + .assert() + .success() + .stdout(predicate::str::contains("--patch ")) + .stdout(predicate::str::contains("--patch-file").not()) + .stdout(predicate::str::contains("--messages ")) + .stdout(predicate::str::contains("--temperature")) + .stdout(predicate::str::contains("--classifications")) + .stdout(predicate::str::contains("--pass-threshold")) + .stdout(predicate::str::contains("--metadata")); +} + #[test] fn topics_report_help_accepts_global_org_short_conflict_free() { bt_command() From f3b3a6a6ab02971c3835505918cf46bb562248bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Wed, 19 Aug 2026 16:25:29 -0700 Subject: [PATCH 3/8] chore: refactor to use code from creating scorer when updating scorers --- src/functions/create.rs | 109 +++++------------------ src/functions/mod.rs | 1 + src/functions/scorer_config.rs | 153 +++++++++++++++++++++++++++++++++ src/functions/update.rs | 150 ++++---------------------------- 4 files changed, 196 insertions(+), 217 deletions(-) create mode 100644 src/functions/scorer_config.rs diff --git a/src/functions/create.rs b/src/functions/create.rs index 77dff0eb..75f77e25 100644 --- a/src/functions/create.rs +++ b/src/functions/create.rs @@ -1,20 +1,17 @@ use anyhow::{bail, Context, Result}; use clap::{builder::BoolishValueParser, ArgGroup, Args}; use dialoguer::Input; -use serde_json::{json, Map, Value}; +use serde_json::{json, Value}; use crate::{ error::user_error, ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, - utils::{merge_json_objects, read_text_source, read_yaml_object_source}, }; use super::{ api, - prompt_config::{ - parse_choice_scores_source, parse_classifications_source, validate_unit_interval, - PromptConfigArgs, - }, + prompt_config::PromptConfigArgs, + scorer_config::{build_scorer_config, ScorerConfig}, IfExistsMode, ResolvedContext, }; @@ -275,101 +272,41 @@ fn build_scorer_definition( name: &str, slug: &str, ) -> Result { - let prompt = resolve_prompt_block(args)?; - let (function_type, parser) = resolve_output_parser(args)?; - - let mut prompt_data = json!({ - "prompt": prompt, - "parser": parser, - }) - .as_object() - .expect("prompt data is an object") - .clone(); - let prompt_config = args - .prompt_config - .build_prompt_data_patch(Some(&args.model))?; - merge_json_objects(&mut prompt_data, &prompt_config); + let config = build_scorer_config( + &ScorerConfig { + messages: Some(&args.messages), + model: Some(&args.model), + prompt_config: &args.prompt_config, + choice_scores: args.choice_scores.as_deref(), + classifications: args.classifications.as_deref(), + use_cot: Some(args.use_cot), + allow_no_match: args.classifications.as_ref().map(|_| args.allow_no_match), + pass_threshold: args.pass_threshold, + metadata: args.metadata.as_deref(), + metadata_label: "scorer metadata", + }, + true, + )?; let mut definition = json!({ "project_id": project_id, "name": name, "slug": slug, - "function_data": { - "type": "prompt", - }, - "prompt_data": prompt_data, + "function_data": { "type": "prompt" }, "if_exists": args.if_exists.as_str(), - "function_type": function_type, }); + definition + .as_object_mut() + .expect("scorer definition is an object") + .extend(config); if let Some(description) = args.description.as_deref() { definition["description"] = Value::String(description.to_string()); } - let metadata = resolve_metadata(args)?; - if !metadata.is_empty() { - definition["metadata"] = Value::Object(metadata); - } - Ok(definition) } -fn resolve_output_parser(args: &CreateArgs) -> Result<(&'static str, Value)> { - match ( - args.choice_scores.as_deref(), - args.classifications.as_deref(), - ) { - (Some(source), None) => Ok(( - "scorer", - json!({ - "type": "llm_classifier", - "use_cot": args.use_cot, - "choice_scores": parse_choice_scores_source(source)?, - }), - )), - (None, Some(source)) => Ok(( - "classifier", - json!({ - "type": "llm_classifier", - "use_cot": args.use_cot, - "choice": parse_classifications_source(source)?, - "allow_no_match": args.allow_no_match, - }), - )), - (Some(_), Some(_)) => bail!( - "use either --choice-scores for score output or --classifications for classification output, not both" - ), - (None, None) => bail!( - "output choices required. Pass --choice-scores or --classifications " - ), - } -} - -fn resolve_metadata(args: &CreateArgs) -> Result> { - let mut metadata = match args.metadata.as_deref() { - Some(source) => read_yaml_object_source(source, "scorer metadata")?, - None => Map::new(), - }; - if let Some(pass_threshold) = args.pass_threshold { - validate_unit_interval(pass_threshold, "--pass-threshold")?; - metadata.insert("__pass_threshold".to_string(), json!(pass_threshold)); - } - Ok(metadata) -} - -fn resolve_prompt_block(args: &CreateArgs) -> Result { - let raw = read_text_source(&args.messages, "messages")?; - parse_messages(&raw) -} - -fn parse_messages(raw: &str) -> Result { - let messages: Value = serde_json::from_str(raw).context("invalid JSON in scorer messages")?; - match messages { - Value::Array(_) => Ok(json!({ "type": "chat", "messages": messages })), - _ => bail!("scorer messages must be a JSON array"), - } -} - #[cfg(test)] mod tests { use clap::Parser; diff --git a/src/functions/mod.rs b/src/functions/mod.rs index aa445272..7861817a 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -22,6 +22,7 @@ pub(crate) mod prompt_patch; mod pull; mod push; pub(crate) mod report; +mod scorer_config; mod update; mod view; diff --git a/src/functions/scorer_config.rs b/src/functions/scorer_config.rs new file mode 100644 index 00000000..96b2a427 --- /dev/null +++ b/src/functions/scorer_config.rs @@ -0,0 +1,153 @@ +use anyhow::{bail, Context, Result}; +use serde_json::{json, Map, Value}; + +use crate::utils::{merge_json_objects, read_text_source, read_yaml_object_source}; + +use super::prompt_config::{ + parse_choice_scores_source, parse_classifications_source, validate_unit_interval, + PromptConfigArgs, +}; + +/// Inputs shared by scorer creation and partial scorer updates. +/// +/// Creation supplies all required values, while update leaves unchanged values +/// as `None`. Keeping this independent of the clap structs lets both commands +/// share schema construction without weakening create-time CLI requirements. +pub(crate) struct ScorerConfig<'a> { + pub(crate) messages: Option<&'a str>, + pub(crate) model: Option<&'a str>, + pub(crate) prompt_config: &'a PromptConfigArgs, + pub(crate) choice_scores: Option<&'a str>, + pub(crate) classifications: Option<&'a str>, + pub(crate) use_cot: Option, + pub(crate) allow_no_match: Option, + pub(crate) pass_threshold: Option, + pub(crate) metadata: Option<&'a str>, + pub(crate) metadata_label: &'a str, +} + +/// Build the top-level fields containing a scorer's prompt configuration. +/// +/// The result is a complete set of fields for create when `require_output` is +/// true and a partial patch for update otherwise. +pub(crate) fn build_scorer_config( + config: &ScorerConfig<'_>, + require_output: bool, +) -> Result> { + validate_output_selection(config, require_output)?; + + let mut result = Map::new(); + let mut prompt_data = Map::new(); + + if let Some(source) = config.messages { + prompt_data.insert( + "prompt".to_string(), + json!({ + "type": "chat", + "messages": parse_messages_source(source)?, + }), + ); + } + + if let Some(output) = build_output_parser(config)? { + if let Some(function_type) = output.function_type { + result.insert( + "function_type".to_string(), + Value::String(function_type.to_string()), + ); + } + prompt_data.insert("parser".to_string(), Value::Object(output.parser)); + } + + let prompt_config = config.prompt_config.build_prompt_data_patch(config.model)?; + merge_json_objects(&mut prompt_data, &prompt_config); + if !prompt_data.is_empty() { + result.insert("prompt_data".to_string(), Value::Object(prompt_data)); + } + + let metadata = build_metadata(config)?; + if !metadata.is_empty() { + result.insert("metadata".to_string(), Value::Object(metadata)); + } + + Ok(result) +} + +fn validate_output_selection(config: &ScorerConfig<'_>, require_output: bool) -> Result<()> { + match (config.choice_scores, config.classifications) { + (Some(_), Some(_)) => bail!( + "use either --choice-scores for score output or --classifications for classification output, not both" + ), + (None, None) if require_output => bail!( + "output choices required. Pass --choice-scores or --classifications " + ), + _ => {} + } + + if config.choice_scores.is_some() && config.allow_no_match.is_some() { + bail!("--allow-no-match applies to classification output, not --choice-scores"); + } + if config.classifications.is_some() && config.pass_threshold.is_some() { + bail!("--pass-threshold applies to score output and cannot be used with --classifications"); + } + Ok(()) +} + +struct OutputParser { + function_type: Option<&'static str>, + parser: Map, +} + +fn build_output_parser(config: &ScorerConfig<'_>) -> Result> { + let mut parser = Map::new(); + let mut function_type = None; + + if let Some(source) = config.choice_scores { + parser.insert("type".to_string(), json!("llm_classifier")); + parser.insert( + "choice_scores".to_string(), + Value::Object(parse_choice_scores_source(source)?), + ); + function_type = Some("scorer"); + } + if let Some(source) = config.classifications { + parser.insert("type".to_string(), json!("llm_classifier")); + parser.insert( + "choice".to_string(), + Value::Array(parse_classifications_source(source)?), + ); + function_type = Some("classifier"); + } + if let Some(use_cot) = config.use_cot { + parser.insert("use_cot".to_string(), Value::Bool(use_cot)); + } + if let Some(allow_no_match) = config.allow_no_match { + parser.insert("allow_no_match".to_string(), Value::Bool(allow_no_match)); + } + + Ok((!parser.is_empty()).then_some(OutputParser { + function_type, + parser, + })) +} + +fn parse_messages_source(source: &str) -> Result { + let raw = read_text_source(source, "messages")?; + let messages: Value = serde_json::from_str(&raw).context("invalid JSON in messages")?; + match messages { + Value::Array(_) => Ok(messages), + _ => bail!("messages must be a JSON array of chat messages"), + } +} + +fn build_metadata(config: &ScorerConfig<'_>) -> Result> { + let mut metadata = match config.metadata { + Some(source) => read_yaml_object_source(source, config.metadata_label)?, + None => Map::new(), + }; + if let Some(pass_threshold) = config.pass_threshold { + validate_unit_interval(pass_threshold, "--pass-threshold")?; + metadata.insert("__pass_threshold".to_string(), json!(pass_threshold)); + } + Ok(metadata) +} diff --git a/src/functions/update.rs b/src/functions/update.rs index 08f4db4b..26f1e5c5 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -6,18 +6,16 @@ use serde_json::{json, Map, Value}; use crate::{ error::user_error, ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, - utils::{merge_json_objects, read_text_source, read_yaml_object_source}, + utils::{merge_json_objects, read_text_source}, }; use super::{ api, create::report_validation_issues, label, label_plural, select_function_interactive, }; use super::{ - prompt_config::{ - parse_choice_scores_source, parse_classifications_source, validate_unit_interval, - PromptConfigArgs, - }, + prompt_config::PromptConfigArgs, prompt_patch::materialize_prompt_data_patch, + scorer_config::{build_scorer_config, ScorerConfig}, FunctionTypeFilter, ResolvedContext, }; @@ -347,7 +345,21 @@ async fn resolve_target_function( } fn build_patch_body(args: &UpdateArgs) -> Result { - let mut patch: Map = Map::new(); + let mut patch = build_scorer_config( + &ScorerConfig { + messages: args.messages.as_deref(), + model: args.model.as_deref(), + prompt_config: &args.prompt_config, + choice_scores: args.choice_scores.as_deref(), + classifications: args.classifications.as_deref(), + use_cot: args.use_cot, + allow_no_match: args.allow_no_match, + pass_threshold: args.pass_threshold, + metadata: args.metadata.as_deref(), + metadata_label: "function metadata", + }, + false, + )?; if let Some(description) = args.description.as_deref() { patch.insert( @@ -356,57 +368,7 @@ fn build_patch_body(args: &UpdateArgs) -> Result { ); } - let metadata = resolve_metadata(args)?; - if !metadata.is_empty() { - patch.insert("metadata".to_string(), Value::Object(metadata)); - } - - if let Some(messages) = resolve_messages(args)? { - let prompt_data_patch = json!({ - "prompt_data": { - "prompt": { "type": "chat", "messages": messages }, - }, - }); - merge_json_objects( - &mut patch, - prompt_data_patch - .as_object() - .expect("prompt data patch is an object"), - ); - } - - let parser_patch = resolve_parser_patch(args)?; - if let Some((function_type, parser)) = parser_patch { - if let Some(function_type) = function_type { - patch.insert( - "function_type".to_string(), - Value::String(function_type.to_string()), - ); - } - let prompt_data_patch = json!({ "prompt_data": { "parser": parser } }); - merge_json_objects( - &mut patch, - prompt_data_patch - .as_object() - .expect("prompt data patch is an object"), - ); - } - - let prompt_config = args - .prompt_config - .build_prompt_data_patch(args.model.as_deref())?; - if !prompt_config.is_empty() { - let prompt_data_patch = json!({ "prompt_data": prompt_config }); - merge_json_objects( - &mut patch, - prompt_data_patch - .as_object() - .expect("prompt data patch is an object"), - ); - } - - let extra = resolve_extra_patch(args)?; - if let Some(extra_obj) = extra { + if let Some(extra_obj) = resolve_extra_patch(args)? { merge_json_objects(&mut patch, &extra_obj); } @@ -417,80 +379,6 @@ fn build_patch_body(args: &UpdateArgs) -> Result { Ok(Value::Object(patch)) } -fn resolve_metadata(args: &UpdateArgs) -> Result> { - if args.classifications.is_some() && args.pass_threshold.is_some() { - bail!("--pass-threshold applies to score output and cannot be used with --classifications"); - } - - let mut metadata = match args.metadata.as_deref() { - Some(source) => read_yaml_object_source(source, "function metadata")?, - None => Map::new(), - }; - if let Some(pass_threshold) = args.pass_threshold { - validate_unit_interval(pass_threshold, "--pass-threshold")?; - metadata.insert("__pass_threshold".to_string(), json!(pass_threshold)); - } - Ok(metadata) -} - -fn resolve_parser_patch(args: &UpdateArgs) -> Result, Value)>> { - if args.choice_scores.is_some() && args.allow_no_match.is_some() { - bail!("--allow-no-match applies to classification output, not --choice-scores"); - } - - let mut parser = Map::new(); - let mut function_type = None; - - if let Some(source) = args.choice_scores.as_deref() { - parser.insert( - "type".to_string(), - Value::String("llm_classifier".to_string()), - ); - parser.insert( - "choice_scores".to_string(), - Value::Object(parse_choice_scores_source(source)?), - ); - function_type = Some("scorer"); - } - if let Some(source) = args.classifications.as_deref() { - parser.insert( - "type".to_string(), - Value::String("llm_classifier".to_string()), - ); - parser.insert( - "choice".to_string(), - Value::Array(parse_classifications_source(source)?), - ); - function_type = Some("classifier"); - } - if let Some(use_cot) = args.use_cot { - parser.insert("use_cot".to_string(), Value::Bool(use_cot)); - } - if let Some(allow_no_match) = args.allow_no_match { - parser.insert("allow_no_match".to_string(), Value::Bool(allow_no_match)); - } - - if parser.is_empty() { - Ok(None) - } else { - Ok(Some((function_type, Value::Object(parser)))) - } -} - -fn resolve_messages(args: &UpdateArgs) -> Result> { - match args.messages.as_deref() { - Some(source) => { - let raw = read_text_source(source, "messages")?; - let parsed: Value = serde_json::from_str(&raw).context("invalid JSON in --messages")?; - match parsed { - Value::Array(_) => Ok(Some(parsed)), - _ => bail!("--messages must be a JSON array of chat messages"), - } - } - None => Ok(None), - } -} - fn resolve_extra_patch(args: &UpdateArgs) -> Result>> { let Some(source) = args.patch.as_deref() else { return Ok(None); From 2751f3aed051513501c298b23161363601015980 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Thu, 20 Aug 2026 16:09:02 -0700 Subject: [PATCH 4/8] chore: fix precommit --- README.md | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 78755829..5a382e78 100644 --- a/README.md +++ b/README.md @@ -135,26 +135,26 @@ Remove-Item -Recurse -Force (Join-Path $env:APPDATA "bt") -ErrorAction SilentlyC ## Commands -| Command | Description | -| ------------- | ------------------------------------------------------------------ | -| `bt init` | Initialize `.bt/` config directory and link to a project | -| `bt login` | Log in to Braintrust or refresh an OAuth login | -| `bt logout` | Remove a saved Braintrust login | -| `bt profiles` | List, delete, and rename saved login profiles | -| `bt switch` | Switch org and project context | -| `bt status` | Show current org and project context | -| `bt datasets` | Manage datasets and dataset pipelines | -| `bt eval` | Run eval files (Unix only) | -| `bt sql` | Run SQL queries against Braintrust | -| `bt view` | View logs, traces, and spans | -| `bt projects` | Manage projects (list, create, view, delete) | -| `bt datasets` | Manage remote datasets (list, create, update, view, delete) | -| `bt prompts` | Manage prompts (list, view, delete) | -| `bt functions` | Manage functions (list, view, invoke, update, push, pull, delete) | +| Command | Description | +| -------------- | ------------------------------------------------------------------ | +| `bt init` | Initialize `.bt/` config directory and link to a project | +| `bt login` | Log in to Braintrust or refresh an OAuth login | +| `bt logout` | Remove a saved Braintrust login | +| `bt profiles` | List, delete, and rename saved login profiles | +| `bt switch` | Switch org and project context | +| `bt status` | Show current org and project context | +| `bt datasets` | Manage datasets and dataset pipelines | +| `bt eval` | Run eval files (Unix only) | +| `bt sql` | Run SQL queries against Braintrust | +| `bt view` | View logs, traces, and spans | +| `bt projects` | Manage projects (list, create, view, delete) | +| `bt datasets` | Manage remote datasets (list, create, update, view, delete) | +| `bt prompts` | Manage prompts (list, view, delete) | +| `bt functions` | Manage functions (list, view, invoke, update, push, pull, delete) | | `bt tools` | Manage tools (list, view, invoke, update, delete) | -| `bt scorers` | Manage scorers (list, create, view, invoke, update, delete) | -| `bt sync` | Synchronize project logs between Braintrust and local NDJSON files | -| `bt update` | Update bt in-place | +| `bt scorers` | Manage scorers (list, create, view, invoke, update, delete) | +| `bt sync` | Synchronize project logs between Braintrust and local NDJSON files | +| `bt update` | Update bt in-place | ## `bt scorers` From 82a3f5bcb57463855b84e050fe77891da24ba06e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Thu, 20 Aug 2026 16:38:57 -0700 Subject: [PATCH 5/8] chore: fix test --- src/functions/create.rs | 2 +- src/functions/prompt_patch.rs | 26 ++++- src/functions/update.rs | 199 ++++++++++++---------------------- 3 files changed, 91 insertions(+), 136 deletions(-) diff --git a/src/functions/create.rs b/src/functions/create.rs index 75f77e25..a3beee0b 100644 --- a/src/functions/create.rs +++ b/src/functions/create.rs @@ -170,7 +170,7 @@ pub(crate) async fn run(ctx: &ResolvedContext, args: &CreateArgs, json_output: b Ok(()) } -pub(super) fn report_validation_issues(report: &api::FunctionValidationReport) -> Result<()> { +fn report_validation_issues(report: &api::FunctionValidationReport) -> Result<()> { let mut blocking = Vec::new(); for result in &report.results { for issue in &result.issues { diff --git a/src/functions/prompt_patch.rs b/src/functions/prompt_patch.rs index 3a3ba7af..fded4fca 100644 --- a/src/functions/prompt_patch.rs +++ b/src/functions/prompt_patch.rs @@ -20,6 +20,20 @@ pub(crate) fn materialize_prompt_data_patch( .cloned() .unwrap_or_default(); merge_json_objects(&mut merged, &patch_prompt_data); + + if let (Some(requested), Some(parser)) = ( + patch_prompt_data.get("parser").and_then(Value::as_object), + merged.get_mut("parser").and_then(Value::as_object_mut), + ) { + if let Some(scores) = requested.get("choice_scores") { + parser.remove("choice"); + parser.remove("allow_no_match"); + parser.insert("choice_scores".to_string(), scores.clone()); + } else if requested.contains_key("choice") { + parser.remove("choice_scores"); + } + } + patch["prompt_data"] = Value::Object(merged); } @@ -33,17 +47,23 @@ mod tests { fn materializes_complete_prompt_data_for_patch() { let existing = json!({ "prompt": {"type": "chat", "messages": []}, - "parser": {"type": "llm_classifier"}, + "parser": {"type": "llm_classifier", "choice_scores": {"old": 0}}, "options": {"model": "test-model", "params": {"temperature": 0.5}} }); let mut patch = json!({ - "prompt_data": {"options": {"params": {"temperature": 0.2}}} + "prompt_data": { + "parser": {"choice_scores": {"new": 1}}, + "options": {"params": {"temperature": 0.2}} + } }); materialize_prompt_data_patch(&mut patch, Some(&existing)); assert_eq!(patch["prompt_data"]["prompt"], existing["prompt"]); - assert_eq!(patch["prompt_data"]["parser"], existing["parser"]); + assert_eq!( + patch["prompt_data"]["parser"]["choice_scores"], + json!({"new": 1}) + ); assert_eq!(patch["prompt_data"]["options"]["model"], "test-model"); assert_eq!( patch["prompt_data"]["options"]["params"]["temperature"], diff --git a/src/functions/update.rs b/src/functions/update.rs index 26f1e5c5..e88ceca5 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -1,22 +1,19 @@ use anyhow::{anyhow, bail, Context, Result}; use clap::{builder::BoolishValueParser, Args}; use dialoguer::Confirm; -use serde_json::{json, Map, Value}; +use serde_json::{Map, Value}; use crate::{ - error::user_error, ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, utils::{merge_json_objects, read_text_source}, }; use super::{ - api, create::report_validation_issues, label, label_plural, select_function_interactive, -}; -use super::{ + api, label, label_plural, prompt_config::PromptConfigArgs, prompt_patch::materialize_prompt_data_patch, scorer_config::{build_scorer_config, ScorerConfig}, - FunctionTypeFilter, ResolvedContext, + select_function_interactive, FunctionTypeFilter, ResolvedContext, }; /// Update a function's prompt configuration or metadata in place. @@ -57,12 +54,13 @@ pub struct UpdateArgs { #[command(flatten)] prompt_config: PromptConfigArgs, - /// Replace choice-to-score mappings for score output. Accepts inline JSON, - /// @PATH to read from a file, or - for stdin. + /// Replace choice-to-score mappings for an existing score-output scorer. + /// Accepts inline JSON, @PATH to read from a file, or - for stdin. #[arg(long, value_name = "SOURCE", conflicts_with = "classifications")] choice_scores: Option, - /// Replace labels for classification output. Accepts an inline JSON array, + /// Replace labels for an existing classifier. This cannot change a + /// score-output scorer into a classifier. Accepts an inline JSON array, /// @PATH to read from a file, or - for stdin. #[arg(long, value_name = "SOURCE", conflicts_with = "choice_scores")] classifications: Option, @@ -85,7 +83,8 @@ pub struct UpdateArgs { )] allow_no_match: Option, - /// Update the score threshold for passing, between 0 and 1. + /// Update the score threshold for an existing score-output scorer, between + /// 0 and 1. #[arg(long, value_name = "NUMBER", conflicts_with = "classifications")] pass_threshold: Option, @@ -156,46 +155,6 @@ enum UpdateSelector<'a> { Slug(Option<&'a str>), } -fn validation_candidate(function: &api::Function, patch: &Value) -> Value { - let mut candidate = json!({ - "project_id": function.project_id, - "name": function.name, - "slug": function.slug, - }); - let object = candidate - .as_object_mut() - .expect("validation candidate is an object"); - - for (key, value) in [ - ( - "description", - function.description.as_ref().map(|v| json!(v)), - ), - ( - "function_type", - function.function_type.as_ref().map(|v| json!(v)), - ), - ("prompt_data", function.prompt_data.clone()), - ("function_data", function.function_data.clone()), - ("tags", function.tags.as_ref().map(|v| json!(v))), - ("metadata", function.metadata.clone()), - ] { - if let Some(value) = value { - object.insert(key.to_string(), value); - } - } - - // PATCH replaces each top-level value. `prompt_data` has already been - // materialized into a complete value before this helper is called. - for (key, value) in patch - .as_object() - .expect("function update patch is an object") - { - object.insert(key.clone(), value.clone()); - } - candidate -} - pub async fn run( ctx: &ResolvedContext, args: &UpdateArgs, @@ -205,67 +164,51 @@ pub async fn run( let mut body = build_patch_body(args)?; let function = resolve_target_function(ctx, args, ft).await?; + if !function_matches_filter(&function, ft) { + bail!("'{}' is not a {}", function.name, label(ft)); + } - // LLM scorer/classifier output flags only apply to prompt-based scorers and - // classifiers. Reject them on other function kinds (for example tools) so an - // unrelated function is not silently patched with a parser it cannot use. - let is_scorer_like = matches!( - function.function_type.as_deref(), - Some("scorer") | Some("classifier") - ); - let scorer_flags = args.scorer_output_flags(); - if !scorer_flags.is_empty() && !is_scorer_like { + let implementation = function + .function_data + .as_ref() + .and_then(|data| data.get("type")) + .and_then(Value::as_str) + .unwrap_or("prompt"); + if body.get("prompt_data").is_some() && implementation != "prompt" { bail!( - "{} apply to LLM scorers and classifiers, not {} '{}'. \ - Run `bt scorers update` on a scorer instead.", - scorer_flags.join(", "), - label(ft), - function.name, + "prompt configuration cannot update {}-backed function '{}'", + implementation, + function.name ); } - // Mirrors `create`, where --allow-no-match requires --classifications: a - // score parser would never consult it. - let produces_classifications = - args.classifications.is_some() || function.function_type.as_deref() == Some("classifier"); - if args.allow_no_match.is_some() && !produces_classifications { + let function_type = function.function_type.as_deref(); + let scorer_flags = args.scorer_output_flags(); + if !scorer_flags.is_empty() && !matches!(function_type, Some("scorer") | Some("classifier")) { bail!( - "--allow-no-match applies to classification output, but '{}' produces scores. \ - Pass --classifications to switch it to labels.", - function.name, + "{} apply only to scorers and classifiers", + scorer_flags.join(", ") ); } + if matches!( + ( + function_type, + args.choice_scores.is_some(), + args.classifications.is_some() + ), + (Some("classifier"), true, _) | (Some("scorer"), _, true) + ) { + bail!("PATCH cannot change between score and classification output"); + } + if args.allow_no_match.is_some() && function_type != Some("classifier") { + bail!("--allow-no-match applies only to classification output"); + } + if args.pass_threshold.is_some() && function_type == Some("classifier") { + bail!("--pass-threshold applies only to score output"); + } materialize_prompt_data_patch(&mut body, function.prompt_data.as_ref()); - // Last of the up-front checks because it hits the network. Validate the - // complete candidate rather than the partial PATCH body so the backend can - // apply the same model-parameter checks as it does for scorer creation. - let candidate = validation_candidate(&function, &body); - let validation = with_spinner( - "Validating function...", - api::validate_functions(&ctx.client, std::slice::from_ref(&candidate)), - ) - .await?; - report_validation_issues(&validation).map_err(user_error)?; - - // Switching output mode updates function_type, but materialization merges - // the parser and does not drop the previous mode's keys. Warn so the - // user can review or recreate for a clean switch. - if !crate::ui::is_quiet() { - match function.function_type.as_deref() { - Some("classifier") if args.choice_scores.is_some() => print_command_status( - CommandStatus::Warning, - "Switching to score output; previous classification labels may remain in the definition. Review with `bt scorers view`.", - ), - Some("scorer") if args.classifications.is_some() => print_command_status( - CommandStatus::Warning, - "Switching to classification output; previous choice scores may remain in the definition. Review with `bt scorers view`.", - ), - _ => {} - } - } - if !args.yes && is_interactive() { let confirm = Confirm::new() .with_prompt(format!( @@ -316,6 +259,24 @@ pub async fn run( Ok(()) } +fn function_matches_filter(function: &api::Function, ft: Option) -> bool { + match ft { + None => true, + Some(FunctionTypeFilter::Scorer) => { + function.function_type.as_deref() == Some("scorer") + || function.function_type.as_deref() == Some("classifier") + && function.prompt_data.is_some() + && function + .function_data + .as_ref() + .and_then(|data| data.get("type")) + .and_then(Value::as_str) + != Some("topic_map") + } + Some(expected) => function.function_type.as_deref() == Some(expected.as_str()), + } +} + async fn resolve_target_function( ctx: &ResolvedContext, args: &UpdateArgs, @@ -360,6 +321,9 @@ fn build_patch_body(args: &UpdateArgs) -> Result { }, false, )?; + // PATCH /v1/function does not support changing function_type. Output-mode + // changes are rejected after resolving the current function. + patch.remove("function_type"); if let Some(description) = args.description.as_deref() { patch.insert( @@ -398,6 +362,7 @@ fn parse_patch_object(raw: &str) -> Result> { #[cfg(test)] mod tests { use clap::Parser; + use serde_json::json; use super::*; @@ -429,36 +394,6 @@ mod tests { } } - #[test] - fn validation_candidate_applies_top_level_patch_to_existing_function() { - let function = api::Function { - id: "fn_test_update".to_string(), - name: "Test scorer".to_string(), - slug: "test-scorer".to_string(), - project_id: "test-project".to_string(), - description: Some("Old description".to_string()), - function_type: Some("scorer".to_string()), - prompt_data: Some(json!({"options": {"model": "test-model"}})), - function_data: Some(json!({"type": "prompt"})), - tags: Some(vec!["test-tag".to_string()]), - metadata: Some(json!({"owner": "test-user"})), - created: None, - _xact_id: None, - }; - let patch = json!({ - "description": "New description", - "prompt_data": {"options": {"model": "test-model-2"}} - }); - - let candidate = validation_candidate(&function, &patch); - - assert_eq!(candidate["project_id"], "test-project"); - assert_eq!(candidate["description"], "New description"); - assert_eq!(candidate["prompt_data"], patch["prompt_data"]); - assert_eq!(candidate["function_data"], function.function_data.unwrap()); - assert!(candidate.get("id").is_none()); - } - #[test] fn scorer_output_flags_reported_only_when_set() { let base = args(None, None); @@ -580,7 +515,7 @@ mod tests { args.metadata = Some("owner: test-team".to_string()); let body = build_patch_body(&args).expect("patch body"); - assert_eq!(body["function_type"], "classifier"); + assert!(body.get("function_type").is_none()); assert_eq!( body["prompt_data"]["parser"]["choice"], json!(["safe", "unsafe"]) @@ -596,7 +531,7 @@ mod tests { args.pass_threshold = Some(0.8); let body = build_patch_body(&args).expect("patch body"); - assert_eq!(body["function_type"], "scorer"); + assert!(body.get("function_type").is_none()); assert_eq!( body["prompt_data"]["parser"]["choice_scores"], json!({"pass": 1, "fail": 0}) From 2e1c889955752329a14e8b3f664f4aa0f88c476b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Fri, 21 Aug 2026 14:14:04 -0700 Subject: [PATCH 6/8] fix(function): opening functions in the web works `bt functions view --web` with a function works --- src/functions/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/functions/mod.rs b/src/functions/mod.rs index 7861817a..8d6dda79 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -117,7 +117,6 @@ pub enum PushLanguage { fn build_web_path(function: &Function) -> String { let id = &function.id; match function.function_type.as_deref() { - Some("tool") => format!("tools?pr={}", urlencoding::encode(id)), Some("scorer") => format!("scorers/{}", urlencoding::encode(id)), Some("classifier") if function.prompt_data.is_some() => { format!("scorers/{}", urlencoding::encode(id)) @@ -131,7 +130,8 @@ fn build_web_path(function: &Function) -> String { ) } Some("parameters") => format!("parameters/{}", urlencoding::encode(id)), - _ => format!("functions/{}", urlencoding::encode(id)), + Some("llm") => format!("prompts/{}", urlencoding::encode(id)), + _ => format!("tools?pr={}", urlencoding::encode(id)), } } From c03cb962912b1b7e2cd3a9a4abd171aff4de9b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Fri, 21 Aug 2026 15:04:02 -0700 Subject: [PATCH 7/8] chore: major redesign of functions/tools update they were just following scorers update before, with the same arguments --- README.md | 6 +- src/functions/mod.rs | 87 +++++++- src/functions/update.rs | 430 ++++++++++++++++++++++++++-------------- src/scorers.rs | 10 +- tests/functions.rs | 33 +++ 5 files changed, 404 insertions(+), 162 deletions(-) diff --git a/README.md b/README.md index 5a382e78..f589c1d6 100644 --- a/README.md +++ b/README.md @@ -181,9 +181,9 @@ Update only the fields you specify, or use `--patch` for fields without dedicate ```bash bt scorers update helpfulness --messages @messages.json -bt scorers update helpfulness --model gpt-5.4-nano -bt functions update my-function --description "Updated" -bt tools update my-tool --patch @tool-patch.json +bt scorers update helpfulness --new-slug answer-helpfulness +bt functions update my-function --name "Updated function" --description "Updated" +bt tools update my-tool --prompt @prompt.txt --new-slug lookup-order ``` The API replaces `prompt_data` rather than merging it, so `bt` reads the current definition and sends a materialized replacement with your changes. A concurrent edit can therefore be overwritten. diff --git a/src/functions/mod.rs b/src/functions/mod.rs index 8d6dda79..95767b2c 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -176,7 +176,7 @@ Examples: bt tools view my-tool bt tools view fn_123 bt tools view --id fn_123 - bt tools update my-tool --patch @tool-patch.json + bt tools update my-tool --name \"Lookup order\" --new-slug lookup-order ")] pub struct FunctionArgs { #[command(subcommand)] @@ -193,8 +193,22 @@ pub(crate) enum FunctionCommands { Delete(DeleteArgs), /// Invoke by slug Invoke(invoke::InvokeArgs), - /// Update a function in place (prompt configuration, metadata, or arbitrary patch) - Update(Box), + /// Update tool fields or prompt content + Update(Box), +} + +#[derive(Debug, Clone, Subcommand)] +pub(crate) enum ScorerFunctionCommands { + /// List all in the current project + List, + /// View details + View(ViewArgs), + /// Delete by slug + Delete(DeleteArgs), + /// Invoke by slug + Invoke(invoke::InvokeArgs), + /// Update scorer configuration + Update(Box), } #[derive(Debug, Clone, Args)] @@ -227,7 +241,7 @@ enum FunctionsCommands { Delete(FunctionsDeleteArgs), /// Invoke a function Invoke(FunctionsInvokeArgs), - /// Update a function in place (prompt configuration, metadata, or arbitrary patch) + /// Update common function fields or apply an arbitrary patch Update(Box), /// Push local function definitions Push(PushArgs), @@ -281,7 +295,7 @@ struct FunctionsInvokeArgs { #[derive(Debug, Clone, Args)] struct FunctionsUpdateArgs { #[command(flatten)] - inner: update::UpdateArgs, + inner: update::GenericUpdateArgs, /// Filter by function type (for interactive selection) #[arg(long = "type", short = 't', value_enum)] function_type: Option, @@ -672,7 +686,7 @@ pub(crate) async fn run_typed_command( None | Some(FunctionCommands::List) => list::run(&ctx, base.json, ft).await, Some(FunctionCommands::Delete(d)) => delete::run(&ctx, d.slug(), d.force, ft).await, Some(FunctionCommands::Invoke(i)) => invoke::run(&ctx, &i, base.json, ft).await, - Some(FunctionCommands::Update(u)) => update::run(&ctx, &u, base.json, ft).await, + Some(FunctionCommands::Update(u)) => update::run_tool(&ctx, &u, base.json).await, Some(FunctionCommands::View(_)) => { unreachable!("handled before context resolution") } @@ -687,6 +701,59 @@ pub(crate) async fn run_scorer_create(base: BaseArgs, args: create::CreateArgs) create::run(&ctx, &args, json_output).await } +pub(crate) async fn run_scorer_command( + base: BaseArgs, + command: Option, +) -> Result<()> { + let ft = Some(FunctionTypeFilter::Scorer); + match command { + Some(ScorerFunctionCommands::View(v)) => match v.selector()? { + ViewSelector::Id(id) => { + let auth_ctx = resolve_auth_context(&base).await?; + view::run_by_id( + &auth_ctx, + id, + v.version.as_deref(), + base.json, + v.web, + base.verbose, + ft, + ) + .await + } + ViewSelector::Slug(slug) => { + let ctx = resolve_context(&base).await?; + view::run( + &ctx, + slug, + v.version.as_deref(), + base.json, + v.web, + base.verbose, + ft, + ) + .await + } + }, + command => { + let ctx = resolve_context(&base).await?; + match command { + None | Some(ScorerFunctionCommands::List) => list::run(&ctx, base.json, ft).await, + Some(ScorerFunctionCommands::Delete(d)) => { + delete::run(&ctx, d.slug(), d.force, ft).await + } + Some(ScorerFunctionCommands::Invoke(i)) => { + invoke::run(&ctx, &i, base.json, ft).await + } + Some(ScorerFunctionCommands::Update(u)) => { + update::run_scorer(&ctx, &u, base.json).await + } + Some(ScorerFunctionCommands::View(_)) => unreachable!("handled above"), + } + } + } +} + pub async fn run(base: BaseArgs, args: FunctionsArgs) -> Result<()> { let function_type = args.function_type; match args.command { @@ -737,7 +804,13 @@ pub async fn run(base: BaseArgs, args: FunctionsArgs) -> Result<()> { invoke::run(&ctx, &i.inner, base.json, i.function_type.or(function_type)).await } Some(FunctionsCommands::Update(u)) => { - update::run(&ctx, &u.inner, base.json, u.function_type.or(function_type)).await + update::run_generic( + &ctx, + &u.inner, + base.json, + u.function_type.or(function_type), + ) + .await } Some(FunctionsCommands::Push(_)) | Some(FunctionsCommands::Pull(_)) diff --git a/src/functions/update.rs b/src/functions/update.rs index e88ceca5..870e1d80 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -16,31 +16,57 @@ use super::{ select_function_interactive, FunctionTypeFilter, ResolvedContext, }; -/// Update a function's prompt configuration or metadata in place. -/// -/// This wraps `PATCH /v1/function/{id}`. The endpoint replaces `prompt_data` -/// wholesale, so the command reads the current definition and materializes a -/// complete replacement while changing only the fields requested by the user. +/// Fields shared by every function kind. +#[derive(Debug, Clone, Args)] +pub(crate) struct CommonUpdateArgs { + #[command(flatten)] + slug: super::SlugArgs, + + /// Function id (alternative to slug). Auto-detected for `fn_`/`func_` prefixes. + #[arg(long = "id")] + id: Option, + + /// Update the display name. + #[arg(long, value_name = "NAME")] + name: Option, + + /// Update the slug. `--slug` identifies the current function. + #[arg(long, value_name = "SLUG")] + new_slug: Option, + + /// Deep-merge metadata from inline YAML, @PATH, or stdin (-). + #[arg(long, value_name = "SOURCE")] + metadata: Option, + + /// Update the function description. + #[arg(long, short = 'd', value_name = "TEXT")] + description: Option, + + /// Arbitrary JSON object deep-merged into the function. Accepts inline + /// JSON, @PATH to read JSON from a file, or - for stdin. + #[arg(long, value_name = "SOURCE")] + patch: Option, + + /// Skip the confirmation prompt. + #[arg(long, short = 'y')] + yes: bool, +} + +/// Update scorer prompt configuration and output behavior. #[derive(Debug, Clone, Args)] #[command(after_help = "\ Examples: + bt scorers update my-scorer --name \"Helpfulness\" --new-slug helpfulness bt scorers update my-scorer --messages @messages.json bt scorers update my-scorer --model gpt-5.4-nano --reasoning-effort none --temperature 0.1 bt scorers update my-scorer --template-format jinja --pass-threshold 0.7 bt scorers update my-scorer --classifications '[\"safe\",\"unsafe\"]' bt scorers update my-scorer --metadata @metadata.yaml - bt scorers update my-scorer --description \"Helpfulness judge\" - bt scorers update my-scorer --patch '{\"prompt_data\":{\"options\":{\"model\":\"gpt-5.4-nano\"}}}' bt scorers update --id fn_123 --patch @scorer-patch.json - bt tools update my-tool --patch @tool-patch.json ")] -pub struct UpdateArgs { +pub(crate) struct ScorerUpdateArgs { #[command(flatten)] - slug: super::SlugArgs, - - /// Function id (alternative to slug). Auto-detected for `fn_`/`func_` prefixes. - #[arg(long = "id")] - id: Option, + common: CommonUpdateArgs, /// Replacement chat messages source: inline JSON, @PATH to read from a /// file, or - for stdin. @@ -87,50 +113,43 @@ pub struct UpdateArgs { /// 0 and 1. #[arg(long, value_name = "NUMBER", conflicts_with = "classifications")] pass_threshold: Option, +} - /// Deep-merge metadata from inline YAML, @PATH, or stdin (-). - #[arg(long, value_name = "SOURCE")] - metadata: Option, - - /// Update the function description. - #[arg(long, short = 'd', value_name = "TEXT")] - description: Option, +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt tools update my-tool --name \"Lookup order\" --new-slug lookup-order + bt tools update my-tool --description \"Look up an order by id\" + bt tools update my-tool --prompt @prompt.txt + bt tools update --id fn_123 --patch @tool-patch.json +")] +pub(crate) struct ToolUpdateArgs { + #[command(flatten)] + common: CommonUpdateArgs, - /// Arbitrary JSON object deep-merged into the function. Accepts inline - /// JSON, @PATH to read JSON from a file, or - for stdin. - #[arg(long, value_name = "SOURCE")] - patch: Option, + /// Replace a completion prompt from inline text, @PATH, or stdin (-). + #[arg(long, value_name = "SOURCE", conflicts_with = "messages")] + prompt: Option, - /// Skip the confirmation prompt. - #[arg(long, short = 'y')] - yes: bool, + /// Replace chat messages from inline JSON/YAML, @PATH, or stdin (-). + #[arg(long, value_name = "SOURCE", conflicts_with = "prompt")] + messages: Option, } -impl UpdateArgs { - /// Flags that only make sense for LLM scorers and classifiers. - /// - /// Returns the flag names that were set so callers can reject them on other - /// function kinds (for example tools) with an actionable message. - fn scorer_output_flags(&self) -> Vec<&'static str> { - let mut flags = Vec::new(); - if self.choice_scores.is_some() { - flags.push("--choice-scores"); - } - if self.classifications.is_some() { - flags.push("--classifications"); - } - if self.allow_no_match.is_some() { - flags.push("--allow-no-match"); - } - if self.use_cot.is_some() { - flags.push("--use-cot"); - } - if self.pass_threshold.is_some() { - flags.push("--pass-threshold"); - } - flags - } +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt functions update my-function --name \"New name\" + bt functions update my-function --new-slug new-slug + bt functions update my-function --description \"Updated description\" + bt functions update --id fn_123 --patch @function-patch.json +")] +pub(crate) struct GenericUpdateArgs { + #[command(flatten)] + common: CommonUpdateArgs, +} +impl CommonUpdateArgs { fn selector(&self) -> Result> { match ( self.id.as_deref(), @@ -155,15 +174,59 @@ enum UpdateSelector<'a> { Slug(Option<&'a str>), } -pub async fn run( +pub(crate) async fn run_scorer( ctx: &ResolvedContext, - args: &UpdateArgs, + args: &ScorerUpdateArgs, + json_output: bool, +) -> Result<()> { + let body = build_scorer_patch_body(args)?; + run_update( + ctx, + &args.common, + body, + json_output, + Some(FunctionTypeFilter::Scorer), + Some(args), + ) + .await +} + +pub(crate) async fn run_tool( + ctx: &ResolvedContext, + args: &ToolUpdateArgs, + json_output: bool, +) -> Result<()> { + let body = build_tool_patch_body(args)?; + run_update( + ctx, + &args.common, + body, + json_output, + Some(FunctionTypeFilter::Tool), + None, + ) + .await +} + +pub(crate) async fn run_generic( + ctx: &ResolvedContext, + args: &GenericUpdateArgs, json_output: bool, ft: Option, ) -> Result<()> { - let mut body = build_patch_body(args)?; + let body = build_common_patch_body(&args.common)?; + run_update(ctx, &args.common, body, json_output, ft, None).await +} - let function = resolve_target_function(ctx, args, ft).await?; +async fn run_update( + ctx: &ResolvedContext, + common: &CommonUpdateArgs, + mut body: Value, + json_output: bool, + ft: Option, + scorer_args: Option<&ScorerUpdateArgs>, +) -> Result<()> { + let function = resolve_target_function(ctx, common, ft).await?; if !function_matches_filter(&function, ft) { bail!("'{}' is not a {}", function.name, label(ft)); } @@ -182,34 +245,29 @@ pub async fn run( ); } - let function_type = function.function_type.as_deref(); - let scorer_flags = args.scorer_output_flags(); - if !scorer_flags.is_empty() && !matches!(function_type, Some("scorer") | Some("classifier")) { - bail!( - "{} apply only to scorers and classifiers", - scorer_flags.join(", ") - ); - } - if matches!( - ( - function_type, - args.choice_scores.is_some(), - args.classifications.is_some() - ), - (Some("classifier"), true, _) | (Some("scorer"), _, true) - ) { - bail!("PATCH cannot change between score and classification output"); - } - if args.allow_no_match.is_some() && function_type != Some("classifier") { - bail!("--allow-no-match applies only to classification output"); - } - if args.pass_threshold.is_some() && function_type == Some("classifier") { - bail!("--pass-threshold applies only to score output"); + if let Some(args) = scorer_args { + let function_type = function.function_type.as_deref(); + if matches!( + ( + function_type, + args.choice_scores.is_some(), + args.classifications.is_some() + ), + (Some("classifier"), true, _) | (Some("scorer"), _, true) + ) { + bail!("PATCH cannot change between score and classification output"); + } + if args.allow_no_match.is_some() && function_type != Some("classifier") { + bail!("--allow-no-match applies only to classification output"); + } + if args.pass_threshold.is_some() && function_type == Some("classifier") { + bail!("--pass-threshold applies only to score output"); + } } materialize_prompt_data_patch(&mut body, function.prompt_data.as_ref()); - if !args.yes && is_interactive() { + if !common.yes && is_interactive() { let confirm = Confirm::new() .with_prompt(format!( "Update {} '{}' in {}?", @@ -252,7 +310,7 @@ pub async fn run( eprintln!( "Run `bt {} view {}` to inspect the updated definition.", label_plural(ft), - function.slug + common.new_slug.as_deref().unwrap_or(&function.slug) ); } @@ -279,7 +337,7 @@ fn function_matches_filter(function: &api::Function, ft: Option, ) -> Result { let project_id = &ctx.project.id; @@ -305,7 +363,7 @@ async fn resolve_target_function( } } -fn build_patch_body(args: &UpdateArgs) -> Result { +fn build_scorer_patch_body(args: &ScorerUpdateArgs) -> Result { let mut patch = build_scorer_config( &ScorerConfig { messages: args.messages.as_deref(), @@ -316,35 +374,90 @@ fn build_patch_body(args: &UpdateArgs) -> Result { use_cot: args.use_cot, allow_no_match: args.allow_no_match, pass_threshold: args.pass_threshold, - metadata: args.metadata.as_deref(), - metadata_label: "function metadata", + metadata: args.common.metadata.as_deref(), + metadata_label: "scorer metadata", }, false, )?; // PATCH /v1/function does not support changing function_type. Output-mode - // changes are rejected after resolving the current function. + // changes are rejected after resolving the current scorer. patch.remove("function_type"); + merge_common_fields(&mut patch, &args.common, false)?; + finish_patch(patch, "bt scorers update --help") +} + +fn build_tool_patch_body(args: &ToolUpdateArgs) -> Result { + let mut patch = Map::new(); - if let Some(description) = args.description.as_deref() { + if let Some(source) = args.prompt.as_deref() { + let content = read_text_source(source, "prompt")?; patch.insert( - "description".to_string(), - Value::String(description.to_string()), + "prompt_data".to_string(), + serde_json::json!({"prompt": {"type": "completion", "content": content}}), ); + } else if let Some(source) = args.messages.as_deref() { + let raw = read_text_source(source, "messages")?; + let messages: Value = + yaml_serde::from_str(&raw).context("invalid JSON or YAML in --messages")?; + if !messages.is_array() { + bail!("--messages must contain an array"); + } + patch.insert( + "prompt_data".to_string(), + serde_json::json!({"prompt": {"type": "chat", "messages": messages}}), + ); + } + + merge_common_fields(&mut patch, &args.common, true)?; + finish_patch(patch, "bt tools update --help") +} + +fn build_common_patch_body(args: &CommonUpdateArgs) -> Result { + let mut patch = Map::new(); + merge_common_fields(&mut patch, args, true)?; + finish_patch(patch, "bt functions update --help") +} + +fn merge_common_fields( + patch: &mut Map, + args: &CommonUpdateArgs, + include_metadata: bool, +) -> Result<()> { + for (key, value) in [ + ("name", args.name.as_deref()), + ("slug", args.new_slug.as_deref()), + ("description", args.description.as_deref()), + ] { + if let Some(value) = value { + if value.trim().is_empty() && key != "description" { + bail!("--{} cannot be empty", key.replace('_', "-")); + } + patch.insert(key.to_string(), Value::String(value.to_string())); + } } - if let Some(extra_obj) = resolve_extra_patch(args)? { - merge_json_objects(&mut patch, &extra_obj); + if include_metadata { + if let Some(source) = args.metadata.as_deref() { + let metadata = crate::utils::read_yaml_object_source(source, "function metadata")?; + patch.insert("metadata".to_string(), Value::Object(metadata)); + } } - if patch.is_empty() { - bail!("no updates requested. Pass an update flag; see `bt scorers update --help`"); + if let Some(extra_obj) = resolve_extra_patch(args.patch.as_deref())? { + merge_json_objects(patch, &extra_obj); } + Ok(()) +} +fn finish_patch(patch: Map, help_command: &str) -> Result { + if patch.is_empty() { + bail!("no updates requested. Pass an update flag; see `{help_command}`"); + } Ok(Value::Object(patch)) } -fn resolve_extra_patch(args: &UpdateArgs) -> Result>> { - let Some(source) = args.patch.as_deref() else { +fn resolve_extra_patch(source: Option<&str>) -> Result>> { + let Some(source) = source else { return Ok(None); }; let raw = read_text_source(source, "patch")?; @@ -367,18 +480,26 @@ mod tests { use super::*; #[derive(Debug, Parser)] - struct UpdateArgsHarness { + struct ScorerUpdateArgsHarness { #[command(flatten)] - args: UpdateArgs, - } - - fn args(model: Option<&str>, description: Option<&str>) -> UpdateArgs { - UpdateArgs { - slug: super::super::SlugArgs { - slug_positional: Some("test-slug".to_string()), - slug_flag: None, + args: ScorerUpdateArgs, + } + + fn args(model: Option<&str>, description: Option<&str>) -> ScorerUpdateArgs { + ScorerUpdateArgs { + common: CommonUpdateArgs { + slug: super::super::SlugArgs { + slug_positional: Some("test-slug".to_string()), + slug_flag: None, + }, + id: None, + name: None, + new_slug: None, + metadata: None, + description: description.map(ToOwned::to_owned), + patch: None, + yes: true, }, - id: None, messages: None, model: model.map(ToOwned::to_owned), prompt_config: PromptConfigArgs::default(), @@ -387,41 +508,14 @@ mod tests { use_cot: None, allow_no_match: None, pass_threshold: None, - metadata: None, - description: description.map(ToOwned::to_owned), - patch: None, - yes: true, } } - #[test] - fn scorer_output_flags_reported_only_when_set() { - let base = args(None, None); - assert!(base.scorer_output_flags().is_empty()); - - let mut scored = args(None, None); - scored.choice_scores = Some(r#"{"pass":1}"#.to_string()); - scored.pass_threshold = Some(0.5); - assert_eq!( - scored.scorer_output_flags(), - vec!["--choice-scores", "--pass-threshold"] - ); - - let mut labeled = args(None, None); - labeled.classifications = Some(r#"["a"]"#.to_string()); - labeled.allow_no_match = Some(true); - labeled.use_cot = Some(false); - assert_eq!( - labeled.scorer_output_flags(), - vec!["--classifications", "--allow-no-match", "--use-cot"] - ); - } - #[test] fn build_patch_body_messages_writes_chat_block() { let mut args = args(None, None); args.messages = Some(r#"[{"role":"user","content":"hi"}]"#.to_string()); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["prompt"]["type"], serde_json::json!("chat") @@ -435,7 +529,7 @@ mod tests { #[test] fn build_patch_body_model_merges_into_prompt_data() { let args = args(Some("gpt-4o-mini"), None); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["options"]["model"], serde_json::json!("gpt-4o-mini") @@ -446,7 +540,7 @@ mod tests { fn build_patch_body_messages_and_model_combine() { let mut args = args(Some("gpt-4o-mini"), None); args.messages = Some(r#"[{"role":"user","content":"Grade it."}]"#.to_string()); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["prompt"]["messages"], json!([{"role": "user", "content": "Grade it."}]) @@ -459,7 +553,7 @@ mod tests { #[test] fn build_patch_body_updates_all_llm_configuration() { - let parsed = UpdateArgsHarness::try_parse_from([ + let parsed = ScorerUpdateArgsHarness::try_parse_from([ "test", "test-scorer", "--model", @@ -488,7 +582,7 @@ mod tests { ]) .expect("parse update"); - let body = build_patch_body(&parsed.args).expect("patch body"); + let body = build_scorer_patch_body(&parsed.args).expect("patch body"); let params = &body["prompt_data"]["options"]["params"]; assert_eq!(body["prompt_data"]["options"]["model"], "gpt-test"); assert_eq!(params["temperature"], 0.2); @@ -512,9 +606,9 @@ mod tests { let mut args = args(None, None); args.classifications = Some(r#"["safe","unsafe"]"#.to_string()); args.allow_no_match = Some(true); - args.metadata = Some("owner: test-team".to_string()); + args.common.metadata = Some("owner: test-team".to_string()); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert!(body.get("function_type").is_none()); assert_eq!( body["prompt_data"]["parser"]["choice"], @@ -530,7 +624,7 @@ mod tests { args.choice_scores = Some(r#"{"pass":1,"fail":0}"#.to_string()); args.pass_threshold = Some(0.8); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert!(body.get("function_type").is_none()); assert_eq!( body["prompt_data"]["parser"]["choice_scores"], @@ -542,10 +636,52 @@ mod tests { #[test] fn build_patch_body_description_is_top_level() { let args = args(None, Some("Helpfulness judge")); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!(body["description"], serde_json::json!("Helpfulness judge")); } + #[test] + fn common_fields_update_name_and_slug() { + let mut args = args(None, None); + args.common.name = Some("Updated scorer".to_string()); + args.common.new_slug = Some("updated-scorer".to_string()); + + let body = build_scorer_patch_body(&args).expect("patch body"); + assert_eq!(body["name"], "Updated scorer"); + assert_eq!(body["slug"], "updated-scorer"); + } + + #[test] + fn tool_prompt_builds_completion_prompt_patch() { + let scorer = args(None, None); + let args = ToolUpdateArgs { + common: scorer.common, + prompt: Some("Look up {{order_id}}".to_string()), + messages: None, + }; + + let body = build_tool_patch_body(&args).expect("patch body"); + assert_eq!(body["prompt_data"]["prompt"]["type"], "completion"); + assert_eq!( + body["prompt_data"]["prompt"]["content"], + "Look up {{order_id}}" + ); + } + + #[test] + fn tool_messages_accept_yaml() { + let scorer = args(None, None); + let args = ToolUpdateArgs { + common: scorer.common, + prompt: None, + messages: Some("- role: user\n content: Look up {{order_id}}\n".to_string()), + }; + + let body = build_tool_patch_body(&args).expect("patch body"); + assert_eq!(body["prompt_data"]["prompt"]["type"], "chat"); + assert_eq!(body["prompt_data"]["prompt"]["messages"][0]["role"], "user"); + } + #[test] fn build_patch_body_reads_at_prefixed_messages_file() { let dir = tempfile::tempdir().expect("tempdir"); @@ -556,7 +692,7 @@ mod tests { let mut args = args(None, None); args.messages = Some(source); - let body = build_patch_body(&args).expect("patch body"); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["prompt"]["messages"], json!([{"role": "user", "content": "Grade from a file."}]) @@ -571,23 +707,23 @@ mod tests { let source = format!("@{}", path.display()); let mut args = args(None, None); - args.patch = Some(source); - let body = build_patch_body(&args).expect("patch body"); + args.common.patch = Some(source); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!(body["description"], "From a file"); } #[test] fn build_patch_body_rejects_empty_update() { let args = args(None, None); - let err = build_patch_body(&args).expect_err("should reject empty"); + let err = build_scorer_patch_body(&args).expect_err("should reject empty"); assert!(err.to_string().contains("no updates requested")); } #[test] fn build_patch_body_extra_patch_merges_into_prompt_data() { let mut args = args(None, None); - args.patch = Some(r#"{"prompt_data":{"parser":{"type":"llm_classifier","use_cot":true,"choice_scores":{"A":1.0,"B":0.0}}}}"#.to_string()); - let body = build_patch_body(&args).expect("patch body"); + args.common.patch = Some(r#"{"prompt_data":{"parser":{"type":"llm_classifier","use_cot":true,"choice_scores":{"A":1.0,"B":0.0}}}}"#.to_string()); + let body = build_scorer_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["parser"]["choice_scores"], serde_json::json!({"A": 1.0, "B": 0.0}) diff --git a/src/scorers.rs b/src/scorers.rs index e9b6a167..45ab7127 100644 --- a/src/scorers.rs +++ b/src/scorers.rs @@ -2,7 +2,7 @@ use anyhow::Result; use clap::{Args, Subcommand}; use crate::args::BaseArgs; -use crate::functions::{self, FunctionCommands, FunctionTypeFilter}; +use crate::functions::{self, ScorerFunctionCommands}; #[derive(Debug, Clone, Args)] #[command(after_help = "\ @@ -30,16 +30,16 @@ enum ScorersCommands { /// Create an LLM scorer or classifier Create(Box), #[command(flatten)] - Function(FunctionCommands), + Function(ScorerFunctionCommands), } pub async fn run(base: BaseArgs, args: ScorersArgs) -> Result<()> { match args.command { Some(ScorersCommands::Create(create)) => functions::run_scorer_create(base, *create).await, Some(ScorersCommands::Function(command)) => { - functions::run_typed_command(base, Some(command), FunctionTypeFilter::Scorer).await + functions::run_scorer_command(base, Some(command)).await } - None => functions::run_typed_command(base, None, FunctionTypeFilter::Scorer).await, + None => functions::run_scorer_command(base, None).await, } } @@ -115,7 +115,7 @@ mod tests { assert!(matches!( parsed.args.command, - Some(ScorersCommands::Function(FunctionCommands::Update(_))) + Some(ScorersCommands::Function(ScorerFunctionCommands::Update(_))) )); } } diff --git a/tests/functions.rs b/tests/functions.rs index 0658f707..69b28aa6 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -561,6 +561,39 @@ fn functions_push_help_includes_expected_flags() { assert!(stdout.contains("--external-packages")); } +#[test] +fn update_help_is_specific_to_each_function_kind() { + let help = |resource: &str| { + let output = Command::new(bt_binary_path()) + .args([resource, "update", "--help"]) + .output() + .unwrap_or_else(|error| panic!("run bt {resource} update --help: {error}")); + assert!(output.status.success()); + String::from_utf8(output.stdout).expect("UTF-8 help output") + }; + + let tools = help("tools"); + assert!(tools.contains("--name")); + assert!(tools.contains("--new-slug")); + assert!(tools.contains("--prompt")); + assert!(tools.contains("--messages")); + assert!(!tools.contains("--choice-scores")); + assert!(!tools.contains("--pass-threshold")); + + let functions = help("functions"); + assert!(functions.contains("--name")); + assert!(functions.contains("--new-slug")); + assert!(functions.contains("--description")); + assert!(functions.contains("--patch")); + assert!(!functions.contains("--model")); + assert!(!functions.contains("--choice-scores")); + + let scorers = help("scorers"); + assert!(scorers.contains("--new-slug")); + assert!(scorers.contains("--model")); + assert!(scorers.contains("--choice-scores")); +} + #[test] fn functions_pull_help_includes_expected_flags() { let output = Command::new(bt_binary_path()) From 3a5cccd6b290bdb600f3e21d42bfc8dd6881e6dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Fri, 21 Aug 2026 16:05:27 -0700 Subject: [PATCH 8/8] fix: the llm supposedly fixed things MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Metadata updates replaced existing metadata, including scorer pass thresholds. 2. Switching completion ↔ chat prompts produced invalid mixed prompt objects. 3. Inline YAML and prompts beginning with - failed Clap parsing. 4. Empty --new-slug incorrectly reported --slug. 5. Slug collisions leaked a raw backend uniqueness error; now preflighted with an actionable message. 6. Expected update errors were classified as internal errors and printed bug-report guidance. 7. Code-backed prompt-update errors now direct users to bt functions push. --- src/functions/prompt_patch.rs | 41 ++++++++++ src/functions/update.rs | 150 ++++++++++++++++++++++++++++------ 2 files changed, 167 insertions(+), 24 deletions(-) diff --git a/src/functions/prompt_patch.rs b/src/functions/prompt_patch.rs index fded4fca..0b7775d2 100644 --- a/src/functions/prompt_patch.rs +++ b/src/functions/prompt_patch.rs @@ -21,6 +21,20 @@ pub(crate) fn materialize_prompt_data_patch( .unwrap_or_default(); merge_json_objects(&mut merged, &patch_prompt_data); + // Prompt blocks are a discriminated union. Deep-merging a completion block + // into a chat block (or vice versa) leaves fields from both variants and + // produces invalid prompt data. + if let Some(requested_prompt) = patch_prompt_data.get("prompt") { + let requested_type = requested_prompt.get("type").and_then(Value::as_str); + let existing_type = existing_prompt_data + .and_then(|data| data.get("prompt")) + .and_then(|prompt| prompt.get("type")) + .and_then(Value::as_str); + if requested_type.is_some() && requested_type != existing_type { + merged.insert("prompt".to_string(), requested_prompt.clone()); + } + } + if let (Some(requested), Some(parser)) = ( patch_prompt_data.get("parser").and_then(Value::as_object), merged.get_mut("parser").and_then(Value::as_object_mut), @@ -43,6 +57,33 @@ mod tests { use super::*; + #[test] + fn replaces_prompt_block_when_switching_prompt_kinds() { + let existing = json!({ + "prompt": {"type": "completion", "content": "Original"}, + "options": {"model": "test-model"} + }); + let mut patch = json!({ + "prompt_data": { + "prompt": { + "type": "chat", + "messages": [{"role": "user", "content": "Hello"}] + } + } + }); + + materialize_prompt_data_patch(&mut patch, Some(&existing)); + + assert_eq!( + patch["prompt_data"]["prompt"], + json!({ + "type": "chat", + "messages": [{"role": "user", "content": "Hello"}] + }) + ); + assert_eq!(patch["prompt_data"]["options"]["model"], "test-model"); + } + #[test] fn materializes_complete_prompt_data_for_patch() { let existing = json!({ diff --git a/src/functions/update.rs b/src/functions/update.rs index 870e1d80..35eb4c07 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -4,6 +4,7 @@ use dialoguer::Confirm; use serde_json::{Map, Value}; use crate::{ + error::user_error, ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, utils::{merge_json_objects, read_text_source}, }; @@ -128,11 +129,21 @@ pub(crate) struct ToolUpdateArgs { common: CommonUpdateArgs, /// Replace a completion prompt from inline text, @PATH, or stdin (-). - #[arg(long, value_name = "SOURCE", conflicts_with = "messages")] + #[arg( + long, + value_name = "SOURCE", + conflicts_with = "messages", + allow_hyphen_values = true + )] prompt: Option, /// Replace chat messages from inline JSON/YAML, @PATH, or stdin (-). - #[arg(long, value_name = "SOURCE", conflicts_with = "prompt")] + #[arg( + long, + value_name = "SOURCE", + conflicts_with = "prompt", + allow_hyphen_values = true + )] messages: Option, } @@ -179,7 +190,7 @@ pub(crate) async fn run_scorer( args: &ScorerUpdateArgs, json_output: bool, ) -> Result<()> { - let body = build_scorer_patch_body(args)?; + let body = build_scorer_patch_body(args).map_err(user_error)?; run_update( ctx, &args.common, @@ -196,7 +207,7 @@ pub(crate) async fn run_tool( args: &ToolUpdateArgs, json_output: bool, ) -> Result<()> { - let body = build_tool_patch_body(args)?; + let body = build_tool_patch_body(args).map_err(user_error)?; run_update( ctx, &args.common, @@ -214,7 +225,7 @@ pub(crate) async fn run_generic( json_output: bool, ft: Option, ) -> Result<()> { - let body = build_common_patch_body(&args.common)?; + let body = build_common_patch_body(&args.common).map_err(user_error)?; run_update(ctx, &args.common, body, json_output, ft, None).await } @@ -228,7 +239,27 @@ async fn run_update( ) -> Result<()> { let function = resolve_target_function(ctx, common, ft).await?; if !function_matches_filter(&function, ft) { - bail!("'{}' is not a {}", function.name, label(ft)); + return Err(user_error(anyhow!( + "'{}' is not a {}", + function.name, + label(ft) + ))); + } + + if let Some(new_slug) = common.new_slug.as_deref() { + if new_slug != function.slug { + if let Some(conflict) = + api::get_function_by_slug(&ctx.client, &ctx.project.id, new_slug, None).await? + { + if conflict.id != function.id { + return Err(user_error(anyhow!( + "--new-slug '{new_slug}' is already used by {} '{}'", + conflict.function_type.as_deref().unwrap_or("function"), + conflict.name + ))); + } + } + } } let implementation = function @@ -238,11 +269,12 @@ async fn run_update( .and_then(Value::as_str) .unwrap_or("prompt"); if body.get("prompt_data").is_some() && implementation != "prompt" { - bail!( - "prompt configuration cannot update {}-backed function '{}'", + return Err(user_error(anyhow!( + "prompt configuration cannot update {}-backed {} '{}'. Edit its source and run: bt functions push --if-exists replace", implementation, + label(ft), function.name - ); + ))); } if let Some(args) = scorer_args { @@ -255,17 +287,24 @@ async fn run_update( ), (Some("classifier"), true, _) | (Some("scorer"), _, true) ) { - bail!("PATCH cannot change between score and classification output"); + return Err(user_error(anyhow!( + "cannot change between score and classification output" + ))); } if args.allow_no_match.is_some() && function_type != Some("classifier") { - bail!("--allow-no-match applies only to classification output"); + return Err(user_error(anyhow!( + "--allow-no-match applies only to classification output" + ))); } if args.pass_threshold.is_some() && function_type == Some("classifier") { - bail!("--pass-threshold applies only to score output"); + return Err(user_error(anyhow!( + "--pass-threshold applies only to score output" + ))); } } materialize_prompt_data_patch(&mut body, function.prompt_data.as_ref()); + materialize_metadata_patch(&mut body, function.metadata.as_ref()); if !common.yes && is_interactive() { let confirm = Confirm::new() @@ -423,14 +462,14 @@ fn merge_common_fields( args: &CommonUpdateArgs, include_metadata: bool, ) -> Result<()> { - for (key, value) in [ - ("name", args.name.as_deref()), - ("slug", args.new_slug.as_deref()), - ("description", args.description.as_deref()), + for (key, flag, value) in [ + ("name", "--name", args.name.as_deref()), + ("slug", "--new-slug", args.new_slug.as_deref()), + ("description", "--description", args.description.as_deref()), ] { if let Some(value) = value { if value.trim().is_empty() && key != "description" { - bail!("--{} cannot be empty", key.replace('_', "-")); + bail!("{flag} cannot be empty"); } patch.insert(key.to_string(), Value::String(value.to_string())); } @@ -449,6 +488,19 @@ fn merge_common_fields( Ok(()) } +fn materialize_metadata_patch(patch: &mut Value, existing_metadata: Option<&Value>) { + let Some(patch_metadata) = patch.get("metadata").and_then(Value::as_object).cloned() else { + return; + }; + + let mut metadata = existing_metadata + .and_then(Value::as_object) + .cloned() + .unwrap_or_default(); + merge_json_objects(&mut metadata, &patch_metadata); + patch["metadata"] = Value::Object(metadata); +} + fn finish_patch(patch: Map, help_command: &str) -> Result { if patch.is_empty() { bail!("no updates requested. Pass an update flag; see `{help_command}`"); @@ -485,6 +537,12 @@ mod tests { args: ScorerUpdateArgs, } + #[derive(Debug, Parser)] + struct ToolUpdateArgsHarness { + #[command(flatten)] + args: ToolUpdateArgs, + } + fn args(model: Option<&str>, description: Option<&str>) -> ScorerUpdateArgs { ScorerUpdateArgs { common: CommonUpdateArgs { @@ -668,16 +726,34 @@ mod tests { ); } + #[test] + fn tool_prompt_accepts_text_beginning_with_a_dash() { + let parsed = ToolUpdateArgsHarness::try_parse_from([ + "test", + "test-tool", + "--prompt", + "- instruction {{value}}", + ]) + .expect("parse prompt beginning with a dash"); + + let body = build_tool_patch_body(&parsed.args).expect("patch body"); + assert_eq!( + body["prompt_data"]["prompt"]["content"], + "- instruction {{value}}" + ); + } + #[test] fn tool_messages_accept_yaml() { - let scorer = args(None, None); - let args = ToolUpdateArgs { - common: scorer.common, - prompt: None, - messages: Some("- role: user\n content: Look up {{order_id}}\n".to_string()), - }; + let parsed = ToolUpdateArgsHarness::try_parse_from([ + "test", + "test-tool", + "--messages", + "- role: user\n content: Look up {{order_id}}\n", + ]) + .expect("parse inline YAML beginning with a dash"); - let body = build_tool_patch_body(&args).expect("patch body"); + let body = build_tool_patch_body(&parsed.args).expect("patch body"); assert_eq!(body["prompt_data"]["prompt"]["type"], "chat"); assert_eq!(body["prompt_data"]["prompt"]["messages"][0]["role"], "user"); } @@ -762,4 +838,30 @@ mod tests { serde_json::json!(0) ); } + + #[test] + fn metadata_patch_preserves_existing_fields() { + let mut patch = json!({"metadata": {"owner": "test-team"}}); + let existing = json!({"__pass_threshold": 0.6, "phase": "test"}); + + materialize_metadata_patch(&mut patch, Some(&existing)); + + assert_eq!( + patch["metadata"], + json!({ + "__pass_threshold": 0.6, + "phase": "test", + "owner": "test-team" + }) + ); + } + + #[test] + fn empty_new_slug_names_the_correct_flag() { + let mut args = args(None, None); + args.common.new_slug = Some(String::new()); + + let error = build_scorer_patch_body(&args).expect_err("empty slug should fail"); + assert_eq!(error.to_string(), "--new-slug cannot be empty"); + } }