From f11eb4d7f78626df92cc3358a0ebfdb98f05767c Mon Sep 17 00:00:00 2001 From: Alexey Panfilov Date: Tue, 11 Aug 2026 20:47:15 +0200 Subject: [PATCH 1/3] fix(frontend): say which emptiness a section has MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A section a person did none of, and a section nothing feeds, are opposite findings: the first is about them, the second is about the install and is nobody's performance. The sections page already draws that line for the whole list, but the section screen itself asked only whether any metric held a value, so both collapsed into "Nothing recorded here for the selected period" — sending a reader to look for missing work that was never being measured. It now reads the same tenant-wide listing the section navigation already asks for, and says which of the two it is. That listing is part of the answer rather than decoration, so the screen waits for it instead of showing one sentence and swapping it for the other; and when the request fails it claims neither absence, because with no listing every section looks unreachable. Signed-off-by: Alexey Panfilov --- .../portal/single-group-view.test.tsx | 63 +++++++++++++++++++ .../components/portal/single-group-view.tsx | 40 ++++++++++-- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/src/frontend/src/components/portal/single-group-view.test.tsx b/src/frontend/src/components/portal/single-group-view.test.tsx index 201aa24ee..74a38c0ca 100644 --- a/src/frontend/src/components/portal/single-group-view.test.tsx +++ b/src/frontend/src/components/portal/single-group-view.test.tsx @@ -18,8 +18,18 @@ const mocks = vi.hoisted(() => ({ isPending: false, isError: false, cohort: [] as string[], + definitions: [] as unknown[], + definitionsPending: false, + definitionsError: false, })); +vi.mock("@/queries/metric-definitions", () => ({ + useMetricDefinitionsResponse: () => ({ + data: mocks.definitionsError ? undefined : { metrics: mocks.definitions }, + isPending: mocks.definitionsPending, + isError: mocks.definitionsError, + }), +})); vi.mock("@/queries/metric-results", () => ({ useMetricCollection: () => ({ byKey: mocks.byKey, @@ -80,11 +90,27 @@ function draw() { return render(); } +/** A listing entry for a metric this installation reads. */ +function wired(key: string) { + return { + metric_key: key, + is_enabled: true, + schema_status: "ok", + origin: "builtin", + last_observed_date: "2026-03-20", + }; +} + beforeEach(() => { mocks.byKey = new Map(); mocks.isPending = false; mocks.isError = false; mocks.cohort = []; + // The section's sources are connected unless a test says otherwise: an + // empty section is then about the person, which is the ordinary case. + mocks.definitions = HEADLINE.map(([k]) => wired(k)); + mocks.definitionsPending = false; + mocks.definitionsError = false; }); describe("SingleGroupView", () => { @@ -100,6 +126,43 @@ describe("SingleGroupView", () => { expect(screen.queryByTestId("composition")).not.toBeInTheDocument(); }); + it("says nothing feeds a section rather than blaming the person for it", () => { + // The two empties are opposite findings. A section this person did none + // of is worth asking them about; one nothing feeds is about the install + // and is nobody's performance. Told the second in the first's words, a + // reader goes looking for missing work that was never being measured. + mocks.byKey = normalizeMetricResults( + HEADLINE.map(([k, l]) => metric(k, l, null)) + ); + mocks.definitions = []; + draw(); + expect(screen.getByText(/No data reaches us for this section/)).toBeInTheDocument(); + expect(screen.queryByText(/Nothing recorded here/)).not.toBeInTheDocument(); + }); + + it("claims neither absence when the listing that decides could not be read", () => { + // With no listing every section looks unreachable, so falling through + // would announce that nothing is measured here for anyone on the strength + // of a request that never arrived. + mocks.byKey = normalizeMetricResults( + HEADLINE.map(([k, l]) => metric(k, l, null)) + ); + mocks.definitionsError = true; + draw(); + expect(screen.getByText(/Nothing to show here/)).toBeInTheDocument(); + expect(screen.queryByText(/No data reaches us/)).not.toBeInTheDocument(); + }); + + it("waits for the listing rather than showing one sentence and swapping it", () => { + mocks.byKey = normalizeMetricResults( + HEADLINE.map(([k, l]) => metric(k, l, null)) + ); + mocks.definitionsPending = true; + draw(); + expect(screen.queryByText(/Nothing recorded here/)).not.toBeInTheDocument(); + expect(screen.queryByText(/No data reaches us/)).not.toBeInTheDocument(); + }); + it("gives a detail block only to the headline metrics that read", () => { mocks.byKey = normalizeMetricResults([ metric("collab.messages_sent", "Messages Sent", 400), diff --git a/src/frontend/src/components/portal/single-group-view.tsx b/src/frontend/src/components/portal/single-group-view.tsx index aea89de6a..1619ec377 100644 --- a/src/frontend/src/components/portal/single-group-view.tsx +++ b/src/frontend/src/components/portal/single-group-view.tsx @@ -6,7 +6,7 @@ import { CollectionDrilldown } from "@/components/widgets/metric-views/collectio import { MetricActivity } from "@/components/widgets/metric-views/metric-activity"; import { SectionMetricIndex } from "@/components/widgets/metric-views/section-metric-index"; import { usePortalPeriod } from "@/hooks/use-portal-period"; -import { groupHasData } from "@/lib/insight/group-data"; +import { partState, reachableMetricKeys } from "@/lib/insight/coverage"; import { GROUPS, type GroupId } from "@/lib/insight/groups"; import { finestGrain } from "@/lib/insight/metric-grain"; import { sectionSources } from "@/lib/insight/section-sources"; @@ -17,6 +17,7 @@ import { } from "@/lib/metrics/collection"; import { normalizePersonId } from "@/lib/metrics/entity"; import { usePersonCohort } from "@/lib/portal/use-person-cohort"; +import { useMetricDefinitionsResponse } from "@/queries/metric-definitions"; import { useMetricCollection } from "@/queries/metric-results"; import { TEXT_TITLE } from "@/lib/type-scale"; @@ -59,6 +60,10 @@ export function SingleGroupView({ // which they neither chose nor can see inside. { previousPeriod: period }, ); + // The tenant-wide listing of what is wired up, which is what tells the two + // empty sections apart below. Already in cache: the section navigation asks + // the same question to mark its rows. + const definitions = useMetricDefinitionsResponse(); const cohortIds = usePersonCohort(entityId); const cohortData = useMetricCollection( def && cohortIds.length ? def.collection : EMPTY_COLLECTION, @@ -114,7 +119,11 @@ export function SingleGroupView({ ); } - if (data.isPending) return ; + // The definitions are part of the answer, not decoration: without them an + // empty section cannot say which of the two emptinesses it is, and guessing + // while they load would show one sentence and then swap it for the other. + if (data.isPending || definitions.isPending) + return ; // A failed fetch must surface as a retryable error, not a drilldown // rendered over an empty dataset (same policy as MetricGroupsView). if (data.isError) { @@ -130,12 +139,35 @@ export function SingleGroupView({ // in this period", a summary card showing a dash, a distribution saying "no // values" — and a reader would meet one fact restated in four wordings down // a full page. One sentence says it once. - if (!groupHasData(def, injectedData.byKey, entityId)) { + // + // WHICH sentence is the point. A section this person did none of, and one + // nothing feeds, are opposite findings: the first is about them and is + // worth asking about, the second is about the install and is nobody's + // performance. The sections page already draws that line for the whole list + // (see `PersonCoverage`); saying "nothing recorded" here threw it away for + // the one section the reader cared enough to open. + // + // A failed definitions request answers neither. It cannot be allowed to + // fall through to one of them: with no listing every section looks + // unreachable, and the screen would announce that nothing is measured here + // for anyone on the strength of a request that did not arrive. It says + // neither instead. + const state = partState( + def, + injectedData.byKey, + entityId, + reachableMetricKeys(definitions.data?.metrics ?? []), + ); + if (state !== "reads") { return (

