AI Code Review for Caching Changes: The Checklist Before a Cache Ships

The diff looks like a performance win. It is a correctness change.

A caching pull request is the easiest kind to approve. It adds a decorator, a a get followed by a set around a slow call, a TTL, maybe a Redis client. The tests pass, because the tests did not care about speed, and the author has a graph showing p95 latency dropping by half. What the diff does not show is that the function just became a different function: it now returns a value computed from some earlier inputs, for some period, to whoever presents the same key. Every one of those italicised words is a place the author made a decision, usually implicitly, and every one is where caches go wrong in production.

This post is the review checklist for that PR: seven questions, the failure each one prevents, what an AI reviewer can verify from the diff and the repository, what it can only ask about, and how to turn the checklist into rules that run on every caching change without a human remembering. It follows the same shape as our checklists for database migrations and dependency updates, because caching shares their defining property: the risk is almost never inside the changed lines.

Question 1: Is every input that affects the value part of the key?

This is the one that produces the memorable incidents. A product price is cached by productId alone. The price depends on the customer's currency, their price list, their region's tax rule and a feature flag. The first request after a deploy was from a German customer, and for the next ten minutes every customer in every market saw euros. Nothing in the diff was wrong; the key was simply missing four dimensions.

A reviewer answers this question by reading the function being cached, not the cache call, and listing every value it reads: arguments, request context, current user, locale, time, flags, configuration, anything fetched inside. Each one is either in the key, or provably constant for the cache's lifetime, or a bug. A diff-only reviewer cannot do this, because the function body is usually unchanged and therefore not in the diff. A reviewer with repository context can, and it is the single highest-value check on a caching PR.

Question 2: Can this entry be served to someone it does not belong to?

Question 1 in its most dangerous form. Anything derived from the current user, session, tenant or authorization decision must have that identity in the key, or must not be cached in a shared store at all. The failures here are not slow pages; they are one customer's order history rendered for another, or an “is admin” check cached across users. The pattern to flag is a cache call placed above the point where identity is resolved, or a key built from route parameters while the handler also reads the session. It is also the one question where the reviewer should be blocking rather than advisory, in the sense we laid out in blocking versus advisory review earlier this year.

Question 3: Does every write path invalidate or version the entry?

The diff shows where the cache is read and, if the author was careful, one place where it is cleared. It does not show the other three places the underlying data is written: the admin tool, the nightly import, the webhook handler another team owns. A reviewer with the repository can enumerate writes to the same table or document and check that each either invalidates the key, bumps a version that is part of the key, or is covered by a TTL the team has explicitly accepted. “We will invalidate on write” in the PR description is a claim; the list of writers is the evidence.

Versioned keys deserve a mention because they are both safer and easier to review: if the key includes an updated_at or a version counter, invalidation becomes a non-event and the only remaining question is whether that version is bumped by every writer, which is a smaller search than finding every cache-clear call.

Question 4: Why this TTL?

A TTL of 300 seconds appears in an enormous number of PRs and is justified in almost none. The number encodes a business decision: this data may be up to five minutes stale, for everyone, always. Sometimes that is fine. Sometimes it means a price change takes five minutes to reach the checkout, or a revoked permission keeps working for five minutes. The reviewer's job is not to pick the number but to make the author say why, and to flag the cases where staleness has a cost the author may not have considered: anything about money, access, inventory or safety.

The related check is whether the TTL is the only invalidation, in which case the answer to question 3 is “no” and the TTL is doing all the work.

Question 5: What happens when the key is cold or expires under load?

A cache that serves ten thousand requests a minute from one key will, at the instant that key expires, send ten thousand requests to the origin. This is the stampede, and the simplest caching code has no defence against it. A reviewer should look for one of the standard mitigations, a lock or single-flight around the recompute, probabilistic early refresh, serving stale while revalidating, or a staggered TTL, and should ask about it explicitly whenever the cached operation is expensive or the key is obviously shared. The diff rarely contains the answer; the question still belongs on the PR, because the first time the team learns about stampedes should not be at 9 a.m. on launch day.

Question 6: Are errors, empty results and partial data being cached by accident?

