perf: bound L1 access history, adopt trailing-window TTL

[groupthinking]

Canonical issue

Closes #1294.

Outcome

This change does two coupled things, and the second one is deliberate. It bounds
each resident key's hit-timestamp history, and — as a direct and inseparable
consequence — moves _calculate_adaptive_ttl from a lifetime-average frequency
estimate to a trailing-window one. The second is not scope that crept in: the list
being bounded is the list the TTL estimator reads, so there is no version of this
containment that leaves TTL semantics untouched, short of retaining an unbounded
copy purely for the estimator — which forfeits the entire saving. Both effects are
measured below, the TTL one in ## Risk.

Does	Does not
Bounds each resident key's hit-timestamp history to ACCESS_HISTORY_WINDOW = 64	Change when history is created — still one append per hit
Change the growth basis of L1's access history from reads served to resident keys	Change any cache-entry payload, key, TTL storage, or serialization
Move _calculate_adaptive_ttl to a trailing-window frequency estimate — deliberate and coupled, measured in ## Risk	Change the adaptive-TTL thresholds (0.1 → 4x, 0.01 → 2x) or how they are applied
Leave entry count, total_size_bytes, eviction and expiry behaviour untouched	Add a sweeper, a settings knob, or any background task
Keep the hot path O(1) — deque(maxlen=) evicts on append	Alter _release_entry() / _is_expired() from #1298

Scope

 src/youtube_extension/backend/services/intelligent_cache.py   (+16/−2)
 tests/unit/test_intelligent_cache.py                          (+117/−0)

• intelligent_cache.py
  • import deque from collections
  • new module constant ACCESS_HISTORY_WINDOW = 64 with a comment explaining
    what the window has to be long enough to do
  • self.access_patterns = defaultdict(list) → defaultdict(lambda: deque(maxlen=ACCESS_HISTORY_WINDOW))
• test_intelligent_cache.py — two new classes, four tests, appended at EOF.

No other call site changed. Every use of access_patterns in src/ (9 references,
all in this file) is append / pop / clear / in / len / [0] / [-1] —
no slicing, no extend, no +=, no list-only method. The structure is a
drop-in.

Risk

The real risk is a semantics change, not a memory change: _calculate_adaptive_ttl
now estimates frequency over a trailing window instead of the key's whole lifetime.
frequency = len(accesses) / (accesses[-1] - accesses[0]). Truncating the front of
the list changes both the numerator and the denominator, so the estimate can move.

I measured exactly when it moves, with synthetic timestamps across five access
patterns:

access pattern	before	after	reading
hot burst (500 hits / 1s)	14400s	14400s	identical
uniform, 1 hit / 20s (300 hits)	7200s	7200s	identical
cold (2 hits / 1h)	3600s	3600s	identical
was hot, then quiet 2.5h	7200s	3600s	diverges
was quiet, now bursting	7200s	14400s	diverges

For any uniform rate the two agree exactly — len / time_span is scale-free
under uniform sampling, so truncation cancels. They diverge only when the key's
rate changed. In both divergent cases the window reports the key's current
behaviour and the unbounded list reports a lifetime average skewed by history the
key has outgrown: a key that was hot two hours ago and is now idle stops being
given a 2x TTL, and a key that has just started bursting gets its 4x TTL
immediately instead of waiting for its dead history to be diluted.

I am claiming this is an improvement, and I want to be explicit that it is a
behaviour change and it is load-bearing, not a no-op. The strictly-memory-only
alternative is to keep an unbounded copy purely for the TTL calculation — which
forfeits the entire saving, since that list is the thing being bounded.

Reviewed and settled. This was put to review explicitly as "should the TTL
semantics be pulled out into a separate follow-up?", and the answer was no: the
shift is inseparable from the retention change rather than hidden extra scope, the
divergence is measured rather than accidental, and it ships as one change. The
condition attached to that was that the writeup say so plainly instead of selling
this as memory-only — which is what the title and ## Outcome above now do.

