How to review an AI-written API rate limiter

To review an AI-written API rate limiter, write down the rule it is meant to enforce before reading the counter code. Which caller gets a quota, which requests spend it, over what interval, and what happens at the limit? Then trace that rule through authentication, shared state, the route handler, and the response. A unit test that gets a 429 after N calls can pass while two workers each admit N calls or while unrelated users share one bucket.

Suppose an agent adds a limit to POST /reports, an expensive endpoint. The PR increments an in-memory map keyed by IP address and returns 429 when the count crosses a threshold. That looks complete in a single-process test. In production, requests may reach several instances, an office may share one public IP, and the report job may start before the middleware rejects the request. Those are the review paths that decide whether the limit actually protects the endpoint without blocking the wrong people.

Define the caller and the budget

Start with a small contract: the operation being limited, the quota owner, the cost of one request, the window or refill rule, and any exemptions. Is this per account, user, API key, or anonymous network source? A forwarded IP header is not a trustworthy identity unless a known proxy has set it and the app discards client-supplied values. An IP-only key can also group many legitimate users behind one NAT. Choose the key for the abuse or capacity problem the service actually has, and state what happens before a caller is authenticated.

Check the key's scope. A per-user bucket across all endpoints behaves differently from a per-user POST /reports bucket. If a report with a large payload is much more expensive than a small one, counting both as one may not match the intended budget. Conversely, allowing clients to provide their own cost or bucket key lets them escape the limit. The PR should name the rule in terms the team can test, such as 'one account may start five reports in ten minutes,' rather than only naming a limiter library.

In Review Assist, compare this contract with problem.user_asks, approach.requirements, and any unverified assumptions in the Intent Document. The document can show why the agent chose an account key or an IP fallback; the reviewer still has to decide whether that choice fits the service. If the limiter expands from one route to every API route, use the scope drift guide to ask why.

Follow the decision across instances and side effects

Find where the counter lives and how an admission is claimed. A process-local map resets on restart and gives each instance its own quota. A shared store only solves that part if the read, decision, and update are atomic for a given key. With a separate read and write, two requests can both see the last available slot and both proceed. Check the store's expiry behavior and the clock source used to reset or refill the bucket. At a window boundary, a fixed window may allow a burst on both sides; that can be acceptable, but it should match the stated capacity goal.

Trace middleware order to the first expensive action. Does the limit run before the report job is enqueued, database work starts, or a provider call happens? Does authentication run early enough to choose the right account without exposing expensive work to unauthenticated traffic? Check bypass routes deliberately, including health checks, internal calls, and webhooks. If the limiter's store is unavailable, the code needs an explicit fail-open or fail-closed decision tied to this endpoint's risk. A silent catch that admits every request changes the protection the PR claims to add.

This is a review of a hypothetical service's limiter, not a claim that Review Assist rate limits that service. Review Assist's architecture describes its own webhook and guided-review paths. Its GitHub App recounts Intent Document coverage on a PR; it does not load-test an application endpoint or certify a quota policy.

Make rejection useful to the client

At the boundary, inspect the actual HTTP response and the client that handles it. RFC 6585 defines 429 for too many requests and permits a Retry-After field. HTTP Semantics defines that field as either an HTTP date or a non-negative number of delay seconds. If the PR sends it, check that the value agrees with the bucket's reset or refill behavior rather than a hard-coded guess. The response should tell a caller enough to recover without exposing another user's quota state.

Follow retries into the client or job runner. A client that immediately retries every 429 can turn a limit into a request storm. A rejected POST /reports must not leave a job running, and a retry after an uncertain accepted response is a separate idempotency question. If the route already has an idempotency key, check whether a retry of the same operation consumes another quota unit and whether that behavior was intended. The payment idempotency review covers the side-effect problem in more detail.

Test the boundary, not just the helper

Ask for a service-level test with the production key selection and storage semantics. Send requests for two accounts so one exhausted bucket does not block the other. Send a burst at the threshold concurrently and assert how many reports were actually started, not only how many responses were 200. Repeat through two application instances or a faithful shared-store test. Exercise expiry with a controlled clock and check the first request after refill.

Add cases for an unauthenticated caller, a spoofed forwarding header if IP fallback exists, and a store failure. Assert that a rejected request has no report side effect. Capture the 429 body and Retry-After value, then test that a client waits or stops as the contract says. A test that mocks the limiter to return true or false will not expose a race in its shared counter or a misplaced middleware call.

Read verification.performed and verification.not_verified against the PR head. Review Assist can anchor the middleware, store, route, and client changes and show what the agent says it exercised. Its coverage check says the counted hunks have explanations; it does not prove the admission rule under load. Turn any missing boundary case into a specific request using the untested paths guide.

Leave a review comment that can be closed

A useful comment is: “This map is per process and keyed by raw IP, but the requirement says five reports per account. Please show the trusted account key, use one atomic claim shared by both instances, and add a concurrent test that asserts only five jobs start. Also show the 429 response and what happens when the counter store is unavailable.”

Approve when the quota contract is explicit, the identity and storage enforce it across the deployed topology, rejection happens before the protected work, and tests cover concurrency, isolation, reset, and failure behavior. If the team accepts a burst or a fail-open mode, record that as a deliberate capacity decision with an owner rather than letting it hide inside the library defaults.