The code path that caches a result usually caches whatever the call returned. If the call returned an empty list because the database timed out, that empty list is now the answer for the next five minutes. If it threw and the wrapper caught, a null may be cached. Negative caching is a legitimate technique, but it must be deliberate, with its own short TTL, and the reviewer should check what the code does on exception, on empty, and on a result that is only partially populated because a downstream call failed. This is a diff-level check and an AI reviewer is good at it, provided the error paths are in the diff or the wrapped function is read.

Question 7: Can the next deploy read what this deploy wrote?

A cache outlives a deploy. If the cached value is a serialized object and the next PR renames a field, the new code reads old entries with the old shape, and the failure is either a crash or, worse, silently missing data. The reviewer should check that the serialized shape is versioned, or that the key includes a schema or app version, or that the team has accepted a flush-on-deploy. The same applies in reverse during a rollback. This question is about the diff two weeks from now, which is exactly the kind of context a human reviewer forgets and a rule does not.

The mechanical checks

Alongside the seven questions, a short list of things a reviewer should flag mechanically on any caching PR:

• A key built by string concatenation without a delimiter, so that ids 12 and 3 collide with 1 and 23.
• Keys that include unbounded user input, which turns the cache into a memory-exhaustion vector.
• In-process caches in code that runs on more than one instance, with invalidation that only reaches the local process.
• A cache client created per request instead of shared, or one with no timeout, so that a slow cache makes the request slower than no cache.
• Secrets, tokens or personal data written to a cache store with different retention and access controls from the primary database.
• Caching added to a function that has side effects, which now run only on a miss.
• Missing metrics: no hit, miss or latency counters, so the team cannot tell next month whether the cache is helping.

What an AI reviewer can and cannot verify

Questions 1, 2, 3, 6 and 7 are answerable from code, if the reviewer can read beyond the diff: the cached function's body, every writer to the underlying data, the error paths, the serialization. That is a context problem, which is why how much of the repository the reviewer sees determines whether caching PRs get a real review or a style pass. The mechanical checks are pure diff work and any competent reviewer should catch them.

Questions 4 and 5 are not answerable from code. Whether five minutes of staleness is acceptable, whether a key is hot, what a miss costs in dollars and seconds: these live in dashboards and product decisions. The right behaviour for an AI reviewer is to ask them, specifically and once, on every PR that introduces a TTL or a shared key, and to accept the author's answer. A review comment that says “this TTL means a revoked permission can keep working for up to 300 seconds; is that intended?” has done its job whichever way the author answers.

Turning the checklist into rules

None of this helps if it depends on the one engineer who has been burned by a cache before happening to be the reviewer. PURA runs custom review rules against every pull request, scoped to paths or patterns, so the checklist can be written down once and applied every time a diff touches a cache client, a memoization decorator or a key builder. A workable starting set, in plain language the reviewer applies to the code:

• When a function is wrapped in a cache, list every input it reads and confirm each is part of the key or constant; flag any that are not.
• Flag any cached value derived from the current user, session or tenant whose key does not include that identity.
• For every cached read, find all writers of the same data in the repository and report any that do not invalidate or version the entry.
• Require a one-line justification for any TTL, and block when the cached data concerns authorization, pricing, inventory or balances and the TTL is the only invalidation.
• Ask about stampede protection whenever the cached operation calls a database or external service.
• Flag caching of exceptions, nulls and empty collections unless a negative-cache TTL is set explicitly.
• Flag serialized cache values without a version in the key or the payload.

Because PURA is self-hosted on your own infrastructure and reviews with your own model keys, the rules and the repository context stay inside your boundary, and the per-review pricing means a rule that fires on every caching PR costs the same whether it finds something or not. The point of the rules is not to replace the discussion about TTLs and hot keys; it is to make sure the discussion happens, on the PR, before the cache ships.

The one-paragraph version

A cache changes what a function means. Review it as a correctness change: every input in the key, no cross-user leaks, every writer invalidates, every TTL justified, stampedes considered, errors not cached by accident, and a shape the next deploy can read. Five of those a reviewer with repository context can verify; two it must ask. Write all seven down as rules, and the next caching PR gets the review this one deserved.