Skip to content

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

Merged
scotej merged 3 commits into
mainfrom
agent/fix-ai-dialog-timeout-197
Aug 8, 2026
Merged

fix(ai): wait for llama-server readiness#202
scotej merged 3 commits into
mainfrom
agent/fix-ai-dialog-timeout-197

Conversation

@scotej

@scotej scotej commented Aug 6, 2026

Copy link
Copy Markdown
Owner

What changed and why

All user-triggered text inference now waits for llama-server to become ready before submitting a completion:

  • Ctrl+] and the in-session AI chat share one abort-aware readiness resolver.
  • The resolver polls authoritative Rust sidecar_status plus /health, waits through initial install/spawn and model loading, follows automatic respawns to a replacement port, and fails promptly on a terminal error or explicit stop.
  • Rust now exposes an internal starting lifecycle bit and uses a start-epoch token so an explicit stop during a slow install cannot later resurrect llama-server. Matching JS generation guards prevent stale start continuations from overwriting newer state.
  • Readiness retains the benchmark's existing 90-second cold-start budget and 500 ms cadence. The separate 60-second completion budget starts only after readiness and now covers response headers and body parsing.
  • Health/status/body waits are logically bounded even if an injected promise ignores AbortSignal; session-chat caller cancellation remains independent.
  • Privacy-safe ai.agent.readiness.ready and ai.agent.request.succeeded telemetry records elapsed time, attempt count, and intent without prompt or response text.

This fixes a concrete startup-readiness race: Rust can report a spawned child and port before model/projector loading completes, while pinned llama-server b9095 returns 503 from /health during that window. The reporter's logs show the Ctrl+] dialog opening during model load and one pre-load completion 503, which strongly supports this cause. The old StudyVis logs lack request outcome telemetry, so they still do not demonstrate that the request was Ctrl+] or prove a fixed run on the affected Windows 11 CPU-only setup.

Pinned llama.cpp b9095 automatically selects four slots with unified KV caching and enables continuous batching by default. This PR leaves those server defaults intact and does not add CLI flags that would override inherited LLAMA_ARG_* settings.

The focus-detection failures reported against v1.10.0 are already addressed by #172, released in v1.10.1. This PR does not duplicate that fix.

Related to #197. Keep #197 open until the reporter's Ctrl+] failure has been reproduced and verified on the affected Windows CPU-only configuration.

Regression coverage

  • retries readiness through connection refusal, 503, then 200 without posting early;
  • waits through initial install/spawn before a port exists and follows crash recovery from an old port to the healthy replacement port;
  • distinguishes crash restart, terminal error, and explicit stop states;
  • applies the same readiness gate to session chat while preserving caller cancellation;
  • gives completion a fresh 60-second budget after a 65-second readiness wait;
  • hard-bounds hung Rust status, health, completion-header, and completion-body promises at their exact deadlines, including non-cooperative mocks;
  • keeps a mocked text completion independent from an unresolved vision completion without presenting that unit test as server-concurrency proof;
  • preserves lifecycle state across start/stop races and tests Rust starting-counter/start-epoch bookkeeping;
  • verifies timers and listeners are cleared.

Manual Windows test

This PR remains a draft until the affected Windows 11 x64 CPU-only smoke test is completed.

  • Download the Windows x64 preview installer produced from current head 1195989 after its checks finish.
  • Fully quit StudyVis and llama-server.exe, reboot to make model loading cold, and do not rerun the benchmark first.
  • Start a session with the reporter's Gemma 3 4B Q4_K_M model plus f16 projector.
  • As soon as llama-server starts loading, press Ctrl+], submit a simple prompt, and verify the dialog waits through loading and renders a non-error reply.
  • Preserve a screen recording plus studyvis.log and llama-server.log; confirm ai.agent.readiness.ready precedes ai.agent.request.succeeded.
  • Theme and reduced-motion checks — not applicable; no visual or motion change.

Compatibility surfaces

No SQLite, peer wire format, identity derivation, persisted setting, updater manifest, release artifact, or benchmark fingerprint changes. SidecarStatus.starting is an internal lockstep Tauri IPC addition. Sidecar spawn arguments remain unchanged from main, including support for inherited LLAMA_ARG_* configuration; no throughput-sensitive runtime flag changed.

Local gates on current head

  • npm run build
  • npm run lint
  • npm run format:check
  • npm test — 94 files / 1,189 tests
  • targeted readiness/lifecycle tests — 3 files / 63 tests
  • check-tokens, check-strings, check-contrast, check-migrations, and check-stories
  • Storybook build and accessibility audit — 74 suites / 327 tests
  • cargo fmt --check, cargo check, correctness clippy, and cargo test --locked — 103 unit + 4 crypto-vector + 13 round-trip tests
  • cargo deny check
  • git diff --check
  • GitHub checks for current head 1195989

