Improve rest cache implementation

[matheus1lva]

Summary

Replaces per-module Keyv factory functions (createListsKeyv, createReportsKeyv, createSnapshotKeyv, createTimeseriesKeyv) with a shared createKeyvClient() factory in cache.ts. Optimizes refresh scripts to use batch setMany writes instead of individual set calls, reducing Redis round-trips and Upstash method call / bandwidth usage. Removes manual JSON.stringify/JSON.parse in favor of native Keyv v5 serialization.

Changes

• cache.ts — New shared factory function that centralizes Redis URL and namespace config
• 4x redis.ts — Stripped down to only export key helper functions (factory functions removed)
• 4x refresh*.ts — Batch setMany writes instead of individual set calls per vault/chain; proper keyv.disconnect() on exit
• 4x route.ts — Simplified reads; Keyv handles deserialization natively, no more manual JSON.parse

Breaking Change

This PR changes how values are stored in Redis. Previously, values were manually JSON.stringify'd before being passed to Keyv, resulting in double-serialized strings in Redis (string-escaped JSON). Now, raw JSON objects are passed to Keyv, which handles serialization internally using @keyv/serialize. This means the stored format changes from double-encoded strings to properly serialized JSON.

Existing cached keys are incompatible with the new format. After merging, the read paths will fail to deserialize old values correctly (or return malformed data).

Post-merge steps

1. Flush all REST cache keys from Upstash:

   redis-cli -u $REST_CACHE_REDIS_URL FLUSHALL

2. Run all refresh jobs to repopulate the cache with the new format

   bun packages/web/app/api/rest/list/refresh.ts
   bun packages/web/app/api/rest/snapshot/refresh-snapshot.ts
   bun packages/web/app/api/rest/reports/refresh.ts
   bun packages/web/app/api/rest/timeseries/refresh-timeseries.ts

How to review

1. Start with packages/web/app/api/rest/cache.ts — the shared Keyv factory
2. Check the 4 redis.ts files (list, reports, snapshot, timeseries) — now only export key helpers
3. Review the 4 refresh*.ts files — batch setMany writes and keyv.disconnect() on exit
4. Review the 4 route.ts files — simplified reads without manual JSON parsing

Test plan

1. Refresh caches

bun packages/web/app/api/rest/list/refresh.ts
# Expected: "✓ Completed: <N> vaults cached across <N> chains"

bun packages/web/app/api/rest/snapshot/refresh-snapshot.ts
# Expected: "✓ Completed: <N> vaults processed"

bun packages/web/app/api/rest/reports/refresh.ts
# Expected: "✓ Completed: <N> vaults processed"

bun packages/web/app/api/rest/timeseries/refresh-timeseries.ts
# Expected: "✓ Completed: <N> vaults processed"

2. Verify keys landed in Redis

redis-cli --scan --pattern 'list:vaults*'

list:vaults::250
list:vaults::1
list:vaults::all
...

3. Start web app and curl endpoints

cd packages/web && bun dev

List — all vaults

curl -s http://localhost:3000/api/rest/list/vaults | jq '.[0]'

List — chain filtered

curl -s http://localhost:3000/api/rest/list/vaults/1 | jq 'length'

Snapshot

curl -s http://localhost:3000/api/rest/snapshot/1/0x46af61661b1e15da5bfe40756495b7881f426214 | jq '{address, name}'

Reports

curl -s http://localhost:3000/api/rest/reports/1/0x4282b8f159ee677559dc6a20cd478dd0bde75ff2 | jq '{count: length, first: .[0].eventName}'

Timeseries

curl -s 'http://localhost:3000/api/rest/timeseries/tvl/1/0x0b9ae07457baed5536b1f3e78c9649e980fb4edc' | jq '{type: type, count: length}'

4. Verify no double-serialization

curl -s 'http://localhost:3000/api/rest/timeseries/tvl/1/0x0b9ae07457baed5536b1f3e78c9649e980fb4edc' | jq 'type'
# Expected: "array"  (NOT "string")

🤖 Generated with Claude Code

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
kong	Ready	Preview, Comment	Feb 23, 2026 4:56pm

[murderteeth]

Review: Improve rest cache implementation

1. (keyv as any).setMany(entries) — unsafe cast used 4 times

createKeyv() returns a Keyv v5.5.5 instance that has setMany, but the root keyv package is v4.5.4 — TypeScript resolves to the v4 types. The as any cast works at runtime but silently bypasses type checking on the entry format.

