feat(templates): add 24h TTL with stale-cache fallback to gallery cache (BE-3393)#559
feat(templates): add 24h TTL with stale-cache fallback to gallery cache (BE-3393)#559mattmillerai wants to merge 2 commits into
Conversation
mattmillerai
added
agent-coded
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
dosubot
Bot
added
size:M
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 7 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 2 |
| 🟡 Medium | 2 |
| 🟢 Low | 3 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
…(BE-3393) Address cursor-review findings on the 24h-TTL gallery cache: - Validate the fetched body as JSON *before* writing the cache, so a 200 with a non-JSON body (rate-limit HTML, captive portal, truncated response) can no longer clobber the last-known-good cache with garbage. - Persist atomically via a temp file + os.replace so a concurrent `templates` reader never sees a half-written index. - Make the cache write best-effort: a read-only cache dir / full disk no longer breaks the command once valid data is in hand. - Route a non-200 status (RuntimeError from _fetch_gallery) through the same stale-cache fallback / gallery_load_failed path instead of letting it escape as an uncaught traceback (shared _GALLERY_LOAD_ERRORS tuple). - Guard the stale-fallback read: if the cache vanished/corrupted under us, surface the original fetch error rather than the read error. - Treat a future mtime (clock skew) as stale so the cache can't be pinned "fresh" indefinitely. Deferred: serve-stale-while-revalidate so offline machines don't block on the 15s fetch each call past the TTL (needs background-refresh design). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dosubot
Bot
added
size:L
|
🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:
|
1 participant
ELI-5
comfy templates ls/showreads its catalog from a small JSON file cached at~/.cache/comfy-cli/gallery/index.json. Today, once that file exists it is served forever — the only way to update it iscomfy templates refresh, which nothing calls automatically. So agents browse a frozen template catalog (the 2026-07-16 dogfooding incident). This adds a 24h expiry: a cache older than a day is silently re-fetched on the nextls, and if the machine is offline the stale cache is still served (with a warning) so browsing never breaks.What changed
GALLERY_TTL_SECONDS = 24 * 60 * 60next toGALLERY_URL. 24h (not the spec's 7 days) — the gallery updates weekly-ish and the fetch is one tiny JSON file, so a tighter TTL is cheap insurance against a frozen catalog._load_gallerynow serves the cache only when it exists and is within the TTL. Past the TTL it re-fetches and rewrites the cache. If that TTL-triggered fetch fails (urllib.error.URLError/OSError), it falls back to the stale cache with a non-fatal renderer warning (stderr in JSON mode) — offline users keep a workingtemplates ls(exit 0)._cache_is_stale(mtime vs TTL) and_cache_age_str(human-friendly age for the warning).refresh_cmdis unchanged — it stays the manual force-refresh.All three
_load_gallerycallers (ls,show,fetch) get the TTL behavior transparently.Behavior preserved (Chesterton's fence)
The stale-fallback applies only to the TTL-triggered auto-refresh. An explicit
--refreshor a genuinely absent cache still surfaces a fetch error and exits non-zero, exactly as before — the user who asked to refresh (or has no cache at all) learns it failed rather than getting silently-stale data. A stale cache never triggered a fetch before this change, so the fetch-failure-on-stale path is brand-new, not a changed existing path.Tests
Added to
tests/comfy_cli/command/test_templates.py:_fetch_gallerymonkeypatched to fail loudly)gallery_load_failed)--refresh+ network failure → fatal, no silent stale fallbackFull suite green locally (2578 passed, 37 skipped);
ruff check+ruff formatclean on the touched files.Negative-claim falsification
N/A — this change adds a resilience capability (list offline from stale cache) rather than denying one. No deny/dead-end path, no "not supported" strings; the only new user-facing message is a non-fatal warning.