Skip to content

Commit dde80af

Browse files
authored
fix(dashboard): window the slop/duplicate trend read by time, not a 2-per-week row cap (#9803)
`SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT` was `WEEKS * 2 = 16` on the false premise that queue-health snapshots are written twice a week. They are written up to once per repo per generate-signal-snapshots run (several per day), so the row cap kept only the most recent ~4 days and the 8-week trend card was almost entirely empty. Add an optional `sinceIso` to `listRecentSignalSnapshotsForTargets`, applied as `AND generated_at >= ?` INSIDE the windowed subquery so row_number() ranks over the time-bounded set (not the whole table); the route passes an 8-week bound derived from the request's generatedAt. `maxPerTarget` stays a hard backstop, re-budgeted to WEEKS * 28 (clamped to the query's 100 ceiling). Omitting sinceIso is byte-identical to today. Closes #9699
1 parent afb3b84 commit dde80af

5 files changed

Lines changed: 77 additions & 5 deletions

File tree

src/api/routes.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ import { loadAllKnobStatuses } from "../services/knob-loosening-run";
320320
import { loadPublicReuseRateTrend } from "../services/public-reuse-rate-trend";
321321
import { loadPublicReviewVolumeTrend } from "../services/public-review-volume-trend";
322322
import { buildMaintainerQualityDashboard, isMaintainerQualityDataStale } from "../services/maintainer-quality-dashboard";
323-
import { buildMaintainerSlopDuplicateTrend, SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT } from "../services/maintainer-slop-duplicate-trend";
323+
import { buildMaintainerSlopDuplicateTrend, SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT, SLOP_DUPLICATE_TREND_WEEKS } from "../services/maintainer-slop-duplicate-trend";
324324
import { buildFederatedBenchmark } from "../orb/federated-benchmark";
325325
import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest";
326326
import { buildGateOutcomeBreakdown, GATE_OUTCOME_BREAKDOWN_WINDOW_DAYS } from "../services/gate-outcome-breakdown";
@@ -1347,11 +1347,15 @@ export function createApp() {
13471347
const scopedSyncCompletions = allSyncStates.filter((state) => qualityRepoNames.has(state.repoFullName.toLowerCase())).map((state) => state.lastCompletedAt);
13481348
const generatedAt = nowIso();
13491349
const qualityStale = isMaintainerQualityDataStale({ lastCompletedAts: scopedSyncCompletions, repoCount: qualityRepos.length, nowMs: Date.parse(generatedAt) });
1350+
// Bound the read to the trend card's own 8-week window (#9699) so the row-count cap is a backstop, not the
1351+
// primary limit — otherwise the ranking kept only the most recent few days and the card covered ~4 days.
1352+
const slopTrendSinceIso = new Date(Date.parse(generatedAt) - SLOP_DUPLICATE_TREND_WEEKS * 7 * 86_400_000).toISOString();
13501353
const queueHealthHistoriesByRepo = await listRecentSignalSnapshotsForTargets(
13511354
c.env,
13521355
"queue-health",
13531356
qualityRepos.map((repo) => repo.fullName),
13541357
SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT,
1358+
slopTrendSinceIso,
13551359
);
13561360
const slopDuplicateTrend = buildMaintainerSlopDuplicateTrend({
13571361
repos: qualityRepoInputs.map((input) => {

src/db/repositories.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6060,10 +6060,15 @@ export async function listRecentSignalSnapshotsForTargets(
60606060
signalType: string,
60616061
targetKeys: readonly string[],
60626062
maxPerTarget = 16,
6063+
sinceIso?: string,
60636064
): Promise<Map<string, SignalSnapshotRecord[]>> {
60646065
const result = new Map<string, SignalSnapshotRecord[]>();
60656066
if (targetKeys.length === 0) return result;
60666067
const perTargetLimit = Math.max(1, Math.min(maxPerTarget, 100));
6068+
// The time bound (#9699) is applied INSIDE the windowed subquery so row_number() ranks over the in-window
6069+
// set, not the whole table — otherwise a repo with many recent snapshots could rank its cap entirely within
6070+
// the last few days and never surface the older weeks the trend card needs. maxPerTarget stays the backstop.
6071+
const sinceClause = sinceIso ? " AND generated_at >= ?" : "";
60676072
for (let i = 0; i < targetKeys.length; i += SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH) {
60686073
const batch = targetKeys.slice(i, i + SIGNAL_SNAPSHOT_TARGET_KEY_SQL_BATCH);
60696074
const placeholders = batch.map(() => "?").join(", ");
@@ -6080,13 +6085,13 @@ export async function listRecentSignalSnapshotsForTargets(
60806085
payload_json,
60816086
row_number() OVER (PARTITION BY target_key ORDER BY generated_at DESC, rowid DESC) AS snapshot_rank
60826087
FROM signal_snapshots
6083-
WHERE signal_type = ? AND target_key IN (${placeholders})
6088+
WHERE signal_type = ? AND target_key IN (${placeholders})${sinceClause}
60846089
)
60856090
WHERE snapshot_rank <= ?
60866091
ORDER BY target_key, generated_at DESC
60876092
`,
60886093
)
6089-
.bind(signalType, ...batch, perTargetLimit)
6094+
.bind(signalType, ...batch, ...(sinceIso ? [sinceIso] : []), perTargetLimit)
60906095
.all<{
60916096
id: string;
60926097
signal_type: string;

src/services/maintainer-slop-duplicate-trend.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@ import { isoWeekStart } from "./public-quality-metrics";
77
// Public-safe: band labels and observable rates only — never raw slop-risk or credibility numbers.
88

99
export const SLOP_DUPLICATE_TREND_WEEKS = 8;
10-
/** Max queue-health snapshots read per repo when shaping the maintainer trend card — two per week of history. */
11-
export const SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT = SLOP_DUPLICATE_TREND_WEEKS * 2;
10+
/** Row-count backstop for the maintainer trend card's per-repo snapshot read. queue-health snapshots are
11+
* written up to ~once per repo per generate-signal-snapshots run (several per day), so an 8-week window can
12+
* hold far more than two rows/week — a `* 2` cap silently truncated the history to its most recent few days.
13+
* The `sinceIso` time bound is the primary constraint now; this cap is a safety limit, budgeted generously
14+
* (up to daily-plus rows across the window) and clamped to the query's own 100 ceiling (#9699). */
15+
export const SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT = SLOP_DUPLICATE_TREND_WEEKS * 28;
1216
const MS_PER_WEEK = 7 * 86_400_000;
1317
const MIN_OPEN_PRS_FOR_RATE = 1;
1418
const SLOP_BAND_LOW_MAX_PCT = 25;

test/unit/data-spine.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,4 +700,33 @@ describe("listRecentSignalSnapshotsForTargets (#2202 — bulk recent-per-target
700700
expect(result.get("owner/b")?.map((row) => row.id)).toEqual(["qh-b"]);
701701
expect(result.has("owner/c")).toBe(false);
702702
});
703+
704+
it("applies sinceIso inside the windowed subquery so only in-window rows are ranked and returned (#9699)", async () => {
705+
const env = createTestEnv();
706+
for (const [id, generatedAt] of [
707+
["old-1", "2026-01-01T00:00:00.000Z"], // before the boundary
708+
["old-2", "2026-01-15T00:00:00.000Z"], // before the boundary
709+
["new-1", "2026-03-01T00:00:00.000Z"], // in window
710+
["new-2", "2026-03-10T00:00:00.000Z"], // in window
711+
] as const) {
712+
await persistSignalSnapshot(env, { id, signalType: "queue-health", targetKey: "owner/a", payload: {}, generatedAt });
713+
}
714+
// A generous per-target cap: without the time bound all four rows would return; the sinceIso must drop the two old ones.
715+
const result = await listRecentSignalSnapshotsForTargets(env, "queue-health", ["owner/a"], 100, "2026-02-01T00:00:00.000Z");
716+
expect(result.get("owner/a")?.map((row) => row.id)).toEqual(["new-2", "new-1"]);
717+
});
718+
719+
it("is byte-identical to today when sinceIso is omitted (#9699)", async () => {
720+
const env = createTestEnv();
721+
for (const [id, generatedAt] of [
722+
["a", "2026-01-01T00:00:00.000Z"],
723+
["b", "2026-02-01T00:00:00.000Z"],
724+
] as const) {
725+
await persistSignalSnapshot(env, { id, signalType: "queue-health", targetKey: "owner/a", payload: {}, generatedAt });
726+
}
727+
const withUndefined = await listRecentSignalSnapshotsForTargets(env, "queue-health", ["owner/a"], 16);
728+
const withExplicitUndefined = await listRecentSignalSnapshotsForTargets(env, "queue-health", ["owner/a"], 16, undefined);
729+
expect(withUndefined.get("owner/a")?.map((r) => r.id)).toEqual(["b", "a"]);
730+
expect(withExplicitUndefined.get("owner/a")?.map((r) => r.id)).toEqual(["b", "a"]);
731+
});
703732
});

test/unit/maintainer-slop-duplicate-trend.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { buildQueueHealth, buildCollisionReport } from "../../src/signals/engine
44
import {
55
buildMaintainerSlopDuplicateTrend,
66
slopBandLabelFromRate,
7+
SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT,
78
SLOP_DUPLICATE_TREND_WEEKS,
89
trendPointFromQueueHealth,
910
} from "../../src/services/maintainer-slop-duplicate-trend";
@@ -113,6 +114,35 @@ describe("buildMaintainerSlopDuplicateTrend", () => {
113114
expect(JSON.stringify(trend)).not.toMatch(FORBIDDEN_PUBLIC_TERMS);
114115
});
115116

117+
it("budgets the read cap for at least daily-plus snapshots across the window, not two per week (#9699)", () => {
118+
// The cap must cover far more than the old "two per week" premise, so the sinceIso time bound (not the row
119+
// cap) is what limits the read. At least WEEKS * 28 (or clamped to the query's 100 ceiling).
120+
expect(SLOP_DUPLICATE_TREND_SNAPSHOT_LIMIT).toBeGreaterThanOrEqual(Math.min(SLOP_DUPLICATE_TREND_WEEKS * 28, 100));
121+
});
122+
123+
it("populates at least 6 weekly buckets from 40 daily snapshots spanning 8 weeks (#9699)", () => {
124+
// Before #9699 the read cap (SLOP_DUPLICATE_TREND_WEEKS * 2 = 16) kept only the most recent ~2 weeks of
125+
// these daily snapshots, so at most 1-2 buckets ever populated. With the full 8-week set fed through, the
126+
// builder must spread them across the window.
127+
const generatedAt = "2026-06-29T12:00:00.000Z";
128+
const nowMs = Date.parse(generatedAt);
129+
const snapshots = Array.from({ length: 40 }, (_, i) =>
130+
queueHealthSnapshot(new Date(nowMs - i * 86_400_000).toISOString(), {
131+
openPullRequests: 10,
132+
slopFlaggedPullRequests: 2,
133+
duplicateFlaggedPullRequests: 1,
134+
}),
135+
);
136+
const trend = buildMaintainerSlopDuplicateTrend({
137+
generatedAt,
138+
stale: false,
139+
nowMs,
140+
repos: [{ repoFullName: "octo/demo", queueHealthSnapshots: snapshots }],
141+
});
142+
const withRate = trend.weeks.filter((week) => week.slopFlagRatePct !== null);
143+
expect(withRate.length).toBeGreaterThanOrEqual(6);
144+
});
145+
116146
it("returns null slop series when snapshots lack slop counts but still shapes duplicate series", () => {
117147
const trend = buildMaintainerSlopDuplicateTrend({
118148
generatedAt: "2026-06-14T12:00:00.000Z",

0 commit comments

Comments
 (0)