Either upgrade root keyv to v5 so types align, or centralize the cast behind a typed wrapper in cache.ts so callers get type safety.

2. List key format changed — this is a breaking change

The old createListsKeyv('list:vaults') passed a namespace, but createKeyv internally sets useKeyPrefix: false, so the namespace was never applied. Old Redis keys were bare: all, 1, 137, etc. New keys are list:vaults:all, list:vaults:1, etc.

This is actually an improvement (prevents collisions), but it's a breaking change that isn't mentioned in the PR description. Old cached data becomes orphaned and invisible to the new code.

Please add a rollout procedure to the PR description. How do we deploy this without a window where endpoints return 404? Do refresh scripts need to run first? Do old keys need cleanup? This should be spelled out before merge.

3. Hold off on timeseries consolidation

Let's not change the timeseries data model in this PR. The setMany batch writes alone should give us a significant reduction in Upstash usage. Changing the key structure (per-label → per-vault) is a separate concern with its own tradeoffs around value size. Let's measure the setMany improvement first, then decide if consolidation is needed.

Please revert the timeseries consolidation and keep the existing per-label key format. Only apply setMany batching to the current key structure.

4. Test plan needs work

The test plan currently says "Manual: Run each refresh script and verify data is cached correctly." That's not reviewable — I can't tell from that whether you've actually tested this or what "correctly" means.

Please update the test plan with:

• Step-by-step commands a reviewer can copy-paste to verify locally (including any infrastructure like Redis)
• Actual endpoint URLs with example parameters so a reviewer can curl them after running refresh scripts

Also — since the whole motivation is Upstash method calls and bandwidth, please test this against an Upstash test instance and report before/after metrics in a comment. We need to see the actual improvement before merging, especially if we're changing key formats.

[matheus1lva]

I have not got used to the namespace thing on keyv, that got very well through the cracks and was the last thing i got attention to, i reverted the timeseries, fair enough that though.

[matheus1lva]

i got asking myself now if that's worth adding a ttl to the keys of 16 minutes at least, so it can be disposed and allow for a more flexible setup here. Given its updated every 15 minutes.

[murderteeth]

Review: Improve REST cache implementation

Overall the consolidation of per-module Keyv factories into a shared cache.ts factory and the switch to batch setMany writes are solid improvements. However, there are a few issues worth addressing before merging.

---

Bug: console.timeEnd label mismatch in list refresh

packages/web/app/api/rest/list/refresh.ts:7 sets console.time('refresh list:vaults') but line 30 calls console.timeEnd('refresh'). The labels don't match — Node will print a warning and won't show elapsed time.

---

Concern: connectionTimeoutMillis 5s → 50s (10x increase)

packages/web/app/api/db/index.ts changed from 5_000 to 50_000. This is a 10x increase to the Postgres connection timeout, not mentioned anywhere in the PR description. 50 seconds is extremely long — if Postgres is unreachable, requests will hang for nearly a minute before failing. Was this intentional, or a typo (extra 0)?

---

Inconsistency: Timeseries still double-serializes

All other refresh scripts (list, reports, snapshot) now correctly pass raw objects to keyv.set/setMany and let Keyv v5 handle serialization natively. But refresh-timeseries.ts:41 still wraps values in JSON.stringify(minimal) before passing to setMany. And the timeseries route (route.ts:57) still manually JSON.parses the result.

This means timeseries data gets double-serialized: once by the explicit JSON.stringify, then again by Keyv's built-in @keyv/serialize. On read, Keyv deserializes the outer layer, returning a JSON string that the route then has to JSON.parse again. Should align with the other modules by passing raw objects and removing the manual parse on the route side.

---

Stale PR description (timeseries consolidation was reverted)

Per the author's comment, the timeseries key consolidation (one key per vault instead of per-label) was reverted due to namespace issues. However, the PR description and risk section still reference this change:

"Consolidates timeseries data into one key per vault (all labels together) instead of one key per label"

"Redis key format changed for timeseries: Keys go from timeseries:{label}:{chainId}:{address} to timeseries:{chainId}:{address}"

These should be removed/updated to avoid confusing reviewers.

---

Re: TTL on cache keys

Responding to the author's question about adding a 16-minute TTL — the existing stale-while-revalidate on the HTTP responses is a CDN/browser-level cache setting, not Redis-level, so it wouldn't protect against stale Redis data if refresh scripts fail. However, adding a Redis TTL could make things worse: if a key expires before the next refresh cycle, routes would return 404 instead of slightly stale data. Serving old data is generally preferable to serving nothing.

