skills/rtk-ai__rtk
1
0
Fork 0
mirror of https://github.com/rtk-ai/rtk.git synced 2026-09-19 07:33:17 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(tracking): record negative savings instead of clamping to zero

Browse Source 
When rtk emitted MORE than the wrapped command, record() saturated saved to 0
and reported a fake '0% — did nothing' instead of reflecting the regression.
Compute saved as signed (input - output) and store the honest negative in both
saved_tokens and savings_pct. Aggregate readers clamp a negative to 0 for their
unsigned token-count API so a regression can never wrap to a huge usize, while
per-command savings_pct keeps the honest negative. Covered by a new test.
This commit is contained in:
breisnerlopez
2026-09-08 16:02:15 +02:00
parent 533b96415e
commit 9da32707ee
1 changed files with 63 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/tracking.rs
+63 -9
View File
@@ -515,7 +515,12 @@ impl Tracker {
output_tokens: usize,
exec_time_ms: u64,
) -> Result<()> {
let saved = input_tokens.saturating_sub(output_tokens);
// Signed, so a command that emitted MORE than the wrapped command records the
// truth (a negative saving / negative pct) instead of `saturating_sub` clamping
// to 0 and reporting a fake "0% — did nothing". SQLite INTEGER/REAL both hold
// negatives. Aggregate readers clamp these to 0 for their unsigned token-count
// API, but per-command `savings_pct` keeps the honest negative.
let saved = input_tokens as i64 - output_tokens as i64;
let pct = if input_tokens > 0 {
(saved as f64 / input_tokens as f64) * 100.0
} else {
@@ -534,7 +539,7 @@ impl Tracker {
project_path, // added
input_tokens as i64,
output_tokens as i64,
saved as i64,
saved,
pct,
exec_time_ms as i64
],
@@ -838,7 +843,9 @@ impl Tracker {
Ok((
row.get::<_, i64>(0)? as usize,
row.get::<_, i64>(1)? as usize,
row.get::<_, i64>(2)? as usize,
// saved_tokens may be negative (a command that worsened output); clamp
// to 0 for the unsigned aggregate so it never wraps to a huge usize.
row.get::<_, i64>(2)?.max(0) as usize,
row.get::<_, i64>(3)? as u64,
))
})?;
@@ -899,7 +906,8 @@ impl Tracker {
Ok((
row.get::<_, String>(0)?,
row.get::<_, i64>(1)? as usize,
row.get::<_, i64>(2)? as usize,
// SUM(saved_tokens): clamp a net-negative group to 0 (unsigned field).
row.get::<_, i64>(2)?.max(0) as usize,
row.get::<_, f64>(3)?,
row.get::<_, f64>(4)? as u64,
))
@@ -924,7 +932,11 @@ impl Tracker {
let rows = stmt.query_map(params![project_exact, project_glob], |row| {
// added: params
Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)? as usize))
// SUM(saved_tokens) per day: clamp a net-negative day to 0 (unsigned field).
Ok((
row.get::<_, String>(0)?,
row.get::<_, i64>(1)?.max(0) as usize,
))
})?;
let mut result: Vec<_> = rows.collect::<Result<Vec<_>, _>>()?;
@@ -974,7 +986,7 @@ impl Tracker {
let rows = stmt.query_map(params![project_exact, project_glob], |row| {
// added: params
let input = row.get::<_, i64>(2)? as usize;
let saved = row.get::<_, i64>(4)? as usize;
let saved = row.get::<_, i64>(4)?.max(0) as usize; // clamp net-negative group
let commands = row.get::<_, i64>(1)? as usize;
let total_time = row.get::<_, i64>(5)? as u64;
let savings_pct = if input > 0 {
@@ -1048,7 +1060,7 @@ impl Tracker {
let rows = stmt.query_map(params![project_exact, project_glob], |row| {
// added: params
let input = row.get::<_, i64>(3)? as usize;
let saved = row.get::<_, i64>(5)? as usize;
let saved = row.get::<_, i64>(5)?.max(0) as usize; // clamp net-negative group
let commands = row.get::<_, i64>(2)? as usize;
let total_time = row.get::<_, i64>(6)? as u64;
let savings_pct = if input > 0 {
@@ -1122,7 +1134,7 @@ impl Tracker {
let rows = stmt.query_map(params![project_exact, project_glob], |row| {
// added: params
let input = row.get::<_, i64>(2)? as usize;
let saved = row.get::<_, i64>(4)? as usize;
let saved = row.get::<_, i64>(4)?.max(0) as usize; // clamp net-negative group
let commands = row.get::<_, i64>(1)? as usize;
let total_time = row.get::<_, i64>(5)? as u64;
let savings_pct = if input > 0 {
@@ -1202,7 +1214,7 @@ impl Tracker {
.map(|dt| dt.with_timezone(&Utc))
.unwrap_or_else(|_| Utc::now()),
rtk_cmd: row.get(1)?,
saved_tokens: row.get::<_, i64>(2)? as usize,
saved_tokens: row.get::<_, i64>(2)?.max(0) as usize, // clamp negative
savings_pct: row.get(3)?,
})
},
@@ -1921,6 +1933,48 @@ mod tests {
// because the savings calculation is correct for both cases
}
// record() must reflect a REGRESSION (output larger than the wrapped command's)
// as a negative saving, not saturate it to 0 and report a fake "0% — did nothing".
#[test]
fn test_record_reflects_worsening_not_zero() {
let tracker = Tracker::new_in_memory().expect("Failed to create tracker");
// Emitted 150 tokens where plain git emitted 100: a real regression.
tracker
.record("cmd", "rtk cmd worse", 100, 150, 5)
.expect("Failed to record worsening command");
// The DB stores the honest negative saving (isolated in-memory DB, one row).
assert_eq!(
tracker.total_tokens_saved().expect("sum saved"),
-50,
"worsening must be stored as a negative saving, not saturated to 0"
);
// The per-command savings_pct is negative, not the old fake 0%.
let recent = tracker.get_recent(10).expect("Failed to get recent");
let rec = recent
.iter()
.find(|r| r.rtk_cmd == "rtk cmd worse")
.expect("worsening record not found");
assert!(
(rec.savings_pct - (-50.0)).abs() < 1e-9,
"expected -50% savings, got {}",
rec.savings_pct
);
// Aggregate telemetry also reflects the regression as negative, and the unsigned
// summary counter clamps rather than wrapping to a huge usize.
assert!(
tracker.overall_savings_pct().expect("overall pct") < 0.0,
"overall savings must be negative for a net regression"
);
let summary = tracker.get_summary().expect("summary");
assert_eq!(
summary.total_saved, 0,
"unsigned aggregate clamps a negative saving to 0 (never wraps)"
);
}
// 5. TimedExecution::track records with exec_time > 0
//
// TimedExecution::track() hardcodes Tracker::new() internally (real