Skip to content

Commit bc508a5

Browse files
committed
fix: harden session analytics — schema, silent failures, missing tests
Schema: - migrations/003: duration_secs NOT NULL DEFAULT 0 (was nullable, causing NULL rows for pre-migration sessions that silently distorted AVG) Silent failure fixes: - db.rs update_session_analytics: check rows_modified; eprintln! warn when session_id not found instead of silent no-op UPDATE - main.rs cmd_stats: match on gain_stats() Err instead of if let Ok silently discarding SQLite errors - main.rs cmd_gain: fix UTF-8 panic on multi-byte project names by using .chars().take(19) instead of byte-slice indexing &project[..19] Tests (3 new, 64 total): - update_session_analytics_persists_and_gain_stats_reads_back: write-read round-trip verifying analytics survive to the DB and aggregate correctly - gain_stats_aggregates_multiple_sessions: multi-project aggregation with top_projects ordering - update_session_analytics_missing_session_id_is_no_op: missing session_id returns Ok (no panic, no Err propagation)
1 parent 9715979 commit bc508a5

2 files changed

Lines changed: 107 additions & 22 deletions

File tree

migrations/003_session_analytics.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
-- Note: turn_count already exists from 001_init.sql — not added here.
44

55
BEGIN;
6-
ALTER TABLE sessions ADD COLUMN duration_secs INTEGER;
6+
ALTER TABLE sessions ADD COLUMN duration_secs INTEGER NOT NULL DEFAULT 0;
77
ALTER TABLE sessions ADD COLUMN input_tokens INTEGER NOT NULL DEFAULT 0;
88
ALTER TABLE sessions ADD COLUMN output_tokens INTEGER NOT NULL DEFAULT 0;
99
ALTER TABLE sessions ADD COLUMN cache_read_tokens INTEGER NOT NULL DEFAULT 0;

src/db.rs

