feat(ratelimit): per-provider bulk token reserve for interactive UI headroom (bookshelf-zvlod) #1130

Merged

zombor merged 3 commits from bd-bookshelf-zvlod into main 2026-07-13 17:02:57 +00:00

Conversation 6 Commits 3 Files changed 17 +1993 -109

zombor commented 2026-07-13 15:30:05 +00:00

Owner

Copy link

Summary

• Adds ClaimTokenBucketSlotWithReserve SQL query: bulk callers blocked at tokens < reserve + 1.0, interactive callers drain to 0
• Adds WaitBulk(ctx, reserve) on DBTokenBucket; falls back to Wait when ClaimWithReserve == nil or reserve <= 0
• Wires makeDBBulkWaitFor on all burst>1 providers (ComicVine, AniList, Hardcover, Metron) in both buildEnrichDeps and buildBulkLLMSweepDeps; replaces the old metronBulkBurst/metronBulkInterval hack with the same reserve mechanism
• DB-backed settings metadata.<provider>_bulk_reserve (default=5, clamped to [0, burst-1]); live — no restart needed
• Settings > Rate Limits tab: per-provider editable reserve number input with Stimulus ratelimit-reserve controller, inline save feedback, CSRF-safe PUT to /settings/ratelimit-reserve
• 100% Go coverage (no exclusions added), all 3963 JS tests pass

Test plan

• Unit: internal/metadata/ratelimiter — WaitBulk coverage (nil ClaimWithReserve fallback, reserve<=0, immediate grant, deny-then-grant, claim/seed errors, ctx cancellation during sleep)
• Unit: internal/settings — LoadBulkReserve, SaveBulkReserve, BuildSaveBulkReserves, BuildLoadBulkReserves, SaveRateLimitReserveHandler, shell_handler BulkReserves loading
• Integration: internal/db — ClaimTokenBucketSlotWithReserve grants when full, denies when drained to reserve floor
• JS: ratelimit_reserve_controller — happy path, CSRF injection, saving states, error handling, 3s auto-clear
• make coverage — composite 97.2%, check-coverage OK

Closes bead bookshelf-zvlod on merge.

## Summary - Adds `ClaimTokenBucketSlotWithReserve` SQL query: bulk callers blocked at `tokens < reserve + 1.0`, interactive callers drain to 0 - Adds `WaitBulk(ctx, reserve)` on `DBTokenBucket`; falls back to `Wait` when `ClaimWithReserve == nil` or `reserve <= 0` - Wires `makeDBBulkWaitFor` on all burst>1 providers (ComicVine, AniList, Hardcover, Metron) in both `buildEnrichDeps` and `buildBulkLLMSweepDeps`; replaces the old `metronBulkBurst`/`metronBulkInterval` hack with the same reserve mechanism - DB-backed settings `metadata.<provider>_bulk_reserve` (default=5, clamped to [0, burst-1]); live — no restart needed - Settings > Rate Limits tab: per-provider editable reserve number input with Stimulus `ratelimit-reserve` controller, inline save feedback, CSRF-safe PUT to `/settings/ratelimit-reserve` - 100% Go coverage (no exclusions added), all 3963 JS tests pass ## Test plan - [x] Unit: `internal/metadata/ratelimiter` — WaitBulk coverage (nil ClaimWithReserve fallback, reserve<=0, immediate grant, deny-then-grant, claim/seed errors, ctx cancellation during sleep) - [x] Unit: `internal/settings` — LoadBulkReserve, SaveBulkReserve, BuildSaveBulkReserves, BuildLoadBulkReserves, SaveRateLimitReserveHandler, shell_handler BulkReserves loading - [x] Integration: `internal/db` — ClaimTokenBucketSlotWithReserve grants when full, denies when drained to reserve floor - [x] JS: `ratelimit_reserve_controller` — happy path, CSRF injection, saving states, error handling, 3s auto-clear - [x] `make coverage` — composite 97.2%, check-coverage OK Closes bead bookshelf-zvlod on merge.

ratelimit-reserve-ui.png

111 KiB

ratelimit-reserve-ui.png