{def.title}

- Nothing recorded here for the selected period. + {definitions.isError + ? "Nothing to show here for the selected period." + : state === "no_data_reaches_us" + ? "No data reaches us for this section — nothing is measured here for anyone yet." + : "Nothing recorded here for the selected period."}

); From 3fa3688287d2ed4fda71806d0a75e6ed9357adca Mon Sep 17 00:00:00 2001 From: Alexey Panfilov Date: Tue, 11 Aug 2026 20:47:26 +0200 Subject: [PATCH 2/3] fix(frontend): mark a section nothing feeds apart in the nav MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The section navigation drew one grey dot for both kinds of empty, so the distinction the screen behind it now makes was invisible until the reader opened the section. The standings hook has carried the flag that tells them apart since the coverage work landed; nothing read it. A grey dot now means the section reads and holds nothing for this person this period. A hollow ring means nothing feeds it — not worth opening at all until that changes. The tooltip says which. Signed-off-by: Alexey Panfilov --- .../components/portal/context-pane.test.tsx | 27 +++++++++++++++++++ .../src/components/portal/context-pane.tsx | 24 ++++++++++++----- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/frontend/src/components/portal/context-pane.test.tsx b/src/frontend/src/components/portal/context-pane.test.tsx index 331df884a..89b35fdfb 100644 --- a/src/frontend/src/components/portal/context-pane.test.tsx +++ b/src/frontend/src/components/portal/context-pane.test.tsx @@ -24,6 +24,7 @@ const mocks = vi.hoisted(() => ({ status: string; phrase: string; hasData: boolean; + peersHaveData: boolean; isPending: boolean; }>, })); @@ -136,6 +137,7 @@ describe("ContextPane", () => { status: "bad", phrase: "4 of 6 behind peers", hasData: true, + peersHaveData: true, isPending: false, }, ]; @@ -153,6 +155,7 @@ describe("ContextPane", () => { status: "neutral", phrase: "no peer data", hasData: false, + peersHaveData: true, isPending: false, }, ]; @@ -161,6 +164,29 @@ describe("ContextPane", () => { expect(button.querySelector(".bg-muted-foreground\\/30")).not.toBeNull(); }); + it("marks a section nothing feeds apart from one this person is absent from", () => { + // Same grey dot for both sent readers into a section to look for work + // that was never being measured. The hollow mark says the section itself + // is not wired, which is not worth opening at all. + mocks.zone = { activeZone: "person", activePerson: "boss@x" }; + mocks.standings = [ + { + id: "git_output", + title: "Git output", + status: "neutral", + phrase: "no peer data", + hasData: false, + peersHaveData: false, + isPending: false, + }, + ]; + pane(); + const button = screen.getByTitle("No data reaches us for this section"); + const mark = button.querySelector("span[aria-hidden]")!; + expect(mark.className).not.toContain("bg-muted-foreground"); + expect(mark.className).toContain("border"); + }); + it("draws no mark while the standings are still loading", () => { // A pending section drawn grey would read as "nothing here" — an answer // the hook has not given yet. @@ -172,6 +198,7 @@ describe("ContextPane", () => { status: "neutral", phrase: "", hasData: false, + peersHaveData: true, isPending: true, }, ]; diff --git a/src/frontend/src/components/portal/context-pane.tsx b/src/frontend/src/components/portal/context-pane.tsx index 7e904d27a..f86d78ecd 100644 --- a/src/frontend/src/components/portal/context-pane.tsx +++ b/src/frontend/src/components/portal/context-pane.tsx @@ -523,23 +523,35 @@ function PersonSectionsNav() { dismiss(); }} title={ - standing?.hasData === false - ? "No data this period" - : standing?.phrase + standing?.hasData !== false + ? standing?.phrase + : standing.peersHaveData + ? "No data this period" + : "No data reaches us for this section" } > {g.title} {/* The mark that answers "which section is worth opening", - beside the thing you click. Grey means the section has - nothing this period — worth knowing before you open it. */} + beside the thing you click. + + Three marks, not two, because empty means two different + things. A grey dot is a section that reads fine and holds + nothing for this person this period — a fact about them. + A hollow ring is one nothing feeds — a fact about the + install, and not worth opening at all until that changes. + Drawn identically, the second sent readers looking for a + person's missing work when the connector was the whole + story. */} {standing && !standing.isPending ? ( From cce2a4390bba0c252e63fbac211713e4f3c86360 Mon Sep 17 00:00:00 2001 From: Alexey Panfilov Date: Wed, 12 Aug 2026 09:59:25 +0200 Subject: [PATCH 3/3] fix(frontend): say nothing about a section still being loaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both coverage flags read false while the standings queries are in flight, so the section tooltip fell through to the strongest of the three claims — that nothing feeds this section — on an answer the hook had not given yet. The mark beside the row is already hidden for that reason; the words now follow it. Signed-off-by: Alexey Panfilov --- .../components/portal/context-pane.test.tsx | 4 ++++ .../src/components/portal/context-pane.tsx | 18 +++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/frontend/src/components/portal/context-pane.test.tsx b/src/frontend/src/components/portal/context-pane.test.tsx index 89b35fdfb..b475258f9 100644 --- a/src/frontend/src/components/portal/context-pane.test.tsx +++ b/src/frontend/src/components/portal/context-pane.test.tsx @@ -205,5 +205,9 @@ describe("ContextPane", () => { pane(); const button = screen.getByText("Git output").closest("button")!; expect(button.querySelector("span[aria-hidden]")).toBeNull(); + // And says nothing either. Both flags read false while the queries are in + // flight, so a tooltip that trusted them would announce the strongest + // claim of the three on an answer the hook has not given. + expect(button.getAttribute("title")).toBeNull(); }); }); diff --git a/src/frontend/src/components/portal/context-pane.tsx b/src/frontend/src/components/portal/context-pane.tsx index f86d78ecd..5d3ee1aa5 100644 --- a/src/frontend/src/components/portal/context-pane.tsx +++ b/src/frontend/src/components/portal/context-pane.tsx @@ -523,11 +523,19 @@ function PersonSectionsNav() { dismiss(); }} title={ - standing?.hasData !== false - ? standing?.phrase - : standing.peersHaveData - ? "No data this period" - : "No data reaches us for this section" + // Nothing to say until the standings arrive. Both flags + // read false while the queries are in flight, so left to + // fall through, the tooltip announced the strongest of the + // three — that nothing feeds this section — on an answer + // the hook had not given. The mark is hidden for that + // reason already; the words have to follow it. + standing == null || standing.isPending + ? undefined + : standing.hasData + ? standing.phrase + : standing.peersHaveData + ? "No data this period" + : "No data reaches us for this section" } >