Secondary risks:

• Window size is a judgement call. 64 is large enough that
  len/(last-first) is a rate rather than a two-sample artefact, and small
  enough that 10k resident keys cost ~2.7 MiB of history. It is a module
  constant, not a setting, per the guidance on perf: bound L1 cache access-history retention #1295 — there is no telemetry
  that would let anyone tune it, so a knob would just be an untestable branch.
• Not a correctness fix. Nothing was returning wrong results. This is a
  containment change.
• Entry-lifecycle bookkeeping (eviction / lazy expiry / expired-key replacement)
  is deliberately not touched here — that was L1 cache removal paths leak entry bookkeeping, costing 45% of capacity #1297 / PR perf: release full entry bookkeeping on all L1 cache removal paths #1298, already on
  main. This PR is only about the growth of a resident key's history, which
  perf: release full entry bookkeeping on all L1 cache removal paths #1298 does not address and cannot address.

Verification

Non-vacuity

Probe: 500 resident keys, 2,000 hits each — 1,000,000 reads. Nothing is evicted
or expired, so every key is a live resident key throughout.

Retained access history: 31,340 KiB → 1,379 KiB (22.7x, 29,961 KiB released).

Controls, measured in the same run:

control	before	after
total_size_bytes reported by the cache	1.5 KiB	1.5 KiB
resident entries	500	500
adaptive TTL for hot key k0	14400s	14400s

The headline number is what makes the case, and the first control is what makes
the case interesting: 30.6 MiB of history against 1.5 KiB of accounted
payload. CacheStats counts CacheEntry.size_bytes only, so neither the byte
budget nor the entry budget can see this memory, let alone reclaim it.

Supporting: retained timestamps per key 2,000 → 64; total 1,000,000 → 32,000.

The framing that matters is not the ratio, which is an artefact of how long I ran
the probe. It is that before, retention was a function of reads served, and
after, it is a function of resident keys — a quantity the cache already bounds
by two independent budgets. The 22.7x doubles if I double the read count and the
new number does not move.

Prove-fail

Reverted only the bound — deque(maxlen=ACCESS_HISTORY_WINDOW) back to
list — while deliberately keeping ACCESS_HISTORY_WINDOW defined, so the
tests fail on their assertions rather than on ImportError:

2 failed, 177 passed
assert 1280 <= 64

The two failures are the retention tests. The two adaptive-TTL tests passed
under the revert — they are supposed to, because they assert that the TTL
calculation is correct, which it is with or without the bound. That is the
point of including them: they prove the two failing tests are detecting the
bound specifically, and are not vacuously coupled to the edit.

Restoring the bound: 179 passed.

Tests added

test	asserts
test_hot_key_history_stays_within_window	after 200 hits on a key that is still resident (precondition asserted), retention ≤ 64
test_window_retains_the_most_recent_timestamps	retained samples are ordered and all ≥ a mid-run boundary — i.e. the newest are kept, not the oldest
test_saturated_window_still_yields_high_frequency_ttl	a hot key with a saturated window still gets base_ttl * 4
test_indexing_and_len_work_over_bounded_history	len, [0], [-1] behave over a deque as _calculate_adaptive_ttl requires

The last two are the fairness controls described above.

Commands

.venv/bin/python -m pytest tests/unit/test_intelligent_cache.py \
  --override-ini="addopts=" -p no:cacheprovider -p no:logging -q
# 179 passed

.venv/bin/ruff check src/youtube_extension/backend/services/intelligent_cache.py \
                     tests/unit/test_intelligent_cache.py
# identical code set to origin/main for both files (parity checked by content swap
# in place, since ruff per-file-ignores are path-scoped)

.venv/bin/black --check --diff <both files>
# every one of the 135 added lines survives Black verbatim

Production evidence

