From c5c59036c32b3918b9b91c60730b61599bd0d24b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Wed, 22 Jul 2026 14:17:53 -0700 Subject: [PATCH 1/7] draft --- src/functions/api.rs | 14 ++ src/functions/create.rs | 459 ++++++++++++++++++++++++++++++++++ src/functions/mod.rs | 70 +++++- src/functions/update.rs | 535 ++++++++++++++++++++++++++++++++++++++++ src/prompts/api.rs | 11 + src/prompts/mod.rs | 28 +++ src/prompts/update.rs | 491 ++++++++++++++++++++++++++++++++++++ src/scorers.rs | 89 ++++++- tests/cli.rs | 30 +++ 9 files changed, 1720 insertions(+), 7 deletions(-) create mode 100644 src/functions/create.rs create mode 100644 src/functions/update.rs create mode 100644 src/prompts/update.rs diff --git a/src/functions/api.rs b/src/functions/api.rs index 57f8829c..0fd06bb2 100644 --- a/src/functions/api.rs +++ b/src/functions/api.rs @@ -144,6 +144,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. +/// +/// The Braintrust API deep-merges object fields, so callers can send only the +/// nested fields they want to change (for example `prompt_data.prompt`) without +/// sending the complete function definition. +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 new file mode 100644 index 00000000..bc57f342 --- /dev/null +++ b/src/functions/create.rs @@ -0,0 +1,459 @@ +use std::{io::Read, path::PathBuf}; + +use anyhow::{bail, Context, Result}; +use clap::{builder::BoolishValueParser, Args}; +use dialoguer::Input; +use serde_json::{json, Map, Value}; + +use crate::ui::{is_interactive, print_command_status, with_spinner, CommandStatus}; + +use super::{api, IfExistsMode, ResolvedContext}; + +/// Create an LLM scorer. +/// +/// The generated definition matches `project.scorers.create(...)`: a prompt +/// function with an `llm_classifier` parser, model, chain-of-thought setting, +/// and numeric score for each possible choice. +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt scorers create \"Helpfulness\" --model gpt-4o-mini --prompt-file judge.md \\ + --choice-scores '{\"A\":1,\"B\":0}' --use-cot + bt scorers create \"Correctness\" --slug correctness --model gpt-4o-mini \\ + --prompt \"Score {{output}} against {{expected}}\" \\ + --choice-scores '{\"correct\":1,\"incorrect\":0}' --use-cot=false + bt scorers create \"Tone\" --model gpt-4o-mini \\ + --messages '[{\"role\":\"user\",\"content\":\"Judge {{output}}\"}]' \\ + --choice-scores-file scores.json --use-cot +")] +pub(crate) struct CreateArgs { + /// Scorer name. + #[arg(value_name = "NAME", conflicts_with = "name")] + name_positional: Option, + + /// Scorer name (alternative to the positional name). + #[arg(long, env = "BT_SCORERS_CREATE_NAME", value_name = "NAME")] + name: Option, + + /// Unique scorer slug. Defaults to a slug generated from the name. + #[arg(long, short = 's', env = "BT_SCORERS_CREATE_SLUG")] + slug: Option, + + /// Scorer description. + #[arg(long, short = 'd', env = "BT_SCORERS_CREATE_DESCRIPTION")] + description: Option, + + /// Completion prompt text. Use --prompt-file for a file, or pipe the + /// prompt through stdin when no prompt option is supplied. + #[arg( + long, + env = "BT_SCORERS_CREATE_PROMPT", + value_name = "TEXT", + conflicts_with_all = ["prompt_file", "messages", "messages_file"] + )] + prompt: Option, + + /// Read the completion prompt text from a file. + #[arg( + long, + env = "BT_SCORERS_CREATE_PROMPT_FILE", + value_name = "PATH", + conflicts_with_all = ["prompt", "messages", "messages_file"] + )] + prompt_file: Option, + + /// Chat prompt messages as a JSON array. + #[arg( + long, + env = "BT_SCORERS_CREATE_MESSAGES", + value_name = "JSON", + conflicts_with_all = ["prompt", "prompt_file", "messages_file"] + )] + messages: Option, + + /// Read chat prompt messages as a JSON array from a file. + #[arg( + long, + env = "BT_SCORERS_CREATE_MESSAGES_FILE", + value_name = "PATH", + conflicts_with_all = ["prompt", "prompt_file", "messages"] + )] + messages_file: Option, + + /// Model used by the LLM judge. + #[arg( + long, + short = 'm', + env = "BT_SCORERS_CREATE_MODEL", + value_name = "MODEL" + )] + model: String, + + /// JSON object mapping each classifier choice to a numeric score. + #[arg( + long, + env = "BT_SCORERS_CREATE_CHOICE_SCORES", + value_name = "JSON", + required_unless_present = "choice_scores_file", + conflicts_with = "choice_scores_file" + )] + choice_scores: Option, + + /// Read the choice-to-score JSON object from a file. + #[arg( + long, + env = "BT_SCORERS_CREATE_CHOICE_SCORES_FILE", + value_name = "PATH", + conflicts_with = "choice_scores" + )] + choice_scores_file: Option, + + /// Whether the scorer should use chain-of-thought reasoning. This option + /// is required; pass --use-cot or --use-cot=false. + #[arg( + long, + env = "BT_SCORERS_CREATE_USE_COT", + num_args = 0..=1, + default_missing_value = "true", + required = true, + value_parser = BoolishValueParser::new() + )] + use_cot: Option, + + /// Behavior when a scorer with the same slug already exists. + #[arg( + long, + env = "BT_SCORERS_CREATE_IF_EXISTS", + value_enum, + default_value = "error" + )] + if_exists: IfExistsMode, +} + +pub(crate) async fn run(ctx: &ResolvedContext, args: &CreateArgs, json_output: bool) -> Result<()> { + let name = resolve_name(args)?; + let slug = resolve_slug(args, &name)?; + let definition = build_scorer_definition(args, &ctx.project.id, &name, &slug)?; + + let result = match with_spinner( + "Creating scorer...", + api::insert_functions(&ctx.client, std::slice::from_ref(&definition)), + ) + .await + { + Ok(result) => result, + Err(error) => { + print_command_status(CommandStatus::Error, &format!("Failed to create '{name}'")); + return Err(error); + } + }; + + let ignored = result.ignored_entries.is_some_and(|count| count > 0); + + if json_output { + println!( + "{}", + serde_json::to_string(&json!({ + "scorer": definition, + "ignored": ignored, + }))? + ); + return Ok(()); + } + + if ignored { + print_command_status( + CommandStatus::Warning, + &format!("Scorer '{name}' already exists; left it unchanged"), + ); + } else if args.if_exists == IfExistsMode::Replace { + print_command_status(CommandStatus::Success, &format!("Saved '{name}'")); + } else { + print_command_status(CommandStatus::Success, &format!("Created '{name}'")); + } + + Ok(()) +} + +fn resolve_name(args: &CreateArgs) -> Result { + let name = match (&args.name_positional, &args.name) { + (Some(_), Some(_)) => bail!("use either a positional name or --name, not both"), + (Some(name), None) | (None, Some(name)) => name.trim().to_string(), + (None, None) if is_interactive() => Input::::new() + .with_prompt("Scorer name") + .interact_text()? + .trim() + .to_string(), + (None, None) => bail!("scorer name required. Use: bt scorers create ..."), + }; + + if name.is_empty() { + bail!("scorer name cannot be empty"); + } + Ok(name) +} + +fn resolve_slug(args: &CreateArgs, name: &str) -> Result { + let slug = args + .slug + .as_deref() + .map(str::trim) + .map(ToOwned::to_owned) + .unwrap_or_else(|| slugify(name)); + if slug.is_empty() { + bail!("could not generate a slug from the scorer name; pass --slug explicitly"); + } + Ok(slug) +} + +fn slugify(value: &str) -> String { + let mut slug = String::new(); + let mut pending_separator = false; + + for character in value.trim().chars() { + if character.is_alphanumeric() { + if pending_separator && !slug.is_empty() { + slug.push('-'); + } + slug.extend(character.to_lowercase()); + pending_separator = false; + } else if !slug.is_empty() { + pending_separator = true; + } + } + + slug +} + +fn build_scorer_definition( + args: &CreateArgs, + project_id: &str, + name: &str, + slug: &str, +) -> Result { + if args.model.trim().is_empty() { + bail!("--model cannot be empty"); + } + let use_cot = args.use_cot.ok_or_else(|| { + anyhow::anyhow!("--use-cot is required; pass --use-cot or --use-cot=false") + })?; + let prompt = resolve_prompt_block(args)?; + let choice_scores = resolve_choice_scores(args)?; + + let mut definition = json!({ + "project_id": project_id, + "name": name, + "slug": slug, + "function_data": { + "type": "prompt", + }, + "prompt_data": { + "prompt": prompt, + "options": { + "model": args.model, + }, + "parser": { + "type": "llm_classifier", + "use_cot": use_cot, + "choice_scores": choice_scores, + }, + }, + "if_exists": args.if_exists.as_str(), + "function_type": "scorer", + }); + + if let Some(description) = args.description.as_deref() { + definition["description"] = Value::String(description.to_string()); + } + + Ok(definition) +} + +fn resolve_prompt_block(args: &CreateArgs) -> Result { + let selected = usize::from(args.prompt.is_some()) + + usize::from(args.prompt_file.is_some()) + + usize::from(args.messages.is_some()) + + usize::from(args.messages_file.is_some()); + if selected > 1 { + bail!("use only one of --prompt, --prompt-file, --messages, or --messages-file"); + } + + if let Some(prompt) = args.prompt.as_deref() { + return Ok(json!({ "type": "completion", "content": prompt })); + } + if let Some(path) = args.prompt_file.as_deref() { + let prompt = std::fs::read_to_string(path) + .with_context(|| format!("failed to read prompt file {}", path.display()))?; + return Ok(json!({ "type": "completion", "content": prompt })); + } + if let Some(raw) = args.messages.as_deref() { + return parse_messages(raw); + } + if let Some(path) = args.messages_file.as_deref() { + let raw = std::fs::read_to_string(path) + .with_context(|| format!("failed to read messages file {}", path.display()))?; + return parse_messages(&raw); + } + + if is_interactive() { + bail!("scorer prompt required. Pass --prompt, --prompt-file, or --messages"); + } + + let mut prompt = String::new(); + std::io::stdin() + .read_to_string(&mut prompt) + .context("failed to read prompt from stdin")?; + if prompt.is_empty() { + bail!( + "scorer prompt required. Pass --prompt, --prompt-file, or --messages, or pipe prompt text through stdin" + ); + } + Ok(json!({ "type": "completion", "content": prompt })) +} + +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"), + } +} + +fn resolve_choice_scores(args: &CreateArgs) -> Result { + let raw = match (&args.choice_scores, &args.choice_scores_file) { + (Some(_), Some(_)) => { + bail!("use either --choice-scores or --choice-scores-file, not both") + } + (Some(raw), None) => raw.clone(), + (None, Some(path)) => std::fs::read_to_string(path) + .with_context(|| format!("failed to read choice scores file {}", path.display()))?, + (None, None) => bail!( + "--choice-scores is required; pass a JSON object such as '{{\"yes\":1,\"no\":0}}'" + ), + }; + + let value: Value = serde_json::from_str(&raw).context("invalid JSON in choice scores")?; + let scores = match value { + Value::Object(scores) => scores, + _ => bail!("choice scores must be a JSON object mapping choices to numeric scores"), + }; + validate_choice_scores(&scores)?; + Ok(Value::Object(scores)) +} + +fn validate_choice_scores(scores: &Map) -> Result<()> { + if scores.is_empty() { + bail!("choice scores cannot be empty"); + } + for (choice, score) in scores { + if !score.is_number() { + bail!("score for choice '{choice}' must be a number"); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn args() -> CreateArgs { + CreateArgs { + name_positional: Some("Test Helpfulness".to_string()), + name: None, + slug: None, + description: Some("Synthetic test scorer".to_string()), + prompt: Some("Judge {{output}}.".to_string()), + prompt_file: None, + messages: None, + messages_file: None, + model: "gpt-test".to_string(), + choice_scores: Some(r#"{"A":1,"B":0}"#.to_string()), + choice_scores_file: None, + use_cot: Some(true), + if_exists: IfExistsMode::Error, + } + } + + #[test] + fn builds_sdk_compatible_llm_scorer_definition() { + let args = args(); + let body = build_scorer_definition( + &args, + "00000000-0000-0000-0000-000000000001", + "Test Helpfulness", + "test-helpfulness", + ) + .expect("definition"); + + assert_eq!(body["function_data"], json!({ "type": "prompt" })); + assert_eq!(body["function_type"], "scorer"); + assert_eq!(body["prompt_data"]["prompt"]["type"], "completion"); + assert_eq!(body["prompt_data"]["options"]["model"], "gpt-test"); + assert_eq!( + body["prompt_data"]["parser"], + json!({ + "type": "llm_classifier", + "use_cot": true, + "choice_scores": { "A": 1, "B": 0 }, + }) + ); + assert_eq!(body["if_exists"], "error"); + assert_eq!(body["description"], "Synthetic test scorer"); + } + + #[test] + fn builds_chat_prompt_definition() { + let mut args = args(); + args.prompt = None; + args.messages = Some(r#"[{"role":"user","content":"Judge {{output}}"}]"#.to_string()); + + let body = + build_scorer_definition(&args, "test-project", "Test", "test").expect("definition"); + + assert_eq!(body["prompt_data"]["prompt"]["type"], "chat"); + assert_eq!( + body["prompt_data"]["prompt"]["messages"], + json!([{ "role": "user", "content": "Judge {{output}}" }]) + ); + } + + #[test] + fn rejects_multiple_prompt_sources() { + let mut args = args(); + args.messages = Some("[]".to_string()); + + let error = build_scorer_definition(&args, "test-project", "Test", "test") + .expect_err("prompt sources should conflict"); + assert!(error.to_string().contains("use only one")); + } + + #[test] + fn rejects_non_numeric_choice_score() { + let mut args = args(); + args.choice_scores = Some(r#"{"A":"one"}"#.to_string()); + + let error = build_scorer_definition(&args, "test-project", "Test", "test") + .expect_err("string score should fail"); + assert!(error.to_string().contains("must be a number")); + } + + #[test] + fn requires_explicit_use_cot() { + let mut args = args(); + args.use_cot = None; + + let error = build_scorer_definition(&args, "test-project", "Test", "test") + .expect_err("missing use-cot should fail"); + assert!(error.to_string().contains("--use-cot is required")); + } + + #[test] + fn slugify_normalizes_name() { + assert_eq!( + slugify(" Test Helpfulness / Judge "), + "test-helpfulness-judge" + ); + assert_eq!(slugify("Already--Separated"), "already-separated"); + } +} diff --git a/src/functions/mod.rs b/src/functions/mod.rs index d055c231..e5d98623 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -13,12 +13,14 @@ use crate::{ }; pub(crate) mod api; +pub(crate) mod create; mod delete; mod invoke; mod list; mod pull; mod push; pub(crate) mod report; +mod update; mod view; use api::Function; @@ -168,8 +170,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-file tool-patch.json ")] pub struct FunctionArgs { #[command(subcommand)] @@ -177,7 +178,7 @@ pub struct FunctionArgs { } #[derive(Debug, Clone, Subcommand)] -enum FunctionCommands { +pub(crate) enum FunctionCommands { /// List all in the current project List, /// View a function's details @@ -186,6 +187,8 @@ enum FunctionCommands { Delete(DeleteArgs), /// Invoke a function Invoke(invoke::InvokeArgs), + /// Update a function in place (prompt, model, description, or arbitrary patch) + Update(update::UpdateArgs), } #[derive(Debug, Clone, Args)] @@ -218,6 +221,8 @@ enum FunctionsCommands { Delete(FunctionsDeleteArgs), /// Invoke a function Invoke(FunctionsInvokeArgs), + /// Update a function in place (prompt, model, description, or arbitrary patch) + Update(FunctionsUpdateArgs), /// Push local function definitions Push(PushArgs), /// Pull remote function definitions @@ -267,6 +272,20 @@ 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', + env = "BT_FUNCTIONS_UPDATE_TYPE", + value_enum + )] + function_type: Option, +} + #[derive(Debug, Clone, Args)] pub(crate) struct PushArgs { /// File or directory path(s) to scan for function definitions. @@ -608,8 +627,16 @@ pub(crate) async fn select_function_interactive( } pub async fn run_typed(base: BaseArgs, args: FunctionArgs, kind: FunctionTypeFilter) -> Result<()> { + run_typed_command(base, args.command, kind).await +} + +pub(crate) async fn run_typed_command( + base: BaseArgs, + command: Option, + kind: FunctionTypeFilter, +) -> Result<()> { let ft = Some(kind); - match args.command { + match command { Some(FunctionCommands::View(v)) => match v.selector()? { ViewSelector::Id(id) => { let auth_ctx = resolve_auth_context(&base).await?; @@ -644,6 +671,7 @@ pub async fn run_typed(base: BaseArgs, args: FunctionArgs, kind: FunctionTypeFil 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") } @@ -652,6 +680,12 @@ pub async fn run_typed(base: BaseArgs, args: FunctionArgs, kind: FunctionTypeFil } } +pub(crate) async fn run_scorer_create(base: BaseArgs, args: create::CreateArgs) -> Result<()> { + let json_output = base.json; + let ctx = resolve_context(&base).await?; + create::run(&ctx, &args, json_output).await +} + pub async fn run(base: BaseArgs, args: FunctionsArgs) -> Result<()> { let function_type = args.function_type; match args.command { @@ -701,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(_)) => { @@ -1162,6 +1199,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/update.rs b/src/functions/update.rs new file mode 100644 index 00000000..5215476d --- /dev/null +++ b/src/functions/update.rs @@ -0,0 +1,535 @@ +use std::path::PathBuf; + +use anyhow::{anyhow, bail, Context, Result}; +use clap::Args; +use dialoguer::Confirm; +use serde_json::{json, Map, Value}; + +use crate::ui::{is_interactive, print_command_status, with_spinner, CommandStatus}; + +use super::{api, label, label_plural, select_function_interactive}; +use super::{FunctionTypeFilter, ResolvedContext}; + +/// Update a function (scorer, tool, prompt, ...) in place. +/// +/// This wraps `PATCH /v1/function/{id}`. The Braintrust API deep-merges object +/// fields, so you can send just the nested fields you want to change (for +/// example `prompt_data.prompt` for an LLM scorer) without re-authoring the +/// whole definition. +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt scorers update my-scorer --prompt-file judge.md + bt scorers update my-scorer --model gpt-4o-mini + bt scorers update my-scorer --description \"Helpfulness judge\" + bt scorers update my-scorer --patch '{\"prompt_data\":{\"options\":{\"model\":\"gpt-4o-mini\"}}}' + bt scorers update --id fn_123 --patch-file scorer-patch.json + bt tools update my-tool --patch-file 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", env = "BT_FUNCTIONS_UPDATE_ID")] + id: Option, + + /// Replace the completion prompt text (LLM scorers/prompts). Writes + /// `prompt_data.prompt` as `{"type":"completion","content":}`. + /// Read from a file with --prompt-file, or stdin when no value is given + /// in a non-interactive shell. + #[arg( + long, + env = "BT_FUNCTIONS_UPDATE_PROMPT", + value_name = "TEXT", + conflicts_with_all = ["prompt_file", "messages"] + )] + prompt: Option, + + /// Read the completion prompt text from a file. Mutually exclusive with --prompt. + #[arg( + long, + env = "BT_FUNCTIONS_UPDATE_PROMPT_FILE", + value_name = "PATH", + conflicts_with_all = ["prompt", "messages"] + )] + prompt_file: Option, + + /// Replace the chat prompt messages (LLM scorers/prompts) as JSON. Writes + /// `prompt_data.prompt` as `{"type":"chat","messages":}`. + #[arg( + long, + env = "BT_FUNCTIONS_UPDATE_MESSAGES", + value_name = "JSON", + conflicts_with_all = ["prompt", "prompt_file"] + )] + messages: Option, + + /// Update the model used by an LLM scorer/prompt. Writes + /// `prompt_data.options.model`. + #[arg( + long, + short = 'm', + env = "BT_FUNCTIONS_UPDATE_MODEL", + value_name = "MODEL" + )] + model: Option, + + /// Update the function description. + #[arg( + long, + short = 'd', + env = "BT_FUNCTIONS_UPDATE_DESCRIPTION", + value_name = "TEXT" + )] + description: Option, + + /// Arbitrary JSON object deep-merged into the function on patch. Use this + /// for fields without a dedicated flag (for example + /// `prompt_data.parser.choice_scores`, `prompt_data.parser.use_cot`, + /// `function_data`, `tags`, or `metadata`). + #[arg( + long, + env = "BT_FUNCTIONS_UPDATE_PATCH", + value_name = "JSON", + conflicts_with = "patch_file" + )] + patch: Option, + + /// Read the arbitrary patch JSON from a file. Mutually exclusive with --patch. + #[arg( + long, + env = "BT_FUNCTIONS_UPDATE_PATCH_FILE", + value_name = "PATH", + conflicts_with = "patch" + )] + patch_file: Option, + + /// Skip the confirmation prompt. + #[arg(long, short = 'y', env = "BT_FUNCTIONS_UPDATE_YES", default_value_t = false, value_parser = clap::builder::BoolishValueParser::new())] + yes: bool, +} + +impl UpdateArgs { + 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>), +} + +pub async fn run( + ctx: &ResolvedContext, + args: &UpdateArgs, + json_output: bool, + ft: Option, +) -> Result<()> { + let body = build_patch_body(args)?; + + let function = resolve_target_function(ctx, args, ft).await?; + + 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 prompt_text = resolve_prompt_text(args)?; + let messages_json = resolve_messages(args)?; + + if prompt_text.is_some() || messages_json.is_some() { + let prompt_block = match (prompt_text, messages_json) { + (Some(text), None) => json!({ + "type": "completion", + "content": text, + }), + (None, Some(messages)) => json!({ + "type": "chat", + "messages": messages, + }), + (Some(_), Some(_)) => { + bail!("use either --prompt/--prompt-file or --messages, not both") + } + (None, None) => unreachable!("guarded above"), + }; + + let prompt_data = match patch.get("prompt_data") { + Some(Value::Object(existing)) => { + let mut merged = existing.clone(); + merged.insert("prompt".to_string(), prompt_block); + Value::Object(merged) + } + _ => json!({ "prompt": prompt_block }), + }; + patch.insert("prompt_data".to_string(), prompt_data); + } + + if let Some(model) = args.model.as_deref() { + let prompt_data = match patch.get("prompt_data") { + Some(Value::Object(existing)) => { + let mut merged = existing.clone(); + let options = match merged.get("options") { + Some(Value::Object(opts)) => opts.clone(), + _ => Map::new(), + }; + let mut options = options; + options.insert("model".to_string(), Value::String(model.to_string())); + merged.insert("options".to_string(), Value::Object(options)); + Value::Object(merged) + } + _ => json!({ "options": { "model": model } }), + }; + patch.insert("prompt_data".to_string(), prompt_data); + } + + let extra = resolve_extra_patch(args)?; + if let Some(extra_obj) = extra { + merge_objects(&mut patch, &extra_obj); + } + + if patch.is_empty() { + bail!( + "no updates requested. Pass one of --prompt/--prompt-file, --messages, --model, --description, or --patch/--patch-file" + ); + } + + Ok(Value::Object(patch)) +} + +fn resolve_prompt_text(args: &UpdateArgs) -> Result> { + match (&args.prompt, &args.prompt_file) { + (Some(_), Some(_)) => bail!("use either --prompt or --prompt-file, not both"), + (Some(text), None) => Ok(Some(text.clone())), + (None, Some(path)) => { + let content = std::fs::read_to_string(path) + .with_context(|| format!("failed to read prompt file {}", path.display()))?; + Ok(Some(content)) + } + (None, None) => { + // Allow piping the prompt text via stdin in a non-interactive shell. + if args.messages.is_some() + || args.model.is_some() + || args.description.is_some() + || args.patch.is_some() + || args.patch_file.is_some() + { + return Ok(None); + } + if !is_interactive() { + use std::io::Read; + let mut buf = String::new(); + std::io::stdin() + .read_to_string(&mut buf) + .context("failed to read prompt from stdin")?; + let trimmed = buf.trim(); + if trimmed.is_empty() { + return Ok(None); + } + return Ok(Some(trimmed.to_string())); + } + Ok(None) + } + } +} + +fn resolve_messages(args: &UpdateArgs) -> Result> { + match &args.messages { + Some(raw) => { + 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>> { + match (&args.patch, &args.patch_file) { + (Some(_), Some(_)) => bail!("use either --patch or --patch-file, not both"), + (Some(raw), None) => parse_patch_object(raw).map(Some), + (None, Some(path)) => { + let content = std::fs::read_to_string(path) + .with_context(|| format!("failed to read patch file {}", path.display()))?; + parse_patch_object(&content).map(Some) + } + (None, None) => Ok(None), + } +} + +fn parse_patch_object(raw: &str) -> Result> { + let value: Value = serde_json::from_str(raw).context("invalid JSON in --patch/--patch-file")?; + match value { + Value::Object(map) => Ok(map), + _ => bail!("--patch/--patch-file must be a JSON object"), + } +} + +fn merge_objects(target: &mut Map, source: &Map) { + for (key, value) in source { + match (target.get_mut(key), value) { + (Some(Value::Object(target_inner)), Value::Object(source_inner)) => { + merge_objects(target_inner, source_inner); + } + _ => { + target.insert(key.clone(), value.clone()); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn args(prompt: Option<&str>, model: Option<&str>, description: Option<&str>) -> UpdateArgs { + UpdateArgs { + slug: super::super::SlugArgs { + slug_positional: Some("test-slug".to_string()), + slug_flag: None, + }, + id: None, + prompt: prompt.map(ToOwned::to_owned), + prompt_file: None, + messages: None, + model: model.map(ToOwned::to_owned), + description: description.map(ToOwned::to_owned), + patch: None, + patch_file: None, + yes: true, + } + } + + #[test] + fn build_patch_body_prompt_writes_completion_block() { + let args = args(Some("Grade the answer."), None, None); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["prompt"]["type"], + serde_json::json!("completion") + ); + assert_eq!( + body["prompt_data"]["prompt"]["content"], + serde_json::json!("Grade the answer.") + ); + } + + #[test] + fn build_patch_body_messages_writes_chat_block() { + let mut args = args(None, 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(None, 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_prompt_and_model_combine() { + let args = args(Some("Grade it."), Some("gpt-4o-mini"), None); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!(body["prompt_data"]["prompt"]["content"], "Grade it."); + assert_eq!( + body["prompt_data"]["options"]["model"], + serde_json::json!("gpt-4o-mini") + ); + } + + #[test] + fn build_patch_body_description_is_top_level() { + let args = args(None, 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_rejects_prompt_and_messages_together() { + let mut args = args(Some("Grade it."), None, None); + args.messages = Some(r#"[{"role":"user","content":"hi"}]"#.to_string()); + let err = build_patch_body(&args).expect_err("should reject"); + assert!(err.to_string().contains("not both")); + } + + #[test] + fn build_patch_body_rejects_prompt_and_prompt_file_together() { + let mut args = args(Some("Grade it."), None, None); + args.prompt_file = Some(PathBuf::from("/tmp/ignore.md")); + let err = build_patch_body(&args).expect_err("should reject"); + assert!(err.to_string().contains("not both")); + } + + #[test] + fn build_patch_body_rejects_patch_and_patch_file_together() { + let mut args = args(None, None, None); + args.patch = Some("{}".to_string()); + args.patch_file = Some(PathBuf::from("/tmp/ignore.json")); + let err = build_patch_body(&args).expect_err("should reject"); + assert!(err.to_string().contains("not both")); + } + + #[test] + fn build_patch_body_rejects_empty_update() { + let args = args(None, 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, 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_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/prompts/api.rs b/src/prompts/api.rs index 5a40a8e7..91ab38b5 100644 --- a/src/prompts/api.rs +++ b/src/prompts/api.rs @@ -1,5 +1,6 @@ use anyhow::Result; use serde::{Deserialize, Serialize}; +use serde_json::Value; use urlencoding::encode; use crate::http::ApiClient; @@ -51,3 +52,13 @@ pub async fn delete_prompt(client: &ApiClient, prompt_id: &str) -> Result<()> { let path = format!("/v1/prompt/{}", encode(prompt_id)); client.delete(&path).await } + +/// Partially update a prompt by id via `PATCH /v1/prompt/{id}`. +/// +/// The Braintrust API deep-merges object fields, so callers can send the +/// nested fields they want to change (for example `prompt_data.prompt`) +/// without sending the whole prompt object. +pub async fn patch_prompt(client: &ApiClient, prompt_id: &str, body: &Value) -> Result { + let path = format!("/v1/prompt/{}", encode(prompt_id)); + client.patch(&path, body).await +} diff --git a/src/prompts/mod.rs b/src/prompts/mod.rs index 440ac341..04d46157 100644 --- a/src/prompts/mod.rs +++ b/src/prompts/mod.rs @@ -8,6 +8,7 @@ pub(crate) use crate::project_context::ProjectContext as ResolvedContext; mod api; mod delete; mod list; +mod update; mod view; #[derive(Debug, Clone, Args)] @@ -16,6 +17,7 @@ Examples: bt prompts list bt prompts view my-prompt bt prompts delete my-prompt + bt prompts update my-prompt --prompt-file prompt.md ")] pub struct PromptsArgs { #[command(subcommand)] @@ -28,6 +30,8 @@ enum PromptsCommands { List, /// View a prompt's content View(ViewArgs), + /// Update a prompt in place (prompt text, model, description, or arbitrary patch) + Update(update::UpdateArgs), /// Delete a prompt Delete(DeleteArgs), } @@ -87,6 +91,7 @@ pub async fn run(base: BaseArgs, args: PromptsArgs) -> Result<()> { Some(PromptsCommands::View(p)) => { view::run(&ctx, p.slug(), base.json, p.web, base.verbose).await } + Some(PromptsCommands::Update(p)) => update::run(&ctx, &p, base.json).await, Some(PromptsCommands::Delete(p)) => delete::run(&ctx, p.slug(), p.force).await, } } @@ -100,8 +105,16 @@ fn prompts_command_is_read_only(command: Option<&PromptsCommands>) -> bool { #[cfg(test)] mod tests { + use clap::Parser; + use super::*; + #[derive(Debug, Parser)] + struct PromptsArgsHarness { + #[command(flatten)] + args: PromptsArgs, + } + #[test] fn prompts_routes_list_and_view_to_read_only_auth() { assert!(prompts_command_is_read_only(None)); @@ -125,4 +138,19 @@ mod tests { }) ))); } + + #[test] + fn prompts_routes_update_to_validated_auth() { + let parsed = PromptsArgsHarness::try_parse_from([ + "bt-prompts", + "update", + "my-prompt", + "--description", + "updated", + "--yes", + ]) + .expect("parse update"); + + assert!(!prompts_command_is_read_only(parsed.args.command.as_ref())); + } } diff --git a/src/prompts/update.rs b/src/prompts/update.rs new file mode 100644 index 00000000..b21eb4b0 --- /dev/null +++ b/src/prompts/update.rs @@ -0,0 +1,491 @@ +use std::path::PathBuf; + +use anyhow::{anyhow, bail, Context, Result}; +use clap::Args; +use dialoguer::Confirm; +use serde_json::{json, Map, Value}; + +use crate::ui::{is_interactive, print_command_status, with_spinner, CommandStatus}; + +use super::{api, ResolvedContext}; + +/// Update a prompt in place via `PATCH /v1/prompt/{id}`. +/// +/// The Braintrust API deep-merges object fields, so you can send just the +/// nested fields you want to change (for example `prompt_data.prompt`) without +/// re-authoring the whole prompt. +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt prompts update my-prompt --prompt-file prompt.md + bt prompts update my-prompt --model gpt-4o-mini + bt prompts update my-prompt --description \"Customer support prompt\" + bt prompts update my-prompt --patch '{\"prompt_data\":{\"options\":{\"model\":\"gpt-4o-mini\"}}}' + bt prompts update my-prompt --patch-file prompt-patch.json +")] +pub struct UpdateArgs { + /// Prompt slug (positional) + #[arg(value_name = "SLUG", conflicts_with = "slug_flag")] + slug_positional: Option, + + /// Prompt slug (flag) + #[arg(long = "slug", short = 's', env = "BT_PROMPTS_UPDATE_SLUG")] + slug_flag: Option, + + /// Replace the completion prompt text. Writes `prompt_data.prompt` as + /// `{"type":"completion","content":}`. Read from a file with + /// --prompt-file, or stdin when no value is given in a non-interactive shell. + #[arg( + long, + env = "BT_PROMPTS_UPDATE_PROMPT", + value_name = "TEXT", + conflicts_with_all = ["prompt_file", "messages"] + )] + prompt: Option, + + /// Read the completion prompt text from a file. Mutually exclusive with --prompt. + #[arg( + long, + env = "BT_PROMPTS_UPDATE_PROMPT_FILE", + value_name = "PATH", + conflicts_with_all = ["prompt", "messages"] + )] + prompt_file: Option, + + /// Replace the chat prompt messages (JSON array). Writes + /// `prompt_data.prompt` as `{"type":"chat","messages":}`. + #[arg( + long, + env = "BT_PROMPTS_UPDATE_MESSAGES", + value_name = "JSON", + conflicts_with_all = ["prompt", "prompt_file"] + )] + messages: Option, + + /// Update the model used by the prompt. Writes `prompt_data.options.model`. + #[arg( + long, + short = 'm', + env = "BT_PROMPTS_UPDATE_MODEL", + value_name = "MODEL" + )] + model: Option, + + /// Update the prompt description. + #[arg( + long, + short = 'd', + env = "BT_PROMPTS_UPDATE_DESCRIPTION", + value_name = "TEXT" + )] + description: Option, + + /// Arbitrary JSON object deep-merged into the prompt on patch. Use this + /// for fields without a dedicated flag (for example `tags`, `metadata`, + /// or nested `prompt_data.options.params`). + #[arg( + long, + env = "BT_PROMPTS_UPDATE_PATCH", + value_name = "JSON", + conflicts_with = "patch_file" + )] + patch: Option, + + /// Read the arbitrary patch JSON from a file. Mutually exclusive with --patch. + #[arg( + long, + env = "BT_PROMPTS_UPDATE_PATCH_FILE", + value_name = "PATH", + conflicts_with = "patch" + )] + patch_file: Option, + + /// Skip the confirmation prompt. + #[arg(long, short = 'y', env = "BT_PROMPTS_UPDATE_YES", default_value_t = false, value_parser = clap::builder::BoolishValueParser::new())] + yes: bool, +} + +impl UpdateArgs { + fn slug(&self) -> Option<&str> { + self.slug_positional + .as_deref() + .or(self.slug_flag.as_deref()) + } +} + +pub async fn run(ctx: &ResolvedContext, args: &UpdateArgs, json_output: bool) -> Result<()> { + let project_name = &ctx.project.name; + let body = build_patch_body(args)?; + + let prompt = match args.slug() { + Some(slug) => with_spinner( + "Loading prompt...", + api::get_prompt_by_slug(&ctx.client, project_name, slug), + ) + .await? + .ok_or_else(|| anyhow!("prompt with slug '{slug}' not found"))?, + None => { + if !is_interactive() { + bail!("prompt slug required. Use: bt prompts update [--patch ...]"); + } + super::delete::select_prompt_interactive(&ctx.client, project_name).await? + } + }; + + if !args.yes && is_interactive() { + let confirm = Confirm::new() + .with_prompt(format!( + "Update prompt '{}' in {}?", + prompt.name, project_name + )) + .default(false) + .interact()?; + if !confirm { + return Ok(()); + } + } + + let updated = match with_spinner( + "Updating prompt...", + api::patch_prompt(&ctx.client, &prompt.id, &body), + ) + .await + { + Ok(value) => { + print_command_status( + CommandStatus::Success, + &format!("Updated '{}'", prompt.name), + ); + value + } + Err(error) => { + print_command_status( + CommandStatus::Error, + &format!("Failed to update '{}'", prompt.name), + ); + return Err(error); + } + }; + + if json_output { + println!("{}", serde_json::to_string(&updated)?); + } else if !crate::ui::is_quiet() { + eprintln!( + "Run `bt prompts view {}` to inspect the updated prompt.", + prompt.slug + ); + } + + Ok(()) +} + +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 prompt_text = resolve_prompt_text(args)?; + let messages_json = resolve_messages(args)?; + + if prompt_text.is_some() || messages_json.is_some() { + let prompt_block = match (prompt_text, messages_json) { + (Some(text), None) => json!({ + "type": "completion", + "content": text, + }), + (None, Some(messages)) => json!({ + "type": "chat", + "messages": messages, + }), + (Some(_), Some(_)) => { + bail!("use either --prompt/--prompt-file or --messages, not both") + } + (None, None) => unreachable!("guarded above"), + }; + + let prompt_data = match patch.get("prompt_data") { + Some(Value::Object(existing)) => { + let mut merged = existing.clone(); + merged.insert("prompt".to_string(), prompt_block); + Value::Object(merged) + } + _ => json!({ "prompt": prompt_block }), + }; + patch.insert("prompt_data".to_string(), prompt_data); + } + + if let Some(model) = args.model.as_deref() { + let prompt_data = match patch.get("prompt_data") { + Some(Value::Object(existing)) => { + let mut merged = existing.clone(); + let options = match merged.get("options") { + Some(Value::Object(opts)) => opts.clone(), + _ => Map::new(), + }; + let mut options = options; + options.insert("model".to_string(), Value::String(model.to_string())); + merged.insert("options".to_string(), Value::Object(options)); + Value::Object(merged) + } + _ => json!({ "options": { "model": model } }), + }; + patch.insert("prompt_data".to_string(), prompt_data); + } + + let extra = resolve_extra_patch(args)?; + if let Some(extra_obj) = extra { + merge_objects(&mut patch, &extra_obj); + } + + if patch.is_empty() { + bail!( + "no updates requested. Pass one of --prompt/--prompt-file, --messages, --model, --description, or --patch/--patch-file" + ); + } + + Ok(Value::Object(patch)) +} + +fn resolve_prompt_text(args: &UpdateArgs) -> Result> { + match (&args.prompt, &args.prompt_file) { + (Some(_), Some(_)) => bail!("use either --prompt or --prompt-file, not both"), + (Some(text), None) => Ok(Some(text.clone())), + (None, Some(path)) => { + let content = std::fs::read_to_string(path) + .with_context(|| format!("failed to read prompt file {}", path.display()))?; + Ok(Some(content)) + } + (None, None) => { + if args.messages.is_some() + || args.model.is_some() + || args.description.is_some() + || args.patch.is_some() + || args.patch_file.is_some() + { + return Ok(None); + } + if !is_interactive() { + use std::io::Read; + let mut buf = String::new(); + std::io::stdin() + .read_to_string(&mut buf) + .context("failed to read prompt from stdin")?; + let trimmed = buf.trim(); + if trimmed.is_empty() { + return Ok(None); + } + return Ok(Some(trimmed.to_string())); + } + Ok(None) + } + } +} + +fn resolve_messages(args: &UpdateArgs) -> Result> { + match &args.messages { + Some(raw) => { + 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>> { + match (&args.patch, &args.patch_file) { + (Some(_), Some(_)) => bail!("use either --patch or --patch-file, not both"), + (Some(raw), None) => parse_patch_object(raw).map(Some), + (None, Some(path)) => { + let content = std::fs::read_to_string(path) + .with_context(|| format!("failed to read patch file {}", path.display()))?; + parse_patch_object(&content).map(Some) + } + (None, None) => Ok(None), + } +} + +fn parse_patch_object(raw: &str) -> Result> { + let value: Value = serde_json::from_str(raw).context("invalid JSON in --patch/--patch-file")?; + match value { + Value::Object(map) => Ok(map), + _ => bail!("--patch/--patch-file must be a JSON object"), + } +} + +fn merge_objects(target: &mut Map, source: &Map) { + for (key, value) in source { + match (target.get_mut(key), value) { + (Some(Value::Object(target_inner)), Value::Object(source_inner)) => { + merge_objects(target_inner, source_inner); + } + _ => { + target.insert(key.clone(), value.clone()); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn args(prompt: Option<&str>, model: Option<&str>, description: Option<&str>) -> UpdateArgs { + UpdateArgs { + slug_positional: Some("test-prompt".to_string()), + slug_flag: None, + prompt: prompt.map(ToOwned::to_owned), + prompt_file: None, + messages: None, + model: model.map(ToOwned::to_owned), + description: description.map(ToOwned::to_owned), + patch: None, + patch_file: None, + yes: true, + } + } + + #[test] + fn build_patch_body_prompt_writes_completion_block() { + let args = args(Some("Answer the question."), None, None); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["prompt"]["type"], + serde_json::json!("completion") + ); + assert_eq!( + body["prompt_data"]["prompt"]["content"], + serde_json::json!("Answer the question.") + ); + } + + #[test] + fn build_patch_body_messages_writes_chat_block() { + let mut args = args(None, 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(None, 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_prompt_and_model_combine() { + let args = args(Some("Answer it."), Some("gpt-4o-mini"), None); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!(body["prompt_data"]["prompt"]["content"], "Answer it."); + assert_eq!( + body["prompt_data"]["options"]["model"], + serde_json::json!("gpt-4o-mini") + ); + } + + #[test] + fn build_patch_body_description_is_top_level() { + let args = args(None, None, Some("Customer support prompt")); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["description"], + serde_json::json!("Customer support prompt") + ); + } + + #[test] + fn build_patch_body_rejects_prompt_and_messages_together() { + let mut args = args(Some("Answer it."), None, None); + args.messages = Some(r#"[{"role":"user","content":"hi"}]"#.to_string()); + let err = build_patch_body(&args).expect_err("should reject"); + assert!(err.to_string().contains("not both")); + } + + #[test] + fn build_patch_body_rejects_prompt_and_prompt_file_together() { + let mut args = args(Some("Answer it."), None, None); + args.prompt_file = Some(PathBuf::from("/tmp/ignore.md")); + let err = build_patch_body(&args).expect_err("should reject"); + assert!(err.to_string().contains("not both")); + } + + #[test] + fn build_patch_body_rejects_patch_and_patch_file_together() { + let mut args = args(None, None, None); + args.patch = Some("{}".to_string()); + args.patch_file = Some(PathBuf::from("/tmp/ignore.json")); + let err = build_patch_body(&args).expect_err("should reject"); + assert!(err.to_string().contains("not both")); + } + + #[test] + fn build_patch_body_rejects_empty_update() { + let args = args(None, 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, None); + args.patch = + Some(r#"{"prompt_data":{"options":{"params":{"temperature":0}}}}"#.to_string()); + let body = build_patch_body(&args).expect("patch body"); + assert_eq!( + body["prompt_data"]["options"]["params"]["temperature"], + serde_json::json!(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_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 842240b3..ada1f490 100644 --- a/src/scorers.rs +++ b/src/scorers.rs @@ -1,10 +1,93 @@ use anyhow::Result; +use clap::{Args, Subcommand}; use crate::args::BaseArgs; -use crate::functions::{self, FunctionArgs, FunctionTypeFilter}; +use crate::functions::{self, FunctionCommands, FunctionTypeFilter}; -pub type ScorersArgs = FunctionArgs; +#[derive(Debug, Clone, Args)] +#[command(after_help = "\ +Examples: + bt scorers list + bt scorers view my-scorer + bt scorers create \"Helpfulness\" --model gpt-4o-mini --prompt-file judge.md \\ + --choice-scores '{\"A\":1,\"B\":0}' --use-cot + bt scorers update my-scorer --prompt-file judge.md + bt scorers delete my-scorer +")] +pub struct ScorersArgs { + #[command(subcommand)] + command: Option, +} + +#[derive(Debug, Clone, Subcommand)] +enum ScorersCommands { + /// Create an LLM scorer + Create(functions::create::CreateArgs), + #[command(flatten)] + Function(FunctionCommands), +} pub async fn run(base: BaseArgs, args: ScorersArgs) -> Result<()> { - functions::run_typed(base, args, FunctionTypeFilter::Scorer).await + 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 + } + None => functions::run_typed_command(base, None, FunctionTypeFilter::Scorer).await, + } +} + +#[cfg(test)] +mod tests { + use clap::Parser; + + use super::*; + + #[derive(Debug, Parser)] + struct ScorersArgsHarness { + #[command(flatten)] + args: ScorersArgs, + } + + #[test] + fn parses_create_scorer() { + let parsed = ScorersArgsHarness::try_parse_from([ + "bt-scorers", + "create", + "Test scorer", + "--model", + "gpt-test", + "--prompt", + "Judge {{output}}", + "--choice-scores", + r#"{"yes":1,"no":0}"#, + "--use-cot=false", + "--if-exists", + "replace", + ]) + .expect("parse create"); + + 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 acb09bfd..7ef2369c 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -109,6 +109,36 @@ fn top_level_help_shows_update_not_self() { .stdout(predicate::str::contains("self Self-management commands").not()); } +#[test] +fn scorers_create_help_includes_llm_judge_flags() { + bt_command() + .args(["scorers", "create", "--help"]) + .assert() + .success() + .stdout(predicate::str::contains("--prompt-file")) + .stdout(predicate::str::contains("--model")) + .stdout(predicate::str::contains("--choice-scores")) + .stdout(predicate::str::contains("--use-cot")) + .stdout(predicate::str::contains("--if-exists")); +} + +#[test] +fn scorer_and_prompt_update_help_is_conflict_free() { + bt_command() + .args(["scorers", "update", "--help"]) + .assert() + .success() + .stdout(predicate::str::contains("--patch-file")) + .stdout(predicate::str::contains("--prompt-file")); + + bt_command() + .args(["prompts", "update", "--help"]) + .assert() + .success() + .stdout(predicate::str::contains("--patch-file")) + .stdout(predicate::str::contains("--prompt-file")); +} + #[test] fn topics_report_help_accepts_global_org_short_conflict_free() { bt_command() From 05012ad7f7348cb8b8fce626a3fcf5e6b9932629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Fri, 24 Jul 2026 15:47:34 -0700 Subject: [PATCH 2/7] add(scorer): llm parameters, templating, classifiers, pass threshold, metadata New fields can be added when creating/updating a prompt to achieve feature parity with the web ui --- Cargo.lock | 1 + Cargo.toml | 1 + scripts/skill-smoke-test.sh | 2 +- src/functions/api.rs | 32 +- src/functions/create.rs | 455 ++++++++++++++++------------- src/functions/mod.rs | 41 ++- src/functions/prompt_config.rs | 354 ++++++++++++++++++++++ src/functions/update.rs | 520 +++++++++++++++++++-------------- src/prompts/mod.rs | 6 +- src/prompts/update.rs | 350 +++++++++------------- src/scorers.rs | 44 ++- src/utils/json_object.rs | 34 +++ src/utils/mod.rs | 6 +- src/utils/structured_source.rs | 39 +++ src/utils/text_source.rs | 70 +++++ tests/cli.rs | 40 ++- 16 files changed, 1334 insertions(+), 661 deletions(-) create mode 100644 src/functions/prompt_config.rs create mode 100644 src/utils/structured_source.rs create mode 100644 src/utils/text_source.rs diff --git a/Cargo.lock b/Cargo.lock index cefa77c6..c7134b75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -531,6 +531,7 @@ dependencies = [ "serde", "serde_json 1.0.149", "serde_path_to_error", + "serde_yaml", "sha2", "strip-ansi-escapes", "tempfile", diff --git a/Cargo.toml b/Cargo.toml index a7da2549..e7889160 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ reqwest = { version = "0.12.7", default-features = false, features = ["json", "r serde = { version = "1.0.210", features = ["derive"] } serde_json = "1.0.128" serde_path_to_error = "0.1.20" +serde_yaml = "0.9" toml = "0.8" sha2 = "0.10.8" strip-ansi-escapes = "0.2.0" diff --git a/scripts/skill-smoke-test.sh b/scripts/skill-smoke-test.sh index 7cb18219..f0a84045 100755 --- a/scripts/skill-smoke-test.sh +++ b/scripts/skill-smoke-test.sh @@ -27,7 +27,7 @@ Options: Examples: scripts/skill-smoke-test.sh --agent codex - scripts/skill-smoke-test.sh --agent codex --agent-cmd 'codex run --prompt-file AGENT_TASK.md' + scripts/skill-smoke-test.sh --agent codex --agent-cmd 'codex exec - < AGENT_TASK.md' scripts/skill-smoke-test.sh --demo-dir /tmp/bt-skill-demo --verify-only EOF } diff --git a/src/functions/api.rs b/src/functions/api.rs index 0fd06bb2..ae46a402 100644 --- a/src/functions/api.rs +++ b/src/functions/api.rs @@ -68,17 +68,29 @@ pub async fn list_functions( project_id: &str, function_type: Option<&str>, ) -> Result> { + let query = list_functions_query(project_id, function_type); + let response = client.btql::(&query).await?; + + Ok(response.data) +} + +fn list_functions_query(project_id: &str, function_type: Option<&str>) -> String { let pid = escape_sql(project_id); - let query = match function_type { + let type_filter = match function_type { + // The Braintrust UI lists score-producing scorers and label-producing + // classifiers together in the Scorers section. + Some("scorer") => " AND function_type IN ('scorer', 'classifier')".to_string(), Some(ft) => { let ft = escape_sql(ft); - format!("SELECT * FROM project_functions('{pid}') WHERE function_type = '{ft}'") + format!(" AND function_type = '{ft}'") } - None => format!("SELECT * FROM project_functions('{pid}')"), + None => String::new(), }; - let response = client.btql::(&query).await?; - - Ok(response.data) + // Function definitions can be arbitrarily old, but retain an explicit + // timestamp constraint to keep every BTQL query bounded. + format!( + "SELECT * FROM project_functions('{pid}') WHERE created >= '1970-01-01T00:00:00Z'{type_filter}" + ) } pub async fn get_function_by_slug( @@ -291,6 +303,14 @@ fn ignored_count(raw: &Value) -> Option { mod tests { use super::*; + #[test] + fn scorer_list_query_includes_classifiers_and_a_timestamp_bound() { + let query = list_functions_query("test-project-id", Some("scorer")); + + assert!(query.contains("created >= '1970-01-01T00:00:00Z'")); + assert!(query.contains("function_type IN ('scorer', 'classifier')")); + } + #[test] fn ignored_count_extracts_canonical_shape() { let first = serde_json::json!({ "ignored_count": 3 }); diff --git a/src/functions/create.rs b/src/functions/create.rs index bc57f342..7bb76ced 100644 --- a/src/functions/create.rs +++ b/src/functions/create.rs @@ -1,30 +1,51 @@ -use std::{io::Read, path::PathBuf}; - use anyhow::{bail, Context, Result}; -use clap::{builder::BoolishValueParser, Args}; +use clap::{builder::BoolishValueParser, ArgGroup, Args}; use dialoguer::Input; use serde_json::{json, Map, Value}; -use crate::ui::{is_interactive, print_command_status, with_spinner, CommandStatus}; - -use super::{api, IfExistsMode, ResolvedContext}; - -/// Create an LLM scorer. +use crate::{ + 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, + }, + IfExistsMode, ResolvedContext, +}; + +/// Create an LLM scorer or classifier. /// -/// The generated definition matches `project.scorers.create(...)`: a prompt -/// function with an `llm_classifier` parser, model, chain-of-thought setting, -/// and numeric score for each possible choice. +/// The generated definition matches Braintrust's prompt-function schema with +/// an `llm_classifier` parser. `--choice-scores` produces numeric scores; +/// `--classifications` produces labels. #[derive(Debug, Clone, Args)] +#[command(group( + ArgGroup::new("output") + .required(true) + .multiple(false) + .args(["choice_scores", "classifications"]) +))] #[command(after_help = "\ Examples: - bt scorers create \"Helpfulness\" --model gpt-4o-mini --prompt-file judge.md \\ - --choice-scores '{\"A\":1,\"B\":0}' --use-cot - bt scorers create \"Correctness\" --slug correctness --model gpt-4o-mini \\ - --prompt \"Score {{output}} against {{expected}}\" \\ + bt scorers create \"Helpfulness\" --model gpt-5.4-nano --messages @messages.json \\ + --choice-scores '{\"A\":1,\"B\":0}' + bt scorers create \"Correctness\" --slug correctness --model gpt-5.4-nano \\ + --messages @messages.json \\ --choice-scores '{\"correct\":1,\"incorrect\":0}' --use-cot=false - bt scorers create \"Tone\" --model gpt-4o-mini \\ - --messages '[{\"role\":\"user\",\"content\":\"Judge {{output}}\"}]' \\ - --choice-scores-file scores.json --use-cot + bt scorers create \"Tone\" --model gpt-5.4-nano \\ + --messages @messages.json --choice-scores @scores.json + bt scorers create \"Safety label\" --model gpt-5.4-nano --messages @messages.json \\ + --classifications '[\"safe\",\"unsafe\"]' --template-format jinja + +TypeScript and Python code scorers: + TypeScript: projects.create({ name: \"test-project\" }).scorers.create({...}) + bt functions push scorer.ts + Python: projects.create(\"test-project\").scorers.create(...) + bt functions push scorer.py ")] pub(crate) struct CreateArgs { /// Scorer name. @@ -32,101 +53,65 @@ pub(crate) struct CreateArgs { name_positional: Option, /// Scorer name (alternative to the positional name). - #[arg(long, env = "BT_SCORERS_CREATE_NAME", value_name = "NAME")] + #[arg(long, value_name = "NAME")] name: Option, /// Unique scorer slug. Defaults to a slug generated from the name. - #[arg(long, short = 's', env = "BT_SCORERS_CREATE_SLUG")] + #[arg(long, short = 's')] slug: Option, /// Scorer description. - #[arg(long, short = 'd', env = "BT_SCORERS_CREATE_DESCRIPTION")] + #[arg(long, short = 'd')] description: Option, - /// Completion prompt text. Use --prompt-file for a file, or pipe the - /// prompt through stdin when no prompt option is supplied. - #[arg( - long, - env = "BT_SCORERS_CREATE_PROMPT", - value_name = "TEXT", - conflicts_with_all = ["prompt_file", "messages", "messages_file"] - )] - prompt: Option, - - /// Read the completion prompt text from a file. - #[arg( - long, - env = "BT_SCORERS_CREATE_PROMPT_FILE", - value_name = "PATH", - conflicts_with_all = ["prompt", "messages", "messages_file"] - )] - prompt_file: Option, - - /// Chat prompt messages as a JSON array. - #[arg( - long, - env = "BT_SCORERS_CREATE_MESSAGES", - value_name = "JSON", - conflicts_with_all = ["prompt", "prompt_file", "messages_file"] - )] - messages: Option, - - /// Read chat prompt messages as a JSON array from a file. - #[arg( - long, - env = "BT_SCORERS_CREATE_MESSAGES_FILE", - value_name = "PATH", - conflicts_with_all = ["prompt", "prompt_file", "messages"] - )] - messages_file: Option, + /// Chat messages source: inline JSON, @PATH to read from a file, or - for + /// stdin. + #[arg(long, value_name = "SOURCE")] + messages: String, /// Model used by the LLM judge. - #[arg( - long, - short = 'm', - env = "BT_SCORERS_CREATE_MODEL", - value_name = "MODEL" - )] + #[arg(long, short = 'm', value_name = "MODEL")] model: String, - /// JSON object mapping each classifier choice to a numeric score. - #[arg( - long, - env = "BT_SCORERS_CREATE_CHOICE_SCORES", - value_name = "JSON", - required_unless_present = "choice_scores_file", - conflicts_with = "choice_scores_file" - )] + #[command(flatten)] + prompt_config: PromptConfigArgs, + + /// Choice-to-score mapping for score output: inline JSON, @PATH to read + /// from a file, or - for stdin. Scores must be between 0 and 1. + #[arg(long, value_name = "SOURCE")] choice_scores: Option, - /// Read the choice-to-score JSON object from a file. - #[arg( - long, - env = "BT_SCORERS_CREATE_CHOICE_SCORES_FILE", - value_name = "PATH", - conflicts_with = "choice_scores" - )] - choice_scores_file: Option, + /// Labels for classification output: an inline JSON array, @PATH to read + /// from a file, or - for stdin. This creates an LLM classifier, which is + /// shown alongside scorers in the Braintrust UI. + #[arg(long, value_name = "SOURCE")] + classifications: Option, + + /// Allow a classifier to return no matching classification. + #[arg(long, requires = "classifications")] + allow_no_match: bool, - /// Whether the scorer should use chain-of-thought reasoning. This option - /// is required; pass --use-cot or --use-cot=false. + /// Whether the scorer should use chain-of-thought reasoning. Defaults to + /// true; pass --use-cot=false to disable it. #[arg( long, - env = "BT_SCORERS_CREATE_USE_COT", num_args = 0..=1, default_missing_value = "true", - required = true, + default_value_t = true, value_parser = BoolishValueParser::new() )] - use_cot: Option, + use_cot: bool, + + /// Score threshold for passing, between 0 and 1. + #[arg(long, value_name = "NUMBER", conflicts_with = "classifications")] + pass_threshold: Option, + + /// Metadata as inline YAML, @PATH to a YAML file, or - for stdin. + #[arg(long, value_name = "SOURCE")] + metadata: Option, /// Behavior when a scorer with the same slug already exists. - #[arg( - long, - env = "BT_SCORERS_CREATE_IF_EXISTS", - value_enum, - default_value = "error" - )] + #[arg(long, value_enum, default_value = "error")] if_exists: IfExistsMode, } @@ -231,14 +216,20 @@ fn build_scorer_definition( name: &str, slug: &str, ) -> Result { - if args.model.trim().is_empty() { - bail!("--model cannot be empty"); - } - let use_cot = args.use_cot.ok_or_else(|| { - anyhow::anyhow!("--use-cot is required; pass --use-cot or --use-cot=false") - })?; let prompt = resolve_prompt_block(args)?; - let choice_scores = resolve_choice_scores(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 mut definition = json!({ "project_id": project_id, @@ -247,68 +238,69 @@ fn build_scorer_definition( "function_data": { "type": "prompt", }, - "prompt_data": { - "prompt": prompt, - "options": { - "model": args.model, - }, - "parser": { - "type": "llm_classifier", - "use_cot": use_cot, - "choice_scores": choice_scores, - }, - }, + "prompt_data": prompt_data, "if_exists": args.if_exists.as_str(), - "function_type": "scorer", + "function_type": function_type, }); 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_prompt_block(args: &CreateArgs) -> Result { - let selected = usize::from(args.prompt.is_some()) - + usize::from(args.prompt_file.is_some()) - + usize::from(args.messages.is_some()) - + usize::from(args.messages_file.is_some()); - if selected > 1 { - bail!("use only one of --prompt, --prompt-file, --messages, or --messages-file"); - } - - if let Some(prompt) = args.prompt.as_deref() { - return Ok(json!({ "type": "completion", "content": prompt })); - } - if let Some(path) = args.prompt_file.as_deref() { - let prompt = std::fs::read_to_string(path) - .with_context(|| format!("failed to read prompt file {}", path.display()))?; - return Ok(json!({ "type": "completion", "content": prompt })); - } - if let Some(raw) = args.messages.as_deref() { - return parse_messages(raw); - } - if let Some(path) = args.messages_file.as_deref() { - let raw = std::fs::read_to_string(path) - .with_context(|| format!("failed to read messages file {}", path.display()))?; - return parse_messages(&raw); +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 " + ), } +} - if is_interactive() { - bail!("scorer prompt required. Pass --prompt, --prompt-file, or --messages"); +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) +} - let mut prompt = String::new(); - std::io::stdin() - .read_to_string(&mut prompt) - .context("failed to read prompt from stdin")?; - if prompt.is_empty() { - bail!( - "scorer prompt required. Pass --prompt, --prompt-file, or --messages, or pipe prompt text through stdin" - ); - } - Ok(json!({ "type": "completion", "content": prompt })) +fn resolve_prompt_block(args: &CreateArgs) -> Result { + let raw = read_text_source(&args.messages, "messages")?; + parse_messages(&raw) } fn parse_messages(raw: &str) -> Result { @@ -319,62 +311,54 @@ fn parse_messages(raw: &str) -> Result { } } -fn resolve_choice_scores(args: &CreateArgs) -> Result { - let raw = match (&args.choice_scores, &args.choice_scores_file) { - (Some(_), Some(_)) => { - bail!("use either --choice-scores or --choice-scores-file, not both") - } - (Some(raw), None) => raw.clone(), - (None, Some(path)) => std::fs::read_to_string(path) - .with_context(|| format!("failed to read choice scores file {}", path.display()))?, - (None, None) => bail!( - "--choice-scores is required; pass a JSON object such as '{{\"yes\":1,\"no\":0}}'" - ), - }; - - let value: Value = serde_json::from_str(&raw).context("invalid JSON in choice scores")?; - let scores = match value { - Value::Object(scores) => scores, - _ => bail!("choice scores must be a JSON object mapping choices to numeric scores"), - }; - validate_choice_scores(&scores)?; - Ok(Value::Object(scores)) -} - -fn validate_choice_scores(scores: &Map) -> Result<()> { - if scores.is_empty() { - bail!("choice scores cannot be empty"); - } - for (choice, score) in scores { - if !score.is_number() { - bail!("score for choice '{choice}' must be a number"); - } - } - Ok(()) -} - #[cfg(test)] mod tests { + use clap::Parser; + use super::*; + #[derive(Debug, Parser)] + struct CreateArgsHarness { + #[command(flatten)] + args: CreateArgs, + } + fn args() -> CreateArgs { CreateArgs { name_positional: Some("Test Helpfulness".to_string()), name: None, slug: None, description: Some("Synthetic test scorer".to_string()), - prompt: Some("Judge {{output}}.".to_string()), - prompt_file: None, - messages: None, - messages_file: None, + messages: r#"[{"role":"user","content":"Judge {{output}}."}]"#.to_string(), model: "gpt-test".to_string(), + prompt_config: PromptConfigArgs::default(), choice_scores: Some(r#"{"A":1,"B":0}"#.to_string()), - choice_scores_file: None, - use_cot: Some(true), + classifications: None, + allow_no_match: false, + use_cot: true, + pass_threshold: None, + metadata: None, if_exists: IfExistsMode::Error, } } + #[test] + fn use_cot_defaults_to_true() { + let parsed = CreateArgsHarness::try_parse_from([ + "bt-scorers-create", + "Test scorer", + "--model", + "gpt-test", + "--messages", + r#"[{"role":"user","content":"Judge {{output}}"}]"#, + "--choice-scores", + r#"{"yes":1,"no":0}"#, + ]) + .expect("parse create args"); + + assert!(parsed.args.use_cot); + } + #[test] fn builds_sdk_compatible_llm_scorer_definition() { let args = args(); @@ -388,7 +372,7 @@ mod tests { assert_eq!(body["function_data"], json!({ "type": "prompt" })); assert_eq!(body["function_type"], "scorer"); - assert_eq!(body["prompt_data"]["prompt"]["type"], "completion"); + assert_eq!(body["prompt_data"]["prompt"]["type"], "chat"); assert_eq!(body["prompt_data"]["options"]["model"], "gpt-test"); assert_eq!( body["prompt_data"]["parser"], @@ -404,9 +388,7 @@ mod tests { #[test] fn builds_chat_prompt_definition() { - let mut args = args(); - args.prompt = None; - args.messages = Some(r#"[{"role":"user","content":"Judge {{output}}"}]"#.to_string()); + let args = args(); let body = build_scorer_definition(&args, "test-project", "Test", "test").expect("definition"); @@ -414,18 +396,18 @@ mod tests { assert_eq!(body["prompt_data"]["prompt"]["type"], "chat"); assert_eq!( body["prompt_data"]["prompt"]["messages"], - json!([{ "role": "user", "content": "Judge {{output}}" }]) + json!([{ "role": "user", "content": "Judge {{output}}." }]) ); } #[test] - fn rejects_multiple_prompt_sources() { + fn rejects_non_array_messages() { let mut args = args(); - args.messages = Some("[]".to_string()); + args.messages = r#"{"role":"user","content":"Judge {{output}}"}"#.to_string(); let error = build_scorer_definition(&args, "test-project", "Test", "test") - .expect_err("prompt sources should conflict"); - assert!(error.to_string().contains("use only one")); + .expect_err("messages should be an array"); + assert!(error.to_string().contains("messages must be a JSON array")); } #[test] @@ -439,13 +421,88 @@ mod tests { } #[test] - fn requires_explicit_use_cot() { + fn supports_disabling_use_cot() { let mut args = args(); - args.use_cot = None; + args.use_cot = false; - let error = build_scorer_definition(&args, "test-project", "Test", "test") - .expect_err("missing use-cot should fail"); - assert!(error.to_string().contains("--use-cot is required")); + let body = + build_scorer_definition(&args, "test-project", "Test", "test").expect("definition"); + assert_eq!(body["prompt_data"]["parser"]["use_cot"], false); + } + + #[test] + fn builds_classification_output() { + let mut args = args(); + args.choice_scores = None; + args.classifications = Some(r#"["safe","unsafe"]"#.to_string()); + args.allow_no_match = true; + + let body = + build_scorer_definition(&args, "test-project", "Test", "test").expect("definition"); + + 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!(body["prompt_data"]["parser"].get("choice_scores").is_none()); + } + + #[test] + fn builds_model_params_template_metadata_and_pass_threshold() { + let parsed = CreateArgsHarness::try_parse_from([ + "bt-scorers-create", + "Test scorer", + "--model", + "gpt-test", + "--messages", + r#"[{"role":"user","content":"Judge {{output}}"}]"#, + "--choice-scores", + r#"{"yes":1,"no":0}"#, + "--temperature", + "0.1", + "--max-tokens", + "256", + "--top-p", + "0.8", + "--frequency-penalty", + "-0.25", + "--presence-penalty", + "0.5", + "--stop-sequence", + "END", + "--tool-choice", + "required", + "--reasoning-effort", + "medium", + "--verbosity", + "high", + "--template-format", + "jinja", + "--pass-threshold", + "0.7", + "--metadata", + "owner: test-team", + ]) + .expect("parse create args"); + + let body = + build_scorer_definition(&parsed.args, "test-project", "Test scorer", "test-scorer") + .expect("definition"); + let params = &body["prompt_data"]["options"]["params"]; + assert_eq!(params["temperature"], 0.1); + assert_eq!(params["max_tokens"], 256); + assert_eq!(params["top_p"], 0.8); + assert_eq!(params["frequency_penalty"], -0.25); + assert_eq!(params["presence_penalty"], 0.5); + assert_eq!(params["stop"], json!(["END"])); + assert_eq!(params["tool_choice"], "required"); + assert_eq!(params["reasoning_effort"], "medium"); + assert_eq!(params["verbosity"], "high"); + assert_eq!(body["prompt_data"]["template_format"], "nunjucks"); + assert_eq!(body["metadata"]["owner"], "test-team"); + assert_eq!(body["metadata"]["__pass_threshold"], 0.7); } #[test] diff --git a/src/functions/mod.rs b/src/functions/mod.rs index e5d98623..090a37ba 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -17,6 +17,7 @@ pub(crate) mod create; mod delete; mod invoke; mod list; +pub(crate) mod prompt_config; mod pull; mod push; pub(crate) mod report; @@ -116,6 +117,9 @@ fn build_web_path(function: &Function) -> String { 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)) + } Some("classifier") => { let xact_id = function._xact_id.as_deref().unwrap_or(""); format!( @@ -170,7 +174,7 @@ Examples: bt tools view my-tool bt tools view fn_123 bt tools view --id fn_123 - bt tools update my-tool --patch-file tool-patch.json + bt tools update my-tool --patch @tool-patch.json ")] pub struct FunctionArgs { #[command(subcommand)] @@ -187,8 +191,8 @@ pub(crate) enum FunctionCommands { Delete(DeleteArgs), /// Invoke a function Invoke(invoke::InvokeArgs), - /// Update a function in place (prompt, model, description, or arbitrary patch) - Update(update::UpdateArgs), + /// Update a function in place (prompt configuration, metadata, or arbitrary patch) + Update(Box), } #[derive(Debug, Clone, Args)] @@ -221,8 +225,8 @@ enum FunctionsCommands { Delete(FunctionsDeleteArgs), /// Invoke a function Invoke(FunctionsInvokeArgs), - /// Update a function in place (prompt, model, description, or arbitrary patch) - Update(FunctionsUpdateArgs), + /// Update a function in place (prompt configuration, metadata, or arbitrary patch) + Update(Box), /// Push local function definitions Push(PushArgs), /// Pull remote function definitions @@ -277,12 +281,7 @@ struct FunctionsUpdateArgs { #[command(flatten)] inner: update::UpdateArgs, /// Filter by function type (for interactive selection) - #[arg( - long = "type", - short = 't', - env = "BT_FUNCTIONS_UPDATE_TYPE", - value_enum - )] + #[arg(long = "type", short = 't', value_enum)] function_type: Option, } @@ -1122,6 +1121,26 @@ mod tests { assert!(err.to_string().contains("either --id or a slug")); } + #[test] + fn prompt_classifier_web_path_uses_scorers_page() { + let function = Function { + id: "fn_test_classifier".to_string(), + name: "Test classifier".to_string(), + slug: "test-classifier".to_string(), + project_id: "test-project".to_string(), + description: None, + function_type: Some("classifier".to_string()), + prompt_data: Some(serde_json::json!({"parser": {"choice": ["a", "b"]}})), + function_data: Some(serde_json::json!({"type": "prompt"})), + tags: None, + metadata: None, + created: None, + _xact_id: None, + }; + + assert_eq!(build_web_path(&function), "scorers/fn_test_classifier"); + } + #[test] fn function_selection_label_includes_slug_when_name_differs() { let function = Function { diff --git a/src/functions/prompt_config.rs b/src/functions/prompt_config.rs new file mode 100644 index 00000000..cc3ef6cc --- /dev/null +++ b/src/functions/prompt_config.rs @@ -0,0 +1,354 @@ +use std::collections::HashSet; + +use anyhow::{bail, Context, Result}; +use clap::{Args, ValueEnum}; +use serde_json::{json, Map, Number, Value}; + +use crate::utils::read_text_source; + +#[derive(Debug, Clone, Default, Args)] +pub(crate) struct PromptConfigArgs { + /// Sampling temperature. + #[arg(long, value_name = "NUMBER")] + temperature: Option, + + /// Maximum number of generated tokens. + #[arg(long, value_name = "N")] + max_tokens: Option, + + /// Nucleus sampling probability. + #[arg(long, value_name = "NUMBER")] + top_p: Option, + + /// Frequency penalty. + #[arg(long, value_name = "NUMBER", allow_hyphen_values = true)] + frequency_penalty: Option, + + /// Presence penalty. + #[arg(long, value_name = "NUMBER", allow_hyphen_values = true)] + presence_penalty: Option, + + /// Stop sequence. Repeat this flag to specify multiple sequences. + #[arg(long, value_name = "TEXT", action = clap::ArgAction::Append)] + stop_sequence: Vec, + + /// Tool choice: auto, none, required, or a specific function name. + #[arg(long, value_name = "CHOICE")] + tool_choice: Option, + + /// Reasoning effort for supported models. + #[arg(long, value_enum)] + reasoning_effort: Option, + + /// Response verbosity for supported models. + #[arg(long, value_enum)] + verbosity: Option, + + /// Prompt template syntax. Jinja is stored using Braintrust's `nunjucks` + /// format; `nunjucks` and `jinja2` are accepted aliases. + #[arg(long, value_enum, value_name = "FORMAT")] + template_format: Option, +} + +#[derive(Debug, Clone, Copy, ValueEnum)] +enum ReasoningEffort { + None, + Minimal, + Low, + Medium, + High, +} + +impl ReasoningEffort { + fn as_str(self) -> &'static str { + match self { + Self::None => "none", + Self::Minimal => "minimal", + Self::Low => "low", + Self::Medium => "medium", + Self::High => "high", + } + } +} + +#[derive(Debug, Clone, Copy, ValueEnum)] +enum Verbosity { + Low, + Medium, + High, +} + +impl Verbosity { + fn as_str(self) -> &'static str { + match self { + Self::Low => "low", + Self::Medium => "medium", + Self::High => "high", + } + } +} + +#[derive(Debug, Clone, Copy, ValueEnum)] +enum TemplateFormat { + Mustache, + #[value(name = "jinja", alias = "nunjucks", alias = "jinja2")] + Nunjucks, + None, +} + +impl TemplateFormat { + fn as_str(self) -> &'static str { + match self { + Self::Mustache => "mustache", + Self::Nunjucks => "nunjucks", + Self::None => "none", + } + } +} + +impl PromptConfigArgs { + /// Build a partial `prompt_data` object matching the app's prompt schema. + pub(crate) fn build_prompt_data_patch( + &self, + model: Option<&str>, + ) -> Result> { + let mut prompt_data = Map::new(); + let mut options = Map::new(); + let mut params = Map::new(); + + if let Some(model) = model { + let model = model.trim(); + if model.is_empty() { + bail!("--model cannot be empty"); + } + options.insert("model".to_string(), Value::String(model.to_string())); + } + + insert_optional_number(&mut params, "temperature", self.temperature)?; + if let Some(max_tokens) = self.max_tokens { + params.insert("max_tokens".to_string(), Value::Number(max_tokens.into())); + } + if let Some(top_p) = self.top_p { + validate_unit_interval(top_p, "--top-p")?; + insert_number(&mut params, "top_p", top_p, "--top-p")?; + } + insert_optional_number(&mut params, "frequency_penalty", self.frequency_penalty)?; + insert_optional_number(&mut params, "presence_penalty", self.presence_penalty)?; + + if !self.stop_sequence.is_empty() { + params.insert( + "stop".to_string(), + Value::Array( + self.stop_sequence + .iter() + .map(|value| Value::String(value.clone())) + .collect(), + ), + ); + } + + if let Some(tool_choice) = self.tool_choice.as_deref() { + let tool_choice = tool_choice.trim(); + if tool_choice.is_empty() { + bail!("--tool-choice cannot be empty"); + } + let value = match tool_choice { + "auto" | "none" | "required" => Value::String(tool_choice.to_string()), + function_name => json!({ + "type": "function", + "function": { "name": function_name }, + }), + }; + params.insert("tool_choice".to_string(), value); + } + + if let Some(reasoning_effort) = self.reasoning_effort { + params.insert( + "reasoning_effort".to_string(), + Value::String(reasoning_effort.as_str().to_string()), + ); + } + if let Some(verbosity) = self.verbosity { + params.insert( + "verbosity".to_string(), + Value::String(verbosity.as_str().to_string()), + ); + } + + if !params.is_empty() { + options.insert("params".to_string(), Value::Object(params)); + } + if !options.is_empty() { + prompt_data.insert("options".to_string(), Value::Object(options)); + } + if let Some(template_format) = self.template_format { + prompt_data.insert( + "template_format".to_string(), + Value::String(template_format.as_str().to_string()), + ); + } + + Ok(prompt_data) + } +} + +fn insert_optional_number( + target: &mut Map, + key: &str, + value: Option, +) -> Result<()> { + if let Some(value) = value { + insert_number(target, key, value, &format!("--{}", key.replace('_', "-")))?; + } + Ok(()) +} + +fn insert_number( + target: &mut Map, + key: &str, + value: f64, + label: &str, +) -> Result<()> { + let number = + Number::from_f64(value).ok_or_else(|| anyhow::anyhow!("{label} must be finite"))?; + target.insert(key.to_string(), Value::Number(number)); + Ok(()) +} + +pub(crate) fn validate_unit_interval(value: f64, label: &str) -> Result<()> { + if !value.is_finite() || !(0.0..=1.0).contains(&value) { + bail!("{label} must be between 0 and 1"); + } + Ok(()) +} + +pub(crate) fn parse_choice_scores_source(source: &str) -> Result> { + let raw = read_text_source(source, "choice scores")?; + let value: Value = serde_json::from_str(&raw).context("invalid JSON in choice scores")?; + let scores = match value { + Value::Object(scores) => scores, + _ => bail!("choice scores must be a JSON object mapping choices to numeric scores"), + }; + if scores.is_empty() { + bail!("choice scores cannot be empty"); + } + for (choice, score) in &scores { + if choice.trim().is_empty() { + bail!("choice score labels cannot be empty"); + } + let Some(score) = score.as_f64() else { + bail!("score for choice '{choice}' must be a number"); + }; + validate_unit_interval(score, &format!("score for choice '{choice}'"))?; + } + Ok(scores) +} + +pub(crate) fn parse_classifications_source(source: &str) -> Result> { + let raw = read_text_source(source, "classifications")?; + let value: Value = serde_json::from_str(&raw).context("invalid JSON in classifications")?; + let choices = match value { + Value::Array(choices) => choices, + _ => bail!("classifications must be a JSON array of strings"), + }; + if choices.is_empty() { + bail!("classifications cannot be empty"); + } + + let mut seen = HashSet::new(); + choices + .into_iter() + .map(|choice| { + let Value::String(choice) = choice else { + bail!("every classification must be a string"); + }; + let choice = choice.trim(); + if choice.is_empty() { + bail!("classifications cannot contain an empty label"); + } + if !seen.insert(choice.to_string()) { + bail!("classification labels must be unique; found '{choice}' more than once"); + } + Ok(Value::String(choice.to_string())) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use clap::Parser; + + use super::*; + + #[derive(Debug, Parser)] + struct Harness { + #[command(flatten)] + config: PromptConfigArgs, + } + + #[test] + fn builds_web_ui_compatible_prompt_configuration() { + let args = Harness::try_parse_from([ + "test", + "--temperature", + "0.2", + "--max-tokens", + "512", + "--top-p", + "0.9", + "--frequency-penalty", + "-0.5", + "--presence-penalty", + "0.25", + "--stop-sequence", + "END", + "--stop-sequence", + "DONE", + "--tool-choice", + "test_tool", + "--reasoning-effort", + "high", + "--verbosity", + "low", + "--template-format", + "jinja", + ]) + .expect("parse arguments"); + + let patch = args + .config + .build_prompt_data_patch(Some("gpt-test")) + .expect("prompt data"); + assert_eq!(patch["options"]["model"], "gpt-test"); + assert_eq!(patch["options"]["params"]["temperature"], 0.2); + assert_eq!(patch["options"]["params"]["max_tokens"], 512); + assert_eq!(patch["options"]["params"]["top_p"], 0.9); + assert_eq!(patch["options"]["params"]["frequency_penalty"], -0.5); + assert_eq!(patch["options"]["params"]["presence_penalty"], 0.25); + assert_eq!(patch["options"]["params"]["stop"], json!(["END", "DONE"])); + assert_eq!( + patch["options"]["params"]["tool_choice"], + json!({"type": "function", "function": {"name": "test_tool"}}) + ); + assert_eq!(patch["options"]["params"]["reasoning_effort"], "high"); + assert_eq!(patch["options"]["params"]["verbosity"], "low"); + assert_eq!(patch["template_format"], "nunjucks"); + } + + #[test] + fn validates_scores_against_api_range() { + let error = parse_choice_scores_source(r#"{"bad":1.5}"#) + .expect_err("out-of-range score should fail"); + assert!(error.to_string().contains("between 0 and 1")); + } + + #[test] + fn parses_unique_classification_labels() { + let choices = + parse_classifications_source(r#"["safe","unsafe"]"#).expect("classifications"); + assert_eq!( + choices, + json!(["safe", "unsafe"]).as_array().unwrap().clone() + ); + } +} diff --git a/src/functions/update.rs b/src/functions/update.rs index 5215476d..2d6c9a36 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -1,16 +1,23 @@ -use std::path::PathBuf; - use anyhow::{anyhow, bail, Context, Result}; -use clap::Args; +use clap::{builder::BoolishValueParser, Args}; use dialoguer::Confirm; use serde_json::{json, Map, Value}; -use crate::ui::{is_interactive, print_command_status, with_spinner, CommandStatus}; +use crate::{ + ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, + utils::{merge_json_objects, read_text_source, read_yaml_object_source}, +}; use super::{api, label, label_plural, select_function_interactive}; -use super::{FunctionTypeFilter, ResolvedContext}; - -/// Update a function (scorer, tool, prompt, ...) in place. +use super::{ + prompt_config::{ + parse_choice_scores_source, parse_classifications_source, validate_unit_interval, + PromptConfigArgs, + }, + FunctionTypeFilter, ResolvedContext, +}; + +/// Update a function's prompt configuration or metadata in place. /// /// This wraps `PATCH /v1/function/{id}`. The Braintrust API deep-merges object /// fields, so you can send just the nested fields you want to change (for @@ -19,94 +26,83 @@ use super::{FunctionTypeFilter, ResolvedContext}; #[derive(Debug, Clone, Args)] #[command(after_help = "\ Examples: - bt scorers update my-scorer --prompt-file judge.md - bt scorers update my-scorer --model gpt-4o-mini + bt scorers update my-scorer --messages @messages.json + bt scorers update my-scorer --model gpt-5.4-nano --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-4o-mini\"}}}' - bt scorers update --id fn_123 --patch-file scorer-patch.json - bt tools update my-tool --patch-file tool-patch.json + 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", env = "BT_FUNCTIONS_UPDATE_ID")] + #[arg(long = "id")] id: Option, - /// Replace the completion prompt text (LLM scorers/prompts). Writes - /// `prompt_data.prompt` as `{"type":"completion","content":}`. - /// Read from a file with --prompt-file, or stdin when no value is given - /// in a non-interactive shell. - #[arg( - long, - env = "BT_FUNCTIONS_UPDATE_PROMPT", - value_name = "TEXT", - conflicts_with_all = ["prompt_file", "messages"] - )] - prompt: Option, + /// Replacement chat messages source: inline JSON, @PATH to read from a + /// file, or - for stdin. + #[arg(long, value_name = "SOURCE")] + messages: Option, - /// Read the completion prompt text from a file. Mutually exclusive with --prompt. - #[arg( - long, - env = "BT_FUNCTIONS_UPDATE_PROMPT_FILE", - value_name = "PATH", - conflicts_with_all = ["prompt", "messages"] - )] - prompt_file: 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 the chat prompt messages (LLM scorers/prompts) as JSON. Writes - /// `prompt_data.prompt` as `{"type":"chat","messages":}`. + /// 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, - env = "BT_FUNCTIONS_UPDATE_MESSAGES", - value_name = "JSON", - conflicts_with_all = ["prompt", "prompt_file"] + num_args = 0..=1, + default_missing_value = "true", + value_parser = BoolishValueParser::new() )] - messages: Option, + use_cot: Option, - /// Update the model used by an LLM scorer/prompt. Writes - /// `prompt_data.options.model`. + /// Update whether a classifier may return no matching classification. #[arg( long, - short = 'm', - env = "BT_FUNCTIONS_UPDATE_MODEL", - value_name = "MODEL" + num_args = 0..=1, + default_missing_value = "true", + value_parser = BoolishValueParser::new() )] - model: Option, + 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', - env = "BT_FUNCTIONS_UPDATE_DESCRIPTION", - value_name = "TEXT" - )] + #[arg(long, short = 'd', value_name = "TEXT")] description: Option, - /// Arbitrary JSON object deep-merged into the function on patch. Use this - /// for fields without a dedicated flag (for example - /// `prompt_data.parser.choice_scores`, `prompt_data.parser.use_cot`, - /// `function_data`, `tags`, or `metadata`). - #[arg( - long, - env = "BT_FUNCTIONS_UPDATE_PATCH", - value_name = "JSON", - conflicts_with = "patch_file" - )] + /// 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, - /// Read the arbitrary patch JSON from a file. Mutually exclusive with --patch. - #[arg( - long, - env = "BT_FUNCTIONS_UPDATE_PATCH_FILE", - value_name = "PATH", - conflicts_with = "patch" - )] - patch_file: Option, - /// Skip the confirmation prompt. - #[arg(long, short = 'y', env = "BT_FUNCTIONS_UPDATE_YES", default_value_t = false, value_parser = clap::builder::BoolishValueParser::new())] + #[arg(long, short = 'y')] yes: bool, } @@ -233,24 +229,18 @@ fn build_patch_body(args: &UpdateArgs) -> Result { ); } - let prompt_text = resolve_prompt_text(args)?; + let metadata = resolve_metadata(args)?; + if !metadata.is_empty() { + patch.insert("metadata".to_string(), Value::Object(metadata)); + } + let messages_json = resolve_messages(args)?; - if prompt_text.is_some() || messages_json.is_some() { - let prompt_block = match (prompt_text, messages_json) { - (Some(text), None) => json!({ - "type": "completion", - "content": text, - }), - (None, Some(messages)) => json!({ - "type": "chat", - "messages": messages, - }), - (Some(_), Some(_)) => { - bail!("use either --prompt/--prompt-file or --messages, not both") - } - (None, None) => unreachable!("guarded above"), - }; + if let Some(messages) = messages_json { + let prompt_block = json!({ + "type": "chat", + "messages": messages, + }); let prompt_data = match patch.get("prompt_data") { Some(Value::Object(existing)) => { @@ -263,78 +253,113 @@ fn build_patch_body(args: &UpdateArgs) -> Result { patch.insert("prompt_data".to_string(), prompt_data); } - if let Some(model) = args.model.as_deref() { - let prompt_data = match patch.get("prompt_data") { - Some(Value::Object(existing)) => { - let mut merged = existing.clone(); - let options = match merged.get("options") { - Some(Value::Object(opts)) => opts.clone(), - _ => Map::new(), - }; - let mut options = options; - options.insert("model".to_string(), Value::String(model.to_string())); - merged.insert("options".to_string(), Value::Object(options)); - Value::Object(merged) - } - _ => json!({ "options": { "model": model } }), - }; - patch.insert("prompt_data".to_string(), prompt_data); + 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_objects(&mut patch, &extra_obj); + merge_json_objects(&mut patch, &extra_obj); } if patch.is_empty() { - bail!( - "no updates requested. Pass one of --prompt/--prompt-file, --messages, --model, --description, or --patch/--patch-file" - ); + bail!("no updates requested. Pass an update flag; see `bt scorers update --help`"); } Ok(Value::Object(patch)) } -fn resolve_prompt_text(args: &UpdateArgs) -> Result> { - match (&args.prompt, &args.prompt_file) { - (Some(_), Some(_)) => bail!("use either --prompt or --prompt-file, not both"), - (Some(text), None) => Ok(Some(text.clone())), - (None, Some(path)) => { - let content = std::fs::read_to_string(path) - .with_context(|| format!("failed to read prompt file {}", path.display()))?; - Ok(Some(content)) - } - (None, None) => { - // Allow piping the prompt text via stdin in a non-interactive shell. - if args.messages.is_some() - || args.model.is_some() - || args.description.is_some() - || args.patch.is_some() - || args.patch_file.is_some() - { - return Ok(None); - } - if !is_interactive() { - use std::io::Read; - let mut buf = String::new(); - std::io::stdin() - .read_to_string(&mut buf) - .context("failed to read prompt from stdin")?; - let trimmed = buf.trim(); - if trimmed.is_empty() { - return Ok(None); - } - return Ok(Some(trimmed.to_string())); - } - Ok(None) - } +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 { - Some(raw) => { - let parsed: Value = serde_json::from_str(raw).context("invalid JSON in --messages")?; + 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"), @@ -345,78 +370,58 @@ fn resolve_messages(args: &UpdateArgs) -> Result> { } fn resolve_extra_patch(args: &UpdateArgs) -> Result>> { - match (&args.patch, &args.patch_file) { - (Some(_), Some(_)) => bail!("use either --patch or --patch-file, not both"), - (Some(raw), None) => parse_patch_object(raw).map(Some), - (None, Some(path)) => { - let content = std::fs::read_to_string(path) - .with_context(|| format!("failed to read patch file {}", path.display()))?; - parse_patch_object(&content).map(Some) - } - (None, None) => Ok(None), - } + 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/--patch-file")?; + let value: Value = serde_json::from_str(raw).context("invalid JSON in --patch")?; match value { Value::Object(map) => Ok(map), - _ => bail!("--patch/--patch-file must be a JSON object"), - } -} - -fn merge_objects(target: &mut Map, source: &Map) { - for (key, value) in source { - match (target.get_mut(key), value) { - (Some(Value::Object(target_inner)), Value::Object(source_inner)) => { - merge_objects(target_inner, source_inner); - } - _ => { - target.insert(key.clone(), value.clone()); - } - } + _ => bail!("--patch must be a JSON object"), } } #[cfg(test)] mod tests { + use clap::Parser; + use super::*; - fn args(prompt: Option<&str>, model: Option<&str>, description: Option<&str>) -> UpdateArgs { + #[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, - prompt: prompt.map(ToOwned::to_owned), - prompt_file: 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, - patch_file: None, yes: true, } } - #[test] - fn build_patch_body_prompt_writes_completion_block() { - let args = args(Some("Grade the answer."), None, None); - let body = build_patch_body(&args).expect("patch body"); - assert_eq!( - body["prompt_data"]["prompt"]["type"], - serde_json::json!("completion") - ); - assert_eq!( - body["prompt_data"]["prompt"]["content"], - serde_json::json!("Grade the answer.") - ); - } - #[test] fn build_patch_body_messages_writes_chat_block() { - let mut args = args(None, None, None); + 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!( @@ -431,7 +436,7 @@ mod tests { #[test] fn build_patch_body_model_merges_into_prompt_data() { - let args = args(None, Some("gpt-4o-mini"), None); + let args = args(Some("gpt-4o-mini"), None); let body = build_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["options"]["model"], @@ -440,10 +445,14 @@ mod tests { } #[test] - fn build_patch_body_prompt_and_model_combine() { - let args = args(Some("Grade it."), Some("gpt-4o-mini"), None); + 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"]["content"], "Grade it."); + 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") @@ -451,47 +460,134 @@ mod tests { } #[test] - fn build_patch_body_description_is_top_level() { - let args = args(None, None, Some("Helpfulness judge")); + 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["description"], serde_json::json!("Helpfulness judge")); + 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_rejects_prompt_and_messages_together() { - let mut args = args(Some("Grade it."), None, None); - args.messages = Some(r#"[{"role":"user","content":"hi"}]"#.to_string()); - let err = build_patch_body(&args).expect_err("should reject"); - assert!(err.to_string().contains("not both")); + 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_rejects_prompt_and_prompt_file_together() { - let mut args = args(Some("Grade it."), None, None); - args.prompt_file = Some(PathBuf::from("/tmp/ignore.md")); - let err = build_patch_body(&args).expect_err("should reject"); - assert!(err.to_string().contains("not both")); + 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_rejects_patch_and_patch_file_together() { - let mut args = args(None, None, None); - args.patch = Some("{}".to_string()); - args.patch_file = Some(PathBuf::from("/tmp/ignore.json")); - let err = build_patch_body(&args).expect_err("should reject"); - assert!(err.to_string().contains("not both")); + 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, None); + 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, None); + 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!( @@ -521,7 +617,7 @@ mod tests { .expect("object") .clone(); - merge_objects(&mut target, &source); + merge_json_objects(&mut target, &source); assert_eq!( target["prompt_data"]["options"]["model"], diff --git a/src/prompts/mod.rs b/src/prompts/mod.rs index 04d46157..bfe6bf41 100644 --- a/src/prompts/mod.rs +++ b/src/prompts/mod.rs @@ -17,7 +17,7 @@ Examples: bt prompts list bt prompts view my-prompt bt prompts delete my-prompt - bt prompts update my-prompt --prompt-file prompt.md + bt prompts update my-prompt --messages @messages.json ")] pub struct PromptsArgs { #[command(subcommand)] @@ -30,8 +30,8 @@ enum PromptsCommands { List, /// View a prompt's content View(ViewArgs), - /// Update a prompt in place (prompt text, model, description, or arbitrary patch) - Update(update::UpdateArgs), + /// Update a prompt in place (prompt configuration, metadata, or arbitrary patch) + Update(Box), /// Delete a prompt Delete(DeleteArgs), } diff --git a/src/prompts/update.rs b/src/prompts/update.rs index b21eb4b0..cbd997ac 100644 --- a/src/prompts/update.rs +++ b/src/prompts/update.rs @@ -1,15 +1,17 @@ -use std::path::PathBuf; - use anyhow::{anyhow, bail, Context, Result}; use clap::Args; use dialoguer::Confirm; use serde_json::{json, Map, Value}; -use crate::ui::{is_interactive, print_command_status, with_spinner, CommandStatus}; +use crate::{ + functions::prompt_config::PromptConfigArgs, + ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, + utils::{merge_json_objects, read_text_source, read_yaml_object_source}, +}; use super::{api, ResolvedContext}; -/// Update a prompt in place via `PATCH /v1/prompt/{id}`. +/// Update a prompt's configuration or metadata in place. /// /// The Braintrust API deep-merges object fields, so you can send just the /// nested fields you want to change (for example `prompt_data.prompt`) without @@ -17,11 +19,11 @@ use super::{api, ResolvedContext}; #[derive(Debug, Clone, Args)] #[command(after_help = "\ Examples: - bt prompts update my-prompt --prompt-file prompt.md - bt prompts update my-prompt --model gpt-4o-mini + bt prompts update my-prompt --messages @messages.json + bt prompts update my-prompt --model gpt-5.4-nano bt prompts update my-prompt --description \"Customer support prompt\" - bt prompts update my-prompt --patch '{\"prompt_data\":{\"options\":{\"model\":\"gpt-4o-mini\"}}}' - bt prompts update my-prompt --patch-file prompt-patch.json + bt prompts update my-prompt --patch '{\"prompt_data\":{\"options\":{\"model\":\"gpt-5.4-nano\"}}}' + bt prompts update my-prompt --patch @prompt-patch.json ")] pub struct UpdateArgs { /// Prompt slug (positional) @@ -29,79 +31,36 @@ pub struct UpdateArgs { slug_positional: Option, /// Prompt slug (flag) - #[arg(long = "slug", short = 's', env = "BT_PROMPTS_UPDATE_SLUG")] + #[arg(long = "slug", short = 's')] slug_flag: Option, - /// Replace the completion prompt text. Writes `prompt_data.prompt` as - /// `{"type":"completion","content":}`. Read from a file with - /// --prompt-file, or stdin when no value is given in a non-interactive shell. - #[arg( - long, - env = "BT_PROMPTS_UPDATE_PROMPT", - value_name = "TEXT", - conflicts_with_all = ["prompt_file", "messages"] - )] - prompt: Option, - - /// Read the completion prompt text from a file. Mutually exclusive with --prompt. - #[arg( - long, - env = "BT_PROMPTS_UPDATE_PROMPT_FILE", - value_name = "PATH", - conflicts_with_all = ["prompt", "messages"] - )] - prompt_file: Option, - - /// Replace the chat prompt messages (JSON array). Writes - /// `prompt_data.prompt` as `{"type":"chat","messages":}`. - #[arg( - long, - env = "BT_PROMPTS_UPDATE_MESSAGES", - value_name = "JSON", - conflicts_with_all = ["prompt", "prompt_file"] - )] + /// 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 the prompt. Writes `prompt_data.options.model`. - #[arg( - long, - short = 'm', - env = "BT_PROMPTS_UPDATE_MODEL", - value_name = "MODEL" - )] + /// Update the model used by the prompt. + #[arg(long, short = 'm', value_name = "MODEL")] model: Option, + #[command(flatten)] + prompt_config: PromptConfigArgs, + + /// Deep-merge metadata from inline YAML, @PATH, or stdin (-). + #[arg(long, value_name = "SOURCE")] + metadata: Option, + /// Update the prompt description. - #[arg( - long, - short = 'd', - env = "BT_PROMPTS_UPDATE_DESCRIPTION", - value_name = "TEXT" - )] + #[arg(long, short = 'd', value_name = "TEXT")] description: Option, - /// Arbitrary JSON object deep-merged into the prompt on patch. Use this - /// for fields without a dedicated flag (for example `tags`, `metadata`, - /// or nested `prompt_data.options.params`). - #[arg( - long, - env = "BT_PROMPTS_UPDATE_PATCH", - value_name = "JSON", - conflicts_with = "patch_file" - )] + /// Arbitrary JSON object deep-merged into the prompt. Accepts inline JSON, + /// @PATH to read JSON from a file, or - for stdin. + #[arg(long, value_name = "SOURCE")] patch: Option, - /// Read the arbitrary patch JSON from a file. Mutually exclusive with --patch. - #[arg( - long, - env = "BT_PROMPTS_UPDATE_PATCH_FILE", - value_name = "PATH", - conflicts_with = "patch" - )] - patch_file: Option, - /// Skip the confirmation prompt. - #[arg(long, short = 'y', env = "BT_PROMPTS_UPDATE_YES", default_value_t = false, value_parser = clap::builder::BoolishValueParser::new())] + #[arg(long, short = 'y')] yes: bool, } @@ -189,24 +148,20 @@ fn build_patch_body(args: &UpdateArgs) -> Result { ); } - let prompt_text = resolve_prompt_text(args)?; + if let Some(source) = args.metadata.as_deref() { + patch.insert( + "metadata".to_string(), + Value::Object(read_yaml_object_source(source, "prompt metadata")?), + ); + } + let messages_json = resolve_messages(args)?; - if prompt_text.is_some() || messages_json.is_some() { - let prompt_block = match (prompt_text, messages_json) { - (Some(text), None) => json!({ - "type": "completion", - "content": text, - }), - (None, Some(messages)) => json!({ - "type": "chat", - "messages": messages, - }), - (Some(_), Some(_)) => { - bail!("use either --prompt/--prompt-file or --messages, not both") - } - (None, None) => unreachable!("guarded above"), - }; + if let Some(messages) = messages_json { + let prompt_block = json!({ + "type": "chat", + "messages": messages, + }); let prompt_data = match patch.get("prompt_data") { Some(Value::Object(existing)) => { @@ -219,77 +174,36 @@ fn build_patch_body(args: &UpdateArgs) -> Result { patch.insert("prompt_data".to_string(), prompt_data); } - if let Some(model) = args.model.as_deref() { - let prompt_data = match patch.get("prompt_data") { - Some(Value::Object(existing)) => { - let mut merged = existing.clone(); - let options = match merged.get("options") { - Some(Value::Object(opts)) => opts.clone(), - _ => Map::new(), - }; - let mut options = options; - options.insert("model".to_string(), Value::String(model.to_string())); - merged.insert("options".to_string(), Value::Object(options)); - Value::Object(merged) - } - _ => json!({ "options": { "model": model } }), - }; - patch.insert("prompt_data".to_string(), prompt_data); + 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_objects(&mut patch, &extra_obj); + merge_json_objects(&mut patch, &extra_obj); } if patch.is_empty() { - bail!( - "no updates requested. Pass one of --prompt/--prompt-file, --messages, --model, --description, or --patch/--patch-file" - ); + bail!("no updates requested. Pass an update flag; see `bt prompts update --help`"); } Ok(Value::Object(patch)) } -fn resolve_prompt_text(args: &UpdateArgs) -> Result> { - match (&args.prompt, &args.prompt_file) { - (Some(_), Some(_)) => bail!("use either --prompt or --prompt-file, not both"), - (Some(text), None) => Ok(Some(text.clone())), - (None, Some(path)) => { - let content = std::fs::read_to_string(path) - .with_context(|| format!("failed to read prompt file {}", path.display()))?; - Ok(Some(content)) - } - (None, None) => { - if args.messages.is_some() - || args.model.is_some() - || args.description.is_some() - || args.patch.is_some() - || args.patch_file.is_some() - { - return Ok(None); - } - if !is_interactive() { - use std::io::Read; - let mut buf = String::new(); - std::io::stdin() - .read_to_string(&mut buf) - .context("failed to read prompt from stdin")?; - let trimmed = buf.trim(); - if trimmed.is_empty() { - return Ok(None); - } - return Ok(Some(trimmed.to_string())); - } - Ok(None) - } - } -} - fn resolve_messages(args: &UpdateArgs) -> Result> { - match &args.messages { - Some(raw) => { - let parsed: Value = serde_json::from_str(raw).context("invalid JSON in --messages")?; + 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"), @@ -300,75 +214,50 @@ fn resolve_messages(args: &UpdateArgs) -> Result> { } fn resolve_extra_patch(args: &UpdateArgs) -> Result>> { - match (&args.patch, &args.patch_file) { - (Some(_), Some(_)) => bail!("use either --patch or --patch-file, not both"), - (Some(raw), None) => parse_patch_object(raw).map(Some), - (None, Some(path)) => { - let content = std::fs::read_to_string(path) - .with_context(|| format!("failed to read patch file {}", path.display()))?; - parse_patch_object(&content).map(Some) - } - (None, None) => Ok(None), - } + 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/--patch-file")?; + let value: Value = serde_json::from_str(raw).context("invalid JSON in --patch")?; match value { Value::Object(map) => Ok(map), - _ => bail!("--patch/--patch-file must be a JSON object"), - } -} - -fn merge_objects(target: &mut Map, source: &Map) { - for (key, value) in source { - match (target.get_mut(key), value) { - (Some(Value::Object(target_inner)), Value::Object(source_inner)) => { - merge_objects(target_inner, source_inner); - } - _ => { - target.insert(key.clone(), value.clone()); - } - } + _ => bail!("--patch must be a JSON object"), } } #[cfg(test)] mod tests { + use clap::Parser; + use super::*; - fn args(prompt: Option<&str>, model: Option<&str>, description: Option<&str>) -> UpdateArgs { + #[derive(Debug, Parser)] + struct UpdateArgsHarness { + #[command(flatten)] + args: UpdateArgs, + } + + fn args(model: Option<&str>, description: Option<&str>) -> UpdateArgs { UpdateArgs { slug_positional: Some("test-prompt".to_string()), slug_flag: None, - prompt: prompt.map(ToOwned::to_owned), - prompt_file: None, messages: None, model: model.map(ToOwned::to_owned), + prompt_config: PromptConfigArgs::default(), + metadata: None, description: description.map(ToOwned::to_owned), patch: None, - patch_file: None, yes: true, } } - #[test] - fn build_patch_body_prompt_writes_completion_block() { - let args = args(Some("Answer the question."), None, None); - let body = build_patch_body(&args).expect("patch body"); - assert_eq!( - body["prompt_data"]["prompt"]["type"], - serde_json::json!("completion") - ); - assert_eq!( - body["prompt_data"]["prompt"]["content"], - serde_json::json!("Answer the question.") - ); - } - #[test] fn build_patch_body_messages_writes_chat_block() { - let mut args = args(None, None, None); + 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!( @@ -383,7 +272,7 @@ mod tests { #[test] fn build_patch_body_model_merges_into_prompt_data() { - let args = args(None, Some("gpt-4o-mini"), None); + let args = args(Some("gpt-4o-mini"), None); let body = build_patch_body(&args).expect("patch body"); assert_eq!( body["prompt_data"]["options"]["model"], @@ -392,19 +281,46 @@ mod tests { } #[test] - fn build_patch_body_prompt_and_model_combine() { - let args = args(Some("Answer it."), Some("gpt-4o-mini"), None); + fn build_patch_body_messages_and_model_combine() { + let mut args = args(Some("gpt-4o-mini"), None); + args.messages = Some(r#"[{"role":"user","content":"Answer it."}]"#.to_string()); let body = build_patch_body(&args).expect("patch body"); - assert_eq!(body["prompt_data"]["prompt"]["content"], "Answer it."); + assert_eq!( + body["prompt_data"]["prompt"]["messages"], + json!([{"role": "user", "content": "Answer it."}]) + ); assert_eq!( body["prompt_data"]["options"]["model"], serde_json::json!("gpt-4o-mini") ); } + #[test] + fn build_patch_body_updates_prompt_configuration_and_metadata() { + let parsed = UpdateArgsHarness::try_parse_from([ + "test", + "test-prompt", + "--temperature", + "0.3", + "--max-tokens", + "100", + "--template-format", + "mustache", + "--metadata", + "owner: test-team", + ]) + .expect("parse update"); + + let body = build_patch_body(&parsed.args).expect("patch body"); + assert_eq!(body["prompt_data"]["options"]["params"]["temperature"], 0.3); + assert_eq!(body["prompt_data"]["options"]["params"]["max_tokens"], 100); + assert_eq!(body["prompt_data"]["template_format"], "mustache"); + assert_eq!(body["metadata"]["owner"], "test-team"); + } + #[test] fn build_patch_body_description_is_top_level() { - let args = args(None, None, Some("Customer support prompt")); + let args = args(None, Some("Customer support prompt")); let body = build_patch_body(&args).expect("patch body"); assert_eq!( body["description"], @@ -413,40 +329,48 @@ mod tests { } #[test] - fn build_patch_body_rejects_prompt_and_messages_together() { - let mut args = args(Some("Answer it."), None, None); - args.messages = Some(r#"[{"role":"user","content":"hi"}]"#.to_string()); - let err = build_patch_body(&args).expect_err("should reject"); - assert!(err.to_string().contains("not both")); - } + 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":"Answer from a file."}]"#, + ) + .expect("write messages"); + let source = format!("@{}", path.display()); - #[test] - fn build_patch_body_rejects_prompt_and_prompt_file_together() { - let mut args = args(Some("Answer it."), None, None); - args.prompt_file = Some(PathBuf::from("/tmp/ignore.md")); - let err = build_patch_body(&args).expect_err("should reject"); - assert!(err.to_string().contains("not both")); + 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": "Answer from a file."}]) + ); } #[test] - fn build_patch_body_rejects_patch_and_patch_file_together() { - let mut args = args(None, None, None); - args.patch = Some("{}".to_string()); - args.patch_file = Some(PathBuf::from("/tmp/ignore.json")); - let err = build_patch_body(&args).expect_err("should reject"); - assert!(err.to_string().contains("not both")); + 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, None); + 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, None); + let mut args = args(None, None); args.patch = Some(r#"{"prompt_data":{"options":{"params":{"temperature":0}}}}"#.to_string()); let body = build_patch_body(&args).expect("patch body"); @@ -477,7 +401,7 @@ mod tests { .expect("object") .clone(); - merge_objects(&mut target, &source); + merge_json_objects(&mut target, &source); assert_eq!( target["prompt_data"]["options"]["model"], diff --git a/src/scorers.rs b/src/scorers.rs index ada1f490..e9b6a167 100644 --- a/src/scorers.rs +++ b/src/scorers.rs @@ -9,10 +9,16 @@ use crate::functions::{self, FunctionCommands, FunctionTypeFilter}; Examples: bt scorers list bt scorers view my-scorer - bt scorers create \"Helpfulness\" --model gpt-4o-mini --prompt-file judge.md \\ - --choice-scores '{\"A\":1,\"B\":0}' --use-cot - bt scorers update my-scorer --prompt-file judge.md + 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: + TypeScript: projects.create({ name: \"test-project\" }).scorers.create({...}) + bt functions push scorer.ts + Python: projects.create(\"test-project\").scorers.create(...) + bt functions push scorer.py ")] pub struct ScorersArgs { #[command(subcommand)] @@ -21,15 +27,15 @@ pub struct ScorersArgs { #[derive(Debug, Clone, Subcommand)] enum ScorersCommands { - /// Create an LLM scorer - Create(functions::create::CreateArgs), + /// Create an LLM scorer or classifier + Create(Box), #[command(flatten)] Function(FunctionCommands), } 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::Create(create)) => functions::run_scorer_create(base, *create).await, Some(ScorersCommands::Function(command)) => { functions::run_typed_command(base, Some(command), FunctionTypeFilter::Scorer).await } @@ -57,8 +63,8 @@ mod tests { "Test scorer", "--model", "gpt-test", - "--prompt", - "Judge {{output}}", + "--messages", + r#"[{"role":"user","content":"Judge {{output}}"}]"#, "--choice-scores", r#"{"yes":1,"no":0}"#, "--use-cot=false", @@ -73,6 +79,28 @@ mod tests { )); } + #[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([ diff --git a/src/utils/json_object.rs b/src/utils/json_object.rs index 20bc850c..4f2ee3d8 100644 --- a/src/utils/json_object.rs +++ b/src/utils/json_object.rs @@ -1,5 +1,18 @@ use serde_json::{Map, Value}; +pub(crate) fn merge_json_objects(target: &mut Map, source: &Map) { + for (key, value) in source { + match (target.get_mut(key), value) { + (Some(Value::Object(target_inner)), Value::Object(source_inner)) => { + merge_json_objects(target_inner, source_inner); + } + _ => { + target.insert(key.clone(), value.clone()); + } + } + } +} + pub(crate) fn lookup_object_path<'a, P>( object: &'a Map, path: &[P], @@ -20,6 +33,27 @@ mod tests { use super::*; + #[test] + fn merge_json_objects_deep_merges_nested_maps() { + let mut target = json!({ + "prompt_data": { "options": { "model": "gpt-test" } } + }) + .as_object() + .expect("object") + .clone(); + let source = json!({ + "prompt_data": { "options": { "params": { "temperature": 0 } } } + }) + .as_object() + .expect("object") + .clone(); + + merge_json_objects(&mut target, &source); + + assert_eq!(target["prompt_data"]["options"]["model"], "gpt-test"); + assert_eq!(target["prompt_data"]["options"]["params"]["temperature"], 0); + } + #[test] fn lookup_object_path_finds_nested_values() { let object = json!({ diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 05429ee0..bc22541d 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -6,12 +6,16 @@ mod ids; mod json_object; mod plurals; mod profile; +mod structured_source; +mod text_source; pub(crate) use app_url::{app_project_url, app_project_url_with_encoded_path}; pub use duration::parse_duration_to_seconds; pub use fs_atomic::{write_bytes_atomic, write_text_atomic}; pub use git::GitRepo; pub(crate) use ids::new_uuid_id; -pub(crate) use json_object::lookup_object_path; +pub(crate) use json_object::{lookup_object_path, merge_json_objects}; pub use plurals::pluralize; pub(crate) use profile::{profile_author_slug, resolve_profile_info, sanitize_name_segment}; +pub(crate) use structured_source::read_yaml_object_source; +pub(crate) use text_source::read_text_source; diff --git a/src/utils/structured_source.rs b/src/utils/structured_source.rs new file mode 100644 index 00000000..ab7cab8c --- /dev/null +++ b/src/utils/structured_source.rs @@ -0,0 +1,39 @@ +use anyhow::{bail, Context, Result}; +use serde_json::{Map, Value}; + +use super::read_text_source; + +pub(crate) fn read_yaml_object_source( + source: &str, + description: &str, +) -> Result> { + let raw = read_text_source(source, description)?; + let value: Value = + serde_yaml::from_str(&raw).with_context(|| format!("invalid YAML in {description}"))?; + match value { + Value::Object(object) => Ok(object), + _ => bail!("{description} must be a YAML mapping/object"), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_inline_yaml_object() { + let value = + read_yaml_object_source("owner: test-team\nsettings:\n enabled: true\n", "metadata") + .expect("metadata"); + + assert_eq!(value["owner"], "test-team"); + assert_eq!(value["settings"]["enabled"], true); + } + + #[test] + fn rejects_yaml_array() { + let error = + read_yaml_object_source("- one\n- two\n", "metadata").expect_err("array should fail"); + assert!(error.to_string().contains("mapping/object")); + } +} diff --git a/src/utils/text_source.rs b/src/utils/text_source.rs new file mode 100644 index 00000000..8d6261f5 --- /dev/null +++ b/src/utils/text_source.rs @@ -0,0 +1,70 @@ +use std::io::Read; + +use anyhow::{bail, Context, Result}; + +/// Resolve inline text, an `@PATH` file reference, or `-` for stdin. +/// +/// A leading literal `@` can be escaped as `@@`. +pub(crate) fn read_text_source(value: &str, label: &str) -> Result { + if value == "-" { + let mut content = String::new(); + std::io::stdin() + .read_to_string(&mut content) + .with_context(|| format!("failed to read {label} from stdin"))?; + return Ok(content); + } + + if let Some(literal) = value.strip_prefix("@@") { + return Ok(format!("@{literal}")); + } + + if let Some(path) = value.strip_prefix('@') { + if path.is_empty() { + bail!("{label} file path cannot be empty after '@'"); + } + return std::fs::read_to_string(path) + .with_context(|| format!("failed to read {label} file {path}")); + } + + Ok(value.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn returns_inline_text() { + assert_eq!( + read_text_source("Judge the answer.", "prompt").expect("inline prompt"), + "Judge the answer." + ); + } + + #[test] + fn reads_at_prefixed_file() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("judge.md"); + std::fs::write(&path, "Judge from a file.\n").expect("write prompt"); + + let source = format!("@{}", path.display()); + assert_eq!( + read_text_source(&source, "prompt").expect("file prompt"), + "Judge from a file.\n" + ); + } + + #[test] + fn double_at_escapes_literal_at() { + assert_eq!( + read_text_source("@@mention", "prompt").expect("literal prompt"), + "@mention" + ); + } + + #[test] + fn rejects_empty_file_reference() { + let error = read_text_source("@", "prompt").expect_err("empty path should fail"); + assert!(error.to_string().contains("cannot be empty")); + } +} diff --git a/tests/cli.rs b/tests/cli.rs index 7ef2369c..fb37251e 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -110,16 +110,33 @@ fn top_level_help_shows_update_not_self() { } #[test] -fn scorers_create_help_includes_llm_judge_flags() { +fn scorers_create_help_includes_llm_judge_configuration() { bt_command() .args(["scorers", "create", "--help"]) .assert() .success() - .stdout(predicate::str::contains("--prompt-file")) + .stdout(predicate::str::contains("--messages ")) .stdout(predicate::str::contains("--model")) + .stdout(predicate::str::contains("--temperature")) + .stdout(predicate::str::contains("--max-tokens")) + .stdout(predicate::str::contains("--top-p")) + .stdout(predicate::str::contains("--frequency-penalty")) + .stdout(predicate::str::contains("--presence-penalty")) + .stdout(predicate::str::contains("--stop-sequence")) + .stdout(predicate::str::contains("--tool-choice")) + .stdout(predicate::str::contains("--reasoning-effort")) + .stdout(predicate::str::contains("--verbosity")) + .stdout(predicate::str::contains("--template-format")) .stdout(predicate::str::contains("--choice-scores")) + .stdout(predicate::str::contains("--classifications")) .stdout(predicate::str::contains("--use-cot")) - .stdout(predicate::str::contains("--if-exists")); + .stdout(predicate::str::contains("--pass-threshold")) + .stdout(predicate::str::contains("--metadata")) + .stdout(predicate::str::contains("--if-exists")) + .stdout(predicate::str::contains("TypeScript: projects.create")) + .stdout(predicate::str::contains("Python: projects.create")) + .stdout(predicate::str::contains("bt functions push scorer.ts")) + .stdout(predicate::str::contains("bt functions push scorer.py")); } #[test] @@ -128,15 +145,24 @@ fn scorer_and_prompt_update_help_is_conflict_free() { .args(["scorers", "update", "--help"]) .assert() .success() - .stdout(predicate::str::contains("--patch-file")) - .stdout(predicate::str::contains("--prompt-file")); + .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")); bt_command() .args(["prompts", "update", "--help"]) .assert() .success() - .stdout(predicate::str::contains("--patch-file")) - .stdout(predicate::str::contains("--prompt-file")); + .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("--template-format")) + .stdout(predicate::str::contains("--metadata")); } #[test] From bfe8f87be4175b5c3205f91ed0aa68c9a43451bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Tue, 28 Jul 2026 14:07:13 -0700 Subject: [PATCH 3/7] chore: nits --- Cargo.lock | 21 ++++++++- Cargo.toml | 2 +- src/functions/update.rs | 82 ++++++++++++++++++++++++++++++++++ src/utils/structured_source.rs | 2 +- 4 files changed, 104 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c7134b75..0b7bb77e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -531,7 +531,6 @@ dependencies = [ "serde", "serde_json 1.0.149", "serde_path_to_error", - "serde_yaml", "sha2", "strip-ansi-escapes", "tempfile", @@ -541,6 +540,7 @@ dependencies = [ "urlencoding", "uuid", "windows-sys 0.59.0", + "yaml_serde", ] [[package]] @@ -1812,6 +1812,12 @@ dependencies = [ "libc", ] +[[package]] +name = "libyaml-rs" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e126dda6f34391ab7b444f9922055facc83c07a910da3eb16f1e4d9c45dc777" + [[package]] name = "lingua" version = "0.1.0" @@ -4132,6 +4138,19 @@ version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9edde0db4769d2dc68579893f2306b26c6ecfbe0ef499b013d731b7b9247e0b9" +[[package]] +name = "yaml_serde" +version = "0.10.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08c7c1b1a6a7c8a6b2741a6c21a4f8918e51899b111cfa08d1288202656e3975" +dependencies = [ + "indexmap 2.13.0", + "itoa", + "libyaml-rs", + "ryu", + "serde", +] + [[package]] name = "yoke" version = "0.8.1" diff --git a/Cargo.toml b/Cargo.toml index e7889160..0013e079 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,7 +28,7 @@ reqwest = { version = "0.12.7", default-features = false, features = ["json", "r serde = { version = "1.0.210", features = ["derive"] } serde_json = "1.0.128" serde_path_to_error = "0.1.20" -serde_yaml = "0.9" +yaml_serde = "0.10" toml = "0.8" sha2 = "0.10.8" strip-ansi-escapes = "0.2.0" diff --git a/src/functions/update.rs b/src/functions/update.rs index 2d6c9a36..157e6a2c 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -107,6 +107,30 @@ pub struct UpdateArgs { } 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(), @@ -141,6 +165,41 @@ pub async fn run( 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, + ); + } + + // Switching output mode updates function_type, but the API deep-merges + // prompt_data.parser and will 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!( @@ -419,6 +478,29 @@ mod tests { } } + #[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); diff --git a/src/utils/structured_source.rs b/src/utils/structured_source.rs index ab7cab8c..fd5e0818 100644 --- a/src/utils/structured_source.rs +++ b/src/utils/structured_source.rs @@ -9,7 +9,7 @@ pub(crate) fn read_yaml_object_source( ) -> Result> { let raw = read_text_source(source, description)?; let value: Value = - serde_yaml::from_str(&raw).with_context(|| format!("invalid YAML in {description}"))?; + yaml_serde::from_str(&raw).with_context(|| format!("invalid YAML in {description}"))?; match value { Value::Object(object) => Ok(object), _ => bail!("{description} must be a YAML mapping/object"), From 696295eb0d61666d3664f8f0e775e2a9c1f024c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Tue, 28 Jul 2026 15:04:30 -0700 Subject: [PATCH 4/7] fix(scorers): model parameter validation --- src/functions/prompt_config.rs | 279 ++++++++++++++++++++++++++++++++- src/functions/update.rs | 7 +- src/prompts/update.rs | 4 +- 3 files changed, 280 insertions(+), 10 deletions(-) diff --git a/src/functions/prompt_config.rs b/src/functions/prompt_config.rs index cc3ef6cc..134e3157 100644 --- a/src/functions/prompt_config.rs +++ b/src/functions/prompt_config.rs @@ -4,27 +4,28 @@ use anyhow::{bail, Context, Result}; use clap::{Args, ValueEnum}; use serde_json::{json, Map, Number, Value}; -use crate::utils::read_text_source; +use crate::utils::{merge_json_objects, read_text_source}; #[derive(Debug, Clone, Default, Args)] pub(crate) struct PromptConfigArgs { - /// Sampling temperature. + /// Sampling temperature, between 0 and 2. Some models support a smaller + /// range or do not support custom temperatures. #[arg(long, value_name = "NUMBER")] temperature: Option, - /// Maximum number of generated tokens. + /// Maximum number of generated tokens. Must be greater than 0. #[arg(long, value_name = "N")] max_tokens: Option, - /// Nucleus sampling probability. + /// Nucleus sampling probability, between 0 and 1. #[arg(long, value_name = "NUMBER")] top_p: Option, - /// Frequency penalty. + /// Frequency penalty, between -2 and 2. #[arg(long, value_name = "NUMBER", allow_hyphen_values = true)] frequency_penalty: Option, - /// Presence penalty. + /// Presence penalty, between -2 and 2. #[arg(long, value_name = "NUMBER", allow_hyphen_values = true)] presence_penalty: Option, @@ -175,6 +176,9 @@ impl PromptConfigArgs { ); } + let effective_model = options.get("model").and_then(Value::as_str); + validate_model_params(effective_model, ¶ms)?; + if !params.is_empty() { options.insert("params".to_string(), Value::Object(params)); } @@ -192,6 +196,156 @@ impl PromptConfigArgs { } } +/// Validate the effective model configuration produced by deep-merging a patch +/// into an existing prompt definition. +/// +/// The API accepts partial prompt updates without checking provider-specific +/// model constraints. Validate whenever an update touches the model or its +/// parameters so a successful PATCH cannot leave a scorer or prompt unusable. +pub(crate) fn validate_prompt_data_patch( + existing_prompt_data: Option<&Value>, + patch: &Value, +) -> Result<()> { + let Some(patch_prompt_data) = patch.get("prompt_data").and_then(Value::as_object) else { + return Ok(()); + }; + if !patch_prompt_data.contains_key("options") { + return Ok(()); + } + + let mut effective_prompt_data = existing_prompt_data + .and_then(Value::as_object) + .cloned() + .unwrap_or_default(); + merge_json_objects(&mut effective_prompt_data, patch_prompt_data); + + let Some(options) = effective_prompt_data.get("options") else { + return Ok(()); + }; + if options.is_null() { + return Ok(()); + } + let options = options + .as_object() + .ok_or_else(|| anyhow::anyhow!("prompt_data.options must be a JSON object"))?; + + let model = match options.get("model") { + Some(Value::String(model)) if !model.trim().is_empty() => Some(model.trim()), + Some(Value::String(_)) => bail!("model cannot be empty"), + Some(Value::Null) | None => None, + Some(_) => bail!("prompt_data.options.model must be a string"), + }; + let params = match options.get("params") { + Some(Value::Object(params)) => params, + Some(Value::Null) | None => return Ok(()), + Some(_) => bail!("prompt_data.options.params must be a JSON object"), + }; + + validate_model_params(model, params) +} + +fn validate_model_params(model: Option<&str>, params: &Map) -> Result<()> { + let temperature = optional_number(params, "temperature", "--temperature")?; + if let Some(temperature) = temperature { + let max = if model.is_some_and(uses_anthropic_temperature_range) { + 1.0 + } else { + 2.0 + }; + validate_number_range(temperature, 0.0, max, "--temperature")?; + + if let Some(model) = model { + validate_temperature_support(model, params)?; + } + } + + if let Some(top_p) = optional_number(params, "top_p", "--top-p")? { + validate_number_range(top_p, 0.0, 1.0, "--top-p")?; + if let Some(model) = model.filter(|model| has_unsupported_opus_sampling_params(model)) { + bail!("--top-p is not supported by model '{model}'"); + } + } + + for (key, label) in [ + ("frequency_penalty", "--frequency-penalty"), + ("presence_penalty", "--presence-penalty"), + ] { + if let Some(value) = optional_number(params, key, label)? { + validate_number_range(value, -2.0, 2.0, label)?; + } + } + + if let Some(max_tokens) = params.get("max_tokens") { + match max_tokens { + Value::Null => {} + Value::Number(number) if number.as_u64().is_some_and(|value| value > 0) => {} + _ => bail!("--max-tokens must be a positive integer"), + } + } + + Ok(()) +} + +fn optional_number(params: &Map, key: &str, label: &str) -> Result> { + match params.get(key) { + Some(Value::Null) | None => Ok(None), + Some(Value::Number(number)) => number + .as_f64() + .filter(|value| value.is_finite()) + .map(Some) + .ok_or_else(|| anyhow::anyhow!("{label} must be a finite number")), + Some(_) => bail!("{label} must be a number"), + } +} + +fn validate_number_range(value: f64, min: f64, max: f64, label: &str) -> Result<()> { + if !value.is_finite() || !(min..=max).contains(&value) { + bail!("{label} must be between {min} and {max}"); + } + Ok(()) +} + +/// Keep this in sync with `modelSupportsCustomTemperature` in the backend's +/// `typespecs/src/model-capabilities.ts`. +fn validate_temperature_support(model: &str, params: &Map) -> Result<()> { + let lower = model.to_ascii_lowercase(); + + if lower.contains("claude-opus-4-7") { + bail!("--temperature is not supported by model '{model}'"); + } + + if lower.contains("gpt-5") { + let has_no_reasoning_effort = params + .get("reasoning_effort") + .and_then(Value::as_str) + .is_some_and(|effort| effort == "none"); + if !has_no_reasoning_effort { + bail!( + "--temperature is not supported by model '{model}' unless reasoning effort is 'none'; pass `--reasoning-effort none` or omit `--temperature`" + ); + } + } else if ["o1", "o2", "o3", "o4"] + .iter() + .any(|prefix| lower.starts_with(prefix)) + { + bail!("--temperature is not supported by model '{model}'"); + } + + Ok(()) +} + +fn has_unsupported_opus_sampling_params(model: &str) -> bool { + model.to_ascii_lowercase().contains("claude-opus-4-7") +} + +fn uses_anthropic_temperature_range(model: &str) -> bool { + let lower = model.to_ascii_lowercase(); + lower.contains("claude") + || lower.starts_with("anthropic.") + || lower.contains(".anthropic.") + || lower.contains("/anthropic/") +} + fn insert_optional_number( target: &mut Map, key: &str, @@ -335,6 +489,119 @@ mod tests { assert_eq!(patch["template_format"], "nunjucks"); } + #[test] + fn rejects_model_parameters_outside_provider_ranges() { + for (arguments, expected) in [ + ( + vec!["--temperature", "99"], + "--temperature must be between 0 and 2", + ), + ( + vec!["--max-tokens", "0"], + "--max-tokens must be a positive integer", + ), + ( + vec!["--frequency-penalty", "-2.1"], + "--frequency-penalty must be between -2 and 2", + ), + ( + vec!["--presence-penalty", "2.1"], + "--presence-penalty must be between -2 and 2", + ), + ] { + let parsed = + Harness::try_parse_from(std::iter::once("test").chain(arguments.iter().copied())) + .expect("parse arguments"); + let error = parsed + .config + .build_prompt_data_patch(Some("gpt-4.1-mini")) + .expect_err("out-of-range parameter should fail"); + assert_eq!(error.to_string(), expected); + } + } + + #[test] + fn enforces_model_specific_temperature_support() { + let parsed = + Harness::try_parse_from(["test", "--temperature", "0.2"]).expect("parse arguments"); + + for model in ["gpt-5.4-nano", "o3", "claude-opus-4-7"] { + let error = parsed + .config + .build_prompt_data_patch(Some(model)) + .expect_err("unsupported temperature should fail"); + assert!(error.to_string().contains("not supported by model")); + } + } + + #[test] + fn allows_gpt5_temperature_when_reasoning_effort_is_none() { + let parsed = + Harness::try_parse_from(["test", "--temperature", "0.2", "--reasoning-effort", "none"]) + .expect("parse arguments"); + + let patch = parsed + .config + .build_prompt_data_patch(Some("gpt-5.4-nano")) + .expect("compatible parameters"); + assert_eq!(patch["options"]["params"]["temperature"], 0.2); + assert_eq!(patch["options"]["params"]["reasoning_effort"], "none"); + } + + #[test] + fn validates_update_against_the_existing_model_and_params() { + let existing = json!({ + "options": { + "model": "gpt-5.4-nano", + "params": { "reasoning_effort": "medium" } + } + }); + let patch = json!({ + "prompt_data": { + "options": { "params": { "temperature": 0.2 } } + } + }); + let error = validate_prompt_data_patch(Some(&existing), &patch) + .expect_err("effective model does not support temperature"); + assert!(error.to_string().contains("--reasoning-effort none")); + + let existing = json!({ + "options": { + "model": "gpt-5.4-nano", + "params": { "reasoning_effort": "none" } + } + }); + validate_prompt_data_patch(Some(&existing), &patch) + .expect("existing reasoning effort makes temperature valid"); + } + + #[test] + fn rejects_anthropic_temperature_above_one_in_arbitrary_patch() { + let patch = json!({ + "prompt_data": { + "options": { + "model": "claude-sonnet-4-5", + "params": { "temperature": 1.5 } + } + } + }); + let error = validate_prompt_data_patch(None, &patch) + .expect_err("Anthropic temperature should use the smaller range"); + assert_eq!(error.to_string(), "--temperature must be between 0 and 1"); + } + + #[test] + fn ignores_unrelated_updates_to_existing_model_configuration() { + let existing = json!({ + "options": { + "model": "gpt-4.1-mini", + "params": { "temperature": 99 } + } + }); + validate_prompt_data_patch(Some(&existing), &json!({"description": "Updated"})) + .expect("an unrelated metadata update should remain possible"); + } + #[test] fn validates_scores_against_api_range() { let error = parse_choice_scores_source(r#"{"bad":1.5}"#) diff --git a/src/functions/update.rs b/src/functions/update.rs index 157e6a2c..da64d943 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -11,8 +11,8 @@ use crate::{ use super::{api, label, label_plural, select_function_interactive}; use super::{ prompt_config::{ - parse_choice_scores_source, parse_classifications_source, validate_unit_interval, - PromptConfigArgs, + parse_choice_scores_source, parse_classifications_source, validate_prompt_data_patch, + validate_unit_interval, PromptConfigArgs, }, FunctionTypeFilter, ResolvedContext, }; @@ -27,7 +27,7 @@ use super::{ #[command(after_help = "\ Examples: bt scorers update my-scorer --messages @messages.json - bt scorers update my-scorer --model gpt-5.4-nano --temperature 0.1 + 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 @@ -164,6 +164,7 @@ pub async fn run( let body = build_patch_body(args)?; let function = resolve_target_function(ctx, args, ft).await?; + validate_prompt_data_patch(function.prompt_data.as_ref(), &body)?; // LLM scorer/classifier output flags only apply to prompt-based scorers and // classifiers. Reject them on other function kinds (for example tools) so an diff --git a/src/prompts/update.rs b/src/prompts/update.rs index cbd997ac..511c488b 100644 --- a/src/prompts/update.rs +++ b/src/prompts/update.rs @@ -4,7 +4,7 @@ use dialoguer::Confirm; use serde_json::{json, Map, Value}; use crate::{ - functions::prompt_config::PromptConfigArgs, + functions::prompt_config::{validate_prompt_data_patch, PromptConfigArgs}, ui::{is_interactive, print_command_status, with_spinner, CommandStatus}, utils::{merge_json_objects, read_text_source, read_yaml_object_source}, }; @@ -91,6 +91,8 @@ pub async fn run(ctx: &ResolvedContext, args: &UpdateArgs, json_output: bool) -> } }; + validate_prompt_data_patch(prompt.prompt_data.as_ref(), &body)?; + if !args.yes && is_interactive() { let confirm = Confirm::new() .with_prompt(format!( From fdb145fdd4e0854588844a4734ee958666b949e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Tue, 28 Jul 2026 16:41:30 -0700 Subject: [PATCH 5/7] chore: follow up to the model parameters validation --- README.md | 105 +++++- src/functions/create.rs | 9 +- src/functions/mod.rs | 1 + src/functions/model_capabilities.rs | 327 ++++++++++++++++++ src/functions/prompt_config.rs | 492 ++++++++++++++++++++++++---- src/functions/update.rs | 6 +- src/prompts/update.rs | 6 +- 7 files changed, 865 insertions(+), 81 deletions(-) create mode 100644 src/functions/model_capabilities.rs diff --git a/README.md b/README.md index cfa19e70..3b7465ef 100644 --- a/README.md +++ b/README.md @@ -135,21 +135,96 @@ 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 auth` | Authenticate with Braintrust | -| `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 sync` | Synchronize project logs between Braintrust and local NDJSON files | -| `bt update` | Update bt in-place | +| Command | Description | +| -------------- | ------------------------------------------------------------------ | +| `bt init` | Initialize `.bt/` config directory and link to a project | +| `bt auth` | Authenticate with Braintrust | +| `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, update, 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` + +Create and update prompt-based LLM scorers in the current project: + +```bash +bt scorers create "Helpfulness" \ + --model gpt-5.4-nano \ + --messages @messages.json \ + --choice-scores '{"A":1,"B":0}' + +bt scorers update helpfulness --messages @messages.json +bt scorers update helpfulness --model gpt-5.4-nano +``` + +`@PATH` and `-` are CLI-only source notation, not scorer settings in the web UI. For example, `--messages @messages.json` reads chat messages from `messages.json`, while `--messages -` reads them from stdin. + +LLM scorer configuration mirrors the web UI: + +```bash +bt scorers create "Quality judge" \ + --model gpt-5.4-nano \ + --messages @messages.json \ + --choice-scores '{"pass":1,"fail":0}' \ + --temperature 0.1 \ + --max-tokens 512 \ + --top-p 0.9 \ + --frequency-penalty 0 \ + --presence-penalty 0 \ + --stop-sequence END \ + --tool-choice auto \ + --reasoning-effort none \ + --verbosity low \ + --template-format mustache \ + --pass-threshold 0.7 \ + --metadata @metadata.yaml +``` + +Use `--template-format mustache|jinja|none`; `nunjucks` and `jinja2` are accepted aliases for Jinja. Repeat `--stop-sequence` for multiple values. Tool choice accepts `auto`, `none`, `required`, or a function name. Model parameters are validated against the same model catalog and custom-model metadata used by the web UI, including parameter availability, provider-specific ranges, reasoning options, and output-token limits. Unknown custom models receive only provider-independent validation instead of being assigned capabilities based on their names. + +For classification output instead of a numeric score, use classifications in place of choice scores: + +```bash +bt scorers create "Safety label" \ + --model gpt-5.4-nano \ + --messages @messages.json \ + --classifications '["safe","unsafe"]' \ + --allow-no-match +``` + +Use `--if-exists error|ignore|replace` when creating a scorer. Text and structured input flags accept an inline value, `@PATH` to read from a file, or `-` for stdin. For fields without a dedicated update flag, use `--patch` with a JSON object. + +For code scorers, use the Braintrust SDK for your language and push the source file: + +```ts +// TypeScript +import { projects } from "braintrust"; +const project = projects.create({ name: "test-project" }); +project.scorers.create({ name: "Test scorer", handler: ({ output }) => 1 }); +``` + +```python +# Python +from braintrust import projects +project = projects.create("test-project") +project.scorers.create(name="Test scorer", handler=test_scorer, parameters=ScorerInput) +``` + +```bash +bt functions push scorer.ts +bt functions push scorer.py +``` ## `bt eval` diff --git a/src/functions/create.rs b/src/functions/create.rs index 7bb76ced..6057f58f 100644 --- a/src/functions/create.rs +++ b/src/functions/create.rs @@ -11,8 +11,8 @@ use crate::{ use super::{ api, prompt_config::{ - parse_choice_scores_source, parse_classifications_source, validate_unit_interval, - PromptConfigArgs, + parse_choice_scores_source, parse_classifications_source, validate_prompt_data_patch, + validate_unit_interval, PromptConfigArgs, }, IfExistsMode, ResolvedContext, }; @@ -119,6 +119,11 @@ pub(crate) async fn run(ctx: &ResolvedContext, args: &CreateArgs, json_output: b let name = resolve_name(args)?; let slug = resolve_slug(args, &name)?; let definition = build_scorer_definition(args, &ctx.project.id, &name, &slug)?; + with_spinner( + "Validating model parameters...", + validate_prompt_data_patch(ctx, None, &definition), + ) + .await?; let result = match with_spinner( "Creating scorer...", diff --git a/src/functions/mod.rs b/src/functions/mod.rs index 090a37ba..a501ba90 100644 --- a/src/functions/mod.rs +++ b/src/functions/mod.rs @@ -17,6 +17,7 @@ pub(crate) mod create; mod delete; mod invoke; mod list; +mod model_capabilities; pub(crate) mod prompt_config; mod pull; mod push; diff --git a/src/functions/model_capabilities.rs b/src/functions/model_capabilities.rs new file mode 100644 index 00000000..f0fc9a2b --- /dev/null +++ b/src/functions/model_capabilities.rs @@ -0,0 +1,327 @@ +use std::{collections::HashMap, time::Duration}; + +use serde::Deserialize; +use serde_json::Value; + +use crate::{http::build_http_client, project_context::ProjectContext}; + +const MODEL_CATALOG_TIMEOUT: Duration = Duration::from_secs(5); + +/// The model metadata used by the web UI to decide which controls and ranges +/// to expose. Unknown fields are intentionally ignored so newer app versions +/// can extend the catalog without breaking older CLI versions. +#[derive(Debug, Clone, Deserialize)] +pub(crate) struct ModelSpec { + pub(crate) format: String, + #[serde(rename = "flavor")] + pub(crate) _flavor: String, + #[serde(default, rename = "displayName")] + pub(crate) display_name: Option, + #[serde(default)] + pub(crate) o1_like: Option, + #[serde(default)] + pub(crate) reasoning: Option, + #[serde(default)] + pub(crate) reasoning_budget: Option, + #[serde(default)] + pub(crate) max_output_tokens: Option, +} + +impl ModelSpec { + pub(crate) fn supports_reasoning(&self, model: &str) -> bool { + if self.reasoning.unwrap_or(false) || self.o1_like.unwrap_or(false) { + return true; + } + + // Match `modelProviderHasReasoning` from the web model catalog. The UI + // applies these fallbacks to custom models that omit `reasoning`. + let lower = model.to_ascii_lowercase(); + match self.format.as_str() { + "openai" => { + ["o1", "o2", "o3", "o4"] + .iter() + .any(|prefix| lower.starts_with(prefix)) + || lower.contains("gpt-5") + } + "anthropic" => lower.starts_with("claude-3.7"), + "google" => lower.ends_with("gemini-2.0-flash") || lower.contains("gemini-2.5"), + _ => false, + } + } +} + +#[derive(Debug, Deserialize)] +struct SecretWithMetadata { + #[serde(default)] + metadata: Option, +} + +#[derive(Debug, Deserialize)] +struct SecretListResponse { + objects: Vec, +} + +/// Resolve a model from the same shared catalog and configured custom-model +/// metadata used by the prompt UI. Project custom models take precedence over +/// org custom models, which take precedence over the shared catalog. Failure +/// to load metadata is non-fatal; callers then apply only provider-independent +/// validation so arbitrary/custom model names remain usable. +pub(crate) async fn resolve_model_spec(ctx: &ProjectContext, model: &str) -> Option { + let http = build_http_client(MODEL_CATALOG_TIMEOUT).ok()?; + let app_url = ctx.app_url.trim_end_matches('/'); + let catalog_url = format!("{app_url}/api/models/model_list.json"); + let org_secrets_url = format!("{app_url}/api/ai_secret/get"); + let project_secrets_path = format!( + "/v1/env_var?object_type=project&object_id={}&secret_category=ai_provider", + urlencoding::encode(&ctx.project.id), + ); + + let catalog_request = async { + let response = http.get(catalog_url).send().await.ok()?; + if !response.status().is_success() { + return None; + } + response.json::>().await.ok() + }; + let org_selector = if ctx.client.org_id().trim().is_empty() { + serde_json::json!({ "org_name": ctx.client.org_name() }) + } else { + serde_json::json!({ "org_id": ctx.client.org_id() }) + }; + let org_models_request = async { + let response = http + .post(org_secrets_url) + .bearer_auth(ctx.client.api_key()) + .json(&org_selector) + .send() + .await + .ok()?; + if !response.status().is_success() { + return None; + } + let secrets = response.json::>().await.ok()?; + Some(custom_models_from_secrets(secrets)) + }; + let project_models_request = async { + let response = tokio::time::timeout( + MODEL_CATALOG_TIMEOUT, + ctx.client.get::(&project_secrets_path), + ) + .await + .ok()? + .ok()?; + Some(custom_models_from_secrets(response.objects)) + }; + + let (catalog, org_models, project_models) = + tokio::join!(catalog_request, org_models_request, project_models_request); + + let mut models = catalog.unwrap_or_default(); + models.extend(org_models.unwrap_or_default()); + models.extend(project_models.unwrap_or_default()); + + resolve_from_models(&models, model).cloned() +} + +fn custom_models_from_secrets(secrets: Vec) -> HashMap { + secrets + .into_iter() + .filter_map(|secret| secret.metadata) + .filter_map(|metadata| metadata.get("customModels").cloned()) + .filter_map(|models| serde_json::from_value::>(models).ok()) + .flatten() + .collect() +} + +fn resolve_from_models<'a>( + models: &'a HashMap, + model: &str, +) -> Option<&'a ModelSpec> { + models.get(model).or_else(|| { + models + .values() + .find(|spec| spec.display_name.as_deref() == Some(model)) + }) +} + +#[cfg(test)] +mod tests { + use actix_web::{dev::ServerHandle, web, App, HttpResponse, HttpServer}; + + use braintrust_sdk_rust::LoginState; + + use crate::{auth::LoginContext, http::ApiClient, projects::api::Project}; + + use super::*; + + fn spec(format: &str, display_name: Option<&str>) -> ModelSpec { + ModelSpec { + format: format.to_string(), + _flavor: "chat".to_string(), + display_name: display_name.map(ToOwned::to_owned), + o1_like: None, + reasoning: None, + reasoning_budget: None, + max_output_tokens: None, + } + } + + #[test] + fn extracts_custom_models_from_secret_metadata() { + let secrets = vec![SecretWithMetadata { + metadata: Some(serde_json::json!({ + "customModels": { + "test-custom-model": { + "format": "anthropic", + "flavor": "chat", + "max_output_tokens": 4096 + } + } + })), + }]; + + let models = custom_models_from_secrets(secrets); + let model = models.get("test-custom-model").expect("custom model"); + assert_eq!(model.format, "anthropic"); + assert_eq!(model.max_output_tokens, Some(4096)); + } + + #[test] + fn applies_web_ui_reasoning_fallbacks_for_custom_models() { + let openai = spec("openai", None); + assert!(openai.supports_reasoning("o3")); + assert!(openai.supports_reasoning("test-gpt-5-deployment")); + assert!(!openai.supports_reasoning("gpt-4.1")); + + let anthropic = spec("anthropic", None); + assert!(anthropic.supports_reasoning("claude-3.7-sonnet")); + } + + #[test] + fn resolves_catalog_models_by_id_or_display_name() { + let models = HashMap::from([( + "test-model-id".to_string(), + spec("openai", Some("Test model")), + )]); + + assert!(resolve_from_models(&models, "test-model-id").is_some()); + assert!(resolve_from_models(&models, "Test model").is_some()); + assert!(resolve_from_models(&models, "missing").is_none()); + } + + struct MockServer { + base_url: String, + handle: ServerHandle, + } + + impl MockServer { + async fn start() -> Self { + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind mock server"); + let address = listener.local_addr().expect("mock server address"); + let base_url = format!("http://{address}"); + let server = HttpServer::new(|| { + App::new() + .route( + "/api/models/model_list.json", + web::get().to(|| async { + HttpResponse::Ok().json(serde_json::json!({ + "test-shared-model": { + "format": "openai", + "flavor": "chat", + "max_output_tokens": 100 + } + })) + }), + ) + .route( + "/api/ai_secret/get", + web::post().to(|| async { + HttpResponse::Ok().json(serde_json::json!([{ + "metadata": { + "customModels": { + "test-precedence-model": { + "format": "anthropic", + "flavor": "chat", + "max_output_tokens": 200 + } + } + } + }])) + }), + ) + .route( + "/v1/env_var", + web::get().to(|| async { + HttpResponse::Ok().json(serde_json::json!({ + "objects": [{ + "metadata": { + "customModels": { + "test-precedence-model": { + "format": "google", + "flavor": "chat", + "max_output_tokens": 300 + } + } + } + }] + })) + }), + ) + }) + .workers(1) + .listen(listener) + .expect("listen mock server") + .run(); + let handle = server.handle(); + tokio::spawn(server); + Self { base_url, handle } + } + + async fn stop(self) { + self.handle.stop(false).await; + } + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn resolves_shared_and_custom_models_with_project_precedence() { + let server = MockServer::start().await; + let login = LoginState::new(); + login.set( + "test-key".to_string(), + "test-org-id".to_string(), + "test-org".to_string(), + server.base_url.clone(), + server.base_url.clone(), + ); + let client = ApiClient::new(&LoginContext { + login, + api_url: server.base_url.clone(), + app_url: server.base_url.clone(), + }) + .expect("API client"); + let ctx = ProjectContext { + client, + app_url: server.base_url.clone(), + project: Project { + id: "test-project-id".to_string(), + name: "test-project".to_string(), + org_id: "test-org-id".to_string(), + description: None, + }, + }; + + let shared = resolve_model_spec(&ctx, "test-shared-model") + .await + .expect("shared model"); + assert_eq!(shared.format, "openai"); + assert_eq!(shared.max_output_tokens, Some(100)); + + let custom = resolve_model_spec(&ctx, "test-precedence-model") + .await + .expect("custom model"); + assert_eq!(custom.format, "google"); + assert_eq!(custom.max_output_tokens, Some(300)); + + server.stop().await; + } +} diff --git a/src/functions/prompt_config.rs b/src/functions/prompt_config.rs index 134e3157..00a398d1 100644 --- a/src/functions/prompt_config.rs +++ b/src/functions/prompt_config.rs @@ -4,7 +4,12 @@ use anyhow::{bail, Context, Result}; use clap::{Args, ValueEnum}; use serde_json::{json, Map, Number, Value}; -use crate::utils::{merge_json_objects, read_text_source}; +use crate::{ + project_context::ProjectContext, + utils::{merge_json_objects, read_text_source}, +}; + +use super::model_capabilities::{resolve_model_spec, ModelSpec}; #[derive(Debug, Clone, Default, Args)] pub(crate) struct PromptConfigArgs { @@ -21,11 +26,11 @@ pub(crate) struct PromptConfigArgs { #[arg(long, value_name = "NUMBER")] top_p: Option, - /// Frequency penalty, between -2 and 2. + /// Frequency penalty. Availability and range depend on the model. #[arg(long, value_name = "NUMBER", allow_hyphen_values = true)] frequency_penalty: Option, - /// Presence penalty, between -2 and 2. + /// Presence penalty. Availability and range depend on the model. #[arg(long, value_name = "NUMBER", allow_hyphen_values = true)] presence_penalty: Option, @@ -177,7 +182,8 @@ impl PromptConfigArgs { } let effective_model = options.get("model").and_then(Value::as_str); - validate_model_params(effective_model, ¶ms)?; + let changed_params = params.keys().cloned().collect(); + validate_model_params(effective_model, ¶ms, &changed_params, None)?; if !params.is_empty() { options.insert("params".to_string(), Value::Object(params)); @@ -197,21 +203,53 @@ impl PromptConfigArgs { } /// Validate the effective model configuration produced by deep-merging a patch -/// into an existing prompt definition. -/// -/// The API accepts partial prompt updates without checking provider-specific -/// model constraints. Validate whenever an update touches the model or its -/// parameters so a successful PATCH cannot leave a scorer or prompt unusable. -pub(crate) fn validate_prompt_data_patch( +/// into an existing prompt definition. Model metadata comes from the same +/// catalog and custom-model configuration used by the web UI. If metadata is +/// unavailable for an arbitrary model name, validation deliberately falls back +/// to provider-independent type and range checks. +pub(crate) async fn validate_prompt_data_patch( + ctx: &ProjectContext, existing_prompt_data: Option<&Value>, patch: &Value, ) -> Result<()> { - let Some(patch_prompt_data) = patch.get("prompt_data").and_then(Value::as_object) else { + let Some(update) = prepare_model_params_update(existing_prompt_data, patch)? else { return Ok(()); }; - if !patch_prompt_data.contains_key("options") { - return Ok(()); + let spec = match update.model.as_deref() { + Some(model) => resolve_model_spec(ctx, model).await, + None => None, + }; + validate_model_params( + update.model.as_deref(), + &update.params, + &update.changed_params, + spec.as_ref(), + ) +} + +#[derive(Debug)] +struct ModelParamsUpdate { + model: Option, + params: Map, + changed_params: HashSet, +} + +fn prepare_model_params_update( + existing_prompt_data: Option<&Value>, + patch: &Value, +) -> Result> { + let Some(patch_prompt_data) = patch.get("prompt_data").and_then(Value::as_object) else { + return Ok(None); + }; + let Some(patch_options_value) = patch_prompt_data.get("options") else { + return Ok(None); + }; + if patch_options_value.is_null() { + return Ok(None); } + let patch_options = patch_options_value + .as_object() + .ok_or_else(|| anyhow::anyhow!("prompt_data.options must be a JSON object"))?; let mut effective_prompt_data = existing_prompt_data .and_then(Value::as_object) @@ -219,50 +257,77 @@ pub(crate) fn validate_prompt_data_patch( .unwrap_or_default(); merge_json_objects(&mut effective_prompt_data, patch_prompt_data); - let Some(options) = effective_prompt_data.get("options") else { - return Ok(()); - }; - if options.is_null() { - return Ok(()); - } - let options = options - .as_object() + let options = effective_prompt_data + .get("options") + .and_then(Value::as_object) .ok_or_else(|| anyhow::anyhow!("prompt_data.options must be a JSON object"))?; - let model = match options.get("model") { - Some(Value::String(model)) if !model.trim().is_empty() => Some(model.trim()), + Some(Value::String(model)) if !model.trim().is_empty() => Some(model.trim().to_string()), Some(Value::String(_)) => bail!("model cannot be empty"), Some(Value::Null) | None => None, Some(_) => bail!("prompt_data.options.model must be a string"), }; let params = match options.get("params") { - Some(Value::Object(params)) => params, - Some(Value::Null) | None => return Ok(()), + Some(Value::Object(params)) => params.clone(), + Some(Value::Null) | None => Map::new(), Some(_) => bail!("prompt_data.options.params must be a JSON object"), }; - validate_model_params(model, params) -} + let model_changed = patch_options.contains_key("model"); + let mut changed_params = if model_changed { + params.keys().cloned().collect() + } else { + match patch_options.get("params") { + Some(Value::Object(params)) => params.keys().cloned().collect(), + Some(Value::Null) | None => HashSet::new(), + Some(_) => bail!("prompt_data.options.params must be a JSON object"), + } + }; -fn validate_model_params(model: Option<&str>, params: &Map) -> Result<()> { - let temperature = optional_number(params, "temperature", "--temperature")?; - if let Some(temperature) = temperature { - let max = if model.is_some_and(uses_anthropic_temperature_range) { - 1.0 - } else { - 2.0 - }; - validate_number_range(temperature, 0.0, max, "--temperature")?; + // Temperature support can depend on reasoning effort, so changing either + // side of that relationship must validate the effective temperature. + if changed_params.contains("reasoning_effort") && params.contains_key("temperature") { + changed_params.insert("temperature".to_string()); + } - if let Some(model) = model { - validate_temperature_support(model, params)?; + if changed_params.is_empty() { + return Ok(None); + } + + Ok(Some(ModelParamsUpdate { + model, + params, + changed_params, + })) +} + +fn validate_model_params( + model: Option<&str>, + params: &Map, + changed_params: &HashSet, + spec: Option<&ModelSpec>, +) -> Result<()> { + if changed_params.contains("temperature") { + ensure_parameter_supported(spec, model, "--temperature", TEMPERATURE_FORMATS)?; + if let Some(temperature) = optional_number(params, "temperature", "--temperature")? { + let max = match spec.map(|spec| spec.format.as_str()) { + Some("anthropic" | "converse") => 1.0, + _ => 2.0, + }; + validate_number_range(temperature, 0.0, max, "--temperature")?; + if let Some(model) = model { + validate_temperature_support(model, params)?; + } } } - if let Some(top_p) = optional_number(params, "top_p", "--top-p")? { - validate_number_range(top_p, 0.0, 1.0, "--top-p")?; - if let Some(model) = model.filter(|model| has_unsupported_opus_sampling_params(model)) { - bail!("--top-p is not supported by model '{model}'"); + if changed_params.contains("top_p") { + ensure_parameter_supported(spec, model, "--top-p", TOP_P_FORMATS)?; + if let Some(top_p) = optional_number(params, "top_p", "--top-p")? { + validate_number_range(top_p, 0.0, 1.0, "--top-p")?; + if let Some(model) = model.filter(|model| has_unsupported_opus_sampling_params(model)) { + bail!("--top-p is not supported by model '{model}'"); + } } } @@ -270,22 +335,182 @@ fn validate_model_params(model: Option<&str>, params: &Map) -> Re ("frequency_penalty", "--frequency-penalty"), ("presence_penalty", "--presence-penalty"), ] { - if let Some(value) = optional_number(params, key, label)? { - validate_number_range(value, -2.0, 2.0, label)?; + if changed_params.contains(key) { + ensure_parameter_supported(spec, model, label, PENALTY_FORMATS)?; + if let Some(value) = optional_number(params, key, label)? { + // The web UI exposes 0..=1. For an unknown/custom model whose + // metadata could not be loaded, retain the provider API's + // broader OpenAI-compatible range rather than guessing. + let (min, max) = if spec.is_some_and(|spec| spec.format == "openai") { + (0.0, 1.0) + } else { + (-2.0, 2.0) + }; + validate_number_range(value, min, max, label)?; + } + } + } + + if changed_params.contains("max_tokens") { + ensure_parameter_supported(spec, model, "--max-tokens", MAX_TOKENS_FORMATS)?; + if let Some(max_tokens) = params.get("max_tokens") { + match max_tokens { + Value::Null => {} + Value::Number(number) if number.as_u64().is_some_and(|value| value > 0) => { + if let (Some(spec), Some(value)) = (spec, number.as_u64()) { + let max = spec + .max_output_tokens + .filter(|max| *max > 0) + .unwrap_or(32_768); + if value > max { + bail!( + "--max-tokens must be between 1 and {max} for model '{}'", + model.unwrap_or("") + ); + } + } + } + _ => bail!("--max-tokens must be a positive integer"), + } + } + } + + if changed_params.contains("stop") { + match params.get("stop") { + Some(Value::Null) | None => {} + Some(Value::Array(values)) if values.iter().all(Value::is_string) => {} + _ => bail!("--stop-sequence values must be strings"), } } - if let Some(max_tokens) = params.get("max_tokens") { - match max_tokens { - Value::Null => {} - Value::Number(number) if number.as_u64().is_some_and(|value| value > 0) => {} - _ => bail!("--max-tokens must be a positive integer"), + if changed_params.contains("tool_choice") { + if let Some(value) = params.get("tool_choice").filter(|value| !value.is_null()) { + ensure_parameter_supported(spec, model, "--tool-choice", TOOL_FORMATS)?; + validate_tool_choice(value)?; + } + } + + if changed_params.contains("reasoning_effort") { + validate_reasoning_effort(model, params, spec)?; + } + + if changed_params.contains("verbosity") { + if let Some(value) = params.get("verbosity").filter(|value| !value.is_null()) { + let verbosity = value + .as_str() + .ok_or_else(|| anyhow::anyhow!("--verbosity must be a string"))?; + if !["low", "medium", "high"].contains(&verbosity) { + bail!("--verbosity must be one of low, medium, high"); + } + if let (Some(model), Some(spec)) = (model, spec) { + let display_name = spec.display_name.as_deref().unwrap_or(model); + if !display_name.to_ascii_lowercase().contains("gpt-5") { + bail!("--verbosity is not supported by model '{model}'"); + } + } } } Ok(()) } +// Keep these in sync with `defaultModelParamSettings`, `getSliderSpecs`, and +// `modelProviderHasTools` in `proxy/packages/proxy/schema/index.ts`. +const KNOWN_FORMATS: &[&str] = &["openai", "anthropic", "google", "js", "window", "converse"]; +const TEMPERATURE_FORMATS: &[&str] = &["openai", "anthropic", "google", "window", "converse"]; +const MAX_TOKENS_FORMATS: &[&str] = &["openai", "anthropic", "google", "converse"]; +const TOP_P_FORMATS: &[&str] = &["openai", "anthropic", "google", "converse"]; +const PENALTY_FORMATS: &[&str] = &["openai"]; +const TOOL_FORMATS: &[&str] = &["openai", "anthropic", "google", "converse"]; + +fn ensure_parameter_supported( + spec: Option<&ModelSpec>, + model: Option<&str>, + label: &str, + supported_formats: &[&str], +) -> Result<()> { + let Some(spec) = spec else { + return Ok(()); + }; + if KNOWN_FORMATS.contains(&spec.format.as_str()) + && !supported_formats.contains(&spec.format.as_str()) + { + bail!( + "{label} is not supported by model '{}' (format: {})", + model.unwrap_or(""), + spec.format, + ); + } + Ok(()) +} + +fn validate_tool_choice(value: &Value) -> Result<()> { + match value { + Value::String(choice) if ["auto", "none", "required"].contains(&choice.as_str()) => Ok(()), + Value::Object(choice) + if choice.get("type").and_then(Value::as_str) == Some("function") + && choice + .get("function") + .and_then(Value::as_object) + .and_then(|function| function.get("name")) + .and_then(Value::as_str) + .is_some_and(|name| !name.trim().is_empty()) => + { + Ok(()) + } + _ => bail!("--tool-choice must be auto, none, required, or a non-empty function name"), + } +} + +fn validate_reasoning_effort( + model: Option<&str>, + params: &Map, + spec: Option<&ModelSpec>, +) -> Result<()> { + let Some(effort) = params.get("reasoning_effort") else { + return Ok(()); + }; + if effort.is_null() { + return Ok(()); + } + let effort = effort + .as_str() + .ok_or_else(|| anyhow::anyhow!("--reasoning-effort must be a string"))?; + let (Some(model), Some(spec)) = (model, spec) else { + return Ok(()); + }; + if !KNOWN_FORMATS.contains(&spec.format.as_str()) { + return Ok(()); + } + if !spec.supports_reasoning(model) { + bail!("--reasoning-effort is not supported by model '{model}'"); + } + + let gemini_thinking_level = spec.format == "google" && is_gemini_3_model(model); + if spec.format != "openai" && spec.reasoning_budget.unwrap_or(false) && !gemini_thinking_level { + bail!( + "--reasoning-effort is not supported by model '{model}'; this model uses a reasoning budget" + ); + } + + let options: &[&str] = if is_gpt_5_pro_model(model) { + &["high"] + } else if is_gpt_5_1_or_later(model) { + &["none", "low", "medium", "high"] + } else if is_gpt_5_model(model) || gemini_thinking_level { + &["minimal", "low", "medium", "high"] + } else { + &["low", "medium", "high"] + }; + if !options.contains(&effort) { + bail!( + "--reasoning-effort must be one of {} for model '{model}'", + options.join(", ") + ); + } + Ok(()) +} + fn optional_number(params: &Map, key: &str, label: &str) -> Result> { match params.get(key) { Some(Value::Null) | None => Ok(None), @@ -338,12 +563,31 @@ fn has_unsupported_opus_sampling_params(model: &str) -> bool { model.to_ascii_lowercase().contains("claude-opus-4-7") } -fn uses_anthropic_temperature_range(model: &str) -> bool { +fn is_gpt_5_pro_model(model: &str) -> bool { + model.to_ascii_lowercase().contains("gpt-5-pro") +} + +fn is_gpt_5_1_or_later(model: &str) -> bool { let lower = model.to_ascii_lowercase(); - lower.contains("claude") - || lower.starts_with("anthropic.") - || lower.contains(".anthropic.") - || lower.contains("/anthropic/") + let Some(start) = lower.find("gpt-5.") else { + return false; + }; + let version = lower[start + "gpt-5.".len()..] + .chars() + .take_while(char::is_ascii_digit) + .collect::(); + version.parse::().is_ok_and(|version| version >= 1) +} + +fn is_gpt_5_model(model: &str) -> bool { + model.to_ascii_lowercase().contains("gpt-5") + && !is_gpt_5_1_or_later(model) + && !is_gpt_5_pro_model(model) +} + +fn is_gemini_3_model(model: &str) -> bool { + let lower = model.to_ascii_lowercase(); + lower.starts_with("gemini-3") || lower.contains("/gemini-3") } fn insert_optional_number( @@ -440,6 +684,39 @@ mod tests { config: PromptConfigArgs, } + fn model_spec( + format: &str, + reasoning: bool, + reasoning_budget: bool, + max_output_tokens: Option, + ) -> ModelSpec { + ModelSpec { + format: format.to_string(), + _flavor: "chat".to_string(), + display_name: None, + o1_like: None, + reasoning: Some(reasoning), + reasoning_budget: Some(reasoning_budget), + max_output_tokens, + } + } + + fn validate_patch( + existing_prompt_data: Option<&Value>, + patch: &Value, + spec: Option<&ModelSpec>, + ) -> Result<()> { + let Some(update) = prepare_model_params_update(existing_prompt_data, patch)? else { + return Ok(()); + }; + validate_model_params( + update.model.as_deref(), + &update.params, + &update.changed_params, + spec, + ) + } + #[test] fn builds_web_ui_compatible_prompt_configuration() { let args = Harness::try_parse_from([ @@ -561,7 +838,7 @@ mod tests { "options": { "params": { "temperature": 0.2 } } } }); - let error = validate_prompt_data_patch(Some(&existing), &patch) + let error = validate_patch(Some(&existing), &patch, None) .expect_err("effective model does not support temperature"); assert!(error.to_string().contains("--reasoning-effort none")); @@ -571,34 +848,125 @@ mod tests { "params": { "reasoning_effort": "none" } } }); - validate_prompt_data_patch(Some(&existing), &patch) + validate_patch(Some(&existing), &patch, None) .expect("existing reasoning effort makes temperature valid"); } #[test] - fn rejects_anthropic_temperature_above_one_in_arbitrary_patch() { + fn applies_format_ranges_without_model_name_heuristics() { let patch = json!({ "prompt_data": { "options": { - "model": "claude-sonnet-4-5", + "model": "test-custom-model", "params": { "temperature": 1.5 } } } }); - let error = validate_prompt_data_patch(None, &patch) + let anthropic = model_spec("anthropic", false, false, None); + let error = validate_patch(None, &patch, Some(&anthropic)) .expect_err("Anthropic temperature should use the smaller range"); assert_eq!(error.to_string(), "--temperature must be between 0 and 1"); + + validate_patch(None, &patch, None) + .expect("unknown custom models should not be assigned a format by name"); + } + + #[test] + fn applies_web_ui_parameter_availability_and_model_token_limit() { + let window = model_spec("window", false, false, None); + let tool_patch = json!({ + "prompt_data": { + "options": { + "model": "test-window-model", + "params": { "tool_choice": "auto" } + } + } + }); + let error = validate_patch(None, &tool_patch, Some(&window)) + .expect_err("Window models do not expose tool choice in the UI"); + assert!(error.to_string().contains("--tool-choice is not supported")); + + let openai = model_spec("openai", false, false, Some(4096)); + let token_patch = json!({ + "prompt_data": { + "options": { + "model": "test-limited-model", + "params": { "max_tokens": 4097 } + } + } + }); + let error = validate_patch(None, &token_patch, Some(&openai)) + .expect_err("model output token limit should be enforced"); + assert!(error.to_string().contains("between 1 and 4096")); + } + + #[test] + fn applies_web_ui_reasoning_options() { + let reasoning = model_spec("openai", true, false, None); + let invalid = json!({ + "prompt_data": { + "options": { + "model": "o3", + "params": { "reasoning_effort": "minimal" } + } + } + }); + let error = validate_patch(None, &invalid, Some(&reasoning)) + .expect_err("generic reasoning models accept low, medium, or high"); + assert!(error.to_string().contains("low, medium, high")); + + let non_reasoning = model_spec("openai", false, false, None); + let non_reasoning_patch = json!({ + "prompt_data": { + "options": { + "model": "gpt-4.1", + "params": { "reasoning_effort": "low" } + } + } + }); + let error = validate_patch(None, &non_reasoning_patch, Some(&non_reasoning)) + .expect_err("non-reasoning models should reject reasoning effort"); + assert!(error.to_string().contains("not supported")); + } + + #[test] + fn validates_existing_parameters_when_switching_models() { + let existing = json!({ + "options": { + "model": "gpt-4.1", + "params": { "temperature": 0.5 } + } + }); + let patch = json!({ + "prompt_data": { + "options": { "model": "o3" } + } + }); + let error = validate_patch( + Some(&existing), + &patch, + Some(&model_spec("openai", true, false, None)), + ) + .expect_err("switching models must validate retained parameters"); + assert!(error.to_string().contains("--temperature is not supported")); } #[test] - fn ignores_unrelated_updates_to_existing_model_configuration() { + fn validates_only_parameters_touched_by_an_update() { let existing = json!({ "options": { - "model": "gpt-4.1-mini", + "model": "test-model", "params": { "temperature": 99 } } }); - validate_prompt_data_patch(Some(&existing), &json!({"description": "Updated"})) + validate_patch( + Some(&existing), + &json!({"prompt_data": {"options": {"params": {"top_p": 0.5}}}}), + Some(&model_spec("openai", false, false, None)), + ) + .expect("an unrelated stale parameter should not block an update"); + + validate_patch(Some(&existing), &json!({"description": "Updated"}), None) .expect("an unrelated metadata update should remain possible"); } diff --git a/src/functions/update.rs b/src/functions/update.rs index da64d943..86eb29c3 100644 --- a/src/functions/update.rs +++ b/src/functions/update.rs @@ -164,7 +164,11 @@ pub async fn run( let body = build_patch_body(args)?; let function = resolve_target_function(ctx, args, ft).await?; - validate_prompt_data_patch(function.prompt_data.as_ref(), &body)?; + with_spinner( + "Validating model parameters...", + validate_prompt_data_patch(ctx, function.prompt_data.as_ref(), &body), + ) + .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 diff --git a/src/prompts/update.rs b/src/prompts/update.rs index 511c488b..34349f56 100644 --- a/src/prompts/update.rs +++ b/src/prompts/update.rs @@ -91,7 +91,11 @@ pub async fn run(ctx: &ResolvedContext, args: &UpdateArgs, json_output: bool) -> } }; - validate_prompt_data_patch(prompt.prompt_data.as_ref(), &body)?; + with_spinner( + "Validating model parameters...", + validate_prompt_data_patch(ctx, prompt.prompt_data.as_ref(), &body), + ) + .await?; if !args.yes && is_interactive() { let confirm = Confirm::new() From 27085652b37d2b05aae7c0664a8d9070eddc74ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Thu, 30 Jul 2026 19:31:22 -0700 Subject: [PATCH 6/7] chore: allow --force when selecting scorer to delete interactively --- src/functions/delete.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/functions/delete.rs b/src/functions/delete.rs index 96df5fb9..5885d245 100644 --- a/src/functions/delete.rs +++ b/src/functions/delete.rs @@ -12,13 +12,6 @@ pub async fn run( force: bool, ft: Option, ) -> Result<()> { - if force && slug.is_none() { - bail!( - "slug required when using --force. Use: bt {} delete --force", - label_plural(ft), - ); - } - let project_id = &ctx.project.id; let function = match slug { From 27611e96f5133485eff8564298814f2b8dfdf779 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Halber?= Date: Thu, 30 Jul 2026 21:08:16 -0700 Subject: [PATCH 7/7] fix: api url being ignored --- src/auth.rs | 69 +++++++++++++++++++++++++++-- src/functions/model_capabilities.rs | 4 +- src/functions/prompt_config.rs | 1 - 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index a915a636..424bee3a 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -557,15 +557,13 @@ pub async fn login(base: &BaseArgs) -> Result { Err(err) => return Err(err.into()), }; - let api_url = login - .api_url() - .or(auth.api_url.clone()) - .unwrap_or_else(|| DEFAULT_API_URL.to_string()); + let api_url = resolve_login_api_url(auth.api_url.clone(), login.api_url()); let app_url = auth .app_url .clone() .unwrap_or_else(|| DEFAULT_APP_URL.to_string()); + let login = normalize_login_state(login, api_key, &api_url, &app_url); let ctx = LoginContext { login, @@ -576,6 +574,35 @@ pub async fn login(base: &BaseArgs) -> Result { Ok(ctx) } +fn resolve_login_api_url(configured: Option, discovered: Option) -> String { + // The configured CLI/env/profile URL is the request target. Do not let a + // cached or server-returned login URL silently replace it. + configured + .or(discovered) + .unwrap_or_else(|| DEFAULT_API_URL.to_string()) +} + +fn normalize_login_state( + login: LoginState, + api_key: String, + api_url: &str, + app_url: &str, +) -> LoginState { + // Keep LoginContext's two URL sources consistent. Most commands use + // LoginContext::api_url through ApiClient, but SDK-backed paths may inspect + // LoginState directly. + let normalized = LoginState::new(); + let did_set = normalized.set( + api_key, + login.org_id().unwrap_or_default(), + login.org_name().unwrap_or_default(), + api_url.to_string(), + app_url.to_string(), + ); + debug_assert!(did_set, "new login state should be unset"); + normalized +} + #[derive(Debug, Deserialize)] struct AiProviderSecret { #[serde(default)] @@ -4001,6 +4028,40 @@ mod tests { } } + #[test] + fn configured_urls_override_discovered_login_state() { + let discovered = LoginState::new(); + assert!(discovered.set( + "test-api-key".to_string(), + "org_test".to_string(), + "test-org".to_string(), + DEFAULT_API_URL.to_string(), + DEFAULT_APP_URL.to_string(), + )); + let api_url = resolve_login_api_url( + Some("https://api.test.example".to_string()), + discovered.api_url(), + ); + + let normalized = normalize_login_state( + discovered, + "test-api-key".to_string(), + &api_url, + "https://app.test.example", + ); + + assert_eq!( + normalized.api_url().as_deref(), + Some("https://api.test.example") + ); + assert_eq!( + normalized.app_url().as_deref(), + Some("https://app.test.example") + ); + assert_eq!(normalized.org_id().as_deref(), Some("org_test")); + assert_eq!(normalized.org_name().as_deref(), Some("test-org")); + } + fn assert_invalid_api_url(result: Result) { assert_err_contains(result, "invalid api_url"); } diff --git a/src/functions/model_capabilities.rs b/src/functions/model_capabilities.rs index f0fc9a2b..a09d2b95 100644 --- a/src/functions/model_capabilities.rs +++ b/src/functions/model_capabilities.rs @@ -12,9 +12,8 @@ const MODEL_CATALOG_TIMEOUT: Duration = Duration::from_secs(5); /// can extend the catalog without breaking older CLI versions. #[derive(Debug, Clone, Deserialize)] pub(crate) struct ModelSpec { + #[serde(default)] pub(crate) format: String, - #[serde(rename = "flavor")] - pub(crate) _flavor: String, #[serde(default, rename = "displayName")] pub(crate) display_name: Option, #[serde(default)] @@ -157,7 +156,6 @@ mod tests { fn spec(format: &str, display_name: Option<&str>) -> ModelSpec { ModelSpec { format: format.to_string(), - _flavor: "chat".to_string(), display_name: display_name.map(ToOwned::to_owned), o1_like: None, reasoning: None, diff --git a/src/functions/prompt_config.rs b/src/functions/prompt_config.rs index 00a398d1..66ca96b7 100644 --- a/src/functions/prompt_config.rs +++ b/src/functions/prompt_config.rs @@ -692,7 +692,6 @@ mod tests { ) -> ModelSpec { ModelSpec { format: format.to_string(), - _flavor: "chat".to_string(), display_name: None, o1_like: None, reasoning: Some(reasoning),