111 KiB

ratelimit-reserve-ui.png

111 KiB

ratelimit-reserve-ui.png

111 KiB

ratelimit-reserve-ui.png

111 KiB

ratelimit-reserve-ui.png

111 KiB

zombor added 1 commit 2026-07-13 15:30:05 +00:00

feat(ratelimit): per-provider bulk token reserve for interactive UI headroom (bookshelf-zvlod) ... 2c00dd03be

Background/bulk jobs leave a configurable floor of tokens so interactive UI
fetches always have headroom. Replaces the old metronBulkBurst/metronBulkInterval
hack with the same reserve mechanism applied to all burst>1 providers.

- SQL: ClaimTokenBucketSlotWithReserve query (WHERE refilled_tokens >= reserve+1)
- Limiter: WaitBulk(ctx, reserve) on DBTokenBucket; interactive Wait unchanged
- Wiring: buildBulkLLMSweepDeps + buildEnrichDeps use makeDBBulkWaitFor for
  ComicVine, AniList, Hardcover, Metron; burst=1 providers use regular Wait
- Settings: DB-backed metadata.<provider>_bulk_reserve keys, default=5
- UI: editable per-provider reserve control on Settings > Rate Limits tab
  (Stimulus ratelimit-reserve controller + SaveRateLimitReserveHandler)
- 100% Go coverage, 100% JS coverage; no golangci exclusions added

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-07-13 15:45:04 +00:00

fix(ratelimit): complete JS coverage for ratelimit_reserve_controller ... bcacb0ad86

Add tests for disconnect(), hasStatusTarget=false inside timer callback,
empty response body branch, and error fallback with no message. Achieves
100% statement/branch/function/line coverage on the new controller.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor commented 2026-07-13 16:02:52 +00:00

Author

Owner

Copy link

Security / robustness review — PR #1130 (per-provider bulk token reserve)

Reviewed the diff (origin/main...origin/bd-bookshelf-zvlod) against the rate-limit-weakening, footgun, admin-gating, Metron-regression, and arch-boundary focus areas.

1. No rate-limit weakening — CONFIRMED. ClaimTokenBucketSlotWithReserve uses the identical decrement (GREATEST(0, LEAST(burst, refilled - 1.0))) as the base ClaimTokenBucketSlot; only the WHERE grant threshold is raised from >= 1.0 to >= reserve + 1.0. Bulk is therefore strictly stricter, never grants more than base, and the row's burst cap + refill_per_ms are unchanged. Interactive keeps the plain Claim (internal/app/providers.go:159 newComicVineWait, unchanged file) draining to 0. Both hit the same token_bucket.comicvine / token_bucket.<id>.rate row, so total traffic stays bound by the documented per-provider budget. No path can exceed the cap.

2. reserve>=burst footgun — GUARDED. SaveBulkReserve rejects reserve < 0 || reserve >= burst with ErrValidation (internal/settings/ratelimit_reserve.go); clampReserve clamps an out-of-range stored value to 0 (= disabled = base behaviour) rather than permanently starving bulk; HTML input caps at max={{.MaxReserve}} (burst-1). reserve=0 reproduces pre-feature behaviour. Bulk-starvation self-DoS is prevented at write, load, and UI layers.

3. UI admin-only / injection / secrets — CLEAN. PUT /settings/ratelimit-reserve is wrapped in adminRequired (routes.go); the ratelimits tab (and its BulkReserves data load) is gated by adminOnlyTabs returning ErrForbidden for non-admins before loadRateLimitsTabData runs. Reserve is an int, JSON-decoded, range-validated, and passed as a parameterized sqlc float64 arg — no injection. Canonical metadata-field/metadata-field-input classes, no inline style= (CSP-safe). No secrets exposed.

4. metronBulkInterval-hack replacement — NO REGRESSION. The old headroom hack (bulk burst clamped 20→5 via divergent LEAST on the shared row + slower metronBulkInterval) is removed; bulk Metron now passes the same burst (20) and interval (cfg.MetadataMetronRateLimit) as interactive to the shared token_bucket.metron.rate row, with headroom coming from the reserve floor (default 5). This is a firmer guarantee (≥5 tokens always instantly available to interactive) than the previous probabilistic slow-pacing, and it resolves the carry-over minor #2 (divergent burst/refill on one row). Interactive is not starved worse.