No production telemetry is attached, and I do not want to imply otherwise. This
memory is invisible to the cache's own metrics — that is the substance of the
issue — so there is no existing dashboard that would have shown it and none that
will show it shrink.

The evidence is the probe above, which drives the real InMemoryCacheLayer on
the real code path (await layer.get(...)), not a model of it, and measures
retention with sys.getsizeof over the actual structures. Its controls
(total_size_bytes, resident entries, adaptive TTL) are what establish that the
change is confined to what it claims.

Agent handoff

• Issue perf: bound L1 access history, adopt trailing-window TTL #1294 re-scoped to windowing-only before this PR was opened
• All 9 access_patterns references audited for deque compatibility
• Prove-fail run with the constant retained, so failures are behavioural
• Fairness controls included and confirmed passing under the revert
• TTL semantics change measured across 5 access patterns, not asserted
• ruff parity + Black verified on added lines

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
v0-uvai	Ready	Preview, v0	Aug 3, 2026 10:55pm

[coderabbitai]

Warning

Review limit reached

@groupthinking, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4ab1e891-88f1-4696-91a4-03213aca07e9

📥 Commits

Reviewing files that changed from the base of the PR and between b04c402 and 77ef316.

⛔ Files ignored due to path filters (1)

• tests/unit/test_intelligent_cache.py is excluded by !tests/**

📒 Files selected for processing (1)

• src/youtube_extension/backend/services/intelligent_cache.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 77ef316.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[github-actions]

Agent Completion Truth Gate: NOT_APPLICABLE

Evidence agrees.

Machine-readable verdict

{
  "details": {},
  "reasons": [],
  "verdict": "not_applicable"
}

Workflow evidence

[groupthinking]

@linear This is the second half of the #1295 split you recommended. #1298 fixed the
bookkeeping leaks and is merged; this one does only the thing that PR deliberately
left alone — bounding the history of a key that never leaves the cache.

Three judgement calls, weakest first.

1. The weakest point by a distance: this is not actually a memory-only change,
   and I want that challenged rather than waved through. _calculate_adaptive_ttl
   computes len(accesses) / (accesses[-1] - accesses[0]). Truncating the front of
   the list moves both terms, so the TTL can change. I measured where:

   pattern	before	after
   hot burst	14400s	14400s
   uniform 1/20s	7200s	7200s
   cold	3600s	3600s
   was hot, now quiet 2.5h	7200s	3600s
   was quiet, now bursting	7200s	14400s

   Uniform rates agree exactly (len/span is scale-free under uniform sampling).
   They diverge only when the key's rate changed, and there the window reports
   current behaviour while the unbounded list reports a lifetime average. I think
   that is strictly better and I've argued so in ## Risk — but it is a behaviour
   change riding along in a PR whose headline is memory. Do you want it framed that
   way, or would you rather I not ship a TTL semantics change under a retention
   title? The only strictly-memory-only alternative I can see is keeping an
   unbounded copy for the TTL maths, which forfeits the entire saving.

2. ACCESS_HISTORY_WINDOW = 64, module constant, no setting. You endorsed the
   constant-over-knob call on perf: bound L1 cache access-history retention #1295 and I've kept it. 64 is picked so len/span is
   a rate rather than a two-sample artefact, and so 10k resident keys cost ~2.7 MiB.
   But 64 is a guess — there's no telemetry to fit it to. Is a defensible guess in a
   named constant the right resolution, or would you rather see a wider window given
   the divergence in (1) is a direct function of window length?

3. Magnitude. I've framed this as containment, not a fix — nothing returned
   wrong results. The number is 30.6 MiB retained against 1.5 KiB of accounted
   payload, but I've tried to keep the ratio out of the headline because it's an
   artefact of how long I ran the probe. The claim I actually want to stand behind
   is that the growth basis changes from reads-served to resident-keys. Is that
   the right level to pitch it?

One thing I did not change: there is still no sweeper, so history for a key that
is resident-but-idle is retained until the key is evicted. That's the same
out-of-scope disclosure I made on #1298 and I'd rather it stay a separate change.

Verification: 179 passed. Prove-fail reverted only the bound while keeping the
constant defined, so the failures are behavioural rather than ImportError —
2 failed, 177 passed, headline assert 1280 <= 64. The two adaptive-TTL tests
passed under that revert, which is the part I care about: they show the two
failing tests are detecting the bound specifically and aren't just coupled to the edit.

Good to merge, or do you want the TTL semantics pulled out into its own change?

@coderabbitai review

[linear-code]

I’d merge this, but I would not sell it as memory-only.

• The TTL shift feels inseparable from the retention fix, not like hidden extra scope. You’ve measured exactly where it moves, explained why the new answer is better, and shown the change is “current-rate window” rather than accidental churn. That’s enough to ship it as one change.
• I would make that explicit in the framing: bounded retention and trailing-window adaptive TTL semantics. The current title undersells that.
• 64 as a named constant is the right call. A knob without telemetry would be fake precision, and a wider window just trades back toward the old lifetime-average behavior.
• Your magnitude framing is right: the important claim is that growth changes from reads-served to resident-keys, not the particular probe ratio.

So: good to merge, with the writeup clearly stating that the behavior change is deliberate and coupled to the containment strategy, not pulled into its own follow-up.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- PR diff ---'
git diff --find-renames origin/main...HEAD -- \
  src/youtube_extension/backend/services/intelligent_cache.py \
  tests/unit/test_intelligent_cache.py

printf '%s\n' '--- cache module outline ---'
ast-grep outline src/youtube_extension/backend/services/intelligent_cache.py --items all --view expanded

printf '%s\n' '--- access-history and TTL references ---'
rg -n -C 5 '\b(access_patterns|_calculate_adaptive_ttl|ACCESS_HISTORY_WINDOW)\b' \
  src/youtube_extension/backend/services/intelligent_cache.py \
  tests/unit/test_intelligent_cache.py

Length of output: 26697

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test-file candidates ---'
fd -t f 'test_intelligent_cache\.py$' . || true

printf '%s\n' '--- L1 access and lifecycle implementation ---'
sed -n '140,320p' src/youtube_extension/backend/services/intelligent_cache.py

printf '%s\n' '--- adaptive TTL implementation ---'
sed -n '760,815p' src/youtube_extension/backend/services/intelligent_cache.py

printf '%s\n' '--- all source access_patterns references ---'
rg -n -C 3 '\baccess_patterns\b' src

Length of output: 16956

---

@groupthinking I found no blocking issue in the implementation.

Keep the adaptive-TTL behavior in this PR. The bounded history necessarily makes _calculate_adaptive_ttl() use a recent-window frequency estimate. Keeping an unbounded TTL-only history would preserve the memory-growth failure that this PR fixes.

Frame this as both of the following:

• L1 access-history retention is bounded by resident keys.
• Adaptive TTL uses the most recent 64 hits for a key after the window saturates.

That framing is accurate. The changed behavior for a key whose rate changes is intentional and beneficial. A key that is now quiet should not retain a long TTL only because it was previously hot. A key that is now hot should not wait for obsolete quiet history to decay.

ACCESS_HISTORY_WINDOW = 64 is appropriate as a named module constant. A settings knob would add an unsupported tuning surface because no telemetry defines a valid operating range.

The production call sites are compatible with deque: they use append, pop, clear, membership testing, len, and first/last indexing only. The bounded deque preserves the required hot-path complexity.

The sandbox checkout did not contain the unit-test file, so I could not inspect the final file directly. The PR diff shows the expected retention and deque-compatibility coverage. The supplied prove-fail result also isolates the retention bound correctly.

Good to merge.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[Copilot]

Pull request overview

Bounds L1 cache access-history growth while preserving adaptive-TTL behavior.

Changes:

• Uses a 64-entry bounded deque per resident key.
• Adds retention, ordering, and adaptive-TTL regression tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/youtube_extension/backend/services/intelligent_cache.py	Bounds per-key access history.
tests/unit/test_intelligent_cache.py	Verifies bounded retention and TTL compatibility.

[groupthinking]

Independent verification (review requested)

Checked out at head 77ef316 and re-ran the claims rather than taking them on trust. All hold:

Check	Command	Result
Full suite	pytest tests/unit/test_intelligent_cache.py	179 passed — matches the PR claim exactly
New tests only	-k "AccessHistoryRetentionIsBounded or AdaptiveTtlOverBoundedHistory"	4 passed

Correctness of the drop-in: _calculate_adaptive_ttl is the only consumer, and it touches access_patterns[key] through len(accesses), accesses[0], and accesses[-1] only — all O(1) on a deque, both index reads guarded by len(accesses) > 1 and the division guarded by time_span > 0. No slicing, extend, or += anywhere in src, so defaultdict(lambda: deque(maxlen=64)) is a true drop-in. Confirmed independently, agreeing with the diff's audit.

On the lint note: ruff/black flag pre-existing whole-file drift, not this diff — black's reformat hunks stop around line ~1251 of the test file, while the added classes begin at line 2123, and the ruff hit is the pre-existing top-of-file import block. Nothing the added lines introduce. The PR's scoped lint claim (added lines only) is accurate.

Framing: agree with @linear and CodeRabbit — ship it as both bounded retention and a deliberate trailing-window adaptive-TTL semantics change. The ## Risk section already measures exactly where the TTL diverges (only when a key's rate changes) and argues the new answer is the better one; that's the correct level to pitch it, and I wouldn't split the TTL change into a follow-up — an unbounded TTL-only copy would forfeit the entire saving.

Verdict: good to merge on the merits. Not auto-merging: per the runbook's publish gate this targets protected main, carries no automerge label, and auto_merge_policy is unset → human sign-off required. agent-completion/truth-gate/pr-1301 is mid-re-run (its prior verdict this head was NOT_APPLICABLE — a pass); worth letting it settle green before merge.

Staged for a maintainer:

gh pr merge 1301 --repo groupthinking/EventRelay --squash

---

Generated by Claude Code

[groupthinking]

@linear Actioned — thank you, that's the right correction and it was the part I was
least sure I'd got right.

Two changes, both framing, no source change:

• Title — perf: bound L1 cache access-history retention →
  perf: bound L1 access history, adopt trailing-window TTL. The canonical issue
  perf: bound L1 access history, adopt trailing-window TTL #1294 is retitled to match, so the squash-merge subject and the issue agree.
• ## Outcome now opens by stating both effects and why they are coupled,
  rather than leading with memory and burying the TTL shift in ## Risk. The row
  that previously read "keep _calculate_adaptive_ttl working over the bounded
  history" was the specific piece of underselling you're pointing at — it framed a
  deliberate semantics change as a compatibility detail. It now reads as a Does.
  ## Risk records that the split question was asked and settled here rather than
  deferred, so a later reader doesn't re-litigate it.

I've left the divergence table where it is, in ## Risk rather than ## Outcome —
## Outcome now points at it explicitly. If you'd rather the five rows sit above
the fold I'll move them, but my read is that ## Outcome should say what changed
and ## Risk should carry the evidence for how much.

CI is settled and green. Merging on that basis unless you'd like the table moved.

[linear-code]

Thanks — this is exactly the kind of challenge I wanted on it.

Your re-run closes the main doubts for me: the container swap is a real drop-in, the lint note is scoped honestly, and the TTL change is deliberate rather than accidental baggage. I agree with your framing and verdict: mergeable on the merits, with human sign-off and the gate finishing green first.

[linear-code]

Looks right as-is. Keeping the divergence table in ## Risk makes sense to me: ## Outcome states the semantic change clearly, and ## Risk carries the measured boundary of that change.

[linear-code]

GRV-280