From 87323cb26b70de19aedc1828990ecd76b19212f5 Mon Sep 17 00:00:00 2001 From: Marcos Sabatino Date: Sat, 20 Jun 2026 01:52:56 -0300 Subject: [PATCH 1/3] feat(dashboard): compute the Cycle Time verdict from tenant data Replace the hardcoded "lead-time bottlenecks live before/after, not in code execution" sentence in the Cycle Time card with a verdict computed from the tenant's own flow data. The engine already emits per-PR phase medians (time_in_phase_median_hours) and the per-repo page renders them, but the org dashboard still showed a fixed editorial string. This aggregates those phases org-wide (weighted by merged PRs), picks the dominant phase, and renders a stacked phase bar. The verdict only ever describes the code window (PR open -> merge); it never claims anything about the "before" (demand) or "after" (deploy) windows the engine does not measure here. - summarizeFlow / selectCycleTimeVerdict: pure, fully unit-tested (100% lines, ~98% branches on the new module) - coverage-gated: below 60% the card shows a partial sample, not a verdict - aggregated by repo/org only, never per person - no DB migration, no new ingestion, no engine change Co-Authored-By: Claude Opus 4.8 (1M context) --- platform/lib/queries/cycle-time-flow.ts | 178 +++++++++++ platform/lib/queries/org-summary.ts | 14 + platform/lib/translations.ts | 38 ++- .../[tenant]/dashboard/sections/CycleTime.tsx | 147 ++++++++- .../src/components/charts/FlowPhaseBar.tsx | 98 ++++++ platform/src/types/org-summary.ts | 41 +++ platform/tests/cycle-time-flow.test.ts | 290 ++++++++++++++++++ 7 files changed, 788 insertions(+), 18 deletions(-) create mode 100644 platform/lib/queries/cycle-time-flow.ts create mode 100644 platform/src/components/charts/FlowPhaseBar.tsx create mode 100644 platform/tests/cycle-time-flow.test.ts diff --git a/platform/lib/queries/cycle-time-flow.ts b/platform/lib/queries/cycle-time-flow.ts new file mode 100644 index 0000000..9cbcad0 --- /dev/null +++ b/platform/lib/queries/cycle-time-flow.ts @@ -0,0 +1,178 @@ +/** + * Cycle Time — flow decomposition + honest verdict selection. + * + * Pure functions behind the dashboard's Cycle Time card. They turn the + * per-repo phase medians the engine already emits (`time_in_phase_median_hours`) + * into an org-level decomposition and a *computed* verdict — replacing the old + * hardcoded editorial sentence about where bottlenecks "live". + * + * The verdict only ever describes the code window it actually measures + * (PR open -> merge); it never claims anything about the "before" (demand) + * or "after" (deploy) windows, which the engine does not measure here. + * + * No I/O, no per-PR data: fully unit-testable. + */ +import type { + DominantPhase, + FlowDecomposition, + FlowPhaseKey, +} from "@/types/org-summary"; + +/** Canonical phase order (earliest lifecycle stage first). */ +export const FLOW_PHASE_ORDER: readonly FlowPhaseKey[] = [ + "coding", + "awaiting_first_review", + "in_review_active", + "in_review_wait", + "awaiting_merge", +]; + +/** Phases that are *waiting* (queue) rather than *active* (work being done). */ +export const WAIT_PHASES: ReadonlySet = new Set([ + "awaiting_first_review", + "in_review_wait", + "awaiting_merge", +]); + +/** One repo's contribution to the org-level flow aggregation. */ +export interface FlowRow { + /** Merged PRs in the window — the aggregation weight. */ + merged: number; + /** Per-phase median hours for this repo (engine output). */ + phases: Partial>; + ttfrHours?: number | null; + flowEfficiency?: number | null; +} + +function round(value: number, places: number): number { + const factor = 10 ** places; + return Math.round(value * factor) / factor; +} + +/** + * Aggregate per-repo phase medians into an org-level decomposition, weighted + * by merged-PR count. This is a weighted average of per-repo medians — an + * approximation (we never persist per-PR timings, by design), so callers MUST + * gate display on `flowCoveragePct`. Returns null when no row carried phase + * data (e.g. only older payloads in the window). + */ +export function summarizeFlow( + rows: FlowRow[], + totalMerged: number, +): FlowDecomposition | null { + const weighted: Record = { + coding: 0, + awaiting_first_review: 0, + in_review_active: 0, + in_review_wait: 0, + awaiting_merge: 0, + }; + let weight = 0; + let ttfrWeighted = 0; + let ttfrWeight = 0; + let effWeighted = 0; + let effWeight = 0; + + for (const row of rows) { + if (!row.phases || row.merged <= 0) continue; + weight += row.merged; + for (const key of FLOW_PHASE_ORDER) { + weighted[key] += (row.phases[key] ?? 0) * row.merged; + } + if (row.ttfrHours != null) { + ttfrWeighted += row.ttfrHours * row.merged; + ttfrWeight += row.merged; + } + if (row.flowEfficiency != null) { + effWeighted += row.flowEfficiency * row.merged; + effWeight += row.merged; + } + } + + if (weight <= 0) return null; + + const phaseMedianHours = {} as Record; + for (const key of FLOW_PHASE_ORDER) { + phaseMedianHours[key] = round(weighted[key] / weight, 1); + } + + const totalHours = FLOW_PHASE_ORDER.reduce( + (sum, key) => sum + phaseMedianHours[key], + 0, + ); + let dominantPhase: DominantPhase | null = null; + if (totalHours > 0) { + // Ties resolve to the earlier (lower-index) phase — deterministic. + const key = FLOW_PHASE_ORDER.reduce((best, candidate) => + phaseMedianHours[candidate] > phaseMedianHours[best] ? candidate : best, + ); + dominantPhase = { + key, + hours: phaseMedianHours[key], + sharePct: round((phaseMedianHours[key] / totalHours) * 100, 1), + isWait: WAIT_PHASES.has(key), + }; + } + + return { + phaseMedianHours, + medianTimeToFirstReviewHours: + ttfrWeight > 0 ? round(ttfrWeighted / ttfrWeight, 1) : null, + flowEfficiencyMedian: + effWeight > 0 ? round(effWeighted / effWeight, 3) : null, + dominantPhase, + prsWithFlow: weight, + flowCoveragePct: totalMerged > 0 ? weight / totalMerged : null, + }; +} + +export type CycleTimeVerdictVariant = + | "verdict" + | "lowCoverage" + | "noFlow" + | "none"; + +export interface CycleTimeVerdict { + variant: CycleTimeVerdictVariant; + dominantPhase: DominantPhase | null; + prsWithFlow: number | null; + /** 0.0–1.0 */ + flowCoveragePct: number | null; +} + +/** + * Decide which honest verdict the card shows. + * + * - `none` — not enough merged PRs (or no within-24h figure): show nothing. + * - `noFlow` — dense enough, but no phase decomposition: state the KPIs and + * explicitly make NO claim about where the bottleneck is. + * - `lowCoverage` — decomposition exists but covers too little: show as a sample. + * - `verdict` — full computed read of the dominant code-window phase. + */ +export function selectCycleTimeVerdict( + data: { + totalPRsMerged: number; + pctMergedWithin24h: number | null; + flow: FlowDecomposition | null; + }, + opts: { minMerged: number; coverageFloor: number }, +): CycleTimeVerdict { + const base = { + dominantPhase: data.flow?.dominantPhase ?? null, + prsWithFlow: data.flow?.prsWithFlow ?? null, + flowCoveragePct: data.flow?.flowCoveragePct ?? null, + }; + if ( + data.totalPRsMerged < opts.minMerged || + data.pctMergedWithin24h === null + ) { + return { variant: "none", ...base }; + } + if (!data.flow || !data.flow.dominantPhase) { + return { variant: "noFlow", ...base }; + } + if ((data.flow.flowCoveragePct ?? 0) < opts.coverageFloor) { + return { variant: "lowCoverage", ...base }; + } + return { variant: "verdict", ...base }; +} diff --git a/platform/lib/queries/org-summary.ts b/platform/lib/queries/org-summary.ts index e065156..45d117e 100644 --- a/platform/lib/queries/org-summary.ts +++ b/platform/lib/queries/org-summary.ts @@ -5,6 +5,7 @@ import type { SupabaseClient } from "@supabase/supabase-js"; +import { summarizeFlow, type FlowRow } from "@/lib/queries/cycle-time-flow"; import { DEFAULT_WINDOW_DAYS } from "@/lib/queries/temporal"; import type { ReportMetrics } from "@/types/metrics"; import type { @@ -630,6 +631,9 @@ export function computeCycleTime( type Row = NonNullable[number]; const rows: Row[] = []; + // Per-repo phase decomposition the engine already emits — aggregated below + // into the org-level "where does PR time go" read (see summarizeFlow). + const flowRows: FlowRow[] = []; let totalMerged = 0; let totalWithin24h = 0; @@ -666,6 +670,15 @@ export function computeCycleTime( totalMerged += merged; totalWithin24h += buckets.same_day; + if (p.time_in_phase_median_hours) { + flowRows.push({ + merged, + phases: p.time_in_phase_median_hours, + ttfrHours: p.median_time_to_first_review_hours ?? null, + flowEfficiency: p.flow_efficiency_median ?? null, + }); + } + if (p.pr_median_time_to_merge_hours !== undefined) { medianWeightedSum += p.pr_median_time_to_merge_hours * merged; weightTotal += merged; @@ -695,6 +708,7 @@ export function computeCycleTime( medianHours: weightTotal > 0 ? medianWeightedSum / weightTotal : null, meanHours: weightTotal > 0 ? meanWeightedSum / weightTotal : null, p90Hours: maxP90, + flow: summarizeFlow(flowRows, totalMerged), perRepo: rows, }; } diff --git a/platform/lib/translations.ts b/platform/lib/translations.ts index c272ff6..39286f6 100644 --- a/platform/lib/translations.ts +++ b/platform/lib/translations.ts @@ -378,8 +378,23 @@ export const translations = { title: "Cycle Time", subtitle: "How long engineering takes to deliver, from PR open to merge.", - insight: - "Engineering ships fast. {pct} of PRs are merged within a day. Median cycle time is {median}. Lead-time bottlenecks live before (demand/product) and after (infra, environments, deploy) — not in code execution.", + verdict: + "Within the code window (PR open → merge), the stage that consumes the most time is {phase}{waitTag}: {phaseHours} median, {sharePct} of cycle time. Median cycle time: {median} · {pct} of PRs within 1 day. Based on {n} PRs with phase decomposition ({coverage} coverage).", + verdictLowCoverage: + "Partial sample: only {coverage} of merged PRs carry a phase decomposition — a hint, not a verdict. Where data exists, the longest stage is {phase} ({phaseHours}).", + verdictNoFlow: + "Median cycle time {median}; {pct} of PRs within 1 day. Phase decomposition isn't available for this view yet — without it, the dashboard makes no claim about where the bottleneck is.", + waitTag: " (wait)", + flowBarTitle: "Where PR time goes", + flowBarSubtitle: + "Code-window time by phase (median, weighted by merged PRs).", + phaseLabels: { + coding: "Coding", + awaiting_first_review: "Awaiting first review", + in_review_active: "In review · active", + in_review_wait: "In review · wait", + awaiting_merge: "Awaiting merge", + }, kpi: { pctWithin24h: "PRs merged within 1 day", median: "Median cycle time", @@ -1757,8 +1772,23 @@ export const translations = { title: "Cycle Time", subtitle: "Quanto tempo a engenharia leva para entregar, do PR aberto ao merge.", - insight: - "A engenharia entrega rápido. {pct} dos PRs são mesclados em até 1 dia. A mediana do cycle time é {median}. Os gargalos de lead time estão antes (definição de demanda/produto) e depois (infra, ambientes, deploy) — não na execução do código.", + verdict: + "Dentro da janela de código (PR aberto → merge), a etapa que mais consome tempo é {phase}{waitTag}: mediana {phaseHours}, {sharePct} do tempo de ciclo. Mediana do cycle time: {median} · {pct} dos PRs em ≤1 dia. Base: {n} PRs com decomposição de fase ({coverage} de cobertura).", + verdictLowCoverage: + "Amostra parcial: só {coverage} dos PRs mesclados têm decomposição de fase — indício, não veredito. Onde há dado, a etapa mais longa é {phase} ({phaseHours}).", + verdictNoFlow: + "Mediana do cycle time {median}; {pct} dos PRs em ≤1 dia. A decomposição por fase ainda não está disponível para este recorte — sem ela, o dashboard não afirma onde está o gargalo.", + waitTag: " (espera)", + flowBarTitle: "Onde o tempo do PR é gasto", + flowBarSubtitle: + "Tempo da janela de código por fase (mediana, ponderada por PRs mesclados).", + phaseLabels: { + coding: "Escrita de código", + awaiting_first_review: "Espera pelo 1º review", + in_review_active: "Em review · ativo", + in_review_wait: "Em review · espera", + awaiting_merge: "Espera pelo merge", + }, kpi: { pctWithin24h: "PRs merged em até 1 dia", median: "Mediana do cycle time", diff --git a/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx b/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx index a651c95..2d09b06 100644 --- a/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx +++ b/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx @@ -12,6 +12,7 @@ import { Cell, } from "recharts"; +import { FlowPhaseBar } from "@/components/charts/FlowPhaseBar"; import { MetricCard } from "@/components/charts/MetricCard"; import { Card, @@ -21,13 +22,22 @@ import { CardTitle, } from "@/components/ui/card"; import { useTranslation } from "@/hooks/useTranslation"; -import type { CycleTimeData } from "@/types/org-summary"; +import { + selectCycleTimeVerdict, + type CycleTimeVerdict, +} from "@/lib/queries/cycle-time-flow"; +import { cn } from "@/lib/utils"; +import type { CycleTimeData, FlowPhaseKey } from "@/types/org-summary"; -// Insight banner is only shown once cycle-time data is dense enough +// The verdict banner is only shown once cycle-time data is dense enough // to make a confident statement. Below this many merged PRs we still // render the section but hide the headline. const INSIGHT_MIN_MERGED = 50; +// Below this coverage the phase decomposition is shown as a partial sample, +// never as a verdict. See selectCycleTimeVerdict. +const COVERAGE_FLOOR = 0.6; + // Cutoffs for the "% merged within 24h" bar color ramp. Tuned so a repo that // ships in a day most of the time reads green, "mixed" reads yellow, and slow // repos read orange. @@ -74,9 +84,15 @@ interface CycleTimeProps { export function CycleTime({ data }: CycleTimeProps) { const { t } = useTranslation(); - const showInsight = - data.totalPRsMerged >= INSIGHT_MIN_MERGED && - data.pctMergedWithin24h !== null; + const verdict = selectCycleTimeVerdict( + { + totalPRsMerged: data.totalPRsMerged, + pctMergedWithin24h: data.pctMergedWithin24h, + flow: data.flow, + }, + { minMerged: INSIGHT_MIN_MERGED, coverageFloor: COVERAGE_FLOOR }, + ); + const verdictText = buildVerdictText(verdict, data, t); return (
@@ -89,16 +105,43 @@ export function CycleTime({ data }: CycleTimeProps) {

- {showInsight && ( - + {verdictText && ( + - -

- {t("dashboard.cycleTime.insight", { - pct: formatPct(data.pctMergedWithin24h), - median: formatHoursAsDays(data.medianHours), - })} -

+ +

{verdictText.text}

+ + + )} + + {data.flow && verdict.dominantPhase && ( + + + {t("dashboard.cycleTime.flowBarTitle")} + + {t("dashboard.cycleTime.flowBarSubtitle")} + + + + )} @@ -339,3 +382,79 @@ function formatHoursAsDays(hours: number | null): string { : rounded.toFixed(1).replace(".", ","); return `${label} d`; } + +type Translate = ( + path: string, + params?: Record, +) => string; + +function phaseLabels(t: Translate): Record { + return { + coding: t("dashboard.cycleTime.phaseLabels.coding"), + awaiting_first_review: t( + "dashboard.cycleTime.phaseLabels.awaiting_first_review", + ), + in_review_active: t("dashboard.cycleTime.phaseLabels.in_review_active"), + in_review_wait: t("dashboard.cycleTime.phaseLabels.in_review_wait"), + awaiting_merge: t("dashboard.cycleTime.phaseLabels.awaiting_merge"), + }; +} + +type VerdictText = { text: string; tone: "signal" | "muted" }; + +/** + * Turn a computed verdict into the banner copy. Only ever describes the code + * window it measured — never claims anything about "before" or "after". + */ +function buildVerdictText( + verdict: CycleTimeVerdict, + data: CycleTimeData, + t: Translate, +): VerdictText | null { + if (verdict.variant === "none") return null; + + const median = formatHoursAsDays(data.medianHours); + const pct = formatPct(data.pctMergedWithin24h); + + if (verdict.variant === "noFlow") { + return { + tone: "muted", + text: t("dashboard.cycleTime.verdictNoFlow", { median, pct }), + }; + } + + const dp = verdict.dominantPhase; + if (!dp) return null; + const phase = phaseLabels(t)[dp.key]; + const phaseHours = formatHoursAsDays(dp.hours); + const coverage = + verdict.flowCoveragePct !== null + ? `${Math.round(verdict.flowCoveragePct * 100)}%` + : "—"; + + if (verdict.variant === "lowCoverage") { + return { + tone: "muted", + text: t("dashboard.cycleTime.verdictLowCoverage", { + coverage, + phase, + phaseHours, + }), + }; + } + + const waitTag = dp.isWait ? t("dashboard.cycleTime.waitTag") : ""; + return { + tone: "signal", + text: t("dashboard.cycleTime.verdict", { + phase, + waitTag, + phaseHours, + sharePct: `${dp.sharePct.toFixed(0)}%`, + median, + pct, + n: verdict.prsWithFlow ?? 0, + coverage, + }), + }; +} diff --git a/platform/src/components/charts/FlowPhaseBar.tsx b/platform/src/components/charts/FlowPhaseBar.tsx new file mode 100644 index 0000000..422f07f --- /dev/null +++ b/platform/src/components/charts/FlowPhaseBar.tsx @@ -0,0 +1,98 @@ +import { FLOW_PHASE_ORDER } from "@/lib/queries/cycle-time-flow"; +import { cn } from "@/lib/utils"; +import type { FlowPhaseKey } from "@/types/org-summary"; + +/** + * Tailwind colors per lifecycle phase. Mirrors the per-repo Flow Efficiency + * card so the org-level bar reads consistently across the product. + */ +export const FLOW_PHASE_COLORS: Record = { + coding: "bg-signal-green", + awaiting_first_review: "bg-signal-yellow", + in_review_active: "bg-signal-purple", + in_review_wait: "bg-signal-red", + awaiting_merge: "bg-signal-gray", +}; + +interface FlowPhaseBarProps { + /** Median hours per phase (org-aggregated). */ + phaseHours: Partial>; + /** Localized label per phase. */ + labels: Record; + /** Format a phase duration for display (e.g. "14 h"). */ + formatHours: (hours: number) => string; + /** Phase to highlight as the dominant (widest) one, if any. */ + dominantKey?: FlowPhaseKey | null; +} + +/** + * Horizontal stacked bar + legend showing how the code window (PR open -> + * merge) splits across the five lifecycle phases. Presentational only. + */ +export function FlowPhaseBar({ + phaseHours, + labels, + formatHours, + dominantKey, +}: FlowPhaseBarProps) { + const total = FLOW_PHASE_ORDER.reduce( + (sum, key) => sum + (phaseHours[key] ?? 0), + 0, + ); + if (total <= 0) return null; + + return ( +
+
+ {FLOW_PHASE_ORDER.map((key) => { + const hours = phaseHours[key] ?? 0; + if (hours === 0) return null; + const pct = (hours / total) * 100; + return ( +
+ ); + })} +
+
+ {FLOW_PHASE_ORDER.map((key) => { + const hours = phaseHours[key] ?? 0; + if (hours === 0) return null; + return ( +
+ + + + {labels[key]} + + + + {formatHours(hours)} + +
+ ); + })} +
+
+ ); +} diff --git a/platform/src/types/org-summary.ts b/platform/src/types/org-summary.ts index 962f604..f3660df 100644 --- a/platform/src/types/org-summary.ts +++ b/platform/src/types/org-summary.ts @@ -93,6 +93,41 @@ export interface PRHealthData { reposWithData: number; } +/** The five lifecycle phases of a merged PR, in canonical order. */ +export type FlowPhaseKey = + | "coding" + | "awaiting_first_review" + | "in_review_active" + | "in_review_wait" + | "awaiting_merge"; + +/** The phase that consumes the most time within the code window. */ +export interface DominantPhase { + key: FlowPhaseKey; + /** Org-weighted median hours spent in this phase. */ + hours: number; + /** This phase's share of the total code-window time, 0–100. */ + sharePct: number; + /** True when the phase is a wait (queue) phase — the actionable kind. */ + isWait: boolean; +} + +/** + * Org-level decomposition of the code window (PR open → merge) into phases. + * Weighted by per-repo merged count — an approximation (per-PR timings are + * never persisted, by design), so display is gated on `flowCoveragePct`. + */ +export interface FlowDecomposition { + phaseMedianHours: Record; + medianTimeToFirstReviewHours: number | null; + flowEfficiencyMedian: number | null; + dominantPhase: DominantPhase | null; + /** Merged PRs that carried phase data (the aggregation weight). */ + prsWithFlow: number; + /** prsWithFlow ÷ total merged PRs, 0.0–1.0. */ + flowCoveragePct: number | null; +} + /** * Cycle Time view — how long the org takes to go from PR open to merge. * @@ -113,6 +148,12 @@ export interface CycleTimeData { meanHours: number | null; /** Hours. Worst-case P90 across repos (max of repo P90s). */ p90Hours: number | null; + /** + * Code-window phase decomposition + dominant phase. Null when no repo in + * the window carried phase data (older payloads). This is what replaces the + * old hardcoded "bottleneck" sentence with a computed read of the tenant. + */ + flow: FlowDecomposition | null; /** Per-repo rows, sorted from fastest to slowest. */ perRepo: Array<{ name: string; diff --git a/platform/tests/cycle-time-flow.test.ts b/platform/tests/cycle-time-flow.test.ts new file mode 100644 index 0000000..e2d93c1 --- /dev/null +++ b/platform/tests/cycle-time-flow.test.ts @@ -0,0 +1,290 @@ +import { describe, expect, it } from "vitest"; + +import { + FLOW_PHASE_ORDER, + WAIT_PHASES, + selectCycleTimeVerdict, + summarizeFlow, + type FlowRow, +} from "@/lib/queries/cycle-time-flow"; +import { computeCycleTime } from "@/lib/queries/org-summary"; +import type { ReportMetrics } from "@/types/metrics"; +import type { FlowDecomposition } from "@/types/org-summary"; +import type { RepoSummary } from "@/types/temporal"; + +// --------------------------------------------------------------------------- +// summarizeFlow +// --------------------------------------------------------------------------- + +describe("summarizeFlow", () => { + it("weights per-repo phase medians by merged count and picks the dominant", () => { + const rows: FlowRow[] = [ + { merged: 10, phases: { coding: 2, awaiting_first_review: 8 } }, + { merged: 30, phases: { coding: 6, awaiting_first_review: 2 } }, + ]; + const out = summarizeFlow(rows, 40)!; + + // coding = (2*10 + 6*30)/40 = 5 ; awaiting = (8*10 + 2*30)/40 = 3.5 + expect(out.phaseMedianHours.coding).toBe(5); + expect(out.phaseMedianHours.awaiting_first_review).toBe(3.5); + expect(out.dominantPhase).toEqual({ + key: "coding", + hours: 5, + sharePct: 58.8, // round(5 / 8.5 * 100, 1) + isWait: false, + }); + expect(out.prsWithFlow).toBe(40); + expect(out.flowCoveragePct).toBe(1); + }); + + it("flags a wait phase as the actionable dominant kind", () => { + const rows: FlowRow[] = [ + { merged: 5, phases: { coding: 1, awaiting_first_review: 9 } }, + ]; + const out = summarizeFlow(rows, 5)!; + expect(out.dominantPhase?.key).toBe("awaiting_first_review"); + expect(out.dominantPhase?.isWait).toBe(true); + }); + + it("reports partial coverage when some merged PRs lack phase data", () => { + const rows: FlowRow[] = [{ merged: 20, phases: { coding: 4 } }]; + const out = summarizeFlow(rows, 50)!; // 30 merged PRs carried no phase data + expect(out.prsWithFlow).toBe(20); + expect(out.flowCoveragePct).toBe(0.4); + }); + + it("weights TTFR and flow efficiency only over rows that carry them", () => { + const rows: FlowRow[] = [ + { merged: 10, phases: { coding: 1 }, ttfrHours: 12, flowEfficiency: 0.5 }, + { merged: 30, phases: { coding: 1 }, ttfrHours: null }, // skipped for ttfr/eff + ]; + const out = summarizeFlow(rows, 40)!; + expect(out.medianTimeToFirstReviewHours).toBe(12); // only the 10-weight row + expect(out.flowEfficiencyMedian).toBe(0.5); + }); + + it("returns null when no row carries phase data", () => { + expect(summarizeFlow([], 10)).toBeNull(); + expect( + summarizeFlow([{ merged: 0, phases: { coding: 5 } }], 10), + ).toBeNull(); + }); + + it("returns a decomposition with no dominant phase when all phases are zero", () => { + const out = summarizeFlow([{ merged: 5, phases: {} }], 5)!; + expect(out.dominantPhase).toBeNull(); + expect(out.prsWithFlow).toBe(5); + expect(out.flowCoveragePct).toBe(1); + }); + + it("returns null coverage when the total merged count is unknown", () => { + const out = summarizeFlow([{ merged: 5, phases: { coding: 2 } }], 0)!; + expect(out.flowCoveragePct).toBeNull(); + }); + + it("breaks ties toward the earlier phase deterministically", () => { + const out = summarizeFlow( + [{ merged: 1, phases: { coding: 4, awaiting_merge: 4 } }], + 1, + )!; + expect(out.dominantPhase?.key).toBe("coding"); + }); + + it("exposes the canonical order and the wait-phase set", () => { + expect(FLOW_PHASE_ORDER).toEqual([ + "coding", + "awaiting_first_review", + "in_review_active", + "in_review_wait", + "awaiting_merge", + ]); + expect(WAIT_PHASES.has("in_review_wait")).toBe(true); + expect(WAIT_PHASES.has("coding")).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// selectCycleTimeVerdict +// --------------------------------------------------------------------------- + +const OPTS = { minMerged: 50, coverageFloor: 0.6 }; + +function flow(over: Partial): FlowDecomposition { + return { + phaseMedianHours: { + coding: 1, + awaiting_first_review: 4, + in_review_active: 1, + in_review_wait: 1, + awaiting_merge: 1, + }, + medianTimeToFirstReviewHours: 4, + flowEfficiencyMedian: 0.3, + dominantPhase: { + key: "awaiting_first_review", + hours: 4, + sharePct: 50, + isWait: true, + }, + prsWithFlow: 80, + flowCoveragePct: 0.9, + ...over, + }; +} + +describe("selectCycleTimeVerdict", () => { + it("returns none when there are too few merged PRs", () => { + const v = selectCycleTimeVerdict( + { totalPRsMerged: 10, pctMergedWithin24h: 0.7, flow: flow({}) }, + OPTS, + ); + expect(v.variant).toBe("none"); + }); + + it("returns none when the within-24h figure is missing", () => { + const v = selectCycleTimeVerdict( + { totalPRsMerged: 100, pctMergedWithin24h: null, flow: flow({}) }, + OPTS, + ); + expect(v.variant).toBe("none"); + }); + + it("returns noFlow when there is no decomposition", () => { + const v = selectCycleTimeVerdict( + { totalPRsMerged: 100, pctMergedWithin24h: 0.7, flow: null }, + OPTS, + ); + expect(v.variant).toBe("noFlow"); + expect(v.dominantPhase).toBeNull(); + }); + + it("returns noFlow when the decomposition has no dominant phase", () => { + const v = selectCycleTimeVerdict( + { + totalPRsMerged: 100, + pctMergedWithin24h: 0.7, + flow: flow({ dominantPhase: null }), + }, + OPTS, + ); + expect(v.variant).toBe("noFlow"); + }); + + it("returns lowCoverage below the coverage floor", () => { + const v = selectCycleTimeVerdict( + { + totalPRsMerged: 100, + pctMergedWithin24h: 0.7, + flow: flow({ flowCoveragePct: 0.4 }), + }, + OPTS, + ); + expect(v.variant).toBe("lowCoverage"); + expect(v.flowCoveragePct).toBe(0.4); + }); + + it("returns the full verdict at or above the coverage floor", () => { + const v = selectCycleTimeVerdict( + { + totalPRsMerged: 100, + pctMergedWithin24h: 0.7, + flow: flow({ flowCoveragePct: 0.6 }), + }, + OPTS, + ); + expect(v.variant).toBe("verdict"); + expect(v.dominantPhase?.key).toBe("awaiting_first_review"); + expect(v.prsWithFlow).toBe(80); + }); +}); + +// --------------------------------------------------------------------------- +// computeCycleTime — flow wiring (integration) +// --------------------------------------------------------------------------- + +const repos = [ + { id: "r1", name: "checkout" }, + { id: "r2", name: "orders" }, +] as unknown as RepoSummary[]; + +function metrics(over: Partial): ReportMetrics { + return { + commits_total: 0, + commits_revert: 0, + revert_rate: 0, + churn_events: 0, + churn_lines_affected: 0, + files_touched: 0, + files_stabilized: 0, + stabilization_ratio: 0, + pr_cycle_time_buckets: { + same_day: 0, + one_day: 0, + two_to_three_days: 0, + four_to_seven_days: 0, + seven_plus_days: 0, + }, + ...over, + } as ReportMetrics; +} + +describe("computeCycleTime — flow decomposition", () => { + it("aggregates phase data the engine emits into data.flow", () => { + const payloads = new Map(); + payloads.set( + "r1", + metrics({ + pr_merged_count: 20, + pr_cycle_time_buckets: { + same_day: 12, + one_day: 4, + two_to_three_days: 2, + four_to_seven_days: 1, + seven_plus_days: 1, + }, + time_in_phase_median_hours: { coding: 2, awaiting_first_review: 10 }, + median_time_to_first_review_hours: 10, + }), + ); + payloads.set( + "r2", + metrics({ + pr_merged_count: 30, + pr_cycle_time_buckets: { + same_day: 20, + one_day: 6, + two_to_three_days: 2, + four_to_seven_days: 1, + seven_plus_days: 1, + }, + time_in_phase_median_hours: { coding: 3, awaiting_first_review: 3 }, + }), + ); + + const out = computeCycleTime(repos, payloads)!; + expect(out.flow).not.toBeNull(); + // awaiting = (10*20 + 3*30)/50 = 5.8 ; coding = (2*20 + 3*30)/50 = 2.6 + expect(out.flow!.phaseMedianHours.awaiting_first_review).toBe(5.8); + expect(out.flow!.dominantPhase?.key).toBe("awaiting_first_review"); + expect(out.flow!.flowCoveragePct).toBe(1); + }); + + it("leaves data.flow null when no payload carries phase data", () => { + const payloads = new Map(); + payloads.set( + "r1", + metrics({ + pr_merged_count: 10, + pr_cycle_time_buckets: { + same_day: 8, + one_day: 2, + two_to_three_days: 0, + four_to_seven_days: 0, + seven_plus_days: 0, + }, + }), + ); + const out = computeCycleTime(repos, payloads)!; + expect(out.flow).toBeNull(); + }); +}); From 81bccd081861ba9c742bbda36df97091d01cfe39 Mon Sep 17 00:00:00 2001 From: Marcos Sabatino Date: Sat, 20 Jun 2026 02:42:48 -0300 Subject: [PATCH 2/3] fix(dashboard): address code-review majors M1-M4 on the Cycle Time verdict An adversarial self-review of the first commit found four accuracy defects in the "honest verdict" card. This fixes all four. M1 - share denominator: relabel "% of cycle time" -> "% of measured phase time" and document that the window total is a SUM of phase medians, not the cycle-time median (a separate open->merge aggregation shown alongside). M2 - coding contamination: the dominant phase and its share are now computed over WINDOW_PHASES (the 4 post-open phases). `coding` (first commit -> PR open) is pre-window authoring time and can no longer be reported as the bottleneck of a card that measures only PR open->merge. The phase bar shows the window phases only. M3 - coverage guard-rail was inert: flowCoveragePct used pr_merged_count as both numerator and denominator (always ~100%). The engine now emits flow_pr_count (the real decomposed-PR count; pr_count already existed on FlowEfficiencyResult), and the platform weights/measures coverage by it. Payloads without flow_pr_count are skipped so coverage never fakes 100%. M4 - removed the redundant "(wait)/(espera)" tag, which only ever appended to phase labels that already said "wait/espera". Tests rewritten for the new semantics (weight, window dominant, real coverage) with cases locking M2 (coding never wins) and M3 (coverage < 1 and null without flow_pr_count). TS 231 pass, Python 239 pass, tsc clean, cycle-time-flow.ts 100% lines / 97.5% branch. Co-Authored-By: Claude Opus 4.8 (1M context) --- iris/metrics/aggregator.py | 3 + iris/models/metrics.py | 4 + platform/lib/queries/cycle-time-flow.ts | 67 ++++++--- platform/lib/queries/org-summary.ts | 7 +- platform/lib/translations.ts | 10 +- .../[tenant]/dashboard/sections/CycleTime.tsx | 2 - .../src/components/charts/FlowPhaseBar.tsx | 13 +- platform/src/types/metrics.ts | 3 + platform/src/types/org-summary.ts | 15 +- platform/tests/cycle-time-flow.test.ts | 137 ++++++++++++++---- 10 files changed, 190 insertions(+), 71 deletions(-) diff --git a/iris/metrics/aggregator.py b/iris/metrics/aggregator.py index 7a7658c..7d0569a 100644 --- a/iris/metrics/aggregator.py +++ b/iris/metrics/aggregator.py @@ -311,6 +311,9 @@ def aggregate( flow_efficiency_kwargs["time_in_phase_median_hours"] = ( flow_efficiency_result.time_in_phase_median_hours ) + flow_efficiency_kwargs["flow_pr_count"] = ( + flow_efficiency_result.pr_count + ) if flow_efficiency_result.median_time_to_first_review_hours is not None: flow_efficiency_kwargs["median_time_to_first_review_hours"] = ( flow_efficiency_result.median_time_to_first_review_hours diff --git a/iris/models/metrics.py b/iris/models/metrics.py index bbf743e..95aa490 100644 --- a/iris/models/metrics.py +++ b/iris/models/metrics.py @@ -132,6 +132,10 @@ class ReportMetrics: flow_efficiency_by_origin: dict[str, float] | None = None time_in_phase_median_hours: dict[str, float] | None = None median_time_to_first_review_hours: float | None = None + # Count of merged PRs that survived Flow Efficiency's filters and carry a + # phase decomposition. The honest denominator for flow coverage — NOT + # pr_merged_count (which also counts PRs with no phase data). + flow_pr_count: int | None = None # Human Review Coverage — fraction of merged PRs with genuine human review # (optional — None when no merged PR exists in the window). Disambiguates diff --git a/platform/lib/queries/cycle-time-flow.ts b/platform/lib/queries/cycle-time-flow.ts index 9cbcad0..cbee13f 100644 --- a/platform/lib/queries/cycle-time-flow.ts +++ b/platform/lib/queries/cycle-time-flow.ts @@ -10,6 +10,13 @@ * (PR open -> merge); it never claims anything about the "before" (demand) * or "after" (deploy) windows, which the engine does not measure here. * + * Because of that, the dominant phase and its share are computed over + * WINDOW_PHASES only — `coding` (first commit -> PR open) is authoring time + * that happens BEFORE the window, so it never wins the verdict. And the share + * denominator is the SUM of per-phase medians: an approximation of where time + * concentrates, NOT the cycle-time median (which comes from a separate + * open->merge aggregation and is shown alongside). + * * No I/O, no per-PR data: fully unit-testable. */ import type { @@ -27,6 +34,19 @@ export const FLOW_PHASE_ORDER: readonly FlowPhaseKey[] = [ "awaiting_merge", ]; +/** + * Phases inside the measured window (PR open -> merge). Excludes `coding`, + * which is pre-open authoring time. The verdict's dominant phase and its + * share are computed over these only, so the card never blames a stage that + * happens before the PR exists. + */ +export const WINDOW_PHASES: readonly FlowPhaseKey[] = [ + "awaiting_first_review", + "in_review_active", + "in_review_wait", + "awaiting_merge", +]; + /** Phases that are *waiting* (queue) rather than *active* (work being done). */ export const WAIT_PHASES: ReadonlySet = new Set([ "awaiting_first_review", @@ -36,8 +56,13 @@ export const WAIT_PHASES: ReadonlySet = new Set([ /** One repo's contribution to the org-level flow aggregation. */ export interface FlowRow { - /** Merged PRs in the window — the aggregation weight. */ - merged: number; + /** + * Merged PRs that actually carry a phase decomposition (engine + * `flow_pr_count`) — both the aggregation weight and the coverage + * numerator. Using the real decomposed count (not total merged) is what + * keeps `flowCoveragePct` honest instead of pinned at ~100%. + */ + weight: number; /** Per-phase median hours for this repo (engine output). */ phases: Partial>; ttfrHours?: number | null; @@ -51,10 +76,12 @@ function round(value: number, places: number): number { /** * Aggregate per-repo phase medians into an org-level decomposition, weighted - * by merged-PR count. This is a weighted average of per-repo medians — an - * approximation (we never persist per-PR timings, by design), so callers MUST - * gate display on `flowCoveragePct`. Returns null when no row carried phase - * data (e.g. only older payloads in the window). + * by each repo's decomposed-PR count (`FlowRow.weight`). This is a weighted + * average of per-repo medians — an approximation (per-PR timings are never + * persisted, by design), so callers MUST gate display on `flowCoveragePct`. + * The dominant phase + share are computed over WINDOW_PHASES (coding is + * pre-open authoring time and never wins). Returns null when no row carried + * decomposed phase data. */ export function summarizeFlow( rows: FlowRow[], @@ -74,18 +101,18 @@ export function summarizeFlow( let effWeight = 0; for (const row of rows) { - if (!row.phases || row.merged <= 0) continue; - weight += row.merged; + if (!row.phases || row.weight <= 0) continue; + weight += row.weight; for (const key of FLOW_PHASE_ORDER) { - weighted[key] += (row.phases[key] ?? 0) * row.merged; + weighted[key] += (row.phases[key] ?? 0) * row.weight; } if (row.ttfrHours != null) { - ttfrWeighted += row.ttfrHours * row.merged; - ttfrWeight += row.merged; + ttfrWeighted += row.ttfrHours * row.weight; + ttfrWeight += row.weight; } if (row.flowEfficiency != null) { - effWeighted += row.flowEfficiency * row.merged; - effWeight += row.merged; + effWeighted += row.flowEfficiency * row.weight; + effWeight += row.weight; } } @@ -96,20 +123,24 @@ export function summarizeFlow( phaseMedianHours[key] = round(weighted[key] / weight, 1); } - const totalHours = FLOW_PHASE_ORDER.reduce( + // Dominant phase + share are computed over the measured window only + // (WINDOW_PHASES excludes `coding`, the pre-PR-open authoring time). + // windowHours is the SUM of those phase medians — an approximation of where + // time concentrates, NOT the cycle-time median. + const windowHours = WINDOW_PHASES.reduce( (sum, key) => sum + phaseMedianHours[key], 0, ); let dominantPhase: DominantPhase | null = null; - if (totalHours > 0) { - // Ties resolve to the earlier (lower-index) phase — deterministic. - const key = FLOW_PHASE_ORDER.reduce((best, candidate) => + if (windowHours > 0) { + // Ties resolve to the earlier (lower-index) window phase — deterministic. + const key = WINDOW_PHASES.reduce((best, candidate) => phaseMedianHours[candidate] > phaseMedianHours[best] ? candidate : best, ); dominantPhase = { key, hours: phaseMedianHours[key], - sharePct: round((phaseMedianHours[key] / totalHours) * 100, 1), + sharePct: round((phaseMedianHours[key] / windowHours) * 100, 1), isWait: WAIT_PHASES.has(key), }; } diff --git a/platform/lib/queries/org-summary.ts b/platform/lib/queries/org-summary.ts index 45d117e..96cd8ae 100644 --- a/platform/lib/queries/org-summary.ts +++ b/platform/lib/queries/org-summary.ts @@ -670,9 +670,12 @@ export function computeCycleTime( totalMerged += merged; totalWithin24h += buckets.same_day; - if (p.time_in_phase_median_hours) { + // Only contribute to the flow decomposition when the engine reported the + // real count of decomposed PRs (flow_pr_count). Older payloads without it + // are skipped so coverage stays honest instead of pinning at ~100%. + if (p.time_in_phase_median_hours && (p.flow_pr_count ?? 0) > 0) { flowRows.push({ - merged, + weight: p.flow_pr_count as number, phases: p.time_in_phase_median_hours, ttfrHours: p.median_time_to_first_review_hours ?? null, flowEfficiency: p.flow_efficiency_median ?? null, diff --git a/platform/lib/translations.ts b/platform/lib/translations.ts index 39286f6..6055316 100644 --- a/platform/lib/translations.ts +++ b/platform/lib/translations.ts @@ -379,15 +379,14 @@ export const translations = { subtitle: "How long engineering takes to deliver, from PR open to merge.", verdict: - "Within the code window (PR open → merge), the stage that consumes the most time is {phase}{waitTag}: {phaseHours} median, {sharePct} of cycle time. Median cycle time: {median} · {pct} of PRs within 1 day. Based on {n} PRs with phase decomposition ({coverage} coverage).", + "Within the code window (PR open → merge), the stage that consumes the most time is {phase}: {phaseHours} median, {sharePct} of measured phase time. Median cycle time: {median} · {pct} of PRs within 1 day. Based on {n} PRs with phase decomposition ({coverage} coverage).", verdictLowCoverage: "Partial sample: only {coverage} of merged PRs carry a phase decomposition — a hint, not a verdict. Where data exists, the longest stage is {phase} ({phaseHours}).", verdictNoFlow: "Median cycle time {median}; {pct} of PRs within 1 day. Phase decomposition isn't available for this view yet — without it, the dashboard makes no claim about where the bottleneck is.", - waitTag: " (wait)", flowBarTitle: "Where PR time goes", flowBarSubtitle: - "Code-window time by phase (median, weighted by merged PRs).", + "Code-window time by phase (median, weighted by decomposed PRs).", phaseLabels: { coding: "Coding", awaiting_first_review: "Awaiting first review", @@ -1773,15 +1772,14 @@ export const translations = { subtitle: "Quanto tempo a engenharia leva para entregar, do PR aberto ao merge.", verdict: - "Dentro da janela de código (PR aberto → merge), a etapa que mais consome tempo é {phase}{waitTag}: mediana {phaseHours}, {sharePct} do tempo de ciclo. Mediana do cycle time: {median} · {pct} dos PRs em ≤1 dia. Base: {n} PRs com decomposição de fase ({coverage} de cobertura).", + "Dentro da janela de código (PR aberto → merge), a etapa que mais consome tempo é {phase}: mediana {phaseHours}, {sharePct} do tempo das fases medidas. Mediana do cycle time: {median} · {pct} dos PRs em ≤1 dia. Base: {n} PRs com decomposição de fase ({coverage} de cobertura).", verdictLowCoverage: "Amostra parcial: só {coverage} dos PRs mesclados têm decomposição de fase — indício, não veredito. Onde há dado, a etapa mais longa é {phase} ({phaseHours}).", verdictNoFlow: "Mediana do cycle time {median}; {pct} dos PRs em ≤1 dia. A decomposição por fase ainda não está disponível para este recorte — sem ela, o dashboard não afirma onde está o gargalo.", - waitTag: " (espera)", flowBarTitle: "Onde o tempo do PR é gasto", flowBarSubtitle: - "Tempo da janela de código por fase (mediana, ponderada por PRs mesclados).", + "Tempo da janela de código por fase (mediana, ponderada por PRs decompostos).", phaseLabels: { coding: "Escrita de código", awaiting_first_review: "Espera pelo 1º review", diff --git a/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx b/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx index 2d09b06..482a43a 100644 --- a/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx +++ b/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx @@ -443,12 +443,10 @@ function buildVerdictText( }; } - const waitTag = dp.isWait ? t("dashboard.cycleTime.waitTag") : ""; return { tone: "signal", text: t("dashboard.cycleTime.verdict", { phase, - waitTag, phaseHours, sharePct: `${dp.sharePct.toFixed(0)}%`, median, diff --git a/platform/src/components/charts/FlowPhaseBar.tsx b/platform/src/components/charts/FlowPhaseBar.tsx index 422f07f..ff18092 100644 --- a/platform/src/components/charts/FlowPhaseBar.tsx +++ b/platform/src/components/charts/FlowPhaseBar.tsx @@ -1,4 +1,4 @@ -import { FLOW_PHASE_ORDER } from "@/lib/queries/cycle-time-flow"; +import { WINDOW_PHASES } from "@/lib/queries/cycle-time-flow"; import { cn } from "@/lib/utils"; import type { FlowPhaseKey } from "@/types/org-summary"; @@ -26,8 +26,9 @@ interface FlowPhaseBarProps { } /** - * Horizontal stacked bar + legend showing how the code window (PR open -> - * merge) splits across the five lifecycle phases. Presentational only. + * Horizontal stacked bar + legend showing how the measured code window + * (PR open -> merge) splits across its phases. Excludes `coding` (pre-open + * authoring time), matching the verdict's window. Presentational only. */ export function FlowPhaseBar({ phaseHours, @@ -35,7 +36,7 @@ export function FlowPhaseBar({ formatHours, dominantKey, }: FlowPhaseBarProps) { - const total = FLOW_PHASE_ORDER.reduce( + const total = WINDOW_PHASES.reduce( (sum, key) => sum + (phaseHours[key] ?? 0), 0, ); @@ -44,7 +45,7 @@ export function FlowPhaseBar({ return (
- {FLOW_PHASE_ORDER.map((key) => { + {WINDOW_PHASES.map((key) => { const hours = phaseHours[key] ?? 0; if (hours === 0) return null; const pct = (hours / total) * 100; @@ -63,7 +64,7 @@ export function FlowPhaseBar({ })}
- {FLOW_PHASE_ORDER.map((key) => { + {WINDOW_PHASES.map((key) => { const hours = phaseHours[key] ?? 0; if (hours === 0) return null; return ( diff --git a/platform/src/types/metrics.ts b/platform/src/types/metrics.ts index 2c9ce81..b09a030 100644 --- a/platform/src/types/metrics.ts +++ b/platform/src/types/metrics.ts @@ -261,6 +261,9 @@ export interface ReportMetrics { awaiting_merge?: number; }; median_time_to_first_review_hours?: number; + // Merged PRs that actually carry a phase decomposition — the honest + // denominator for flow coverage (not pr_merged_count). + flow_pr_count?: number; // Human Review Coverage — fraction of merged PRs with genuine human review. // Disambiguates pr_single_pass_rate: "merged in one pass" vs "no human looked". diff --git a/platform/src/types/org-summary.ts b/platform/src/types/org-summary.ts index f3660df..375d535 100644 --- a/platform/src/types/org-summary.ts +++ b/platform/src/types/org-summary.ts @@ -101,12 +101,16 @@ export type FlowPhaseKey = | "in_review_wait" | "awaiting_merge"; -/** The phase that consumes the most time within the code window. */ +/** The phase that consumes the most time within the measured window. */ export interface DominantPhase { key: FlowPhaseKey; /** Org-weighted median hours spent in this phase. */ hours: number; - /** This phase's share of the total code-window time, 0–100. */ + /** + * This phase's share of the summed window-phase medians (an approximation + * of where time concentrates — NOT a share of the cycle-time median), 0–100. + * Computed over the post-open window only (excludes `coding`). + */ sharePct: number; /** True when the phase is a wait (queue) phase — the actionable kind. */ isWait: boolean; @@ -114,15 +118,16 @@ export interface DominantPhase { /** * Org-level decomposition of the code window (PR open → merge) into phases. - * Weighted by per-repo merged count — an approximation (per-PR timings are - * never persisted, by design), so display is gated on `flowCoveragePct`. + * Weighted by each repo's decomposed-PR count — an approximation (per-PR + * timings are never persisted, by design), so display is gated on + * `flowCoveragePct`. `dominantPhase` is chosen over the post-open window only. */ export interface FlowDecomposition { phaseMedianHours: Record; medianTimeToFirstReviewHours: number | null; flowEfficiencyMedian: number | null; dominantPhase: DominantPhase | null; - /** Merged PRs that carried phase data (the aggregation weight). */ + /** Merged PRs that carried a phase decomposition (the aggregation weight). */ prsWithFlow: number; /** prsWithFlow ÷ total merged PRs, 0.0–1.0. */ flowCoveragePct: number | null; diff --git a/platform/tests/cycle-time-flow.test.ts b/platform/tests/cycle-time-flow.test.ts index e2d93c1..395e515 100644 --- a/platform/tests/cycle-time-flow.test.ts +++ b/platform/tests/cycle-time-flow.test.ts @@ -3,6 +3,7 @@ import { describe, expect, it } from "vitest"; import { FLOW_PHASE_ORDER, WAIT_PHASES, + WINDOW_PHASES, selectCycleTimeVerdict, summarizeFlow, type FlowRow, @@ -17,18 +18,18 @@ import type { RepoSummary } from "@/types/temporal"; // --------------------------------------------------------------------------- describe("summarizeFlow", () => { - it("weights per-repo phase medians by merged count and picks the dominant", () => { + it("weights per-repo phase medians by the decomposed-PR count and picks the window dominant", () => { const rows: FlowRow[] = [ - { merged: 10, phases: { coding: 2, awaiting_first_review: 8 } }, - { merged: 30, phases: { coding: 6, awaiting_first_review: 2 } }, + { weight: 10, phases: { awaiting_first_review: 8, in_review_active: 2 } }, + { weight: 30, phases: { awaiting_first_review: 2, in_review_active: 6 } }, ]; const out = summarizeFlow(rows, 40)!; - // coding = (2*10 + 6*30)/40 = 5 ; awaiting = (8*10 + 2*30)/40 = 3.5 - expect(out.phaseMedianHours.coding).toBe(5); + // awaiting = (8*10 + 2*30)/40 = 3.5 ; active = (2*10 + 6*30)/40 = 5 expect(out.phaseMedianHours.awaiting_first_review).toBe(3.5); + expect(out.phaseMedianHours.in_review_active).toBe(5); expect(out.dominantPhase).toEqual({ - key: "coding", + key: "in_review_active", hours: 5, sharePct: 58.8, // round(5 / 8.5 * 100, 1) isWait: false, @@ -37,60 +38,79 @@ describe("summarizeFlow", () => { expect(out.flowCoveragePct).toBe(1); }); - it("flags a wait phase as the actionable dominant kind", () => { + it("never lets 'coding' (pre-PR-open authoring) win the verdict — M2", () => { + // coding is the largest phase, but it is OUTSIDE the measured window. const rows: FlowRow[] = [ - { merged: 5, phases: { coding: 1, awaiting_first_review: 9 } }, + { + weight: 10, + phases: { coding: 20, awaiting_first_review: 8, in_review_wait: 2 }, + }, ]; - const out = summarizeFlow(rows, 5)!; + const out = summarizeFlow(rows, 10)!; + // coding is still computed in the data... + expect(out.phaseMedianHours.coding).toBe(20); + // ...but the dominant is the largest WINDOW phase, and the share denominator + // is the window total (8 + 2 = 10), not the all-phase total. expect(out.dominantPhase?.key).toBe("awaiting_first_review"); + expect(out.dominantPhase?.hours).toBe(8); + expect(out.dominantPhase?.sharePct).toBe(80); // 8 / 10 window total expect(out.dominantPhase?.isWait).toBe(true); }); - it("reports partial coverage when some merged PRs lack phase data", () => { - const rows: FlowRow[] = [{ merged: 20, phases: { coding: 4 } }]; - const out = summarizeFlow(rows, 50)!; // 30 merged PRs carried no phase data + it("reports partial coverage from the decomposed count, not total merged — M3", () => { + const rows: FlowRow[] = [{ weight: 20, phases: { in_review_wait: 4 } }]; + const out = summarizeFlow(rows, 50)!; // 50 merged, only 20 decomposed expect(out.prsWithFlow).toBe(20); expect(out.flowCoveragePct).toBe(0.4); }); it("weights TTFR and flow efficiency only over rows that carry them", () => { const rows: FlowRow[] = [ - { merged: 10, phases: { coding: 1 }, ttfrHours: 12, flowEfficiency: 0.5 }, - { merged: 30, phases: { coding: 1 }, ttfrHours: null }, // skipped for ttfr/eff + { + weight: 10, + phases: { awaiting_first_review: 1 }, + ttfrHours: 12, + flowEfficiency: 0.5, + }, + { weight: 30, phases: { awaiting_first_review: 1 }, ttfrHours: null }, ]; const out = summarizeFlow(rows, 40)!; - expect(out.medianTimeToFirstReviewHours).toBe(12); // only the 10-weight row + expect(out.medianTimeToFirstReviewHours).toBe(12); expect(out.flowEfficiencyMedian).toBe(0.5); }); - it("returns null when no row carries phase data", () => { + it("returns null when no row carries decomposed phase data", () => { expect(summarizeFlow([], 10)).toBeNull(); expect( - summarizeFlow([{ merged: 0, phases: { coding: 5 } }], 10), + summarizeFlow([{ weight: 0, phases: { awaiting_first_review: 5 } }], 10), ).toBeNull(); }); - it("returns a decomposition with no dominant phase when all phases are zero", () => { - const out = summarizeFlow([{ merged: 5, phases: {} }], 5)!; + it("has no dominant phase when every window phase is zero (even if coding > 0)", () => { + const out = summarizeFlow([{ weight: 5, phases: { coding: 9 } }], 5)!; expect(out.dominantPhase).toBeNull(); + expect(out.phaseMedianHours.coding).toBe(9); expect(out.prsWithFlow).toBe(5); expect(out.flowCoveragePct).toBe(1); }); it("returns null coverage when the total merged count is unknown", () => { - const out = summarizeFlow([{ merged: 5, phases: { coding: 2 } }], 0)!; + const out = summarizeFlow( + [{ weight: 5, phases: { awaiting_first_review: 2 } }], + 0, + )!; expect(out.flowCoveragePct).toBeNull(); }); - it("breaks ties toward the earlier phase deterministically", () => { + it("breaks ties toward the earlier window phase deterministically", () => { const out = summarizeFlow( - [{ merged: 1, phases: { coding: 4, awaiting_merge: 4 } }], + [{ weight: 1, phases: { awaiting_first_review: 4, awaiting_merge: 4 } }], 1, )!; - expect(out.dominantPhase?.key).toBe("coding"); + expect(out.dominantPhase?.key).toBe("awaiting_first_review"); }); - it("exposes the canonical order and the wait-phase set", () => { + it("exposes the canonical order, the window phases, and the wait-phase set", () => { expect(FLOW_PHASE_ORDER).toEqual([ "coding", "awaiting_first_review", @@ -98,8 +118,15 @@ describe("summarizeFlow", () => { "in_review_wait", "awaiting_merge", ]); + expect(WINDOW_PHASES).toEqual([ + "awaiting_first_review", + "in_review_active", + "in_review_wait", + "awaiting_merge", + ]); + expect(WINDOW_PHASES).not.toContain("coding"); expect(WAIT_PHASES.has("in_review_wait")).toBe(true); - expect(WAIT_PHASES.has("coding")).toBe(false); + expect(WAIT_PHASES.has("in_review_active")).toBe(false); }); }); @@ -123,7 +150,7 @@ function flow(over: Partial): FlowDecomposition { dominantPhase: { key: "awaiting_first_review", hours: 4, - sharePct: 50, + sharePct: 57.1, isWait: true, }, prsWithFlow: 80, @@ -229,12 +256,13 @@ function metrics(over: Partial): ReportMetrics { } describe("computeCycleTime — flow decomposition", () => { - it("aggregates phase data the engine emits into data.flow", () => { + it("aggregates phase data weighted by flow_pr_count and excludes coding", () => { const payloads = new Map(); payloads.set( "r1", metrics({ pr_merged_count: 20, + flow_pr_count: 20, pr_cycle_time_buckets: { same_day: 12, one_day: 4, @@ -242,14 +270,14 @@ describe("computeCycleTime — flow decomposition", () => { four_to_seven_days: 1, seven_plus_days: 1, }, - time_in_phase_median_hours: { coding: 2, awaiting_first_review: 10 }, - median_time_to_first_review_hours: 10, + time_in_phase_median_hours: { coding: 30, awaiting_first_review: 10 }, }), ); payloads.set( "r2", metrics({ pr_merged_count: 30, + flow_pr_count: 30, pr_cycle_time_buckets: { same_day: 20, one_day: 6, @@ -257,16 +285,60 @@ describe("computeCycleTime — flow decomposition", () => { four_to_seven_days: 1, seven_plus_days: 1, }, - time_in_phase_median_hours: { coding: 3, awaiting_first_review: 3 }, + time_in_phase_median_hours: { coding: 30, awaiting_first_review: 3 }, }), ); const out = computeCycleTime(repos, payloads)!; expect(out.flow).not.toBeNull(); - // awaiting = (10*20 + 3*30)/50 = 5.8 ; coding = (2*20 + 3*30)/50 = 2.6 + // awaiting = (10*20 + 3*30)/50 = 5.8 ; coding present but EXCLUDED from dominant expect(out.flow!.phaseMedianHours.awaiting_first_review).toBe(5.8); + expect(out.flow!.phaseMedianHours.coding).toBe(30); expect(out.flow!.dominantPhase?.key).toBe("awaiting_first_review"); - expect(out.flow!.flowCoveragePct).toBe(1); + expect(out.flow!.flowCoveragePct).toBe(1); // 50 decomposed / 50 merged + }); + + it("yields partial coverage when flow_pr_count < merged — M3 guard-rail is live", () => { + const payloads = new Map(); + payloads.set( + "r1", + metrics({ + pr_merged_count: 100, + flow_pr_count: 40, // only 40 of 100 merged PRs were decomposed + pr_cycle_time_buckets: { + same_day: 60, + one_day: 20, + two_to_three_days: 10, + four_to_seven_days: 6, + seven_plus_days: 4, + }, + time_in_phase_median_hours: { awaiting_first_review: 9 }, + }), + ); + const out = computeCycleTime(repos, payloads)!; + expect(out.flow!.flowCoveragePct).toBe(0.4); + expect(out.flow!.prsWithFlow).toBe(40); + }); + + it("leaves data.flow null when phase data has no flow_pr_count (old payloads) — M3", () => { + const payloads = new Map(); + payloads.set( + "r1", + metrics({ + pr_merged_count: 60, + // time_in_phase present but NO flow_pr_count → cannot trust coverage + time_in_phase_median_hours: { awaiting_first_review: 5 }, + pr_cycle_time_buckets: { + same_day: 40, + one_day: 12, + two_to_three_days: 4, + four_to_seven_days: 2, + seven_plus_days: 2, + }, + }), + ); + const out = computeCycleTime(repos, payloads)!; + expect(out.flow).toBeNull(); }); it("leaves data.flow null when no payload carries phase data", () => { @@ -275,6 +347,7 @@ describe("computeCycleTime — flow decomposition", () => { "r1", metrics({ pr_merged_count: 10, + flow_pr_count: 10, pr_cycle_time_buckets: { same_day: 8, one_day: 2, From 1fc83777b21027d2ff8f8daea31df746ebf49f23 Mon Sep 17 00:00:00 2001 From: Marcos Sabatino Date: Sat, 20 Jun 2026 03:20:32 -0300 Subject: [PATCH 3/3] fix(dashboard): gate phase bar to the verdict variant; show wait vs active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up from the post-fix re-review, which found one new major (a UI consistency regression that the M3 coverage fix made routine) plus the orphaned isWait field. - FlowPhaseBar now renders only when verdict.variant === "verdict". Before, it showed in lowCoverage and none too (dominantPhase is set in every variant), so an affirmative phase bar appeared next to a "partial sample, not a verdict" banner — and below the density floor with no banner at all. Honest coverage (M3) made lowCoverage common, so this was about to ship as a systematic contradiction. - Re-expose the wait/active signal lost when M4 dropped the waitTag: the phase bar now hatches wait-phase segments (via WAIT_PHASES) with a short legend note, instead of a redundant text tag. The orphaned DominantPhase.isWait field (its only reader was the removed waitTag) is deleted; tests assert wait-ness via WAIT_PHASES directly. TS 231 pass, tsc clean, cycle-time-flow.ts 100% lines / 97.5% branch. Co-Authored-By: Claude Opus 4.8 (1M context) --- platform/lib/queries/cycle-time-flow.ts | 1 - platform/lib/translations.ts | 2 ++ .../[tenant]/dashboard/sections/CycleTime.tsx | 3 ++- .../src/components/charts/FlowPhaseBar.tsx | 20 +++++++++++++++++-- platform/src/types/org-summary.ts | 2 -- platform/tests/cycle-time-flow.test.ts | 5 ++--- 6 files changed, 24 insertions(+), 9 deletions(-) diff --git a/platform/lib/queries/cycle-time-flow.ts b/platform/lib/queries/cycle-time-flow.ts index cbee13f..41ccfb4 100644 --- a/platform/lib/queries/cycle-time-flow.ts +++ b/platform/lib/queries/cycle-time-flow.ts @@ -141,7 +141,6 @@ export function summarizeFlow( key, hours: phaseMedianHours[key], sharePct: round((phaseMedianHours[key] / windowHours) * 100, 1), - isWait: WAIT_PHASES.has(key), }; } diff --git a/platform/lib/translations.ts b/platform/lib/translations.ts index 6055316..12edd62 100644 --- a/platform/lib/translations.ts +++ b/platform/lib/translations.ts @@ -387,6 +387,7 @@ export const translations = { flowBarTitle: "Where PR time goes", flowBarSubtitle: "Code-window time by phase (median, weighted by decomposed PRs).", + waitNote: "Hatched = queue (wait) time.", phaseLabels: { coding: "Coding", awaiting_first_review: "Awaiting first review", @@ -1780,6 +1781,7 @@ export const translations = { flowBarTitle: "Onde o tempo do PR é gasto", flowBarSubtitle: "Tempo da janela de código por fase (mediana, ponderada por PRs decompostos).", + waitNote: "Hachurado = tempo em fila (espera).", phaseLabels: { coding: "Escrita de código", awaiting_first_review: "Espera pelo 1º review", diff --git a/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx b/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx index 482a43a..c0e8605 100644 --- a/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx +++ b/platform/src/app/[tenant]/dashboard/sections/CycleTime.tsx @@ -127,7 +127,7 @@ export function CycleTime({ data }: CycleTimeProps) { )} - {data.flow && verdict.dominantPhase && ( + {verdict.variant === "verdict" && data.flow && verdict.dominantPhase && ( {t("dashboard.cycleTime.flowBarTitle")} @@ -141,6 +141,7 @@ export function CycleTime({ data }: CycleTimeProps) { labels={phaseLabels(t)} formatHours={formatHoursAsDays} dominantKey={verdict.dominantPhase.key} + waitNote={t("dashboard.cycleTime.waitNote")} /> diff --git a/platform/src/components/charts/FlowPhaseBar.tsx b/platform/src/components/charts/FlowPhaseBar.tsx index ff18092..36d5bd1 100644 --- a/platform/src/components/charts/FlowPhaseBar.tsx +++ b/platform/src/components/charts/FlowPhaseBar.tsx @@ -1,4 +1,4 @@ -import { WINDOW_PHASES } from "@/lib/queries/cycle-time-flow"; +import { WAIT_PHASES, WINDOW_PHASES } from "@/lib/queries/cycle-time-flow"; import { cn } from "@/lib/utils"; import type { FlowPhaseKey } from "@/types/org-summary"; @@ -23,6 +23,8 @@ interface FlowPhaseBarProps { formatHours: (hours: number) => string; /** Phase to highlight as the dominant (widest) one, if any. */ dominantKey?: FlowPhaseKey | null; + /** Localized footnote explaining the wait-phase hatch (optional). */ + waitNote?: string; } /** @@ -35,6 +37,7 @@ export function FlowPhaseBar({ labels, formatHours, dominantKey, + waitNote, }: FlowPhaseBarProps) { const total = WINDOW_PHASES.reduce( (sum, key) => sum + (phaseHours[key] ?? 0), @@ -57,7 +60,15 @@ export function FlowPhaseBar({ FLOW_PHASE_COLORS[key], key === dominantKey && "ring-2 ring-inset ring-foreground/40", )} - style={{ width: `${pct}%` }} + style={{ + width: `${pct}%`, + // Wait phases (queue time) get a diagonal hatch so active vs + // wait reads at a glance — the flow-efficiency signal. + ...(WAIT_PHASES.has(key) && { + backgroundImage: + "repeating-linear-gradient(45deg, rgba(255,255,255,0.35) 0, rgba(255,255,255,0.35) 1.5px, transparent 1.5px, transparent 4px)", + }), + }} title={`${labels[key]}: ${formatHours(hours)}`} /> ); @@ -94,6 +105,11 @@ export function FlowPhaseBar({ ); })}
+ {waitNote && ( +

+ {waitNote} +

+ )}
); } diff --git a/platform/src/types/org-summary.ts b/platform/src/types/org-summary.ts index 375d535..4951de0 100644 --- a/platform/src/types/org-summary.ts +++ b/platform/src/types/org-summary.ts @@ -112,8 +112,6 @@ export interface DominantPhase { * Computed over the post-open window only (excludes `coding`). */ sharePct: number; - /** True when the phase is a wait (queue) phase — the actionable kind. */ - isWait: boolean; } /** diff --git a/platform/tests/cycle-time-flow.test.ts b/platform/tests/cycle-time-flow.test.ts index 395e515..a372ff5 100644 --- a/platform/tests/cycle-time-flow.test.ts +++ b/platform/tests/cycle-time-flow.test.ts @@ -32,8 +32,8 @@ describe("summarizeFlow", () => { key: "in_review_active", hours: 5, sharePct: 58.8, // round(5 / 8.5 * 100, 1) - isWait: false, }); + expect(WAIT_PHASES.has(out.dominantPhase!.key)).toBe(false); expect(out.prsWithFlow).toBe(40); expect(out.flowCoveragePct).toBe(1); }); @@ -54,7 +54,7 @@ describe("summarizeFlow", () => { expect(out.dominantPhase?.key).toBe("awaiting_first_review"); expect(out.dominantPhase?.hours).toBe(8); expect(out.dominantPhase?.sharePct).toBe(80); // 8 / 10 window total - expect(out.dominantPhase?.isWait).toBe(true); + expect(WAIT_PHASES.has(out.dominantPhase!.key)).toBe(true); }); it("reports partial coverage from the decomposed count, not total merged — M3", () => { @@ -151,7 +151,6 @@ function flow(over: Partial): FlowDecomposition { key: "awaiting_first_review", hours: 4, sharePct: 57.1, - isWait: true, }, prsWithFlow: 80, flowCoveragePct: 0.9,