5. Key scheme + arch boundary — INTACT. Bulk paths key token_bucket.<id>.rate (matching the interactive registry) and token_bucket.comicvine; no kentd-style collision. internal/metadata/ratelimiter/db_limiter.go imports only stdlib (context, log/slog, math/rand, sync, time) — no workflow-engine import. Fail-open on settings/DB error degrades to reserve=0 (loses headroom, never exceeds the cap). All four new/changed test files are black-box (*_test packages).

Findings

[MINOR] internal/app/build_extended_deps.go:2093 — bookdrop import path keeps the plain (no-reserve) wait
buildBookdropImportMetadataDeps still wires Hardcover (burst>1) via makeDBWaitFor (base claim, drains to 0). Not a weakening or regression — it matches prior behaviour and is outside the bead's named scope (sweep + bulk enrich) — but this bulk-ish path gets no interactive headroom. Consider folding it into the reserve mechanism in a follow-up if bookdrop import contends with interactive Hardcover fetches.

[MINOR] internal/app/build_extended_deps.go:783 — carry-over: Metron burst(20)+20/min refill still permits ~40 req in a pathological rolling 60s vs the documented 20/min window
Flagged on m7n5c/#1128; the reserve does not touch burst/refill so it neither fixes nor worsens this. This PR does improve consistency (interactive and bulk now share identical burst/refill on the row), but the window-vs-burst overshoot is a separate provider-bucket-shape concern. Low risk (single-user, daily-window backstop). Track separately if tightening is wanted.

REVIEW VERDICT: 0 blocker, 0 major, 2 minor

## Security / robustness review — PR #1130 (per-provider bulk token reserve) Reviewed the diff (`origin/main...origin/bd-bookshelf-zvlod`) against the rate-limit-weakening, footgun, admin-gating, Metron-regression, and arch-boundary focus areas. **1. No rate-limit weakening — CONFIRMED.** `ClaimTokenBucketSlotWithReserve` uses the *identical* decrement (`GREATEST(0, LEAST(burst, refilled - 1.0))`) as the base `ClaimTokenBucketSlot`; only the WHERE grant threshold is raised from `>= 1.0` to `>= reserve + 1.0`. Bulk is therefore strictly *stricter*, never grants more than base, and the row's `burst` cap + `refill_per_ms` are unchanged. Interactive keeps the plain `Claim` (`internal/app/providers.go:159` `newComicVineWait`, unchanged file) draining to 0. Both hit the same `token_bucket.comicvine` / `token_bucket.<id>.rate` row, so total traffic stays bound by the documented per-provider budget. No path can exceed the cap. **2. reserve>=burst footgun — GUARDED.** `SaveBulkReserve` rejects `reserve < 0 || reserve >= burst` with `ErrValidation` (`internal/settings/ratelimit_reserve.go`); `clampReserve` clamps an out-of-range *stored* value to 0 (= disabled = base behaviour) rather than permanently starving bulk; HTML input caps at `max={{.MaxReserve}}` (burst-1). `reserve=0` reproduces pre-feature behaviour. Bulk-starvation self-DoS is prevented at write, load, and UI layers. **3. UI admin-only / injection / secrets — CLEAN.** `PUT /settings/ratelimit-reserve` is wrapped in `adminRequired` (`routes.go`); the `ratelimits` tab (and its `BulkReserves` data load) is gated by `adminOnlyTabs` returning `ErrForbidden` for non-admins *before* `loadRateLimitsTabData` runs. Reserve is an `int`, JSON-decoded, range-validated, and passed as a parameterized sqlc `float64` arg — no injection. Canonical `metadata-field`/`metadata-field-input` classes, no inline `style=` (CSP-safe). No secrets exposed. **4. metronBulkInterval-hack replacement — NO REGRESSION.** The old headroom hack (bulk burst clamped 20→5 via divergent `LEAST` on the shared row + slower `metronBulkInterval`) is removed; bulk Metron now passes the *same* burst (20) and interval (`cfg.MetadataMetronRateLimit`) as interactive to the shared `token_bucket.metron.rate` row, with headroom coming from the reserve floor (default 5). This is a firmer guarantee (≥5 tokens always instantly available to interactive) than the previous probabilistic slow-pacing, and it resolves the carry-over minor #2 (divergent burst/refill on one row). Interactive is not starved worse. **5. Key scheme + arch boundary — INTACT.** Bulk paths key `token_bucket.<id>.rate` (matching the interactive registry) and `token_bucket.comicvine`; no kentd-style collision. `internal/metadata/ratelimiter/db_limiter.go` imports only stdlib (`context`, `log/slog`, `math/rand`, `sync`, `time`) — no workflow-engine import. Fail-open on settings/DB error degrades to `reserve=0` (loses headroom, never exceeds the cap). All four new/changed test files are black-box (`*_test` packages). ### Findings [MINOR] internal/app/build_extended_deps.go:2093 — bookdrop import path keeps the plain (no-reserve) wait `buildBookdropImportMetadataDeps` still wires Hardcover (burst>1) via `makeDBWaitFor` (base claim, drains to 0). Not a weakening or regression — it matches prior behaviour and is outside the bead's named scope (sweep + bulk enrich) — but this bulk-ish path gets no interactive headroom. Consider folding it into the reserve mechanism in a follow-up if bookdrop import contends with interactive Hardcover fetches. [MINOR] internal/app/build_extended_deps.go:783 — carry-over: Metron burst(20)+20/min refill still permits ~40 req in a pathological rolling 60s vs the documented 20/min window Flagged on m7n5c/#1128; the reserve does not touch burst/refill so it neither fixes nor worsens this. This PR does *improve* consistency (interactive and bulk now share identical burst/refill on the row), but the window-vs-burst overshoot is a separate provider-bucket-shape concern. Low risk (single-user, daily-window backstop). Track separately if tightening is wanted. REVIEW VERDICT: 0 blocker, 0 major, 2 minor

