diff --git a/platform/lib/pr-enrichment.ts b/platform/lib/pr-enrichment.ts index f8157a2..b51b24c 100644 --- a/platform/lib/pr-enrichment.ts +++ b/platform/lib/pr-enrichment.ts @@ -18,10 +18,44 @@ const KNOWN_STEPS: Record = { */ export function prDegradedSteps(value: unknown): PrReadStep[] { if (!Array.isArray(value)) return []; - return value.filter( + const steps = value.filter( (step): step is PrReadStep => typeof step === "string" && Object.hasOwn(KNOWN_STEPS, step), ); + // Same step twice adds nothing; keep the first occurrence's position. + return [...new Set(steps)]; +} + +/** The `t()` of both `useTranslation` and `getServerTranslation`. */ +export type Translate = ( + path: string, + params?: Record, +) => string; + +/** Degraded steps as a localized, comma-separated list for tooltips/notes. */ +export function formatPrSteps( + steps: readonly PrReadStep[], + t: Translate, +): string { + return steps.map((step) => t(`repos.detail.prData.steps.${step}`)).join(", "); +} + +/** Most repo names the dashboard note lists before collapsing the rest. */ +export const MAX_LISTED_REPOS = 5; + +/** + * Repo names for the dashboard note: the first `max`, then a localized + * "and N more" for the remainder, so one bad batch cannot flood the page. + */ +export function formatRepoNames( + names: readonly string[], + t: Translate, + max: number = MAX_LISTED_REPOS, +): string { + const listed = names.slice(0, max).join(", "); + const rest = names.length - max; + if (rest <= 0) return listed; + return `${listed} ${t("dashboard.prDataMoreRepos", { count: rest })}`; } export interface DegradedRepo { @@ -49,10 +83,3 @@ export function degradedReposFromPayloads( } return degraded; } - -/** Whether any row of a table is marked as having a partial PR read. */ -export function anyPrDataIncomplete( - rows: ReadonlyArray<{ pr_degraded_steps: readonly string[] }>, -): boolean { - return rows.some((row) => row.pr_degraded_steps.length > 0); -} diff --git a/platform/lib/queries/temporal.ts b/platform/lib/queries/temporal.ts index defc052..d6a128a 100644 --- a/platform/lib/queries/temporal.ts +++ b/platform/lib/queries/temporal.ts @@ -17,9 +17,30 @@ import { classifyHealth } from "@/types/temporal"; const SPARKLINE_POINTS = 12; -// Loosely typed on purpose: this mirrors the view's columns (see -// SUMMARY_COLUMNS) as the untyped client used to return them. -type SummaryRow = Record; +// The columns of `repo_metric_summaries` that SUMMARY_COLUMNS selects, plus the +// optional payload key. Columns are nullable because the view aggregates over +// runs that may lack a metric. +interface SummaryRow { + repository_id: string; + runs_count: number | null; + last_run_at: string | null; + stabilization_ratio: number | null; + prev_stabilization_ratio: number | null; + revert_rate: number | null; + churn_events: number | null; + commits_total: number | null; + ai_detection_coverage_pct: number | null; + pr_merged_count: number | null; + pr_single_pass_rate: number | null; + fix_latency_median_hours: number | null; + cascade_rate: number | null; + merge_strategy: string | null; + commit_metrics_reliable: boolean | null; + // Newest first; entries can be null. + recent_stabilization: Array | null; + // Untrusted JSON from the payload; read through `prDegradedSteps`. + pr_enrichment_degraded?: unknown; +} const SUMMARY_COLUMNS = "repository_id, runs_count, last_run_at, stabilization_ratio, prev_stabilization_ratio, revert_rate, churn_events, commits_total, ai_detection_coverage_pct, pr_merged_count, pr_single_pass_rate, fix_latency_median_hours, cascade_rate, merge_strategy, commit_metrics_reliable, recent_stabilization"; @@ -212,6 +233,17 @@ export async function getRepoAITimeSeries( .filter((p) => p.ai_pct !== null && p.ai_pct > 0); } +/** + * Whether a PostgREST error says the view has no `payload` column: PostgreSQL's + * undefined_column (42703), or a PostgREST-side error naming the column. + */ +function isMissingPayloadColumn(error: PostgrestError): boolean { + return ( + error.code === "42703" || + /payload|pr_enrichment_degraded/i.test(error.message ?? "") + ); +} + /** Get summary for all repos in an org (latest + previous for delta). * * Uses 2 bulk queries instead of 3N+1 per-repo queries: @@ -224,11 +256,17 @@ export async function getRepoAITimeSeries( * window, repos whose latest run fell outside the newest-1000 slice came back * with zero rows and rendered "0 runs" despite having metrics. The view * returns ~one row per repo, so the result set stays well under any cap. + * + * `options.includePrDegraded` also selects `payload->pr_enrichment_degraded`. + * It is off by default because reading any key of `payload` makes the view + * aggregate the payload of every run, which only the compare table needs. + * Without it every row has `pr_degraded_steps: []`. */ export async function getOrgReposSummary( supabase: SupabaseClient, organizationId: string, windowDays: number = DEFAULT_WINDOW_DAYS, + options: { includePrDegraded?: boolean } = {}, ): Promise { // Query 1: all repos const { data: repos, error: reposError } = await supabase @@ -241,29 +279,39 @@ export async function getOrgReposSummary( if (!repos || repos.length === 0) return []; // Query 2: pre-aggregated summary, one row per repo (see doc comment). - // The column list is built at runtime, so supabase-js can't infer the row. - const readSummaries = async (columns: string) => - (await supabase + const readSummaries = async (columns: string) => { + const result = await supabase .from("repo_metric_summaries") .select(columns) .eq("organization_id", organizationId) - .eq("window_days", windowDays)) as unknown as { + .eq("window_days", windowDays); + // The column list is built at runtime, so supabase-js can't infer the row. + return result as unknown as { data: SummaryRow[] | null; error: PostgrestError | null; }; - - // Selecting one key out of `payload` marks degraded PR reads without - // shipping whole payloads. The view only has `payload` since migration 023; - // when the select fails (older database) retry without it so the table stays - // populated, just unmarked. - let { data: summaries, error: summariesError } = await readSummaries( - `${SUMMARY_COLUMNS}, pr_enrichment_degraded:payload->pr_enrichment_degraded`, - ); - if (summariesError) { - logQueryError( - "getOrgReposSummary (repo_metric_summaries with payload)", - summariesError, - ); + }; + + let summaries: SummaryRow[] | null; + let summariesError: PostgrestError | null; + if (options.includePrDegraded) { + // Selecting one key out of `payload` marks degraded PR reads without + // shipping whole payloads. The view only has `payload` since migration + // 023; when the column is missing (older database) retry without it so the + // table stays populated, just unmarked. Any other error (a timeout, say) + // is not retried: it goes the normal error path below. + ({ data: summaries, error: summariesError } = await readSummaries( + `${SUMMARY_COLUMNS}, pr_enrichment_degraded:payload->pr_enrichment_degraded`, + )); + if (summariesError && isMissingPayloadColumn(summariesError)) { + logQueryError( + "getOrgReposSummary (repo_metric_summaries with payload)", + summariesError, + ); + ({ data: summaries, error: summariesError } = + await readSummaries(SUMMARY_COLUMNS)); + } + } else { ({ data: summaries, error: summariesError } = await readSummaries(SUMMARY_COLUMNS)); } @@ -287,7 +335,7 @@ export async function getOrgReposSummary( const sparkline = (s?.recent_stabilization ?? []) .slice(0, SPARKLINE_POINTS) .reverse() - .filter((v: number | null): v is number => v !== null); + .filter((v): v is number => v !== null); return { id: repo.id, @@ -306,7 +354,9 @@ export async function getOrgReposSummary( cascade_rate: s?.cascade_rate ?? null, merge_strategy: s?.merge_strategy ?? null, commit_metrics_reliable: s?.commit_metrics_reliable ?? null, - pr_degraded_steps: prDegradedSteps(s?.pr_enrichment_degraded), + pr_degraded_steps: options.includePrDegraded + ? prDegradedSteps(s?.pr_enrichment_degraded) + : [], stabilization_delta: delta, health: classifyHealth(stabilization), sparkline, diff --git a/platform/lib/translations.ts b/platform/lib/translations.ts index 36047ce..2b89e53 100644 --- a/platform/lib/translations.ts +++ b/platform/lib/translations.ts @@ -267,6 +267,7 @@ export const translations = { member: "Member", }, dashboard: { + prDataMoreRepos: "and {count} more", prDataIncomplete: "PR data is incomplete in the latest run of {count} repository(ies) ({repos}); PR metrics below may be missing or skewed.", pulse: { @@ -622,8 +623,14 @@ export const translations = { }, prData: { incomplete: "PR data incomplete", + steps: { + basic: "PR list", + enrichment: "commit enrichment", + reviews: "reviews", + fetch: "entire read", + }, incompleteTooltip: - "This run's PR read failed at: {steps}. Merge strategy isn't classified when enrichment fails, and flow, review and in-PR metrics may be missing or skewed.", + "This run's PR read failed at: {steps}. Merge strategy isn't classified when enrichment fails for merged PRs, and PR lifecycle (counts, time to merge, cycle time), flow, review, in-PR and open-PR metrics may be missing or skewed.", }, dora: { title: "DORA", @@ -1833,6 +1840,7 @@ export const translations = { member: "Membro", }, dashboard: { + prDataMoreRepos: "e mais {count}", prDataIncomplete: "Os dados de PR estão incompletos na execução mais recente de {count} repositório(s) ({repos}); as métricas de PR abaixo podem faltar ou sair distorcidas.", pulse: { @@ -2189,8 +2197,14 @@ export const translations = { }, prData: { incomplete: "Dados de PR incompletos", + steps: { + basic: "listagem de PRs", + enrichment: "enriquecimento de commits", + reviews: "reviews", + fetch: "leitura inteira", + }, incompleteTooltip: - "A leitura de PRs desta execução falhou em: {steps}. A estratégia de merge não é classificada quando o enriquecimento falha, e métricas de flow, de review e de commits em PR podem faltar ou sair distorcidas.", + "A leitura de PRs desta execução falhou em: {steps}. A estratégia de merge não é classificada quando o enriquecimento dos PRs mergeados falha, e métricas de ciclo de vida de PR (contagens, tempo até o merge, cycle time), de flow, de review, de commits em PR e de PRs abertos podem faltar ou sair distorcidas.", }, dora: { title: "DORA", diff --git a/platform/src/app/[tenant]/compare/compare-view.tsx b/platform/src/app/[tenant]/compare/compare-view.tsx index 07c7df9..c3ef5f4 100644 --- a/platform/src/app/[tenant]/compare/compare-view.tsx +++ b/platform/src/app/[tenant]/compare/compare-view.tsx @@ -7,7 +7,7 @@ import { Search } from "lucide-react"; import { Sparkline } from "@/components/charts/Sparkline"; import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { useTranslation } from "@/hooks/useTranslation"; -import { anyPrDataIncomplete } from "@/lib/pr-enrichment"; +import { formatPrSteps } from "@/lib/pr-enrichment"; import { cn } from "@/lib/utils"; import type { RepoSummary } from "@/types/temporal"; import { healthIndicator } from "@/types/temporal"; @@ -218,14 +218,47 @@ function MergeStrategyCell({ ); } +/** + * The failed PR-read steps of a repo whose latest run read PR data only + * partially, on a line of their own under the repo name in both layouts: an + * amber dot (the legend under the list says what it means, so nothing hinges + * on colour) and the steps as text, for touch and keyboard users who get no + * hover. Amber 600 on light and 500 on dark keep the dot above 3:1 against + * the card (WCAG 1.4.11). + * + * The dot and the visible steps are aria-hidden, with the full sentence as + * their hover title; screen readers get that sentence once, as sr-only text. + */ +function PrDataSteps({ + repo, + label, + steps, +}: { + repo: RepoSummary; + label: string; + steps: string; +}) { + if (repo.pr_degraded_steps.length === 0) return null; + return ( + + + ); +} + export function CompareView({ repos }: CompareViewProps) { const { t } = useTranslation(); - // Amber dot next to a repo whose latest run read PR data only partially. - // The text is also in the DOM (sr-only), and a legend under the table says - // what the dot means, so it doesn't hinge on colour or on a hover tooltip. const prDataTooltip = (repo: RepoSummary) => t("repos.detail.prData.incompleteTooltip", { - steps: repo.pr_degraded_steps.join(", "), + steps: formatPrSteps(repo.pr_degraded_steps, t), }); const [sort, setSort] = useState({ key: "stabilization_ratio", @@ -419,15 +452,11 @@ export function CompareView({ repos }: CompareViewProps) { {repo.name} - {repo.pr_degraded_steps.length > 0 && ( - - - )} + - {anyPrDataIncomplete(sorted) && ( -

-

- )} {/* Mobile: sort control + card stack */} @@ -539,13 +559,20 @@ export function CompareView({ repos }: CompareViewProps) { className="flex flex-col gap-3 rounded-lg border border-border/60 p-3" >
-
- - #{i + 1} - - - {repo.name} - +
+
+ + #{i + 1} + + + {repo.name} + +
+
+ + {sorted.some((repo) => repo.pr_degraded_steps.length > 0) && ( +

+

+ )} ); diff --git a/platform/src/app/[tenant]/compare/page.tsx b/platform/src/app/[tenant]/compare/page.tsx index 88766d4..27debab 100644 --- a/platform/src/app/[tenant]/compare/page.tsx +++ b/platform/src/app/[tenant]/compare/page.tsx @@ -44,7 +44,9 @@ export default async function ComparePage({ availableWindows, ); - const repos = await getOrgReposSummary(supabaseAdmin, org.id, windowDays); + const repos = await getOrgReposSummary(supabaseAdmin, org.id, windowDays, { + includePrDegraded: true, + }); const { t } = await getServerTranslation(); return ( diff --git a/platform/src/app/[tenant]/dashboard/panels/PRDataNotice.tsx b/platform/src/app/[tenant]/dashboard/panels/PRDataNotice.tsx index 6d115f3..f86af60 100644 --- a/platform/src/app/[tenant]/dashboard/panels/PRDataNotice.tsx +++ b/platform/src/app/[tenant]/dashboard/panels/PRDataNotice.tsx @@ -4,7 +4,10 @@ import { type DashboardPanelProps, } from "../data"; -import { degradedReposFromPayloads } from "@/lib/pr-enrichment"; +import { + degradedReposFromPayloads, + formatRepoNames, +} from "@/lib/pr-enrichment"; import { getServerTranslation } from "@/lib/server-translation"; /** @@ -28,7 +31,10 @@ export async function PRDataNotice({ orgId, windowDays }: DashboardPanelProps) { > {t("dashboard.prDataIncomplete", { count: degraded.length, - repos: degraded.map((r) => r.name).join(", "), + repos: formatRepoNames( + degraded.map((r) => r.name), + t, + ), })}

); diff --git a/platform/src/app/[tenant]/repos/[repoName]/page.tsx b/platform/src/app/[tenant]/repos/[repoName]/page.tsx index eaa0128..6cc7666 100644 --- a/platform/src/app/[tenant]/repos/[repoName]/page.tsx +++ b/platform/src/app/[tenant]/repos/[repoName]/page.tsx @@ -13,7 +13,7 @@ import { MetricCard } from "@/components/charts/MetricCard"; import { Badge } from "@/components/ui/badge"; import { WindowSelector } from "@/components/WindowSelector"; import { authOptions } from "@/lib/auth"; -import { prDegradedSteps } from "@/lib/pr-enrichment"; +import { formatPrSteps, prDegradedSteps } from "@/lib/pr-enrichment"; import { extractAdoptionSummary } from "@/lib/queries/adoption-timeline"; import { computeRepoDORA } from "@/lib/queries/dora"; import { computeInvestmentHotspots } from "@/lib/queries/invest-here"; @@ -254,18 +254,23 @@ export default async function RepoDetailPage({ {insights.mergeStrategy} ))} - {/* In the header, not in the flow or merge panels: both are absent - exactly when the read degraded. */} + {/* In the header, not in the flow or merge panels: those can + disappear when the read degrades, which would hide the cause. + The failed steps are text next to the badge, for touch and + keyboard users too; no title repeating them, which screen + readers may read as a description on top of the text. */} {prDegraded.length > 0 && ( - - {t("repos.detail.prData.incomplete")} - + + + {t("repos.detail.prData.incomplete")} + + + {formatPrSteps(prDegraded, t)} + + )}
{repo.remote_url && ( diff --git a/platform/tests/compare-pr-data-marker.test.tsx b/platform/tests/compare-pr-data-marker.test.tsx new file mode 100644 index 0000000..75dea81 --- /dev/null +++ b/platform/tests/compare-pr-data-marker.test.tsx @@ -0,0 +1,105 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vitest"; + +import { translations } from "@/lib/translations"; +import type { RepoSummary } from "@/types/temporal"; + +// The real hook reads the session; the view only needs `t()` over en-US. +vi.mock("@/hooks/useTranslation", () => ({ + useTranslation: () => ({ + t: (path: string, params?: Record) => { + let value: unknown = translations["en-US"]; + for (const key of path.split(".")) { + value = (value as Record)?.[key]; + } + let text = String(value); + for (const [k, v] of Object.entries(params ?? {})) { + text = text.replace(`{${k}}`, String(v)); + } + return text; + }, + }), +})); + +function repo( + id: string, + name: string, + steps: RepoSummary["pr_degraded_steps"], +): RepoSummary { + return { + id, + name, + remote_url: null, + last_run_at: null, + runs_count: 1, + stabilization_ratio: 0.9, + revert_rate: 0, + churn_events: 1, + commits_total: 10, + ai_detection_coverage_pct: null, + pr_merged_count: 1, + pr_single_pass_rate: null, + fix_latency_median_hours: null, + cascade_rate: null, + merge_strategy: null, + commit_metrics_reliable: null, + pr_degraded_steps: steps, + stabilization_delta: null, + health: "healthy", + sparkline: [], + }; +} + +async function render(): Promise { + const { CompareView } = await import("@/app/[tenant]/compare/compare-view"); + return renderToStaticMarkup( + , + ); +} + +const count = (text: string, part: string) => text.split(part).length - 1; + +describe("CompareView PR-data marker", () => { + // Touch and keyboard users get no hover title, and sighted keyboard users + // on the desktop table neither: both layouts show the steps as text. + it("shows the failed steps as text in the table and on the card", async () => { + const html = await render(); + + expect(count(html, "PR list, reviews")).toBe(2); + }); + + // Each layout reads the full sentence once: the visible steps (and the + // dot) are aria-hidden, and only the sr-only sentence is announced. The + // table and the card are never shown at once, so a reader meets one. + it("announces the full sentence once per layout", async () => { + const html = await render(); + + expect(count(html, '