fix(ai): start the sample loop on a fresh launch, and let the report say when AI didn't run by scotej · Pull Request #147 · scotej/studyvis

scotej · 2026-07-28T08:25:46Z

Fixes #92. Very likely fixes #94 too (same root cause, different platform) — worth a re-test before closing that one.

The report in #92 was telling the truth. Nothing had measured anything.

useModelStore is never hydrated outside Settings → AI. hydrate() had exactly one caller — ModelPickerContainer's mount effect (ModelPickerContainer.tsx:85) — and that component mounts only inside the Settings → AI pane.

useSettingsStore is hydrated at boot, by ThemeProvider (src/design/theme.tsx:52). So on a fresh launch aiFeaturesEnabled read correctly as true while activeModelId sat at its null initial value. And activeModelId gates the entire pipeline:

SessionView.tsx's sample-loop effect returns early on if (!activeModelId) — so startSampleLoop is never called, and its onStartFail('no_active_model') toast, the one surface that names this problem, can never fire.
Home.tsx's handleTopicSubmit skipped the V2-P9 gesture-context preacquireScreenStream() on the same condition — separately fatal on WebView2.

So any launch where the user didn't happen to open Settings → AI ran a full session with AI silently dead: no loop, no toast, no audit row, no log line, and an unscored sessions row. Cross-platform, present at HEAD. The user in #92 had a configured, benchmarked model; the app just never looked at it.

Then the report made the silence permanent. score, focused_pct, confident_samples and skipped_samples all read NULL for an AI-off session, an AI-on-but-dead session and a row predating the 003 counters — so no surface could tell a deliberate choice from a malfunction, and the distractions section asserted a clean measurement that never happened:

No focus score was recorded for this session.
…
No distractions detected. Nice work.

What changed

The loop actually starts

Hydrate useModelStore in Home's boot effect.
Pre-acquire the screen stream from every real gesture that (re)boots the loop: topic submit, Rejoin, and the camera/mic "Try again". A store still mid-hydration counts as "maybe active" — a spurious acquire is cheap, a missed one is fatal on WebView2.
Toast once per session when AI is on, the model store is ready, and no model is active. That is the gap onStartFail could never cover, because the loop it belongs to was never constructed.

Failures stop being silent — every path below was console.warn-only, and release builds have no devtools

onStalled fires once per loop lifetime after 3 consecutive unproductive ticks, with a distinct reason and actionable copy per cause (engine_unavailable / engine_error / inference_timeout / unknown), and flips the AI chip to error so the state outlives the toast. Paused states — break, camera off, pomodoro rest, battery — are deliberately not stalls.
The per-tick timeout is derived from the model's benchmarked p95 (effectiveRequestTimeoutMs: 3× p95, floored at the old 90 s, capped at benchmark.ts's own 300 s). The flat 90 s was 3.3× tighter than the benchmark's bound, so a model could benchmark successfully — the only thing that sets activeModelId — and then abort every live inference, forever.
SCREEN_ACQUIRE_TIMEOUT_MS (120 s) bounds the acquire. getDisplayMedia never times out on its own, so an unanswered picker wedged boot() permanently — and since stop() awaits bootPromise, the sidecar it had already started was never killed either. A late-arriving stream is stopped rather than leaked.
InvalidStateError / InvalidAccessError (missing transient activation — Chromium and WebKit respectively) now map to screen_capture_denied, which mounts the recovery overlay. Its "Try again" button is itself the user gesture the call was missing, so the retry genuinely works; the old classification produced a dead-end toast carrying a raw DOMException string.
Release the pre-acquired stream when a session never starts (rejoin failure; host identity guard), instead of leaving the OS recording indicator lit with nothing behind it.

The report can say what happened — migration 004 adds sessions.ai_enabled

Written from live settings at teardown; NULL only on pre-004 rows.
New aiCoverage() gives five honest states: ran keeps the earned "Nice work", noConfident covers checks that ran but couldn't be read (see round 2), noChecks names the malfunction and points at Settings → AI, off says AI was off, unknown stays cause-neutral for old rows.
Shared by the rendered report and the text export, so a pasted copy can never disagree with the screen.

Rust sidecar diagnostics

resolve_runtime_dir falls back to the binary's own directory instead of None. None meant spawning with no working directory and no PATH prepend — the exact lethal-on-Windows state I75 fixed, still reachable on any resource-path miss. The exe's own directory is one of the two places ggml_backend_load_best globs anyway, so the fallback is never worse.
The crash-loop give-up path now carries append_windows_dll_hint. A child that dies inside the Windows loader spawns successfully (CreateProcess returns before the DLL resolution that kills it), so it never reached the spawn-failure path where that hint lived — it crash-looped to the restart budget and surfaced an exit code the user could do nothing with.

Round 2: what an adversarial sweep found in the first draft

The first three commits were then verified by a 65-agent sweep (two independent checkers per finding). It confirmed the root cause above — both checkers, produces_screenshot: yes, still-broken at HEAD — and found six more defects on the same path, all fixed in the fourth commit:

hydrate()'s 'error' status was terminal. ModelPickerContainer retried only on 'loading', so one failed models.json read (AV lock, partial write) left activeModelId null for the whole process — reopening this very issue through a narrower door that the hydration fix itself widened. The gate is now status !== 'ready', and the session notice distinguishes "no model picked" from "model list unreadable".
The chip read "AI off" while AI was ON. With no model active, the footer chip claimed the feature was off — the only on-screen signal during Ai feature log for sessions broken on windows #92, pointing at exactly the wrong setting. New 'unconfigured' status ("AI needs a model") behind a pure, unit-tested deriveAiChipStatus(). 'loading' deliberately still reads 'off', or every launch would flash a warning during the hydration window.
aiCoverage returned 'ran' for a session where no check was readable. I had defended this in a comment and a test, arguing the Project analysis (post-v1.3.1): verified improvement backlog — 4 real bugs, 22 improvements #47 D5 data-quality line caveats it. It does not: that line needs SKIPPED_SAMPLES_MIN (3) skips before it renders anything, so for 1–2 unreadable checks the page showed "Focused-time —", no caveat at all, and "No distractions detected. Nice work." — this issue's own defect surviving its own fix. Added a fifth state 'noConfident', and edited the two tests that asserted the old behaviour rather than appending around them.
append_windows_dll_hint probed only vcruntime140.dll. llama-server.exe also links the C++ standard library, and a machine can carry one without the other, so the actionable hint stayed silent on some of the boxes that needed it. Requires both now.
A slow crash loop never terminated. next_attempts resets the streak whenever a child clears MIN_HEALTHY_UPTIME (120 s), so a sidecar dying every ~2.5 minutes crash-looped forever without ever setting errored: the stall notice fired once and the session then ran for an hour on a dying engine. TOTAL_RESTART_BUDGET (12 per generation) closes it, sized so an 8-hour session dying hourly never trips while a 121-second cycle trips at ~24 minutes.
Settings → Sessions now marks an unmeasured row not measured when ai_enabled === 1 — the second place a user looks for a missing number. Never inferred for 0 or NULL.

One finding I rejected: "onSidecarErrored re-arms every tick, so a flapping sidecar re-toasts forever." errored is cleared only by sidecar_start / sidecar_stop (sidecar.rs:233/:270) and the watcher returns after setting it, so errored → running requires deliberate user action. Re-notifying after the user restarts and it fails again is correct, and the existing test documents that as intended.

Testing

npm run build/lint/test (903 pass), check-tokens, check-strings, check-migrations, check-stories, check-contrast, and cargo test/fmt --check/clippy all green. The one clippy warning (watch() has 8 args) pre-exists on main.

New tests (914 total, up from 903):

