feat(roi): default people and branch lists to matched accounts (#44465)

* feat(roi): show matched people by default in contributor lists

* fix(roi): keep matched filter tabs readable on narrow screens

* fix(roi): retain spend-only users and support older browsers
This commit is contained in:
moe-berri 2026-10-03 17:24:18 -07:00 • committed by GitHub
parent f850b2c324
commit 4b67a2b845
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 519 additions and 56 deletions

View file

@ -30,6 +30,8 @@ The gateway encrypts access and refresh tokens using its configured encryption k
Use **Link accounts** to associate several current or historical usernames with one internal email. Each connection has a separate username field, so a GitHub username never matches a GitLab user implicitly. Saving immediately recalculates the report without fetching repositories again. Public profile emails match automatically when they resolve unambiguously to an internal user
**Matched people only** is on by default for people, merged changes, and branch lists. Turn it off to include outside contributors and their branches. Matching depends on the linked internal account, even when no spend was recorded. This switch filters the lists; summary metrics and quality signals still cover all selected repositories
Agent-authored changes count for a person only when the supported agent metadata explicitly names a requester. Repository issue counts and revert titles are quality signals, not an individual defect score
Bug and regression counts combine repositories with issue tracking enabled. They remain unavailable when none of the selected repositories has issue tracking enabled

View file

@ -0,0 +1,12 @@
import { useId } from "react";
import { Switch } from "@/components/ui/switch";
export function MatchedPeopleToggle({ checked, onChange }: { checked: boolean; onChange: (checked: boolean) => void }) {
const id = useId();
return (
<label htmlFor={id} className="flex shrink-0 cursor-pointer items-center gap-2 text-xs text-muted-foreground">
<Switch id={id} size="sm" checked={checked} onCheckedChange={onChange} />
Matched people only
</label>
);
}

View file

@ -24,11 +24,16 @@ import {
export function PullList({
pulls,
provider,
matchedOnly = false,
}: {
matchedOnly?: boolean;
pulls: ObservedPull[];
provider: ObservedSnapshot["source_provider"];
}) {
const terms = changeTerms(provider);
const emptyMessage = matchedOnly
? "No merged changes from matched people in this period"
: `No ${terms.lower} in this period`;
const [query, setQuery] = useState("");
const [limit, setLimit] = useState(20);
const filtered = pulls.filter((pull) =>
@ -104,7 +109,7 @@ export function PullList({
</p>
{filtered.length === 0 && (
<p className="py-8 text-center text-sm text-muted-foreground">
{query ? `No ${terms.lower} match this search` : `No ${terms.lower} in this period`}
{query ? `No ${terms.lower} match this search` : emptyMessage}
</p>
)}
<div className="flex items-center justify-between text-xs text-muted-foreground">
@ -150,9 +155,7 @@ export function PersonDetails({
<SheetContent className="overflow-y-auto p-6 data-[side=right]:w-full data-[side=right]:sm:max-w-3xl">
<SheetHeader className="p-0 pr-8">
<SheetTitle className="text-xl">{person.name}</SheetTitle>
<SheetDescription>
{person.email} · {person.logins.join(", ")}
</SheetDescription>
<SheetDescription>{[person.email, person.logins.join(", ")].filter(Boolean).join(" · ")}</SheetDescription>
</SheetHeader>
{onEdit && (
<Button variant="outline" className="w-fit" onClick={onEdit}>
@ -200,8 +203,8 @@ export function PersonDetails({
);
}
export function BranchSpend({ snapshot }: { snapshot: ObservedSnapshot }) {
const rows = recordedBranches(snapshot);
export function BranchSpend({ snapshot, matchedOnly = true }: { snapshot: ObservedSnapshot; matchedOnly?: boolean }) {
const rows = recordedBranches(snapshot, matchedOnly);
return (
<div className="rounded-xl border">
<Table>
@ -225,7 +228,11 @@ export function BranchSpend({ snapshot }: { snapshot: ObservedSnapshot }) {
</TableBody>
</Table>
{rows.length === 0 && (
<p className="p-8 text-center text-sm text-muted-foreground">No tagged branch spend in this period</p>
<p className="p-8 text-center text-sm text-muted-foreground">
{matchedOnly
? "No tagged branch spend for matched people in this period"
: "No tagged branch spend in this period"}
</p>
)}
</div>
);

View file

@ -2,6 +2,7 @@ import { fireEvent, render, screen, waitFor, within } from "@testing-library/rea
import userEvent from "@testing-library/user-event";
import { afterEach, describe, expect, it, vi } from "vitest";
import ObservedROIView from "./ObservedROIView";
import { createObservedDemo } from "./observedDemo";
import type { ObservedSettings, ObservedSnapshot, ObservedStatus } from "./observedData";
const settings: ObservedSettings = {
@ -56,6 +57,81 @@ afterEach(() => {
});
describe("observed ROI dashboard", () => {
it("defaults every contributor list to matched people and keeps the switch across tabs", async () => {
const sample = createObservedDemo(7);
const outside = {
...sample.pulls.current[0],
url: "https://gitlab.com/outside/api/-/merge_requests/999",
author: "outside",
agent: false,
title: "Outside change",
source_branch: "outside-only",
branch_cost: {
repo: "gitlab.com/outside/api",
branch: "outside-only",
spend: 123,
requests: 5,
status: "matched" as const,
},
};
const report = {
...sample,
pulls: { ...sample.pulls, current: [...sample.pulls.current, outside] },
unlinked_branches: [{ repo: "gitlab.com/outside/api", branch: "orphan-only", spend: 5, requests: 1 }],
};
const requests = vi.fn(async (input: string, _init: RequestInit) => {
const path = new URL(input, "http://localhost").pathname;
if (path.endsWith("/settings")) return Response.json(settings);
if (path.endsWith("/report")) return Response.json({ report });
if (path.endsWith("/sync")) return Response.json(idle);
throw new Error(path);
});
vi.stubGlobal("fetch", requests);
const user = userEvent.setup();
render(<ObservedROIView accessToken="test-only-gateway-token" isViewOnly />);
expect(await screen.findByRole("switch", { name: "Matched people only" })).toBeChecked();
expect(screen.getByRole("tab", { name: "Engineers 3" })).toBeInTheDocument();
expect(screen.queryByText("outside")).not.toBeInTheDocument();
await user.click(screen.getByRole("switch", { name: "Matched people only" }));
expect(screen.getByRole("tab", { name: "Engineers 4" })).toBeInTheDocument();
await user.click(screen.getByRole("button", { name: "View outside's merged changes" }));
expect(await screen.findByRole("dialog", { name: "outside" })).toHaveTextContent("Outside change");
expect(screen.queryByRole("button", { name: "Edit linked accounts" })).not.toBeInTheDocument();
await user.click(screen.getByRole("button", { name: "Close" }));
await user.click(screen.getByRole("switch", { name: "Matched people only" }));
await user.click(screen.getByRole("tab", { name: "Merged changes" }));
expect(screen.queryByText("Outside change")).not.toBeInTheDocument();
await user.click(screen.getByRole("tab", { name: "Branch spend" }));
expect(screen.getAllByText(/feature\/sample-/).length).toBeGreaterThan(0);
expect(screen.queryByText("outside-only")).not.toBeInTheDocument();
expect(screen.queryByText("orphan-only")).not.toBeInTheDocument();
await user.click(screen.getByRole("switch", { name: "Matched people only" }));
expect(screen.getByText("outside-only")).toBeInTheDocument();
expect(screen.getByText("orphan-only")).toBeInTheDocument();
await user.click(screen.getByRole("tab", { name: "Merged changes" }));
expect(screen.getByText("Outside change")).toBeInTheDocument();
expect(requests.mock.calls.every(([, init]) => init.method === "GET")).toBe(true);
});
it("shows a useful empty matched view and exposes all changes when the filter is off", async () => {
const sample = { ...createObservedDemo(7), people: [] };
vi.stubGlobal(
"fetch",
vi.fn(async (input: string) => {
const path = new URL(input, "http://localhost").pathname;
if (path.endsWith("/settings")) return Response.json(settings);
if (path.endsWith("/report")) return Response.json({ report: sample });
return Response.json(idle);
}),
);
const user = userEvent.setup();
render(<ObservedROIView accessToken="test-only-gateway-token" />);
expect(await screen.findByText("No merged changes from matched people in this period")).toBeInTheDocument();
await user.click(screen.getByRole("switch", { name: "Matched people only" }));
expect(screen.queryByText("No merged changes from matched people in this period")).not.toBeInTheDocument();
expect(screen.getAllByRole("link", { name: /Add repository search/ }).length).toBeGreaterThan(0);
});
it("previews every sample view before setup, changes sample periods without writes, and exits back to setup", async () => {
const requests = vi.fn(async (input: string, _init: RequestInit) => {
const path = new URL(input, "http://localhost").pathname;

View file

@ -1,6 +1,6 @@
"use client";
import { useState } from "react";
import { useMemo, useState } from "react";
import { ArrowDown, ArrowUp, CalendarDays, ChevronDown, ChevronRight, Link2, Search, Users } from "lucide-react";
import { Page, PageTabsList, PageTabsTrigger } from "@/components/shared/Page";
import { PageHeader, PageHeaderTitle } from "@/components/shared/PageHeader";
@ -11,6 +11,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@
import { Tabs, TabsContent } from "@/components/ui/tabs";
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table";
import ObservedAccounts from "./ObservedAccounts";
import { MatchedPeopleToggle } from "./MatchedPeopleToggle";
import { BranchSpend, PersonDetails, PullList } from "./ObservedDetails";
import {
change,
@ -20,9 +21,11 @@ import {
money,
number,
visiblePeople,
reportPeople,
filterObservedPulls,
weeklyMerges,
type Comparison,
type ObservedPerson,
type ReportPerson,
type ObservedSnapshot,
type PeopleSort,
} from "./observedData";
@ -83,7 +86,7 @@ function ShippingTrend({ snapshot, comparison }: { snapshot: ObservedSnapshot; c
return (
<div className="rounded-xl border p-5">
<div className="flex flex-wrap items-center justify-between gap-2">
<h2 className="text-sm font-medium">Shipping activity</h2>
<h2 className="text-sm font-medium">Repository shipping activity</h2>
<div className="flex gap-4 text-xs text-muted-foreground">
<span className="flex items-center gap-1.5">
<span className="size-2 rounded-sm bg-blue-500" />
@ -128,18 +131,25 @@ function ShippingTrend({ snapshot, comparison }: { snapshot: ObservedSnapshot; c
}
function PeopleTable({
snapshot,
rows,
provider,
matchedOnly,
comparison,
onSelect,
}: {
snapshot: ObservedSnapshot;
rows: ReportPerson[];
provider: ObservedSnapshot["source_provider"];
matchedOnly: boolean;
comparison: Comparison;
onSelect: (person: ObservedPerson) => void;
onSelect: (person: ReportPerson) => void;
}) {
const terms = changeTerms(snapshot.source_provider);
const terms = changeTerms(provider);
const [query, setQuery] = useState("");
const [sort, setSort] = useState<PeopleSort>("merged");
const people = visiblePeople(snapshot.people, query, sort);
const people = visiblePeople(rows, query, sort);
const emptyMessage = matchedOnly
? "No matched people. Link accounts or turn off the filter to see all contributors"
: "No contributors in these periods";
return (
<div className="space-y-3">
<div className="flex flex-wrap items-center justify-between gap-3">
@ -204,7 +214,7 @@ function PeopleTable({
const current = person.periods.current;
const baseline = person.periods[comparison];
return (
<TableRow key={person.email}>
<TableRow key={person.id}>
<TableCell className="py-3 pl-5">
<button
className="flex items-center gap-3 rounded text-left hover:underline"
@ -215,7 +225,9 @@ function PeopleTable({
</span>
<span>
<span className="block font-medium">{person.name}</span>
<span className="text-xs text-muted-foreground">{person.email}</span>
<span className="text-xs text-muted-foreground">
{person.email || `Not linked · ${person.host}`}
</span>
</span>
</button>
</TableCell>
@ -269,7 +281,7 @@ function PeopleTable({
</Table>
{people.length === 0 && (
<div className="p-10 text-center text-sm text-muted-foreground">
{query ? `No engineers match “${query}”` : "Link accounts to see your engineers"}
{query ? `No engineers match “${query}”` : emptyMessage}
</div>
)}
</div>
@ -380,8 +392,11 @@ export default function ObservedReport({
snapshot.people.length && snapshot.periods.current.merged_prs > 0 ? "people" : "pulls",
);
const [accountEmail, setAccountEmail] = useState<string | null>(null);
const [personEmail, setPersonEmail] = useState<string | null>(null);
const person = snapshot.people.find((entry) => entry.email === personEmail) ?? null;
const [matchedOnly, setMatchedOnly] = useState(true);
const people = useMemo(() => reportPeople(snapshot, matchedOnly), [snapshot, matchedOnly]);
const pulls = useMemo(() => filterObservedPulls(snapshot, "current", matchedOnly), [snapshot, matchedOnly]);
const [personId, setPersonId] = useState<string | null>(null);
const person = people.find((entry) => entry.id === personId) ?? null;
const terms = changeTerms(snapshot.source_provider);
const current = snapshot.periods.current;
const baseline = snapshot.periods[comparison];
@ -497,14 +512,15 @@ export default function ObservedReport({
</div>
<Tabs value={activeTab} onValueChange={setActiveTab} className="gap-3">
<div className="flex min-w-0 flex-wrap items-center gap-x-4 gap-y-2 border-b">
<PageTabsList className="min-w-0 flex-1 gap-4 border-0">
<PageTabsList className="min-w-0 flex-1 basis-full gap-4 border-0 xl:basis-auto">
<PageTabsTrigger value="people">
Engineers <span className="ml-1.5 text-muted-foreground">{snapshot.people.length}</span>
Engineers <span className="ml-1.5 text-muted-foreground">{people.length}</span>
</PageTabsTrigger>
<PageTabsTrigger value="pulls">{terms.requests}</PageTabsTrigger>
<PageTabsTrigger value="quality">Quality</PageTabsTrigger>
<PageTabsTrigger value="branches">Branch spend</PageTabsTrigger>
</PageTabsList>
{activeTab !== "quality" && <MatchedPeopleToggle checked={matchedOnly} onChange={setMatchedOnly} />}
{!readOnly && (
<Button
size="sm"
@ -520,9 +536,11 @@ export default function ObservedReport({
</div>
<TabsContent value="people">
<PeopleTable
snapshot={snapshot}
rows={people}
provider={snapshot.source_provider}
matchedOnly={matchedOnly}
comparison={comparison}
onSelect={(selected) => setPersonEmail(selected.email)}
onSelect={(selected) => setPersonId(selected.id)}
/>
</TabsContent>
<TabsContent value="pulls" className="space-y-5">
@ -561,15 +579,17 @@ export default function ObservedReport({
)}
<p className="mb-4 text-xs text-muted-foreground">
All repository {terms.plural}, including agent work without a known requester
{matchedOnly
? "Changes from people linked to internal accounts"
: `All repository ${terms.plural}, including agent work without a known requester`}
</p>
<PullList pulls={snapshot.pulls.current} provider={snapshot.source_provider} />
<PullList pulls={pulls} provider={snapshot.source_provider} matchedOnly={matchedOnly} />
</TabsContent>
<TabsContent value="quality">
<Quality snapshot={snapshot} comparison={comparison} />
</TabsContent>
<TabsContent value="branches">
<BranchSpend snapshot={snapshot} />
<BranchSpend snapshot={snapshot} matchedOnly={matchedOnly} />
</TabsContent>
</Tabs>
<div className="flex flex-wrap items-center justify-between gap-2 border-t pt-3 text-xs text-muted-foreground">
@ -591,17 +611,17 @@ export default function ObservedReport({
)}
{person && (
<PersonDetails
key={person.email}
key={person.id}
person={person}
snapshot={snapshot}
comparison={comparison}
onClose={() => setPersonEmail(null)}
onClose={() => setPersonId(null)}
onEdit={
readOnly
readOnly || !person.matched
? undefined
: () => {
setAccountEmail(person.email);
setPersonEmail(null);
setPersonId(null);
}
}
/>

View file

@ -137,6 +137,68 @@ describe("ROICalculatorView", () => {
vi.mocked(apiClient.put).mockResolvedValue({ report: summary, identity_map: { alice: "alice@example.com" } });
});
it.each(["github", "gitlab"])("filters people and branches by linked accounts for %s", async (provider) => {
const outsidePerson = {
...summary.people[0],
id: "outside",
email: "",
logins: ["outside"],
match_methods: ["no gateway match"],
spend: null,
};
const outsidePull = {
...summary.pulls[0],
number: 43,
title: "External contribution",
login: "outside",
email: "",
match_method: "no gateway match",
matched: false,
};
const linkedPerson = { ...summary.people[0], spend: null, match_methods: ["manual"], eligible: false };
const spendOnlyPerson = {
...summary.people[0],
id: "internal@example.test",
email: "internal@example.test",
logins: [],
spend: 8.5,
prs: 0,
match_methods: [],
estimated_prs: 0,
hours: 0,
eligible: false,
cost_per_hour: null,
};
const linkedPull = { ...summary.pulls[0], matched: false, match_method: "manual" };
const report = {
...summary,
source_provider: provider,
people: [linkedPerson, spendOnlyPerson, outsidePerson],
pulls: [linkedPull, outsidePull],
};
vi.mocked(apiClient.get).mockImplementation((path: string) => {
if (path.endsWith("/settings")) return Promise.resolve(settings);
if (path.endsWith("/report")) return Promise.resolve({ report });
return Promise.resolve(idleStatus);
});
const user = userEvent.setup();
render(<ROICalculatorView accessToken="token" />);
await user.click(await screen.findByRole("tab", { name: "People" }));
expect(screen.getByRole("switch", { name: "Matched people only" })).toBeChecked();
expect(screen.getByRole("button", { name: "alice" })).toBeInTheDocument();
expect(screen.getByRole("row", { name: /internal@example.test/ })).toHaveTextContent("$8.50");
expect(screen.queryByRole("button", { name: "outside" })).not.toBeInTheDocument();
await user.click(screen.getByRole("switch", { name: "Matched people only" }));
expect(screen.getByRole("button", { name: "outside" })).toBeInTheDocument();
await user.click(screen.getByRole("tab", { name: "Branches" }));
expect(screen.getByRole("switch", { name: "Matched people only" })).not.toBeChecked();
expect(screen.getByText("External contribution")).toBeInTheDocument();
await user.click(screen.getByRole("switch", { name: "Matched people only" }));
expect(screen.queryByText("External contribution")).not.toBeInTheDocument();
expect(screen.getByText("Improve request routing")).toBeInTheDocument();
expect(apiClient.put).not.toHaveBeenCalled();
});
it("shows the spend summary and opens an accessible pull reasoning dialog", async () => {
render(<ROICalculatorView accessToken="token" />);

View file

@ -16,6 +16,7 @@ import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogDescription } f
import { extractErrorMessage } from "@/utils/errorUtils";
import { isProxyAdminTierRole } from "@/utils/roles";
import ROISettingsPanel from "./ROISettingsPanel";
import { MatchedPeopleToggle } from "./MatchedPeopleToggle";
import { IdentityMatchDialog, type PersonMatchSelection, PullReasoningDialog } from "./ROICalculatorDialogs";
import { ROIBranches, ROIOverview, ROIPeopleView } from "./ROICalculatorViews";
import { filterPulls, formatSyncedAt } from "./roiCalculatorData";
@ -83,6 +84,7 @@ export default function ROICalculatorView({
const settingsLoaded = settings !== null && !loadingInitialData && !loadingLiveData;
const requestError = [error, demoError, reportError, syncError].filter(Boolean).join(" ");
const [query, setQuery] = React.useState("");
const [matchedOnly, setMatchedOnly] = React.useState(true);
const loadReport = React.useCallback(async () => {
if (!accessToken) return null;
@ -243,7 +245,10 @@ export default function ROICalculatorView({
[accessToken, readOnly],
);
const filteredPulls = React.useMemo(() => (summary ? filterPulls(summary.pulls, query) : []), [query, summary]);
const filteredPulls = React.useMemo(
() => (summary ? filterPulls(summary.pulls, query, matchedOnly) : []),
[query, summary, matchedOnly],
);
if (error && !settings && !loadingInitialData) {
return (
@ -368,6 +373,11 @@ export default function ROICalculatorView({
<PageTabsTrigger value="people">People</PageTabsTrigger>
<PageTabsTrigger value="branches">Branches</PageTabsTrigger>
</PageTabsList>
{view !== "overview" && (
<div className="flex justify-end">
<MatchedPeopleToggle checked={matchedOnly} onChange={setMatchedOnly} />
</div>
)}
</Tabs>
)}
@ -443,6 +453,7 @@ export default function ROICalculatorView({
<ROIBranches
summary={summary}
pulls={filteredPulls}
matchedOnly={matchedOnly}
query={query}
onQueryChange={setQuery}
onSelectPull={setSelectedPull}
@ -452,6 +463,7 @@ export default function ROICalculatorView({
<ROIPeopleView
summary={summary}
identityMap={settings.identity_map}
matchedOnly={matchedOnly}
onMatch={(person, login) => setMatchingPerson({ person, login })}
readOnly={readOnly}
/>

View file

@ -7,6 +7,7 @@ import { Input } from "@/components/ui/input";
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table";
import {
peopleCsv,
isMatchedPerson,
effortNote,
estimateLabel,
formatMoney,
@ -128,9 +129,11 @@ export function ROIBranches({
pulls,
query,
onQueryChange,
matchedOnly,
}: PullSelection & {
pulls: ROIPull[];
query: string;
matchedOnly: boolean;
onQueryChange: (query: string) => void;
}) {
return (
@ -145,6 +148,7 @@ export function ROIBranches({
summary={summary}
pulls={pulls}
query={query}
matchedOnly={matchedOnly}
onQueryChange={onQueryChange}
onSelectPull={onSelectPull}
/>
@ -159,12 +163,14 @@ function ROIPulls({
query = "",
onQueryChange,
compact = false,
matchedOnly = false,
onViewBranches,
}: PullSelection & {
pulls: ROIPull[];
query?: string;
onQueryChange?: (query: string) => void;
compact?: boolean;
matchedOnly?: boolean;
onViewBranches?: () => void;
}) {
const changeName = summary.source_provider === "gitlab" ? "merge request" : "pull request";
@ -173,7 +179,7 @@ function ROIPulls({
const metrics = summary.metrics;
const emptyMessage = compact
? "No merged changes with tagged costs yet. Open Branches to see how to add tags."
: `No merged ${changeName}s in this period.`;
: `No merged ${changeName}s ${matchedOnly ? "from matched people " : ""}in this period.`;
return (
<section aria-label={`Merged ${changeName}s`} className="space-y-4">
<div className="flex flex-wrap items-center justify-between gap-4">
@ -182,10 +188,7 @@ function ROIPulls({
<p className="text-sm text-muted-foreground">
{compact
? "Merged work ranked by tagged AI cost"
: `${metrics.merged_prs} ${changeName}s · ${metrics.estimated_prs} estimated`}
{!compact && metrics.pending_prs > 0 && (
<span className="text-amber-700 dark:text-amber-400"> · {metrics.pending_prs} need attention</span>
)}
: `${pulls.length} of ${metrics.merged_prs} ${changeName}s`}
</p>
</div>
{compact ? (
@ -440,14 +443,17 @@ export function ROIPeopleView({
identityMap,
onMatch,
readOnly = false,
matchedOnly = true,
}: {
summary: ROISummary;
identityMap: Record<string, string>;
onMatch: (person: ROIPerson, login: string) => void;
readOnly?: boolean;
matchedOnly?: boolean;
}) {
const people = matchedOnly ? summary.people.filter(isMatchedPerson) : summary.people;
const exportCsv = () => {
const url = URL.createObjectURL(new Blob([peopleCsv(summary)], { type: "text/csv;charset=utf-8" }));
const url = URL.createObjectURL(new Blob([peopleCsv({ ...summary, people })], { type: "text/csv;charset=utf-8" }));
const link = document.createElement("a");
link.href = url;
link.download = "litellm-roi.csv";
@ -481,7 +487,7 @@ export function ROIPeopleView({
</TableRow>
</TableHeader>
<TableBody>
{summary.people.map((person) => (
{people.map((person) => (
<TableRow key={person.id}>
<TableCell className="px-4 py-3">
<div className="flex flex-wrap items-center gap-2">
@ -504,12 +510,7 @@ export function ROIPeopleView({
<span>Unassigned gateway spend</span>
)}
<span className="text-xs text-muted-foreground">
{person.match_methods.some(
(method) =>
["manual", "commit email", "profile email"].includes(method) && person.spend != null,
)
? "Matched"
: "Unmatched"}
{isMatchedPerson(person) ? "Matched" : "Unmatched"}
</span>
</div>
<p className="mt-1 text-xs text-muted-foreground">
@ -531,10 +532,12 @@ export function ROIPeopleView({
</TableCell>
</TableRow>
))}
{summary.people.length === 0 && (
{people.length === 0 && (
<TableRow>
<TableCell className="h-32 text-center text-muted-foreground" colSpan={4}>
No people in this period.
{matchedOnly
? "No matched people in this period. Turn off the filter to see all contributors"
: "No people in this period"}
</TableCell>
</TableRow>
)}

View file

@ -1,13 +1,17 @@
import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";
import {
change,
duration,
money,
visiblePeople,
weeklyMerges,
filterObservedPulls,
reportPeople,
recordedBranches,
type ObservedPerson,
type ObservedSnapshot,
} from "./observedData";
import { createObservedDemo } from "./observedDemo";
const period = (merged: number, spend: number | null) => ({
merged_prs: merged,
@ -28,6 +32,126 @@ const person = (name: string, merged: number, spend: number | null): ObservedPer
});
describe("observed ROI metrics", () => {
it("shows the same contributors when the browser has no Map.groupBy", () => {
const sample = { ...createObservedDemo(7), people: [] };
const expected = reportPeople(sample, false);
const legacyMap = new Proxy(Map, {
get: (target, key, receiver) => (key === "groupBy" ? undefined : Reflect.get(target, key, receiver)),
});
vi.stubGlobal("Map", legacyMap);
try {
expect(reportPeople(sample, false)).toEqual(expected);
} finally {
vi.unstubAllGlobals();
}
});
it("filters by attributed changes across providers, independently of spend or author names", () => {
const sample = createObservedDemo(7);
const external = {
...sample.pulls.current[0],
url: "https://gitlab.com/demo/api/-/merge_requests/999",
author: "alex-demo",
agent: false,
connection_id: "demo-gitlab",
};
const report = { ...sample, pulls: { ...sample.pulls, current: [...sample.pulls.current, external] } };
const noSpend = {
...report,
people: report.people.map((row) => ({
...row,
periods: {
...row.periods,
current: { ...row.periods.current, gateway_recorded_spend: 0, spend_observation: "no_records" as const },
},
})),
};
expect(filterObservedPulls(noSpend, "current", true)).toEqual(sample.pulls.current);
expect(filterObservedPulls(noSpend, "current", false)).toEqual(report.pulls.current);
expect(filterObservedPulls(noSpend, "current", true).some((pull) => pull.agent)).toBe(true);
expect(reportPeople(noSpend, true).map((row) => row.email)).toEqual(sample.people.map((row) => row.email));
});
it("keeps unmatched identities separate by host, with unknown spend and accurate periods", () => {
const sample = createObservedDemo(7);
const external = {
...sample.pulls.current[0],
author: "contributor",
agent: false,
connection_id: "gitlab-public",
url: "https://gitlab.com/demo/api/-/merge_requests/999",
merge_hours: 4,
};
const otherHost = {
...external,
connection_id: "gitlab-private",
url: "https://git.example.test/demo/api/-/merge_requests/999",
merge_hours: null,
};
const requested = {
...external,
url: "https://gitlab.com/demo/api/-/merge_requests/1000",
author: "devin-ai",
agent: true,
requester: "contributor",
merge_hours: 8,
};
const unassigned = { ...requested, url: "https://gitlab.com/demo/api/-/merge_requests/1001", requester: "" };
const report = {
...sample,
pulls: {
...sample.pulls,
current: [...sample.pulls.current, external, otherHost, requested, unassigned],
previous: [external],
},
};
const outsiders = reportPeople(report, false).filter((row) => !row.matched);
expect(outsiders).toHaveLength(2);
expect(new Set(outsiders.map((row) => row.id)).size).toBe(2);
const publicPerson = outsiders.find((row) => row.host === "gitlab.com")!;
const expectedCurrent = {
merged_prs: 2,
prs_per_week: 2,
median_merge_hours: 6,
direct_authored: 1,
declared_agent_owned: 1,
spend_observation: "no_records",
recorded_spend_per_attributed_pr: null,
};
expect(publicPerson.periods.current).toMatchObject(expectedCurrent);
expect(publicPerson.periods.previous.merged_prs).toBe(1);
expect(outsiders.find((row) => row.host === "git.example.test")!.periods.current.median_merge_hours).toBeNull();
expect(filterObservedPulls(report, "current", false)).toContain(unassigned);
});
it("filters branch spend using matched change ownership and the full repository and branch key", () => {
const sample = createObservedDemo(7);
const own = sample.pulls.current[0];
const externalCost = { ...own.branch_cost, repo: "gitlab.com/outside/service", spend: 99 };
const external = {
...own,
url: "https://gitlab.com/outside/service/-/merge_requests/999",
branch_cost: externalCost,
};
const shared = { repo: own.branch_cost.repo, branch: "shared", spend: 12, requests: 4 };
const ambiguous = {
...own,
source_branch: shared.branch,
branch_cost: { ...shared, status: "ambiguous" as const },
};
const report = {
...sample,
pulls: { ...sample.pulls, current: [ambiguous, external] },
unlinked_branches: [shared, { ...shared, branch: "unowned" }],
};
expect(recordedBranches(report, true)).toEqual([shared]);
expect(recordedBranches(report, false).map((row) => row.branch)).toEqual([
externalCost.branch,
"shared",
"unowned",
]);
});
it("does not claim infinite growth when the baseline is missing", () => {
expect(change(12, 0)).toBeNull();
expect(change(0, 0)).toBeNull();

View file

@ -57,6 +57,7 @@ const pullFields = {
.refine((url) => new URL(url).protocol === "https:"),
author: z.string(),
agent: z.boolean(),
requester: z.string().optional(),
merged_at: z.string(),
merge_hours: z.number().nullable(),
repo: z.string(),
@ -111,6 +112,7 @@ export type ObservedPull = ObservedSnapshot["pulls"]["current"][number];
export type Period = keyof ObservedSnapshot["periods"];
export type Comparison = Exclude<Period, "current">;
export type PeopleSort = "merged" | "spend" | "cost" | "name";
export type ReportPerson = ObservedPerson & { id: string; matched: boolean; host: string };
export const number = (value: number | null) =>
value === null ? "Unavailable" : value.toLocaleString("en-US", { maximumFractionDigits: 1 });
@ -159,7 +161,7 @@ export function dateRange(window: { start: string; end: string }) {
return `${date(window.start)} – ${date(window.end)}, ${window.end.slice(0, 4)}`;
}
export function visiblePeople(people: ObservedPerson[], query: string, sort: PeopleSort) {
export function visiblePeople<T extends ObservedPerson>(people: T[], query: string, sort: PeopleSort) {
const value = (person: ObservedPerson) => {
const current = person.periods.current;
if (sort === "spend") return current.spend_observation === "no_records" ? -1 : current.gateway_recorded_spend;
@ -192,15 +194,99 @@ export function syncMessage(status: ObservedStatus, report: ObservedSnapshot | n
return report ? `Updated ${new Date(report.captured_at).toLocaleString()}` : "";
}
export function recordedBranches(snapshot: ObservedSnapshot) {
export function recordedBranches(snapshot: ObservedSnapshot, matchedOnly = false) {
const matched = snapshot.pulls.current.flatMap((pull) => {
const cost = pull.branch_cost;
if (cost.status !== "matched" || cost.spend === null) return [];
return [{ repo: cost.repo, branch: cost.branch, spend: cost.spend, requests: cost.requests }];
});
return [
const rows = [
...new Map([...matched, ...snapshot.unlinked_branches].map((row) => [`${row.repo}\n${row.branch}`, row])).values(),
].toSorted((a, b) => b.spend - a.spend);
];
const keys = new Set(
filterObservedPulls(snapshot, "current", true).map(
(pull) => `${pull.branch_cost.repo}\n${pull.branch_cost.branch}`,
),
);
return rows
.filter((row) => !matchedOnly || keys.has(`${row.repo}\n${row.branch}`))
.toSorted((a, b) => b.spend - a.spend);
}
export function filterObservedPulls(snapshot: ObservedSnapshot, period: Period, matchedOnly: boolean) {
if (!matchedOnly) return snapshot.pulls[period];
const urls = new Set(snapshot.people.flatMap((person) => person.periods[period].pr_urls));
return snapshot.pulls[period].filter((pull) => urls.has(pull.url));
}
function unmatchedPeriod(
pulls: ObservedPull[],
window: { start: string; end: string },
): ObservedPerson["periods"]["current"] {
const hours = pulls
.flatMap((pull) => (pull.merge_hours === null ? [] : [pull.merge_hours]))
.toSorted((a, b) => a - b);
const middle = Math.floor(hours.length / 2);
const median = hours.length % 2 ? hours[middle] : (hours[middle - 1] + hours[middle]) / 2;
const days = (Date.parse(window.end) - Date.parse(window.start)) / 86_400_000 + 1;
return {
merged_prs: pulls.length,
prs_per_week: (pulls.length * 7) / days,
median_merge_hours: hours.length ? median : null,
direct_authored: pulls.filter((pull) => !pull.agent).length,
declared_agent_owned: pulls.filter((pull) => pull.agent).length,
gateway_recorded_spend: 0,
recorded_spend_per_attributed_pr: null,
spend_observation: "no_records",
pr_urls: pulls.map((pull) => pull.url),
};
}
const ownerLogin = (pull: ObservedPull) => (pull.agent ? pull.requester ?? "" : pull.author);
const accountKey = (pull: ObservedPull) =>
`${pull.connection_id ?? new URL(pull.url).origin}\n${ownerLogin(pull).toLowerCase()}`;
export function reportPeople(snapshot: ObservedSnapshot, matchedOnly: boolean): ReportPerson[] {
const matched = snapshot.people.map((person) => ({ ...person, id: person.email, matched: true, host: "" }));
if (matchedOnly) return matched;
const unmatchedGroups = (period: Period) => {
const urls = new Set(snapshot.people.flatMap((person) => person.periods[period].pr_urls));
const pulls = snapshot.pulls[period].filter((pull) => !urls.has(pull.url) && ownerLogin(pull));
const sorted = pulls
.map((pull) => ({ key: accountKey(pull), pull }))
.toSorted((a, b) => a.key.localeCompare(b.key));
const starts = sorted.flatMap((entry, index) =>
index === 0 || entry.key !== sorted[index - 1].key ? [index] : [],
);
return new Map(
starts.map((start, index) => [
sorted[start].key,
sorted.slice(start, starts[index + 1]).map((entry) => entry.pull),
]),
);
};
const groups = {
current: unmatchedGroups("current"),
previous: unmatchedGroups("previous"),
last_year: unmatchedGroups("last_year"),
};
const accounts = new Map([...groups.current, ...groups.previous, ...groups.last_year]);
const unmatched = [...accounts].map(([id, pulls]) => {
const pull = pulls[0];
const login = ownerLogin(pull);
const period = (key: Period) => unmatchedPeriod(groups[key].get(id) ?? [], snapshot.periods[key].window);
return {
id,
matched: false,
host: new URL(pull.url).host,
name: login,
email: "",
logins: [login],
accounts: [{ connection_id: pull.connection_id ?? "", login }],
periods: { current: period("current"), previous: period("previous"), last_year: period("last_year") },
};
});
return [...matched, ...unmatched];
}
export function changeTerms(provider: ObservedSnapshot["source_provider"]) {

View file

@ -10,9 +10,11 @@ import {
formatNumber,
formatSyncedAt,
highestCostPulls,
isMatchedPerson,
isMatchedPull,
peopleCsv,
} from "./roiCalculatorData";
import type { ROIPull } from "./roiCalculatorData";
import type { ROIPerson, ROIPull } from "./roiCalculatorData";
const pull = (overrides: Partial<ROIPull>): ROIPull => ({
source_repo: "github.com/org/repo",
@ -43,6 +45,57 @@ const summary = {
};
describe("ROI calculator display helpers", () => {
it.each([0, 8.5])("retains internal accounts with %s recorded spend and no changes in the filtered CSV", (spend) => {
const internal: ROIPerson = {
id: "internal@example.test",
email: "internal@example.test",
logins: [],
spend,
hours: 0,
prs: 0,
estimated_prs: 0,
pending_prs: 0,
match_methods: [],
eligible: false,
cost_per_hour: null,
};
const external = { ...internal, id: "outside", email: "", spend: null, logins: ["outside"] };
const people = [internal, external].filter(isMatchedPerson);
expect(people).toEqual([internal]);
const report = { start: "2026-09-01", end: "2026-09-30", effort_basis: "without_ai", people };
expect(peopleCsv(report)).toContain(`"internal@example.test","","${spend}","0","0"`);
expect(peopleCsv(report)).not.toContain("outside");
});
it("keeps linked identities without spend or estimates and excludes unrelated tagged spend", () => {
const manual = pull({ match_method: "manual", matched: false });
const external = pull({
email: "",
match_method: "no gateway match",
branch_cost: { repo: "org/repo", branch: "external", spend: 10, requests: 2, status: "matched" },
});
expect(isMatchedPull(manual)).toBe(true);
expect(filterPulls([manual, external], "", true)).toEqual([manual]);
expect(filterPulls([manual, external], "", false)).toEqual([manual, external]);
expect(filterPulls([manual, external], "missing", true)).toEqual([]);
const person: ROIPerson = {
id: "linked",
email: "linked@example.test",
logins: ["linked"],
spend: null,
hours: 0,
prs: 1,
estimated_prs: 0,
pending_prs: 1,
match_methods: ["manual"],
eligible: false,
cost_per_hour: null,
};
expect(isMatchedPerson(person)).toBe(true);
expect(isMatchedPerson({ ...person, match_methods: ["no gateway match"] })).toBe(false);
expect(isMatchedPerson({ ...person, email: "" })).toBe(false);
});
it("ranks only attributed PR costs, limits the overview to five and preserves report order", () => {
const pulls = [2, 6, 1, 4, 3, 5].map((spend) =>
pull({

View file

@ -60,10 +60,16 @@ export const estimateLabel = (estimate: ROIEstimate): string => {
return "Needs review";
};
export const filterPulls = (pulls: ROIPull[], query: string): ROIPull[] => {
const matchedMethods = new Set(["manual", "commit email", "profile email"]);
export const isMatchedPerson = (person: ROIPerson) =>
Boolean(person.email) && (person.spend !== null || person.match_methods.some((method) => matchedMethods.has(method)));
export const isMatchedPull = (pull: ROIPull) => Boolean(pull.email) && matchedMethods.has(pull.match_method);
export const filterPulls = (pulls: ROIPull[], query: string, matchedOnly = false): ROIPull[] => {
const normalized = query.trim().toLocaleLowerCase();
if (!normalized) return pulls;
return pulls.filter((pull) =>
const visible = matchedOnly ? pulls.filter(isMatchedPull) : pulls;
if (!normalized) return visible;
return visible.filter((pull) =>
`${pull.title} ${pull.repo} ${pull.number} ${pull.login} ${pull.source_branch ?? ""}`
.toLocaleLowerCase()
.includes(normalized),