feat(ui): compare network monitor targets within the same system (#2477)

Co-authored-by: henrygd <hank@henrygd.me>

internal/site/src/components/network-monitors-table/monitor-dialog.tsx

@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useRef, useState } from "react"
+import { type ReactNode, useCallback, useEffect, useRef, useState } from "react"
 import { Trans, useLingui } from "@lingui/react/macro"
 import { useStore } from "@nanostores/react"
 import { pb } from "@/lib/api"
@@ -23,10 +23,11 @@ import { Input } from "@/components/ui/input"
 import { Label } from "@/components/ui/label"
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"
 import { Textarea } from "@/components/ui/textarea"
-import { ChevronDownIcon, ListIcon, PlusIcon, SearchIcon, ServerIcon } from "lucide-react"
+import { ChevronDownIcon, GlobeIcon, ListIcon, type LucideIcon, PlusIcon, SearchIcon, ServerIcon } from "lucide-react"
 import { useToast } from "@/components/ui/use-toast"
 import { $systems } from "@/lib/stores"
 import { cn, supportsNetworkMonitors } from "@/lib/utils"
+import { getMonitorTarget } from "@/lib/network-monitor-utils"
 import type { NetworkMonitorRecord } from "@/types"
 import * as v from "valibot"
 
@@ -202,6 +203,7 @@ export function SystemMultiSelect({
 	className,
 	systemIds,
 	placeholder,
+	canSelectMore,
 }: {
 	id: string
 	selectedSystemIds: Set<string>
@@ -211,8 +213,107 @@ export function SystemMultiSelect({
 	/** Limit the options to these systems. Defaults to all systems that support network monitors. */
 	systemIds?: string[]
 	placeholder?: string
+	canSelectMore?: boolean
 }) {
 	const systems = useStore($systems)
+	const { t } = useLingui()
+	const options = systems
+		.filter((system) => (systemIds ? systemIds.includes(system.id) : supportsNetworkMonitors(system)))
+		.map((system) => ({ id: system.id, label: system.name }))
+	return (
+		<MultiSelect
+			id={id}
+			options={options}
+			selectedIds={selectedSystemIds}
+			onChange={onChange}
+			disabled={disabled}
+			className={className}
+			icon={ServerIcon}
+			canSelectMore={canSelectMore}
+			placeholder={placeholder ?? t`Select systems`}
+			searchPlaceholder={t`Search systems`}
+			emptyText={<Trans>No systems found.</Trans>}
+		/>
+	)
+}
+
+/** Pick monitors by target, e.g. other targets on the same system to compare against. */
+export function MonitorMultiSelect({
+	id,
+	monitors,
+	selectedMonitorIds,
+	onChange,
+	disabled,
+	className,
+	placeholder,
+	canSelectMore,
+}: {
+	id: string
+	monitors: NetworkMonitorRecord[]
+	selectedMonitorIds: Set<string>
+	onChange: (ids: Set<string>) => void
+	disabled?: boolean
+	className?: string
+	placeholder?: string
+	canSelectMore?: boolean
+}) {
+	const { t } = useLingui()
+	const options = monitors
+		.map((monitor) => ({ id: monitor.id, label: getMonitorTarget(monitor), server: monitor.server }))
+		.sort((a, b) => a.label.localeCompare(b.label))
+	return (
+		<MultiSelect
+			id={id}
+			options={options}
+			selectedIds={selectedMonitorIds}
+			onChange={onChange}
+			disabled={disabled}
+			className={cn("ps-9.5", className)}
+			icon={GlobeIcon}
+			canSelectMore={canSelectMore}
+			placeholder={placeholder ?? t`Select targets`}
+			searchPlaceholder={t`Search targets`}
+			emptyText={<Trans>No targets found.</Trans>}
+			renderOption={(option) => (
+				<>
+					<span className="truncate">{option.label}</span>
+					{option.server && <span className="ms-auto shrink-0 text-xs text-muted-foreground">{option.server}</span>}
+				</>
+			)}
+		/>
+	)
+}
+
+type MultiSelectOption = { id: string; label: string }
+
+function MultiSelect<T extends MultiSelectOption>({
+	id,
+	options,
+	selectedIds,
+	onChange,
+	disabled,
+	className,
+	icon: Icon,
+	placeholder,
+	searchPlaceholder,
+	emptyText,
+	renderOption = (option) => <span className="truncate">{option.label}</span>,
+	canSelectMore = true,
+}: {
+	id: string
+	options: T[]
+	selectedIds: Set<string>
+	onChange: (ids: Set<string>) => void
+	disabled?: boolean
+	className?: string
+	icon: LucideIcon
+	placeholder: string
+	searchPlaceholder: string
+	emptyText: ReactNode
+	renderOption?: (option: T) => ReactNode
+	/** False once the selection is full; only already selected options can then be toggled. */
+	canSelectMore?: boolean
+}) {
 	const { t } = useLingui()
 	const [search, setSearch] = useState("")
 	const searchRef = useRef<HTMLInputElement>(null)
@@ -225,19 +326,15 @@ export function SystemMultiSelect({
 	}, [])
 	const contentRef = useRef<HTMLDivElement>(null)
 	const query = search.trim().toLocaleLowerCase()
-	const filteredSystems = systems.filter(
-		(system) =>
-			(systemIds ? systemIds.includes(system.id) : supportsNetworkMonitors(system)) &&
-			system.name.toLocaleLowerCase().includes(query)
-	)
-	const allSelected = filteredSystems.every((system) => selectedSystemIds.has(system.id))
-	const anySelected = filteredSystems.some((system) => selectedSystemIds.has(system.id))
+	const filteredOptions = options.filter((option) => option.label.toLocaleLowerCase().includes(query))
+	const allSelected = filteredOptions.every((option) => selectedIds.has(option.id))
+	const anySelected = filteredOptions.some((option) => selectedIds.has(option.id))
 
 	const selectFiltered = (selected: boolean) => {
-		const next = new Set(selectedSystemIds)
-		for (const system of filteredSystems) {
-			if (selected) next.add(system.id)
-			else next.delete(system.id)
+		const next = new Set(selectedIds)
+		for (const option of filteredOptions) {
+			if (selected) next.add(option.id)
+			else next.delete(option.id)
 		}
 		onChange(next)
 	}
@@ -251,13 +348,13 @@ export function SystemMultiSelect({
 					variant="outline"
 					className={cn("relative w-full min-w-0 ps-10 pe-10 justify-start font-normal text-start", className)}
 				>
-					<ServerIcon className="size-3.5 absolute start-4 top-1/2 -translate-y-1/2 opacity-85" />
+					<Icon className="size-3.5 absolute start-4 top-1/2 -translate-y-1/2 opacity-85" />
 					<span className="truncate">
-						{selectedSystemIds.size === 0
-							? (placeholder ?? t`Select systems`)
-							: selectedSystemIds.size === 1
-								? systems.find((s) => selectedSystemIds.has(s.id))?.name
-								: t`${selectedSystemIds.size} selected`}
+						{selectedIds.size === 0
+							? placeholder
+							: selectedIds.size === 1
+								? options.find((option) => selectedIds.has(option.id))?.label
+								: t`${selectedIds.size} selected`}
 					</span>
 					<ChevronDownIcon className="size-4 absolute end-4 top-1/2 -translate-y-1/2 opacity-50" />
 				</Button>
@@ -280,8 +377,8 @@ export function SystemMultiSelect({
 							ref={focusSearchOnMount}
 							value={search}
 							onChange={(event) => setSearch(event.target.value)}
-							placeholder={t`Search systems`}
-							aria-label={t`Search systems`}
+							placeholder={searchPlaceholder}
+							aria-label={searchPlaceholder}
 							className="h-10 min-w-0 rounded-none border-0 bg-transparent px-0 shadow-none focus-visible:ring-0 focus-visible:ring-offset-0"
 							onKeyDown={(event) => {
 								if (event.key === "Escape") return
@@ -303,7 +400,7 @@ export function SystemMultiSelect({
 						<div className="flex items-center">
 							<DropdownMenuItem
 								className="px-1.5 py-1 text-xs text-muted-foreground"
-								disabled={!filteredSystems.length || allSelected}
+								disabled={!filteredOptions.length || allSelected || !canSelectMore}
 								onSelect={(event) => {
 									event.preventDefault()
 									selectFiltered(true)
@@ -325,32 +422,29 @@ export function SystemMultiSelect({
 								{query ? <Trans>Clear matches</Trans> : <Trans>Clear all</Trans>}
 							</DropdownMenuItem>
 						</div>
-						<span className="px-1.5 text-xs tabular-nums text-muted-foreground">
-							{t`${selectedSystemIds.size} selected`}
-						</span>
+						<span className="px-1.5 text-xs tabular-nums text-muted-foreground">{t`${selectedIds.size} selected`}</span>
 					</div>
 				</div>
 				<div className="min-h-0 overflow-y-auto">
-					{filteredSystems.length === 0 && (
-						<output className="block px-2.5 py-3 text-sm text-muted-foreground">
-							<Trans>No systems found.</Trans>
-						</output>
+					{filteredOptions.length === 0 && (
+						<output className="block px-2.5 py-3 text-sm text-muted-foreground">{emptyText}</output>
 					)}
-					{filteredSystems.map((sys) => (
+					{filteredOptions.map((option) => (
 						<DropdownMenuCheckboxItem
-							key={sys.id}
-							checked={selectedSystemIds.has(sys.id)}
+							key={option.id}
+							checked={selectedIds.has(option.id)}
+							disabled={!canSelectMore && !selectedIds.has(option.id)}
 							onSelect={(event) => event.preventDefault()}
 							onCheckedChange={(checked) => {
-								const next = new Set(selectedSystemIds)
-								if (checked) next.add(sys.id)
-								else next.delete(sys.id)
+								const next = new Set(selectedIds)
+								if (checked) next.add(option.id)
+								else next.delete(option.id)
 								onChange(next)
 							}}
 							className="group min-w-0 gap-2.5 py-2 ps-2.5"
 							indicatorClassName="static size-4 shrink-0 rounded border border-input group-data-[state=checked]:border-primary group-data-[state=checked]:bg-primary group-data-[state=checked]:text-primary-foreground [&_svg]:size-3"
 						>
-							<span className="truncate">{sys.name}</span>
+							{renderOption(option)}
 						</DropdownMenuCheckboxItem>
 					))}
 				</div>

internal/site/src/components/network-monitors-table/network-monitors-table.tsx

@@ -39,7 +39,7 @@ import { SystemStatus } from "@/lib/enums"
 import { $allSystemsById, $direction, $textMeasureVersion, $userSettings, getUserChartTime } from "@/lib/stores"
 import { cn, formatShortDate, isVisuallyLonger, matchesFilterGroups, parseFilterGroups, parseSemVer } from "@/lib/utils"
 import type { ChartOptions, MonitorCertInfo, NetworkMonitorRecord } from "@/types"
-import { AddMonitorDialog, EditMonitorDialog, SystemMultiSelect } from "./monitor-dialog"
+import { AddMonitorDialog, EditMonitorDialog, MonitorMultiSelect, SystemMultiSelect } from "./monitor-dialog"
 import {
 	ArrowDownIcon,
 	ArrowLeftRightIcon,
@@ -67,7 +67,8 @@ import {
 import { Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle } from "@/components/ui/sheet"
 import ChartTimeSelect from "@/components/charts/chart-time-select"
 import { LossChart, AvgMinMaxResponseChart, ResponseChart } from "@/components/routes/system/charts/monitors-charts"
-import { useMatchingMonitors, useNetworkMonitorStats } from "@/lib/use-network-monitors"
+import { getMonitorCompareState } from "@/lib/monitor-compare"
+import { useCompareMonitors, useNetworkMonitorStats } from "@/lib/use-network-monitors"
 import { useStore } from "@nanostores/react"
 import { atom } from "nanostores"
 import { Separator } from "../ui/separator"
@@ -470,6 +471,7 @@ export default function NetworkMonitorsTableNew({
 					visibleColumnsKey={visibleColumnsKey}
 					rowSelection={rowSelection}
 					isLoading={isLoading}
+					includesAllSystems={!systemId}
 				/>
 			</div>
 		</Card>
@@ -483,6 +485,7 @@ const NetworkMonitorsTable = memo(function NetworkMonitorTable({
 	visibleColumnsKey,
 	rowSelection,
 	isLoading,
+	includesAllSystems,
 }: {
 	table: TableType<NetworkMonitorRecord>
 	rows: Row<NetworkMonitorRecord>[]
@@ -490,6 +493,8 @@ const NetworkMonitorsTable = memo(function NetworkMonitorTable({
 	visibleColumnsKey: string
 	rowSelection: RowSelectionState
 	isLoading: boolean
+	/** The table lists every system's monitors, so the sheet can compare without fetching. */
+	includesAllSystems: boolean
 }) {
 	const scrollRef = useRef<HTMLDivElement>(null)
 	const [sheetOpen, setSheetOpen] = useState(false)
@@ -560,6 +565,8 @@ const NetworkMonitorsTable = memo(function NetworkMonitorTable({
 					setSheetOpen(nextOpen)
 				}}
 				monitor={activeMonitor}
+				monitors={table.options.data}
+				includesAllSystems={includesAllSystems}
 			/>
 		</div>
 	)
@@ -630,16 +637,29 @@ function NetworkMonitorSheet({
 	open,
 	onOpenChange,
 	monitor,
+	monitors,
+	includesAllSystems,
 }: {
 	open: boolean
 	onOpenChange: (open: boolean) => void
 	monitor?: NetworkMonitorRecord
+	monitors: NetworkMonitorRecord[]
+	includesAllSystems: boolean
 }) {
 	if (!monitor) {
 		return null
 	}
 
-	return <NetworkMonitorSheetContent key={monitor.system} open={open} onOpenChange={onOpenChange} monitor={monitor} />
+	return (
+		<NetworkMonitorSheetContent
+			key={monitor.system}
+			open={open}
+			onOpenChange={onOpenChange}
+			monitor={monitor}
+			monitors={monitors}
+			includesAllSystems={includesAllSystems}
+		/>
+	)
 }
 
 const certExpiryTextColors = { ok: "", warning: "text-yellow-600 dark:text-yellow-500", critical: "text-red-500" }
@@ -676,10 +696,16 @@ function NetworkMonitorSheetContent({
 	open,
 	onOpenChange,
 	monitor,
+	monitors,
+	includesAllSystems,
 }: {
 	open: boolean
 	onOpenChange: (open: boolean) => void
 	monitor: NetworkMonitorRecord
+	/** Table monitors; used to find other targets on the same system to compare against. */
+	monitors: NetworkMonitorRecord[]
+	/** Whether `monitors` covers every system, so other systems' monitors needn't be fetched. */
+	includesAllSystems: boolean
 }) {
 	// Keep monitor exploration independent of the system charts' time range.
 	const [chartTimeStore] = useState(() => {
@@ -691,8 +717,7 @@ function NetworkMonitorSheetContent({
 	const systems = useStore($allSystemsById)
 	const system = systems[monitor.system]
 
-	// Same target probed from other systems, for side-by-side comparison (#2385).
-	const matchingMonitors = useMatchingMonitors(monitor, open)
+	const [compareTargetIds, setCompareTargetIds] = useState<Set<string>>(() => new Set())
 	const [compareSystemIds, setCompareSystemIds] = useState<Set<string>>(() => new Set())
 	// Scoped to this sheet so a filter doesn't carry over to other monitors' sheets.
 	const [compareFilterStore, setCompareFilterStore] = useState(() => atom(""))
@@ -701,16 +726,25 @@ function NetworkMonitorSheetContent({
 	if (compareMonitorId !== monitor.id) {
 		setCompareMonitorId(monitor.id)
 		setCompareSystemIds(new Set())
+		setCompareTargetIds(new Set())
 		setCompareFilterStore(atom(""))
 	}
-	const matchingSystemIds = useMemo(() => matchingMonitors.map((m) => m.system), [matchingMonitors])
-	// The opened system is always charted; the picker only adds other systems to compare against.
-	const compareMonitors = useMemo(
-		() => [monitor, ...matchingMonitors.filter((m) => compareSystemIds.has(m.system))],
-		[monitor, matchingMonitors, compareSystemIds]
+	// Other systems' monitors come from the table when it lists every system, otherwise from one fetch.
+	const fetchedMonitors = useCompareMonitors(monitor.system, monitor.protocol, open && !includesAllSystems)
+	const compare = useMemo(
+		() =>
+			getMonitorCompareState({
+				monitor,
+				localMonitors: monitors,
+				otherMonitors: includesAllSystems ? monitors : fetchedMonitors,
+				selectedSystemIds: compareSystemIds,
+				selectedTargetIds: compareTargetIds,
+				getSystemName: (id) => systems[id]?.name ?? id,
+			}),
+		[monitor, monitors, includesAllSystems, fetchedMonitors, compareSystemIds, compareTargetIds, systems]
 	)
+	const { compareMonitors } = compare
 	const comparing = compareMonitors.length > 1
-	const getSystemName = useCallback((m: NetworkMonitorRecord) => systems[m.system]?.name ?? m.system, [systems])
 
 	const monitorStats = useNetworkMonitorStats({
 		systemId: monitor.system,
@@ -766,21 +800,31 @@ function NetworkMonitorSheetContent({
 				<div className="grid gap-4">
 					<div className="flex flex-wrap items-center gap-2">
 						<ChartTimeSelect
-							className="bg-card flex-1 basis-48"
+							className="bg-card flex-1 min-w-0 basis-full sm:basis-0"
 							agentVersion={chartData.agentVersion}
 							chartTimeStore={chartTimeStore}
 							allowRealtime={false}
 						/>
-						{matchingMonitors.length > 0 && (
-							<SystemMultiSelect
-								id="monitor-compare-systems"
-								className="w-full sm:w-1/3 shrink-0 bg-card"
-								systemIds={matchingSystemIds}
-								selectedSystemIds={compareSystemIds}
-								onChange={setCompareSystemIds}
-								placeholder={t`Compare with other systems`}
-							/>
-						)}
+						<MonitorMultiSelect
+							id="monitor-compare-targets"
+							className="flex-1 min-w-0 basis-full sm:basis-0 bg-card"
+							monitors={compare.targetOptions}
+							selectedMonitorIds={compare.selectedTargetIds}
+							onChange={setCompareTargetIds}
+							disabled={compare.targetOptions.length === 0}
+							canSelectMore={compare.canAddTarget}
+							placeholder={t`Compare with other targets`}
+						/>
+						<SystemMultiSelect
+							id="monitor-compare-systems"
+							className="flex-1 min-w-0 basis-full sm:basis-0 bg-card"
+							systemIds={compare.systemOptions}
+							selectedSystemIds={compare.selectedSystemIds}
+							onChange={setCompareSystemIds}
+							disabled={compare.systemOptions.length === 0}
+							canSelectMore={compare.canAddSystem}
+							placeholder={t`Compare with other systems`}
+						/>
 					</div>
 					{comparing ? (
 						<>
@@ -790,7 +834,7 @@ function NetworkMonitorSheetContent({
 								monitors={compareMonitors}
 								chartData={chartData}
 								empty={!hasMonitorStats}
-								getLabel={getSystemName}
+								getLabel={compare.getLabel}
 								filterStore={compareFilterStore}
 							/>
 							<LossChart
@@ -799,7 +843,7 @@ function NetworkMonitorSheetContent({
 								monitors={compareMonitors}
 								chartData={chartData}
 								empty={!hasMonitorStats}
-								getLabel={getSystemName}
+								getLabel={compare.getLabel}
 								filterStore={compareFilterStore}
 							/>
 						</>

internal/site/src/lib/monitor-compare.test.ts

@@ -0,0 +1,173 @@
+import { expect, test } from "bun:test"
+import type { NetworkMonitorRecord } from "@/types"
+import {
+	getMonitorCompareState,
+	getMonitorIdentityKey,
+	getMonitorTarget,
+	MAX_COMPARE_MONITORS,
+} from "./monitor-compare"
+
+function mon(id: string, system: string, target: string, extra: Partial<NetworkMonitorRecord> = {}) {
+	return { id, system, target, protocol: "icmp", port: 0, server: "", interval: 30, ...extra } as NetworkMonitorRecord
+}
+
+const systemNames: Record<string, string> = { a: "Alpha", b: "Bravo", c: "Charlie" }
+
+function state(
+	monitor: NetworkMonitorRecord,
+	monitors: NetworkMonitorRecord[],
+	selectedSystemIds: string[] = [],
+	selectedTargetIds: string[] = []
+) {
+	return getMonitorCompareState({
+		monitor,
+		localMonitors: monitors,
+		otherMonitors: monitors,
+		selectedSystemIds: new Set(selectedSystemIds),
+		selectedTargetIds: new Set(selectedTargetIds),
+		getSystemName: (id) => systemNames[id] ?? id,
+	})
+}
+
+const ids = (monitors: NetworkMonitorRecord[]) => monitors.map((m) => m.id)
+
+// a probes one and two; b probes one and two; c probes only one
+const a1 = mon("a1", "a", "one.example")
+const a2 = mon("a2", "a", "two.example")
+const b1 = mon("b1", "b", "one.example")
+const b2 = mon("b2", "b", "two.example")
+const c1 = mon("c1", "c", "one.example")
+const all = [a1, a2, b1, b2, c1]
+
+test("formats tcp targets with their port", () => {
+	expect(getMonitorTarget({ target: "example.com", protocol: "icmp", port: 0 })).toBe("example.com")
+	expect(getMonitorTarget({ target: "example.com", protocol: "tcp", port: 443 })).toBe("example.com:443")
+	expect(getMonitorTarget({ target: "::1", protocol: "tcp", port: 22 })).toBe("[::1]:22")
+})
+
+test("identity ignores the system but not protocol, port or server", () => {
+	expect(getMonitorIdentityKey(a1)).toBe(getMonitorIdentityKey(b1))
+	expect(getMonitorIdentityKey(a1)).not.toBe(getMonitorIdentityKey({ ...a1, protocol: "http" }))
+	expect(getMonitorIdentityKey(a1)).not.toBe(getMonitorIdentityKey({ ...a1, port: 80 }))
+	expect(getMonitorIdentityKey(a1)).not.toBe(getMonitorIdentityKey({ ...a1, server: "1.1.1.1" }))
+})
+
+test("with nothing selected only the opened monitor is charted", () => {
+	const s = state(a1, all)
+	expect(s.systemOptions).toEqual(["b", "c"])
+	expect(ids(s.targetOptions)).toEqual(["a2"])
+	expect(ids(s.compareMonitors)).toEqual(["a1"])
+})
+
+test("only offers targets and systems with the same protocol", () => {
+	const aHttp = mon("aHttp", "a", "https://two.example", { protocol: "http" })
+	const bTcp = mon("bTcp", "b", "one.example", { protocol: "tcp", port: 443 })
+	const s = state(a1, [a1, a2, aHttp, bTcp, c1])
+	expect(ids(s.targetOptions)).toEqual(["a2"])
+	expect(s.systemOptions).toEqual(["c"])
+})
+
+test("comparing targets charts them on the opened system, labelled by target", () => {
+	const s = state(a1, all, [], ["a2"])
+	expect(ids(s.compareMonitors)).toEqual(["a1", "a2"])
+	expect(s.getLabel(a1)).toBe("one.example")
+	expect(s.getLabel(a2)).toBe("two.example")
+})
+
+test("comparing systems charts the opened target on them, labelled by system", () => {
+	const s = state(a1, all, ["b", "c"])
+	expect(ids(s.compareMonitors)).toEqual(["a1", "b1", "c1"])
+	expect(s.getLabel(a1)).toBe("Alpha")
+	expect(s.getLabel(c1)).toBe("Charlie")
+})
+
+test("each compared target is charted on every selected system", () => {
+	const s = state(a1, all, ["b"], ["a2"])
+	expect(ids(s.compareMonitors)).toEqual(["a1", "a2", "b1", "b2"])
+	expect(s.getLabel(b2)).toBe("Bravo · two.example")
+})
+
+test("selecting a system limits targets to ones it also probes", () => {
+	const s = state(a1, all, ["c"])
+	expect(ids(s.targetOptions)).toEqual([])
+})
+
+test("selecting a target limits systems to ones that also probe it", () => {
+	const s = state(a1, all, [], ["a2"])
+	expect(s.systemOptions).toEqual(["b"])
+})
+
+test("the pickers can't produce conflicting selections, but targets win if they conflict", () => {
+	const s = state(a1, all, ["c"], ["a2"])
+	expect(s.selectedSystemIds.size).toBe(0)
+	expect([...s.selectedTargetIds]).toEqual(["a2"])
+	expect(ids(s.compareMonitors)).toEqual(["a1", "a2"])
+})
+
+test("selected systems that are no longer options are ignored", () => {
+	const s = state(a1, all, ["gone"])
+	expect(s.selectedSystemIds.size).toBe(0)
+	expect(ids(s.compareMonitors)).toEqual(["a1"])
+})
+
+test("DNS lookups of the same name against different servers get the server in their label", () => {
+	const d1 = mon("d1", "a", "example.com", { protocol: "dns", server: "1.1.1.1" })
+	const d2 = mon("d2", "a", "example.com", { protocol: "dns", server: "8.8.8.8" })
+	const s = state(d1, [d1, d2], [], ["d2"])
+	expect(s.getLabel(d1)).toBe("example.com (1.1.1.1)")
+	expect(s.getLabel(d2)).toBe("example.com (8.8.8.8)")
+})
+
+test("uses separate local and fetched monitor lists on single-system pages", () => {
+	const s = getMonitorCompareState({
+		monitor: a1,
+		localMonitors: [a1, a2],
+		otherMonitors: [b1, b2],
+		selectedSystemIds: new Set(["b"]),
+		selectedTargetIds: new Set(["a2"]),
+		getSystemName: (id) => systemNames[id],
+	})
+	expect(ids(s.compareMonitors)).toEqual(["a1", "a2", "b1", "b2"])
+})
+
+test("duplicate labels without a server are left alone", () => {
+	const b1Dupe = mon("b1Dupe", "b", "one.example")
+	const s = state(a1, [...all, b1Dupe], ["b"])
+	expect(ids(s.compareMonitors)).toEqual(["a1", "b1", "b1Dupe"])
+	expect(s.getLabel(b1)).toBe("Bravo")
+	expect(s.getLabel(b1Dupe)).toBe("Bravo")
+})
+
+// Every system probes every target, so selections multiply into targets × systems lines.
+function grid(systemCount: number, targetCount: number) {
+	const monitors: NetworkMonitorRecord[] = []
+	for (let s = 0; s < systemCount; s++) {
+		for (let t = 0; t < targetCount; t++) monitors.push(mon(`s${s}t${t}`, `s${s}`, `t${t}.example`))
+	}
+	return monitors
+}
+
+test("selecting everything is trimmed to the line limit, keeping targets and earlier picks", () => {
+	const monitors = grid(30, 5)
+	const allSystems = Array.from({ length: 29 }, (_, i) => `s${i + 1}`)
+	const allTargets = ["s0t1", "s0t2", "s0t3", "s0t4"]
+	const s = state(monitors[0], monitors, allSystems, allTargets)
+	expect([...s.selectedTargetIds]).toEqual(allTargets)
+	// 5 targets fit on 4 systems (the opened one plus 3) within the limit
+	expect([...s.selectedSystemIds]).toEqual(["s1", "s2", "s3"])
+	expect(s.compareMonitors.length).toBe(20)
+	expect(s.canAddSystem).toBe(false)
+})
+
+test("more can be added only while the next pick fits the line limit", () => {
+	const monitors = grid(12, 3)
+	const s = state(monitors[0], monitors, ["s1", "s2", "s3", "s4", "s5", "s6"])
+	// 1 target × 7 systems; another target makes 14, another system makes 8
+	expect(s.canAddTarget).toBe(true)
+	expect(s.canAddSystem).toBe(true)
+	const full = state(monitors[0], monitors, ["s1", "s2", "s3", "s4", "s5", "s6", "s7"], ["s0t1", "s0t2"])
+	// 3 targets × 8 systems = 24
+	expect(full.compareMonitors.length).toBe(MAX_COMPARE_MONITORS)
+	expect(full.canAddTarget).toBe(false)
+	expect(full.canAddSystem).toBe(false)
+})

internal/site/src/lib/monitor-compare.ts

@@ -0,0 +1,128 @@
+import type { NetworkMonitorRecord } from "@/types"
+
+// Kept free of UI and store imports so it can be unit tested with bun.
+
+/** Most lines a comparison charts, to keep it readable and the stats request's filter short. */
+export const MAX_COMPARE_MONITORS = 24
+
+type MonitorTarget = Pick<NetworkMonitorRecord, "target" | "protocol" | "port">
+
+export function getMonitorTarget(monitor: MonitorTarget) {
+	if (monitor.protocol !== "tcp") return monitor.target
+	const host = monitor.target.includes(":") && !monitor.target.startsWith("[") ? `[${monitor.target}]` : monitor.target
+	return `${host}:${monitor.port}`
+}
+
+/** Identifies what a monitor probes, regardless of which system probes it. */
+export function getMonitorIdentityKey({
+	protocol,
+	target,
+	port,
+	server,
+}: Pick<NetworkMonitorRecord, "protocol" | "target" | "port" | "server">) {
+	return JSON.stringify([protocol, target, port, server])
+}
+
+interface MonitorCompareInput {
+	/** The monitor whose sheet is open; always charted. */
+	monitor: NetworkMonitorRecord
+	/** Monitors that may include other targets on the opened monitor's system. */
+	localMonitors: NetworkMonitorRecord[]
+	/** Monitors that may include other systems' monitors. */
+	otherMonitors: NetworkMonitorRecord[]
+	selectedSystemIds: Set<string>
+	selectedTargetIds: Set<string>
+	getSystemName: (systemId: string) => string
+}
+
+/**
+ * Works out what the monitor sheet can compare and what it charts. Comparisons are limited to the opened
+ * monitor's protocol, since response time and loss mean different things per protocol.
+ */
+export function getMonitorCompareState({
+	monitor,
+	localMonitors,
+	otherMonitors,
+	selectedSystemIds,
+	selectedTargetIds,
+	getSystemName,
+}: MonitorCompareInput) {
+	const sameProtocol = (m: NetworkMonitorRecord) => m.protocol === monitor.protocol
+	const systemTargets = localMonitors.filter(
+		(m) => m.system === monitor.system && m.id !== monitor.id && sameProtocol(m)
+	)
+	const systemMonitors = otherMonitors.filter((m) => m.system !== monitor.system && sameProtocol(m))
+
+	const keysBySystem = new Map<string, Set<string>>()
+	for (const m of systemMonitors) {
+		const keys = keysBySystem.get(m.system) ?? new Set<string>()
+		keys.add(getMonitorIdentityKey(m))
+		keysBySystem.set(m.system, keys)
+	}
+
+	// Each picker only offers what fits the other's selection, so every pick charts a line per system:
+	// systems must probe the opened target and every selected target, and targets must be probed by
+	// every selected system. Selections outside the options (e.g. a monitor deleted while the sheet is
+	// open) are ignored, and ones past MAX_COMPARE_MONITORS lines are dropped, keeping targets over
+	// systems and earlier picks over later ones.
+	const systemTargetsById = new Map(systemTargets.map((m) => [m.id, m]))
+	const pickedTargets = [...selectedTargetIds]
+		.flatMap((id) => systemTargetsById.get(id) ?? [])
+		.slice(0, MAX_COMPARE_MONITORS - 1)
+	const targetMonitors = [monitor, ...pickedTargets]
+	const requiredKeys = targetMonitors.map(getMonitorIdentityKey)
+	const systemOptions = [...keysBySystem]
+		.filter(([, keys]) => requiredKeys.every((key) => keys.has(key)))
+		.map(([id]) => id)
+	const systemOptionSet = new Set(systemOptions)
+	const systemIds = [...selectedSystemIds]
+		.filter((id) => systemOptionSet.has(id))
+		.slice(0, Math.floor(MAX_COMPARE_MONITORS / targetMonitors.length) - 1)
+
+	const targetOptions = systemTargets.filter((m) => {
+		const key = getMonitorIdentityKey(m)
+		return systemIds.every((id) => keysBySystem.get(id)?.has(key))
+	})
+	const targetIds = new Set(pickedTargets.map((m) => m.id))
+
+	// Every charted target is also charted for each selected system.
+	const targetKeys = new Set(targetMonitors.map(getMonitorIdentityKey))
+	const selectedSystems = new Set(systemIds)
+	const compareMonitors = [
+		...targetMonitors,
+		...systemMonitors.filter((m) => selectedSystems.has(m.system) && targetKeys.has(getMonitorIdentityKey(m))),
+	]
+
+	// Label series by whatever differs between them: system, target, or both.
+	const multiSystem = systemIds.length > 0
+	const multiTarget = targetIds.size > 0
+	const labels = new Map<string, string>()
+	const counts = new Map<string, number>()
+	for (const m of compareMonitors) {
+		const systemName = getSystemName(m.system)
+		const target = getMonitorTarget(m)
+		const label = multiSystem && multiTarget ? `${systemName} · ${target}` : multiTarget ? target : systemName
+		labels.set(m.id, label)
+		counts.set(label, (counts.get(label) ?? 0) + 1)
+	}
+	// DNS lookups of the same name against different servers would otherwise share a label.
+	for (const m of compareMonitors) {
+		const label = labels.get(m.id) as string
+		if ((counts.get(label) ?? 0) > 1 && m.server) labels.set(m.id, `${label} (${m.server})`)
+	}
+
+	return {
+		/** Systems that can be selected to compare against. */
+		systemOptions,
+		selectedSystemIds: new Set(systemIds),
+		/** Other targets on the opened monitor's system that can be selected. */
+		targetOptions,
+		selectedTargetIds: targetIds,
+		/** Whether one more target or system still fits within MAX_COMPARE_MONITORS lines. */
+		canAddTarget: (targetMonitors.length + 1) * (systemIds.length + 1) <= MAX_COMPARE_MONITORS,
+		canAddSystem: targetMonitors.length * (systemIds.length + 2) <= MAX_COMPARE_MONITORS,
+		/** Monitors to chart, starting with the opened one. */
+		compareMonitors,
+		getLabel: (m: NetworkMonitorRecord) => labels.get(m.id) ?? getMonitorTarget(m),
+	}
+}

internal/site/src/lib/network-monitor-utils.ts

@@ -98,11 +98,7 @@ export function mergeMonitorStats(rawRecords: RawMonitorStatsRecord[], bucketMs
 		.map(([created, stats]) => ({ created, stats }))
 }
 
-export function getMonitorTarget(monitor: Pick<NetworkMonitorRecord, "target" | "protocol" | "port">) {
-	if (monitor.protocol !== "tcp") return monitor.target
-	const host = monitor.target.includes(":") && !monitor.target.startsWith("[") ? `[${monitor.target}]` : monitor.target
-	return `${host}:${monitor.port}`
-}
+export { getMonitorTarget } from "./monitor-compare"
 
 /** Whole days until the certificate expires; negative once expired. */
 export function getCertDaysLeft(cert: Pick<MonitorCertInfo, "expires">, now = Date.now()) {

internal/site/src/lib/use-network-monitors.ts

@@ -323,38 +323,38 @@ export function useNetworkMonitorStats(props: UseNetworkMonitorStatsProps) {
 	}, [monitorStats, cacheKey, interval, chartTime])
 }
 
+/** Only what comparison charts and labels need. */
+const COMPARE_MONITOR_FIELDS = "id,system,target,protocol,port,server,interval,resAvg1h"
+
 /**
- * Monitors on other systems that probe the same target (same protocol, target, port, and DNS server).
- * Fetched once per open so it also works in single-system tables, which only hold one system's monitors.
+ * Monitors of one protocol on all systems except the given one, to compare against (#2385).
+ * Fetched per open so it also works in single-system tables, which only hold one system's monitors.
  */
-export function useMatchingMonitors(monitor: NetworkMonitorRecord, enabled = true) {
-	const [matches, setMatches] = useState<NetworkMonitorRecord[]>([])
-	const { id, system, protocol, target, port, server } = monitor
+export function useCompareMonitors(system: string, protocol: string, enabled = true) {
+	const key = `${system}:${protocol}`
+	const [result, setResult] = useState<{ key: string; monitors: NetworkMonitorRecord[] }>({ key, monitors: [] })
 
 	useEffect(() => {
-		setMatches([])
 		if (!enabled) return
 		let cancelled = false
 		pb.collection<NetworkMonitorRecord>("network_monitors")
 			.getFullList({
-				fields: NETWORK_MONITOR_FIELDS,
-				filter: pb.filter(
-					"id!={:id} && system!={:system} && protocol={:protocol} && target={:target} && port={:port} && server={:server}",
-					{ id, system, protocol, target, port, server }
-				),
+				fields: COMPARE_MONITOR_FIELDS,
+				filter: pb.filter("system!={:system} && protocol={:protocol}", { system, protocol }),
 			})
-			.then((records) => {
-				if (!cancelled) setMatches(records)
+			.then((monitors) => {
+				if (!cancelled) setResult({ key: `${system}:${protocol}`, monitors })
 			})
 			.catch((error) => {
-				if (!cancelled) console.error("Failed to fetch matching monitors:", error)
+				if (!cancelled) console.error("Failed to fetch compare monitors:", error)
 			})
 		return () => {
 			cancelled = true
 		}
-	}, [id, system, protocol, target, port, server, enabled])
+	}, [system, protocol, enabled])
 
-	return matches
+	// Keep showing the last result while reopening refreshes it, but never another monitor's.
+	return result.key === key ? result.monitors : []
 }
 
 async function fetchMonitors(system?: string) {