Skip to content

feat(review): add hit/miss telemetry to the repo-culture-profile cache (#4509) - #4601

Merged
JSONbored merged 1 commit into
mainfrom
fix/repo-culture-profile-cache-telemetry-4509
Jul 10, 2026
Merged

feat(review): add hit/miss telemetry to the repo-culture-profile cache (#4509)#4601
JSONbored merged 1 commit into
mainfrom
fix/repo-culture-profile-cache-telemetry-4509

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • readCachedCultureProfile's cache (a signal_snapshots-backed TTL, plus a merged-PR-count drift invalidation) works correctly — but its hit/miss branch was completely unlabeled, unlike every sibling cache in this codebase (ai_review, ai_slop, linked_issue_satisfaction, miner_detection), one of the six capability gaps issue feat(stats): AI-feature cache/reuse-efficiency rate, tracked over time, public #4448 identified.
  • Adds an incr()/recordAuditEvent pair on both the hit path (inside extractRepoCultureProfile, right where a non-null cached result is returned) and the miss/re-derive path, mirroring the ai_review cache's instrumentation exactly (event names follow the same github_app.X_cache_hit/X_cache_miss convention, metric names follow gittensory_X_cache_hit_total/_miss_total).
  • The single hit/miss branch point correctly counts the merged-PR-count drift-invalidation path as a miss too, since readCachedCultureProfile's null return already unifies every invalidation reason (no snapshot, TTL expiry, drift, malformed row) — there's no separate code path to accidentally leave uninstrumented.
  • Purely observability — no functional change to caching behavior, confirmed by the pre-existing 24 cache/invalidation tests passing unchanged.

Closes #4509.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (Closes #4509).

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — src/review/repo-culture-profile.ts is 100% statements/branches/functions/lines in isolation (confirmed via a scoped coverage run), and the precise diff-line cross-reference against the full-suite lcov.info shows zero uncovered lines/branches.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — 4 new tests: a hit invariant (fires only the hit pair), a miss invariant (TTL expiry, fires only the miss pair), a regression test for the drift-invalidation path specifically (confirms it's counted as a miss, not silently uninstrumented), and a fail-safe test (a failing audit-event write on the hit path is swallowed, never surfacing to the caller) — all empirically verified to fail against a reverted (uninstrumented) implementation and pass against the real one.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no such surface touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (N/A — no API/OpenAPI/MCP surface changed.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
  • Visible UI changes include a UI Evidence section below. (N/A — backend-only observability change, no visible UI.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A)

#4509)

readCachedCultureProfile's cache works correctly -- TTL plus a
merged-PR-count drift check -- but emitted no incr()/audit-event pair
distinguishing a hit from a fresh derive, unlike every sibling cache
(ai_review, ai_slop, linked_issue_satisfaction, miner_detection).
Mirrors the ai_review cache's instrumentation exactly; the single
hit/miss branch point in extractRepoCultureProfile correctly counts
the drift-invalidation path as a miss too, since
readCachedCultureProfile's null return already unifies every
invalidation reason.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@JSONbored JSONbored self-assigned this Jul 10, 2026
@JSONbored
JSONbored merged commit da9dba5 into main Jul 10, 2026
7 checks passed
@JSONbored
JSONbored deleted the fix/repo-culture-profile-cache-telemetry-4509 branch July 10, 2026 07:32
@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.09%. Comparing base (6646b87) to head (78eb11f).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4601   +/-   ##
=======================================
  Coverage   94.09%   94.09%           
=======================================
  Files         430      430           
  Lines       38215    38222    +7     
  Branches    13931    13931           
=======================================
+ Hits        35958    35965    +7     
  Misses       1600     1600           
  Partials      657      657           
Files with missing lines Coverage Δ
src/review/repo-culture-profile.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

fix(review): repo-culture-profile cache works correctly but has zero hit/miss telemetry

1 participant