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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 84 additions & 83 deletions ISSUES.md

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions src-tauri/src/commands/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ pub fn sessions_insert(
generated_at: Option<i64>,
confident_samples: Option<i64>,
skipped_samples: Option<i64>,
// I83 — 1 = AI focus detection was on for this session, 0 = off, None =
// caller didn't say (older frontend). None coalesces, so an omitted value
// never overwrites a recorded one.
ai_enabled: Option<i64>,
) -> Result<(), String> {
let conn = lock(&state)?;
let row = sessions::SessionRow {
Expand All @@ -43,6 +47,7 @@ pub fn sessions_insert(
generated_at,
confident_samples,
skipped_samples,
ai_enabled,
};
sessions::insert(&conn, &row).map_err(|e| e.to_string())
}
Expand Down
115 changes: 108 additions & 7 deletions src-tauri/src/commands/sidecar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,20 @@ const LOG_MAX_BYTES: u64 = 5 * 1024 * 1024;
const RESTART_BUDGET: u32 = 3;
const MIN_HEALTHY_UPTIME: Duration = Duration::from_secs(120);
const RESTART_BACKOFF: Duration = Duration::from_millis(500);
// I83 — a lifetime ceiling on respawns within one generation, because the
// consecutive-streak rule above has a hole: a child that dies every ~2.5
// minutes clears MIN_HEALTHY_UPTIME every time, so `restart_attempts` resets to
// 1 forever and `errored` is never set. The JS stall notice fires once and the
// session then runs for an hour on an engine that is dying and respawning the
// whole time, recording nothing. This counts EVERY respawn in the generation,
// not just the sub-uptime ones, or a 121-second cycle would escape it again.
//
// 12 is chosen so the two cases stay far apart: a genuinely long session whose
// sidecar dies once an hour after clean uptime spends 8 respawns across 8 hours
// and never trips, while a 121-second crash cycle trips at roughly 24 minutes.
// An explicit sidecar_stop + sidecar_start bumps the generation, so a
// deliberate retry always starts from zero.
const TOTAL_RESTART_BUDGET: u32 = 12;

#[derive(Default)]
struct SidecarInner {
Expand Down Expand Up @@ -490,7 +504,21 @@ fn spawn_with_fallback<R: Runtime>(
for (source, binary) in candidates {
let runtime_dir = match source {
// Companion dylibs/dlls that tauri bundles under Resources/.
super::engine::EngineSource::Bundled => resolve_runtime_dir(app).ok().flatten(),
//
// I83 — fall back to the binary's own directory when the resource
// path doesn't resolve. `resolve_runtime_dir` returns Ok(None) for
// any miss (wrong triple, bundler laid the companions out
// differently — I73 is precisely that having happened once), and a
// None used to mean spawning with NO working directory and NO PATH
// prepend: the exact state I75 fixed, still reachable, and fatal on
// Windows where the ggml backends are dlopen-only. The exe's own
// directory is one of the two places ggml_backend_load_best globs
// anyway, so this fallback is never worse than None and is right
// whenever the companions ship beside the binary.
super::engine::EngineSource::Bundled => resolve_runtime_dir(app)
.ok()
.flatten()
.or_else(|| binary.parent().map(Path::to_path_buf)),
// The managed install keeps libraries next to the binary, where
// @loader_path / $ORIGIN already resolve them; prepending the dir
// anyway keeps both sources on one code path.
Expand Down Expand Up @@ -518,15 +546,21 @@ fn spawn_with_fallback<R: Runtime>(
// bare CreateProcess error.
#[cfg(target_os = "windows")]
fn append_windows_dll_hint(err: String) -> String {
// I83 — probe BOTH halves of the redistributable. llama-server.exe links
// the C++ standard library (msvcp140.dll) as well as the C runtime
// (vcruntime140.dll), and a machine can carry one without the other: some
// installers ship vcruntime140 alone, and a repair/uninstall can leave a
// partial set. Checking only vcruntime140 meant the actionable hint stayed
// silent on exactly the boxes that needed it most.
let sysroot = std::env::var("SystemRoot").unwrap_or_else(|_| r"C:\Windows".to_string());
let vcruntime = Path::new(&sysroot)
.join("System32")
.join("vcruntime140.dll");
if vcruntime.exists() {
let system32 = Path::new(&sysroot).join("System32");
let both_present =
system32.join("vcruntime140.dll").exists() && system32.join("msvcp140.dll").exists();
if both_present {
err
} else {
format!(
"{err}; the Microsoft Visual C++ runtime is missing — install it from https://aka.ms/vs/17/release/vc_redist.x64.exe and try again"
"{err}; the Microsoft Visual C++ runtime is missing or incomplete — install it from https://aka.ms/vs/17/release/vc_redist.x64.exe and try again"
)
}
}
Expand Down Expand Up @@ -633,6 +667,14 @@ fn next_attempts(prev: u32, uptime: Duration) -> u32 {
}
}

// I83 — has this generation respawned so many times that the engine should be
// called dead regardless of how long each child survived? Separate from
// `next_attempts` on purpose: that one answers "is this a crash loop right
// now?", this one answers "has this been going on all session?".
fn exceeded_total_restarts(total: u32) -> bool {
total > TOTAL_RESTART_BUDGET
}

async fn watch<R: Runtime>(
app: AppHandle<R>,
state: Arc<Mutex<SidecarInner>>,
Expand All @@ -645,6 +687,8 @@ async fn watch<R: Runtime>(
) {
let mut log = log_file;
let mut restart_attempts: u32 = 0;
// I83 — every respawn in this generation, never reset by a clean run.
let mut total_restarts: u32 = 0;
let mut child_started_at = Instant::now();

loop {
Expand Down Expand Up @@ -691,11 +735,36 @@ async fn watch<R: Runtime>(

// Count consecutive short-lived deaths; a durable child resets to 1.
restart_attempts = next_attempts(restart_attempts, child_started_at.elapsed());
total_restarts += 1;
// I83 — a slow crash loop clears MIN_HEALTHY_UPTIME on every cycle, so
// the streak check below can never fire for it. Stop pretending this
// engine is going to recover.
if exceeded_total_restarts(total_restarts) {
guard.errored = true;
guard.last_error = Some(append_windows_dll_hint(format!(
"the AI engine restarted {total_restarts} times this session and kept dying"
)));
guard.port = None;
let _ = writeln!(
log,
"[event] giving up after {total_restarts} lifetime restarts (slow crash loop)"
);
let _ = log.flush();
return;
}
if restart_attempts > RESTART_BUDGET {
guard.errored = true;
// I83 — carry the Windows VC++ hint here too. A child that dies
// inside the loader spawns successfully (CreateProcess returns a
// handle before the DLL resolution that kills it), so it never
// reaches the spawn-failure path where this hint was applied — it
// crash-loops to the restart budget instead, and the toast the JS
// side raises from `last_error` named an exit code and nothing the
// user could act on.
guard.last_error = last_exit
.clone()
.or_else(|| Some(format!("restart budget exceeded ({RESTART_BUDGET})")));
.or_else(|| Some(format!("restart budget exceeded ({RESTART_BUDGET})")))
.map(append_windows_dll_hint);
guard.port = None;
let _ = writeln!(
log,
Expand Down Expand Up @@ -807,4 +876,36 @@ mod tests {
fn first_crash_starts_at_one() {
assert_eq!(next_attempts(0, Duration::from_secs(1)), 1);
}

// I83 — the hole the lifetime cap closes: a child dying just past
// MIN_HEALTHY_UPTIME resets the consecutive streak on every cycle, so
// `restart_attempts` never exceeds RESTART_BUDGET and `errored` is never
// set. Simulate that cycle and show the streak rule alone never gives up.
#[test]
fn slow_crash_loop_never_trips_the_consecutive_streak() {
let mut attempts = 0;
let just_past_healthy = MIN_HEALTHY_UPTIME + Duration::from_secs(1);
for _ in 0..50 {
attempts = next_attempts(attempts, just_past_healthy);
assert!(
attempts <= RESTART_BUDGET,
"a 121s crash cycle must never reach the streak budget — \
that is exactly why the lifetime cap exists"
);
}
}

#[test]
fn lifetime_cap_stops_a_slow_crash_loop() {
// The same cycle, counted the other way: every respawn accumulates.
assert!(!exceeded_total_restarts(TOTAL_RESTART_BUDGET));
assert!(exceeded_total_restarts(TOTAL_RESTART_BUDGET + 1));
}

#[test]
fn lifetime_cap_leaves_a_long_healthy_session_alone() {
// An 8-hour session whose sidecar dies once an hour after clean uptime
// spends 8 respawns. It must never be called a crash loop.
assert!(!exceeded_total_restarts(8));
}
}
55 changes: 54 additions & 1 deletion src-tauri/src/db/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ use rusqlite::{Connection, TransactionBehavior};
const MIGRATION_001_INITIAL: &str = include_str!("migrations/001_initial.sql");
const MIGRATION_002_V2: &str = include_str!("migrations/002_v2.sql");
const MIGRATION_003_SAMPLE_COUNTS: &str = include_str!("migrations/003_sample_counts.sql");
const MIGRATION_004_AI_ENABLED: &str = include_str!("migrations/004_ai_enabled.sql");

const MIGRATIONS: &[(u32, &str)] = &[
(1, MIGRATION_001_INITIAL),
(2, MIGRATION_002_V2),
(3, MIGRATION_003_SAMPLE_COUNTS),
(4, MIGRATION_004_AI_ENABLED),
];

pub const MAX_KNOWN_VERSION: u32 = MIGRATIONS[MIGRATIONS.len() - 1].0;
Expand Down Expand Up @@ -110,7 +112,7 @@ mod tests {
.unwrap_or(0)
}

const LATEST_VERSION: u32 = 3;
const LATEST_VERSION: u32 = 4;

#[test]
fn applies_full_schema_on_empty_db() {
Expand Down Expand Up @@ -235,6 +237,53 @@ mod tests {
assert_eq!(skipped, None);
}

// I83 acceptance: 004 runs cleanly on a database already at schema_version
// 3 with a real session row, and that pre-migration row reads back a NULL
// `ai_enabled` — "unknown", never a fabricated 0 that would let the report
// claim AI was off in a session nobody recorded the setting for.
#[test]
fn upgrades_v3_db_to_v4_with_null_ai_enabled_on_old_rows() {
let mut conn = Connection::open_in_memory().expect("open in-memory");
{
conn.execute(
"CREATE TABLE IF NOT EXISTS schema_version (version INTEGER PRIMARY KEY)",
[],
)
.expect("schema_version");
let tx = conn.transaction().expect("tx");
tx.execute_batch(MIGRATION_001_INITIAL).expect("apply 001");
tx.execute_batch(MIGRATION_002_V2).expect("apply 002");
tx.execute_batch(MIGRATION_003_SAMPLE_COUNTS)
.expect("apply 003");
tx.execute(
"INSERT INTO schema_version (version) VALUES (1), (2), (3)",
[],
)
.expect("record v3");
tx.commit().expect("commit v3");
}
conn.execute(
"INSERT INTO sessions (id, started_at, score, confident_samples)
VALUES ('s1', 1, 90, 24)",
[],
)
.expect("insert session");
assert_eq!(current_version(&conn), 3);

let applied = run_migrations(&mut conn).expect("upgrade run");
assert_eq!(applied, LATEST_VERSION);

let (ai_enabled, confident): (Option<i64>, Option<i64>) = conn
.query_row(
"SELECT ai_enabled, confident_samples FROM sessions WHERE id = 's1'",
[],
|row| Ok((row.get(0)?, row.get(1)?)),
)
.expect("read ai_enabled");
assert_eq!(ai_enabled, None, "pre-004 rows must read as unknown");
assert_eq!(confident, Some(24), "003 data must survive the 004 upgrade");
}

#[test]
fn refuses_db_created_by_newer_version() {
let mut conn = Connection::open_in_memory().expect("open in-memory");
Expand Down Expand Up @@ -298,6 +347,10 @@ mod tests {
3,
"a1ef24581336a04ecb9f9636afe3d0c574d9e47072f88ffccd1ff3c9aefffa42",
),
(
4,
"f394e5e3179254fafb8f682dac3b189687e7ff7c5200b0b280b75cd5a26607e3",
),
];
assert_eq!(
MIGRATIONS.len(),
Expand Down
24 changes: 24 additions & 0 deletions src-tauri/src/db/migrations/004_ai_enabled.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
-- I83 migration. Chained behind 003 (never edit a shipped migration).
--
-- Whether on-device AI focus detection was ENABLED for this session, recorded
-- at teardown from the live settings value. Without it, `score`/`focused_pct`/
-- `confident_samples`/`skipped_samples` all reading NULL is ambiguous three
-- ways — AI deliberately off, AI on but never able to run a single check, or a
-- row written by a build older than the counters — and the post-session report
-- cannot tell the user which happened. Issue #92 is that ambiguity seen from
-- the outside: a Windows session where AI was on and silently dead rendered
-- identically to a clean AI-off session, down to "No distractions detected.
-- Nice work."
--
-- 1 = AI features were on, 0 = off, NULL = unknown (any row written before
-- this migration). The report treats NULL as "unknown" and keeps its existing
-- cause-neutral copy, so old rows never gain a claim nobody recorded.
--
-- Read at TEARDOWN, not at session start, so a mid-session toggle is recorded
-- as its final state. That is deliberate rather than merely convenient: the
-- question this column answers is "should the report explain an absence?", and
-- a session that recorded checks is identified by the sample counters, which
-- the report consults FIRST (see aiCoverage in reportData.ts). So a user who
-- toggles AI off after a scored session still reads as measured, and only a
-- session with nothing to show falls through to this column for its wording.
ALTER TABLE sessions ADD COLUMN ai_enabled INTEGER;
1 change: 1 addition & 0 deletions src-tauri/src/db/migrations/MANIFEST.sha256
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
d19c380c48d5986806f36eedd72332f2d96e57390ce92ed839fbffe51bc8300e 001_initial.sql
f01897d50e1d448a0995ace633c04c82b454c32c0e792a94964a81ae46031685 002_v2.sql
a1ef24581336a04ecb9f9636afe3d0c574d9e47072f88ffccd1ff3c9aefffa42 003_sample_counts.sql
f394e5e3179254fafb8f682dac3b189687e7ff7c5200b0b280b75cd5a26607e3 004_ai_enabled.sql
Loading
Loading