[Birfy]

Closes #269

What

meta.cached_completion memoises paid model calls on disk. Its write path now follows the same best-effort contract as FileCache (#263), TrajectoryStore (#259), Ledger (#265) and runstore (#267): the cache can fail, the paid call cannot be lost.

Hit path

• An entry counts as a hit only after it parsed to a dict whose response is a str. Before, stats["hits"] += 1 ran before json.load, so a truncated entry was counted as both a hit and a miss.
• A well-formed but wrong-shaped entry ({"response": null}, {"response": 7}, {"other": ...}) is a miss instead of returning a non-str value. A JSON list entry used to raise TypeError out of the call (it was not in the except); it is now a miss.

Write path (new _store helper)

• Only str responses are cached (the contract is prompt -> str); anything else is returned uncached.
• Encode with json.dumps before touching the disk, so an unencodable response leaves no temp file.
• mkstemp + os.replace as before; any OSError (disk full, read-only dir, cache dir removed mid-run) is swallowed and the response is still returned.
• finally removes the *.partial sibling if the replace did not happen.

stats keys are unchanged (hits, misses).

Tests

New tests/test_cached_completion.py (13 cases):

• write failure (ENOSPC on write) → paid response returned, no *.partial, no entry
• os.replace failure → no *.partial
• cache dir removed mid-run → no raise
• non-str responses (None, int, dict, object()) → returned uncached, not served from cache
• corrupt entries (truncated, null, int, list, missing key) → miss only, then rewritten and a real hit
• happy path (unicode) still round-trips with {"hits": 2, "misses": 1}

Against main: 12 of 13 fail (only the happy path passes). On this branch: all pass.
tests/test_cached_completion.py + test_metasearch_slots.py + test_meta.py + test_metasearch.py + test_metasearch_algotune.py pass; full suite clean locally (no failures/errors).

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-14T22:20:21.148901Z	a5668f0	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[Birfy]

LGTM

Closes #269 cleanly and matches the encode-first / best-effort write contract from FileCache (#263/#264), TrajectoryStore (#259/#260), Ledger (#265/#266), and runstore (#267/#268) — with the important difference that OSError is swallowed here so a flaky cache never costs the paid completion.

(Self-approve blocked by GitHub; posting LGTM as COMMENT.)

What I verified

Hit path

• Entry is loaded first; stats["hits"] increments only after isinstance(entry, dict) and isinstance(cached, str). A truncated / wrong-shaped / list entry is a miss only (and only a miss).
• A JSON list no longer raises TypeError out of the call (was not covered by the old except (OSError, ValueError, KeyError)).
• Non-str response values (null, int, missing key) are misses instead of being returned as hits and violating prompt -> str.

Write path (_store)

• Only str responses are cached; anything else is returned uncached (paid call still returned to the caller).
• json.dumps runs before mkstemp, so an unencodable payload leaves no *.partial.
• mkstemp + write blob + os.replace; any OSError is swallowed; response is still returned.
• tmp = None after successful replace; finally removes the sibling on every failure path (secondary OSError on remove ignored).
• Missing cache dir mid-run no longer raises after the paid call.

Design fit

• Same encode-first + always-clean-tmp shape as the recent on-disk cache fixes.
• Swallowing write OSError (unlike runstore status writes) is the right contract for a memoisation layer in front of a paid LLM call — the cache is best effort; the response is not.

Tests

• New tests/test_cached_completion.py: 13 collected.
• Against main (tests checked out on main code): 12 failed, 1 passed (only happy path).
• On this branch (a5668f0): 13 passed.
• Related meta suite green locally: test_meta + test_metasearch + test_metasearch_slots + test_metasearch_algotune.
• Smoke: test_cached_completion + test_evalcache + test_runstore_cli green (Python 3.13).

Docs

• CHANGELOG Fixed entry accurately covers write-failure / no-partial / encode-first / corrupt-entry miss-only behaviour and cites #269.

CI

GitHub Actions matrix still pending at review time (build + test 3.9–3.13). Local coverage is solid for this change.

Non-blocking nits

1. Secondary OSError on os.remove(tmp) is swallowed silently — same as FileCache / Ledger / runstore; fine for this contract.
2. After isinstance(response, str), the json.dumps TypeError/ValueError path is essentially unreachable for ordinary strings (belt-and-suspenders). Harmless and consistent with the other caches.
3. If _store fails after a miss that rewrote a corrupt entry, the bad file remains and the next call pays again — correct best-effort behaviour; worth keeping in mind for long meta runs on a full disk.

No merge from this review. Ready once CI goes green.

[Birfy]

Automated review — LGTM ✅ (Closes #269)

Correctness

• Hit is counted only after the entry parsed to dict with a str response; fixes the double hit+miss on a truncated entry and the uncaught TypeError on a JSON-list entry (json.load(...)["response"] on a list).
• _store encodes first (json.dumps), so an unencodable response never creates a temp file; OSError from mkstemp/write/os.replace is swallowed and the paid response is still returned; tmp = None after a successful replace, so finally only removes a temp that was not published.
• Non-str responses are not cached — consistent with the prompt -> str contract, and they cannot become "sticky" wrong-shaped hits.
• stats keys unchanged → existing test_metasearch_slots assertions hold.

Verification (local, Python 3 venv)

• tests/test_cached_completion.py: 12/13 fail on main, 13/13 pass on branch.
• test_cached_completion + test_metasearch_slots + test_meta + test_metasearch + test_metasearch_algotune green; full suite ran clean (no F/E).
• CI: pending at time of review.

Nits (non-blocking)

• If os.fdopen itself raised (practically never), the raw fd would leak; the temp file is still removed. Could wrap with os.close(fd) on that path.
• A secondary os.remove failure in finally is silent — matches the FileCache/runstore precedent.