Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions cli/src/builtins/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ use crate::llm::{LLMSampler, Sampler};
use crate::metrics_store::MetricsStore;

/// Fields shared by every builtin benchmark config. When adding a new builtin,
/// embed this with `#[serde(flatten)]` so that `limit`, `model`, and
/// embed this with `#[serde(flatten)]` so that `samples`, `model`, and
/// `max_workers` are automatically supported without duplication.
#[derive(Debug, Default, Deserialize)]
pub(crate) struct BuiltinConfig {
/// Number of dataset rows to evaluate. If omitted, the entire dataset is used.
#[serde(default)]
pub(crate) limit: Option<usize>,
///
/// This field used to be called `limit`, so we are aliasing it to provide
/// backward compatibility.
#[serde(default, alias = "limit")]
pub(crate) samples: Option<usize>,
/// Which model sampler to use. If omitted, the builtin chooses a sensible default.
#[serde(default)]
pub(crate) model: Option<Sampler>,
Expand Down Expand Up @@ -228,6 +231,13 @@ mod tests {
use super::*;
use rstest::rstest;

#[test]
fn builtin_config_accepts_samples() {
let config: BuiltinConfig = serde_json::from_str(r#"{"samples":10}"#).unwrap();

assert_eq!(config.samples, Some(10));
}

#[rstest]
#[case("hello", Some("hello"))]
#[case("", Some(""))]
Expand Down
11 changes: 7 additions & 4 deletions cli/src/builtins/custom_nocode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ impl BuiltinWorkflow for CustomNoCodeBuiltin {
config.dataset.config_name.as_deref(),
config.dataset.split.as_deref(),
config.dataset.revision.as_deref(),
config.limit,
config.samples,
)
.await?;
config.limit = Some(limit);
config.samples = Some(limit);
crate::db::set_run_input(ctx.db, ctx.run_id, &serde_json::to_string(&config)?).await?;

let db = ctx.db.clone();
Expand Down Expand Up @@ -214,7 +214,7 @@ mod tests {
"dataset": {"name": "fixture/qa"},
"model": "random",
"prompt_template_file": template_path.to_str().unwrap(),
"limit": 2,
"samples": 2,
}))
.unwrap();

Expand Down Expand Up @@ -257,7 +257,10 @@ mod tests {
let recorded_input: serde_json::Value =
serde_json::from_str(recorded_run.input.as_deref().unwrap()).unwrap();
assert_eq!(recorded_input["style"]["type"], "exact_match");
assert_eq!(recorded_input["limit"], 2);
assert_eq!(recorded_input["samples"], 2);
let recorded_output: serde_json::Value =
serde_json::from_str(recorded_run.output.as_deref().unwrap()).unwrap();
assert_eq!(recorded_output, json!({"samples": 2}));

let all_metrics = metrics_store.list_for_run(run_id).await.unwrap();
let is_correct_count = all_metrics
Expand Down
6 changes: 3 additions & 3 deletions cli/src/builtins/custom_nocode/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ pub(super) fn parse_input(input: Option<&str>) -> Result<crate::config::CustomNo
.context("invalid builtin input JSON")?
.context("custom_nocode benchmark requires input configuration")?;

if config.limit == Some(0) {
bail!("limit must be > 0");
if config.samples == Some(0) {
bail!("samples must be > 0");
}

Ok(config)
Expand Down Expand Up @@ -65,7 +65,7 @@ pub(super) async fn resolve_dataset_limit(

let total = info
.total_rows
.context("could not determine dataset size; pass an explicit limit")?;
.context("could not determine dataset size; pass an explicit samples value")?;
let limit = limit.unwrap_or(total).min(total);

Ok((manager, info, limit))
Expand Down
25 changes: 22 additions & 3 deletions cli/src/builtins/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::llm::Sampler;
#[derive(Serialize)]
pub(crate) struct BuiltinRunInput {
pub(crate) model: String,
pub(crate) num_samples: usize,
pub(crate) samples: usize,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) max_workers: Option<usize>,
}
Expand All @@ -16,12 +16,12 @@ pub(crate) async fn set_builtin_run_input(
db: &sea_orm::DatabaseConnection,
run_id: i64,
model: Option<&Sampler>,
num_samples: usize,
samples: usize,
max_workers: Option<usize>,
) -> anyhow::Result<()> {
let input = serde_json::to_string(&BuiltinRunInput {
model: builtin_model_name(model),
num_samples,
samples,
max_workers,
})?;
crate::db::set_run_input(db, run_id, &input).await
Expand All @@ -35,3 +35,22 @@ fn builtin_model_name(model: Option<&Sampler>) -> String {
Some(other) => other.to_string(),
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn normalized_input_uses_samples() {
let input = BuiltinRunInput {
model: "demo-builtin".to_owned(),
samples: 10,
max_workers: None,
};

assert_eq!(
serde_json::to_value(input).unwrap(),
serde_json::json!({"model": "demo-builtin", "samples": 10})
);
}
}
21 changes: 18 additions & 3 deletions cli/src/builtins/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,30 @@ use serde::Serialize;
/// Normalized run output schema for all builtins.
#[derive(Serialize)]
pub(crate) struct BuiltinRunOutput {
pub(crate) samples_completed: usize,
pub(crate) samples: usize,
}

/// Rewrite the run record output to the normalized builtin shape.
pub(crate) async fn set_builtin_run_output(
db: &sea_orm::DatabaseConnection,
run_id: i64,
samples_completed: usize,
samples: usize,
) -> anyhow::Result<()> {
let output = serde_json::to_string(&BuiltinRunOutput { samples_completed })?;
let output = serde_json::to_string(&BuiltinRunOutput { samples })?;
crate::db::set_run_output(db, run_id, &output).await
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn normalized_output_uses_samples() {
let output = BuiltinRunOutput { samples: 10 };

assert_eq!(
serde_json::to_value(output).unwrap(),
serde_json::json!({"samples": 10})
);
}
}
2 changes: 1 addition & 1 deletion cli/src/builtins/pubmedqa/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::builtins::common::BuiltinConfig;

/// Parsed user input for the builtin.
///
/// All common fields (`limit`, `model`, `max_workers`) live in [`BuiltinConfig`]
/// All common fields (`samples`, `model`, `max_workers`) live in [`BuiltinConfig`]
/// and are flattened during deserialization so that the TOML surface stays flat.
#[derive(Debug, Default, Deserialize)]
pub(crate) struct PubMedQAConfig {
Expand Down
8 changes: 4 additions & 4 deletions cli/src/builtins/pubmedqa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ impl BuiltinWorkflow for PubmedqaBuiltin {
.context("invalid builtin input JSON")?
.unwrap_or_default();

if config.base.limit == Some(0) {
bail!("limit must be > 0");
if config.base.samples == Some(0) {
bail!("samples must be > 0");
}

let max_workers = config.base.max_workers.unwrap_or_else(get_max_workers);
Expand All @@ -55,8 +55,8 @@ impl BuiltinWorkflow for PubmedqaBuiltin {

let total = info
.total_rows
.context("could not determine dataset size; pass an explicit limit")?;
let limit = config.base.limit.unwrap_or(total).min(total);
.context("could not determine dataset size; pass an explicit samples value")?;
let limit = config.base.samples.unwrap_or(total).min(total);

set_builtin_run_input(
ctx.db,
Expand Down
10 changes: 5 additions & 5 deletions cli/src/builtins/similarity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::similarity::{

/// Configuration shared by all similarity-based builtins.
///
/// All common fields (`limit`, `model`, `max_workers`) live in [`BuiltinConfig`]
/// All common fields (`samples`, `model`, `max_workers`) live in [`BuiltinConfig`]
/// and are flattened during deserialization so that the TOML surface stays flat.
#[derive(Debug, Default, Deserialize)]
struct SimilarityConfig {
Expand Down Expand Up @@ -80,8 +80,8 @@ impl BuiltinWorkflow for SimilarityBenchmark {
.context("invalid builtin input JSON")?
.unwrap_or_default();

if config.base.limit == Some(0) {
bail!("limit must be > 0");
if config.base.samples == Some(0) {
bail!("samples must be > 0");
}

let metric: Box<dyn SimilarityMetric> = match config.metric {
Expand All @@ -98,8 +98,8 @@ impl BuiltinWorkflow for SimilarityBenchmark {

let total = info
.total_rows
.context("could not determine dataset size; pass an explicit limit")?;
let limit = config.base.limit.unwrap_or(total).min(total);
.context("could not determine dataset size; pass an explicit samples value")?;
let limit = config.base.samples.unwrap_or(total).min(total);

let db = ctx.db;
let run_id = ctx.run_id;
Expand Down
6 changes: 3 additions & 3 deletions cli/src/commands/resume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ mod tests {
},
model: Some(qt::llm::Sampler::Random {}),
prompt_template_file: file.path().to_str().unwrap().to_owned(),
limit: None,
samples: None,
max_workers: None,
metrics: Vec::new(),
style: qt::config::CustomNoCodeStyleConfig::ExactMatch {
Expand Down Expand Up @@ -319,7 +319,7 @@ style = {{ type = "exact_match", golden_column = "answer" }}
dataset = {{ name = "fixture/qa" }}
model = "random"
prompt_template_file = "{}"
limit = 2
samples = 2
"#,
template_path.to_str().unwrap()
),
Expand All @@ -330,7 +330,7 @@ limit = 2
// reconstruct the executable configuration from quantiles.toml.
let input_json = serde_json::to_string(&serde_json::json!({
"model": "demo-builtin",
"num_samples": 2,
"samples": 2,
}))
.unwrap();

Expand Down
Loading
Loading