fix(ai): hold stale prompts on a cooldown after a failed refetch - #4775
fix(ai): hold stale prompts on a cooldown after a failed refetch#4775posthog[bot] wants to merge 3 commits into
Conversation
A failed refetch fell back to the stale cache entry but left its fetchedAt timestamp untouched. The entry stayed expired, so every later prompts.get() call went back to the network. A rate-limited client turned a cached read into a per-call request and held itself against the limit. Set a cooldown on the entry when a refetch fails, and serve the stale entry until it expires. On a 429 the cooldown follows the server's Retry-After header, clamped to an hour. Generated-By: PostHog Desktop Task-Id: 00a804cf-4efc-4168-afce-b8b2f1ebff63
|
The The failure is 8 cases in Cause looks like an ordering problem between two commits already on
#4732 was written against Jest and landed after the migration without a rebase, so its I reproduced it on |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
2 participants
Problem
getInternalfalls back to the stale cache entry but never touches itsfetchedAt. The entry stays expired, so the nextprompts.get()treats it as stale and goes back to the network.Changes
429, the cooldown length follows the server'sRetry-Afterheader, clamped to one hour. Any other failure uses a 60 second default, which is one refill of the per-minute bucket.source: 'stale_cache', so callers that already branch onsourcesee no change. The warning is still logged once, by the call that actually failed.PromptFetchErrorcarriesRetry-Afterout offetchPromptFromApi, and the three stale/fresh return paths share onereadCacheEntryhelper.Before / after
Ten
get()calls on one throttled prompt, against the built package:stale_cache×10stale_cache×10The prompt text served to the caller is identical in both.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset fileTest strategy
Unit — three cases added to
packages/ai/tests/prompts.test.ts, over the existing fake-timer setup:429cooldown that followsRetry-Afterpast the 60 second defaultTwo of the three fail on
mainand pass here. The third passes onmainby construction — it guards the fix against over-caching, which is the risk the fix introduces.packages/aisuite: 883 passed, 1 skipped.turbo build lint --filter=@posthog/ai: clean.End-to-end — a script against the built
dist/index.mjswith a stubbedfetchreturning429with a realHeadersobject. It produced the before/after table above and confirmedretryAfterSeconds: 30was read off the header.Edge cases considered — a
Retry-Afterthat is absent, non-numeric, zero, or negative falls back to the default; an oversized one is clamped to an hour so a bad header cannot pin the cache for a day. A successful refetch replaces the whole entry, which clears the cooldown. A cold cache with no entry is unchanged: there is nothing to serve, so it still fetches and falls through tofallback.Note
Two things named in the report are deliberately left alone. The cache key is
version ?? label, so the same prompt fetched with and without a label is two entries and two requests — that is two different server queries, not a bug, and merging them would change what a labeled fetch returns. Batch fetching is #4690. A throttled request also emits no$llm_prompt_fetchedevent, which leaves this load invisible in a team's own usage logs; that is server-side, inposthog/posthog, and needs its own change.🤖 Agent context
Autonomy: Fully autonomous
fetchedAt. I rejected that: it would make the entry claim an age it does not have, so a later read would reportsource: 'cache'for data that was never revalidated, and the TTL would silently become the backoff. A separateretryNotBeforefield keepsfetchedAthonest and keeps the resultstale_cache.fallbackmeans, so I left it and kept the diff on the described symptom.Retry-Afterparser handles only the delta-seconds form. The HTTP-date form is legal but DRF does not send it, and accepting both would add a branch no caller reaches.Created with PostHog Desktop from this inbox report.