The cache key that ignored the question

This is a submission for DEV’s Summer Bug Smash: Clear the Lineup powered by Sentry.

Project Overview

Paritok is a context compressor. You
hand it a blob of text, a query describing what you care about, and a compression level,
and it hands back a shorter version that is supposed to keep what matters for that query
and drop the rest. Results are cached, because compression is the expensive part.

I was not looking for bugs in it. I was building an audit harness to measure how much
meaning compression costs — feeding the same corpus through with different queries and
checking which critical substrings survive: file paths, error types, line numbers,
identifiers.

Bug Fix or Performance Improvement

Two rows in my results table were byte-for-byte identical. Different query, different
compression level, same 544 characters:

query="Fix the IntegrityError on commit"              level=L0  ->  159 tok  cache_hit=False
query="Explain the tax rounding TODO in compute_tax"  level=L3  ->  159 tok  cache_hit=True

Enter fullscreen mode Exit fullscreen mode

compress() takes four things that shape its output — content, query, level and
kind. The cache was keyed on one of them:

sid = content_hash(content)
cached = self.storage.get_cached_compressed(sid)

Enter fullscreen mode Exit fullscreen mode

So the first caller’s answer was served to every later caller with the same input text, no
matter what they actually asked. The second caller got a compression tuned to somebody
else’s question, marked cache_hit=True, with nothing in the response to say so.

This is worse than a stale cache. A stale cache gives you an old answer to your question.
This gives you a fresh answer to someone else’s.

Code

The fix is open as a PR: Paritok-official/paritok-4b-v1#17 — 2 files, +118/−12.

The audit harness that surfaced it is at
EazyHood/paritok-audit (Apache-2.0).

# The compressed result depends on query, level and kind as well as on the
# content -- query is documented as driving keep/drop -- so all four belong
# in the key. Keying on content alone returned the first caller's answer to
# every later caller. `sid` deliberately stays content-only, because
# expand_context resolves originals by content and that behaviour is correct.
cache_key = content_hash(
    "\x00".join([sid, level or "", kind or "", query or ""])
)

# 4. Cache check (same content AND same intent gets the same answer)
cached = self.storage.get_cached_compressed(cache_key)

Enter fullscreen mode Exit fullscreen mode

My Improvements

Three decisions in this patch are worth explaining, because the obvious fix is wrong.

1. Don’t change sid — add a key beside it.

The tempting one-liner is to fold the query into sid and move on. That breaks something
else: expand_context resolves a compressed reference back to its original by content
hash. sid has to stay content-only for that to keep working. So the patch introduces a
separate cache_key and leaves sid exactly as it was. Two identifiers, two jobs.

2. Classify kind before building the key, not after.

kind was being resolved after the cache check. That meant a caller passing kind=None
and a caller passing the same kind explicitly would key differently, even though the
compressor would treat them identically — a cache miss on every first call, forever.

Moving classification above the cache check fixed that, and it fixed a second bug on the
way: only LocalModelStrategy sniffed kind internally, so GpuServerStrategy was
forwarding kind=None to the server. Central classification means every backend now
receives a real value.

3. Join the parts with a null byte.

"\x00".join(...) rather than "-".join(...) or plain concatenation. Without a separator
that cannot occur in the inputs, level="L0" + kind="1code" and level="L01" +
kind="code" collide into the same key. It costs nothing and removes a class of bug that
would be genuinely miserable to track down later.

The test. tests/test_cache_key_intent.py (+92) asserts the property directly: same
content with different queries must not return the same object, and the same content with
the same intent must hit the cache. It fails on the original code and passes on the patch,
which is the only claim about a fix I actually trust.

What I took away from it

I found this because my harness printed cache_hit next to every row. If it had only
printed token counts, two identical numbers would have looked like a compressor being
consistent, which is exactly what you want it to be.

The bug was invisible in the metric everyone watches and obvious in the one nobody logs.
Since then I print the cache flag next to every cached result I benchmark — the cost is one
column, and it is the column that told me the answer I was measuring wasn’t mine.