diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 76499ad138..8fd4caefb8 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -82,6 +82,11 @@ const overrides = new Map([ // command's SQLite core end to end. Test-only content; ratcheted // 1208 -> 1424. ["src-tauri/src/archive/mod_tests.rs", 1424], + // agent-usage-harness: M1 migration (atomic transaction wrapper, PRAGMA + // guard, canonical-store worklist) + rebuild_metric_index_in_tx helper + // + apply_schema_migrations + store_migration_tests.rs #[path] include. + // Load-bearing correctness fix (Thufir IMPORTANT). M1 tests split out. + ["src-tauri/src/archive/store.rs", 1044], // unified-agent-model 1A.1: profile reconcile split to agents_profile.rs, // ratcheting 1443 -> 1295. Queued to split further in the A2 fold. // global-agent-config: resolve_deploy_model_provider + visibility exports diff --git a/desktop/src-tauri/src/archive/agent_usage.rs b/desktop/src-tauri/src/archive/agent_usage.rs index 431c068a1b..5c9917cc77 100644 --- a/desktop/src-tauri/src/archive/agent_usage.rs +++ b/desktop/src-tauri/src/archive/agent_usage.rs @@ -125,6 +125,7 @@ pub struct SeriesBucket { #[derive(Debug, Clone, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct ModelUsage { + pub harness: Option, pub model: Option, pub usage: ReportedUsage, pub report_count: i64, @@ -456,6 +457,32 @@ fn assign_bucket_index(boundaries: &[i64], t: i64) -> Option { (0..boundaries.len().saturating_sub(1)).find(|&i| t >= boundaries[i] && t < boundaries[i + 1]) } +// ── Sort helpers ───────────────────────────────────────────────────────────── + +/// Compare two known-total values for the A2 ranking rule: known totals rank +/// before unknown totals (None); within known totals, descending; within +/// unknown totals, equal (callers chain further tiebreaks). +fn cmp_known_total(a: Option, b: Option) -> std::cmp::Ordering { + match (a, b) { + (Some(av), Some(bv)) => bv.cmp(&av), // descending + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + } +} + +/// Ordinal (byte-order) comparison of two optional strings, with `None` +/// sorting after any `Some` value — used as a stable tiebreak for harness and +/// model names so ordering is locale-independent and matches the Rust backend. +fn cmp_option_str_none_last(a: &Option, b: &Option) -> std::cmp::Ordering { + match (a, b) { + (Some(av), Some(bv)) => av.cmp(bv), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + } +} + // ── compute_series ─────────────────────────────────────────────────────────── /// Per-agent accumulation scope, built while walking `window_rows` once. @@ -464,7 +491,8 @@ struct AgentScope { bucket_counts: Vec, total: UsageAccumulator, report_count: i64, - models: HashMap, (UsageAccumulator, i64)>, + /// Keyed by `(harness, model)` — same model via two harnesses → two rows. + models: HashMap<(Option, Option), (UsageAccumulator, i64)>, } /// Compute the full [`AgentUsageSeries`] from already-loaded rows. @@ -551,7 +579,7 @@ pub(super) fn compute_series( let model_entry = scope .models - .entry(row.model.clone()) + .entry((row.harness.clone(), row.model.clone())) .or_insert_with(|| (UsageAccumulator::default(), 0i64)); model_entry.0.add(&outcome); model_entry.1 += 1; @@ -602,31 +630,43 @@ pub(super) fn compute_series( }) .collect(); - let mut model_rows: Vec<(Option, Option, ModelUsage)> = scope + // Named sort key so the 4-tuple doesn't exceed clippy's type_complexity + // threshold and the ordering intent reads at a glance. + struct ModelSortKey { + total: Option, + harness: Option, + model: Option, + usage: ModelUsage, + } + let mut model_rows: Vec = scope .models .into_iter() - .map(|(model, (acc, count))| { - let model_total = acc.total_tokens_value(); + .map(|((harness, model), (acc, count))| { + let total = acc.total_tokens_value(); let has_unknown_usage = acc.has_unknown(); - ( - model_total, - model.clone(), - ModelUsage { + ModelSortKey { + total, + harness: harness.clone(), + model: model.clone(), + usage: ModelUsage { + harness, model, usage: acc.finish(), report_count: count, has_unknown_usage, }, - ) + } }) .collect(); - model_rows.sort_by(|a, b| match (a.0, b.0) { - (Some(av), Some(bv)) => bv.cmp(&av).then_with(|| a.1.cmp(&b.1)), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => a.1.cmp(&b.1), + // A2 ranking: known totalTokens descending, unknown-total after; + // tiebreak: harness ascending (None last), then model ascending + // (None last), for deterministic ordering. + model_rows.sort_by(|a, b| { + cmp_known_total(a.total, b.total) + .then_with(|| cmp_option_str_none_last(&a.harness, &b.harness)) + .then_with(|| cmp_option_str_none_last(&a.model, &b.model)) }); - let models = model_rows.into_iter().map(|(_, _, m)| m).collect(); + let models = model_rows.into_iter().map(|k| k.usage).collect(); ( total_tokens_value, @@ -642,12 +682,7 @@ pub(super) fn compute_series( ) }) .collect(); - agent_rows.sort_by(|a, b| match (a.0, b.0) { - (Some(av), Some(bv)) => bv.cmp(&av).then_with(|| a.1.cmp(&b.1)), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => a.1.cmp(&b.1), - }); + agent_rows.sort_by(|a, b| cmp_known_total(a.0, b.0).then_with(|| a.1.cmp(&b.1))); let any_agent_unknown = agent_rows.iter().any(|(_, _, a)| a.has_unknown_usage); let agents: Vec = agent_rows.into_iter().map(|(_, _, a)| a).collect(); diff --git a/desktop/src-tauri/src/archive/agent_usage_tests.rs b/desktop/src-tauri/src/archive/agent_usage_tests.rs index 4e08c31c72..e5974ce2d6 100644 --- a/desktop/src-tauri/src/archive/agent_usage_tests.rs +++ b/desktop/src-tauri/src/archive/agent_usage_tests.rs @@ -18,6 +18,7 @@ fn row(id: &str, agent: &str, session: &str, seq: u64, reported_at: i64) -> Agen reported_at: Some(reported_at), session_id: Some(session.to_string()), turn_seq: Some(seq), + harness: None, model: None, delta_reliable: Some(true), turn_input_tokens: None, @@ -590,6 +591,73 @@ fn compute_series_model_breakdown_attributes_per_event_model() { assert_eq!(series.agents[0].models.len(), 2); } +#[test] +fn compute_series_same_model_two_harnesses_produces_two_rows() { + // Collapse-fix requirement: same model via two different harnesses must + // NOT collapse into one row — (harness, model) is the grouping key. + let r1 = AgentMetricIndexRow { + harness: Some("goose".to_string()), + model: Some("claude-sonnet".to_string()), + turn_input_tokens: Some(100), + delta_reliable: Some(true), + ..row("e1", "agent1", "s1", 1, 0) + }; + let r2 = AgentMetricIndexRow { + harness: Some("claude-code".to_string()), + model: Some("claude-sonnet".to_string()), + turn_input_tokens: Some(200), + delta_reliable: Some(true), + ..row("e2", "agent1", "s2", 1, 1) + }; + let boundaries = boundaries_7(); + let rows = vec![r1, r2]; + let series = compute_series(&rows, &rows, 0, &boundaries, None, true); + assert_eq!(series.agents.len(), 1); + // Two distinct harnesses → two rows, not one collapsed row. + assert_eq!( + series.agents[0].models.len(), + 2, + "same model under two harnesses must produce two rows" + ); + let harneses: Vec> = series.agents[0] + .models + .iter() + .map(|m| m.harness.as_deref()) + .collect(); + assert!( + harneses.contains(&Some("claude-code")), + "claude-code row must be present" + ); + assert!( + harneses.contains(&Some("goose")), + "goose row must be present" + ); +} + +#[test] +fn compute_series_single_harness_data_looks_unchanged_with_harness_label() { + // Single-harness data: still exactly one row per model, harness label present. + let r = AgentMetricIndexRow { + harness: Some("goose".to_string()), + model: Some("claude-sonnet".to_string()), + turn_total_tokens: Some(500), + delta_reliable: Some(true), + ..row("e1", "agent1", "s1", 1, 0) + }; + let boundaries = boundaries_7(); + let rows = vec![r]; + let series = compute_series(&rows, &rows, 0, &boundaries, None, true); + assert_eq!(series.agents[0].models.len(), 1); + assert_eq!( + series.agents[0].models[0].harness, + Some("goose".to_string()) + ); + assert_eq!( + series.agents[0].models[0].model, + Some("claude-sonnet".to_string()) + ); +} + #[test] fn compute_series_invalid_report_count_passed_through_and_not_bucketed() { let boundaries = boundaries_7(); diff --git a/desktop/src-tauri/src/archive/metric_store.rs b/desktop/src-tauri/src/archive/metric_store.rs index a2c739dc84..88e77180b1 100644 --- a/desktop/src-tauri/src/archive/metric_store.rs +++ b/desktop/src-tauri/src/archive/metric_store.rs @@ -80,6 +80,7 @@ pub(super) struct AgentMetricIndexRow { pub reported_at: Option, pub session_id: Option, pub turn_seq: Option, + pub harness: Option, pub model: Option, pub delta_reliable: Option, pub turn_input_tokens: Option, @@ -141,6 +142,7 @@ impl AgentMetricIndexRow { reported_at, session_id: payload.session_id, turn_seq: payload.turn_seq, + harness: Some(payload.harness), model: payload.model, delta_reliable: Some(payload.delta_reliable), turn_input_tokens: turn.and_then(|t| t.input_tokens), @@ -164,6 +166,7 @@ impl AgentMetricIndexRow { reported_at: None, session_id: None, turn_seq: None, + harness: None, model: None, delta_reliable: None, turn_input_tokens: None, @@ -208,6 +211,7 @@ fn row_from_sql(row: &rusqlite::Row) -> rusqlite::Result { reported_at: row.get("reported_at")?, session_id: row.get("session_id")?, turn_seq: turn_seq_text.as_deref().and_then(decode_u64_sortable), + harness: row.get("harness")?, model: row.get("model")?, delta_reliable: delta_reliable_int.map(|v| v != 0), turn_input_tokens: turn_input_text.as_deref().and_then(decode_u64_sortable), @@ -223,7 +227,7 @@ fn row_from_sql(row: &rusqlite::Row) -> rusqlite::Result { } const ROW_COLUMNS: &str = "id, agent_pubkey, event_created_at, archived_at, reported_at, \ - session_id, turn_seq, model, delta_reliable, turn_input_tokens, turn_output_tokens, \ + session_id, turn_seq, harness, model, delta_reliable, turn_input_tokens, turn_output_tokens, \ turn_total_tokens, turn_cost_usd, cumulative_input_tokens, cumulative_output_tokens, \ cumulative_total_tokens, cumulative_cost_usd, parse_status"; @@ -256,13 +260,13 @@ pub(super) fn insert_metric_index_row( .execute( "INSERT INTO agent_metric_index (identity_pubkey, relay_url, id, agent_pubkey, event_created_at, - archived_at, reported_at, session_id, turn_seq, model, + archived_at, reported_at, session_id, turn_seq, harness, model, delta_reliable, turn_input_tokens, turn_output_tokens, turn_total_tokens, turn_cost_usd, cumulative_input_tokens, cumulative_output_tokens, cumulative_total_tokens, cumulative_cost_usd, parse_status) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, - ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20) + ?11, ?12, ?13, ?14, ?15, ?16, ?17, ?18, ?19, ?20, ?21) ON CONFLICT (identity_pubkey, relay_url, id) DO NOTHING", params![ identity_pubkey, @@ -274,6 +278,7 @@ pub(super) fn insert_metric_index_row( row.reported_at, row.session_id, turn_seq, + row.harness, row.model, delta_reliable, turn_input, diff --git a/desktop/src-tauri/src/archive/metric_store_tests.rs b/desktop/src-tauri/src/archive/metric_store_tests.rs index 48fa091fdf..5f6b849028 100644 --- a/desktop/src-tauri/src/archive/metric_store_tests.rs +++ b/desktop/src-tauri/src/archive/metric_store_tests.rs @@ -87,6 +87,15 @@ fn from_payload_parses_valid_row() { assert_eq!(row.turn_input_tokens, Some(10)); assert_eq!(row.cumulative_input_tokens, Some(100)); assert_eq!(row.model, Some("claude".to_string())); + assert_eq!(row.harness, Some("goose".to_string())); +} + +#[test] +fn from_payload_parses_harness_field() { + let json = r#"{"harness":"claude-code","model":"claude-sonnet","timestamp":"2026-07-01T20:11:03Z","turn":{"inputTokens":5,"outputTokens":null,"totalTokens":null,"costUsd":null}}"#; + let row = AgentMetricIndexRow::from_payload(json, "eid1", "agent1", 100, 200); + assert_eq!(row.parse_status, ParseStatus::Valid); + assert_eq!(row.harness, Some("claude-code".to_string())); } #[test] @@ -577,3 +586,68 @@ fn predecessor_lookup_uses_session_index() { "predecessor lookup must use the session index, plan was:\n{plan}" ); } + +// ── Migration: old-shape rows get harness populated on rebuild ──────────────── + +/// Simulate a pre-harness archive row by inserting an archived_event and an +/// index row with harness = NULL (as the old schema would have stored it), +/// then running the backfill rebuild path to verify harness is populated. +#[test] +fn migration_old_shape_rows_get_harness_populated_after_rebuild() { + let conn = in_memory(); + let json = valid_payload_json("s1", 1, "2026-07-01T00:00:00Z"); + // Seed an archived event. + insert_archived_event( + &conn, "id", "relay", "eid1", 44200, "agent1", 100, &json, 200, + ); + // Seed an index row with harness explicitly NULL (simulates pre-migration row). + conn.execute( + "INSERT INTO agent_metric_index + (identity_pubkey, relay_url, id, agent_pubkey, event_created_at, + archived_at, reported_at, session_id, turn_seq, harness, model, + delta_reliable, turn_input_tokens, turn_output_tokens, + turn_total_tokens, turn_cost_usd, cumulative_input_tokens, + cumulative_output_tokens, cumulative_total_tokens, + cumulative_cost_usd, parse_status) + VALUES ('id','relay','eid1','agent1',100,200,1751414400000, + 's1','00000000000000000001',NULL,'claude', + 1,'00000000000000000010','00000000000000000020', + '00000000000000000030',0.01, + '00000000000000000100','00000000000000000200', + '00000000000000000300',0.1,'valid')", + [], + ) + .unwrap(); + + // Verify the row is there with NULL harness. + let harness_before: Option = conn + .query_row( + "SELECT harness FROM agent_metric_index WHERE id = 'eid1'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + harness_before, None, + "old-shape row should start with NULL harness" + ); + + // Simulate the migration: delete index rows and re-run backfill. + conn.execute_batch("DELETE FROM agent_metric_index") + .unwrap(); + backfill_agent_metric_index(&conn, "id", "relay").unwrap(); + + // The rebuilt row must have harness populated. + let harness_after: Option = conn + .query_row( + "SELECT harness FROM agent_metric_index WHERE id = 'eid1'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + harness_after, + Some("goose".to_string()), + "rebuilt row must have harness populated from raw_json" + ); +} diff --git a/desktop/src-tauri/src/archive/store.rs b/desktop/src-tauri/src/archive/store.rs index d3e469c6b5..7e9054cc6b 100644 --- a/desktop/src-tauri/src/archive/store.rs +++ b/desktop/src-tauri/src/archive/store.rs @@ -105,6 +105,7 @@ CREATE TABLE IF NOT EXISTS agent_metric_index ( cumulative_output_tokens TEXT, cumulative_total_tokens TEXT, cumulative_cost_usd REAL, + harness TEXT, parse_status TEXT NOT NULL CHECK (parse_status IN ('valid','invalid')), PRIMARY KEY (identity_pubkey, relay_url, id) ); @@ -153,9 +154,172 @@ pub fn open_archive_db(path: &Path) -> Result { conn.execute_batch(SCHEMA) .map_err(|e| format!("failed to initialize archive schema: {e}"))?; + apply_schema_migrations(&conn)?; + Ok(conn) } +/// One-shot, idempotent schema migrations recorded in `archive_migrations`. +/// +/// Each migration is guarded by a `SELECT 1 FROM archive_migrations WHERE +/// name = '...'` check so it is safe to call on every open — a migration +/// that already ran is a no-op. +fn apply_schema_migrations(conn: &Connection) -> Result<(), String> { + migrate_add_harness_to_metric_index(conn) +} + +/// M1: add `harness TEXT` column to `agent_metric_index` and rebuild index +/// rows so pre-existing rows gain harness populated from `archived_events`. +/// +/// The entire migration — column addition, full DELETE of stale index rows, +/// fresh re-insertion from `archived_events.raw_json`, and the +/// `archive_migrations` marker — is wrapped in a single `unchecked_transaction` +/// (BEGIN DEFERRED). A crash anywhere before COMMIT leaves the DB in its +/// pre-migration state and the marker absent, so the next open re-runs the +/// migration from scratch. +/// +/// The worklist is built from `archived_events` (the canonical store) rather +/// than from `agent_metric_index` so that a partially-rebuilt or entirely +/// empty index never causes us to forget which (identity, relay) scopes exist. +fn migrate_add_harness_to_metric_index(conn: &Connection) -> Result<(), String> { + let already_run: bool = conn + .query_row( + "SELECT COUNT(*) FROM archive_migrations WHERE name = 'add_harness_to_metric_index'", + [], + |r| r.get::<_, i64>(0), + ) + .map_err(|e| format!("migration M1: guard check: {e}"))? + > 0; + + if already_run { + return Ok(()); + } + + // All steps run inside one transaction so a crash before COMMIT leaves the + // DB fully pre-migration and the marker absent — the next open re-runs. + let tx = conn + .unchecked_transaction() + .map_err(|e| format!("migration M1: begin transaction: {e}"))?; + + // 1. Add the `harness` column only when it is genuinely absent, so we + // propagate unexpected DDL errors instead of swallowing them. + let harness_exists: bool = { + let mut stmt = tx + .prepare("PRAGMA table_info(agent_metric_index)") + .map_err(|e| format!("migration M1: PRAGMA table_info prepare: {e}"))?; + let names: Vec = stmt + .query_map([], |row| row.get::<_, String>(1)) + .map_err(|e| format!("migration M1: PRAGMA table_info query: {e}"))? + .collect::, _>>() + .map_err(|e| format!("migration M1: PRAGMA table_info read: {e}"))?; + names.iter().any(|n| n == "harness") + }; + if !harness_exists { + tx.execute_batch("ALTER TABLE agent_metric_index ADD COLUMN harness TEXT") + .map_err(|e| format!("migration M1: ALTER TABLE failed: {e}"))?; + } + + // 2. Collect all (identity, relay) scopes from `archived_events` — the + // canonical store — so the worklist is correct even if the index was + // partially rebuilt or empty before this migration runs. + let scopes: Vec<(String, String)> = { + let mut stmt = tx + .prepare( + "SELECT DISTINCT identity_pubkey, relay_url + FROM archived_events + WHERE kind = 44200", + ) + .map_err(|e| format!("migration M1: prepare scope query: {e}"))?; + let result = stmt + .query_map([], |row| Ok((row.get(0)?, row.get(1)?))) + .map_err(|e| format!("migration M1: query scopes: {e}"))? + .collect::, _>>() + .map_err(|e| format!("migration M1: read scopes: {e}"))?; + result + }; + + if !scopes.is_empty() { + // 3. Delete all stale index rows; re-insert them with harness populated + // from the shared `from_payload` parser below. Both the DELETE and + // all inserts run inside the same outer transaction — no intermediate + // commit, so readers never observe an empty index. + tx.execute_batch("DELETE FROM agent_metric_index") + .map_err(|e| format!("migration M1: delete index rows: {e}"))?; + + for (identity, relay) in &scopes { + rebuild_metric_index_in_tx(&tx, identity, relay)?; + } + } + + // 4. Record migration as applied — written last so the marker is only + // present in a fully committed transaction. + tx.execute( + "INSERT OR IGNORE INTO archive_migrations (name, applied_at) \ + VALUES ('add_harness_to_metric_index', ?1)", + params![std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs() as i64], + ) + .map_err(|e| format!("migration M1: record marker: {e}"))?; + + tx.commit() + .map_err(|e| format!("migration M1: commit: {e}"))?; + + Ok(()) +} + +/// Re-insert all kind-44200 rows for `(identity, relay)` from +/// `archived_events.raw_json` through the shared `from_payload` parser. +/// +/// Unlike `metric_store::backfill_agent_metric_index`, this function runs +/// entirely inside the caller's transaction — no nested `BEGIN`/`COMMIT`. +/// It is used exclusively by M1 where the outer transaction provides atomicity. +fn rebuild_metric_index_in_tx( + conn: &Connection, + identity_pubkey: &str, + relay_url: &str, +) -> Result<(), String> { + let mut stmt = conn + .prepare( + "SELECT id, pubkey, created_at, archived_at, raw_json + FROM archived_events + WHERE identity_pubkey = ?1 + AND relay_url = ?2 + AND kind = 44200", + ) + .map_err(|e| format!("migration M1: prepare rebuild select: {e}"))?; + + let rows: Vec<(String, String, i64, i64, String)> = stmt + .query_map(params![identity_pubkey, relay_url], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, i64>(2)?, + row.get::<_, i64>(3)?, + row.get::<_, String>(4)?, + )) + }) + .map_err(|e| format!("migration M1: query rebuild rows: {e}"))? + .collect::, _>>() + .map_err(|e| format!("migration M1: read rebuild row: {e}"))?; + drop(stmt); + + for (id, pubkey, created_at, archived_at, raw_json) in &rows { + let parsed = super::metric_store::AgentMetricIndexRow::from_payload( + raw_json, + id, + pubkey, + *created_at, + *archived_at, + ); + super::metric_store::insert_metric_index_row(conn, identity_pubkey, relay_url, &parsed) + .map_err(|e| format!("migration M1: insert rebuilt row: {e}"))?; + } + + Ok(()) +} + fn set_wal_mode(conn: &Connection) -> Result<(), String> { let deadline = Instant::now() + Duration::from_secs(5); loop { @@ -871,6 +1035,9 @@ pub fn gc_orphaned_events( // ── Tests ─────────────────────────────────────────────────────────────────── +#[cfg(test)] +#[path = "store_migration_tests.rs"] +mod store_migration_tests; #[cfg(test)] #[path = "store_tests.rs"] mod store_tests; diff --git a/desktop/src-tauri/src/archive/store_migration_tests.rs b/desktop/src-tauri/src/archive/store_migration_tests.rs new file mode 100644 index 0000000000..f8fedc0ec3 --- /dev/null +++ b/desktop/src-tauri/src/archive/store_migration_tests.rs @@ -0,0 +1,248 @@ +//! Migration tests for `archive/store.rs` — M1: harness column. +//! +//! Kept in a sibling file so `store_tests.rs` stays under the 1000-line gate; +//! `#[path]`-included from `store.rs`. + +use super::*; + +// ── Migration M1: harness column + open_archive_db reopen tests ────────────── + +/// Build a DB file that looks like a pre-harness archive: schema without the +/// `harness` column, a seeded `archived_events` row and a corresponding +/// `agent_metric_index` row with `harness = NULL`, and no `archive_migrations` +/// marker. Return the path to the temp file. +fn build_old_schema_db(path: &std::path::Path) { + // Old schema has no `harness TEXT` column and no `archive_migrations` table. + const OLD_SCHEMA: &str = " +CREATE TABLE IF NOT EXISTS archived_events ( + identity_pubkey TEXT NOT NULL, relay_url TEXT NOT NULL, id TEXT NOT NULL, + kind INTEGER NOT NULL, pubkey TEXT NOT NULL, created_at INTEGER NOT NULL, + raw_json TEXT NOT NULL, archived_at INTEGER NOT NULL, + PRIMARY KEY (identity_pubkey, relay_url, id) +); +CREATE TABLE IF NOT EXISTS archived_event_scopes ( + identity_pubkey TEXT NOT NULL, relay_url TEXT NOT NULL, id TEXT NOT NULL, + scope_type TEXT NOT NULL, scope_value TEXT NOT NULL, archived_at INTEGER NOT NULL, + PRIMARY KEY (identity_pubkey, relay_url, id, scope_type, scope_value) +); +CREATE TABLE IF NOT EXISTS save_subscriptions ( + identity_pubkey TEXT NOT NULL, relay_url TEXT NOT NULL, scope_type TEXT NOT NULL, + scope_value TEXT NOT NULL, kinds TEXT NOT NULL, created_at INTEGER NOT NULL, + PRIMARY KEY (identity_pubkey, relay_url, scope_type, scope_value) +); +CREATE TABLE IF NOT EXISTS observer_channel_index ( + identity_pubkey TEXT NOT NULL, relay_url TEXT NOT NULL, id TEXT NOT NULL, + channel_id TEXT, created_at INTEGER NOT NULL, + PRIMARY KEY (identity_pubkey, relay_url, id) +); +CREATE TABLE IF NOT EXISTS agent_metric_index ( + identity_pubkey TEXT NOT NULL, relay_url TEXT NOT NULL, id TEXT NOT NULL, + agent_pubkey TEXT NOT NULL, event_created_at INTEGER NOT NULL, + archived_at INTEGER NOT NULL, reported_at INTEGER, session_id TEXT, + turn_seq TEXT, model TEXT, delta_reliable INTEGER, + turn_input_tokens TEXT, turn_output_tokens TEXT, turn_total_tokens TEXT, + turn_cost_usd REAL, cumulative_input_tokens TEXT, cumulative_output_tokens TEXT, + cumulative_total_tokens TEXT, cumulative_cost_usd REAL, + parse_status TEXT NOT NULL CHECK (parse_status IN ('valid','invalid')), + PRIMARY KEY (identity_pubkey, relay_url, id) +); +"; + let conn = Connection::open(path).unwrap(); + conn.pragma_update(None, "journal_mode", "WAL").unwrap(); + conn.pragma_update(None, "busy_timeout", 5000).unwrap(); + conn.execute_batch(OLD_SCHEMA).unwrap(); + + // A valid payload that carries harness="goose". + let raw_json = r#"{"harness":"goose","model":"claude","channelId":null,"sessionId":"s1","turnId":null,"turnSeq":1,"timestamp":"2026-07-01T00:00:00Z","turn":{"inputTokens":10,"outputTokens":20,"totalTokens":30,"costUsd":0.01},"cumulative":{"inputTokens":100,"outputTokens":200,"totalTokens":300,"costUsd":0.1},"deltaReliable":true,"stopReason":"end_turn"}"#; + conn.execute( + "INSERT INTO archived_events + (identity_pubkey, relay_url, id, kind, pubkey, created_at, raw_json, archived_at) + VALUES ('id', 'relay', 'eid1', 44200, 'agent1', 100, ?1, 200)", + rusqlite::params![raw_json], + ) + .unwrap(); + // Seed the index row with harness absent (old schema has no column). + conn.execute( + "INSERT INTO agent_metric_index + (identity_pubkey, relay_url, id, agent_pubkey, event_created_at, + archived_at, reported_at, session_id, turn_seq, model, + delta_reliable, turn_input_tokens, turn_output_tokens, + turn_total_tokens, turn_cost_usd, cumulative_input_tokens, + cumulative_output_tokens, cumulative_total_tokens, + cumulative_cost_usd, parse_status) + VALUES ('id','relay','eid1','agent1',100,200,1751414400000, + 's1','00000000000000000001','claude', + 1,'00000000000000000010','00000000000000000020', + '00000000000000000030',0.01, + '00000000000000000100','00000000000000000200', + '00000000000000000300',0.1,'valid')", + [], + ) + .unwrap(); +} + +/// Opening a pre-harness DB via `open_archive_db` must: +/// +/// 1. Add the `harness` column. +/// 2. Rebuild all index rows so harness is populated from `archived_events`. +/// 3. Record the migration marker in `archive_migrations`. +/// +/// This proves M1 fires on a real legacy file, not just a hand-rolled +/// DELETE+backfill as in the metric_store migration test. +#[test] +fn migration_m1_reopen_old_schema_db_populates_harness_and_records_marker() { + use tempfile::NamedTempFile; + let db_file = NamedTempFile::new().unwrap(); + let db_path = db_file.path().to_path_buf(); + + build_old_schema_db(&db_path); + + // Reopen via the real migration path. + let conn = open_archive_db(&db_path).expect("open_archive_db must succeed on legacy DB"); + + // Harness must be populated. + let harness: Option = conn + .query_row( + "SELECT harness FROM agent_metric_index WHERE id = 'eid1'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + harness, + Some("goose".to_string()), + "M1: harness must be populated from raw_json after reopen" + ); + + // Marker must be recorded. + let marker_count: i64 = conn + .query_row( + "SELECT COUNT(*) FROM archive_migrations \ + WHERE name = 'add_harness_to_metric_index'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(marker_count, 1, "M1: migration marker must be recorded"); +} + +/// Re-opening the same DB a second time must be a no-op: marker already present, +/// harness already populated, no error. +#[test] +fn migration_m1_reopen_twice_is_idempotent() { + use tempfile::NamedTempFile; + let db_file = NamedTempFile::new().unwrap(); + let db_path = db_file.path().to_path_buf(); + + build_old_schema_db(&db_path); + + // First open: runs M1. + open_archive_db(&db_path).expect("first open must succeed"); + // Second open: M1 is already marked; must also succeed without error. + let conn2 = open_archive_db(&db_path).expect("second open must succeed (M1 is idempotent)"); + + let count: i64 = conn2 + .query_row( + "SELECT COUNT(*) FROM agent_metric_index WHERE harness = 'goose'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + count, 1, + "after idempotent second open, harness row must still be present" + ); +} + +/// If the migration is aborted after DELETE but before COMMIT (simulated by +/// manually applying each sub-step without committing), the DB must be in its +/// pre-migration state and the marker must be absent, so the next +/// `open_archive_db` can re-run M1 from scratch and complete successfully. +/// +/// We simulate the "crash before commit" scenario by: +/// 1. Building a pre-harness DB on disk. +/// 2. Opening a raw connection, running ALTER + DELETE but rolling back before +/// inserting the marker — leaving the DB in pre-migration state. +/// 3. Calling `open_archive_db` on the same file and asserting M1 completes. +#[test] +fn migration_m1_crashed_before_commit_reruns_fully_on_next_open() { + use tempfile::NamedTempFile; + let db_file = NamedTempFile::new().unwrap(); + let db_path = db_file.path().to_path_buf(); + + build_old_schema_db(&db_path); + + // Simulate a crash: start a transaction, ALTER + DELETE, then ROLLBACK + // (leaving DB exactly as-built — pre-migration, no marker). + { + let conn = Connection::open(&db_path).unwrap(); + conn.pragma_update(None, "journal_mode", "WAL").unwrap(); + conn.execute_batch("BEGIN IMMEDIATE").unwrap(); + // ALTER inside the transaction. + conn.execute_batch("ALTER TABLE agent_metric_index ADD COLUMN harness TEXT") + .unwrap(); + // DELETE index rows (simulating the mid-migration state). + conn.execute_batch("DELETE FROM agent_metric_index") + .unwrap(); + // Rollback — no marker inserted, no new rows. + conn.execute_batch("ROLLBACK").unwrap(); + } + + // After rollback: marker must be absent and the original index row still there. + { + let verify = Connection::open(&db_path).unwrap(); + // archive_migrations table may not exist at all (old schema). If it does, + // the marker must not be present. + let marker_count: i64 = verify + .query_row( + "SELECT COUNT(*) FROM sqlite_master \ + WHERE type='table' AND name='archive_migrations'", + [], + |r| r.get(0), + ) + .unwrap(); + if marker_count > 0 { + let applied: i64 = verify + .query_row( + "SELECT COUNT(*) FROM archive_migrations \ + WHERE name = 'add_harness_to_metric_index'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(applied, 0, "marker must be absent after rollback"); + } + } + + // The next open must run M1 fully and succeed. + let conn = + open_archive_db(&db_path).expect("open_archive_db must succeed after simulated crash"); + + // Harness must be populated. + let harness: Option = conn + .query_row( + "SELECT harness FROM agent_metric_index WHERE id = 'eid1'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + harness, + Some("goose".to_string()), + "M1 must re-run after simulated crash and populate harness" + ); + + // Marker must now be recorded. + let marker: i64 = conn + .query_row( + "SELECT COUNT(*) FROM archive_migrations \ + WHERE name = 'add_harness_to_metric_index'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!( + marker, 1, + "M1 marker must be recorded after successful re-run" + ); +} diff --git a/desktop/src/features/agent-usage/lib/agentUsage.test.mjs b/desktop/src/features/agent-usage/lib/agentUsage.test.mjs index 1ed5a152c4..b3d5b47396 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.test.mjs +++ b/desktop/src/features/agent-usage/lib/agentUsage.test.mjs @@ -49,6 +49,7 @@ function agentUsage(pubkey, totalTokensValue, overrides = {}) { function modelUsage(model, totalTokensValue, overrides = {}) { return { + harness: null, model, usage: reportedUsage({ totalTokens: usageField({ value: totalTokensValue }), @@ -368,6 +369,32 @@ test("sortModelsByKnownTotal sorts null model ('Unknown model') last among ties" ); }); +test("sortModelsByKnownTotal tiebreaks harness before model when totals are equal", () => { + const models = [ + modelUsage("m", "100", { harness: "z-harness" }), + modelUsage("m", "100", { harness: "a-harness" }), + modelUsage("m", "100", { harness: null }), + ]; + const sorted = sortModelsByKnownTotal(models); + assert.deepEqual( + sorted.map((m) => m.harness), + ["a-harness", "z-harness", null], + ); +}); + +test("sortModelsByKnownTotal same model two harnesses produces two rows in harness order", () => { + // Same model via two harnesses should be distinct rows; harness-ascending tiebreak. + const models = [ + modelUsage("claude-sonnet", "500", { harness: "goose" }), + modelUsage("claude-sonnet", "500", { harness: "claude-code" }), + ]; + const sorted = sortModelsByKnownTotal(models); + assert.deepEqual( + sorted.map((m) => m.harness), + ["claude-code", "goose"], + ); +}); + // ── isPartialField / isUnknownField ────────────────────────────────────────── test("isPartialField is true only for a known value flagged incomplete", () => { diff --git a/desktop/src/features/agent-usage/lib/agentUsage.ts b/desktop/src/features/agent-usage/lib/agentUsage.ts index 0c54c20b75..2ff21b95a1 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.ts +++ b/desktop/src/features/agent-usage/lib/agentUsage.ts @@ -221,7 +221,12 @@ export function sortAgentsByKnownTotal( ); } -/** Model rows use the same ranking rule as agents, tiebroken by model name (`null` model sorts last as "Unknown model"). */ +/** Model rows use the same ranking rule as agents, tiebroken by harness name + * (null harness sorts last), then by model name (null model sorts last). + * Ordinal (`<`/`>`) comparators are used so ordering is locale-independent + * and matches the Rust backend's `String::cmp` byte order. + * Note: harness/model identifiers are ASCII in practice; UTF-16 vs UTF-8 + * scalar divergence for astral code points is accepted and not a use case. */ export function sortModelsByKnownTotal( models: readonly AgentUsageModel[], ): AgentUsageModel[] { @@ -229,10 +234,23 @@ export function sortModelsByKnownTotal( models, (model) => model.usage.totalTokens, (a, b) => { + // Harness tiebreak first (ordinal, None last). + const harnessCmp = + a.harness === b.harness + ? 0 + : a.harness === null + ? 1 + : b.harness === null + ? -1 + : a.harness < b.harness + ? -1 + : 1; + if (harnessCmp !== 0) return harnessCmp; + // Then model (ordinal, None last). if (a.model === b.model) return 0; if (a.model === null) return 1; if (b.model === null) return -1; - return a.model.localeCompare(b.model); + return a.model < b.model ? -1 : 1; }, ); } diff --git a/desktop/src/features/agent-usage/ui/AgentUsageFocusedView.tsx b/desktop/src/features/agent-usage/ui/AgentUsageFocusedView.tsx index 8b81edef07..4cf68b6536 100644 --- a/desktop/src/features/agent-usage/ui/AgentUsageFocusedView.tsx +++ b/desktop/src/features/agent-usage/ui/AgentUsageFocusedView.tsx @@ -244,10 +244,18 @@ function AgentUsageFocusedTotals({ {models.map((model) => (
{model.model ?? "Unknown model"} + {model.harness !== null ? ( + + {model.harness} + + ) : null} {isUnknownField(model.usage.totalTokens) diff --git a/desktop/src/shared/api/tauriArchive.ts b/desktop/src/shared/api/tauriArchive.ts index 96c2e6d7be..2166e15a08 100644 --- a/desktop/src/shared/api/tauriArchive.ts +++ b/desktop/src/shared/api/tauriArchive.ts @@ -28,6 +28,7 @@ export type AgentUsageSeriesBucket = { }; export type AgentUsageModel = { + harness: string | null; model: string | null; usage: ReportedUsage; reportCount: number; diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 23cbffd2ca..26913106f5 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -147,6 +147,7 @@ type RawAgentUsageSeriesBucket = { }; type RawAgentUsageModel = { + harness: string | null; model: string | null; usage: RawReportedUsage; reportCount: number; diff --git a/desktop/tests/e2e/agent-usage-screenshots.spec.ts b/desktop/tests/e2e/agent-usage-screenshots.spec.ts index 5e345b6493..ec3c1654ca 100644 --- a/desktop/tests/e2e/agent-usage-screenshots.spec.ts +++ b/desktop/tests/e2e/agent-usage-screenshots.spec.ts @@ -298,6 +298,7 @@ test.describe("agent usage screenshots", () => { ], models: [ { + harness: "claude-code", hasUnknownUsage: false, model: "claude-opus-4-5", reportCount: 4, @@ -309,6 +310,7 @@ test.describe("agent usage screenshots", () => { }), }, { + harness: "goose", hasUnknownUsage: false, model: "claude-sonnet-4-5", reportCount: 1, @@ -351,6 +353,11 @@ test.describe("agent usage screenshots", () => { await expect(page.getByTestId("user-profile-panel")).toBeVisible(); await expect(page.getByTestId("agent-usage-focused-view")).toBeVisible(); + // Shot 02 must render a visible harness label on the model breakdown rows. + await expect( + page.getByTestId("agent-usage-model-harness-label").first(), + ).toBeVisible(); + const panel = page.getByTestId("user-profile-panel"); await waitForAnimations(page); await panel.screenshot({ path: `${SHOTS}/02-focused-usage-view.png` }); diff --git a/desktop/tests/e2e/agent-usage.spec.ts b/desktop/tests/e2e/agent-usage.spec.ts index 1e6490a49f..21f1aa5d8e 100644 --- a/desktop/tests/e2e/agent-usage.spec.ts +++ b/desktop/tests/e2e/agent-usage.spec.ts @@ -257,6 +257,7 @@ test("clicking an agent row opens the profile panel's Usage focused view", async mockAgentUsage(agentPubkey, { models: [ { + harness: "goose", hasUnknownUsage: false, model: "claude-opus", reportCount: 1, @@ -288,6 +289,9 @@ test("clicking an agent row opens the profile panel's Usage focused view", async await expect(page.getByTestId("agent-usage-focused-models")).toContainText( "claude-opus", ); + await expect(page.getByTestId("agent-usage-focused-models")).toContainText( + "goose", + ); // Reaching the same view via the Info-tab ingress row lands on the same // subview (covers the ProfileIngressRow entry point, not just the diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index c3c797fcc8..ebaf044165 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -150,6 +150,7 @@ export type MockAgentUsageSeriesBucket = { }; export type MockAgentUsageModel = { + harness: string | null; model: string | null; usage: MockReportedUsage; reportCount: number;