Lines changed: 106 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ impl Db {
336336
id: &str,
337337
analytics: &TranscriptAnalytics,
338338
) -> Result<()> {
339-
self.conn.execute(
339+
let rows = self.conn.execute(
340340
"UPDATE sessions SET
341341
turn_count = ?1,
342342
duration_secs = ?2,
@@ -355,6 +355,9 @@ impl Db {
355355
id
356356
],
357357
)?;
358+
if rows == 0 {
359+
eprintln!("[mem] warn: session_id {id} not found — analytics not stored");
360+
}
358361
Ok(())
359362
}
360363

@@ -421,7 +424,7 @@ impl Db {
421424

422425
/// Aggregate token/time analytics across all sessions.
423426
pub fn gain_stats(&self) -> Result<GainStats> {
424-
let row = self.conn.query_row(
427+
let aggregate = self.conn.query_row(
425428
"SELECT
426429
COUNT(*) as session_count,
427430
COALESCE(SUM(duration_secs), 0) as total_secs,
@@ -434,16 +437,17 @@ impl Db {
434437
FROM sessions",
435438
[],
436439
|r| {
437-
Ok((
438-
r.get::<_, i64>(0)?,
439-
r.get::<_, i64>(1)?,
440-
r.get::<_, i64>(2)?,
441-
r.get::<_, i64>(3)?,
442-
r.get::<_, i64>(4)?,
443-
r.get::<_, i64>(5)?,
444-
r.get::<_, f64>(6)?,
445-
r.get::<_, f64>(7)?,
446-
))
440+
Ok(GainStats {
441+
session_count: r.get(0)?,
442+
total_secs: r.get(1)?,
443+
total_input: r.get(2)?,
444+
total_output: r.get(3)?,
445+
total_cache_read: r.get(4)?,
446+
total_cache_creation: r.get(5)?,
447+
avg_turns: r.get(6)?,
448+
avg_secs: r.get(7)?,
449+
top_projects: Vec::new(),
450+
})
447451
},
448452
)?;
449453

@@ -464,19 +468,12 @@ impl Db {
464468
total_tokens: r.get::<_, Option<i64>>(2)?.unwrap_or(0),
465469
})
466470
})?
467-
.map(|r| r.map_err(anyhow::Error::from))
471+
.map(|r| r.map_err(Into::into))
468472
.collect::<Result<Vec<_>>>()?;
469473

470474
Ok(GainStats {
471-
session_count: row.0,
472-
total_secs: row.1,
473-
total_input: row.2,
474-
total_output: row.3,
475-
total_cache_read: row.4,
476-
total_cache_creation: row.5,
477-
avg_turns: row.6,
478-
avg_secs: row.7,
479475
top_projects,
476+
..aggregate
480477
})
481478
}
482479

@@ -1387,4 +1384,92 @@ mod tests {
13871384
let result = db.demote_memory("nonexistent-id-xyz").unwrap();
13881385
assert!(!result, "demote should return false for nonexistent ID");
13891386
}
1387+
1388+
// ── Session analytics write-read round-trip ───────────────────────────────
1389+
1390+
#[test]
1391+
fn update_session_analytics_persists_and_gain_stats_reads_back() {
1392+
use crate::types::TranscriptAnalytics;
1393+
1394+
let db = in_memory_db();
1395+
db.start_session("sess-analytics", Some("/proj-a"), None)
1396+
.unwrap();
1397+
1398+
let analytics = TranscriptAnalytics {
1399+
turn_count: 7,
1400+
duration_secs: 300,
1401+
input_tokens: 1000,
1402+
output_tokens: 500,
1403+
cache_read_tokens: 200,
1404+
cache_creation_tokens: 50,
1405+
};
1406+
db.update_session_analytics("sess-analytics", &analytics)
1407+
.unwrap();
1408+
1409+
let g = db.gain_stats().unwrap();
1410+
assert_eq!(g.session_count, 1);
1411+
assert_eq!(g.total_input, 1000);
1412+
assert_eq!(g.total_output, 500);
1413+
assert_eq!(g.total_cache_read, 200);
1414+
assert_eq!(g.total_cache_creation, 50);
1415+
assert_eq!(g.total_secs, 300);
1416+
assert_eq!(g.avg_turns, 7.0);
1417+
}
1418+
1419+
#[test]
1420+
fn gain_stats_aggregates_multiple_sessions() {
1421+
use crate::types::TranscriptAnalytics;
1422+
1423+
let db = in_memory_db();
1424+
for (id, proj) in [("s1", "/a"), ("s2", "/b"), ("s3", "/a")] {
1425+
db.start_session(id, Some(proj), None).unwrap();
1426+
db.update_session_analytics(
1427+
id,
1428+
&TranscriptAnalytics {
1429+
turn_count: 4,
1430+
duration_secs: 120,
1431+
input_tokens: 100,
1432+
output_tokens: 50,
1433+
cache_read_tokens: 30,
1434+
cache_creation_tokens: 10,
1435+
},
1436+
)
1437+
.unwrap();
1438+
}
1439+
1440+
let g = db.gain_stats().unwrap();
1441+
assert_eq!(g.session_count, 3);
1442+
assert_eq!(g.total_input, 300);
1443+
assert_eq!(g.total_output, 150);
1444+
assert_eq!(g.total_cache_read, 90);
1445+
assert_eq!(g.total_secs, 360);
1446+
assert_eq!(g.avg_turns, 4.0);
1447+
// top_projects: /a has 2 sessions, /b has 1
1448+
assert!(!g.top_projects.is_empty());
1449+
assert_eq!(g.top_projects[0].project, "/a");
1450+
assert_eq!(g.top_projects[0].sessions, 2);
1451+
}
1452+
1453+
#[test]
1454+
fn update_session_analytics_missing_session_id_is_no_op() {
1455+
use crate::types::TranscriptAnalytics;
1456+
1457+
let db = in_memory_db();
1458+
// No session created — should complete without error (warning goes to stderr)
1459+
let result = db.update_session_analytics(
1460+
"nonexistent-session",
1461+
&TranscriptAnalytics {
1462+
turn_count: 3,
1463+
duration_secs: 60,
1464+
input_tokens: 100,
1465+
output_tokens: 50,
1466+
cache_read_tokens: 20,
1467+
cache_creation_tokens: 5,
1468+
},
1469+
);
1470+
assert!(result.is_ok(), "missing session_id must not return Err");
1471+
// No sessions exist, so gain_stats shows 0
1472+
let g = db.gain_stats().unwrap();
1473+
assert_eq!(g.session_count, 0);
1474+
}
13901475
}

0 commit comments

Comments
 (0)