[devorun]

Problem

The success-tracking call sites in src/cli.js read the cache flag off the payload:

from_cache: !!result?.fromCache

Nothing ever sets fromCache at the top level. getCachedResponse() records the hit one level down, on _meta:

_meta: { ...cached.data._meta, fromCache: true, cacheAge: Math.round(age) }

So result.fromCache was undefined on every command and from_cache reported a miss for every event — including responses that never touched the network. grep -rn fromCache src shows the whole story: one writer, on _meta, and two readers that look somewhere else.

The alerts list --table path had the same metric broken a second way: it never passed from_cache at all.

Why not just read _meta.fromCache

That was the first fix I tried, and it is not enough. Handlers are free to rebuild their result, and some do — alerts list extracts the array out of the response and filters it into a fresh one (src/commands/alerts.js:588), so any marker carried on the body is gone before tracking runs. A test driving alerts list --table through the real cache still reported a miss.

Cache-hit state is therefore recorded on the API instance, for exactly the reason lastResponseMeta already lives there — the class comment on that field says it "survives any reshaping a command handler does to the response body". servedFromCache is set next to the early return in request() and read at the tracking sites, so it works regardless of what a handler does to the payload.

Fix

• src/api.js — this.servedFromCache is set true beside the cache-hit early return in request() and false on the path that goes to the network. Declared in the constructor next to lastResponseMeta, with the same last-write-wins semantics when a handler makes several calls.
• src/cli.js — the three tracking sites that report on a cacheable response now read api.servedFromCache. The two sites that cannot involve the cache are left alone: the result === undefined branch (the command wrote its own output) and schema (served locally).

Tests

New src/__tests__/telemetry-from-cache.test.js drives runCLI end to end with the real NansenAPI, a mocked fetch and a temp HOME, running the same --cache command twice and asserting on the tracked event. It covers all three touched paths — default output, --stream, and alerts list --table.

Reverting the fix fails all three, which is what the bug looks like from the outside:

× reports the cache hit on the second identical --cache call
× reports the cache hit on the --stream output path
× reports the cache hit on the alerts list --table path
AssertionError: expected false to be true
AssertionError: expected false to be true
AssertionError: expected undefined to be false
Tests  3 failed (3)

Each test also asserts fetch was called once across the two runs, so it is a genuine cache hit being measured rather than a second network call.

With the fix: Test Files 1 passed (1), Tests 3 passed (3).

npm run lint passes clean.

On the full npm test run this branch produces the same failure set as unmodified main — identical files, identical line numbers, identical counts — with 2640 passing against 2637 on main, the difference being exactly the three new tests. No existing test changes status.

Note

This touches src/api.js in different places than my open PR for the cached-array shape, so the two are independent and do not conflict.

Checklist

• npm test — no test changes status vs main; the new file passes 3/3
• npm run lint passes
• New code path has tests
• No console.log in core, no hardcoded secrets
• Changeset added (patch, user-facing fix)
• src/schema.json — not applicable, no commands or options changed

[nansen-pr-reviewer]

pr-reviewer Summary for #7825eef

✅ No issues found

The code review completed successfully with no findings.

Review effort: 2/5 (Simple)

Summary

This PR correctly fixes a long-standing telemetry bug where from_cache always reported false. The approach — storing cache state on the API instance rather than the response payload — directly mirrors the existing lastResponseMeta pattern and is the right design for surviving handler reshaping.

Production changes reviewed:

• src/api.js — this.servedFromCache = false is set in the constructor (safe default before any call), true on the cache-hit early-return path, and false on the fall-through to the network. The placement of the false assignment (after the cache-check block, before the retry loop) is correct: a cache miss sets it to false immediately, and any network failure path that throws will leave the field accurately reflecting the miss for that request. No issues.

• src/cli.js — All three cacheable tracking sites are updated consistently. The two non-cacheable sites (result === undefined / schema) are intentionally left without from_cache, which is correct. No issues.

• src/__tests__/telemetry-from-cache.test.js — End-to-end with a real NansenAPI and mocked fetch. The vi.resetModules() + vi.stubGlobal('fetch', …) pattern is consistent with the repo's other integration-style tests. All three affected output paths (default, --stream, alerts list --table) are covered, and the fetch call-count assertion confirms a genuine cache hit rather than a second network call.

• Changeset — Present, correct package name (nansen-cli), correct semver bump (patch).

No issues found. This is a clean, targeted fix.

---

Token usage: 4,093 input, 2,766 output, 240,180 cache read, 34,542 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

[nansen-pr-reviewer]

Auto-approved

This PR was automatically approved because:

• Claude recommends approval
• Claude assessed this as a moderate effort change
• The effort level is within the auto-approval threshold of 2
• No high or critical issues were detected

If you have any concerns, please request a manual review.