zombor added 1 commit 2026-07-13 16:11:08 +00:00

fix(ratelimit): update stale metronBulkBurst/Interval comments to reflect WaitBulk reserve mechanism bdeb6a5b85

zombor commented 2026-07-13 16:15:17 +00:00

Author

Owner

Copy link

Rate Limit Reserve UI screenshot (ratelimit-reserve-ui)

**Rate Limit Reserve UI screenshot** (ratelimit-reserve-ui) 

zombor commented 2026-07-13 16:16:11 +00:00

Author

Owner

Copy link

Rate Limit Reserve UI screenshot (ratelimit-reserve-ui)

**Rate Limit Reserve UI screenshot** (ratelimit-reserve-ui) 

zombor commented 2026-07-13 16:18:06 +00:00

Author

Owner

Copy link

Rate Limit Reserve UI screenshot (ratelimit-reserve-ui)

**Rate Limit Reserve UI screenshot** (ratelimit-reserve-ui) 

zombor commented 2026-07-13 16:19:22 +00:00

Author

Owner

Copy link

Rate Limit Reserve UI screenshot (ratelimit-reserve-ui)

**Rate Limit Reserve UI screenshot** (ratelimit-reserve-ui) 

zombor commented 2026-07-13 16:20:59 +00:00

Author

Owner

Copy link

Rate Limit Reserve UI screenshot (ratelimit-reserve-ui)

**Rate Limit Reserve UI screenshot** (ratelimit-reserve-ui) 

zombor force-pushed bd-bookshelf-zvlod from bdeb6a5b85 to ea8bd5c7bb 2026-07-13 16:50:13 +00:00 Compare

zombor merged commit d364c8655e into main 2026-07-13 17:02:57 +00:00

zombor referenced this pull request from a commit 2026-07-13 17:04:12 +00:00

Merge pull request 'feat(ratelimit): per-provider bulk token reserve for interactive UI headroom (bookshelf-zvlod)' (#1130) from bd-bookshelf-zvlod into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1130

No description provided.