The non-blocking live relay probe previously passed 10/11 endpoints twice; relay.nostr.place rejected the test event because it requires proof of work. That external relay policy is unrelated to this AI change.

Merge style

  • Squash
  • Merge commit

Summary by CodeRabbit

  • New Features

    • Added clear sidecar startup status indicators.
    • Added health checks with retries, timeouts, and cancellation support.
    • Improved AI request timeout handling and error reporting.
    • Added safeguards for concurrent starts, stops, crashes, and reinstalls.
  • Bug Fixes

    • Prevented outdated startup operations from overriding newer sidecar states.
    • Improved recovery when the sidecar starts without an available port or becomes unavailable.
  • Tests

    • Expanded coverage for readiness, cancellation, timeouts, crash recovery, and concurrent operations.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2e596c98-52b7-4409-aa55-2b22ae96dc5b

📥 Commits

Reviewing files that changed from the base of the PR and between 2050ff6 and 1195989.

📒 Files selected for processing (8)
  • src-tauri/src/commands/sidecar.rs
  • src/features/ai/aiAgent.ts
  • src/features/ai/sidecar.ts
  • src/features/session/sessionChatAi.ts
  • src/stories/SessionNotesPanel.stories.tsx
  • tests/unit/ai-agent.test.ts
  • tests/unit/ai-sidecar.test.ts
  • tests/unit/session-chat-ai.test.ts

📝 Walkthrough

Walkthrough

The PR adds cancellable sidecar startup tracking, exposes startup status, and replaces direct port lookup with abort-aware health polling. AI requests now use separate readiness and completion timeouts, with expanded lifecycle, retry, cancellation, concurrency, and cleanup tests.

Changes

Sidecar lifecycle and readiness flow

Layer / File(s) Summary
Cancellable sidecar lifecycle
src-tauri/src/commands/sidecar.rs, src/features/ai/sidecar.ts, tests/unit/ai-sidecar.test.ts
Sidecar startup tracks active attempts and epochs or generations. Stop and teardown operations invalidate pending starts. Status now reports starting, and stale completions cannot overwrite newer state.
Status-based readiness contract
src/features/ai/aiAgent.ts, src/features/ai/sidecar.ts, src/features/ai/index.ts, src/features/ai/benchmark.ts, src/features/session/sessionChatAi.ts
AI runtime access returns full sidecar status. Readiness polling validates ports, retries /health, supports abort signals, and uses shared timeout and retry constants.
Completion integration and timeout handling
src/features/ai/aiAgent.ts, src/features/session/sessionChatAi.ts, src/stories/SessionNotesPanel.stories.tsx, tests/unit/ai-agent.test.ts, tests/unit/session-chat-ai.test.ts
Chat flows wait for readiness before completion. Header and response-body parsing use bounded timeouts. Tests cover retries, crash recovery, cancellation, stalled responses, concurrent requests, and timer cleanup.

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

Sequence Diagram(s)

sequenceDiagram
  participant Chat
  participant AiAgent
  participant SidecarStatus
  participant SidecarHealth
  participant CompletionEndpoint
  Chat->>AiAgent: Submit chat request
  AiAgent->>SidecarStatus: Get sidecar status
  SidecarStatus-->>AiAgent: Starting or running status
  AiAgent->>SidecarHealth: Poll /health
  SidecarHealth-->>AiAgent: Healthy response
  AiAgent->>CompletionEndpoint: Submit completion
  CompletionEndpoint-->>AiAgent: Headers and response body
  AiAgent-->>Chat: Parsed reply or classified error
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address the linked issue's Ctrl+] startup-readiness failure and document that focus detection was fixed separately.
Out of Scope Changes check ✅ Passed The sidecar flags, readiness polling, shared constants, and regression tests all support the stated objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely describes the main change: waiting for llama-server readiness before AI requests.
Description check ✅ Passed The description covers the change, rationale, regression coverage, manual test status, compatibility surfaces, gates, and merge style.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch agent/fix-ai-dialog-timeout-197

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.

@scotej
scotej force-pushed the agent/fix-ai-dialog-timeout-197 branch from 056bd13 to 2050ff6 Compare August 8, 2026 06:21
@scotej scotej changed the title fix(ai): let dialog wait behind vision inference fix(ai): wait for llama-server readiness Aug 8, 2026
@scotej
scotej marked this pull request as ready for review August 8, 2026 06:39
Copilot AI lite review requested due to automatic review settings August 8, 2026 06:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@scotej
scotej marked this pull request as ready for review August 8, 2026 09:58
@scotej
scotej merged commit 71bdb98 into main Aug 8, 2026
20 of 21 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Aug 9, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants