From 7d115938707ef99989753ffff265c7981d1c590e Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Fri, 24 Jul 2026 19:40:51 -0400 Subject: [PATCH 1/3] feat(desktop): add harness to NIP-AM agent-usage breakdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group the per-agent model breakdown by (harness, model) instead of model alone, so the same model running under two different harnesses (e.g. claude-sonnet via goose vs claude-code) produces two distinct rows rather than collapsing into one. Changes: - agent_metric_index: add nullable TEXT harness column; schema migration (M1) does ADD COLUMN + index rebuild via the shared backfill parser so all existing rows get harness populated from retained raw_json — no data loss, ingest/backfill remain in sync - AgentMetricIndexRow: parse harness from AgentTurnMetricPayload (REQUIRED field), write/read through all store paths - AgentScope.models: widen key from Option to (Option, Option) i.e. (harness, model); sort tiebreak is harness-ascending then model-ascending, None last in each - ModelUsage wire type: add harness: Option field - AgentUsageFocusedView: render harness as a dimmed sub-label on each model row; null harness (pre-migration or unknown) renders no label so single-harness data is visually unchanged except for the label - tauriArchive.ts / bridge.ts: add harness to AgentUsageModel type - agentUsage.ts: update sortModelsByKnownTotal tiebreak for (harness, model) compound key - Tests: collapse-fix integration test (same model / two harnesses → two rows), migration test (old-shape rows get harness after rebuild), harness sort tests, E2E fixture + assertion updates Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src-tauri/src/archive/agent_usage.rs | 82 +++++++++++++------ .../src/archive/agent_usage_tests.rs | 68 +++++++++++++++ desktop/src-tauri/src/archive/metric_store.rs | 11 ++- .../src/archive/metric_store_tests.rs | 74 +++++++++++++++++ desktop/src-tauri/src/archive/store.rs | 81 ++++++++++++++++++ .../agent-usage/lib/agentUsage.test.mjs | 27 ++++++ .../features/agent-usage/lib/agentUsage.ts | 15 +++- .../agent-usage/ui/AgentUsageFocusedView.tsx | 10 ++- desktop/src/shared/api/tauriArchive.ts | 1 + .../tests/e2e/agent-usage-screenshots.spec.ts | 7 ++ desktop/tests/e2e/agent-usage.spec.ts | 4 + desktop/tests/helpers/bridge.ts | 1 + 12 files changed, 353 insertions(+), 28 deletions(-) diff --git a/desktop/src-tauri/src/archive/agent_usage.rs b/desktop/src-tauri/src/archive/agent_usage.rs index 431c068a1b..7335cdee94 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, @@ -464,7 +465,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 +553,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 +604,65 @@ pub(super) fn compute_series( }) .collect(); - let mut model_rows: Vec<(Option, Option, ModelUsage)> = scope - .models - .into_iter() - .map(|(model, (acc, count))| { - let model_total = acc.total_tokens_value(); - let has_unknown_usage = acc.has_unknown(); - ( - model_total, - model.clone(), - ModelUsage { - model, - usage: acc.finish(), - report_count: count, - has_unknown_usage, - }, - ) - }) - .collect(); + let mut model_rows: Vec<(Option, Option, Option, ModelUsage)> = + scope + .models + .into_iter() + .map(|((harness, model), (acc, count))| { + let model_total = acc.total_tokens_value(); + let has_unknown_usage = acc.has_unknown(); + ( + model_total, + harness.clone(), + model.clone(), + ModelUsage { + harness, + model, + usage: acc.finish(), + report_count: count, + has_unknown_usage, + }, + ) + }) + .collect(); + // 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| match (a.0, b.0) { - (Some(av), Some(bv)) => bv.cmp(&av).then_with(|| a.1.cmp(&b.1)), + (Some(av), Some(bv)) => bv.cmp(&av).then_with(|| { + // harness tiebreak + match (&a.1, &b.1) { + (Some(ah), Some(bh)) => ah.cmp(bh), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + } + .then_with(|| { + // model tiebreak + match (&a.2, &b.2) { + (Some(am), Some(bm)) => am.cmp(bm), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + } + }) + }), (Some(_), None) => std::cmp::Ordering::Less, (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => a.1.cmp(&b.1), + (None, None) => match (&a.1, &b.1) { + (Some(ah), Some(bh)) => ah.cmp(bh), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + } + .then_with(|| match (&a.2, &b.2) { + (Some(am), Some(bm)) => am.cmp(bm), + (Some(_), None) => std::cmp::Ordering::Less, + (None, Some(_)) => std::cmp::Ordering::Greater, + (None, None) => std::cmp::Ordering::Equal, + }), }); - let models = model_rows.into_iter().map(|(_, _, m)| m).collect(); + let models = model_rows.into_iter().map(|(_, _, _, m)| m).collect(); ( total_tokens_value, 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..9632611e96 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,89 @@ 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. `ALTER TABLE ... ADD COLUMN` is also itself +/// idempotent on SQLite ≥ 3.37 (the column already exists → error, so we +/// guard with the migrations table instead of relying on that). +fn apply_schema_migrations(conn: &Connection) -> Result<(), String> { + // Migration M1: add `harness` column to `agent_metric_index`. + // + // Pre-existing rows land with `harness = NULL` (TEXT nullable). The + // backfill path in `metric_store::backfill_agent_metric_index` re-parses + // `archived_events.raw_json` — but that anti-join only inserts NEW rows; + // rows already in the index are never re-parsed. To populate `harness` for + // existing valid rows we use the index-rebuild migration below, which + // DELETE-then-reinserts every row so the shared `from_payload` parser + // populates the new column uniformly. `DELETE + reinsert` inside one + // transaction is safe: `archived_events` is the canonical store; the index + // is always fully rebuildable from it. + 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 guard check failed: {e}"))? + > 0; + + if !already_run { + // 1. Add the nullable column (safe no-op on brand-new DBs where SCHEMA + // already included the column — SQLite returns "duplicate column name" + // which we ignore; new DBs have no rows to migrate anyway). + let _ = conn.execute_batch("ALTER TABLE agent_metric_index ADD COLUMN harness TEXT"); + + // 2. Rebuild the entire index so existing valid rows get harness + // populated from the retained raw_json. We delete all existing index + // rows and re-run the backfill anti-join; `backfill_agent_metric_index` + // is the canonical rebuild path shared by ingest and backfill. + // We collect all (identity, relay) pairs present in the index first. + let pairs: Vec<(String, String)> = { + let mut stmt = conn + .prepare("SELECT DISTINCT identity_pubkey, relay_url FROM agent_metric_index") + .map_err(|e| format!("migration M1: prepare pairs query: {e}"))?; + let pairs = stmt + .query_map([], |row| Ok((row.get(0)?, row.get(1)?))) + .map_err(|e| format!("migration M1: query pairs: {e}"))? + .collect::, _>>() + .map_err(|e| format!("migration M1: read pairs: {e}"))?; + pairs + }; + + if !pairs.is_empty() { + // Delete all existing index rows; backfill will re-insert them with + // harness populated. + conn.execute_batch("DELETE FROM agent_metric_index") + .map_err(|e| format!("migration M1: delete index rows: {e}"))?; + + for (identity, relay) in &pairs { + super::metric_store::backfill_agent_metric_index(conn, identity, relay)?; + } + } + + // 3. Record migration as applied. + conn.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 applied: {e}"))?; + } + + Ok(()) +} + fn set_wal_mode(conn: &Connection) -> Result<(), String> { let deadline = Instant::now() + Duration::from_secs(5); loop { 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..d10afd296b 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.ts +++ b/desktop/src/features/agent-usage/lib/agentUsage.ts @@ -221,7 +221,9 @@ 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 as "Unknown harness"), then by model name + * (null model sorts last as "Unknown model"). */ export function sortModelsByKnownTotal( models: readonly AgentUsageModel[], ): AgentUsageModel[] { @@ -229,6 +231,17 @@ export function sortModelsByKnownTotal( models, (model) => model.usage.totalTokens, (a, b) => { + // Harness tiebreak first. + const harnessCmp = + a.harness === b.harness + ? 0 + : a.harness === null + ? 1 + : b.harness === null + ? -1 + : a.harness.localeCompare(b.harness); + if (harnessCmp !== 0) return harnessCmp; + // Then model. if (a.model === b.model) return 0; if (a.model === null) return 1; if (b.model === null) return -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/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; From 91155290027ccce9add591c28cef6b5b668bcac5 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Fri, 24 Jul 2026 21:52:12 -0400 Subject: [PATCH 2/3] fix(desktop): address Thufir pass-1 findings for harness migration - M1 atomicity: wrap ALTER TABLE, full DELETE, per-scope rebuild, and archive_migrations marker in one unchecked_transaction so a crash before commit leaves no orphaned marker and the next open re-runs the whole migration from scratch. - M1 worklist: derive (identity, relay) pairs from archived_events (kind 44200) instead of surviving index rows so an empty index cannot cause M1 to silently declare success with scopes missing. - ALTER error handling: replace blanket let _ = ... with a PRAGMA table_info check; ALTER runs only when the harness column is absent and every other error propagates. - New Rust tests via open_archive_db: legacy-file reopen populates harness + records marker, second open is idempotent, rolled-back mid-migration state re-runs to completion on next open. - TS sort: replace localeCompare with ordinal < / > comparators so frontend ordering is locale-independent and matches Rust String::cmp. - e2eBridge.ts: add harness: string | null to RawAgentUsageModel so the tauriArchive.ts mirror contract is accurate. - Rust comparator: extract cmp_known_total + cmp_option_str_none_last helpers and ModelSortKey struct to eliminate the duplicated None-last ladder; also fixes the clippy type_complexity lint on the 4-tuple. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 9 + desktop/src-tauri/src/archive/agent_usage.rs | 121 +++++---- desktop/src-tauri/src/archive/store.rs | 193 ++++++++++---- desktop/src-tauri/src/archive/store_tests.rs | 242 ++++++++++++++++++ .../features/agent-usage/lib/agentUsage.ts | 15 +- desktop/src/testing/e2eBridge.ts | 1 + 6 files changed, 459 insertions(+), 122 deletions(-) diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 76499ad138..aa291c466f 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -82,6 +82,15 @@ 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 delegating to migrate_add_harness_to_metric_index. + // Load-bearing correctness fix (Thufir IMPORTANT). Queued to split. + ["src-tauri/src/archive/store.rs", 1043], + // agent-usage-harness: three M1 reopen tests (legacy-file open, idempotent + // second open, crash-before-commit re-run) via open_archive_db + build_old_schema_db + // helper. Test-only content; ratcheted from ~856 -> 1092. + ["src-tauri/src/archive/store_tests.rs", 1094], // 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 7335cdee94..5c9917cc77 100644 --- a/desktop/src-tauri/src/archive/agent_usage.rs +++ b/desktop/src-tauri/src/archive/agent_usage.rs @@ -457,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. @@ -604,65 +630,43 @@ pub(super) fn compute_series( }) .collect(); - let mut model_rows: Vec<(Option, Option, Option, ModelUsage)> = - scope - .models - .into_iter() - .map(|((harness, model), (acc, count))| { - let model_total = acc.total_tokens_value(); - let has_unknown_usage = acc.has_unknown(); - ( - model_total, - harness.clone(), - model.clone(), - ModelUsage { - harness, - model, - usage: acc.finish(), - report_count: count, - has_unknown_usage, - }, - ) - }) - .collect(); + // 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(|((harness, model), (acc, count))| { + let total = acc.total_tokens_value(); + let has_unknown_usage = acc.has_unknown(); + ModelSortKey { + total, + harness: harness.clone(), + model: model.clone(), + usage: ModelUsage { + harness, + model, + usage: acc.finish(), + report_count: count, + has_unknown_usage, + }, + } + }) + .collect(); // 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| match (a.0, b.0) { - (Some(av), Some(bv)) => bv.cmp(&av).then_with(|| { - // harness tiebreak - match (&a.1, &b.1) { - (Some(ah), Some(bh)) => ah.cmp(bh), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - } - .then_with(|| { - // model tiebreak - match (&a.2, &b.2) { - (Some(am), Some(bm)) => am.cmp(bm), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - } - }) - }), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => match (&a.1, &b.1) { - (Some(ah), Some(bh)) => ah.cmp(bh), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - } - .then_with(|| match (&a.2, &b.2) { - (Some(am), Some(bm)) => am.cmp(bm), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => std::cmp::Ordering::Equal, - }), + 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, @@ -678,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/store.rs b/desktop/src-tauri/src/archive/store.rs index 9632611e96..1e8b91048a 100644 --- a/desktop/src-tauri/src/archive/store.rs +++ b/desktop/src-tauri/src/archive/store.rs @@ -163,75 +163,158 @@ pub fn open_archive_db(path: &Path) -> Result { /// /// 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. `ALTER TABLE ... ADD COLUMN` is also itself -/// idempotent on SQLite ≥ 3.37 (the column already exists → error, so we -/// guard with the migrations table instead of relying on that). +/// that already ran is a no-op. fn apply_schema_migrations(conn: &Connection) -> Result<(), String> { - // Migration M1: add `harness` column to `agent_metric_index`. - // - // Pre-existing rows land with `harness = NULL` (TEXT nullable). The - // backfill path in `metric_store::backfill_agent_metric_index` re-parses - // `archived_events.raw_json` — but that anti-join only inserts NEW rows; - // rows already in the index are never re-parsed. To populate `harness` for - // existing valid rows we use the index-rebuild migration below, which - // DELETE-then-reinserts every row so the shared `from_payload` parser - // populates the new column uniformly. `DELETE + reinsert` inside one - // transaction is safe: `archived_events` is the canonical store; the index - // is always fully rebuildable from it. + 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 guard check failed: {e}"))? + .map_err(|e| format!("migration M1: guard check: {e}"))? > 0; - if !already_run { - // 1. Add the nullable column (safe no-op on brand-new DBs where SCHEMA - // already included the column — SQLite returns "duplicate column name" - // which we ignore; new DBs have no rows to migrate anyway). - let _ = conn.execute_batch("ALTER TABLE agent_metric_index ADD COLUMN harness TEXT"); - - // 2. Rebuild the entire index so existing valid rows get harness - // populated from the retained raw_json. We delete all existing index - // rows and re-run the backfill anti-join; `backfill_agent_metric_index` - // is the canonical rebuild path shared by ingest and backfill. - // We collect all (identity, relay) pairs present in the index first. - let pairs: Vec<(String, String)> = { - let mut stmt = conn - .prepare("SELECT DISTINCT identity_pubkey, relay_url FROM agent_metric_index") - .map_err(|e| format!("migration M1: prepare pairs query: {e}"))?; - let pairs = stmt - .query_map([], |row| Ok((row.get(0)?, row.get(1)?))) - .map_err(|e| format!("migration M1: query pairs: {e}"))? - .collect::, _>>() - .map_err(|e| format!("migration M1: read pairs: {e}"))?; - pairs - }; + if already_run { + return Ok(()); + } - if !pairs.is_empty() { - // Delete all existing index rows; backfill will re-insert them with - // harness populated. - conn.execute_batch("DELETE FROM agent_metric_index") - .map_err(|e| format!("migration M1: delete index rows: {e}"))?; + // 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}"))?; + } - for (identity, relay) in &pairs { - super::metric_store::backfill_agent_metric_index(conn, identity, relay)?; - } + // 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)?; } + } - // 3. Record migration as applied. - conn.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 - ], + // 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: record applied: {e}"))?; + .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(()) diff --git a/desktop/src-tauri/src/archive/store_tests.rs b/desktop/src-tauri/src/archive/store_tests.rs index bbd15391e7..24dcf7cc68 100644 --- a/desktop/src-tauri/src/archive/store_tests.rs +++ b/desktop/src-tauri/src/archive/store_tests.rs @@ -654,6 +654,248 @@ fn test_read_archived_events_no_duplicates_across_pages() { assert_eq!(all.len(), 3); // 2 + 1 = 3 total, no duplication. } +// ── 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" + ); +} + /// Regression for the scalar-cursor same-second skip defect (Thufir IMPORTANT). /// /// The writer stores `created_at` in whole seconds, so two events can share diff --git a/desktop/src/features/agent-usage/lib/agentUsage.ts b/desktop/src/features/agent-usage/lib/agentUsage.ts index d10afd296b..8af2dd9c97 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.ts +++ b/desktop/src/features/agent-usage/lib/agentUsage.ts @@ -222,8 +222,9 @@ export function sortAgentsByKnownTotal( } /** Model rows use the same ranking rule as agents, tiebroken by harness name - * (null harness sorts last as "Unknown harness"), then by model name - * (null model sorts last as "Unknown model"). */ + * (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. */ export function sortModelsByKnownTotal( models: readonly AgentUsageModel[], ): AgentUsageModel[] { @@ -231,7 +232,7 @@ export function sortModelsByKnownTotal( models, (model) => model.usage.totalTokens, (a, b) => { - // Harness tiebreak first. + // Harness tiebreak first (ordinal, None last). const harnessCmp = a.harness === b.harness ? 0 @@ -239,13 +240,15 @@ export function sortModelsByKnownTotal( ? 1 : b.harness === null ? -1 - : a.harness.localeCompare(b.harness); + : a.harness < b.harness + ? -1 + : 1; if (harnessCmp !== 0) return harnessCmp; - // Then model. + // 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/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; From e45869da16893d003c09d6e81cf3e6f94dda1a6e Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Fri, 24 Jul 2026 22:12:48 -0400 Subject: [PATCH 3/3] fix(desktop): split M1 migration tests + document TS ordering domain Split the M1 harness-migration tests from store_tests.rs into a new store_migration_tests.rs so store_tests.rs falls back under the 1000- line limit without a ratchet override; store_tests.rs is now ~858 lines (checker: 858). The store_tests.rs override is removed; store.rs gains one line for the new #[path] include (checker: 1044). Add a one-line comment at sortModelsByKnownTotal noting the identifier ordering domain is ASCII in practice so UTF-16 vs UTF-8 scalar divergence for astral code points is accepted. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/scripts/check-file-sizes.mjs | 10 +- desktop/src-tauri/src/archive/store.rs | 3 + .../src/archive/store_migration_tests.rs | 248 ++++++++++++++++++ desktop/src-tauri/src/archive/store_tests.rs | 242 ----------------- .../features/agent-usage/lib/agentUsage.ts | 4 +- 5 files changed, 257 insertions(+), 250 deletions(-) create mode 100644 desktop/src-tauri/src/archive/store_migration_tests.rs diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index aa291c466f..8fd4caefb8 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -84,13 +84,9 @@ const overrides = new Map([ ["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 delegating to migrate_add_harness_to_metric_index. - // Load-bearing correctness fix (Thufir IMPORTANT). Queued to split. - ["src-tauri/src/archive/store.rs", 1043], - // agent-usage-harness: three M1 reopen tests (legacy-file open, idempotent - // second open, crash-before-commit re-run) via open_archive_db + build_old_schema_db - // helper. Test-only content; ratcheted from ~856 -> 1092. - ["src-tauri/src/archive/store_tests.rs", 1094], + // + 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/store.rs b/desktop/src-tauri/src/archive/store.rs index 1e8b91048a..7e9054cc6b 100644 --- a/desktop/src-tauri/src/archive/store.rs +++ b/desktop/src-tauri/src/archive/store.rs @@ -1035,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-tauri/src/archive/store_tests.rs b/desktop/src-tauri/src/archive/store_tests.rs index 24dcf7cc68..bbd15391e7 100644 --- a/desktop/src-tauri/src/archive/store_tests.rs +++ b/desktop/src-tauri/src/archive/store_tests.rs @@ -654,248 +654,6 @@ fn test_read_archived_events_no_duplicates_across_pages() { assert_eq!(all.len(), 3); // 2 + 1 = 3 total, no duplication. } -// ── 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" - ); -} - /// Regression for the scalar-cursor same-second skip defect (Thufir IMPORTANT). /// /// The writer stores `created_at` in whole seconds, so two events can share diff --git a/desktop/src/features/agent-usage/lib/agentUsage.ts b/desktop/src/features/agent-usage/lib/agentUsage.ts index 8af2dd9c97..2ff21b95a1 100644 --- a/desktop/src/features/agent-usage/lib/agentUsage.ts +++ b/desktop/src/features/agent-usage/lib/agentUsage.ts @@ -224,7 +224,9 @@ export function sortAgentsByKnownTotal( /** 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. */ + * 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[] {