fix(review): bound impact-map query cache retention - #4992
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4992 +/- ##
===========================================
+ Coverage 74.88% 94.34% +19.45%
===========================================
Files 473 473
Lines 39928 39932 +4
Branches 14557 14557
===========================================
+ Hits 29902 37674 +7772
+ Misses 8235 1583 -6652
+ Partials 1791 675 -1116
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-12 08:35:44 UTC
⏸️ Suggested Action - Manual Review
Review summary Blockers
Nits — 5 non-blocking
Concerns raised — review before merging
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Evicts expired impact-map query-cache rows on read-miss and proactively deletes stale rows before every insert, both scoped by (project, repo, fetched_at). Stops persisting the retrieved context blob, keeping only metrics_json. The new index lands as its own migration (0145) rather than editing the already-applied 0132: D1 does not re-run a migration's SQL once it's recorded as applied, so an edit to 0132 would never reach a deployed database.
891d7f1 to
92b8641
Compare
Motivation
contextblobs in the cache sincecomputeImpactMaponly needsmetrics.pathsfor rendering.Description
DELETE ... WHERE fetched_at < ?inputCachedImpactMapQueryand deleting the expired single-row on stalegetCachedImpactMapQuerymisses insrc/review/impact-map.ts.contextbody for impact-map entries and write an emptycontextpayload while preservingmetrics_jsoninputCachedImpactMapQueryso per-entry storage cost is minimized.(project, repo, fetched_at)index in the raw migration and Drizzle schema so repo-scoped stale-row deletes are efficient (migrations/0132_impact_map_query_cache.sql,src/db/schema.ts).cachingStorageStuband add two regression unit tests that assert expired rows are evicted and cached rows keep onlymetrics(nocontext) intest/unit/impact-map.test.ts.cf-typegencheck (worker-configuration.d.ts).Testing
npx vitest run test/unit/impact-map.test.tsand all impact-map tests passed (24 tests passed).npm run db:schema-drift:check,npm run cf-typegen:check,npm run typecheck, andnpm run db:migrations:check, all of which succeeded locally.npm run test:ci; early checks completed but the full suite did not finish cleanly in this environment (long-running unrelated queue coverage runs surfaced and the run was not completed here).npm audit --audit-level=moderatecould not complete in this environment due to the npm audit endpoint returning403 Forbidden.Codex Task