Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/frontend/src/components/portal/context-pane.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const mocks = vi.hoisted(() => ({
status: string;
phrase: string;
hasData: boolean;
peersHaveData: boolean;
isPending: boolean;
}>,
}));
Expand Down Expand Up @@ -136,6 +137,7 @@ describe("ContextPane", () => {
status: "bad",
phrase: "4 of 6 behind peers",
hasData: true,
peersHaveData: true,
isPending: false,
},
];
Expand All @@ -153,6 +155,7 @@ describe("ContextPane", () => {
status: "neutral",
phrase: "no peer data",
hasData: false,
peersHaveData: true,
isPending: false,
},
];
Expand All @@ -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.
Expand All @@ -172,11 +198,16 @@ describe("ContextPane", () => {
status: "neutral",
phrase: "",
hasData: false,
peersHaveData: true,
isPending: true,
},
];
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();
});
});
32 changes: 26 additions & 6 deletions src/frontend/src/components/portal/context-pane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -523,23 +523,43 @@ function PersonSectionsNav() {
dismiss();
}}
title={
standing?.hasData === false
? "No data this period"
: standing?.phrase
// 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"
}
>
<Layers />
<span className="min-w-0 flex-1 truncate">{g.title}</span>
{/* 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 ? (
<span
className={cn(
"size-1.5 shrink-0 rounded-full",
standing.hasData
? STATUS_BG_CLASS[standing.status]
: "bg-muted-foreground/30",
: standing.peersHaveData
? "bg-muted-foreground/30"
: "border border-muted-foreground/40",
)}
aria-hidden
/>
Expand Down
63 changes: 63 additions & 0 deletions src/frontend/src/components/portal/single-group-view.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -80,11 +90,27 @@ function draw() {
return render(<SingleGroupView personId={ME} groupId="collaboration" />);
}

/** 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", () => {
Expand All @@ -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),
Expand Down
40 changes: 36 additions & 4 deletions src/frontend/src/components/portal/single-group-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -114,7 +119,11 @@ export function SingleGroupView({
</div>
);
}
if (data.isPending) return <CenteredSpinner className="min-h-[60vh]" />;
// 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 <CenteredSpinner className="min-h-[60vh]" />;
// A failed fetch must surface as a retryable error, not a drilldown
// rendered over an empty dataset (same policy as MetricGroupsView).
if (data.isError) {
Expand All @@ -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 (
<div className="flex flex-col gap-3 p-4 md:p-6">
<h1 className={TEXT_TITLE}>{def.title}</h1>
<p className="text-sm text-muted-foreground">
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."}
</p>
</div>
);
Expand Down
Loading