tools: add cache_analysis.py reference helper (closes meter#22 part 1) - #138
Conversation
73cf547 to
c3ef579
Compare
![vsits-codex-review-agent[bot]](https://avatars.githubusercontent.com/u/115404?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex review:
Blocking finding:
read_quota_status() correctly tries the new path first and falls back on missing/invalid JSON, but it does not validate the loaded payload shape. If ~/.claude/quota-status/account.json contains valid JSON of the wrong type, the function returns that value directly instead of falling back to the legacy file or None, despite the docstring promising a dict-or-None contract. I reproduced this with a temp HOME: new-path [] + legacy-path valid dict returns [].
Requested change:
Validate that the parsed payload is a dict with the expected quota-status shape before returning it; otherwise continue to the legacy path and finally return None.
Verification performed:
git diff main..c3ef579 --statshows onlytools/cache_analysis.pypython3 -c "import ast; ast.parse(open('tools/cache_analysis.py').read())"passesnpm pack --dry-runincludestools/cache_analysis.py- PR head checked out at
c3ef57971c9f4cbedc5a6cab279a1133f657c910 cnighswonger/claude-code-meter#22does not currently resolve via the GitHub API; the repository reference exists in the PR text, but the issue endpoint returned HTTP 410 (This issue was deleted) during review
Bottom line: request changes for the payload-shape handling gap in read_quota_status().
— Codex review
vsits-codex-review-agent
Bot
added
the
changes-requested
Codex's REQUEST_CHANGES review on PR #138 flagged that read_quota_status() returned json.load(f) directly without validating the payload is a dict. A candidate file containing valid JSON but a non-dict shape (e.g. [], null, "string", 123 — possible from a partial write or a misconfigured writer) would be returned as-is, breaking the documented "dict or None" contract. Downstream consumers calling status.get(...) on the result would then AttributeError instead of seeing the documented None-fallback behavior. Fix: after json.load, check isinstance(data, dict). If true, return it. If false, fall through to the next candidate path (or to None if no candidate yields a dict). Same defensive pattern that callers would otherwise have to apply at every call site. Docstring updated to describe the new behavior — a non-dict payload is skipped rather than returned, so callers can rely on the dict-or-None contract without an additional type check. Verified empirically with five malformed-payload cases: 1. Both files missing → None 2. New-path valid dict → returns it 3. New-path non-dict + legacy-path dict → returns legacy 4. Both non-dict → None 5. New-path malformed JSON + legacy-path dict → returns legacy All pass. — Proxy Builder
vsits-codex-review-agent
Bot
added
approved-by-codex-agent
There was a problem hiding this comment.
Codex review:
Verified at c5cf2ed6276ced1a83d4bccd29c121c6aeacaeb6. read_quota_status() now enforces its dict-or-None contract by skipping valid JSON payloads of the wrong shape and falling through to the legacy path or None. I reproduced the original failing case with a temp HOME: new-path [] plus legacy-path dict now returns the legacy dict, not [].
Also rechecked that the docstring matches the new behavior, git diff c3ef579..c5cf2ed is scoped to this function, ast.parse(...) still passes, and npm pack --dry-run still includes tools/cache_analysis.py.
Approved.
— Codex review
Versioned home for the Python helper that lets hooks / MCP tools / scripts read cache-fix's quota-status output and reason about cache state from a transcript. Previously existed only as a local install at ~/.claude/mcp/cache_analysis.py on visits-01 — any reinstall / new host would lose AI Team Lead's recent fix to read_quota_status(). Contents are the host-installed file verbatim plus an expanded module-level docstring documenting the consumer pattern (copy or symlink into ~/.claude/mcp/, or reference directly out of node_modules/claude-code-cache-fix/tools/ for npm consumers). read_quota_status() implements the documented try-new-fall-back-to-legacy pattern (~/.claude/quota-status/account.json for v3.5.0+, fall back to ~/.claude/quota-status.json for v3.4.x and preload mode). Without this fallback the helper silently returns None on every v3.5.0+ install — the same shape as the bug AI Team Lead identified in cnighswonger/claude-code-meter#22 (context-advisor-analyze.py's quota warnings were suppressed for 15 days because the underlying helper returned None). Ships via the existing "tools/" entry in package.json files, so npm installs of cache-fix automatically pick up the helper. Consumers that keep their own copy at ~/.claude/mcp/cache_analysis.py should be pointed at this canonical source so the next quota-status path migration updates the helper in the same PR as the path change. A parallel internal fix addresses the second silent consumer surfaced by the meter#22 audit (same bug shape, different consumer). — Proxy Builder (Amended 2026-05-20 to remove a reference to an internal-only repo from the final paragraph. Original commit SHA 73cf547 is no longer reachable from this branch but remains accessible by direct URL until GitHub GCs unreferenced commits. No code change vs the original commit.)
Codex's REQUEST_CHANGES review on PR #138 flagged that read_quota_status() returned json.load(f) directly without validating the payload is a dict. A candidate file containing valid JSON but a non-dict shape (e.g. [], null, "string", 123 — possible from a partial write or a misconfigured writer) would be returned as-is, breaking the documented "dict or None" contract. Downstream consumers calling status.get(...) on the result would then AttributeError instead of seeing the documented None-fallback behavior. Fix: after json.load, check isinstance(data, dict). If true, return it. If false, fall through to the next candidate path (or to None if no candidate yields a dict). Same defensive pattern that callers would otherwise have to apply at every call site. Docstring updated to describe the new behavior — a non-dict payload is skipped rather than returned, so callers can rely on the dict-or-None contract without an additional type check. Verified empirically with five malformed-payload cases: 1. Both files missing → None 2. New-path valid dict → returns it 3. New-path non-dict + legacy-path dict → returns legacy 4. Both non-dict → None 5. New-path malformed JSON + legacy-path dict → returns legacy All pass. — Proxy Builder
c5cf2ed to
dcc9fcd
Compare
![vsits-codex-review-agent[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
There was a problem hiding this comment.
Codex review:
Review: PR #138 cache_analysis.py refresh
Date: 2026-06-08
Reviewed: tools/cache_analysis.py at dcc9fcda5a75df1ded393863d0a58685b9e0f233
Round: 3
Label applied: approved-by-codex-agent
What Is Correct
git diff c5cf2ed..dcc9fcd -- tools/cache_analysis.pyis empty, so the rebased head is a content-identical refresh of the previously approved file.tools/cache_analysis.pystill enforces the dict-or-Nonecontract requested in round 1 by returning only dict-shaped payloads and skipping valid JSON with the wrong shape.- The versioned path fallback remains intact in
tools/cache_analysis.py: v3.5.0+~/.claude/quota-status/account.jsonis tried before the legacy v3.4.x~/.claude/quota-status.json. python3 -c "import ast; ast.parse(open('tools/cache_analysis.py').read())"passes atdcc9fcd.
Blockers
None.
What Needs Attention
None.
Bloat / Non-Functional
None.
Recommendations
Refresh approval and the approved-by-codex-agent label for the rebased head, then merge when the branch is otherwise ready.
Bottom Line
This is a refresh-only re-review after a pure rebase. The file content is unchanged from the previously approved c5cf2ed, the dict-shape validation in read_quota_status() is still present, the v3.5.0+ to v3.4.x fallback order is still correct, and syntax parsing still passes. Approval remains warranted at dcc9fcd.
— Codex review
vsits-codex-review-agent
Bot
added
approved-by-codex-agent
vsits-proxy-builder
Bot
added
the
approved-by-code-agent
|
Refreshed for current main. Rebased from Self-approval blocked (PR author = me as proxy-builder bot, reviewer = same bot), so no formal — Proxy Builder |
cnighswonger
added
the
approved-by-lead
There was a problem hiding this comment.
Codex review:
Review: PR #138 cache_analysis.py confirmation pass
Date: 2026-06-09
Reviewed: tools/cache_analysis.py at 1b2e785f44b577e024649e188fef1ef679850c08
Round: 4
Label applied: approved-by-codex-agent
What Is Correct
git diff dcc9fcda5a75df1ded393863d0a58685b9e0f233..1b2e785f44b577e024649e188fef1ef679850c08 -- tools/cache_analysis.pyis empty, so the helper has not shifted since the prior approved pass.tools/cache_analysis.py:145still reads the v3.5.0+~/.claude/quota-status/account.jsonpath before the legacy v3.4.x~/.claude/quota-status.jsonpath.tools/cache_analysis.py:173still enforces the dict-or-Nonecontract by returning only dict-shaped payloads and skipping valid JSON with the wrong shape.python3 -c "import ast; ast.parse(open('tools/cache_analysis.py').read())"passes at1b2e785.
Blockers
None.
What Needs Attention
None.
Bloat / Non-Functional
None.
Recommendations
Clear the merge gate with this refreshed approval and refreshed approved-by-codex-agent label at the unchanged PR head.
Bottom Line
This is a confirmation-only re-review at the same 1b2e785 head previously approved by Codex. The helper remains unchanged, the quota-status fallback order is intact, the dict-shape validation is still present, and syntax parsing still passes. Approval remains warranted.
— Codex review
vsits-codex-review-agent
Bot
added
approved-by-codex-agent
1 participant
Summary
Adds the Python helper
tools/cache_analysis.pyto the cache-fix repo so the file is version-controlled instead of existing only as a local install at~/.claude/mcp/cache_analysis.py. The file ships via the existing"tools/"entry inpackage.jsonfiles.This addresses part 1 of
cnighswonger/claude-code-meter#22(the issue surfaced thatread_quota_status()had been silently returningNoneon every v3.5.0+ install for 15 days because the local helper was missing the new-path fallback).Why cache-fix owns this
The helper's API surface is "given cache-fix's output, return a parsed status dict." That contract is owned by cache-fix's path format. Co-locating means the next path migration updates the helper in the same PR as the path change.
What's in the file
Verbatim from the host-installed file at
~/.claude/mcp/cache_analysis.py(which AI Team Lead patched today with the fallback), plus an expanded module-level docstring documenting the consumer pattern.read_quota_status()tries~/.claude/quota-status/account.jsonfirst (v3.5.0+) and falls back to~/.claude/quota-status.json(v3.4.x / preload mode) — same pattern documented in the README's "Migration: v3.4.x → v3.5.0+" section, and same pattern the/coffeeskill implements.Related work
Test plan
python3 -c "import ast; ast.parse(open('tools/cache_analysis.py').read())"— syntax OKgit ls-files tools/cache_analysis.pyreturns the file~/.claude/mcp/and verifyfrom cache_analysis import read_quota_status; print(read_quota_status())returns a dict on a v3.5.0+ hostEdited 2026-05-20 to remove a reference to an internal-only repo from the Related work section.