Checkpoint: auto-retry after a failed pass; release rate-limit slot on failure
A paste fires exactly one grammar checkpoint, and a failed one never retried until the next keystroke — stranding the writer on "Petal's helper is resting" after a paste. Long docs make it worse: their 15-25s checks have a wide window to catch a transient 502 from the shared Ollama (co-tenant apps load other models and evict the 9B). A failed pass also burned the per-document rate-limit slot, so a retry within 30s hit the throttle path and got an empty set back. - llm.RateLimiter.Release rolls back a slot when its pass fails; Allow now returns the recorded timestamp so Release only frees its own slot. - suggestions.runPass releases the slot on LLM failure before returning 502. - useCheckpoint auto-retries a failed checkpoint with backoff (3/12/35s), keeping the breathing dot up and only flagging "resting" once retries exhaust. Claude-Session: https://claude.ai/code/session_016Yr6jELuRc7hyzYLccQKZd
This commit is contained in:
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -20,17 +20,29 @@ export function useCheckpoint(docId: string | null) {
|
||||
const [llmDown, setLlmDown] = useState(false)
|
||||
|
||||
const debounceRef = useRef<ReturnType<typeof setTimeout>>(undefined)
|
||||
// Pending auto-retry timer for a failed checkpoint (see runCheck).
|
||||
const retryRef = useRef<ReturnType<typeof setTimeout>>(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) => {
|
||||
|
||||
Reference in New Issue
Block a user