diff --git a/src/api/routes.ts b/src/api/routes.ts index b23cecbac..aa475e850 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -320,7 +320,7 @@ import { loadAllKnobStatuses } from "../services/knob-loosening-run"; import { loadPublicReuseRateTrend } from "../services/public-reuse-rate-trend"; import { loadPublicReviewVolumeTrend } from "../services/public-review-volume-trend"; import { buildMaintainerQualityDashboard, isMaintainerQualityDataStale } from "../services/maintainer-quality-dashboard"; -import { buildMaintainerSlopDuplicateTrend, SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT } from "../services/maintainer-slop-duplicate-trend"; +import { buildMaintainerSlopDuplicateTrend, SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT, SLOP_DUPLICATE_TREND_WEEKS } from "../services/maintainer-slop-duplicate-trend"; import { buildFederatedBenchmark } from "../orb/federated-benchmark"; import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest"; import { buildGateOutcomeBreakdown, GATE_OUTCOME_BREAKDOWN_WINDOW_DAYS } from "../services/gate-outcome-breakdown"; @@ -1347,11 +1347,15 @@ export function createApp() { const scopedSyncCompletions = allSyncStates.filter((state) => qualityRepoNames.has(state.repoFullName.toLowerCase())).map((state) => state.lastCompletedAt); const generatedAt = nowIso(); const qualityStale = isMaintainerQualityDataStale({ lastCompletedAts: scopedSyncCompletions, repoCount: qualityRepos.length, nowMs: Date.parse(generatedAt) }); + // Bound the read to the trend card's own 8-week window (#9699) so the row-count cap is a backstop, not the + // primary limit — otherwise the ranking kept only the most recent few days and the card covered ~4 days. + const slopTrendSinceIso = new Date(Date.parse(generatedAt) - SLOP_DUPLICATE_TREND_WEEKS * 7 * 86_400_000).toISOString(); const queueHealthHistoriesByRepo = await listRecentSignalSnapshotsForTargets( c.env, "queue-health", qualityRepos.map((repo) => repo.fullName), SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT, + slopTrendSinceIso, ); const slopDuplicateTrend = buildMaintainerSlopDuplicateTrend({ repos: qualityRepoInputs.map((input) => { diff --git a/src/db/repositories.ts b/src/db/repositories.ts index ec9bed2e7..f5ff10c5f 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -6060,10 +6060,15 @@ export async function listRecentSignalSnapshotsForTargets( signalType: string, targetKeys: readonly string[], maxPerTarget = 16, + sinceIso?: string, ): Promise> { const result = new Map(); if (targetKeys.length === 0) return result; const perTargetLimit = Math.max(1, Math.min(maxPerTarget, 100)); + // The time bound (#9699) is applied INSIDE the windowed subquery so row_number() ranks over the in-window + // set, not the whole table — otherwise a repo with many recent snapshots could rank its cap entirely within + // the last few days and never surface the older weeks the trend card needs. maxPerTarget stays the backstop. + const sinceClause = sinceIso ? " AND generated_at >= ?" : ""; for (let i = 0; i < targetKeys.length; i += SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH) { const batch = targetKeys.slice(i, i + SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH); const placeholders = batch.map(() => "?").join(", "); @@ -6080,13 +6085,13 @@ export async function listRecentSignalSnapshotsForTargets( payload_json, row_number() OVER (PARTITION BY target_key ORDER BY generated_at DESC, rowid DESC) AS snapshot_rank FROM signal_snapshots - WHERE signal_type = ? AND target_key IN (${placeholders}) + WHERE signal_type = ? AND target_key IN (${placeholders})${sinceClause} ) WHERE snapshot_rank <= ? ORDER BY target_key, generated_at DESC `, ) - .bind(signalType, ...batch, perTargetLimit) + .bind(signalType, ...batch, ...(sinceIso ? [sinceIso] : []), perTargetLimit) .all<{ id: string; signal_type: string; diff --git a/src/services/maintainer-slop-duplicate-trend.ts b/src/services/maintainer-slop-duplicate-trend.ts index f00733024..ffe4814a8 100644 --- a/src/services/maintainer-slop-duplicate-trend.ts +++ b/src/services/maintainer-slop-duplicate-trend.ts @@ -7,8 +7,12 @@ import { isoWeekStart } from "./public-quality-metrics"; // Public-safe: band labels and observable rates only — never raw slop-risk or credibility numbers. export const SLOP_DUPLICATE_TREND_WEEKS = 8; -/** Max queue-health snapshots read per repo when shaping the maintainer trend card — two per week of history. */ -export const SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT = SLOP_DUPLICATE_TREND_WEEKS * 2; +/** Row-count backstop for the maintainer trend card's per-repo snapshot read. queue-health snapshots are + * written up to ~once per repo per generate-signal-snapshots run (several per day), so an 8-week window can + * hold far more than two rows/week — a `* 2` cap silently truncated the history to its most recent few days. + * The `sinceIso` time bound is the primary constraint now; this cap is a safety limit, budgeted generously + * (up to daily-plus rows across the window) and clamped to the query's own 100 ceiling (#9699). */ +export const SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT = SLOP_DUPLICATE_TREND_WEEKS * 28; const MS_PER_WEEK = 7 * 86_400_000; const MIN_OPEN_PRS_FOR_RATE = 1; const SLOP_BAND_LOW_MAX_PCT = 25; diff --git a/test/unit/data-spine.test.ts b/test/unit/data-spine.test.ts index 826689267..54aba53fd 100644 --- a/test/unit/data-spine.test.ts +++ b/test/unit/data-spine.test.ts @@ -700,4 +700,33 @@ describe("listRecentSignalSnapshotsForTargets (#2202 — bulk recent-per-target expect(result.get("owner/b")?.map((row) => row.id)).toEqual(["qh-b"]); expect(result.has("owner/c")).toBe(false); }); + + it("applies sinceIso inside the windowed subquery so only in-window rows are ranked and returned (#9699)", async () => { + const env = createTestEnv(); + for (const [id, generatedAt] of [ + ["old-1", "2026-01-01T00:00:00.000Z"], // before the boundary + ["old-2", "2026-01-15T00:00:00.000Z"], // before the boundary + ["new-1", "2026-03-01T00:00:00.000Z"], // in window + ["new-2", "2026-03-10T00:00:00.000Z"], // in window + ] as const) { + await persistSignalSnapshot(env, { id, signalType: "queue-health", targetKey: "owner/a", payload: {}, generatedAt }); + } + // A generous per-target cap: without the time bound all four rows would return; the sinceIso must drop the two old ones. + const result = await listRecentSignalSnapshotsForTargets(env, "queue-health", ["owner/a"], 100, "2026-02-01T00:00:00.000Z"); + expect(result.get("owner/a")?.map((row) => row.id)).toEqual(["new-2", "new-1"]); + }); + + it("is byte-identical to today when sinceIso is omitted (#9699)", async () => { + const env = createTestEnv(); + for (const [id, generatedAt] of [ + ["a", "2026-01-01T00:00:00.000Z"], + ["b", "2026-02-01T00:00:00.000Z"], + ] as const) { + await persistSignalSnapshot(env, { id, signalType: "queue-health", targetKey: "owner/a", payload: {}, generatedAt }); + } + const withUndefined = await listRecentSignalSnapshotsForTargets(env, "queue-health", ["owner/a"], 16); + const withExplicitUndefined = await listRecentSignalSnapshotsForTargets(env, "queue-health", ["owner/a"], 16, undefined); + expect(withUndefined.get("owner/a")?.map((r) => r.id)).toEqual(["b", "a"]); + expect(withExplicitUndefined.get("owner/a")?.map((r) => r.id)).toEqual(["b", "a"]); + }); }); diff --git a/test/unit/maintainer-slop-duplicate-trend.test.ts b/test/unit/maintainer-slop-duplicate-trend.test.ts index 5e4e0cbc5..c26b28e19 100644 --- a/test/unit/maintainer-slop-duplicate-trend.test.ts +++ b/test/unit/maintainer-slop-duplicate-trend.test.ts @@ -4,6 +4,7 @@ import { buildQueueHealth, buildCollisionReport } from "../../src/signals/engine import { buildMaintainerSlopDuplicateTrend, slopBandLabelFromRate, + SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT, SLOP_DUPLICATE_TREND_WEEKS, trendPointFromQueueHealth, } from "../../src/services/maintainer-slop-duplicate-trend"; @@ -113,6 +114,35 @@ describe("buildMaintainerSlopDuplicateTrend", () => { expect(JSON.stringify(trend)).not.toMatch(FORBIDDEN_PUBLIC_TERMS); }); + it("budgets the read cap for at least daily-plus snapshots across the window, not two per week (#9699)", () => { + // The cap must cover far more than the old "two per week" premise, so the sinceIso time bound (not the row + // cap) is what limits the read. At least WEEKS * 28 (or clamped to the query's 100 ceiling). + expect(SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT).toBeGreaterThanOrEqual(Math.min(SLOP_DUPLICATE_TREND_WEEKS * 28, 100)); + }); + + it("populates at least 6 weekly buckets from 40 daily snapshots spanning 8 weeks (#9699)", () => { + // Before #9699 the read cap (SLOP_DUPLICATE_TREND_WEEKS * 2 = 16) kept only the most recent ~2 weeks of + // these daily snapshots, so at most 1-2 buckets ever populated. With the full 8-week set fed through, the + // builder must spread them across the window. + const generatedAt = "2026-06-29T12:00:00.000Z"; + const nowMs = Date.parse(generatedAt); + const snapshots = Array.from({ length: 40 }, (_, i) => + queueHealthSnapshot(new Date(nowMs - i * 86_400_000).toISOString(), { + openPullRequests: 10, + slopFlaggedPullRequests: 2, + duplicateFlaggedPullRequests: 1, + }), + ); + const trend = buildMaintainerSlopDuplicateTrend({ + generatedAt, + stale: false, + nowMs, + repos: [{ repoFullName: "octo/demo", queueHealthSnapshots: snapshots }], + }); + const withRate = trend.weeks.filter((week) => week.slopFlagRatePct !== null); + expect(withRate.length).toBeGreaterThanOrEqual(6); + }); + it("returns null slop series when snapshots lack slop counts but still shapes duplicate series", () => { const trend = buildMaintainerSlopDuplicateTrend({ generatedAt: "2026-06-14T12:00:00.000Z",