---

Nit: Not actually a singleton

The PR summary says "single shared Keyv instance" but cache.ts exports a factory function createKeyvClient() that creates a new Keyv connection each time it's called. Every route file and refresh script creates its own instance at module level. This is correct architecturally (shared config, separate connections), but the description is slightly misleading.

---

Summary

Issue	Severity	File
console.timeEnd label mismatch	Bug	list/refresh.ts
connectionTimeoutMillis 50s	Likely typo	db/index.ts
Timeseries double serialization	Inconsistency	timeseries/refresh-timeseries.ts, timeseries/.../route.ts
PR description stale after timeseries revert	Docs	PR description
"Singleton" description	Nit	PR description

The core refactor (shared factory, setMany batching, removing manual JSON parse) is a clean improvement. The items above are mostly quick fixes.

[murderteeth]

Reiterating from my earlier review — the test plan should be updated to make this PR easier to review. Specifically:

• Step-by-step commands a reviewer can copy-paste to verify locally (including Redis setup)
• Actual endpoint URLs with example parameters to curl after running refresh scripts
• Before/after Upstash metrics — since reducing method calls and bandwidth is the primary motivation, seeing actual numbers from a test instance would go a long way

[murderteeth]

From my earlier review — the list key format breaking change still needs to be addressed. The @keyv/redis upgrade from v4 to v5 likely changes how namespaces are applied to keys. Old keys may look like list:vaults::1 (double colon) while the new version may produce list:vaults:1 (single colon). If so, old cached data becomes orphaned and endpoints return 404 until the next refresh cycle.

Can you verify by checking the actual keys in Redis before and after? Something like:

# before merging, check current key format
redis-cli --scan --pattern 'list:vaults*'

If the format did change, please document the rollout procedure in the PR description (e.g. run refresh scripts immediately after deploy, or flush old keys).

[matheus1lva]

```shell
redis-cli --scan --pattern 'list:vaults*'

Yeah i saw a space in between the keys in redis insigh, but didnt know about this command, ill try that!

[murderteeth]

Three items from the previous review still need attention before this can merge:

1. connectionTimeoutMillis: 50_000 (packages/web/app/api/db/index.ts:17) — The 10x increase from 5s to 50s was flagged as a likely typo and hasn't been addressed. If intentional, please explain the reasoning.

2. Key format change — Can you confirm whether the keyv v5 upgrade changes the key format in Redis? (e.g. list:vaults::1 vs list:vaults:1). If it does, please document a rollout procedure in the PR description so endpoints don't return 404 between deploy and the next refresh cycle.

3. Upstash before/after metrics — Since reducing method calls and bandwidth is the primary motivation, it would help to see actual numbers from a test run before merging.

[matheus1lva]

Yep, i got side tracked by some personal stuff on friday, gonna go back into this!

[matheus1lva]

1 - typo commited
2 - formats didnt change

redis-cli --scan --pattern 'list:vaults*'
list:vaults::250
list:vaults::1
list:vaults::747474
list:vaults::80094
list:vaults::8453
list:vaults::10
list:vaults::137
list:vaults::146
list:vaults::34443
list:vaults::42161
list:vaults::100
list:vaults::all

vs live:

list:vaults::1
list:vaults::10
list:vaults::100
list:vaults::137
list:vaults::146
list:vaults::250
list:vaults::34443
list:vaults::42161
list:vaults::747474
list:vaults::80094
list:vaults::8453
list:vaults::all

3 -

same refresh script, same fresh instance. Storage is lagged behind, some stats don't update neither in a "soft real time" manner.

[matheus1lva]

The previous value was double json parsed,

Which now is plain json, which might be also more optimized.

[murderteeth]

is this a breaking change then? if the format changes

[matheus1lva]

Yeah that's a fair point, im adding to the thread. But i also wanna confirm some stats before. I created a new instance and set the keys as it is rn as plain json, ill let it sit for an hour to get the most accurate stats.

[murderteeth]

same refresh script, same fresh instance. Storage is lagged behind, some stats don't update neither in a "soft real time" manner.

did the branch numbers ever update? i'm concerned the storage size is so much smaller

[matheus1lva]

ok after more than an hour and two attempts, i got somewhat decent updated stats. They are not very reliable when you need to get consistent usage report.

But yeah im gonna update the pr to stat ethe breaking change, it is required given that decreased the size, not much, but did. Summed all together decreases everything.