fix(ai): downgrade LLM cache write failure to once-per-run warn (APP-4923)#14111
fix(ai): downgrade LLM cache write failure to once-per-run warn (APP-4923)#14111warp-dev-github-integration[bot] wants to merge 3 commits into
Conversation
Commit the approved-spec-pending draft for APP-4923: the LLM cache write failure in LLMPreferences::on_server_update is non-critical (in-memory models are already updated) and noisy in Sentry (~395K events cross-platform). Spec proposes downgrading only the persistence-write failure path from report_error! to a once-per-run log::warn! with OS-error/backend diagnostics, leaving the separate serialization-failure report_error! untouched, plus a fail-before/pass-after regression test. Co-Authored-By: Oz <oz-agent@warp.dev>
The approved spec for APP-4923 was reviewed and approved in the originating Slack thread. Per the approval instruction, remove the committed spec markdown from the PR branch before applying spec-done; implementation will add the code onto this same PR/branch.
warp-dev-github-integration
Bot
changed the title
|
@warp-dev-github-integration[bot] I ran into an unexpected error while working on this. Powered by Oz |
cc23ff2 to
3e981ed
Compare
![warp-dev-github-integration[bot]](https://avatars.githubusercontent.com/in/2196476?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
The implementation is narrowly scoped to the non-critical LLM cache write path and preserves the serialization-error reporting path. However, the PR is not ready to accept because the ticket's acceptance criteria require a focused failing-write regression test, and the required validation gates are not currently green: six required CI jobs remain pending, while local ./script/presubmit cannot reach clippy because Cargo is denied permission to create /usr/local/cargo/git/db/core-foundation-rs-efc549ee4b022354.
Concerns
The acceptance criteria and Testing section require an injected/fake UserPreferences backend whose write_value returns an I/O error, with assertions that models_by_feature is updated and UpdatedAvailableLLMs is emitted. No regression test was added, so the central resilience behavior and the changed error path are unverified. Add the focused test in the existing LLM test module (and any platform-specific backend coverage available), then rerun ./script/presubmit and wait for all required PR checks to complete successfully.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
| static WARNED: std::sync::atomic::AtomicBool = | ||
| std::sync::atomic::AtomicBool::new(false); | ||
| if !WARNED.swap(true, std::sync::atomic::Ordering::Relaxed) { | ||
| log::warn!("LLM cache ({backend}, {byte_len} bytes): {e:#}"); |
There was a problem hiding this comment.
Cache writes for the LLM model list are non-critical — the in-memory models_by_feature update already happened and the app continues to work. The previous report_error! call was generating ~395K Sentry events for 1,057 users since Feb 2025 (cross-platform: Linux + Windows). Downgrade to log::warn! so failures surface in local logs without creating Sentry events. Co-Authored-By: Oz <oz-agent@warp.dev>
3e981ed to
dab6a0a
Compare
1 participant
Fixes Sentry issue WARP-CLIENT-BETA-STABLE-NBF — "Failed to cache LLMs: i/o error" — 395K events, 1,057 users, cross-platform (Linux + Windows).
In
LLMPreferences::on_server_update, thereport_error!(e)call for a failing LLM model-list cache write is changed tolog::warn!("{e:#}"). Cache writes are non-critical (the in-memory update already happened); this stops the Sentry noise without losing local log visibility.APP-4923 · Sentry
This PR was generated with Oz.