diff --git a/src/frontend/src/api/metric-definitions-client.test.ts b/src/frontend/src/api/metric-definitions-client.test.ts index 69f198208..cc6cd3cbc 100644 --- a/src/frontend/src/api/metric-definitions-client.test.ts +++ b/src/frontend/src/api/metric-definitions-client.test.ts @@ -27,6 +27,7 @@ const METRIC: MetricDefinition = { direction: "higher_is_better", dimensions: ["repo"], is_enabled: true, + origin: "builtin" as const, schema_status: "ok", schema_error_code: null, last_observed_date: "2026-07-20", diff --git a/src/frontend/src/api/metric-definitions-client.ts b/src/frontend/src/api/metric-definitions-client.ts index 17e203cea..103c35b6c 100644 --- a/src/frontend/src/api/metric-definitions-client.ts +++ b/src/frontend/src/api/metric-definitions-client.ts @@ -21,6 +21,8 @@ const BASE = export type MetricDefinitionSchemaStatus = "ok" | "error" | "unchecked"; +export type MetricDefinitionOrigin = "builtin" | "custom"; + export interface MetricDefinition { metric_key: string; label: string; @@ -32,6 +34,14 @@ export interface MetricDefinition { direction: MetricDirection; dimensions: string[]; is_enabled: boolean; + /** + * `builtin` reads managed observation relations; `custom` executes inline + * SQL at query time. The validator stamps `schema_status` and + * `last_observed_date` from materialized relations only, so for `custom` + * they stay "unchecked" / absent however much data the metric serves — + * reading their absence as "never measured" is wrong for those. + */ + origin: MetricDefinitionOrigin; schema_status: MetricDefinitionSchemaStatus; /** Why schema_status is "error"; null otherwise. */ schema_error_code: MetricSchemaErrorCode | null; diff --git a/src/frontend/src/components/portal/context-pane.test.tsx b/src/frontend/src/components/portal/context-pane.test.tsx index 6aa3164d7..91f41a457 100644 --- a/src/frontend/src/components/portal/context-pane.test.tsx +++ b/src/frontend/src/components/portal/context-pane.test.tsx @@ -77,7 +77,7 @@ describe("ContextPane", () => { expect(screen.getByText("Overview")).toBeInTheDocument(); expect(screen.getByText("Cross-functional org rollup")).toBeInTheDocument(); const item = renderHook(() => usePortalItem()); - await userEvent.click(screen.getByText("Health radar")); + await userEvent.click(screen.getByText("What we can see")); expect(item.result.current).toBe("health"); }); diff --git a/src/frontend/src/components/portal/domain-lens-view.test.tsx b/src/frontend/src/components/portal/domain-lens-view.test.tsx index e93ace477..2a667dc74 100644 --- a/src/frontend/src/components/portal/domain-lens-view.test.tsx +++ b/src/frontend/src/components/portal/domain-lens-view.test.tsx @@ -15,8 +15,10 @@ vi.mock("@tanstack/react-router", async () => { import { portalRouter } from "@/test/portal-router"; import { act, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { GROUPS } from "@/lib/insight/groups"; import type { NormalizedMetricResult } from "@/lib/metrics/collection"; import { identityPerson, pid } from "@/test/identity"; import type { IdentityPerson } from "@/types/insight"; @@ -35,6 +37,8 @@ const mocks = vi.hoisted(() => ({ refetch: vi.fn(), }, call: 0, + collectionSet: new Map(), + definitions: [] as unknown[], collections: [] as Array<{ byKey: Map; isPending: boolean; @@ -68,6 +72,16 @@ vi.mock("@/queries/metric-results", () => ({ mocks.call += 1; return r; }, + // Only the coverage section reads the set form; it fetches its own + // period-only collection rather than riding the zone grid. + useMetricCollectionSet: () => mocks.collectionSet, +})); +vi.mock("@/queries/metric-definitions", async (orig) => ({ + ...(await orig>()), + useMetricDefinitionsResponse: () => ({ + data: { metrics: mocks.definitions }, + isPending: false, + }), })); vi.mock("@/hooks/use-portal-period", () => ({ usePortalPeriod: () => ({ @@ -411,7 +425,7 @@ describe("by-unit auto-section (rule 7: slice cohorts inside scope)", () => { }); }); -describe("direction-cards / coverage-radar / attention sections", () => { +describe("direction-cards / attention sections", () => { it("renders attention rows for cohort outliers, named and linked (O3)", () => { // 7 healthy + 1 collapsed member const labels = ["m1", "m2", "m3", "m4", "m5", "m6", "m7", "z"]; @@ -434,16 +448,68 @@ describe("direction-cards / coverage-radar / attention sections", () => { expect(screen.getByText(/no commits/)).toBeInTheDocument(); }); - it("suppresses the coverage radar below the minimum cohort", () => { - mocks.tree = person("boss", {}, [person("a"), person("b")]); +}); + +describe("coverage section (#2408)", () => { + /** One group reading for the named people, the rest silent. */ + function coverageWorld(seenByGitOutput: string[]) { + const ids = seenByGitOutput.map((l) => pid(l)); + mocks.definitions = GROUPS.flatMap((g) => + g.collection.metrics.map((m) => ({ + metric_key: m.key, + is_enabled: true, + schema_status: "ok", + schema_error_code: null, + // Only git output has ever observed anything for this tenant, so every + // other part must read as "no connector" rather than as idle people. + last_observed_date: g.id === "git_output" ? "2026-07-26" : null, + })), + ); + const gitKey = GROUPS.find((g) => g.id === "git_output")!.collection + .metrics[0]!.key; + mocks.collectionSet = new Map( + GROUPS.map((g) => [ + g.id, + { + byKey: + g.id === "git_output" + ? new Map([ + [gitKey, metric(gitKey, ids.map((id) => [id, 3]))], + ]) + : new Map(), + isPending: false, + }, + ]), + ); + } + + it("opens a level into exactly the people at it, and why each is thin", async () => { + mocks.tree = person("boss", {}, [person("a"), person("b"), person("c")]); + coverageWorld(["a"]); + render( + , + ); + + // One person reads in one part; the other two read in none. + await userEvent.click(screen.getByRole("button", { name: /1 of 5/ })); + expect(screen.getByText("a")).toBeInTheDocument(); + expect(screen.queryByText("b")).not.toBeInTheDocument(); + + // And the reason is the actionable half: nothing feeds those parts for the + // tenant, which is a plumbing job — not people who did no work. + expect(screen.getAllByText(/no connector:/).length).toBeGreaterThan(0); + }); + + it("does not open a level nobody is at", async () => { + mocks.tree = person("boss", {}, [person("a")]); + coverageWorld([]); render( , ); - expect(screen.queryByText("Health radar")).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: /5 of 5/ })).toBeDisabled(); }); }); diff --git a/src/frontend/src/components/portal/domain-lens-view.tsx b/src/frontend/src/components/portal/domain-lens-view.tsx index 5ef0e5ee9..38cec4d77 100644 --- a/src/frontend/src/components/portal/domain-lens-view.tsx +++ b/src/frontend/src/components/portal/domain-lens-view.tsx @@ -1,15 +1,7 @@ +import { Link } from "@tanstack/react-router"; import { useMemo, useState } from "react"; import { MetricName } from "@/components/widgets/metric-help-tooltip"; import { ArrowDownRight, ArrowUpRight } from "lucide-react"; -import { - PolarAngleAxis, - PolarGrid, - PolarRadiusAxis, - Radar, - RadarChart, - ResponsiveContainer, -} from "recharts"; - import { AttentionList } from "@/components/portal/attention-list"; import { ComingSoon } from "@/components/widgets/coming-soon"; import { orgScopeGate } from "@/components/portal/org-scope-gate"; @@ -32,6 +24,8 @@ import { computeAttentionFlags, } from "@/lib/insight/attention-flags"; import { GROUPS } from "@/lib/insight/groups"; +import type { PersonCoverage } from "@/lib/insight/coverage"; +import { useScopeCoverage } from "@/lib/portal/use-scope-coverage"; import { availableSlices, cohortKey, @@ -55,7 +49,6 @@ import { distribution, familyObserved, fmtCompact, - groupCoverage, medianAcross, perCapita, representative, @@ -290,7 +283,14 @@ export function DomainLensView({ if (gate) return gate; // Rule 6: nothing in this family was ever observed → the source isn't wired. - if (!familyObserved(grid.byKey, lensKeys, memberIds)) { + // + // Exempt: a lens that reads no metric of its own cannot be judged by whether + // its metrics were observed, and one whose SUBJECT is coverage must survive + // exactly the case this gate fires on. Telling a reader "no source is + // ingested" on the screen built to tell them which sources are not ingested + // would withhold the answer at the moment it is worth most. + const readsGrid = config.sections.some((s) => s.kind !== "coverage-levels"); + if (readsGrid && !familyObserved(grid.byKey, lensKeys, memberIds)) { return ( ); - case "coverage-radar": - return ; + case "coverage-levels": + return ( + + ); } } +/* ── coverage (#2408): three cuts of one model, read top to bottom — the + verdict, then which parts are missing, then who is thinly seen. Little + prose on purpose: a screen that needs a paragraph to explain itself has + already failed the reader who only glanced at it. ──────────────────── */ + +function CoverageBar({ + filled, + total, + warn, +}: { + filled: number; + total: number; + warn?: boolean; +}) { + return ( +
+
0 ? (filled / total) * 100 : 0}%` }} + /> +
+ ); +} + +function CoverageLevelsSection({ + memberIds, + nameByEntity, + personIdByEntity, +}: { + memberIds: readonly string[]; + nameByEntity: Map; + personIdByEntity: Map; +}) { + const [openLevel, setOpenLevel] = useState(null); + const { distribution, parts, people, thin, isPending, isError } = + useScopeCoverage(memberIds); + if (isPending) return ; + // Before anything else. With a request failed nothing is known to reach the + // tenant, so every part would read "no data reaches us" and every person + // would sit at zero — a fault in our infrastructure printed as a verdict + // about named people. Saying we could not check is the only honest output. + if (isError) { + return ; + } + const counted = distribution.counted; + if (counted === 0) return null; + + const partCount = GROUPS.length; + const levels = [...distribution.byLevel.entries()].sort((a, b) => b[0] - a[0]); + const missing = parts.filter((p) => p.unreachable); + + return ( +
+ {/* 1 — the verdict, as a number rather than a sentence: it is meant to + be seen, not parsed. */} +
+ + {/* Same amber as the rows it is the sum of. The link between the + number and the block of bars is the one thing a reader has to + make unaided, and colour makes it without a caption. */} + {thin} + /{counted} + +

+ people are seen in fewer than half of their work — everything else + this product says about them rests on that fraction. +

+
+ + {/* 2 — where it is missing. A part nothing reaches is NOT drawn as a bar + at zero, because that reads as people who did nothing, which is the + one thing it does not mean. */} +
+

+ By part of work +

+ {parts.map((part) => ( +
+ {part.title} + {part.unreachable ? ( + + {/* No cause named. Absent observations, a disabled metric and + a broken schema all land here, and only the first is a + missing connector — sending someone to plumb a live one is + the wrong direction to be wrong in. */} + nothing reaches us here + + ) : ( + + )} + + {part.unreachable ? "—" : part.seen} + +
+ ))} +
+ + {/* 3 — who. Colour carries the finding, so the shape reads without the + labels: the amber block IS the number at the top. */} +
+

+ By person · parts we can see +

+ {levels.map(([level, n], i) => { + const thinHere = level < partCount / 2; + // The rule sits where it applies. Without it the reader has to work + // out which rows the headline counted, and having to work it out is + // the same as not knowing it. + const boundary = + thinHere && !(levels[i - 1] && levels[i - 1]![0] < partCount / 2); + return ( +
+ {boundary && ( +
+ + fewer than half + + + + {thin} + +
+ )} + + {openLevel === level && ( + p.level === level)} + nameByEntity={nameByEntity} + personIdByEntity={personIdByEntity} + /> + )} +
+ ); + })} +
+ +

+ A part counts when any of its metrics has a value for that person this + period — so this says what we can see, never how well anyone did. + {missing.length > 0 && ( + <> + {" "} + Nobody here can reach the top of that scale, because{" "} + {missing.map((m) => m.title).join(", ")}{" "} + {missing.length === 1 ? "reaches" : "reach"} us for no one. + + )}{" "} + Counted over {counted} {counted === 1 ? "person" : "people"} you can + see in this scope. +

+
+ ); +} + +/** + * The people at one coverage level, and what is missing for each. + * + * The missing parts are the point, not the names. A level says how much we + * cannot see; this says which systems to go and look at — and separates the + * two kinds of absence, because they lead different places. "No connector" + * is somebody's job to fix. "Nothing recorded" is a person who does that work + * elsewhere, or does not do it, and no amount of plumbing changes it. + */ +function CoverageLevelPeople({ + id, + people, + nameByEntity, + personIdByEntity, +}: { + id: string; + people: readonly PersonCoverage[]; + nameByEntity: Map; + personIdByEntity: Map; +}) { + const titleById = new Map(GROUPS.map((g) => [g.id, g.title])); + const rows = [...people].sort((a, b) => + (nameByEntity.get(a.entityId) ?? a.entityId).localeCompare( + nameByEntity.get(b.entityId) ?? b.entityId, + ), + ); + + return ( +
    + {rows.map((p) => { + const unconnected: string[] = []; + const idle: string[] = []; + for (const [id, state] of p.states) { + const title = titleById.get(id) ?? id; + if (state === "no_data_reaches_us") unconnected.push(title); + else if (state === "nothing_recorded") idle.push(title); + } + const personId = personIdByEntity.get(p.entityId); + const name = nameByEntity.get(p.entityId) ?? p.entityId; + return ( +
  • + {personId ? ( + + {name} + + ) : ( + {name} + )} + {unconnected.length > 0 && ( + + no connector: {unconnected.join(", ")} + + )} + {idle.length > 0 && ( + + nothing recorded: {idle.join(", ")} + + )} +
    • + ); + })} +
+ ); +} + /* ── participation (rule 8 variant — "N of M are active") ────────────── */ function ParticipationSection({ @@ -1229,65 +1474,6 @@ function DirectionCardsSection({ ); } -/* ── coverage-radar (Overview design O5: coverage = share of members with ≥1 - OBSERVED metric per group — entityObserved, never zero-filled sums) ── */ - -function CoverageRadarSection({ - grid, - memberIds, -}: { - grid: GridData; - memberIds: readonly string[]; -}) { - if (memberIds.length < MIN_COHORT) return null; - const data = GROUPS.map((g) => ({ - domain: g.title, - coverage: Math.round( - (groupCoverage(grid.byKey, g.card.preview, memberIds) ?? 0) * 100 - ), - })); - if (data.every((d) => d.coverage === 0)) return null; - - return ( -
-

- Health radar -

- - -
- - - - - {/* Percent scale is absolute: full edge = 100% coverage, not - the period's max — and the explicit axis gives recharts a - radius scale (without one the polygon collapses to center). */} - - - - -
-

- Coverage — % of people in scope with any observed activity in each - domain this period. -

- - -
- ); -} - /* ── by-unit auto-section (rule 7) ───────────────────────────────────── */ const NO_COMPARABLE_UNITS_NOTE = diff --git a/src/frontend/src/components/portal/metric-groups-view.test.tsx b/src/frontend/src/components/portal/metric-groups-view.test.tsx index 921a65c8f..9ee3f69d5 100644 --- a/src/frontend/src/components/portal/metric-groups-view.test.tsx +++ b/src/frontend/src/components/portal/metric-groups-view.test.tsx @@ -30,6 +30,16 @@ const mocks = vi.hoisted(() => ({ cohort: [] as string[], })); +// usePersonSectionStandings now reads source availability from the tenant's +// definition listing rather than inferring it from an empty comparison pool. +vi.mock("@/queries/metric-definitions", async (orig) => ({ + ...(await orig>()), + useMetricDefinitionsResponse: () => ({ + data: { metrics: [] }, + isPending: false, + isError: false, + }), +})); vi.mock("@/queries/metric-results", () => ({ useMetricCollection: () => mocks.collection, useMetricCollectionSet: () => mocks.set, diff --git a/src/frontend/src/lib/insight/coverage.test.ts b/src/frontend/src/lib/insight/coverage.test.ts new file mode 100644 index 000000000..9bb2fdabd --- /dev/null +++ b/src/frontend/src/lib/insight/coverage.test.ts @@ -0,0 +1,372 @@ +/** + * What the product can see about a person, and about how many people. + * + * The tests that matter here are the ones about what it REFUSES to conclude: + * that a source is missing because nobody in view uses it, that an unlinked + * person is a person with nothing to show, or that connecting something would + * light up a knowable number of people. + */ +import { describe, expect, it } from "vitest"; + +import type { MetricDefinition } from "@/api/metric-definitions-client"; +import type { MetricResult } from "@/api/metric-results-client"; +import type { MetricGroup } from "@/lib/insight/groups"; +import { + coverageDistribution, + partCoverage, + partState, + personCoverage, + reachableMetricKeys, + thinlyCovered, + unreachableParts, +} from "@/lib/insight/coverage"; +import { normalizeMetricResults } from "@/lib/metrics/collection"; + +const ME = "019e27bc-dec0-7626-81a9-c5524662a6a9"; +const SOMEONE_ELSE = "019e27bc-dec0-7626-81a9-000000000002"; + +/** A definition as the listing delivers it; only the health fields matter. */ +function def( + metric_key: string, + over: Partial = {}, +): MetricDefinition { + return { + metric_key, + is_enabled: true, + origin: "builtin", + schema_status: "ok", + schema_error_code: null, + last_observed_date: "2026-08-01", + ...over, + } as unknown as MetricDefinition; +} + +/** One metric carrying a period value for the listed entities. */ +function metric( + key: string, + values: Array<[entity: string, value: number | null]>, +): MetricResult { + return { + metric_key: key, + label: key, + unit: null, + format: "integer", + computation: "sum", + direction: "higher_is_better", + views: [ + { + view: "period", + values: values.map(([entity_id, value]) => ({ entity_id, value })), + }, + ], + } as unknown as MetricResult; +} + +function group(id: string, keys: string[]): MetricGroup { + return { + id, + title: `${id} title`, + collection: { metrics: keys.map((key) => ({ key, views: [] })) }, + card: { preview: [] }, + drilldown: [], + } as unknown as MetricGroup; +} + +describe("reachableMetricKeys", () => { + it("counts a metric that has observed something", () => { + expect(reachableMetricKeys([def("git.commits")])).toEqual( + new Set(["git.commits"]), + ); + }); + + it("drops a metric that has never observed anything", () => { + // `last_observed_date: null` is the listing saying no data has ever + // arrived — the source is declared but nothing came through it. + expect( + reachableMetricKeys([def("ai.accepted", { last_observed_date: null })]), + ).toEqual(new Set()); + }); + + it("drops a disabled or schema-broken metric", () => { + expect( + reachableMetricKeys([ + def("wiki.edits", { is_enabled: false }), + def("task.closed", { + schema_status: "error", + schema_error_code: "table_not_found", + }), + ]), + ).toEqual(new Set()); + }); + + it("keeps a custom metric that has never stamped a freshness date", () => { + // The validator stamps last_observed_date from materialized relations + // only, and a custom metric runs its SQL at query time — so the field + // stays absent however much data it serves. Reading that as "never + // measured" would report a working metric as one nothing reaches, which is + // the one thing this function must never do. + expect( + reachableMetricKeys([ + def("custom.thing", { origin: "custom", last_observed_date: null }), + ]), + ).toEqual(new Set(["custom.thing"])); + }); + + it("still drops a custom metric that is disabled or schema-broken", () => { + expect( + reachableMetricKeys([ + def("custom.off", { origin: "custom", is_enabled: false }), + def("custom.broken", { + origin: "custom", + schema_status: "error", + schema_error_code: "table_not_found", + }), + ]), + ).toEqual(new Set()); + }); + + it("keeps an unchecked schema — unchecked is not broken", () => { + expect( + reachableMetricKeys([def("ai.accepted", { schema_status: "unchecked" })]), + ).toEqual(new Set(["ai.accepted"])); + }); +}); + +describe("partState", () => { + const PART = group("collaboration", ["collab.messages", "collab.meetings"]); + + it("reads when any metric of the part has a value", () => { + const byKey = normalizeMetricResults([metric("collab.messages", [[ME, 12]])]); + expect( + partState(PART, byKey, ME, reachableMetricKeys([def("collab.messages")])), + ).toBe("reads"); + }); + + it("says nothing recorded when the source reaches us but this person is absent from it", () => { + const byKey = normalizeMetricResults([ + metric("collab.messages", [[ME, null]]), + ]); + expect( + partState(PART, byKey, ME, reachableMetricKeys([def("collab.messages")])), + ).toBe("nothing_recorded"); + }); + + it("says no data reaches us when nothing in the part has ever observed anything", () => { + const byKey = normalizeMetricResults([ + metric("collab.messages", [[ME, null]]), + ]); + const reachable = reachableMetricKeys([ + def("collab.messages", { last_observed_date: null }), + def("collab.meetings", { last_observed_date: null }), + ]); + expect(partState(PART, byKey, ME, reachable)).toBe("no_data_reaches_us"); + }); + + it("does NOT call a part unreachable because nobody in view uses it", () => { + // The whole point. This viewer sees two people, neither of whom has a + // value. The listing says the source has observed data for the tenant, so + // the part is reachable and these two simply did none of that work. + // Concluding otherwise would report a live connector as missing, and would + // do so more often the smaller the viewer's reach. + const byKey = normalizeMetricResults([ + metric("collab.messages", [ + [ME, null], + [SOMEONE_ELSE, null], + ]), + ]); + expect( + partState(PART, byKey, ME, reachableMetricKeys([def("collab.messages")])), + ).toBe("nothing_recorded"); + }); + + it("still reads when only one of several metrics is wired", () => { + // A part is not unobservable because one of its metrics is missing. + const byKey = normalizeMetricResults([metric("collab.meetings", [[ME, 3]])]); + const reachable = reachableMetricKeys([ + def("collab.meetings"), + def("collab.messages", { last_observed_date: null }), + ]); + expect(partState(PART, byKey, ME, reachable)).toBe("reads"); + }); +}); + +describe("personCoverage", () => { + const GROUPS = [ + group("git_output", ["git.commits"]), + group("collaboration", ["collab.messages"]), + group("ai_adoption", ["ai.accepted"]), + ]; + + it("levels a person by how many parts read, and keeps each part's reason", () => { + const byKey = normalizeMetricResults([ + metric("git.commits", [[ME, 4]]), + metric("collab.messages", [[ME, null]]), + metric("ai.accepted", [[ME, null]]), + ]); + const reachable = reachableMetricKeys([ + def("git.commits"), + def("collab.messages"), + def("ai.accepted", { last_observed_date: null }), + ]); + const cov = personCoverage(GROUPS, byKey, ME, reachable); + + expect(cov.level).toBe(1); + expect(cov.states.get("git_output")).toBe("reads"); + expect(cov.states.get("collaboration")).toBe("nothing_recorded"); + expect(cov.states.get("ai_adoption")).toBe("no_data_reaches_us"); + }); + + it("gives level zero without inventing a reason for it", () => { + const byKey = normalizeMetricResults([metric("git.commits", [[ME, null]])]); + const cov = personCoverage(GROUPS, byKey, ME, new Set()); + expect(cov.level).toBe(0); + expect([...cov.states.values()]).toEqual([ + "no_data_reaches_us", + "no_data_reaches_us", + "no_data_reaches_us", + ]); + }); +}); + +describe("coverageDistribution", () => { + const at = (level: number): ReturnType => ({ + entityId: `p${level}`, + states: new Map(), + level, + }); + + it("reports how many people it counted", () => { + // Not decoration. The same distribution is a true statement about the + // people counted and a false one about the organisation, and this number + // is the only thing separating them. + const d = coverageDistribution([at(0), at(2), at(2)], 3); + expect(d.counted).toBe(3); + }); + + it("seeds every level so an empty one reads as zero rather than as a gap", () => { + const d = coverageDistribution([at(3), at(3)], 3); + expect([...d.byLevel.entries()]).toEqual([ + [0, 0], + [1, 0], + [2, 0], + [3, 2], + ]); + }); + + it("counts nobody without claiming a shape", () => { + const d = coverageDistribution([], 2); + expect(d.counted).toBe(0); + expect([...d.byLevel.values()]).toEqual([0, 0, 0]); + }); +}); + +describe("unreachableParts", () => { + const GROUPS = [ + group("git_output", ["git.commits", "git.prs"]), + group("ai_adoption", ["ai.accepted"]), + ]; + + it("names a part no metric of which has ever observed anything", () => { + const reachable = reachableMetricKeys([def("git.commits")]); + expect(unreachableParts(GROUPS, reachable)).toEqual([ + { id: "ai_adoption", title: "ai_adoption title" }, + ]); + }); + + it("does not name a part where one metric of several reaches us", () => { + const reachable = reachableMetricKeys([def("git.prs"), def("ai.accepted")]); + expect(unreachableParts(GROUPS, reachable)).toEqual([]); + }); + + it("offers no estimate of who connecting one would reveal", () => { + // The people who do that work are invisible BECAUSE the source is missing, + // so any such number would be invented. The shape of the return value is + // the guarantee: there is nowhere to put one. + const [only] = unreachableParts(GROUPS, new Set(["git.commits"])); + expect(Object.keys(only)).toEqual(["id", "title"]); + }); +}); + +describe("thinlyCovered", () => { + const at = (level: number): ReturnType => ({ + entityId: `p${level}`, + states: new Map(), + level, + }); + + it("counts people seen in fewer than half their parts", () => { + // With five parts the boundary is unambiguous: two is under half, three is + // over, and the midpoint is not a level anyone can be at. + expect(thinlyCovered([at(0), at(1), at(2), at(3), at(4), at(5)], 5)).toBe(3); + }); + + it("puts exactly half on the covered side", () => { + // Four parts, two seen: half is not "fewer than half". Stated because the + // line this feeds is about whom the product cannot carry, and drawing an + // exact half into that group would overstate the problem. + expect(thinlyCovered([at(2)], 4)).toBe(0); + }); + + it("counts nobody when everyone is fully covered", () => { + expect(thinlyCovered([at(5), at(5)], 5)).toBe(0); + }); + + it("counts everybody when nothing reaches us", () => { + expect(thinlyCovered([at(0), at(0)], 5)).toBe(2); + }); +}); + +describe("partCoverage", () => { + const GROUPS = [ + group("git_output", ["git.commits"]), + group("collaboration", ["collab.messages"]), + ]; + const person = ( + entityId: string, + git: "reads" | "nothing_recorded" | "no_data_reaches_us", + collab: "reads" | "nothing_recorded" | "no_data_reaches_us", + ): ReturnType => ({ + entityId, + states: new Map([ + ["git_output", git], + ["collaboration", collab], + ] as const), + level: [git, collab].filter((s) => s === "reads").length, + }); + + it("counts, per part, the people it reads for", () => { + expect( + partCoverage(GROUPS, [ + person("a", "reads", "reads"), + person("b", "nothing_recorded", "reads"), + ]), + ).toEqual([ + { id: "git_output", title: "git_output title", seen: 1, unreachable: false }, + { id: "collaboration", title: "collaboration title", seen: 2, unreachable: false }, + ]); + }); + + it("separates a part nobody is measured in from one everybody was idle in", () => { + // Both read zero. Only one of them is a missing connector, and drawing + // them the same would blame people for a pipe that was never laid. + const [git, collab] = partCoverage(GROUPS, [ + person("a", "no_data_reaches_us", "nothing_recorded"), + person("b", "no_data_reaches_us", "nothing_recorded"), + ]); + expect(git).toMatchObject({ seen: 0, unreachable: true }); + expect(collab).toMatchObject({ seen: 0, unreachable: false }); + }); + + it("is derived from the same states as the per-person levels", () => { + // The guarantee that matters: one computation, two cuts. Summing the + // per-part counts must equal summing the per-person levels, always. + const people = [ + person("a", "reads", "reads"), + person("b", "reads", "nothing_recorded"), + person("c", "no_data_reaches_us", "reads"), + ]; + const byPart = partCoverage(GROUPS, people).reduce((n, p) => n + p.seen, 0); + const byPerson = people.reduce((n, p) => n + p.level, 0); + expect(byPart).toBe(byPerson); + }); +}); diff --git a/src/frontend/src/lib/insight/coverage.ts b/src/frontend/src/lib/insight/coverage.ts new file mode 100644 index 000000000..8b0a5e988 --- /dev/null +++ b/src/frontend/src/lib/insight/coverage.ts @@ -0,0 +1,218 @@ +import type { GroupId, MetricGroup } from "@/lib/insight/groups"; +import type { MetricDefinition } from "@/api/metric-definitions-client"; +import { + forEntity, + type NormalizedMetricResult, +} from "@/lib/metrics/collection"; + +/** + * What we can say about one part of a person's work, in one period. + * + * Three states, not two. A part with no value is either a source that never + * reaches us or a person who did none of that work, and collapsing them loses + * the only distinction that separates "connect this" from "nothing happened". + */ +export type PartState = "reads" | "nothing_recorded" | "no_data_reaches_us"; + +/** + * The metric keys that have ever produced an observation for this tenant. + * + * Read from the definition listing, which reports availability rather than + * filtering it: a definition that is disabled, schema-broken or has never + * observed anything is still listed, and says so. That makes it the authority + * on whether a source reaches us at all. + * + * This deliberately does NOT infer reachability from nobody in view having a + * value. A viewer whose visible set is small may see no user of a system that + * is connected and busy elsewhere, and the smaller their reach the more often + * that happens — the same shape of error as a statistic drawn from a truncated + * pool. Reachability is a property of the tenant, so it is read from the one + * place that holds it for the tenant. + * + * NOT a duplicate of `useAvailableMetricKeys`, which reads the same listing to + * a different question: that one asks which metrics may be REQUESTED and gates + * on `is_enabled` alone. This asks which have ever ANSWERED. The gap between + * the two is the interesting case — a metric that is enabled and requestable + * but has never observed anything comes back as nulls, and those nulls are + * what has to be read as "no data reaches us" rather than as an idle person. + */ +export function reachableMetricKeys( + definitions: readonly MetricDefinition[], +): Set { + const out = new Set(); + for (const d of definitions) { + if (!d.is_enabled) continue; + if (d.schema_status === "error") continue; + // A custom metric runs its SQL at query time, and the validator stamps + // freshness from materialized relations only — so its `last_observed_date` + // is absent however much data it serves. The listing says so in as many + // words. Judging it by that field would report a working metric as one + // nothing reaches, which is the exact fabrication this function exists to + // avoid, so it is taken at its word instead. + if (d.origin !== "custom" && d.last_observed_date == null) continue; + out.add(d.metric_key); + } + return out; +} + +/** + * Which of the three states a part is in for one person. + * + * Order matters: a value settles it, and only in its absence does the question + * become whose absence it is. A part counts as reaching us when ANY of its + * metrics does — a section is not unobservable because one of its four metrics + * is still unwired. + */ +export function partState( + def: MetricGroup, + byKey: Map, + entityId: string, + reachable: ReadonlySet, +): PartState { + let anyReachable = false; + for (const m of def.collection.metrics) { + const metric = byKey.get(m.key); + if (metric != null && forEntity(metric, entityId).value != null) { + return "reads"; + } + if (reachable.has(m.key)) anyReachable = true; + } + return anyReachable ? "nothing_recorded" : "no_data_reaches_us"; +} + +export interface PersonCoverage { + entityId: string; + /** One entry per part, in the order the parts were given. */ + states: ReadonlyMap; + /** How many parts read. This is the coverage level. */ + level: number; +} + +export function personCoverage( + groups: readonly MetricGroup[], + byKey: Map, + entityId: string, + reachable: ReadonlySet, +): PersonCoverage { + const states = new Map(); + let level = 0; + for (const def of groups) { + const state = partState(def, byKey, entityId, reachable); + states.set(def.id, state); + if (state === "reads") level += 1; + } + return { entityId, states, level }; +} + +export interface CoverageDistribution { + /** + * How many people this count covered. Stated wherever the distribution is, + * and not optional: a count over the people one viewer can see is a true + * statement about those people and a false one about the organisation, and + * this number is the whole difference between the two. + */ + counted: number; + /** Level → how many people sit at it. Every level from 0 to `parts` is present. */ + byLevel: ReadonlyMap; +} + +export function coverageDistribution( + people: readonly PersonCoverage[], + parts: number, +): CoverageDistribution { + const byLevel = new Map(); + // Seeded so an empty level reads as zero people rather than as a gap — the + // shape of the distribution is the finding, and a missing bar hides it. + for (let level = 0; level <= parts; level += 1) byLevel.set(level, 0); + for (const p of people) { + byLevel.set(p.level, (byLevel.get(p.level) ?? 0) + 1); + } + return { counted: people.length, byLevel }; +} + +export interface PartCoverage { + id: GroupId; + title: string; + /** People this part reads for. */ + seen: number; + /** + * True when nothing feeding this part reaches the tenant. Kept separate from + * `seen === 0` on purpose: they render differently because they mean + * different things, and a part nobody is measured in must not be drawn as a + * part everybody failed at. + */ + unreachable: boolean; +} + +/** + * The same coverage, cut by part instead of by person. + * + * Derived from the SAME per-person states as the distribution rather than + * computed its own way. Two counts of one thing on one screen will disagree + * eventually — different key sets, different fetches, different rounding — and + * a reader who spots the disagreement is right to stop trusting both. + */ +export function partCoverage( + groups: readonly MetricGroup[], + people: readonly PersonCoverage[], +): PartCoverage[] { + return groups.map((def) => { + let seen = 0; + let anyReachable = false; + for (const p of people) { + const state = p.states.get(def.id); + if (state === "reads") seen += 1; + if (state !== "no_data_reaches_us") anyReachable = true; + } + return { + id: def.id, + title: def.title, + seen, + unreachable: !anyReachable, + }; + }); +} + +/** + * How many people are seen in fewer than half the parts of their work. + * + * The one number on this screen that is a finding rather than a description. + * A distribution shows a shape; what a reader needs from it is whether the + * rest of the product can bear weight, and for whom it cannot — every metric, + * comparison and flag about these people rests on that fraction of what they + * do. + * + * "Fewer than half" rather than a tuned threshold: it needs no defending and + * means the same when the number of parts changes, which a fixed count would + * not. With an odd number of parts the midpoint is not itself reachable, so + * the boundary is unambiguous either way. + */ +export function thinlyCovered( + people: readonly PersonCoverage[], + parts: number, +): number { + return people.filter((p) => p.level < parts / 2).length; +} + +export interface UnreachablePart { + id: GroupId; + title: string; +} + +/** + * The parts no metric of which reaches this tenant. + * + * What this deliberately does not do is say how many people connecting one + * would light up. Nobody knows: the people who do that work are invisible + * precisely because the source is missing, so any such number would be + * invented. The honest statement is which parts nobody is measured in, next to + * how many people are thinly covered — and the reader draws the conclusion. + */ +export function unreachableParts( + groups: readonly MetricGroup[], + reachable: ReadonlySet, +): UnreachablePart[] { + return groups + .filter((def) => !def.collection.metrics.some((m) => reachable.has(m.key))) + .map((def) => ({ id: def.id, title: def.title })); +} diff --git a/src/frontend/src/lib/insight/group-data.ts b/src/frontend/src/lib/insight/group-data.ts index 49b9f6f5f..3cc6d6cee 100644 --- a/src/frontend/src/lib/insight/group-data.ts +++ b/src/frontend/src/lib/insight/group-data.ts @@ -26,33 +26,6 @@ export function groupHasData( } -/** - * Whether anyone in the comparison pool has a reading for this section. - * - * The difference between "this source does not reach us" and "this person did - * none of that work" is not visible in the person's own row — both arrive as - * a null. It IS visible in the pool: a peer view counts the entities that had - * a reading, so a pool of zero across every metric of a section means nobody - * is measured here, while a pool with readings and an empty own row means the - * measurement works and this person is simply absent from it. - * - * Not proof — a section whose whole cohort was idle looks the same. It fails - * toward the weaker claim, which is the safe direction: saying "no activity" - * where a connector is in fact missing understates what we know, while saying - * "no data reaches us" of a working source is plainly false to anyone whose - * colleagues' numbers are on the next screen. - */ -export function groupPeersHaveData( - def: MetricGroup, - byKey: Map, - entityId: string, -): boolean { - return def.collection.metrics.some((m) => { - const metric = byKey.get(m.key); - return metric != null && (forEntity(metric, entityId).peer?.n ?? 0) > 0; - }); -} - /** Worst standing first — the row a card leads with is the one to look at. */ const HEADLINE_TIER: Record = { bottom: 3, diff --git a/src/frontend/src/lib/portal/lens-configs.test.ts b/src/frontend/src/lib/portal/lens-configs.test.ts index dbf18c6d9..26d68e2c7 100644 --- a/src/frontend/src/lib/portal/lens-configs.test.ts +++ b/src/frontend/src/lib/portal/lens-configs.test.ts @@ -108,12 +108,4 @@ describe("sectionMetricKeys — Overview section kinds", () => { expect(keys).toContain("collab.messages_sent"); expect(keys).toContain("wiki.pages_created"); }); - it("derives coverage-radar keys from every group's card preview", () => { - const keys = new Set( - sectionMetricKeys({ title: "t", sections: [{ kind: "coverage-radar" }] }), - ); - for (const g of GROUPS) { - for (const k of g.card.preview) expect(keys.has(k), k).toBe(true); - } - }); }); diff --git a/src/frontend/src/lib/portal/lens-configs.ts b/src/frontend/src/lib/portal/lens-configs.ts index d1901a49a..64e6c9c36 100644 --- a/src/frontend/src/lib/portal/lens-configs.ts +++ b/src/frontend/src/lib/portal/lens-configs.ts @@ -1,4 +1,3 @@ -import { headlineMetricKeys } from "@/lib/insight/groups"; import type { Readiness } from "@/lib/portal/nav-model"; /** @@ -27,7 +26,10 @@ export type SectionSpec = // Overview-motivated, zone-agnostic sections (design DESIGN-2026-07-27-overview §4). | { kind: "attention"; metrics: readonly string[]; max: number } | { kind: "direction-cards"; variant: "compact" | "full" } - | { kind: "coverage-radar" }; + // How much of each person's work we can see at all, and for how many + // people (#2408). Fetches its own period-only collection across every + // group, so it contributes no keys to the zone grid. + | { kind: "coverage-levels" }; export interface LensConfig { title: string; @@ -85,8 +87,11 @@ export function sectionMetricKeys(config: LensConfig): string[] { } } break; - case "coverage-radar": - for (const k of headlineMetricKeys()) keys.add(k); + case "coverage-levels": + // Deliberately none. Coverage asks whether ANY metric of a group + // reads, so it needs every group's keys rather than the zone's + // chosen few — widening the shared grid to that would make one tab + // pay for all of them. It fetches its own period-only collection. break; default: { const _exhaustive: never = s; diff --git a/src/frontend/src/lib/portal/nav-model.ts b/src/frontend/src/lib/portal/nav-model.ts index c43e8237d..b4d9cd40c 100644 --- a/src/frontend/src/lib/portal/nav-model.ts +++ b/src/frontend/src/lib/portal/nav-model.ts @@ -16,6 +16,7 @@ import { MessageSquare, Plus, Radar, + ScanEye, Server, Settings2, ShieldCheck, @@ -196,7 +197,7 @@ export const ZONE_SECTIONS: Record = { { id: "by-direction", label: "By direction", icon: Layers }, { id: "trend", label: "Trend", icon: TrendingUp }, { id: "attention", label: "Attention needed", icon: AlertTriangle }, - { id: "health", label: "Health radar", icon: Radar }, + { id: "health", label: "What we can see", icon: ScanEye }, { id: "contribution", label: "Contribution breakdown", icon: Users }, ], }, diff --git a/src/frontend/src/lib/portal/overview-configs.ts b/src/frontend/src/lib/portal/overview-configs.ts index 5e93fd598..097e1ae95 100644 --- a/src/frontend/src/lib/portal/overview-configs.ts +++ b/src/frontend/src/lib/portal/overview-configs.ts @@ -62,9 +62,15 @@ export const OVERVIEW_ITEMS: Record = { sections: [{ kind: "attention", metrics: ATTENTION_KEYS, max: 30 }], }, health: { - title: "Overview · Health radar", - tagline: "domain coverage", - sections: [{ kind: "coverage-radar" }], + title: "Overview · What we can see", + tagline: "how much of the work reaches us, by part and by person", + // One model, three cuts: the verdict, the parts nothing reaches, and how + // thinly people are seen. The radar that used to live here computed + // coverage a second way — a different predicate (`entityObserved`, which + // refuses zero-filled sums) over a different key set (each group's card + // preview rather than all its metrics) — so the same screen carried two + // counts of one thing by two definitions. + sections: [{ kind: "coverage-levels" }], }, contribution: { title: "Overview · Contribution breakdown", diff --git a/src/frontend/src/lib/portal/use-person-sections.test.tsx b/src/frontend/src/lib/portal/use-person-sections.test.tsx index e69244ee0..bfbd45948 100644 --- a/src/frontend/src/lib/portal/use-person-sections.test.tsx +++ b/src/frontend/src/lib/portal/use-person-sections.test.tsx @@ -14,11 +14,19 @@ import { GROUPS } from "@/lib/insight/groups"; import { normalizeMetricResults } from "@/lib/metrics/collection"; const mocks = vi.hoisted(() => ({ + definitions: [] as unknown[], + definitionsPending: false, byKey: new Map(), isPending: false, cohort: [] as string[], })); +vi.mock("@/queries/metric-definitions", () => ({ + useMetricDefinitionsResponse: () => ({ + data: { metrics: mocks.definitions }, + isPending: mocks.definitionsPending, + }), +})); vi.mock("@/queries/metric-results", () => ({ useMetricCollectionSet: () => new Map( @@ -123,18 +131,58 @@ describe("usePersonSectionStandings", () => { it("separates a section this person is absent from one nobody is measured in", () => { // Both arrive as a null own value, and the page says different things - // about them: a pool that reads means the measurement works. + // about them. Which one is true is read from the tenant's definition + // listing, never from whether this viewer's comparison pool happens to + // hold readings: a viewer who can see few people would otherwise report a + // live connector as missing, and the smaller their reach the more often. mocks.byKey = normalizeMetricResults([metric("git.commits", null, 20)]); + + mocks.definitions = [ + { + metric_key: "git.commits", + is_enabled: true, + schema_status: "ok", + schema_error_code: null, + last_observed_date: "2026-07-26", + }, + ]; expect(standings().find((s) => s.id === "git_output")!.peersHaveData).toBe( true ); - mocks.byKey = normalizeMetricResults([metric("git.commits", null, 20, 0)]); + // Same person, same empty own value — only the listing changed. + mocks.definitions = [ + { + metric_key: "git.commits", + is_enabled: true, + schema_status: "ok", + schema_error_code: null, + last_observed_date: null, + }, + ]; expect(standings().find((s) => s.id === "git_output")!.peersHaveData).toBe( false ); }); + it("does not call a section unmeasured because this viewer's pool is empty", () => { + // The regression this replaced: an empty pool used to mean "no data + // reaches us". It now means nothing at all — the listing decides. + mocks.byKey = normalizeMetricResults([metric("git.commits", null, 20, 0)]); + mocks.definitions = [ + { + metric_key: "git.commits", + is_enabled: true, + schema_status: "ok", + schema_error_code: null, + last_observed_date: "2026-07-26", + }, + ]; + expect(standings().find((s) => s.id === "git_output")!.peersHaveData).toBe( + true + ); + }); + it("stays pending while the queries are, so the nav shows no mark yet", () => { // A section still loading must not be drawn as one with nothing: the // reader would take the grey mark for an answer. @@ -149,3 +197,16 @@ describe("usePersonSectionStandings", () => { expect(standings().map((s) => s.id)).toEqual(GROUPS.map((g) => g.id)); }); }); + +describe("while the definition listing is still loading", () => { + it("claims nothing about any section", () => { + // Without the listing the reachable set is empty, so every section would + // read as one nothing reaches — the page would tell a reader we see + // nothing about this person, then flip a moment later. Pending has to + // cover the listing too, not just the metric collection. + mocks.definitionsPending = true; + mocks.byKey = normalizeMetricResults([metric("git.commits", null, 20)]); + expect(standings().every((s) => s.isPending)).toBe(true); + mocks.definitionsPending = false; + }); +}); diff --git a/src/frontend/src/lib/portal/use-person-sections.ts b/src/frontend/src/lib/portal/use-person-sections.ts index 65af9d5a6..8a31fa0d1 100644 --- a/src/frontend/src/lib/portal/use-person-sections.ts +++ b/src/frontend/src/lib/portal/use-person-sections.ts @@ -1,5 +1,6 @@ import { GROUPS, type GroupId } from "@/lib/insight/groups"; -import { groupHasData, groupPeersHaveData } from "@/lib/insight/group-data"; +import { groupHasData } from "@/lib/insight/group-data"; +import { partState, reachableMetricKeys } from "@/lib/insight/coverage"; import { injectCohortPeer } from "@/lib/insight/within-team-peer"; import { gradeSectionStanding, @@ -12,6 +13,7 @@ import { derivePeerStanding } from "@/lib/metrics/peer-standing"; import { usePersonCohort } from "@/lib/portal/use-person-cohort"; import type { Status } from "@/lib/status"; import { usePortalPeriod } from "@/hooks/use-portal-period"; +import { useMetricDefinitionsResponse } from "@/queries/metric-definitions"; import { useMetricCollectionSet } from "@/queries/metric-results"; export interface SectionStanding { @@ -24,9 +26,16 @@ export interface SectionStanding { /** False when no metric of the section has a value for this period. */ hasData: boolean; /** - * Whether anyone in the comparison pool reads here. With `hasData` false it - * separates a section this person is absent from (pool reads) from one - * nobody is measured in (pool empty). + * Whether anything feeding this section reaches the tenant. With `hasData` + * false it separates a section this person is absent from (it reaches us, + * they did none of it) from one nobody is measured in (nothing feeds it). + * + * Read from the tenant-wide definition listing, NOT from whether the + * comparison pool happens to hold readings. The pool is whoever the viewer + * can see, so a small pool with no user of a live system would report that + * system as missing — and the smaller the viewer's reach, the more often. It + * also made this page and the org-wide coverage screen answer the same + * question two ways, and they were free to disagree. */ peersHaveData: boolean; isPending: boolean; @@ -60,6 +69,9 @@ export function usePersonSectionStandings(personId: string): SectionStanding[] { dateRange, ); + // Same query key as the availability gate, so this rides its cache. + const definitions = useMetricDefinitionsResponse(); + // The same cohort the screens compare against, so the nav mark and the // section it points at cannot disagree. const cohortIds = usePersonCohort(entityId); @@ -74,6 +86,8 @@ export function usePersonSectionStandings(personId: string): SectionStanding[] { dateRange, ); + const reachable = reachableMetricKeys(definitions.data?.metrics ?? []); + return GROUPS.map((def) => { const result = groupData.get(def.id); const cohort = cohortGroup.get(def.id); @@ -103,8 +117,14 @@ export function usePersonSectionStandings(personId: string): SectionStanding[] { status: gradeSectionStanding(counts), phrase: sectionStandingPhrase(counts), hasData: groupHasData(def, byKey, entityId), - peersHaveData: groupPeersHaveData(def, byKey, entityId), - isPending: result?.isPending ?? true, + peersHaveData: + partState(def, byKey, entityId, reachable) !== "no_data_reaches_us", + // The listing counts too. Without it `reachable` is empty, so every + // section reads as one nothing reaches — the page would tell a reader we + // see nothing about this person, then flip a moment later. The old pool + // inference read the same query whose pending state was already tracked, + // so this gap arrived with the new source. + isPending: definitions.isPending || (result?.isPending ?? true), }; }); } diff --git a/src/frontend/src/lib/portal/use-scope-coverage.test.tsx b/src/frontend/src/lib/portal/use-scope-coverage.test.tsx new file mode 100644 index 000000000..0cc524d9f --- /dev/null +++ b/src/frontend/src/lib/portal/use-scope-coverage.test.tsx @@ -0,0 +1,143 @@ +/** + * Coverage for everyone the viewer may see. + * + * Two of these tests are about the request rather than the answer, because + * both ways this hook can go wrong are in the request: asking for an id the + * viewer may not see refuses the whole screen, and asking for a view that + * cannot be chunked breaks it at roster scale. Neither failure is visible in + * the returned shape, so neither would be caught by testing the output. + */ +import { renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { MetricCollectionConfig } from "@/lib/metrics/collection"; + +const state = vi.hoisted(() => ({ + members: [] as string[], + definitions: undefined as unknown, + definitionsError: false, + collectionSet: new Map(), + lastCall: null as { + collections: readonly { key: string; collection: MetricCollectionConfig }[]; + entity: { type: string; ids: string[] }; + } | null, +})); + +vi.mock("@/hooks/use-portal-period", () => ({ + usePortalPeriod: () => ({ + dateRange: { from: "2026-03-01", to: "2026-03-31" }, + }), +})); +vi.mock("@/queries/metric-definitions", () => ({ + useMetricDefinitionsResponse: () => ({ + data: state.definitions, + isPending: false, + isError: state.definitionsError, + }), +})); +vi.mock("@/queries/metric-results", () => ({ + useMetricCollectionSet: ( + collections: readonly { + key: string; + collection: MetricCollectionConfig; + }[], + entity: { type: string; ids: string[] }, + ) => { + state.lastCall = { collections, entity }; + return state.collectionSet; + }, +})); + +import { GROUPS } from "@/lib/insight/groups"; +import { useScopeCoverage } from "./use-scope-coverage"; + +beforeEach(() => { + state.members = ["viewer-1", "a-1", "a-2", "a-3"]; + state.definitions = { metrics: [] }; + state.definitionsError = false; + state.collectionSet = new Map(); + state.lastCall = null; +}); + +describe("useScopeCoverage", () => { + it("asks for exactly the members the scope selector gave it", () => { + // The visibility check on the metrics endpoint is all-or-nothing: a single + // id the caller may not see refuses the entire request rather than + // filtering it, and does not say which id was at fault. Widening or + // guessing the list therefore empties the screen with no diagnosis. + renderHook(() => useScopeCoverage(state.members)); + + expect(state.lastCall?.entity.type).toBe("person"); + expect([...(state.lastCall?.entity.ids ?? [])].sort()).toEqual([ + "a-1", + "a-2", + "a-3", + "viewer-1", + ]); + }); + + it("requests only the period view, so the roster can still be chunked", () => { + // `entityChunkSize` refuses to chunk a collection carrying timeseries, + // breakdown or histogram views, and an unchunked roster-sized request runs + // into the backend's projected-row limit. Asking for one view keeps the + // existing chunk-and-merge path available. + renderHook(() => useScopeCoverage(state.members)); + + const views = (state.lastCall?.collections ?? []).flatMap((c) => + c.collection.metrics.flatMap((m) => m.views.map((v) => v.view)), + ); + expect(views.length).toBeGreaterThan(0); + expect([...new Set(views)]).toEqual(["period"]); + }); + + it("counts every person in the roster, including the viewer", () => { + const { result } = renderHook(() => useScopeCoverage(state.members)); + expect(result.current.distribution.counted).toBe(4); + expect(result.current.people).toHaveLength(4); + }); + + it("settles rather than waiting forever on an empty scope", () => { + // No members means no collections are sent, so no group ever reports a + // pending state to clear. Reading that as "still loading" leaves the + // section on its spinner permanently instead of saying the scope is empty. + state.members = []; + const { result } = renderHook(() => useScopeCoverage(state.members)); + expect(result.current.isPending).toBe(false); + expect(result.current.distribution.counted).toBe(0); + }); + + it("stays closed while the scope has not resolved", () => { + // An empty id list is refused by the client rather than sent, so the hook + // must not reach that path at all before the roster answers. + state.members = []; + renderHook(() => useScopeCoverage(state.members)); + expect(state.lastCall?.entity.ids).toEqual([]); + expect(state.lastCall?.collections).toEqual([]); + }); + + it("reports every part as unreachable when nothing has ever observed", () => { + // No definition has observed anything, so no part can be claimed to reach + // us — and this is read from the listing, never from the roster's nulls. + const { result } = renderHook(() => useScopeCoverage(state.members)); + expect(result.current.parts.every((p) => p.unreachable)).toBe(true); + expect(result.current.parts).toHaveLength(GROUPS.length); + expect(result.current.distribution.byLevel.get(0)).toBe(4); + }); +}); + +describe("useScopeCoverage failure", () => { + it("reports an error rather than letting it read as an absence verdict", () => { + // With the listing unavailable nothing is known to reach the tenant, so + // every part would come back "no data reaches us" and every person would + // sit at zero. That is our fault printed as a verdict about named people, + // and the caller has to be able to tell the two apart. + state.definitionsError = true; + const { result } = renderHook(() => useScopeCoverage(state.members)); + expect(result.current.isError).toBe(true); + }); + + it("is not in error when everything answered", () => { + const { result } = renderHook(() => useScopeCoverage(state.members)); + expect(result.current.isError).toBe(false); + }); +}); diff --git a/src/frontend/src/lib/portal/use-scope-coverage.ts b/src/frontend/src/lib/portal/use-scope-coverage.ts new file mode 100644 index 000000000..03e68cf92 --- /dev/null +++ b/src/frontend/src/lib/portal/use-scope-coverage.ts @@ -0,0 +1,121 @@ +import { useMemo } from "react"; + +import { + coverageDistribution, + partCoverage, + personCoverage, + reachableMetricKeys, + thinlyCovered, + type CoverageDistribution, + type PartCoverage, + type PersonCoverage, +} from "@/lib/insight/coverage"; +import { GROUPS } from "@/lib/insight/groups"; +import { projectViews } from "@/lib/metrics/collection"; +import { usePortalPeriod } from "@/hooks/use-portal-period"; +import { useMetricDefinitionsResponse } from "@/queries/metric-definitions"; +import { useMetricCollectionSet } from "@/queries/metric-results"; + +export interface ScopeCoverage { + distribution: CoverageDistribution; + /** Per person, so a level can be opened into the people at it. */ + people: readonly PersonCoverage[]; + /** People seen in fewer than half their parts — the screen's finding. */ + thin: number; + /** + * A request failed, so no claim may be made. Not a detail: with the + * definition listing unavailable nothing is known to reach the tenant, and + * every part would read "no data reaches us" for every person — an + * infrastructure fault rendered as a confident verdict about named people, + * which is the failure the three states exist to prevent. The caller must + * check this before reading anything else here. + */ + isError: boolean; + /** The same coverage cut by part, from the same states. */ + parts: readonly PartCoverage[]; + isPending: boolean; +} + +const CLOSED = { type: "person" as const, ids: [] as string[] }; + +/** + * How much of their work the product can see, for everyone the viewer may see. + * + * Computed in the browser, over the viewer's visible set. Both are compromises + * and both are stated: `distribution.counted` says how many people the answer + * covers, and nothing here is presented as being about the organisation unless + * the viewer's reach is the organisation. + * + * That compromise is affordable here and is NOT affordable for a statistic. A + * quartile over a subset is a different quantity from the same quartile over + * the whole group, and biased. A count over a subset stays a true statement + * about that subset as long as the subset's size travels with it — which is + * why `counted` is not optional and not cosmetic. + * + * The roster is the id list, exactly. The visibility check on the metrics + * endpoint is all-or-nothing: one id outside the caller's visible set refuses + * the whole request rather than filtering it, and does not say which id was at + * fault. So the list is built from the tree the viewer was served and is never + * widened or guessed. + */ +export function useScopeCoverage( + memberIds: readonly string[], +): ScopeCoverage { + const { dateRange } = usePortalPeriod(); + // The scope selector owns who is in view, so this takes the member list + // rather than deriving one. Deriving it would leave the tab answering about + // the viewer's whole reach while every other tab answered about the selected + // scope, and the two would silently disagree on the same screen. + const rosterIds = useMemo(() => [...memberIds], [memberIds]); + + // `period` only, deliberately. A collection carrying timeseries, breakdown + // or histogram views cannot be chunked (`entityChunkSize` returns null for + // them), and an unchunked roster-sized request runs into the backend's + // projected-row limit. Asking for the one view this needs keeps the existing + // chunk-and-merge path available at roster scale. + const data = useMetricCollectionSet( + rosterIds.length + ? GROUPS.map((def) => ({ + key: def.id, + collection: projectViews(def.collection, ["period"]), + })) + : [], + rosterIds.length ? { type: "person" as const, ids: rosterIds } : CLOSED, + dateRange, + ); + + // The same query key the availability gate uses, so this rides its cache + // rather than issuing a second listing request. + const definitions = useMetricDefinitionsResponse(); + const reachable = useMemo( + () => reachableMetricKeys(definitions.data?.metrics ?? []), + [definitions.data], + ); + + return useMemo(() => { + const byKey = new Map( + GROUPS.flatMap((def) => [...(data.get(def.id)?.byKey ?? new Map())]), + ); + const people = rosterIds.map((id) => + personCoverage(GROUPS, byKey, id, reachable), + ); + return { + distribution: coverageDistribution(people, GROUPS.length), + people, + thin: thinlyCovered(people, GROUPS.length), + parts: partCoverage(GROUPS, people), + // An empty roster is an answer, not a wait. With no members the hook + // sends no collections, so no group has an entry and the `?? true` + // below would hold every one of them pending forever — the section + // would sit on its loading label for good rather than saying there is + // nobody in this scope. + isPending: + definitions.isPending || + (rosterIds.length > 0 && + GROUPS.some((def) => data.get(def.id)?.isPending ?? true)), + isError: + definitions.isError || + GROUPS.some((def) => data.get(def.id)?.isError ?? false), + }; + }, [data, rosterIds, reachable, definitions.isPending, definitions.isError]); +} diff --git a/src/frontend/src/queries/metric-definitions.test.ts b/src/frontend/src/queries/metric-definitions.test.ts index 62058fba4..c7fc8343a 100644 --- a/src/frontend/src/queries/metric-definitions.test.ts +++ b/src/frontend/src/queries/metric-definitions.test.ts @@ -37,6 +37,7 @@ function metric(metric_key: string): MetricDefinition { direction: "neutral", dimensions: [], is_enabled: true, + origin: "builtin" as const, schema_status: "ok", schema_error_code: null, last_observed_date: null, diff --git a/src/frontend/src/screens/metric-definitions.test.tsx b/src/frontend/src/screens/metric-definitions.test.tsx index 765b9e1a3..0cee63870 100644 --- a/src/frontend/src/screens/metric-definitions.test.tsx +++ b/src/frontend/src/screens/metric-definitions.test.tsx @@ -35,6 +35,7 @@ function metric(over: Partial = {}): MetricDefinition { direction: "higher_is_better", dimensions: [], is_enabled: true, + origin: "builtin" as const, schema_status: "ok", schema_error_code: null, last_observed_date: "2026-07-20",