aiCoverage — 8 cases, including the pre-003 row that recorded a score (must still read ran), the zero-is-not-non-null rule, the skipped: 2 window paired with an assertion that sampleQualitySummary returns null there (that pairing is the evidence for the reversal), and the NULL-is-not-0 rule.
Serializer honesty — 5 cases over the exported text, including that a genuinely clean session keeps "Nice work" and that the noConfident copy never claims "No AI checks ran".
deriveAiChipStatus — 6 cases, pinning the guard order, not just the states.
snapshotFocusForReport.aiEnabled — 3 cases.
Model-store hydration — an 'error' that invents no model, and a second hydrate() after an error reaching 'ready' (pins the retry gate).
Stall notice — 4 cases, including that a resolved sample resets the streak (no crying wolf on intermittent failures) and that a camera-off stretch is not a stall.
effectiveRequestTimeoutMs — 4 boundary cases.
Rust: the 003→004 upgrade (old rows read NULL, 003 data survives), and three restart-budget cases — one of which demonstrates the hole by running a 121-second crash cycle 50 times and asserting the consecutive-streak rule never fires.

Stories: AiOnButNoChecks (the #92 state), AiOffForSession, AiRanNoReadableChecks, and an Unconfigured chip variant; NoAiBaseline re-documented as the pre-004 unknown case.

Manual testing: none

Every fix here is verified by unit tests and code reading only. I did not run the app — no npm run tauri dev, no macOS walk, and no Windows walk. Windows-MCP is Windows-only and this session ran on macOS, so nothing was machine-walked either.

That matters most for the parts that are specifically about Windows runtime behaviour and cannot be proven from source: the WebView2 gesture requirement, the picker-wedge timeout, the InvalidStateError mapping, and the VC++ DLL hint. The primary fix is the exception — model-store hydration is cross-platform and directly observable: with AI on and a benchmarked model, start a session without visiting Settings → AI, and the report should now carry a score where it previously carried none.

Deliberately not fixed

Each of these was confirmed by two independent verifiers during the investigation, and each is out of scope for #92:

"All displays" capture can never acquire display 2+ — the spec consumes the transient activation on the first acquire, so every additional display needs its own gesture. A real defect in a V3 setting, but it doesn't produce an all-NULL report, and fixing it means designing a per-display gesture flow.
001_initial.sql was amended in place pre-release, so any app.db created before 2026-05-11 permanently lacks focused_pct/generated_at and can never persist a sessions row. Still unfixed at HEAD, overlaps Session history and stats broken #99, and needs its own migration rather than a rider on this one.
Session history and stats broken #99 (blank history/stats on exit) is a different failure point in the same teardown chain — the row is never written at all, as opposed to written without AI data. Separate issue, separate fix.
A camera that never comes up blocks the loop before it boots, and the user sees the generic MediaErrorBanner rather than anything AI-specific. Correct behaviour (no camera, nothing to analyse) but the connection to AI going quiet isn't spelled out. Left alone.
I18 / I9 remain accepted deviations per ISSUES.md.

Ledgered as I83.

Rebased onto main after it moved 53 files mid-flight. Only ISSUES.md conflicted (main reformatted the table); every code file auto-merged. Main also took I79 — for a macOS display-capture fix that touches sampleLoop.ts too — so this branch's tag moved to I83 across all 27 files, clear of PR #146's claimed I82.

Overlaps with #146 ("say when on-device AI can't take a reading", I82), which independently addresses the same symptom — silent AI failures — via sampleLoop.ts, SessionView.tsx and a new audit-event kind. This PR is the one that fixes the root cause (useModelStore never hydrating), so they are complementary rather than duplicates, but the two will conflict in sampleLoop.ts and strings.ts and should not both be merged as-is. Worth deciding which lands first and rebasing the other onto it.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added clearer AI status indicators, including “AI needs a model.”
- Reports now distinguish between AI being off, unavailable, unused, or producing no readable results.
- Added tailored guidance for missing models, stalled analysis, and screen-capture permission recovery.
- Session history now identifies sessions where AI scoring was not measured.
Bug Fixes
- Improved AI model hydration and recovery after loading errors.
- Prevented repeated engine restart loops and improved engine startup diagnostics.
- Added bounded screen capture and more reliable timeout handling.

…say when AI didn't run (I79) Issue #92 screenshotted a Windows session report with "Focused-time —", "No focus score was recorded", no ai_* timeline rows — and "No distractions detected. Nice work." right beside all of it. Root cause: useModelStore is never hydrated outside Settings → AI. hydrate() had exactly one caller, ModelPickerContainer's mount effect, and that component lives only in the Settings → AI pane. useSettingsStore is hydrated at boot by ThemeProvider, so aiFeaturesEnabled was correctly true while activeModelId sat at its null initial value — and activeModelId gates everything: SessionView's loop effect returns early on !activeModelId (so startSampleLoop never runs and its no_active_model toast can never fire), and Home's topic-submit skips the V2-P9 gesture-context screen pre-acquire on the same condition, which on WebView2 is separately fatal. Any launch without a Settings → AI visit ran a whole session with AI silently dead. Cross-platform, present at HEAD; also explains #94. Fixes, in the order a session hits them: - hydrate the model store in Home's boot effect - pre-acquire the screen stream from every real gesture that (re)boots the loop: topic submit, Rejoin, camera/mic "Try again"; a store mid-hydration counts as "maybe active" - toast once per session when AI is on, the store is ready, and no model is active — the gap onStartFail could never cover - onStalled: one notice per loop lifetime after 3 consecutive unproductive ticks, with a distinct reason and actionable copy per cause. Every path it covers was console.warn-only, and release builds have no devtools. Paused states are not stalls. - derive the per-tick timeout from the model's benchmarked p95 (3x, floored at 90s, capped at benchmark.ts's 300s). A model could benchmark successfully — the only thing that sets activeModelId — and then abort every live tick. - bound the screen acquire at 120s so an unanswered picker is a retryable error, not a permanent boot() wedge that also strands the sidecar - map InvalidStateError/InvalidAccessError to screen_capture_denied, whose overlay retry is itself the missing user gesture - migration 004 records sessions.ai_enabled, and aiCoverage() gives the report four honest states instead of one shrug: ran (keeps "Nice work"), noChecks (names the malfunction), off, unknown (pre-004). Shared with the text export. - Rust: resolve_runtime_dir falls back to the binary's own directory rather than None (None meant no CWD and no PATH prepend — the state I75 fixed), and the crash-loop give-up path carries the VC++ redist hint, which a child dying in the Windows loader never reached. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…starts (I79) The V2-P9 gesture pre-acquire stashes a live screen MediaStream for boot() to consume, and the only release path was SessionView's unmount. Two paths reach a stash with no SessionView behind it, and this PR added one of them: - handleRejoin: joinSession() throws (bad stored credentials), the catch toasts, and status never flips to 'active'. - handleTopicSubmit + host: runHostInvite returns silently on its identity/display_name guard before hostSession() begins the session. Either way the OS screen-recording indicator stays lit with nothing behind it until the app quits. Discard on the rejoin failure, and don't pre-acquire at all when the host precondition already fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Comment-only on migration 004, whose hash is re-pinned in migrations.rs and re-manifested. Editing a .sql already in MANIFEST.sha256 is exactly what the forward-only guard blocks, and the escape hatch it documents applies here: 004 is new in this PR and has never shipped, so its manifest line was removed by hand before regenerating. The semantic worth stating: the column is the FINAL state of the AI toggle, not its state at session start. That is deliberate. The question it answers is "should the report explain an absence?", and aiCoverage() consults the sample counters first — so a session that recorded checks still reads as measured even if the user switched AI off before leaving, and only a session with nothing to show falls through to this column for its wording. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A 65-agent verification sweep over the first draft confirmed the root cause (two independent verifiers, cross-platform, still-broken at HEAD) and found six more defects on the same path: - hydrate()'s 'error' status was terminal. ModelPickerContainer retried only on 'loading', so a single failed models.json read (AV lock, partial write) left activeModelId null for the whole process — reopening #92 through a narrower door that fix A itself widened. Gate is now status !== 'ready', and the session notice distinguishes "no model picked" from "model list unreadable". - The footer chip read "AI off" while AI was ON with no model. That was the only on-screen signal during #92 and it pointed at the wrong setting. New 'unconfigured' status behind a pure deriveAiChipStatus(), with 'loading' deliberately reading 'off' so no launch flashes a warning during hydration. - aiCoverage returned 'ran' for confident_samples 0 with skipped_samples k, on the argument that the #47 D5 data-quality line caveats it. It does not below SKIPPED_SAMPLES_MIN (3): for k of 1-2 the page rendered "Focused-time —", no caveat, and "No distractions detected. Nice work." — this issue's own defect surviving its own fix. Fifth state 'noConfident'; the two tests that asserted the old behaviour are edited, not appended to. - append_windows_dll_hint probed only vcruntime140.dll, so it stayed silent on a machine with the C runtime but not the C++ one. Requires both now. - next_attempts resets the restart streak whenever a child clears MIN_HEALTHY_UPTIME, so a sidecar dying every ~2.5 minutes crash-looped forever and never set errored: the stall notice fired once and the session then ran an hour on a dying engine. TOTAL_RESTART_BUDGET (12 per generation) closes it, sized so an 8-hour session dying hourly never trips while a 121s cycle trips at ~24 minutes. - Settings → Sessions marks an unmeasured row "not measured" when ai_enabled is 1, never inferring it for 0 or NULL. One finding rejected: "onSidecarErrored re-arms every tick, so a flapping sidecar re-toasts forever". errored is cleared only by sidecar_start/stop and the watcher returns after setting it, so errored→running requires deliberate user action — re-notifying then is correct, as the existing test documents. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coderabbitai · 2026-07-28T08:26:17Z

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (1)

ISSUES.md

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d3e9827f-20d6-4545-95ad-2bccc4cb1047

📥 Commits

Reviewing files that changed from the base of the PR and between f7100d3 and 1cec908.

📒 Files selected for processing (1)

ISSUES.md

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR adds nullable AI enablement tracking to sessions, introduces migration 4, improves sidecar and sampling-loop recovery, distinguishes unconfigured AI status, and makes reports and session metadata aware of AI coverage states.

Changes

AI session and runtime behavior

Layer / File(s)	Summary
Persist session AI state `src-tauri/src/db/...`, `src-tauri/src/commands/sessions.rs`, `src/lib/db/sessions.ts`, `src/features/ai/focusStore.ts`, `src/features/session/lifecycle.ts`	Session records, migrations, inserts, reads, lifecycle persistence, stories, tests, and fixtures now carry nullable AI enablement values.
Harden sidecar and sampling recovery `src-tauri/src/commands/sidecar.rs`, `src/features/ai/sampleLoop.ts`, `src/features/ai/captureScreen.ts`, `src/features/ai/ModelPickerContainer.tsx`, `tests/unit/ai-*`	Sidecar restarts receive a lifetime cap; sampling adds bounded capture, adaptive request timeouts, stall notifications, and model hydration recovery.
Model readiness and session AI status `src/features/session/aiChip.ts`, `src/features/session/SessionView.tsx`, `src/routes/Home.tsx`, `src/components/AiStatusChip.tsx`, `src/stories/AiStatusChip.stories.tsx`	The UI distinguishes disabled, loading, unconfigured, and runtime states while updating startup, retry, toast, and screen-stream flows.
AI coverage-aware reporting `src/features/session/reportData.ts`, `src/features/session/reportSerialize.ts`, `src/features/session/Report.tsx`, `src/features/settings/categories/SessionsCategory.tsx`, `src/strings.ts`, `src/stories/Report.stories.tsx`, `tests/unit/report-*`	Reports and session metadata classify AI coverage as measured, unreadable, unchecked, off, or unknown and select corresponding copy.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Home
  participant ModelStore
  participant SampleLoop
  participant Sidecar
  participant SessionView
  Home->>ModelStore: hydrate model configuration
  SessionView->>SampleLoop: start sampling with stall callback
  SampleLoop->>Sidecar: check health and request inference
  Sidecar-->>SampleLoop: result, error, or timeout
  SampleLoop-->>SessionView: report stall reason
  SessionView-->>SessionView: show toast and update AI status

Possibly related PRs

scotej/studyvis#80 — Modifies the sidecar crash-restart budget and watch-loop logic.
scotej/studyvis#103 — Uses gesture-based screen-stream pre-acquisition and discard handling.
scotej/studyvis#146 — Modifies end-to-end AI sampling stall handling.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	Several large additions go beyond [`#92`, `#94`], including migration 004, stall detection, timeout tuning, AI chip/status UI changes, and report coverage logic.	Move the unrelated runtime/reporting/UI changes into separate PRs, or explicitly expand the linked issue scope to cover them.
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change: boot-time AI loop initialization plus report wording for sessions where AI didn't run.
Description check	✅ Passed	The description is detailed and covers what changed, why, testing, manual testing status, compatibility surfaces, and merge style.
Linked Issues check	✅ Passed	The changes address [`#92`, `#94`] by hydrating the model store at boot, retrying hydration on error, and preserving honest AI session/report logging.

✨ Finishing Touches

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch fix/ai-focus-never-runs-i79

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/features/ai/sampleLoop.ts (1)
1008-1032: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Teardown-triggered aborts/capture failures get misclassified as live stalls, firing callbacks after the loop already stopped.

teardownInternal() (invoked by stop(), called whenever status/aiFeaturesEnabled/activeModelId/localStream/captureDenied change in SessionView, i.e. on leave, AI-toggle, model change, or camera loss) calls activeAbort.abort() on any in-flight fetch. That abort surfaces inside tick()'s catch block (lines 1024-1029) as a DOMException/AbortError, which is now new code that unconditionally calls noteUnproductiveTick('inference_timeout') — this can push unproductiveTicks to STALL_TICKS and fire opts.onStalled even though the loop was told to stop moments earlier. Since toast.error in SessionView's onStalled handler is a global singleton, the misleading "AI stalled" toast (and setAiRuntimeStatus('error')) can still surface after the user has already left the session, toggled AI off, or switched models — precisely the kind of false signal issue #92 was trying to eliminate.

The same gap exists in boot()'s catch around the new acquireScreenStreamBounded() (line 1210): if stop() races the (now up to 2-minute) acquire wait, onCaptureError/onCaptureDenied still fire unconditionally in the catch (1211-1226) with no state.stopped check, even though teardownInternal() isn't called from this path — meaning if the loop was already torn down elsewhere for another reason mid-acquire, this can duplicate/contradict the teardown reason.

Guard both catch blocks with an early state.stopped check so a teardown-in-flight abort/failure is silently discarded rather than reported as a new stall/capture error.
🐛 Proposed fix
     } catch (err) {
+      // I79 — teardownInternal() (via stop()) aborts any in-flight request.
+      // That abort is not a live stall — the loop is already being torn
+      // down — so it must not count toward the stall streak or fire
+      // onStalled/onCaptureError after teardown has started.
+      if (state.stopped) return
       if (err instanceof CaptureError) {
         if (err.code === 'screen_capture_denied') {
Also applies to: 1208-1231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/ai/sampleLoop.ts` around lines 1008 - 1032, Guard both the
tick() catch block and the boot() catch around acquireScreenStreamBounded() with
an immediate state.stopped check that returns before recording unproductive
ticks or invoking capture/error callbacks. Preserve the existing handling for
errors received while the loop remains active, including permission-denied
behavior and inference-timeout reporting.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/features/ai/ModelPickerContainer.tsx`:
- Around line 83-95: Update the hydration effect in ModelPickerContainer so
hydrate() runs once per picker mount instead of re-triggering when its own
status changes to loading or error. Remove status from the effect’s
retry-triggering logic while retaining the existing hydrate dependency and
non-ready initialization behavior, or introduce an explicit user/visit retry
trigger that cannot be caused by hydrate’s state transitions.

In `@src/features/session/aiChip.ts`:
- Around line 44-50: Update the status logic in the AI chip state function
around activeModelId, modelStatus, and hasLocalStream so enabled-but-not-ready
states never return “off”: return distinct statuses for model loading, model
errors, and missing local streams, while reserving “unconfigured” for genuinely
disabled or absent setup. Add tests covering each loading, error, and
missing-stream case.

---

Outside diff comments:
In `@src/features/ai/sampleLoop.ts`:
- Around line 1008-1032: Guard both the tick() catch block and the boot() catch
around acquireScreenStreamBounded() with an immediate state.stopped check that
returns before recording unproductive ticks or invoking capture/error callbacks.
Preserve the existing handling for errors received while the loop remains
active, including permission-denied behavior and inference-timeout reporting.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3e45511f-e034-41a6-b378-65d63aa36e3d

📥 Commits

Reviewing files that changed from the base of the PR and between 6732446 and bbe34c4.

📒 Files selected for processing (36)

ISSUES.md
src-tauri/src/commands/sessions.rs
src-tauri/src/commands/sidecar.rs
src-tauri/src/db/migrations.rs
src-tauri/src/db/migrations/004_ai_enabled.sql
src-tauri/src/db/migrations/MANIFEST.sha256
src-tauri/src/db/sessions.rs
src/components/AiStatusChip.tsx
src/features/ai/ModelPickerContainer.tsx
src/features/ai/captureScreen.ts
src/features/ai/focusStore.ts
src/features/ai/index.ts
src/features/ai/sampleLoop.ts
src/features/session/Report.tsx
src/features/session/SessionView.tsx
src/features/session/aiChip.ts
src/features/session/lifecycle.ts
src/features/session/reportData.ts
src/features/session/reportSerialize.ts
src/features/settings/categories/SessionsCategory.tsx
src/lib/db/sessions.ts
src/routes/Home.tsx
src/stories/AiStatusChip.stories.tsx
src/stories/Dashboard.stories.tsx
src/stories/FocusInsights.stories.tsx
src/stories/Report.stories.tsx
src/strings.ts
tests/unit/ai-chip-status.test.ts
tests/unit/ai-focus-store.test.ts
tests/unit/ai-models.test.ts
tests/unit/ai-sample-loop.test.ts
tests/unit/file-export.test.ts
tests/unit/report-data.test.ts
tests/unit/report-serialize.test.ts
tests/unit/stats-data.test.ts
tests/unit/stats-insights.test.ts

📜 Review details

🧰 Additional context used

📓 Path-based instructions (7)

**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Do not add telemetry or break compatibility with released local data, peer wire formats, or identity derivation.
Avoid unrelated refactoring, speculative abstractions, and comments unless the reason is non-obvious.
Use one focused change per commit and conventional-commit subjects such as feat:, fix:, chore:, docs:, or ci:.

Files:

ISSUES.md
src-tauri/src/db/migrations/004_ai_enabled.sql
src-tauri/src/db/migrations/MANIFEST.sha256
src/stories/Dashboard.stories.tsx
src/features/settings/categories/SessionsCategory.tsx
src/features/ai/ModelPickerContainer.tsx
tests/unit/file-export.test.ts
src/features/session/aiChip.ts
tests/unit/ai-chip-status.test.ts
tests/unit/stats-insights.test.ts
tests/unit/stats-data.test.ts
src/stories/AiStatusChip.stories.tsx
src/features/ai/index.ts
tests/unit/ai-focus-store.test.ts
src/features/ai/captureScreen.ts
src/stories/FocusInsights.stories.tsx
src/features/session/reportData.ts
src/features/session/lifecycle.ts
src-tauri/src/commands/sessions.rs
src/components/AiStatusChip.tsx
src/routes/Home.tsx
src/lib/db/sessions.ts
tests/unit/ai-models.test.ts
tests/unit/report-data.test.ts
src-tauri/src/db/migrations.rs
src/features/ai/focusStore.ts
src/features/session/reportSerialize.ts
tests/unit/report-serialize.test.ts
src-tauri/src/db/sessions.rs
src/strings.ts
src/features/session/Report.tsx
tests/unit/ai-sample-loop.test.ts
src/features/session/SessionView.tsx
src/stories/Report.stories.tsx
src-tauri/src/commands/sidecar.rs
src/features/ai/sampleLoop.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Put toast and notification copy in src/strings.ts; prefer centralized strings over inline user-facing copy.
Maintain WCAG AA contrast in both themes, do not convey information by color alone, and honor the global reduced-motion kill switch; new motion must be gated by default.
The application is local-only: never add telemetry, and never instruct users to share model files or BIP39 mnemonics with an AI service.

Files:

src/stories/Dashboard.stories.tsx
src/features/settings/categories/SessionsCategory.tsx
src/features/ai/ModelPickerContainer.tsx
src/features/session/aiChip.ts
src/stories/AiStatusChip.stories.tsx
src/features/ai/index.ts
src/features/ai/captureScreen.ts
src/stories/FocusInsights.stories.tsx
src/features/session/reportData.ts
src/features/session/lifecycle.ts
src/components/AiStatusChip.tsx
src/routes/Home.tsx
src/lib/db/sessions.ts
src/features/ai/focusStore.ts
src/features/session/reportSerialize.ts
src/strings.ts
src/features/session/Report.tsx
src/features/session/SessionView.tsx
src/stories/Report.stories.tsx
src/features/ai/sampleLoop.ts

**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Provide a Storybook story for every component.

Files:

src/stories/Dashboard.stories.tsx
src/features/settings/categories/SessionsCategory.tsx
src/features/ai/ModelPickerContainer.tsx
src/stories/AiStatusChip.stories.tsx
src/stories/FocusInsights.stories.tsx
src/components/AiStatusChip.tsx
src/routes/Home.tsx
src/features/session/Report.tsx
src/features/session/SessionView.tsx
src/stories/Report.stories.tsx

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript strict mode and ensure the code passes the TypeScript build and type checks.

Files:

src/stories/Dashboard.stories.tsx
src/features/settings/categories/SessionsCategory.tsx
src/features/ai/ModelPickerContainer.tsx
tests/unit/file-export.test.ts
src/features/session/aiChip.ts
tests/unit/ai-chip-status.test.ts
tests/unit/stats-insights.test.ts
tests/unit/stats-data.test.ts
src/stories/AiStatusChip.stories.tsx
src/features/ai/index.ts
tests/unit/ai-focus-store.test.ts
src/features/ai/captureScreen.ts
src/stories/FocusInsights.stories.tsx
src/features/session/reportData.ts
src/features/session/lifecycle.ts
src/components/AiStatusChip.tsx
src/routes/Home.tsx
src/lib/db/sessions.ts
tests/unit/ai-models.test.ts
tests/unit/report-data.test.ts
src/features/ai/focusStore.ts
src/features/session/reportSerialize.ts
tests/unit/report-serialize.test.ts
src/strings.ts
src/features/session/Report.tsx
tests/unit/ai-sample-loop.test.ts
src/features/session/SessionView.tsx
src/stories/Report.stories.tsx
src/features/ai/sampleLoop.ts

src-tauri/**/*.{rs,toml}

📄 CodeRabbit inference engine (CLAUDE.md)

Treat peer wire formats and identity derivation as cross-version contracts; coordinate changes so older peers and existing stored data remain compatible.

Files:

src-tauri/src/commands/sessions.rs
src-tauri/src/db/migrations.rs
src-tauri/src/db/sessions.rs
src-tauri/src/commands/sidecar.rs

src-tauri/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Rust changes must pass cargo test, cargo fmt --check, and cargo clippy.

Files:

src-tauri/src/commands/sessions.rs
src-tauri/src/db/migrations.rs
src-tauri/src/db/sessions.rs
src-tauri/src/commands/sidecar.rs

src/components/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Components under src/components/ must compose from ui/, design/, and shared utilities, and must not reverse-import application components into the primitive layer.

Files:

src/components/AiStatusChip.tsx

🔇 Additional comments (41)

ISSUES.md (1)

13-93: LGTM!

src/stories/Dashboard.stories.tsx (1)

48-48: LGTM!

tests/unit/ai-chip-status.test.ts (1)

1-83: LGTM!

src/stories/FocusInsights.stories.tsx (1)

45-45: LGTM!

src-tauri/src/db/migrations/004_ai_enabled.sql (1)

1-24: LGTM!

tests/unit/file-export.test.ts (1)

79-94: LGTM!

tests/unit/stats-data.test.ts (1)

36-52: LGTM!

src/features/ai/captureScreen.ts (1)

204-216: LGTM!

src/features/ai/index.ts (1)

283-312: LGTM!

tests/unit/ai-models.test.ts (1)

329-376: LGTM!

tests/unit/ai-sample-loop.test.ts (1)

18-22: LGTM!

Also applies to: 891-1046, 1600-1626

src/stories/AiStatusChip.stories.tsx (1)

16-18: LGTM!

Also applies to: 23-31

src/features/settings/categories/SessionsCategory.tsx (1)

200-209: LGTM!

tests/unit/report-data.test.ts (1)

9-9: LGTM!

Also applies to: 350-432

src-tauri/src/db/migrations.rs (1)

16-22: LGTM!

Also applies to: 115-115, 240-285, 350-353

src-tauri/src/db/migrations/MANIFEST.sha256 (1)

10-10: LGTM!

src-tauri/src/db/sessions.rs (1)

36-64: LGTM!

Also applies to: 74-91, 114-140, 232-232, 288-288, 376-376, 391-391, 419-419

src-tauri/src/commands/sessions.rs (1)

32-50: LGTM!

src/lib/db/sessions.ts (1)

29-33: LGTM!

Also applies to: 51-51, 67-67

src/features/ai/focusStore.ts (1)

176-194: LGTM!

src/features/session/lifecycle.ts (1)

312-312: LGTM!

tests/unit/ai-focus-store.test.ts (1)

18-18: LGTM!

Also applies to: 287-323

tests/unit/stats-insights.test.ts (1)

36-36: LGTM!

src-tauri/src/commands/sidecar.rs (1)

81-94: LGTM!

Also applies to: 495-509, 537-551, 658-665, 678-679, 726-755, 867-898

src/features/ai/sampleLoop.ts (4)

82-104: LGTM!

Also applies to: 140-151

360-383: LGTM!

Also applies to: 432-444, 498-533, 796-807

878-884: LGTM!

Also applies to: 914-918, 938-938, 965-965

1078-1124: LGTM! The bounded acquire wrapper correctly races against the deadline, clears its timer, and stops a late-arriving stream to avoid a leaked OS recording indicator.

Also applies to: 1210-1210, 1276-1276

src/components/AiStatusChip.tsx (1)

7-37: LGTM!

src/features/session/SessionView.tsx (3)

77-77: LGTM!

Also applies to: 188-190, 887-915

1021-1038: LGTM!

1399-1412: LGTM!

Also applies to: 1475-1485

src/routes/Home.tsx (2)

28-32: LGTM!

Also applies to: 126-141

282-302: LGTM!

Also applies to: 344-360, 370-370

src/features/session/reportData.ts (1)

38-80: LGTM! The five-state classification and its precedence (score first, for pre-003 rows) is correctly implemented and matches the referenced test cases.

src/features/session/reportSerialize.ts (1)

17-24: LGTM!

Also applies to: 74-106, 126-126, 140-142, 177-177

src/features/session/Report.tsx (1)

58-73: LGTM!

Also applies to: 284-288, 472-472, 526-526, 603-607, 622-622

src/strings.ts (1)

604-606: LGTM! New copy is correctly centralized here and keys line up exactly with the SampleLoopStallReason and AiCoverage unions used at the call sites.

Also applies to: 646-665, 776-785, 805-816, 1065-1069

Source: Coding guidelines

src/stories/Report.stories.tsx (1)

53-53: LGTM! Each new story's field combination correctly exercises the intended AiCoverage branch.

Also applies to: 202-206, 228-255, 257-279, 281-305

tests/unit/report-serialize.test.ts (1)

50-50: LGTM! Good coverage of all the coverage-state permutations, including the pre-003 (ai_enabled: null) cause-neutral case.

Also applies to: 128-203

src/features/session/aiChip.ts (1)

14-43: LGTM!

coderabbitai · 2026-07-28T08:41:53Z

  // Hydrate persistent records once on mount.
+  //
+  // I79 — retry on `'error'` too, not only `'loading'`. A LazyStore read of
+  // models.json can fail transiently (an AV scanner holding the file, a
+  // partial write from a previous run), and `status: 'error'` used to be
+  // terminal for the whole process: `activeModelId` stayed null, so no AI ran
+  // for any session until the app was restarted, and this — the one surface
+  // that could re-read it — refused to try. `hydrate()` early-returns only on
+  // `'ready'`, and this effect's deps re-run it on a status CHANGE rather than
+  // in a loop, so a persistent failure costs one attempt per visit here.
  useEffect(() => {
-    if (status === 'loading') void hydrate()
+    if (status !== 'ready') void hydrate()
  }, [status, hydrate])


🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Avoid retrying hydration from its own loading transition.

When hydrate() changes error to loading, Line 94 invokes it again because loading !== 'ready'. Since hydrate() only short-circuits at ready, a persistent failure can create overlapping reads and an error/loading retry loop, rather than one retry per picker visit. Run hydration once per mount, or use an explicit retry trigger.

Proposed fix

useEffect(() => { - if (status !== 'ready') void hydrate() -}, [status, hydrate]) + void hydrate() +}, [hydrate])

#!/usr/bin/env bash set -euo pipefail # Locate and inspect the model-store hydration state transitions. rg -n -C 6 --glob '*.{ts,tsx}' \ 'hydrate\s*[:=]|status:\s*["'\'']loading["'\'']|status:\s*["'\'']error["'\'']' src # Map candidate exports before inspecting the hydrate implementation. ast-grep outline src --items all --type function --match hydrate

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/ai/ModelPickerContainer.tsx` around lines 83 - 95, Update the hydration effect in ModelPickerContainer so hydrate() runs once per picker mount instead of re-triggering when its own status changes to loading or error. Remove status from the effect’s retry-triggering logic while retaining the existing hydrate dependency and non-ready initialization behavior, or introduce an explicit user/visit retry trigger that cannot be caused by hydrate’s state transitions.

coderabbitai · 2026-07-28T08:41:53Z

+  if (!activeModelId) {
+    return modelStatus === 'loading' ? 'off' : 'unconfigured'
+  }
+  // Camera still spinning up (or off). Reads as 'off' rather than
+  // 'unconfigured' — nothing is missing from the user's setup, and the camera
+  // tile plus MediaErrorBanner already own that story.
+  if (!hasLocalStream) return 'off'


🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Do not map enabled-but-not-ready states to off.

When AI is enabled, Lines 45 and 50 still return off during model hydration and capture startup/recovery, so the chip reports “AI off” even though the user enabled AI. Additionally, modelStatus === 'error' is misclassified as unconfigured. Use distinct non-off statuses for loading, model errors, and missing streams, and cover each case with tests.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/session/aiChip.ts` around lines 44 - 50, Update the status logic in the AI chip state function around activeModelId, modelStatus, and hasLocalStream so enabled-but-not-ready states never return “off”: return distinct statuses for model loading, model errors, and missing local streams, while reserving “unconfigured” for genuinely disabled or absent setup. Add tests covering each loading, error, and missing-stream case.

main moved 53 files while this branch was in flight, and took I79 for the macOS display-capture fix — which also touches sampleLoop.ts. Only ISSUES.md conflicted textually (main reformatted the whole table); every code file auto-merged. Resolution: take main's table wholesale and re-insert this branch's row as I83, after main's I81 and clear of PR #146's claimed I82. All 74 in-code I79 tags across 27 files renumbered with it, so the comment tags still point at a real ledger row. Migration 004's bytes changed with its comment tag, so its hash is re-pinned and re-manifested — again via the documented never-shipped escape hatch, since 004 is new in this PR. 942 vitest + 77 cargo tests pass; build, lint, tokens, strings, migrations, stories, contrast, and cargo fmt all green against the merged tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The merge from main brought prettier 3.9.5 into package-lock.json, but this working tree still had 3.8.3 installed from the pre-merge lockfile. Every `npm run format` I ran after the merge therefore rewrote files — including nine that belong to main and that this PR never meant to touch — in 3.8.3 style, and CI's Frontend job failed its Format check against 3.9.5. `npm ci` + `npm run format` restores all nine to byte-identical with main and formats the two files this PR does own. The diff against main is now exactly the 36 intended files. Worth remembering: `format:check` compares against the LOCKFILE's prettier, so a stale node_modules turns a formatting pass into a formatting regression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Resolves the two sampleLoop.ts conflicts between #98's structured logger and I83's stall tracking: keep main's log.warn call sites, keep this branch's noteUnproductiveTick() calls. The inference.aborted line now reports tickTimeoutMs (the p95-derived bound I83 introduced) — main's requestTimeoutMs binding no longer exists on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/features/ai/captureScreen.ts (1)
204-216: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle non-transient-activation InvalidStateError separately.

getDisplayMedia() can also reject with InvalidStateError for an inactive/unfocused document or a reused CaptureController. These failures should not route to screen_capture_denied’s permission-recovery overlay, which presents permission/Open-Settings guidance plus a retry for the missing gesture instead of the actual state recovery.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/ai/captureScreen.ts` around lines 204 - 216, The
InvalidStateError branch in the capture error handling must distinguish
transient-activation failures from inactive/unfocused documents or reused
CaptureController errors. Route non-transient-activation InvalidStateError cases
through the existing unavailable/state-recovery path, while preserving the
denied recovery overlay only for genuine missing-gesture failures; keep
InvalidAccessError behavior unchanged unless needed for this distinction.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ISSUES.md`:
- Line 96: Update the issue entry’s coverage description to state that
aiCoverage() has five states, including noConfident, and change the test
description from “pre-003 scored row” to “pre-004 scored row” to match migration
004’s boundary.

---

Outside diff comments:
In `@src/features/ai/captureScreen.ts`:
- Around line 204-216: The InvalidStateError branch in the capture error
handling must distinguish transient-activation failures from inactive/unfocused
documents or reused CaptureController errors. Route non-transient-activation
InvalidStateError cases through the existing unavailable/state-recovery path,
while preserving the denied recovery overlay only for genuine missing-gesture
failures; keep InvalidAccessError behavior unchanged unless needed for this
distinction.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 28f8dbc0-3db5-4613-9016-9c8a23a022ce

📥 Commits

Reviewing files that changed from the base of the PR and between bbe34c4 and f7100d3.

📒 Files selected for processing (31)

ISSUES.md
src-tauri/src/commands/sessions.rs
src-tauri/src/commands/sidecar.rs
src-tauri/src/db/migrations.rs
src-tauri/src/db/migrations/004_ai_enabled.sql
src-tauri/src/db/migrations/MANIFEST.sha256
src-tauri/src/db/sessions.rs
src/components/AiStatusChip.tsx
src/features/ai/ModelPickerContainer.tsx
src/features/ai/captureScreen.ts
src/features/ai/focusStore.ts
src/features/ai/index.ts
src/features/ai/sampleLoop.ts
src/features/session/Report.tsx
src/features/session/SessionView.tsx
src/features/session/aiChip.ts
src/features/session/lifecycle.ts
src/features/session/reportData.ts
src/features/session/reportSerialize.ts
src/features/settings/categories/SessionsCategory.tsx
src/lib/db/sessions.ts
src/routes/Home.tsx
src/stories/AiStatusChip.stories.tsx
src/stories/Report.stories.tsx
src/strings.ts
tests/unit/ai-chip-status.test.ts
tests/unit/ai-focus-store.test.ts
tests/unit/ai-models.test.ts
tests/unit/ai-sample-loop.test.ts
tests/unit/report-data.test.ts
tests/unit/report-serialize.test.ts

🚧 Files skipped from review as they are similar to previous changes (29)

src-tauri/src/db/migrations/MANIFEST.sha256
src-tauri/src/db/migrations/004_ai_enabled.sql
tests/unit/ai-chip-status.test.ts
src/features/ai/index.ts
src/components/AiStatusChip.tsx
src/features/ai/ModelPickerContainer.tsx
src/stories/AiStatusChip.stories.tsx
src-tauri/src/commands/sessions.rs
src/features/session/lifecycle.ts
tests/unit/ai-focus-store.test.ts
src/features/settings/categories/SessionsCategory.tsx
src/features/session/aiChip.ts
tests/unit/ai-models.test.ts
tests/unit/report-data.test.ts
src/routes/Home.tsx
tests/unit/report-serialize.test.ts
src/strings.ts
src-tauri/src/db/migrations.rs
src/lib/db/sessions.ts
src-tauri/src/db/sessions.rs
src/features/session/reportData.ts
src/features/ai/focusStore.ts
src/features/session/Report.tsx
src/stories/Report.stories.tsx
src/features/session/reportSerialize.ts
src/features/session/SessionView.tsx
tests/unit/ai-sample-loop.test.ts
src-tauri/src/commands/sidecar.rs
src/features/ai/sampleLoop.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (6)

GitHub Check: Installer (Windows)
GitHub Check: Installer (macOS)
GitHub Check: Analyze (rust)
GitHub Check: Rust (Windows)
GitHub Check: Frontend
GitHub Check: Rust (macOS)

🧰 Additional context used

📓 Path-based instructions (3)

**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: Do not add telemetry or break compatibility with released local data, peer wire formats, or identity derivation.
Avoid unrelated refactoring, speculative abstractions, and comments unless the reason is non-obvious.
Use one focused change per commit and conventional-commit subjects such as feat:, fix:, chore:, docs:, or ci:.

Files:

ISSUES.md
src/features/ai/captureScreen.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Put toast and notification copy in src/strings.ts; prefer centralized strings over inline user-facing copy.
Maintain WCAG AA contrast in both themes, do not convey information by color alone, and honor the global reduced-motion kill switch; new motion must be gated by default.
The application is local-only: never add telemetry, and never instruct users to share model files or BIP39 mnemonics with an AI service.

Files:

src/features/ai/captureScreen.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript strict mode and ensure the code passes the TypeScript build and type checks.

Files:

src/features/ai/captureScreen.ts

🔇 Additional comments (1)

ISSUES.md (1)

93-95: LGTM!

coderabbitai · 2026-07-28T09:52:52Z

-| I69 | Sev3 | `src-tauri/src/lib.rs`                                                                                              | The corrupt-DB recovery dialog asserted re-pairing was required and never mentioned the friends-backup import — wrong at the exact moment a friend loses their list.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 | **fixed (copy)** — the dialog now names Settings → Identity → Import friends as the restore path if a backup exists, otherwise re-pair.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      |
-| I70 | Sev4 | `.github/workflows/release.yml`                                                                                     | A half-built draft (one platform's artifact missing from `latest.json`) could be published, stranding every friend on the missing platform with no update path and a false "you're on the latest".                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   | **fixed** — a job asserts both platforms are present in the draft's `latest.json` and, on failure, stamps the draft title "INCOMPLETE, DO NOT PUBLISH" (needs `contents: write` to read a draft). Not runnable on this box; validated by YAML parse + reading.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               |
-| I71 | Sev2 | `src/features/updater/updaterStore.ts` + `src-tauri/src/commands/system.rs`                                         | Issue #77: an app opened straight from the mounted `.dmg` runs under macOS App Translocation (read-only bundle), where `update.install()`'s rename-into-place can never succeed — every launch re-downloaded the installer, offered "Restart now", and failed with the generic install toast. The one documented install step (drag to Applications) is exactly the one this path skipped, and the updater had no idea.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              | **fixed** — new `system_install_context` command (translocation via exe-path component, read-only volume via `statfs`; fail-open) consulted after a check finds an update: an unswappable bundle sets a new process-permanent `blocked` status _before_ any bytes move, and the banner + Settings → About replace the doomed Restart with move-to-Applications guidance. Verified live: dev binary on a read-only DMG against the real v1.7.0 release showed the blocked row. Windows/NSIS unaffected (always updatable).                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    |
-| I72 | Sev1 | `src-tauri/src/commands/models.rs`                                                                                  | Every model download failed at the picker's preflight with "…The model manifest may be stale." for every catalog entry. `model_head_check` populated `content_length` from `reqwest::Response::content_length()`, which is the body's size hint — an HTTP/1.1 HEAD response body is always empty (hyper decodes it as zero-length regardless of headers), so every probe reported 0 bytes and the size gate rejected all six entries. The manifest itself is current: the raw `Content-Length` (and `x-linked-etag` = pinned sha256) at every pinned revision still matches.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                          


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Correct the final-state coverage description.

This entry first says aiCoverage() has four states, but later records the addition of a fifth (noConfident). It also says “pre-003 scored row” even though migration 004 is the boundary described here. Update both references to match the final implementation.

Suggested wording changes

- gives the report four honest states + gives the report five honest states - the pre-003 scored row + the pre-004 scored row

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

| I72 | Sev1 | `src-tauri/src/commands/models.rs` | Every model download failed at the picker's preflight with "…The model manifest may be stale." for every catalog entry. `model_head_check` populated `content_length` from `reqwest::Response::content_length()`, which is the body's size hint — an HTTP/1.1 HEAD response body is always empty (hyper decodes it as zero-length regardless of headers), so every probe reported 0 bytes and the size gate rejected all six entries. The manifest itself is current: the raw `Content-Length` (and `x-linked-etag` = pinned sha256) at every pinned revision still matches.

| I83 | Sev1 | `src/features/ai/modelStore.ts` + `src/routes/Home.tsx` + `src/features/session/SessionView.tsx` + `sampleLoop.ts` + `Report.tsx` | Issue `#92`: a real 10-minute two-person session on Windows rendered a report with `Focused-time —`, "No focus score was recorded for this session.", zero `ai_*` timeline rows — and, directly beside all that, "No distractions detected. Nice work." **Root cause: `useModelStore` is never hydrated outside Settings → AI.** `hydrate()` had exactly one caller, `ModelPickerContainer`'s mount effect (`ModelPickerContainer.tsx:85`), and that component mounts only inside the Settings → AI pane. `useSettingsStore` is hydrated at boot by `ThemeProvider` (`src/design/theme.tsx:52`), so `aiFeaturesEnabled` was correctly `true` while `activeModelId` sat at its `null` initial value — and `activeModelId` gates everything: `SessionView.tsx`'s sample-loop effect returns early on `if (!activeModelId)`, so `startSampleLoop` is never called and its `onStartFail('no_active_model')` toast — the one surface that names this — can never fire; `Home.tsx`'s `handleTopicSubmit` skips the V2-P9 gesture-context `preacquireScreenStream()` on the same condition, which on WebView2 is separately fatal. So any launch where the user didn't happen to open Settings → AI ran a whole session with AI silently dead: no loop, no toast, no audit row, no log line, and an unscored `sessions` row. Cross-platform and present at HEAD — it also explains `#94` ("Ai does not work on macos when its enabled"). The report then made the silence permanent: `score`/`focused_pct`/`confident_samples`/`skipped_samples` all read NULL for an AI-off session, an AI-on-but-dead session, AND a pre-004 row, so no surface could tell a deliberate choice from a malfunction, and the distractions empty state asserted a clean measurement that never happened. Five further silent-death paths found alongside it: a sidecar that spawns but never reports healthy, an HTTP error from the sidecar, a per-tick abort, and any other tick throw were each `console.warn`-only (no devtools in release builds); the live 90 s per-tick timeout was 3.3× tighter than benchmark.ts's 300 s bound, so a model could benchmark successfully — the only thing that sets `activeModelId` — and then abort every live inference forever; an unanswered screen-share picker wedged `boot()` with no timeout, and `stop()` awaits `bootPromise`, so the sidecar was never killed; the Rejoin path and the camera/mic "Try again" path both re-`boot()` with no gesture pre-acquire; `mapDisplayMediaError` had no `InvalidStateError`/`InvalidAccessError` case, so a missing-transient-activation refusal was filed as `unavailable` (a dead-end toast) instead of reaching the recovery overlay whose retry button IS a gesture; `resolve_runtime_dir`'s `_ => Ok(None)` still degraded to a spawn with no CWD and no PATH prepend — the exact lethal-on-Windows state I75 fixed; and a child that dies in the Windows loader spawns Ok, so it crash-loops to the restart budget without ever reaching the VC++-redist hint. | **fixed** — (1) hydrate `useModelStore` in `Home.tsx`'s boot effect, so the persisted model is the truth from launch rather than from a Settings visit; (2) `handleTopicSubmit` + `handleRejoin` + `handleMediaRetry` all pre-acquire the screen stream inside their real user gesture, and a store still mid-hydration counts as "maybe active" (an unconsumed stream is discarded on unmount; a missed pre-acquire is fatal on WebView2); (3) a once-per-session toast when AI is on, the model store is `ready`, and no model is active — the gap where `onStartFail` could never fire; (4) `onStalled` fires once per loop lifetime after `STALL_TICKS` (3) consecutive unproductive ticks, with a distinct reason per cause (`engine_unavailable` / `engine_error` / `inference_timeout` / `unknown`) and actionable copy; paused states (break, camera off, pomodoro rest, battery) are deliberately not stalls; (5) the per-tick timeout is derived from the model's benchmarked p95 (`effectiveRequestTimeoutMs`: 3× p95, floored at 90 s, capped at benchmark.ts's 300 s); (6) `SCREEN_ACQUIRE_TIMEOUT_MS` (120 s) bounds the acquire so an unanswered picker becomes a visible retryable error instead of a permanent wedge, and a late-arriving stream is stopped rather than leaked; (7) `InvalidStateError` / `InvalidAccessError` → `screen_capture_denied`, routing to the overlay whose retry is itself the missing gesture; (8) migration **004** adds `sessions.ai_enabled` (1/0, NULL = pre-004), written from live settings at teardown, and the new `aiCoverage()` derivation gives the report five honest states — `ran` keeps the earned "Nice work", `noChecks` names the malfunction and points at Settings → AI, `off` says AI was off, `unknown` stays cause-neutral for pre-004 rows — shared by the rendered report and the text export so a pasted copy can never disagree; (9) Rust: `resolve_runtime_dir` falls back to the binary's own directory (one of the two places ggml globs anyway) instead of `None`, and the crash-loop give-up path now carries `append_windows_dll_hint`. Tests: `aiCoverage` (6 cases incl. the pre-004 scored row and the NULL-is-not-0 rule), serializer honesty (4), `snapshotFocusForReport.aiEnabled` (3), stall notice (4 incl. streak-reset and camera-off-is-not-a-stall), `effectiveRequestTimeoutMs` boundaries (4), and a Rust 003→004 upgrade test asserting old rows read NULL. Stories: `AiOnButNoChecks`, `AiOffForSession`. **Round 2** (a 65-agent adversarial sweep over the first draft found six more): (10) `hydrate()`'s `status: 'error'` was terminal — `ModelPickerContainer` only retried on `'loading'`, so one failed models.json read (AV lock, partial write) killed AI for the whole process and reopened this very issue through a narrower door; the gate is now `status !== 'ready'` and the session notice distinguishes it (`modelListUnreadable`). (11) the footer chip read **"AI off" while AI was ON** with no model — the single on-screen signal during `#92`, pointing at exactly the wrong setting; new `'unconfigured'` status via a pure, unit-tested `deriveAiChipStatus()`, with `'loading'` deliberately reading `'off'` so no launch flashes it. (12) `aiCoverage`'s first cut returned `'ran'` for `confident_samples: 0, skipped_samples: k`, defended on the grounds that the `#47` D5 line caveats it — it does not below `SKIPPED_SAMPLES_MIN` (3), so k of 1–2 rendered a fabricated all-clear with no caveat at all; fifth state `'noConfident'` added and the two tests asserting the old behavior **edited**, not appended. (13) `append_windows_dll_hint` probed only `vcruntime140.dll`, staying silent on a box with the C runtime but not the C++ one; now requires both. (14) `next_attempts` resets the streak on any child clearing `MIN_HEALTHY_UPTIME` (120 s), so a sidecar dying every ~2.5 min crash-looped **forever** without ever setting `errored` — the stall notice fired once and the session then ran for an hour on a dying engine; `TOTAL_RESTART_BUDGET` (12 per generation) closes it, sized so an 8-hour session dying hourly never trips while a 121 s cycle trips at ~24 min. (15) Settings → Sessions now marks an unmeasured row `not measured` when `ai_enabled === 1`. One round-2 finding was **rejected**: "`onSidecarErrored` re-arms every tick, so a flapping sidecar re-toasts forever" — `errored` is cleared only by `sidecar_start`/`sidecar_stop` (`sidecar.rs:233`/`:270`) and the watcher `return`s after setting it, so errored→running requires deliberate user action and re-notifying then is correct, as the existing test documents. |

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ISSUES.md` at line 96, Update the issue entry’s coverage description to state that aiCoverage() has five states, including noConfident, and change the test description from “pre-003 scored row” to “pre-004 scored row” to match migration 004’s boundary.

Round 2 added a fifth coverage state (noConfident) and grew the aiCoverage/serializer suites, but the ledger entry still described the first draft's four states and 6/4 case counts. Keeps 'pre-003 scored row' as-is: that case is about migration 003's sample counters, not 004's ai_enabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

#147 (I83) and this branch (I82) independently detect "AI stopped producing readings" by different mechanisms, and they cannot both ship: they share the `stallReported` latch, so a naive resolution silently suppressed I82's audit row and re-armed I83's once-per-lifetime toast. I82's wall-clock watchdog survives; I83's consecutive-tick counter is deleted, for three reasons verified against the code: 1. Cold-start false positive, live on main. boot() schedules the first tick immediately and its own comment says the first few "gracefully skip" while llama-server loads the model — sidecar.rs starts with healthy:false and only an async /health poll flips it. I83 counted those skips, so at the 5 s fallback cadence a perfectly healthy CPU-only Windows session toasted "The AI engine isn't responding" and pinned the chip to 'AI error' ~15-30 s in, with no resume path. 2. Worst-case latency. I83 itself raised the per-tick bound to clamp(3 x p95, 90 s, 300 s), so three consecutive timeouts is 270-900 s before the user hears anything, against I82's <=135 s. 3. Coverage. I83 never reported camera_missing, screen_lost or capture_failing at all. Kept from I83 where it was better: the structured log call sites from #98 (console.warn is now an ESLint error), the p95-derived tick timeout, the "vision files may be incomplete" copy (now aiNoReading.inference_failed), and the four report stories. Dropped strings.session.errors.aiStalled, which auto-merged into a silent duplicate of aiNoReading. Two fixes on top of a straight resolution: - The chip now reads 'error' for engine-side blockers and 'paused' only for the input-absent ones, matching onStartFail/onSidecarErrored. A flat 'paused' made the chip *less* alarming as an outage got longer. - console.error in the ai_stalled append path went through the logger; it was outside the conflict and would have failed no-console. Tests: 1011 pass. I83's 4 stall tests are deleted (3 redundant with I82's 12); its intermittent-failure property is ported onto the watchdog, where it is strictly harder to satisfy. The full-timeout test now pins requestTimeoutMs explicitly instead of relying on effectiveRequestTimeoutMs(0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

scotej and others added 4 commits July 27, 2026 21:33

Copilot AI review requested due to automatic review settings July 28, 2026 08:25

Copilot AI reviewed Jul 28, 2026

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

scotej deployed to preview-macos July 28, 2026 08:47 — with GitHub Actions View deployment

scotej had a problem deploying to preview-windows July 28, 2026 08:47 — with GitHub Actions Error

scotej deployed to preview-windows July 28, 2026 08:51 — with GitHub Actions View deployment

scotej deployed to preview-macos July 28, 2026 08:51 — with GitHub Actions View deployment

scotej deployed to preview-macos July 28, 2026 09:49 — with GitHub Actions View deployment

scotej deployed to preview-windows July 28, 2026 09:49 — with GitHub Actions View deployment

coderabbitai Bot reviewed Jul 28, 2026

View reviewed changes

scotej deployed to preview-windows July 28, 2026 09:57 — with GitHub Actions View deployment

scotej deployed to preview-macos July 28, 2026 09:57 — with GitHub Actions View deployment

scotej merged commit bb354dd into main Jul 28, 2026
17 checks passed

scotej mentioned this pull request Jul 28, 2026

chore(deps): migrate to trystero 0.25 and settle 0.24↔0.25 peer interop #149

Merged

scotej mentioned this pull request Jul 28, 2026

fix(ai): say when on-device AI can't take a reading (I82) #146

Merged

10 tasks

This was referenced Jul 28, 2026

fix(db): repair the sessions schema that broke history and stats, and add a study heatmap (#99) #152

Merged

feat(ai): make benchmarking optional #163

Merged

fix(ai): benchmark uncached vision requests #172

Merged

coderabbitai Bot mentioned this pull request Aug 8, 2026

fix(ai): wait for llama-server readiness #202

Merged

19 tasks

Navigation Menu

fix(ai): start the sample loop on a fresh launch, and let the report say when AI didn't run - #147

fix(ai): start the sample loop on a fresh launch, and let the report say when AI didn't run#147
scotej merged 8 commits into
mainfrom
fix/ai-focus-never-runs-i79

scotej commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 28, 2026

Uh oh!

coderabbitai Bot Jul 28, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

scotej commented Jul 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The report in #92 was telling the truth. Nothing had measured anything.

What changed

Round 2: what an adversarial sweep found in the first draft

Testing

Manual testing: none

Deliberately not fixed

Summary by CodeRabbit

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (2 warnings)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scotej commented Jul 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 28, 2026 •

edited

Loading