diff --git a/internal/llm/checkpoint.go b/internal/llm/checkpoint.go index 714c16e..a769e4d 100644 --- a/internal/llm/checkpoint.go +++ b/internal/llm/checkpoint.go @@ -127,17 +127,33 @@ func NewRateLimiter(interval time.Duration) *RateLimiter { } // Allow reports whether a checkpoint may run for docID now. When allowed it -// records the time and returns (true, 0); when throttled it returns -// (false, retryAfter) where retryAfter is the wait until the next allowed run. -func (rl *RateLimiter) Allow(docID string) (bool, time.Duration) { +// records the time and returns (true, 0, recordedAt); when throttled it returns +// (false, retryAfter, zero) where retryAfter is the wait until the next allowed +// run. recordedAt lets a caller whose pass fails hand the exact timestamp to +// Release so it rolls back only its own slot. +func (rl *RateLimiter) Allow(docID string) (bool, time.Duration, time.Time) { rl.mu.Lock() defer rl.mu.Unlock() now := time.Now() if last, ok := rl.last[docID]; ok { if elapsed := now.Sub(last); elapsed < rl.interval { - return false, rl.interval - elapsed + return false, rl.interval - elapsed, time.Time{} } } rl.last[docID] = now - return true, 0 + return true, 0, now +} + +// Release rolls back the slot a prior Allow recorded for docID, but only if no +// newer Allow has since claimed it. Called when an LLM pass fails: Allow runs +// before the model call, so without this a failed checkpoint would hold the +// per-document slot for the full interval and a retry (or the frontend's +// auto-retry) would hit the throttle path and get the stale/empty set back +// instead of re-running. `at` is the time the failed Allow returned. +func (rl *RateLimiter) Release(docID string, at time.Time) { + rl.mu.Lock() + defer rl.mu.Unlock() + if last, ok := rl.last[docID]; ok && last.Equal(at) { + delete(rl.last, docID) + } } diff --git a/internal/llm/checkpoint_test.go b/internal/llm/checkpoint_test.go index e82283e..b3c8272 100644 --- a/internal/llm/checkpoint_test.go +++ b/internal/llm/checkpoint_test.go @@ -65,16 +65,31 @@ func TestParseCheckpoint(t *testing.T) { func TestRateLimiter(t *testing.T) { rl := NewRateLimiter(30 * time.Second) - if ok, _ := rl.Allow("doc1"); !ok { + ok, _, at := rl.Allow("doc1") + if !ok { t.Fatal("first call should be allowed") } - if ok, retry := rl.Allow("doc1"); ok || retry <= 0 { + if ok, retry, _ := rl.Allow("doc1"); ok || retry <= 0 { t.Fatalf("immediate second call should be throttled, got ok=%v retry=%v", ok, retry) } // A different document is independent. - if ok, _ := rl.Allow("doc2"); !ok { + if ok, _, _ := rl.Allow("doc2"); !ok { t.Fatal("different doc should be allowed") } + + // Releasing doc1's slot (as a failed pass does) lets the next check re-run + // immediately instead of waiting out the interval. + rl.Release("doc1", at) + if ok, _, _ := rl.Allow("doc1"); !ok { + t.Fatal("after Release the next call should be allowed again") + } + // Release only rolls back its own slot: a stale timestamp must not evict the + // slot a newer Allow holds (so a late failure can't cancel a fresh in-flight + // check). doc2 still holds its slot from above; a zero-time Release is a no-op. + rl.Release("doc2", time.Time{}) + if ok, _, _ := rl.Allow("doc2"); ok { + t.Fatal("stale Release must not free doc2's current slot") + } } func TestTruncateDoc(t *testing.T) { diff --git a/internal/suggestions/handlers.go b/internal/suggestions/handlers.go index 8aa1218..5806214 100644 --- a/internal/suggestions/handlers.go +++ b/internal/suggestions/handlers.go @@ -102,7 +102,8 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R return } - if ok, _ := limiter.Allow(docID); !ok { + ok, _, slotAt := limiter.Allow(docID) + if !ok { // Throttled: return the existing pending set unchanged rather than an // error, so the frontend keeps showing current suggestions. existing, err := h.fetchPending(docID) @@ -116,6 +117,10 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R raw, err := run(r.Context(), h.Client, contentText, tone) if err != nil { + // Allow ran before the model call, so a failed pass would otherwise hold + // the per-document slot for the full interval — stranding the frontend's + // auto-retry on the throttle path. Release it so a retry can re-run. + limiter.Release(docID, slotAt) errorJSON(w, http.StatusBadGateway, "llm pass failed: "+err.Error()) return } diff --git a/web/src/hooks/useCheckpoint.ts b/web/src/hooks/useCheckpoint.ts index 186a6db..d9a0750 100644 --- a/web/src/hooks/useCheckpoint.ts +++ b/web/src/hooks/useCheckpoint.ts @@ -20,17 +20,29 @@ export function useCheckpoint(docId: string | null) { const [llmDown, setLlmDown] = useState(false) const debounceRef = useRef>(undefined) + // Pending auto-retry timer for a failed checkpoint (see runCheck). + const retryRef = useRef>(undefined) const docIdRef = useRef(docId) docIdRef.current = docId // Token to discard responses from a doc we've since navigated away from. const runRef = useRef(0) - const runCheck = useCallback(async () => { + // Backoff schedule for re-running a checkpoint that failed. A paste fires + // exactly ONE checkpoint, and nothing re-fires until the next keystroke — so a + // single transient failure (a momentary stall on the shared inference box) + // would otherwise strand the user on "resting" indefinitely after a paste. + // These delays clear the server's 30s per-doc floor by the later attempts, and + // a failed pass now releases its slot server-side so a retry can truly re-run. + const RETRY_DELAYS_MS = [3000, 12000, 35000] + + const runCheck = useCallback(async (attempt = 0) => { const id = docIdRef.current if (!id) return + clearTimeout(retryRef.current) const run = ++runRef.current setChecking(true) + let retrying = false try { const fresh = await api.checkDoc(id) if (run === runRef.current && id === docIdRef.current) { @@ -39,9 +51,18 @@ export function useCheckpoint(docId: string | null) { } } catch (err) { console.error('checkpoint failed', err) - if (run === runRef.current) setLlmDown(true) + if (run !== runRef.current) return + if (attempt < RETRY_DELAYS_MS.length) { + // Keep trying quietly — don't flag "resting" until retries are exhausted. + retrying = true + retryRef.current = setTimeout(() => void runCheck(attempt + 1), RETRY_DELAYS_MS[attempt]) + } else { + setLlmDown(true) + } } finally { - if (run === runRef.current) setChecking(false) + // Stay in the "checking" state while a retry is queued so the breathing dot + // keeps reassuring rather than flickering off between attempts. + if (run === runRef.current && !retrying) setChecking(false) } }, []) @@ -51,6 +72,7 @@ export function useCheckpoint(docId: string | null) { const runVoice = useCallback(async () => { const id = docIdRef.current if (!id) return + clearTimeout(retryRef.current) // a voice pass supersedes a queued grammar retry const run = ++runRef.current setVoicing(true) try { @@ -70,13 +92,15 @@ export function useCheckpoint(docId: string | null) { // Call on every edit; schedules a check 4s after typing settles. const schedule = useCallback(() => { clearTimeout(debounceRef.current) - debounceRef.current = setTimeout(runCheck, DEBOUNCE_MS) + clearTimeout(retryRef.current) // a fresh edit supersedes any queued retry + debounceRef.current = setTimeout(() => void runCheck(), DEBOUNCE_MS) }, [runCheck]) // Load existing pending suggestions whenever the document changes, and cancel // any in-flight debounce from the previous doc. useEffect(() => { clearTimeout(debounceRef.current) + clearTimeout(retryRef.current) runRef.current++ setSuggestions([]) setChecking(false) @@ -97,7 +121,13 @@ export function useCheckpoint(docId: string | null) { } }, [docId]) - useEffect(() => () => clearTimeout(debounceRef.current), []) + useEffect( + () => () => { + clearTimeout(debounceRef.current) + clearTimeout(retryRef.current) + }, + [], + ) // Drop one suggestion locally (after accept/dismiss) without a refetch. const removeSuggestion = useCallback((id: string) => {