diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index fdab8e0cf8..f4b3d9c101 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -495,6 +495,33 @@ "accuracyPct" ] } + }, + "reuseRateTrend": { + "type": "array", + "items": { + "type": "object", + "properties": { + "weekStart": { + "type": "string" + }, + "hits": { + "type": "number" + }, + "misses": { + "type": "number" + }, + "reuseRatePct": { + "type": "number", + "nullable": true + } + }, + "required": [ + "weekStart", + "hits", + "misses", + "reuseRatePct" + ] + } } }, "required": [ @@ -503,7 +530,8 @@ "totals", "weekly", "byProject", - "accuracyTrend" + "accuracyTrend", + "reuseRateTrend" ] }, "PublicQualityMetrics": { diff --git a/src/api/routes.ts b/src/api/routes.ts index b5e6c22afe..f6293414b6 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -260,6 +260,7 @@ import { computePredictedGateAgreement } from "../review/predicted-gate-agreemen import { isRagEnabled } from "../review/rag-wire"; import { getPublicStats, isPublicStatsEnabled } from "../review/public-stats"; import { loadPublicAccuracyTrend } from "../services/public-accuracy-trend"; +import { loadPublicReuseRateTrend } from "../services/public-reuse-rate-trend"; import { buildMaintainerQualityDashboard, isMaintainerQualityDataStale } from "../services/maintainer-quality-dashboard"; import { MAX_LOCAL_SCORER_WARNING_CHARS, MAX_LOCAL_SCORER_WARNING_COUNT } from "../signals/local-scorer-diagnostics"; import { compileFocusManifestPolicy, MAX_FOCUS_MANIFEST_BYTES, normalizeReadinessGateMode } from "../signals/focus-manifest"; @@ -958,9 +959,9 @@ export function createApp() { app.get("/v1/public/stats", async (c) => { if (!isPublicStatsEnabled(c.env)) return c.json({ error: "not_found" }, 404); try { - const [stats, accuracyTrend] = await Promise.all([getPublicStats(c.env), loadPublicAccuracyTrend(c.env)]); + const [stats, accuracyTrend, reuseRateTrend] = await Promise.all([getPublicStats(c.env), loadPublicAccuracyTrend(c.env), loadPublicReuseRateTrend(c.env)]); c.header("Cache-Control", "public, max-age=60, stale-while-revalidate=300"); - return c.json({ ...stats, accuracyTrend }); + return c.json({ ...stats, accuracyTrend, reuseRateTrend }); } catch { return c.json({ error: "public_stats_unavailable" }, 503); } diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index a1ebb0d0fc..48e4a4c849 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -131,6 +131,19 @@ export const PublicStatsSchema = z accuracyPct: z.number().nullable(), }), ), + /** Trailing weekly "how often we avoid redoing AI work" trend (#4448) -- a competence signal, not a cost + * claim. Counts cache hits/misses across every instrumented AI-touching capability (grounding, + * review-memory, impact-map, repo-culture-profile, ai_review, ai_slop, linked_issue_satisfaction, + * miner_detection). null reuseRatePct on a week means too few total attempts to publish a meaningful + * percentage, not zero reuse. */ + reuseRateTrend: z.array( + z.object({ + weekStart: z.string(), + hits: z.number(), + misses: z.number(), + reuseRatePct: z.number().nullable(), + }), + ), }) .openapi("PublicStats"); diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 6bc66e39cd..c83e6abc66 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7546,7 +7546,7 @@ export async function runAiReviewForAdvisory( patch: typeof file.payload?.patch === "string" ? file.payload.patch : undefined, })), ); - impactMapEntries = await computeImpactMap(changedSymbols, { + impactMapEntries = await computeImpactMap(env, changedSymbols, { infra: createReviewAdapters(env), project: impactMapProject, repo: impactMapRepo, diff --git a/src/review/impact-map.ts b/src/review/impact-map.ts index faac53f02b..36ca4057db 100644 --- a/src/review/impact-map.ts +++ b/src/review/impact-map.ts @@ -9,6 +9,8 @@ // FAIL-SAFE (mirrors rag.ts's own guarantee): a missing/cold RAG index, no changed symbols, or any retrieval // error degrades to an EMPTY impact map — this computation can never break or block a review. +import { recordAuditEvent } from "../db/repositories"; +import { incr } from "../selfhost/metrics"; import { sha256Hex } from "../utils/crypto"; import { nowIso } from "../utils/json"; import type { FileChangedSymbols } from "./impact-symbols"; @@ -129,10 +131,12 @@ async function putCachedImpactMapQuery( * retrieval error yields an EMPTY impact map, never a throw. */ export async function computeImpactMap( + env: Env, symbols: FileChangedSymbols[], ragContext: { infra: RagInfra; project: string; repo: string }, ): Promise { const out: ImpactMapEntry[] = []; + const targetKey = ragContext.project ? `${ragContext.project}/${ragContext.repo}` : ragContext.repo; // Symbol-less files never query (nothing to look up) and so never count against the cap below -- filter // them out first so the cap applies to the actual query budget, not a raw slice of the input. const queryableFiles = symbols.filter((file) => file.symbols.length > 0).slice(0, MAX_IMPACT_MAP_INPUT_FILES); @@ -148,8 +152,26 @@ export async function computeImpactMap( const cached = await getCachedImpactMapQuery(ragContext.infra.storage, ragContext.project, ragContext.repo, fingerprint); let result: RagRetrievalResult; if (cached !== null) { + // #4448: mirrors repo-culture-profile's #4509 cache hit/miss instrumentation exactly -- one of the six + // AI-touching capabilities that had no reuse-rate signal at all before this. + incr("gittensory_impact_map_cache_hit_total"); + await recordAuditEvent(env, { + eventType: "github_app.impact_map_cache_hit", + targetKey, + outcome: "completed", + detail: "reused a cached impact-map query result instead of re-querying the vector index", + metadata: { repoFullName: targetKey }, + }).catch(() => undefined); result = cached; } else { + incr("gittensory_impact_map_cache_miss_total"); + await recordAuditEvent(env, { + eventType: "github_app.impact_map_cache_miss", + targetKey, + outcome: "completed", + detail: "no reusable cached impact-map query result; querying the vector index fresh", + metadata: { repoFullName: targetKey }, + }).catch(() => undefined); result = await retrieveContextWithMetrics(ragContext.infra, { project: ragContext.project, repo: ragContext.repo, diff --git a/src/services/public-reuse-rate-trend.ts b/src/services/public-reuse-rate-trend.ts new file mode 100644 index 0000000000..8c0008d933 --- /dev/null +++ b/src/services/public-reuse-rate-trend.ts @@ -0,0 +1,111 @@ +// Public "AI-work reuse rate" weekly trend (#4448, part of epic #4445). An honest engineering-competence +// number, not a cost claim: how often the review engine correctly reused a prior result instead of redoing the +// same work -- across every AI-touching capability that has a cache to hit or miss (grounding, review-memory, +// impact-map, repo-culture-profile, ai_review, ai_slop, linked_issue_satisfaction, miner_detection). Deliberately +// NOT a cost/token-rate metric (out of scope per the parent epic). +// +// DELIBERATELY NOT a persisted/cron rollup, mirroring #4447's own public-accuracy-trend.ts design: audit_events +// is already durable, so a live weekly re-bucketing of the SAME rows can recompute any historical week correctly +// on every request -- no cron-miss gap risk, and no second copy of the number to keep in sync. +// +// DELIBERATELY GLOBAL, not scoped to the public-stats repo allowlist: unlike accuracy/handled-PR counts, a +// cache-hit/miss event carries no PR content, author, or repo-specific outcome -- the aggregate reuse rate +// doesn't reveal anything about any one repo's activity, and target_key isn't uniformly shaped across all eight +// capabilities (some key by bare repoFullName, others by repoFullName#prNumber), so allowlist-filtering it would +// need a fragile per-capability parser for no real privacy benefit. +// +// NAMING CONVENTION, not a hardcoded capability list: every instrumented capability already follows +// `github_app._cache_hit` / `github_app._cache_miss` (confirmed via a full-repo grep before writing +// this), so a single LIKE-pattern query picks up all eight today AND any future capability that follows the +// same convention, with zero code change here. ai_review's three additional REUSE variants (frozen/paused/ +// one-shot) don't fit that exact suffix -- each is a genuine "skipped a redundant AI call" event, so they're +// folded into "hit" alongside the plain ai_review_cache_hit. +import { safeAll } from "../review/public-stats"; +import { isoWeekStart } from "./public-quality-metrics"; + +export const PUBLIC_REUSE_RATE_TREND_WEEKS = 8; +/** Below this many total attempts (hits+misses) in a week, that week's reuse rate is too noisy to publish. */ +export const MIN_REUSE_RATE_TREND_SAMPLE = 5; + +/** ai_review reuse events that don't follow the `_cache_hit` suffix convention but are the SAME "avoided a + * redundant AI call" signal -- each one means the review pass reused a prior state instead of re-running. */ +const AI_REVIEW_REUSE_EVENT_TYPES = ["github_app.ai_review_frozen_reuse", "github_app.ai_review_paused_reuse", "github_app.ai_review_one_shot_reuse"] as const; + +export type PublicReuseRateTrendWeek = { + /** UTC Monday (YYYY-MM-DD) that starts the bucket. */ + weekStart: string; + hits: number; + misses: number; + reuseRatePct: number | null; +}; + +type DayRow = { day: string; hits: number; misses: number }; + +const MS_PER_WEEK = 7 * 86_400_000; + +function roundPct(value: number): number { + return Math.round(value * 1000) / 10; +} + +function reuseRatePctOf(hits: number, misses: number): number | null { + const attempts = hits + misses; + if (attempts < MIN_REUSE_RATE_TREND_SAMPLE) return null; + return roundPct(hits / attempts); +} + +/** Fold day-granularity rows into `weeks` trailing UTC-Monday buckets ending in the week containing `nowMs`. + * Pure -- mirrors buildPublicAccuracyTrend's own bucketing shape (public-accuracy-trend.ts, #4447). */ +export function buildPublicReuseRateTrend(dayRows: DayRow[], nowMs: number, weeks: number = PUBLIC_REUSE_RATE_TREND_WEEKS): PublicReuseRateTrendWeek[] { + const currentStartMs = Date.parse(isoWeekStart(nowMs)); + const oldestStartMs = currentStartMs - (weeks - 1) * MS_PER_WEEK; + const buckets = Array.from({ length: weeks }, () => ({ hits: 0, misses: 0 })); + + for (const row of dayRows) { + const dayMs = Date.parse(`${row.day}T00:00:00.000Z`); + if (!Number.isFinite(dayMs)) continue; + const weekOffset = Math.floor((dayMs - oldestStartMs) / MS_PER_WEEK); + if (weekOffset < 0 || weekOffset >= weeks) continue; + const bucket = buckets[weekOffset]!; + bucket.hits += row.hits; + bucket.misses += row.misses; + } + + return buckets.map((bucket, offset) => ({ + weekStart: isoWeekStart(oldestStartMs + offset * MS_PER_WEEK), + hits: bucket.hits, + misses: bucket.misses, + reuseRatePct: reuseRatePctOf(bucket.hits, bucket.misses), + })); +} + +/** Day-bucketed hit/miss counts across every `github_app._cache_hit` / `_cache_miss` event, plus + * ai_review's three non-suffix-conforming reuse variants (see file header). Fail-safe: degrades to [] on any + * query error (safeAll), yielding under-counted weeks rather than throwing the whole public stats payload. */ +async function loadReuseRateDayRows(env: Env, sinceIso: string): Promise { + const reuseTypePlaceholders = AI_REVIEW_REUSE_EVENT_TYPES.map(() => "?").join(", "); + const rows = await safeAll<{ day: string; hits: number; misses: number }>( + env, + `SELECT date(created_at) AS day, + SUM(CASE WHEN event_type LIKE 'github_app.%cache_hit' OR event_type IN (${reuseTypePlaceholders}) THEN 1 ELSE 0 END) AS hits, + SUM(CASE WHEN event_type LIKE 'github_app.%cache_miss' THEN 1 ELSE 0 END) AS misses + FROM audit_events + WHERE (event_type LIKE 'github_app.%cache_hit' OR event_type LIKE 'github_app.%cache_miss' OR event_type IN (${reuseTypePlaceholders})) + AND created_at >= ? + GROUP BY day`, + ...AI_REVIEW_REUSE_EVENT_TYPES, + ...AI_REVIEW_REUSE_EVENT_TYPES, + sinceIso, + ); + /* v8 ignore next -- SUM(CASE WHEN ... THEN 1 ELSE 0 END) over an existing GROUP BY day always yields a + * defined integer (0 or more), never SQL NULL, so the ?? 0 fallback can't currently be exercised; kept for + * defense against a future query-shape change (mirrors public-accuracy-trend.ts's identical guard). */ + return rows.map((row) => ({ day: row.day, hits: row.hits ?? 0, misses: row.misses ?? 0 })); +} + +/** Assemble the public reuse-rate trend from the SAME live audit_events ledger every instrumented capability + * already writes to. */ +export async function loadPublicReuseRateTrend(env: Env, nowMs: number = Date.now()): Promise { + const sinceIso = new Date(Date.parse(isoWeekStart(nowMs)) - (PUBLIC_REUSE_RATE_TREND_WEEKS - 1) * MS_PER_WEEK).toISOString(); + const dayRows = await loadReuseRateDayRows(env, sinceIso); + return buildPublicReuseRateTrend(dayRows, nowMs); +} diff --git a/test/integration/public-stats-route.test.ts b/test/integration/public-stats-route.test.ts index 9dc6927957..764ba5bf56 100644 --- a/test/integration/public-stats-route.test.ts +++ b/test/integration/public-stats-route.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { createApp } from "../../src/api/routes"; import { createTestEnv } from "../helpers/d1"; import { PUBLIC_ACCURACY_TREND_WEEKS } from "../../src/services/public-accuracy-trend"; +import { PUBLIC_REUSE_RATE_TREND_WEEKS } from "../../src/services/public-reuse-rate-trend"; /** Seed the LIVE ledger: a published-review surface per reviewed PR (audit_events) + each PR's terminal * disposition (pull_requests state/merged_at), plus one live reversal (an engine close on a now-reopened PR). */ @@ -62,6 +63,7 @@ describe("GET /v1/public/stats (#1059)", () => { weekly: { reviewed: number; merged: number }; byProject: Array<{ project: string; reviewed: number }>; accuracyTrend: Array<{ weekStart: string; merged: number; closed: number; reversed: number; accuracyPct: number | null }>; + reuseRateTrend: Array<{ weekStart: string; hits: number; misses: number; reuseRatePct: number | null }>; }; expect(body.totals.handled).toBe(5); // distinct reviewed PRs expect(body.totals.merged).toBe(3); @@ -81,5 +83,8 @@ describe("GET /v1/public/stats (#1059)", () => { // #4447: the weekly accuracy trend rides along on the SAME response, one entry per trailing week. expect(body.accuracyTrend).toHaveLength(PUBLIC_ACCURACY_TREND_WEEKS); for (const week of body.accuracyTrend) expect(typeof week.weekStart).toBe("string"); + // #4448: the weekly AI-work reuse-rate trend rides along on the SAME response too. + expect(body.reuseRateTrend).toHaveLength(PUBLIC_REUSE_RATE_TREND_WEEKS); + for (const week of body.reuseRateTrend) expect(typeof week.weekStart).toBe("string"); }); }); diff --git a/test/unit/impact-map.test.ts b/test/unit/impact-map.test.ts index 9445df9e0a..f0565d4550 100644 --- a/test/unit/impact-map.test.ts +++ b/test/unit/impact-map.test.ts @@ -1,10 +1,21 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { computeImpactMap, MAX_AFFECTED_MODULES_PER_ENTRY, MAX_IMPACT_MAP_INPUT_FILES } from "../../src/review/impact-map"; +import * as repositoriesModule from "../../src/db/repositories"; +import { renderMetrics, resetMetrics } from "../../src/selfhost/metrics"; import type { FileChangedSymbols } from "../../src/review/impact-symbols"; import type { InferenceAdapter, RagInfra, StorageAdapter, VectorAdapter } from "../../src/review/rag"; +import { createTestEnv } from "../helpers/d1"; const ai1024: InferenceAdapter = { run: async () => ({ data: [Array(1024).fill(0.1)] }) }; +/** A fresh env per call -- computeImpactMap's cache hit/miss telemetry (#4448) needs a real D1-backed env for + * recordAuditEvent; the pre-existing tests below only assert on the return value, so a throwaway env per call + * (rather than one shared across the whole file) keeps them fully isolated from each other and from the + * dedicated telemetry tests further down. */ +function testEnv(): Env { + return createTestEnv(); +} + /** A bare storage stub: COUNT(*) returns `n` (warm vs cold index); the chunk-text SELECT always answers empty. * Fine for cold-index / no-adapter / no-match cases, where no chunk text is ever read. */ function storageStub(count: number): StorageAdapter { @@ -78,7 +89,7 @@ describe("computeImpactMap", () => { inference: ai1024, }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(result).toEqual([ { changedModule: "src/review/impact-map.ts", affectedModules: ["src/review/caller.ts"], callers: ["computeImpactMap"] }, ]); @@ -92,7 +103,7 @@ describe("computeImpactMap", () => { })); const infra: RagInfra = { storage: storageStubWithText(5), vector: vectorStub(matches), inference: ai1024 }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(result).toHaveLength(1); expect(result[0]?.affectedModules).toHaveLength(MAX_AFFECTED_MODULES_PER_ENTRY); // Deterministic ordering: the highest-scoring match leads (RAG's own retrieval order). @@ -116,7 +127,7 @@ describe("computeImpactMap", () => { path: `src/review/module${i}.ts`, symbols: [`fn${i}`], })); - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(queryCount).toBe(MAX_IMPACT_MAP_INPUT_FILES); expect(result).toHaveLength(MAX_IMPACT_MAP_INPUT_FILES); // Deterministic: the FIRST N input files are kept, not a sample. @@ -142,7 +153,7 @@ describe("computeImpactMap", () => { symbols: [`fn${i}`], })); symbols.splice(1, 0, { path: "README.md", symbols: [] }, { path: "docs/guide.md", symbols: [] }); - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(queryCount).toBe(MAX_IMPACT_MAP_INPUT_FILES); expect(result).toHaveLength(MAX_IMPACT_MAP_INPUT_FILES); }); @@ -156,7 +167,7 @@ describe("computeImpactMap", () => { inference: ai1024, }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(result).toEqual([]); }); @@ -167,7 +178,7 @@ describe("computeImpactMap", () => { inference: ai1024, }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: [] }]; - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(result).toEqual([]); }); @@ -186,13 +197,13 @@ describe("computeImpactMap", () => { } as unknown as VectorAdapter; const infra: RagInfra = { storage: storageStub(5), vector, inference: ai1024 }; const symbols: FileChangedSymbols[] = [{ path: "a.ts", symbols: ["a"] }]; - expect(await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); + expect(await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); expect(queried).toBe(false); }); it("returns an empty impact map for an empty symbol list", async () => { const infra: RagInfra = { storage: storageStub(5), vector: vectorStub([]), inference: ai1024 }; - expect(await computeImpactMap([], { infra, project: "acme", repo: "widgets" })).toEqual([]); + expect(await computeImpactMap(testEnv(), [], { infra, project: "acme", repo: "widgets" })).toEqual([]); }); it("returns an empty impact map when the RAG index is cold (empty-index, fail-safe)", async () => { @@ -202,13 +213,13 @@ describe("computeImpactMap", () => { inference: ai1024, }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - expect(await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); + expect(await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); }); it("returns an empty impact map when no vector/inference adapter is configured (RAG unavailable)", async () => { const infra: RagInfra = { storage: storageStub(5) }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - expect(await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); + expect(await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); }); it("degrades a single file's entry to no-affected-modules when the vector query throws (fail-safe, never blocks the rest)", async () => { @@ -221,7 +232,7 @@ describe("computeImpactMap", () => { } as unknown as VectorAdapter; const infra: RagInfra = { storage: storageStub(5), vector: throwingVector, inference: ai1024 }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - expect(await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); + expect(await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" })).toEqual([]); }); it("computes independent entries for multiple changed files in input order", async () => { @@ -241,7 +252,7 @@ describe("computeImpactMap", () => { { path: "src/review/impact-symbols.ts", symbols: ["extractChangedSymbols"] }, { path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }, ]; - const result = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const result = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(result).toEqual([ { changedModule: "src/review/impact-symbols.ts", affectedModules: ["src/review/x.ts"], callers: ["extractChangedSymbols"] }, { changedModule: "src/review/impact-map.ts", affectedModules: ["src/review/y.ts"], callers: ["computeImpactMap"] }, @@ -269,8 +280,8 @@ describe("computeImpactMap", () => { const infra: RagInfra = { storage, vector: countingVector, inference: countingInference }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - const first = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); - const second = await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + const first = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); + const second = await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(first).toEqual(second); expect(embedCalls).toBe(1); @@ -302,7 +313,7 @@ describe("computeImpactMap", () => { // non-cacheable cooldown -- previously each one re-embedded and re-queried the vector index from scratch. for (let i = 0; i < 5; i += 1) { // eslint-disable-next-line no-await-in-loop -- sequential passes, mirroring separate review invocations - await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); } expect(embedCalls).toBe(1); @@ -331,7 +342,7 @@ describe("computeImpactMap", () => { // The chunk-text lookup (repo_chunks) ALSO throws via this stub, which retrieveContextWithMetrics itself // already degrades fail-safe -- the cache-read throw specifically is what this test targets, so the // resulting entry is empty (no chunk text survives), but the computation must complete, not throw. - await expect(computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" })).resolves.toEqual([]); + await expect(computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" })).resolves.toEqual([]); }); it("a genuinely different query (different changed symbols) still triggers a fresh embed+query, never masked by another file's cached entry", async () => { @@ -347,10 +358,10 @@ describe("computeImpactMap", () => { const { storage } = cachingStorageStub(5); const infra: RagInfra = { storage, vector: countingVector, inference: ai1024 }; - await computeImpactMap([{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }], { infra, project: "acme", repo: "widgets" }); + await computeImpactMap(testEnv(), [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }], { infra, project: "acme", repo: "widgets" }); // A different changed file with different symbols -- a genuinely different query, even though it shares // the same project/repo as the first call. - await computeImpactMap([{ path: "src/review/rag.ts", symbols: ["retrieveContextWithMetrics"] }], { infra, project: "acme", repo: "widgets" }); + await computeImpactMap(testEnv(), [{ path: "src/review/rag.ts", symbols: ["retrieveContextWithMetrics"] }], { infra, project: "acme", repo: "widgets" }); expect(queryCalls).toBe(2); }); @@ -370,9 +381,92 @@ describe("computeImpactMap", () => { const infra: RagInfra = { storage, vector: countingVector, inference: ai1024 }; const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; - await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); - await computeImpactMap(symbols, { infra, project: "acme", repo: "widgets" }); + await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); + await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "widgets" }); expect(queryCalls).toBe(2); }); + + describe("cache hit/miss telemetry (#4448)", () => { + afterEach(() => resetMetrics()); + + async function auditEvent(env: Env, eventType: string, targetKey: string) { + return env.DB.prepare("SELECT outcome, target_key FROM audit_events WHERE event_type = ? AND target_key = ?") + .bind(eventType, targetKey) + .first<{ outcome: string; target_key: string }>(); + } + + it("INVARIANT: a cache HIT fires exactly the hit counter/audit-event pair, and NOT the miss pair", async () => { + const { storage } = cachingStorageStub(5); + const infra: RagInfra = { storage, vector: vectorStub([{ id: "src/review/caller.ts::0", score: 0.9, metadata: { path: "src/review/caller.ts" } }]), inference: ai1024 }; + const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; + + await computeImpactMap(testEnv(), symbols, { infra, project: "acme", repo: "telemetry-widgets" }); // first call: a miss (cold cache) + const env = testEnv(); + resetMetrics(); + + await computeImpactMap(env, symbols, { infra, project: "acme", repo: "telemetry-widgets" }); // same query -- a hit + + const rendered = await renderMetrics(); + expect(rendered).toContain("gittensory_impact_map_cache_hit_total 1"); + expect(rendered).not.toContain("gittensory_impact_map_cache_miss_total"); + const hitEvent = await auditEvent(env, "github_app.impact_map_cache_hit", "acme/telemetry-widgets"); + expect(hitEvent?.outcome).toBe("completed"); + expect(await auditEvent(env, "github_app.impact_map_cache_miss", "acme/telemetry-widgets")).toBeUndefined(); + }); + + it("INVARIANT: a cache MISS fires exactly the miss counter/audit-event pair, and NOT the hit pair", async () => { + const { storage } = cachingStorageStub(5); + const infra: RagInfra = { storage, vector: vectorStub([{ id: "src/review/caller.ts::0", score: 0.9, metadata: { path: "src/review/caller.ts" } }]), inference: ai1024 }; + const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; + const env = testEnv(); + + await computeImpactMap(env, symbols, { infra, project: "acme", repo: "telemetry-widgets-2" }); + + const rendered = await renderMetrics(); + expect(rendered).toContain("gittensory_impact_map_cache_miss_total 1"); + expect(rendered).not.toContain("gittensory_impact_map_cache_hit_total"); + const missEvent = await auditEvent(env, "github_app.impact_map_cache_miss", "acme/telemetry-widgets-2"); + expect(missEvent?.outcome).toBe("completed"); + expect(await auditEvent(env, "github_app.impact_map_cache_hit", "acme/telemetry-widgets-2")).toBeUndefined(); + }); + + it("swallows a failing cache-hit audit-event write without throwing, still returning the cached-derived entries", async () => { + const { storage } = cachingStorageStub(5); + const infra: RagInfra = { storage, vector: vectorStub([{ id: "src/review/caller.ts::0", score: 0.9, metadata: { path: "src/review/caller.ts" } }]), inference: ai1024 }; + const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; + const env = testEnv(); + await computeImpactMap(env, symbols, { infra, project: "acme", repo: "telemetry-widgets-3" }); // populates the cache + + const writeSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockRejectedValueOnce(new Error("D1 write error")); + const second = await computeImpactMap(env, symbols, { infra, project: "acme", repo: "telemetry-widgets-3" }); // a cache hit + writeSpy.mockRestore(); + + expect(second).toHaveLength(1); // the failed audit write never surfaces to the caller + }); + + it("swallows a failing cache-MISS audit-event write without throwing, still returning the freshly-queried entries", async () => { + const { storage } = cachingStorageStub(5); + const infra: RagInfra = { storage, vector: vectorStub([{ id: "src/review/caller.ts::0", score: 0.9, metadata: { path: "src/review/caller.ts" } }]), inference: ai1024 }; + const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; + const env = testEnv(); + + const writeSpy = vi.spyOn(repositoriesModule, "recordAuditEvent").mockRejectedValueOnce(new Error("D1 write error")); + const first = await computeImpactMap(env, symbols, { infra, project: "acme", repo: "telemetry-widgets-4" }); // cold cache -- a miss + writeSpy.mockRestore(); + + expect(first).toHaveLength(1); // the failed audit write never surfaces to the caller, query still happens + }); + + it("REGRESSION: an empty project (a repoFullName with no owner segment, e.g. splitRepoForRag's fallback) targets the bare repo name, not a leading slash", async () => { + const { storage } = cachingStorageStub(5); + const infra: RagInfra = { storage, vector: vectorStub([{ id: "src/review/caller.ts::0", score: 0.9, metadata: { path: "src/review/caller.ts" } }]), inference: ai1024 }; + const symbols: FileChangedSymbols[] = [{ path: "src/review/impact-map.ts", symbols: ["computeImpactMap"] }]; + const env = testEnv(); + + await computeImpactMap(env, symbols, { infra, project: "", repo: "no-owner-repo" }); + + expect(await auditEvent(env, "github_app.impact_map_cache_miss", "no-owner-repo")).toMatchObject({ outcome: "completed" }); + }); + }); }); diff --git a/test/unit/public-reuse-rate-trend.test.ts b/test/unit/public-reuse-rate-trend.test.ts new file mode 100644 index 0000000000..66e936081d --- /dev/null +++ b/test/unit/public-reuse-rate-trend.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "vitest"; +import { + MIN_REUSE_RATE_TREND_SAMPLE, + PUBLIC_REUSE_RATE_TREND_WEEKS, + buildPublicReuseRateTrend, + loadPublicReuseRateTrend, +} from "../../src/services/public-reuse-rate-trend"; +import { isoWeekStart } from "../../src/services/public-quality-metrics"; +import { recordAuditEvent } from "../../src/db/repositories"; +import { createTestEnv } from "../helpers/d1"; + +const NOW = Date.parse("2026-06-22T12:00:00.000Z"); + +describe("buildPublicReuseRateTrend", () => { + it("buckets day rows into weekly totals and computes hits / (hits + misses)", () => { + const currentMonday = isoWeekStart(NOW); + const priorMonday = isoWeekStart(NOW - 7 * 86_400_000); + const trend = buildPublicReuseRateTrend( + [ + { day: priorMonday, hits: 8, misses: 2 }, + { day: priorMonday, hits: 1, misses: 0 }, // a second day in the SAME week -- must accumulate + { day: currentMonday, hits: 5, misses: 5 }, + ], + NOW, + 2, + ); + expect(trend).toHaveLength(2); + expect(trend[0]).toEqual({ weekStart: priorMonday, hits: 9, misses: 2, reuseRatePct: 81.8 }); + expect(trend[1]).toEqual({ weekStart: currentMonday, hits: 5, misses: 5, reuseRatePct: 50 }); + }); + + it("REGRESSION: ignores day rows outside the trailing window instead of letting them corrupt the oldest bucket", () => { + const currentMonday = isoWeekStart(NOW); + const tooOld = isoWeekStart(NOW - 30 * 86_400_000); + const trend = buildPublicReuseRateTrend([{ day: tooOld, hits: 999, misses: 999 }, { day: currentMonday, hits: MIN_REUSE_RATE_TREND_SAMPLE, misses: 0 }], NOW, 2); + expect(trend[0]).toMatchObject({ hits: 0, misses: 0 }); + expect(trend[1]).toMatchObject({ hits: MIN_REUSE_RATE_TREND_SAMPLE, misses: 0 }); + }); + + it("ignores an unparseable day string rather than throwing or corrupting a bucket", () => { + const currentMonday = isoWeekStart(NOW); + const trend = buildPublicReuseRateTrend([{ day: "not-a-date", hits: 5, misses: 5 }, { day: currentMonday, hits: MIN_REUSE_RATE_TREND_SAMPLE, misses: 0 }], NOW, 1); + expect(trend).toHaveLength(1); + expect(trend[0]).toMatchObject({ hits: MIN_REUSE_RATE_TREND_SAMPLE, misses: 0 }); + }); + + it("returns null reuseRatePct (not a misleading 0% or 100%) below MIN_REUSE_RATE_TREND_SAMPLE total attempts", () => { + const week = isoWeekStart(NOW); + const trend = buildPublicReuseRateTrend([{ day: week, hits: MIN_REUSE_RATE_TREND_SAMPLE - 1, misses: 0 }], NOW, 1); + expect(trend[0]?.reuseRatePct).toBeNull(); + }); + + it("returns a real percentage at exactly MIN_REUSE_RATE_TREND_SAMPLE total attempts", () => { + const week = isoWeekStart(NOW); + const trend = buildPublicReuseRateTrend([{ day: week, hits: MIN_REUSE_RATE_TREND_SAMPLE, misses: 0 }], NOW, 1); + expect(trend[0]?.reuseRatePct).toBe(100); + }); + + it("defaults to PUBLIC_REUSE_RATE_TREND_WEEKS trailing weeks when weeks is omitted", () => { + const trend = buildPublicReuseRateTrend([], NOW); + expect(trend).toHaveLength(PUBLIC_REUSE_RATE_TREND_WEEKS); + }); + + it("returns all-zero, null-rate buckets for an empty input (a brand-new / not-yet-enabled deployment)", () => { + const trend = buildPublicReuseRateTrend([], NOW, 3); + expect(trend).toHaveLength(3); + for (const week of trend) expect(week).toMatchObject({ hits: 0, misses: 0, reuseRatePct: null }); + }); +}); + +describe("loadPublicReuseRateTrend — end-to-end over the real live audit_events ledger", () => { + it("counts every github_app.*_cache_hit / *_cache_miss event, plus ai_review's three non-suffix reuse variants, as hits/misses", async () => { + const env = createTestEnv(); + const thisMonday = isoWeekStart(NOW); + const thisWeekIso = `${thisMonday}T09:00:00.000Z`; + + // Two genuinely different instrumented capabilities' hits. + await recordAuditEvent(env, { eventType: "github_app.grounding_cache_hit", targetKey: "owner/repo", outcome: "completed", createdAt: thisWeekIso }); + await recordAuditEvent(env, { eventType: "github_app.impact_map_cache_hit", targetKey: "owner/repo", outcome: "completed", createdAt: thisWeekIso }); + // A miss from a third capability. + await recordAuditEvent(env, { eventType: "github_app.review_memory_cache_miss", targetKey: "owner/repo", outcome: "completed", createdAt: thisWeekIso }); + // All three ai_review reuse variants -- each counts as a "hit" (avoided a redundant AI call). + await recordAuditEvent(env, { eventType: "github_app.ai_review_frozen_reuse", targetKey: "owner/repo#1", outcome: "completed", createdAt: thisWeekIso }); + await recordAuditEvent(env, { eventType: "github_app.ai_review_paused_reuse", targetKey: "owner/repo#2", outcome: "completed", createdAt: thisWeekIso }); + await recordAuditEvent(env, { eventType: "github_app.ai_review_one_shot_reuse", targetKey: "owner/repo#3", outcome: "completed", createdAt: thisWeekIso }); + // An unrelated event type must NOT be counted at all. + await recordAuditEvent(env, { eventType: "github_app.pr_public_surface_published", targetKey: "owner/repo#4", outcome: "completed", createdAt: thisWeekIso }); + + const trend = await loadPublicReuseRateTrend(env, NOW); + const currentWeek = trend[trend.length - 1]; + expect(currentWeek?.weekStart).toBe(thisMonday); + expect(currentWeek?.hits).toBe(5); // grounding_hit + impact_map_hit + 3 ai_review reuse variants + expect(currentWeek?.misses).toBe(1); // review_memory_miss only + }); + + it("returns all-zero buckets when no instrumented events exist yet", async () => { + const env = createTestEnv(); + const trend = await loadPublicReuseRateTrend(env, NOW); + for (const week of trend) expect(week).toMatchObject({ hits: 0, misses: 0, reuseRatePct: null }); + }); +});