get/dc: resolve a dc: handle exactly once per run

[jqnatividad]

Fixes #4257.

The problem

diskcache::resolve_dc_path is not a pure path lookup. Past TTL it calls get_resource — a network fetch — and on success swaps in a refreshed entry with a different BLAKE3, hence a different materialized CSV.

Nothing resolved a dc: input once per run and shared the result. Four call sites resolved independently:

Location	Caller
src/config.rs:203	Config::new — so every Config built from a dc: input resolves again
src/util.rs:2790	process_input
src/util.rs:3226	get_stats_records
src/util.rs:3634	get_stats_records_readonly

Because the first is inside Config::new, the count scales with how many Configs a command builds — viz smart builds seven. Measured on this branch's test harness: qsv frequency dc:x resolved 4 times in one run.

If the TTL boundary is crossed mid-run and the source actually changed, the reader's CSV, the row count, the frequency counts and the stats cache can each come from a different blob — and nothing detects or reports it. The page renders normally and looks authoritative.

The fix — Option 2 from the issue (per-run memoization)

A (cache_dir, name)-keyed memo in diskcache::rich where the first resolution wins for the life of the process.

• The outer lock is held only long enough to hand out a per-handle slot, so distinct handles never block each other.
• The slot lock is held across resolution, so two concurrent first-callers on the same handle cannot both refresh and end up with different blobs — the very divergence being fixed.
• Successes only — a failed resolution stays unmemoized and is retried, matching today's behavior.
• No signature changes, so all four call sites are untouched. viz's own resolve-once threading from viz: bundle the sidecars a --dict-info dashboard was built from as downloads #4256 is also left as-is: it becomes belt-and-braces rather than load-bearing, and it keeps that PR's provenance guarantee explicit.

Process-wide memoization equals per-run memoization because qsv is strictly one-command-per-process (main.rs dispatches once and exits; the MCP server spawns a fresh binary per tool call). That assumption is stated in the doc comment so a future in-process command loop revisits it.

The trap: side effects are not all idempotent

resolve_dc_path does three kinds of disk work. CSV materialization (need_write-guarded) and .idx materialization are idempotent. The stats-sidecar capture is not, by design — it captures the .stats.csv.data.jsonl sidecar into a durable content-addressed blob only once it is fresher than the CSV, which is only true after util::get_stats_records has written it. It relies on being called again later in the run.

Memoizing the whole function would have silently pushed durable .stats.jsonl.zst capture to a subsequent run. So it is split out into sync_stats_sidecar, which still runs on every call — purely local filesystem work, no network, keyed on the memoized blake3/ext/csv_path. Verified: a single frequency dc:… run still takes the blob count 0 → 1.

Behavior note

With RefreshPolicy::Always, a handle now re-fetches once per run rather than once per resolution. That is the intended fix — one snapshot per run is the point — but it is a real behavior change, and it is called out in the doc comment.

Test

get_dc_resolves_once_per_run seeds a --ttl 0 (permanently stale) entry, then runs frequency dc:… and asserts exactly one conditional revalidation against the mock server.

• Written first and confirmed failing at revalidations() == 4 before the fix, so it is provably non-vacuous.
• Uses the states.csv endpoint, which advertises no Cache-Control: max-age — otherwise http-cache-reqwest would short-circuit the conditional GET and the pre-fix count would read low (a false green).
• revalidations is the discriminator; body_sends == 1 only confirms the 304s never became re-downloads.
• Deliberately ungated, and uses frequency. qsvdp has get without feature_capable, so a schema-based test would have to be feature_capable-gated, leaving GetWebServer::revalidations() unused on qsvdp — a dead-code warning and a -D warnings build break. That helper is ungated from get_cloud for the same reason. Confirmed the test compiles and passes under -F datapusher_plus.

Verification

• test_get 49 passed, test_frequency 179, test_schema 12, test_stats 684 — all green.
• get_dc_stats_cache_for_smart_commands still passes, confirming the sync_stats_sidecar split preserved capture-on-use.
• cargo clippy --bin qsv -F all_features clean (two pre-existing warnings, in untouched files).
• cargo +nightly fmt applied.
• Manual: join and cat rows over two distinct dc: handles in one run — per-handle keying, no cross-talk.

No docopt/USAGE changes, so no help regeneration needed.

Not addressed here

The issue's Not covered here section — whether resolve_dc_path doing network I/O implicitly from inside Config::new is itself the thing to revisit. The memo means only the first resolution fetches, but it does not move the fetch out of the constructor. That is closer to Option 1 and belongs in its own issue.

🤖 Generated with Claude Code

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}