From aca5d24a24b6c7d7416f514cb23c6cad2af8e77d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 21:39:17 -0700 Subject: [PATCH 01/10] fix(usage): correct chart clipping, expand truncated rows, add source mix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two rendering defects on the organization usage panel, plus the follow-on cleanup they surfaced. The y-axis maximum was clipped at the container's left edge. The chart family used a fixed 26px left gutter, which leaves 18px of drawable width once the label gap is taken out — four narrow glyphs — so every tick from `7.3k` up lost its first character. Both charts now derive the gutter from the labels they are about to draw, through one shared `resolveChartPadding` so a bar and a line chart still line up when stacked. Hovering near the foot of the plot raised a vertical scrollbar over the chart. The scroll container sets `overflow-x`, which promotes `overflow-y` to `auto`, and the tooltip's vertical clamp was a fixed inset that ignored the box's real height. The clamp now measures the tooltip, and the container pins `overflow-y`. Also on the panel: - The axis rules were invisible. `hsl(var(--border))` is unparseable — the token is a hex — so the presentation attribute was dropped and SVG's initial `stroke: none` applied. - GLM rendered without a mark: the settings provider-icon map held 11 of the registry's 24 providers. Completed, with a test that fails when the two drift. The server's parallel label map was the same 11-entry duplicate and now reads the registry directly. - `Other (N more)` opens the tail in place, raising the row limit to the API's ceiling. Fixed the drill-down name lookup this exposed, which was pinned to the top ten and so refused to open for any row below it. - A radar chart of the source mix sits beside the Sources list. The rows rank the sources; they cannot show whether spend is concentrated or spread. - "Open logs" pointed at `/workspace//logs` for a workspace picked from an organization-wide list. Organization admin is not workspace membership, so for any workspace the admin had not joined it was a one-way trip to an access denial. It now opens the organization's audit feed scoped to that workspace, which required threading the workspace filter the query builder already supported through the internal contract, route, hook, and URL state. - BYOK is withheld from the tab strip until the ledger carries BYOK rows. - The chip number field suppresses the native stepper, which painted browser chrome inside a flat chip surface. The component owns it, not its callers. - `ChartColumn` was a uniform 0.86 downscale of its source art, so it rendered ~2px small beside every other icon in the settings nav. --- apps/sim/app/api/audit-logs/route.ts | 1 + .../settings/[section]/settings.tsx | 6 +- .../settings/usage/events/loading.tsx | 2 +- apps/sim/components/charts/bar-chart.tsx | 104 +++--- .../components/charts/chart-geometry.test.ts | 33 ++ apps/sim/components/charts/chart-geometry.ts | 42 ++- .../components/charts/chart-tooltip.test.ts | 100 ++++++ apps/sim/components/charts/chart-tooltip.tsx | 42 ++- apps/sim/components/charts/index.ts | 1 + apps/sim/components/charts/line-chart.tsx | 223 ++++++------ apps/sim/components/charts/radar-chart.tsx | 284 +++++++++++++++ apps/sim/components/charts/use-chart-theme.ts | 72 ++-- .../ee/audit-logs/components/audit-logs.tsx | 85 ++++- .../ee/audit-logs/hooks/audit-logs.test.tsx | 38 +- apps/sim/ee/audit-logs/hooks/audit-logs.ts | 19 + apps/sim/ee/audit-logs/search-params.ts | 19 +- .../components/usage-consumers.test.ts | 19 + .../components/usage-consumers.tsx | 131 +++++-- .../components/usage-events-view.tsx | 14 +- .../components/usage-monitoring.tsx | 329 ++++++++++++------ .../components/usage-source-mix.tsx | 80 +++++ .../components/usage-summary.tsx | 52 ++- apps/sim/ee/organization-usage/constants.ts | 19 +- .../hooks/use-usage-window.test.ts | 45 +++ .../hooks/use-usage-window.ts | 26 +- .../ee/organization-usage/search-params.ts | 20 +- apps/sim/hooks/queries/organization-usage.ts | 42 ++- apps/sim/lib/api/contracts/audit-logs.ts | 9 +- .../get-organization-usage-breakdown.ts | 25 +- .../components/chip-input/chip-input.test.tsx | 21 +- .../src/components/chip-input/chip-input.tsx | 21 +- packages/emcn/src/icons/chart-column.tsx | 6 +- 32 files changed, 1501 insertions(+), 429 deletions(-) create mode 100644 apps/sim/components/charts/chart-tooltip.test.ts create mode 100644 apps/sim/components/charts/radar-chart.tsx create mode 100644 apps/sim/ee/organization-usage/components/usage-consumers.test.ts create mode 100644 apps/sim/ee/organization-usage/components/usage-source-mix.tsx create mode 100644 apps/sim/ee/organization-usage/hooks/use-usage-window.test.ts diff --git a/apps/sim/app/api/audit-logs/route.ts b/apps/sim/app/api/audit-logs/route.ts index f2cc96a2b3f..f63e99575c2 100644 --- a/apps/sim/app/api/audit-logs/route.ts +++ b/apps/sim/app/api/audit-logs/route.ts @@ -27,6 +27,7 @@ export const GET = defineInternalJsonRoute({ action: query.action, resourceType: query.resourceType, actorId: query.actorId, + workspaceId: query.workspaceId, startDate: query.startDate, endDate: query.endDate, }, diff --git a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx index b565424b422..8295fa0b230 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/[section]/settings.tsx @@ -176,7 +176,11 @@ export function SettingsPage({ section }: SettingsPageProps) { )} {effectiveSection === 'usage' && organizationId && ( - + )} {effectiveSection === 'apikeys' && } {isBillingEnabled && effectiveSection === 'billing' && ( diff --git a/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx b/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx index aa745a1024e..1e9fd7ecb72 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/usage/events/loading.tsx @@ -16,7 +16,7 @@ export default function UsageEventsLoading() { onSelect: () => router.push(`/workspace/${workspaceId}/settings/usage`), }} title='Usage events' - description='Every credit-consuming event behind your usage.' + description="Every credit-consuming event across your organization's workspaces." /> ) } diff --git a/apps/sim/components/charts/bar-chart.tsx b/apps/sim/components/charts/bar-chart.tsx index 8a40747c18b..c216c64490f 100644 --- a/apps/sim/components/charts/bar-chart.tsx +++ b/apps/sim/components/charts/bar-chart.tsx @@ -8,19 +8,21 @@ import { formatChartTimestamp, } from '@/components/charts/chart-format' import { + CHART_AXIS_LABEL_GAP, CHART_DEFAULT_HEIGHT, CHART_GRID_FRACTIONS, - CHART_PADDING, CHART_TICK_FILL, CHART_TICK_FONT_SIZE, chartPlotBand, formatTimeTick, + resolveChartPadding, resolveSpanMs, resolveTimeTickIndices, } from '@/components/charts/chart-geometry' import { ChartTooltip, ChartTooltipRow, + estimateTooltipHeight, estimateTooltipWidth, positionChartTooltip, } from '@/components/charts/chart-tooltip' @@ -47,6 +49,17 @@ interface BarChartProps { highlightIndex?: number } +/** Tick and tooltip text for a bucket's value, in the caller's unit. */ +function formatBarValue(value: number | undefined, unit: string | undefined): string { + if (typeof value !== 'number' || !Number.isFinite(value)) return '—' + const suffix = (unit ?? '').toLowerCase() + if (suffix.includes('%')) return `${value.toFixed(1)}%` + if (suffix === 'latency') return formatChartLatency(value) + if (suffix.includes('ms')) return `${Math.round(value)}ms` + if (suffix === 'credits') return formatChartCompactNumber(value) + return `${Math.round(value)}${unit ?? ''}` +} + /** * Discrete time buckets as bars. * @@ -71,16 +84,11 @@ function BarChartComponent({ const uniqueId = useId().replace(/:/g, '') const [containerRef, containerWidth] = useChartWidth() const width = containerWidth ?? 0 - const padding = CHART_PADDING - const chartWidth = width - padding.left - padding.right - const chartHeight = height - padding.top - padding.bottom const { yMin, yMax } = chartPlotBand(height) const isDark = useIsDarkTheme() - const [hoverIndex, setHoverIndex] = useState(null) const [hoverPos, setHoverPos] = useState<{ x: number; y: number } | null>(null) - const colorTokens = useMemo(() => ({ base: color }), [color]) - const resolvedColors = useResolvedChartColors(colorTokens) + const resolvedColors = useResolvedChartColors({ base: color }) const resolvedColor = resolvedColors.base || color const hasExternalWrapper = !label @@ -100,10 +108,27 @@ function BarChartComponent({ return peak <= 0 ? 1 : peak * 1.1 }, [data]) + const padding = resolveChartPadding([formatBarValue(maxValue, unit), '0']) + const chartWidth = width - padding.left - padding.right + const chartHeight = height - padding.top - padding.bottom + /** Slot geometry: every bucket owns an equal slice, with the bar centred in it. */ const slot = data.length > 0 ? Math.max(1, chartWidth) / data.length : 0 const barWidth = Math.max(1, Math.min(24, slot * 0.7)) + /** + * The hovered bucket, derived from the stored cursor rather than stored beside it. + * + * Bars own a slot, so this is which slot the cursor is in — not the nearest sample, + * which is how a line chart resolves it. Deriving it means a resize mid-hover cannot + * leave an index that disagrees with the slot geometry it was measured against, and + * a mouse move writes one state instead of two. + */ + const hoverIndex = + hoverPos === null || data.length === 0 || slot <= 0 + ? null + : Math.max(0, Math.min(data.length - 1, Math.floor((hoverPos.x - padding.left) / slot))) + const bars = useMemo( () => data.map((point, index) => { @@ -127,21 +152,14 @@ function BarChartComponent({ [data, slot, barWidth, maxValue, chartHeight, height, padding.left, padding.top, yMin, yMax] ) - const formatValue = (value?: number) => { - if (typeof value !== 'number' || !Number.isFinite(value)) return '—' - const suffix = (unit ?? '').toLowerCase() - if (suffix.includes('%')) return `${value.toFixed(1)}%` - if (suffix === 'latency') return formatChartLatency(value) - if (suffix.includes('ms')) return `${Math.round(value)}ms` - if (suffix === 'credits') return formatChartCompactNumber(value) - return `${Math.round(value)}${unit ?? ''}` - } - if (containerWidth === null) { return (
) @@ -156,7 +174,7 @@ function BarChartComponent({ ref={containerRef} className={cn( 'flex w-full items-center justify-center', - !hasExternalWrapper && 'rounded-lg border bg-card p-4' + !hasExternalWrapper && 'rounded-lg border bg-[var(--surface-1)] p-4' )} /* Height only. `width` is floored at CHART_MIN_WIDTH for the plot geometry, @@ -185,8 +203,14 @@ function BarChartComponent({ contradicted the constant's own note that the chart "scrolls rather than compresses". At or above the floor there is no overflow and nothing changes. */ - 'w-full overflow-x-auto', - !hasExternalWrapper && 'rounded-[11px] border bg-card p-4 shadow-sm' + /* + `overflow-y-hidden` is not redundant with `overflow-x-auto`: a computed + `overflow-x` other than `visible` promotes `overflow-y: visible` to `auto`, + so the tooltip's shadow reaching the foot of the box raised a vertical + scrollbar over the chart whenever the cursor neared the axis. + */ + 'w-full overflow-x-auto overflow-y-hidden', + !hasExternalWrapper && 'rounded-lg border bg-[var(--surface-1)] p-4 shadow-card' )} > {!hasExternalWrapper && ( @@ -202,20 +226,9 @@ function BarChartComponent({ onMouseMove={(e) => { if (bars.length === 0 || slot <= 0) return const rect = (e.currentTarget as SVGSVGElement).getBoundingClientRect() - const x = e.clientX - rect.left - // Bars own a slot, so the hovered bucket is which slot the cursor is in — - // not the nearest sample, which is how a line chart resolves it. - const index = Math.max( - 0, - Math.min(data.length - 1, Math.floor((x - padding.left) / slot)) - ) - setHoverIndex(index) - setHoverPos({ x, y: e.clientY - rect.top }) - }} - onMouseLeave={() => { - setHoverIndex(null) - setHoverPos(null) + setHoverPos({ x: e.clientX - rect.left, y: e.clientY - rect.top }) }} + onMouseLeave={() => setHoverPos(null)} > @@ -229,7 +242,7 @@ function BarChartComponent({ y1={padding.top} x2={padding.left} y2={height - padding.bottom} - stroke='hsl(var(--border))' + stroke='var(--border)' strokeWidth='1' /> @@ -240,7 +253,7 @@ function BarChartComponent({ y1={padding.top + chartHeight * fraction} x2={width - padding.right} y2={padding.top + chartHeight * fraction} - stroke='hsl(var(--muted))' + stroke='var(--border)' strokeOpacity='0.35' strokeWidth='1' /> @@ -313,7 +326,7 @@ function BarChartComponent({ })} {/* Same formatter the tooltip uses, or the axis and the hover disagree about what the numbers mean on any non-`credits` unit. */} - {formatValue(maxValue)} + {formatBarValue(maxValue, unit)} @@ -347,20 +360,19 @@ function BarChartComponent({ bars[hoverIndex] && (() => { const bar = bars[hoverIndex] - const value = formatValue(bar.point.value) + const value = formatBarValue(bar.point.value, unit) + const date = formatChartTimestamp(bar.point.timestamp) const { left, top } = positionChartTooltip({ anchorX: hoverPos?.x ?? bar.x, anchorY: hoverPos?.y ?? bar.y, width, height, tooltipMaxWidth: estimateTooltipWidth(value.length), + tooltipHeight: estimateTooltipHeight(1, Boolean(date)), + padding, }) return ( - + ) diff --git a/apps/sim/components/charts/chart-geometry.test.ts b/apps/sim/components/charts/chart-geometry.test.ts index 82c2694abbe..10820754e0d 100644 --- a/apps/sim/components/charts/chart-geometry.test.ts +++ b/apps/sim/components/charts/chart-geometry.test.ts @@ -3,9 +3,12 @@ */ import { describe, expect, it } from 'vitest' import { + CHART_AXIS_LABEL_GAP, CHART_PADDING, chartPlotBand, + estimateAxisLabelWidth, formatTimeTick, + resolveChartPadding, resolveSpanMs, resolveTimeTickIndices, } from '@/components/charts/chart-geometry' @@ -78,3 +81,33 @@ describe('chartPlotBand', () => { expect(chartPlotBand(240).yMax).toBeGreaterThan(chartPlotBand(166).yMax) }) }) + +describe('resolveChartPadding', () => { + /** + * The bug this exists for: a fixed 26px gutter left 18px of drawable width once + * `CHART_AXIS_LABEL_GAP` was taken out, and a right-anchored `7.3k` at 9px is wider + * than that — so its first glyph was cut off at the container's left edge. + */ + it('widens the gutter until the longest label fits beside the axis', () => { + const { left } = resolveChartPadding(['7.3k', '0']) + expect(left).toBeGreaterThanOrEqual(estimateAxisLabelWidth('7.3k') + CHART_AXIS_LABEL_GAP) + }) + + it('never narrows below the shared padding, so short labels stay aligned', () => { + expect(resolveChartPadding(['0', '0']).left).toBe(CHART_PADDING.left) + expect(resolveChartPadding([]).left).toBe(CHART_PADDING.left) + }) + + it('leaves the other three sides on the shared constant', () => { + const padding = resolveChartPadding(['123.4m']) + expect(padding.top).toBe(CHART_PADDING.top) + expect(padding.right).toBe(CHART_PADDING.right) + expect(padding.bottom).toBe(CHART_PADDING.bottom) + }) + + it('grows monotonically with label length', () => { + const short = resolveChartPadding(['1.2k']).left + const long = resolveChartPadding(['123456.7m']).left + expect(long).toBeGreaterThan(short) + }) +}) diff --git a/apps/sim/components/charts/chart-geometry.ts b/apps/sim/components/charts/chart-geometry.ts index e85a010d251..1bdfff37b30 100644 --- a/apps/sim/components/charts/chart-geometry.ts +++ b/apps/sim/components/charts/chart-geometry.ts @@ -10,6 +10,8 @@ export const CHART_PADDING = { top: 16, right: 28, bottom: 26, left: 26 } as const +export type ChartPadding = { top: number; right: number; bottom: number; left: number } + /** Matches the loader placeholders callers size themselves against. */ export const CHART_DEFAULT_HEIGHT = 166 @@ -17,9 +19,47 @@ export const CHART_DEFAULT_HEIGHT = 166 export const CHART_MIN_WIDTH = 280 export const CHART_TICK_FILL = 'var(--text-tertiary)' -export const CHART_TICK_FONT_SIZE = '9' +export const CHART_TICK_FONT_SIZE = 9 export const CHART_GRID_FRACTIONS = [0.25, 0.5, 0.75] as const +/** Gap between a y-axis tick label's right edge and the axis rule. */ +export const CHART_AXIS_LABEL_GAP = 8 + +/** + * Rendered width of a right-anchored y-axis tick label. + * + * SVG `` cannot be measured before layout, so the gutter that has to hold it + * is estimated from the glyphs instead. The ratios are for the UI sans at + * {@link CHART_TICK_FONT_SIZE}: digits and letters sit near 0.58em, punctuation and + * spaces near 0.3em. Deliberately generous — an over-wide gutter costs a couple of + * plot pixels, an under-wide one clips the label against the container's edge. + */ +export function estimateAxisLabelWidth(text: string): number { + let width = 0 + for (const character of text) { + width += /[.,:\s]/.test(character) ? 0.3 : 0.58 + } + return width * CHART_TICK_FONT_SIZE +} + +/** + * {@link CHART_PADDING} with a left gutter wide enough for the chart's own y-axis + * labels. + * + * The fixed 26px gutter left 18px of drawable width once the label gap is taken out, + * which fits four narrow glyphs — so any tick past `7.3k` was cut off at the left edge + * of the container. Both charts resolve their gutter through this one function from + * the labels they are about to draw, so a bar and a line chart showing comparable + * magnitudes still line up when stacked in one card, and neither can clip. + */ +export function resolveChartPadding(yAxisLabels: readonly string[]): ChartPadding { + const widest = yAxisLabels.reduce((max, label) => Math.max(max, estimateAxisLabelWidth(label)), 0) + return { + ...CHART_PADDING, + left: Math.max(CHART_PADDING.left, Math.ceil(widest) + CHART_AXIS_LABEL_GAP), + } +} + /** Vertical clamp for plotted geometry, keeping strokes off the axis rules. */ export function chartPlotBand(height: number): { yMin: number; yMax: number } { const chartHeight = height - CHART_PADDING.top - CHART_PADDING.bottom diff --git a/apps/sim/components/charts/chart-tooltip.test.ts b/apps/sim/components/charts/chart-tooltip.test.ts new file mode 100644 index 00000000000..36bc19467e1 --- /dev/null +++ b/apps/sim/components/charts/chart-tooltip.test.ts @@ -0,0 +1,100 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { CHART_PADDING, resolveChartPadding } from '@/components/charts/chart-geometry' +import { + estimateTooltipHeight, + estimateTooltipWidth, + positionChartTooltip, +} from '@/components/charts/chart-tooltip' + +const WIDTH = 800 +const HEIGHT = 166 + +function place(anchorY: number, rows = 1, hasDate = true) { + const tooltipHeight = estimateTooltipHeight(rows, hasDate) + const position = positionChartTooltip({ + anchorX: 400, + anchorY, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: estimateTooltipWidth(12), + tooltipHeight, + }) + return { ...position, tooltipHeight } +} + +describe('positionChartTooltip', () => { + /** + * The bug this exists for: the vertical clamp was a fixed inset that ignored the + * box's real height, so near the foot of the plot the tooltip hung a pixel or two + * past the bottom. The scroll container sets `overflow-x`, which promotes + * `overflow-y` to `auto`, and those pixels raised a vertical scrollbar over the + * chart the moment the cursor approached the axis. + */ + it('keeps the whole box inside the chart when the cursor is at the very bottom', () => { + const { top, tooltipHeight } = place(HEIGHT) + expect(top + tooltipHeight).toBeLessThanOrEqual(HEIGHT) + }) + + it('holds for a taller multi-row tooltip, which overflows soonest', () => { + const { top, tooltipHeight } = place(HEIGHT, 5) + expect(top + tooltipHeight).toBeLessThanOrEqual(HEIGHT) + expect(top).toBeGreaterThanOrEqual(0) + }) + + it('never places the box above the chart when the cursor is at the top', () => { + expect(place(0).top).toBeGreaterThanOrEqual(0) + }) + + it('prefers the right of the cursor and flips left near the right edge', () => { + const boxWidth = estimateTooltipWidth(12) + const right = positionChartTooltip({ + anchorX: 100, + anchorY: 80, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: boxWidth, + tooltipHeight: estimateTooltipHeight(1, true), + }) + expect(right.left).toBeGreaterThan(100) + + const flipped = positionChartTooltip({ + anchorX: WIDTH - CHART_PADDING.right, + anchorY: 80, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: boxWidth, + tooltipHeight: estimateTooltipHeight(1, true), + }) + expect(flipped.left + boxWidth).toBeLessThanOrEqual(WIDTH - CHART_PADDING.right) + }) + + /** A chart with wide axis labels has a wider gutter, and the clamp must follow it. */ + it('clamps the left edge to the resolved gutter, not the shared constant', () => { + const padding = resolveChartPadding(['123456.7m']) + const { left } = positionChartTooltip({ + anchorX: 0, + anchorY: 80, + width: WIDTH, + height: HEIGHT, + tooltipMaxWidth: estimateTooltipWidth(12), + tooltipHeight: estimateTooltipHeight(1, true), + padding, + }) + expect(left).toBeGreaterThanOrEqual(padding.left) + expect(padding.left).toBeGreaterThan(CHART_PADDING.left) + }) +}) + +describe('estimateTooltipHeight', () => { + it('grows with each row and with the date header', () => { + expect(estimateTooltipHeight(2, true)).toBeGreaterThan(estimateTooltipHeight(1, true)) + expect(estimateTooltipHeight(1, true)).toBeGreaterThan(estimateTooltipHeight(1, false)) + }) + + it('reserves a row even when told there are none', () => { + expect(estimateTooltipHeight(0, false)).toBe(estimateTooltipHeight(1, false)) + }) +}) diff --git a/apps/sim/components/charts/chart-tooltip.tsx b/apps/sim/components/charts/chart-tooltip.tsx index 6a49032394a..bf92197110c 100644 --- a/apps/sim/components/charts/chart-tooltip.tsx +++ b/apps/sim/components/charts/chart-tooltip.tsx @@ -1,7 +1,7 @@ 'use client' import type { ReactNode } from 'react' -import { CHART_PADDING } from '@/components/charts/chart-geometry' +import { CHART_PADDING, type ChartPadding } from '@/components/charts/chart-geometry' /** * The chart family's hover surface. Defined once so a sibling chart cannot ship a @@ -9,7 +9,7 @@ import { CHART_PADDING } from '@/components/charts/chart-geometry' * between the line chart and the status bar. */ export const CHART_TOOLTIP_CLASSES = - 'pointer-events-none absolute rounded-lg border border-[var(--border-1)] bg-[var(--surface-1)] px-2 py-1.5 text-xs shadow-lg' + 'pointer-events-none absolute rounded-lg border border-[var(--border)] bg-[var(--surface-1)] px-2 py-1.5 text-xs shadow-overlay' interface PositionChartTooltipArgs { anchorX: number @@ -17,11 +17,19 @@ interface PositionChartTooltipArgs { width: number height: number tooltipMaxWidth: number + tooltipHeight: number + /** The chart's resolved padding, whose left gutter varies with its axis labels. */ + padding?: ChartPadding } /** * Places the tooltip beside the cursor, preferring the right and flipping left when - * it would overflow, then clamping into the plot band so it never escapes the card. + * it would overflow, then clamping it wholly inside the chart box. + * + * The vertical clamp is against the tooltip's own height rather than a fixed inset. + * A fixed one let the box hang a pixel or two past the bottom near the foot of the + * plot, and because the scroll container's `overflow-x` forces `overflow-y` to `auto`, + * those pixels raised a vertical scrollbar the moment the cursor approached the axis. */ export function positionChartTooltip({ anchorX, @@ -29,20 +37,19 @@ export function positionChartTooltip({ width, height, tooltipMaxWidth, + tooltipHeight, + padding = CHART_PADDING, }: PositionChartTooltipArgs): { left: number; top: number } { const margin = 10 - const rightEdge = width - CHART_PADDING.right + const rightEdge = width - padding.right const preferRight = anchorX + margin + tooltipMaxWidth <= rightEdge const left = preferRight - ? Math.max(CHART_PADDING.left, Math.min(anchorX + margin, rightEdge - tooltipMaxWidth)) + ? Math.max(padding.left, Math.min(anchorX + margin, rightEdge - tooltipMaxWidth)) : Math.max( - CHART_PADDING.left, + padding.left, Math.min(anchorX - margin - tooltipMaxWidth, rightEdge - tooltipMaxWidth) ) - const top = Math.min( - Math.max(anchorY - 26, CHART_PADDING.top), - height - CHART_PADDING.bottom - 18 - ) + const top = Math.max(0, Math.min(anchorY - 26, height - tooltipHeight)) return { left, top } } @@ -51,6 +58,19 @@ export function estimateTooltipWidth(longestRowLength: number): number { return Math.min(220, Math.max(80, 7 * longestRowLength + 24)) } +/** + * Height of the box {@link ChartTooltip} renders, from its own box model: the border + * and `py-1.5` chrome, the optional date header and its margin, and one line per row. + * Estimated rather than measured because the position is computed in the same render + * that mounts the tooltip — reading a real height would need a second paint, which + * shows up as the tooltip visibly jumping under the cursor. + */ +export function estimateTooltipHeight(rowCount: number, hasDate: boolean): number { + const chrome = 2 + 12 + const dateLine = hasDate ? 14 + 4 : 0 + return chrome + dateLine + Math.max(1, rowCount) * 16 +} + interface ChartTooltipProps { left: number top: number @@ -83,7 +103,7 @@ export function ChartTooltipRow({ color, label, value }: ChartTooltipRowProps) { style={{ backgroundColor: color }} /> {label && {label}} - {value} + {value}
) } diff --git a/apps/sim/components/charts/index.ts b/apps/sim/components/charts/index.ts index 138241f0436..1be948503bf 100644 --- a/apps/sim/components/charts/index.ts +++ b/apps/sim/components/charts/index.ts @@ -10,3 +10,4 @@ export { type LineChartMultiSeries, type LineChartPoint, } from '@/components/charts/line-chart' +export { RadarChart, type RadarChartAxis } from '@/components/charts/radar-chart' diff --git a/apps/sim/components/charts/line-chart.tsx b/apps/sim/components/charts/line-chart.tsx index 6cfd8d43979..9ce78acb586 100644 --- a/apps/sim/components/charts/line-chart.tsx +++ b/apps/sim/components/charts/line-chart.tsx @@ -8,19 +8,21 @@ import { formatChartTimestamp, } from '@/components/charts/chart-format' import { + CHART_AXIS_LABEL_GAP, CHART_DEFAULT_HEIGHT, CHART_GRID_FRACTIONS, - CHART_PADDING, CHART_TICK_FILL, CHART_TICK_FONT_SIZE, chartPlotBand, formatTimeTick, + resolveChartPadding, resolveSpanMs, resolveTimeTickIndices, } from '@/components/charts/chart-geometry' import { ChartTooltip, ChartTooltipRow, + estimateTooltipHeight, estimateTooltipWidth, positionChartTooltip, } from '@/components/charts/chart-tooltip' @@ -53,6 +55,38 @@ interface LineChartProps { height?: number } +/** + * Smoothed path through `points`, with every control point clamped into the plot + * band so a curve between two near-axis samples cannot bow over an axis rule. + * + * At module scope because the base line and each extra series need the identical + * curve: the two copies had drifted apart before, and a clamp fixed in one drew a + * different shape from the other. + */ +function buildSmoothPath( + points: ReadonlyArray<{ x: number; y: number }>, + yMin: number, + yMax: number +): string { + if (points.length <= 1) return '' + const tension = 0.2 + let d = `M ${points[0].x} ${points[0].y}` + for (let i = 0; i < points.length - 1; i++) { + const p0 = points[i - 1] || points[i] + const p1 = points[i] + const p2 = points[i + 1] + const p3 = points[i + 2] || points[i + 1] + const cp1x = p1.x + ((p2.x - p0.x) / 6) * tension + let cp1y = p1.y + ((p2.y - p0.y) / 6) * tension + const cp2x = p2.x - ((p3.x - p1.x) / 6) * tension + let cp2y = p2.y - ((p3.y - p1.y) / 6) * tension + cp1y = Math.max(yMin, Math.min(yMax, cp1y)) + cp2y = Math.max(yMin, Math.min(yMax, cp2y)) + d += ` C ${cp1x} ${cp1y}, ${cp2x} ${cp2y}, ${p2.x} ${p2.y}` + } + return d +} + function LineChartComponent({ data, label, @@ -69,23 +103,16 @@ function LineChartComponent({ const uniqueId = useId().replace(/:/g, '') const [containerRef, containerWidth] = useChartWidth() const width = containerWidth ?? 0 - const padding = CHART_PADDING - const chartWidth = width - padding.left - padding.right - const chartHeight = height - padding.top - padding.bottom - const [hoverIndex, setHoverIndex] = useState(null) const isDark = useIsDarkTheme() const [hoverSeriesId, setHoverSeriesId] = useState(null) const [activeSeriesId, setActiveSeriesId] = useState(null) const [hoverPos, setHoverPos] = useState<{ x: number; y: number } | null>(null) - const colorTokens = useMemo(() => { - const tokens: Record = { base: color } - for (const s of series ?? []) { - const id = s.id || s.label || '' - if (id) tokens[id] = s.color - } - return tokens - }, [color, series]) + const colorTokens: Record = { base: color } + for (const s of series ?? []) { + const id = s.id || s.label || '' + if (id) colorTokens[id] = s.color + } const resolvedColors = useResolvedChartColors(colorTokens) const hasExternalWrapper = !label || label === '' @@ -129,6 +156,25 @@ function LineChartComponent({ } }, [allSeries, unit]) + /** + * The two y-axis tick labels, resolved once so the gutter that has to hold them is + * measured from the same strings the axis draws. + */ + const yAxisLabels = useMemo(() => { + const unitSuffix = (unit || '').trim() + const isLatency = unitSuffix.toLowerCase() === 'latency' + const suffix = unitSuffix === '%' && !isLatency ? unitSuffix : '' + const compact = (value: number) => { + if (isLatency) return value === 0 ? '0' : formatChartLatency(value) + return `${formatChartCompactNumber(value)}${suffix}` + } + return [compact(maxValue), compact(minValue)] as const + }, [maxValue, minValue, unit]) + + const padding = resolveChartPadding(yAxisLabels) + const chartWidth = width - padding.left - padding.right + const chartHeight = height - padding.top - padding.bottom + const { yMin, yMax } = chartPlotBand(height) const scaledPoints = useMemo( @@ -143,6 +189,18 @@ function LineChartComponent({ [data, chartWidth, chartHeight, minValue, valueRange, yMin, yMax, padding.left, padding.top] ) + /** + * The hovered sample, derived from the stored cursor rather than stored beside it. + * + * The x written on hover is the same clamped x the index was resolved from, so a + * second piece of state could only ever agree with this — or go stale after a + * resize, once the width it was measured against had changed. + */ + const hoverIndex = + hoverPos === null || scaledPoints.length === 0 + ? null + : Math.round(((hoverPos.x - padding.left) / (chartWidth || 1)) * (scaledPoints.length - 1)) + const scaledSeries = useMemo( () => allSeries.map((s) => { @@ -169,31 +227,11 @@ function LineChartComponent({ ) const getSeriesById = (id?: string | null) => scaledSeries.find((s) => s.id === id) - const visibleSeries = useMemo( - () => (activeSeriesId ? scaledSeries.filter((s) => s.id === activeSeriesId) : scaledSeries), - [activeSeriesId, scaledSeries] - ) + const visibleSeries = activeSeriesId + ? scaledSeries.filter((s) => s.id === activeSeriesId) + : scaledSeries - const pathD = useMemo(() => { - if (scaledPoints.length <= 1) return '' - const p = scaledPoints - const tension = 0.2 - let d = `M ${p[0].x} ${p[0].y}` - for (let i = 0; i < p.length - 1; i++) { - const p0 = p[i - 1] || p[i] - const p1 = p[i] - const p2 = p[i + 1] - const p3 = p[i + 2] || p[i + 1] - const cp1x = p1.x + ((p2.x - p0.x) / 6) * tension - let cp1y = p1.y + ((p2.y - p0.y) / 6) * tension - const cp2x = p2.x - ((p3.x - p1.x) / 6) * tension - let cp2y = p2.y - ((p3.y - p1.y) / 6) * tension - cp1y = Math.max(yMin, Math.min(yMax, cp1y)) - cp2y = Math.max(yMin, Math.min(yMax, cp2y)) - d += ` C ${cp1x} ${cp1y}, ${cp2x} ${cp2y}, ${p2.x} ${p2.y}` - } - return d - }, [scaledPoints, yMin, yMax]) + const pathD = useMemo(() => buildSmoothPath(scaledPoints, yMin, yMax), [scaledPoints, yMin, yMax]) const currentHoverDate = hoverIndex !== null && data[hoverIndex] ? formatChartTimestamp(data[hoverIndex].timestamp) : '' @@ -202,7 +240,10 @@ function LineChartComponent({ return (
) @@ -213,7 +254,7 @@ function LineChartComponent({
{!hasExternalWrapper && ( @@ -259,11 +306,11 @@ function LineChartComponent({ variant='ghost' aria-pressed={activeSeriesId === s.id} aria-label={`Toggle ${s.label}`} - className='inline-flex items-center gap-1 rounded-md border border-[var(--border)] bg-transparent px-1.5 py-0.5 text-micro' - style={{ - color: resolvedColors[s.id || ''] || s.color, - opacity: dimmed ? 0.4 : isHovered ? 1 : 0.9, - }} + className={cn( + 'inline-flex items-center gap-1 rounded-md border border-[var(--border)] bg-transparent px-1.5 py-0.5 text-micro', + dimmed ? 'opacity-40' : isHovered ? 'opacity-100' : 'opacity-90' + )} + style={{ color: resolvedColors[s.id || ''] || s.color }} onMouseEnter={() => setHoverSeriesId(s.id || null)} onMouseLeave={() => setHoverSeriesId((prev) => (prev === s.id ? null : prev))} onKeyDown={(e) => { @@ -301,7 +348,6 @@ function LineChartComponent({ const clamped = Math.max(padding.left, Math.min(width - padding.right, x)) const ratio = (clamped - padding.left) / (chartWidth || 1) const i = Math.round(ratio * (scaledPoints.length - 1)) - setHoverIndex(i) setHoverPos({ x: clamped, y: e.clientY - rect.top }) const cursorY = e.clientY - rect.top if (activeSeriesId) { @@ -321,7 +367,6 @@ function LineChartComponent({ } }} onMouseLeave={() => { - setHoverIndex(null) setHoverPos(null) setHoverSeriesId(null) }} @@ -355,7 +400,7 @@ function LineChartComponent({ y1={padding.top} x2={padding.left} y2={height - padding.bottom} - stroke='hsl(var(--border))' + stroke='var(--border)' strokeWidth='1' /> @@ -366,7 +411,7 @@ function LineChartComponent({ y1={padding.top + chartHeight * p} x2={width - padding.right} y2={padding.top + chartHeight * p} - stroke='hsl(var(--muted))' + stroke='var(--border)' strokeOpacity='0.35' strokeWidth='1' /> @@ -433,25 +478,7 @@ function LineChartComponent({ /> ) } - const p = (() => { - const p = s.pts - const tension = 0.2 - let d = `M ${p[0].x} ${p[0].y}` - for (let i = 0; i < p.length - 1; i++) { - const p0 = p[i - 1] || p[i] - const p1 = p[i] - const p2 = p[i + 1] - const p3 = p[i + 2] || p[i + 1] - const cp1x = p1.x + ((p2.x - p0.x) / 6) * tension - let cp1y = p1.y + ((p2.y - p0.y) / 6) * tension - const cp2x = p2.x - ((p3.x - p1.x) / 6) * tension - let cp2y = p2.y - ((p3.y - p1.y) / 6) * tension - cp1y = Math.max(yMin, Math.min(yMax, cp1y)) - cp2y = Math.max(yMin, Math.min(yMax, cp2y)) - d += ` C ${cp1x} ${cp1y}, ${cp2x} ${cp2y}, ${p2.x} ${p2.y}` - } - return d - })() + const p = buildSmoothPath(s.pts, yMin, yMax) return ( { - const unitSuffix = (unit || '').trim() - const showInTicks = unitSuffix === '%' - const isLatency = unitSuffix.toLowerCase() === 'latency' - const fmtCompact = (v: number) => { - if (isLatency) return v === 0 ? '0' : formatChartLatency(v) - return formatChartCompactNumber(v) - } - return ( - <> - - {fmtCompact(maxValue)} - {showInTicks && !isLatency ? unit : ''} - - - {fmtCompact(minValue)} - {showInTicks && !isLatency ? unit : ''} - - - ) - })()} + + {yAxisLabels[0]} + + + {yAxisLabels[1]} + @@ -613,6 +625,8 @@ function LineChartComponent({ width, height, tooltipMaxWidth: estimateTooltipWidth(longest), + tooltipHeight: estimateTooltipHeight(toDisplay.length, Boolean(currentHoverDate)), + padding, }) return ( @@ -639,7 +653,4 @@ function LineChartComponent({ ) } -/** - * Memoized LineChart component to prevent re-renders when parent updates. - */ export const LineChart = memo(LineChartComponent) diff --git a/apps/sim/components/charts/radar-chart.tsx b/apps/sim/components/charts/radar-chart.tsx new file mode 100644 index 00000000000..25b26e1a240 --- /dev/null +++ b/apps/sim/components/charts/radar-chart.tsx @@ -0,0 +1,284 @@ +'use client' + +import { memo, useId, useMemo, useState } from 'react' +import { truncate } from '@sim/utils/string' +import { + CHART_GRID_FRACTIONS, + CHART_TICK_FILL, + CHART_TICK_FONT_SIZE, + estimateAxisLabelWidth, +} from '@/components/charts/chart-geometry' +import { + ChartTooltip, + ChartTooltipRow, + estimateTooltipHeight, + estimateTooltipWidth, +} from '@/components/charts/chart-tooltip' +import { + useChartWidth, + useIsDarkTheme, + useResolvedChartColors, +} from '@/components/charts/use-chart-theme' + +export interface RadarChartAxis { + label: string + value: number + /** Text shown for `value` in the hover row. Defaults to the raw number. */ + display?: string +} + +interface RadarChartProps { + axes: RadarChartAxis[] + color: string + height?: number +} + +/** Room above and below the web for the captions on the vertical centreline. */ +const LABEL_GUTTER = 52 + +/** Gap between the outer ring and a caption anchored beyond it. */ +const LABEL_GAP = 12 + +/** + * Caption budget. A long source name would otherwise run past the container, and the + * svg paints outside its box so it would not even clip — it would overlap the section + * beside it. The hover row carries the full name. + */ +const MAX_LABEL_LENGTH = 16 +const RING_COUNT = CHART_GRID_FRACTIONS.length + 1 + +/** + * Polar coordinates for an axis. `-90°` puts the first axis at twelve o'clock, so a + * list read top-down and the web read clockwise start in the same place. + */ +function axisPoint(index: number, count: number, radius: number, cx: number, cy: number) { + const angle = (index / count) * Math.PI * 2 - Math.PI / 2 + return { x: cx + Math.cos(angle) * radius, y: cy + Math.sin(angle) * radius } +} + +function polygon(points: ReadonlyArray<{ x: number; y: number }>): string { + return points.map((p) => `${p.x.toFixed(2)},${p.y.toFixed(2)}`).join(' ') +} + +/** + * Shape of a distribution across a handful of named categories. + * + * The third member of the chart family, and built from the same tokens, tooltip, and + * theme hooks as {@link BarChart} and {@link LineChart}. It answers a question the + * other two cannot: a bar list ranks categories but says nothing about balance, and + * "one source dominates" versus "spend is spread evenly" is legible here at a glance + * and nowhere else on the panel. + * + * Every axis is scaled against the largest value rather than against its own range, + * so the polygon's area is proportional to the real distribution — normalising each + * axis independently would draw a balanced pentagon for any input at all. + */ +function RadarChartComponent({ axes, color, height = 200 }: RadarChartProps) { + const uniqueId = useId().replace(/:/g, '') + const [containerRef, containerWidth] = useChartWidth() + const isDark = useIsDarkTheme() + const [hoverIndex, setHoverIndex] = useState(null) + + const resolvedColors = useResolvedChartColors({ base: color }) + const resolvedColor = resolvedColors.base || color + + const width = containerWidth ?? 0 + const cx = width / 2 + const cy = height / 2 + /* + The horizontal budget is the caption's own estimated width — the same + `estimateAxisLabelWidth` the sibling charts use to size a gutter around SVG text + they cannot measure. A 16-glyph caption runs to ~84px, so a fixed inset let every + side caption paint past the container and over the section beside it; the svg is + `overflow-visible`, so nothing clipped it. + */ + const labelWidth = axes.reduce( + (max, axis) => Math.max(max, estimateAxisLabelWidth(truncate(axis.label, MAX_LABEL_LENGTH))), + 0 + ) + const radius = Math.max( + 0, + Math.min(width / 2 - labelWidth - LABEL_GAP, height / 2 - LABEL_GUTTER / 2) + ) + + const maxValue = Math.max(...axes.map((a) => a.value), 0) + + const points = useMemo( + () => + axes.map((axis, index) => { + const fraction = maxValue > 0 ? axis.value / maxValue : 0 + return { + axis, + outer: axisPoint(index, axes.length, radius, cx, cy), + value: axisPoint(index, axes.length, radius * fraction, cx, cy), + label: axisPoint(index, axes.length, radius + LABEL_GAP, cx, cy), + } + }), + [axes, maxValue, radius, cx, cy] + ) + + if (containerWidth === null) { + return
+ } + + /* + Three axes are the fewest that enclose an area; below that the "polygon" is a + line or a point and reads as a rendering fault rather than as a distribution. + */ + if (axes.length < 3 || maxValue <= 0) { + return ( +
+

No data

+
+ ) + } + + const hovered = hoverIndex !== null ? points[hoverIndex] : null + + return ( + /* + The same scroll guard as the sibling charts: `useChartWidth` floors the svg at + CHART_MIN_WIDTH, so in a narrower box it is wider than its container. The + captions are inside the box by construction — `radius` is budgeted against + `labelWidth` — so nothing legible is clipped, and `overflow-y-hidden` keeps the + `overflow-x` from promoting the vertical axis to `auto`. + */ +
+ + + + + + + + + {/* The web: one ring per grid fraction plus the outer ring, and a spoke per axis. */} + {Array.from({ length: RING_COUNT }, (_, ring) => { + const ringRadius = (radius * (ring + 1)) / RING_COUNT + return ( + axisPoint(index, axes.length, ringRadius, cx, cy)) + )} + fill='none' + stroke='var(--border)' + strokeOpacity={ring === RING_COUNT - 1 ? 0.9 : 0.45} + strokeWidth='1' + /> + ) + })} + {points.map((point, index) => ( + + ))} + + point.value))} + fill={`url(#radar-${uniqueId})`} + stroke={resolvedColor} + strokeWidth='1.5' + strokeLinejoin='round' + /> + + {points.map((point, index) => ( + + ))} + + {points.map((point, index) => ( + cx ? 'start' : 'end' + } + dominantBaseline={Math.abs(point.label.y - cy) < 1 ? 'middle' : 'auto'} + fontSize={CHART_TICK_FONT_SIZE} + fill={CHART_TICK_FILL} + > + {truncate(point.axis.label, MAX_LABEL_LENGTH)} + + ))} + + {/* + Hit targets last so they sit above the painted web. A wedge per axis, drawn + as a transparent triangle from the centre — a vertex-sized target is far too + small to hover on a 200px chart. + */} + {points.map((point, index) => { + const half = Math.PI / axes.length + const angle = (index / axes.length) * Math.PI * 2 - Math.PI / 2 + const reach = radius + LABEL_GUTTER / 2 + const a = { + x: cx + Math.cos(angle - half) * reach, + y: cy + Math.sin(angle - half) * reach, + } + const b = { + x: cx + Math.cos(angle + half) * reach, + y: cy + Math.sin(angle + half) * reach, + } + return ( + setHoverIndex(index)} + onMouseLeave={() => setHoverIndex(null)} + /> + ) + })} + + + {hovered && + (() => { + const value = hovered.axis.display ?? String(hovered.axis.value) + /* + Centred on the chart rather than tracked to the cursor. The wedges all + meet at the centre, so one box there is equidistant from every target and + cannot be pushed outside a chart that — unlike its axis-bearing siblings — + has no padding to clamp against. + */ + const boxWidth = estimateTooltipWidth(Math.max(hovered.axis.label.length, value.length)) + return ( + + + + ) + })()} +
+ ) +} + +export const RadarChart = memo(RadarChartComponent) diff --git a/apps/sim/components/charts/use-chart-theme.ts b/apps/sim/components/charts/use-chart-theme.ts index 30bcd9fd588..b6b864b0644 100644 --- a/apps/sim/components/charts/use-chart-theme.ts +++ b/apps/sim/components/charts/use-chart-theme.ts @@ -1,27 +1,49 @@ 'use client' -import { type RefObject, useEffect, useRef, useState } from 'react' +import { type RefObject, useEffect, useRef, useState, useSyncExternalStore } from 'react' import { CHART_MIN_WIDTH } from '@/components/charts/chart-geometry' +function subscribeToDarkTheme(onStoreChange: () => void): () => void { + const observer = new MutationObserver(onStoreChange) + observer.observe(document.documentElement, { attributes: true, attributeFilter: ['class'] }) + return () => observer.disconnect() +} + +function getDarkThemeSnapshot(): boolean { + return document.documentElement.classList.contains('dark') +} + +/** Dark is the assumed default before the class is readable, matching first paint. */ +function getServerDarkThemeSnapshot(): boolean { + return true +} + /** - * Whether the document is in dark mode, tracked by observing the class the theme - * toggle writes. Charts need this as a *value* rather than a CSS class because SVG - * stroke opacity and blend mode are set per element, not by a selector. + * Whether the document is in dark mode, read from the class the theme toggle writes. + * Charts need this as a *value* rather than a CSS class because SVG stroke opacity + * and blend mode are set per element, not by a selector. + * + * The class is an external store, so it is read through `useSyncExternalStore`: the + * first client render already sees the real value instead of painting the default and + * correcting it in an effect. */ export function useIsDarkTheme(): boolean { - const [isDark, setIsDark] = useState(true) - - useEffect(() => { - if (typeof window === 'undefined') return - const element = document.documentElement - const update = () => setIsDark(element.classList.contains('dark')) - update() - const observer = new MutationObserver(update) - observer.observe(element, { attributes: true, attributeFilter: ['class'] }) - return () => observer.disconnect() - }, []) + return useSyncExternalStore( + subscribeToDarkTheme, + getDarkThemeSnapshot, + getServerDarkThemeSnapshot + ) +} - return isDark +/** Materializes one `var(--token)` into a concrete `rgb()` via a throwaway probe node. */ +function resolveColor(value: string): string { + if (!value.startsWith('var(')) return value + const probe = document.createElement('div') + probe.style.color = value + document.body.appendChild(probe) + const computed = window.getComputedStyle(probe).color + probe.remove() + return computed } /** @@ -34,26 +56,22 @@ export function useIsDarkTheme(): boolean { export function useResolvedChartColors(colors: Record): Record { const [resolved, setResolved] = useState>({}) const serialized = JSON.stringify(colors) + /* + A token resolves to a different `rgb()` per theme, and the probe runs once per + token set — so without this the colours resolved on the theme the chart mounted + under survived a toggle, and the series kept its dark-mode fill on a light page. + */ + const isDark = useIsDarkTheme() useEffect(() => { if (typeof window === 'undefined') return - const resolveColor = (value: string): string => { - if (!value.startsWith('var(')) return value - const probe = document.createElement('div') - probe.style.color = value - document.body.appendChild(probe) - const computed = window.getComputedStyle(probe).color - probe.remove() - return computed - } - const next: Record = {} for (const [key, value] of Object.entries(JSON.parse(serialized) as Record)) { next[key] = resolveColor(value) } setResolved(next) - }, [serialized]) + }, [serialized, isDark]) return resolved } diff --git a/apps/sim/ee/audit-logs/components/audit-logs.tsx b/apps/sim/ee/audit-logs/components/audit-logs.tsx index 62f41b4c16d..82166da1b65 100644 --- a/apps/sim/ee/audit-logs/components/audit-logs.tsx +++ b/apps/sim/ee/audit-logs/components/audit-logs.tsx @@ -1,23 +1,22 @@ 'use client' -import { type ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react' +import { type ReactNode, useEffect, useMemo, useRef, useState } from 'react' import { Badge, Button, Calendar, + Chip, ChipCombobox, ChipInput, ChipSelect, type ComboboxOption, - Download, OverflowText, Popover, PopoverAnchor, PopoverContent, - RefreshCw, - Search, toast, } from '@sim/emcn' +import { Download, RefreshCw, Search, X } from '@sim/emcn/icons' import { createLogger } from '@sim/logger' import { formatDateTime } from '@sim/utils/formatting' import { isRecordLike } from '@sim/utils/object' @@ -33,6 +32,7 @@ import { import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { useSettingsSearch } from '@/app/workspace/[workspaceId]/settings/components/use-settings-search' +import { useOrganizationWorkspaces } from '@/ee/access-control/hooks/permission-groups' import { RESOURCE_TYPE_OPTIONS } from '@/ee/audit-logs/constants' import { type AuditLogFilters, useAuditLogs } from '@/ee/audit-logs/hooks/audit-logs' import { @@ -150,12 +150,15 @@ function renderMetadataValue(value: unknown) { ) } +/** Already rendered as their own labelled rows, so the metadata block would repeat them. */ +const HIDDEN_METADATA_KEYS = new Set(['name', 'description']) + function getMetadataEntries(metadata: unknown) { if (!isRecordLike(metadata)) return [] return Object.entries(metadata).filter(([key, value]) => { if (value === undefined) return false - return !['name', 'description'].includes(key) + return !HIDDEN_METADATA_KEYS.has(key) }) } @@ -251,30 +254,61 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { urlFilters.timeRange === 'Custom range' && (!customStartDate || !customEndDate) ? DEFAULT_AUDIT_TIME_RANGE : urlFilters.timeRange + /** + * Resolved, not merely present. Only the id lives in the URL, and the filter is + * applied once it matches a workspace the organization actually owns — a stale id + * from an old link would otherwise silently narrow the feed to nothing under a + * chip labelled with the bare uuid. + */ + const orgWorkspaces = useOrganizationWorkspaces(organizationId, Boolean(urlFilters.workspace)) + const filteredWorkspace = urlFilters.workspace + ? orgWorkspaces.data?.find((entry) => entry.id === urlFilters.workspace) + : undefined + const [datePickerOpen, setDatePickerOpen] = useState(false) const dateRangeAppliedRef = useRef(false) const [searchTerm, setSearchTerm] = useSettingsSearch() const debouncedSearch = useDebounce(searchTerm, SEARCH_DEBOUNCE_MS).trim() const [isVisuallyRefreshing, setIsVisuallyRefreshing] = useState(false) - const refreshTimersRef = useRef(new Set()) + /* + Lazy-init: `useRef(new Set())` allocates a fresh Set on every render and throws + all but the first away. + */ + const refreshTimersRef = useRef | null>(null) + refreshTimersRef.current ??= new Set() + const refreshTimers = refreshTimersRef.current const [isExporting, setIsExporting] = useState(false) useEffect(() => { - const timers = refreshTimersRef.current return () => { - for (const timerId of timers) window.clearTimeout(timerId) + for (const timerId of refreshTimers) window.clearTimeout(timerId) } - }, []) + }, [refreshTimers]) const filters = useMemo(() => { return { search: debouncedSearch || undefined, resourceType: selectedTypes.length > 0 ? selectedTypes.join(',') : undefined, + workspaceId: filteredWorkspace?.id, startDate: getStartDateFromTimeRange(timeRange, customStartDate)?.toISOString(), endDate: getEndDateFromTimeRange(timeRange, customEndDate)?.toISOString(), } - }, [debouncedSearch, selectedTypes, timeRange, customStartDate, customEndDate]) + }, [ + debouncedSearch, + selectedTypes, + filteredWorkspace?.id, + timeRange, + customStartDate, + customEndDate, + ]) + /** + * A deep-linked workspace scope is only resolvable once the organization's workspace + * list has loaded. Querying before then fetches the whole organization's feed and + * immediately refetches it narrowed — two requests, with a flash of rows the link + * did not ask for in between. + */ + const isWorkspaceFilterPending = Boolean(urlFilters.workspace) && orgWorkspaces.isPending const { data, isLoading, @@ -283,7 +317,7 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { hasNextPage, fetchNextPage, refetch, - } = useAuditLogs(organizationId, filters) + } = useAuditLogs(organizationId, filters, !isWorkspaceFilterPending) const allEntries = useMemo(() => { if (!data?.pages) return [] @@ -324,25 +358,25 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { setDatePickerOpen(false) } - const handleRefresh = useCallback(() => { + const handleRefresh = () => { setIsVisuallyRefreshing(true) const timerId = window.setTimeout(() => { setIsVisuallyRefreshing(false) - refreshTimersRef.current.delete(timerId) + refreshTimers.delete(timerId) }, REFRESH_SPINNER_DURATION_MS) - refreshTimersRef.current.add(timerId) + refreshTimers.add(timerId) refetch().catch((error: unknown) => { logger.error('Failed to refresh audit logs', { error }) }) - }, [refetch]) + } - const handleLoadMore = useCallback(() => { + const handleLoadMore = () => { if (hasNextPage && !isFetchingNextPage) { fetchNextPage().catch((error: unknown) => { logger.error('Failed to load more audit logs', { error }) }) } - }, [hasNextPage, isFetchingNextPage, fetchNextPage]) + } const handleExportCsv = async () => { setIsExporting(true) @@ -351,6 +385,7 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { params.set('organizationId', organizationId) if (filters.search) params.set('search', filters.search) if (filters.resourceType) params.set('resourceType', filters.resourceType) + if (filters.workspaceId) params.set('workspaceId', filters.workspaceId) if (filters.startDate) params.set('startDate', filters.startDate) if (filters.endDate) params.set('endDate', filters.endDate) @@ -410,6 +445,20 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { allOptionLabel='All types' align='start' /> + {filteredWorkspace && ( + /* + A deep-linked scope, not a picker: the organization can hold hundreds of + workspaces, so this narrows the feed only when a link asks it to and + offers exactly one action — take it back off. + */ + void setUrlFilters({ workspace: null })} + aria-label={`Clear the ${filteredWorkspace.name} workspace filter`} + > + {filteredWorkspace.name} + + )}
{/* ChipCombobox (Radix Popover, non-modal), not ChipSelect (Radix DropdownMenu, modal by default) — a modal trigger closing in the @@ -469,7 +518,7 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { No results for "{debouncedSearch}" diff --git a/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx b/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx index 9fd71da3ada..8a26c7e1c5a 100644 --- a/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx +++ b/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx @@ -50,8 +50,14 @@ let container: HTMLDivElement let root: Root let queryClient: QueryClient -function AuditProbe({ organizationId }: { organizationId: string }) { - const auditLogs = useAuditLogs(organizationId, {}) +function AuditProbe({ + organizationId, + workspaceId, +}: { + organizationId: string + workspaceId?: string +}) { + const auditLogs = useAuditLogs(organizationId, { workspaceId }) const entries = auditLogs.data?.pages.flatMap((page) => page.data) ?? [] return ( @@ -62,11 +68,11 @@ function AuditProbe({ organizationId }: { organizationId: string }) { ) } -function renderAuditLogs(organizationId: string) { +function renderAuditLogs(organizationId: string, workspaceId?: string) { act(() => { root.render( - + ) }) @@ -130,4 +136,28 @@ describe('useAuditLogs identity transitions', () => { }) ) }) + + /** + * The other half of the same rule. Blanking the feed on every filter change is what + * the placeholder exists to stop — and it is why the Export action's + * `isPlaceholderData` guard means anything. + */ + it('holds the current entries while a filter change loads, within one organization', async () => { + const filteredPage = createDeferred() + mockRequestJson.mockImplementation( + (contract: unknown, input: { query?: { workspaceId?: string } }) => { + if (contract !== listAuditLogsContract) throw new Error('Unexpected contract') + return input.query?.workspaceId ? filteredPage.promise : Promise.resolve(AUDIT_PAGE_A) + } + ) + + renderAuditLogs('org-a') + await flushQueries() + expect(container).toHaveTextContent('Updated Organization A') + + renderAuditLogs('org-a', 'workspace-a') + await flushQueries() + + expect(container).toHaveTextContent('Updated Organization A') + }) }) diff --git a/apps/sim/ee/audit-logs/hooks/audit-logs.ts b/apps/sim/ee/audit-logs/hooks/audit-logs.ts index 7685f83b3ff..114ac66aad4 100644 --- a/apps/sim/ee/audit-logs/hooks/audit-logs.ts +++ b/apps/sim/ee/audit-logs/hooks/audit-logs.ts @@ -11,11 +11,16 @@ export const auditLogKeys = { [...auditLogKeys.lists(), organizationId, filters] as const, } +/** Position of the organization id in a key built by {@link auditLogKeys.list}. */ +const AUDIT_LOG_KEY_ORGANIZATION_INDEX = 2 + export interface AuditLogFilters { search?: string action?: string resourceType?: string actorId?: string + /** Narrows the feed to one workspace in the organization. */ + workspaceId?: string startDate?: string endDate?: string } @@ -34,6 +39,7 @@ async function fetchAuditLogs( action: filters.action, resourceType: filters.resourceType, actorId: filters.actorId, + workspaceId: filters.workspaceId, startDate: filters.startDate, endDate: filters.endDate, cursor, @@ -50,5 +56,18 @@ export function useAuditLogs(organizationId: string, filters: AuditLogFilters, e getNextPageParam: (lastPage) => lastPage.nextCursor, enabled: Boolean(organizationId) && enabled, staleTime: AUDIT_LOG_LIST_STALE_TIME, + /** + * Held across a filter change, never across an organization change. + * + * Every filter — search, types, window, workspace — is part of the key, so + * without a placeholder the feed blanks to its empty state on each keystroke, and + * the Export action's `isPlaceholderData` guard was dead. But the organization is + * in the key too, and `keepPreviousData` alone would paint one tenant's audit + * entries under another tenant's heading while the new page loaded. + */ + placeholderData: (previous, previousQuery) => + previous && previousQuery?.queryKey[AUDIT_LOG_KEY_ORGANIZATION_INDEX] === organizationId + ? previous + : undefined, }) } diff --git a/apps/sim/ee/audit-logs/search-params.ts b/apps/sim/ee/audit-logs/search-params.ts index 1c4a5284a9e..b28f253b942 100644 --- a/apps/sim/ee/audit-logs/search-params.ts +++ b/apps/sim/ee/audit-logs/search-params.ts @@ -1,4 +1,4 @@ -import { parseAsArrayOf, parseAsString } from 'nuqs/server' +import { createSerializer, parseAsArrayOf, parseAsString } from 'nuqs/server' import { parseAsDateString, parseAsTimeRange, @@ -20,6 +20,13 @@ export const DEFAULT_AUDIT_TIME_RANGE: TimeRange = 'Past 30 days' */ export const auditLogFilterParsers = { types: parseAsArrayOf(parseAsString).withDefault([]), + /** + * Nullable by design: the feed is organization-wide unless a link narrows it, and + * the usage panel's workspace drill-down is what does. Only the id is stored — the + * name is resolved from the loaded workspace list, so a stale id from an old link + * clears the filter rather than labelling it with nothing. + */ + workspace: parseAsString, timeRange: parseAsTimeRange.withDefault(DEFAULT_AUDIT_TIME_RANGE), startDate: parseAsDateString, endDate: parseAsDateString, @@ -36,3 +43,13 @@ export const auditLogFilterUrlKeys = { endDate: 'end-date', }, } as const + +/** + * Outbound links into the audit feed — the usage panel's workspace drill-down builds + * one — serialized from the map the feed itself parses rather than by concatenation, + * which emitted a bare `?workspace=` for a null id and left the value unencoded. + */ +export const serializeAuditLogFilters = createSerializer(auditLogFilterParsers, { + clearOnDefault: true, + urlKeys: auditLogFilterUrlKeys.urlKeys, +}) diff --git a/apps/sim/ee/organization-usage/components/usage-consumers.test.ts b/apps/sim/ee/organization-usage/components/usage-consumers.test.ts new file mode 100644 index 00000000000..2060081e8b2 --- /dev/null +++ b/apps/sim/ee/organization-usage/components/usage-consumers.test.ts @@ -0,0 +1,19 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { USAGE_PROVIDER_ICON_IDS } from '@/ee/organization-usage/components/usage-consumers' +import { PROVIDER_DEFINITIONS } from '@/providers/models' + +describe('PROVIDER_ICONS', () => { + /** + * The breakdown keys model rows by `getProviderFromModel`, which only ever returns + * a registry provider id. A gap here is silent — the row simply renders without a + * mark, which is how GLM shipped as the one iconless model in the list. + */ + it('covers every provider the model registry defines', () => { + const covered = new Set(USAGE_PROVIDER_ICON_IDS) + const missing = Object.keys(PROVIDER_DEFINITIONS).filter((id) => !covered.has(id)) + expect(missing).toEqual([]) + }) +}) diff --git a/apps/sim/ee/organization-usage/components/usage-consumers.tsx b/apps/sim/ee/organization-usage/components/usage-consumers.tsx index e20de0359ac..565694d5eea 100644 --- a/apps/sim/ee/organization-usage/components/usage-consumers.tsx +++ b/apps/sim/ee/organization-usage/components/usage-consumers.tsx @@ -1,21 +1,33 @@ 'use client' import type { ComponentType } from 'react' -import { cn } from '@sim/emcn' -import { ChevronRight } from '@sim/emcn/icons' +import { chipHoverSurfaceClass, cn } from '@sim/emcn' +import { ArrowRight, ChevronDown } from '@sim/emcn/icons' import { formatChartCompactNumber } from '@/components/charts' import { AnthropicIcon, AzureIcon, + BasetenIcon, + BedrockIcon, CerebrasIcon, DeepseekIcon, - GoogleIcon, + FireworksIcon, + GeminiIcon, GroqIcon, + KimiIcon, + LitellmIcon, + MetaIcon, MistralIcon, + NvidiaIcon, OllamaIcon, OpenAIIcon, OpenRouterIcon, + SakanaIcon, + TogetherIcon, + VertexIcon, + VllmIcon, xAIIcon, + ZaiIcon, } from '@/components/icons' import type { OrganizationUsageBreakdown, @@ -27,28 +39,53 @@ import { RowActionsMenu, } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' +import { RESOURCE_ROW_ARROW_CLASSES } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row' import { USAGE_TAB_EMPTY_COPY } from '@/ee/organization-usage/constants' /** * Provider brand marks, keyed by the `providerId` the server resolves. * - * Kept here rather than read from `providers/models.ts`: that module carries the + * Kept here rather than read from `PROVIDER_DEFINITIONS`: that module carries the * whole model registry and would land in this settings chunk for two dozen glyphs. + * The icons themselves come from the same `@/components/icons` module the registry + * imports, so this is a re-keying, never a second set of artwork. + * + * It must list every provider the registry defines, or a model resolving to a + * missing one renders an unexplained blank where every neighbouring row has a mark + * — which is how `zai` (GLM) shipped iconless. `usage-consumers.test.ts` fails when + * the two drift, so the coverage is checked rather than remembered. */ const PROVIDER_ICONS: Readonly>> = { - openai: OpenAIIcon, anthropic: AnthropicIcon, - google: GoogleIcon, - 'azure-openai': AzureIcon, + baseten: BasetenIcon, + bedrock: BedrockIcon, + cerebras: CerebrasIcon, deepseek: DeepseekIcon, - xai: xAIIcon, + fireworks: FireworksIcon, + google: GeminiIcon, groq: GroqIcon, - cerebras: CerebrasIcon, + kimi: KimiIcon, + litellm: LitellmIcon, + meta: MetaIcon, + mistral: MistralIcon, + nvidia: NvidiaIcon, ollama: OllamaIcon, + 'ollama-cloud': OllamaIcon, + openai: OpenAIIcon, openrouter: OpenRouterIcon, - mistral: MistralIcon, + sakana: SakanaIcon, + together: TogetherIcon, + vertex: VertexIcon, + vllm: VllmIcon, + xai: xAIIcon, + zai: ZaiIcon, + 'azure-anthropic': AzureIcon, + /** Not a registry provider — a BYOK credential kind the breakdown can also emit. */ + 'azure-openai': AzureIcon, } +export const USAGE_PROVIDER_ICON_IDS = Object.keys(PROVIDER_ICONS) + interface UsageConsumerRowProps { row: OrganizationUsageBreakdownRow /** BYOK rows carry no cost, so tokens are the only usage they can show. */ @@ -62,8 +99,8 @@ interface UsageConsumerRowProps { * same slot on its `Other` row and keep every figure in one column. */ const TRAILING_SLOT_CLASSES = { - /** `ChevronRight` at the platform icon size. */ - chevron: 'size-[14px]', + /** The canonical resource-row arrow, at its own size. */ + arrow: 'size-4', /** `RowActionsMenu`'s trigger: a 14px glyph in a `chipVariants()` pill. */ menu: 'size-[30px]', } as const @@ -87,7 +124,7 @@ function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsu : {})} className={cn( 'flex w-full items-center gap-2.5 rounded-lg p-2 text-left', - onSelect && 'transition-colors hover:bg-[var(--surface-active)]' + onSelect && cn('transition-colors', chipHoverSurfaceClass) )} > {ProviderIcon && ( @@ -106,11 +143,9 @@ function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsu {showTokensOnly ? formatChartCompactNumber(row.tokens ?? 0) : row.credits.toLocaleString()} - {/* A chevron or a menu, never both — `sim-settings-pages.md`. */} + {/* An arrow or a menu, never both — `sim-settings-pages.md`. */} {onSelect ? ( - + ) : actions?.length ? ( ) : null} @@ -127,6 +162,11 @@ interface UsageConsumersProps { onSelectRow?: (row: OrganizationUsageBreakdownRow) => void /** Set on Members, where a row can open the shared manage-credits modal. */ rowActions?: (row: OrganizationUsageBreakdownRow) => RowAction[] + /** + * Opens the truncated tail. Omitted when the list is already showing everything the + * API will return, which is the one case where the `Other` row has nothing to open. + */ + onExpandOther?: () => void } export function UsageConsumers({ @@ -136,6 +176,7 @@ export function UsageConsumers({ isError, onSelectRow, rowActions, + onExpandOther, }: UsageConsumersProps) { if (isError) { return ( @@ -155,7 +196,7 @@ export function UsageConsumers({ const showTokensOnly = dimension === 'byok' const trailingSlot = onSelectRow - ? TRAILING_SLOT_CLASSES.chevron + ? TRAILING_SLOT_CLASSES.arrow : rowActions ? TRAILING_SLOT_CLASSES.menu : null @@ -174,22 +215,46 @@ export function UsageConsumers({ {/* The truncated tail, named rather than dropped: a ranked list that does not add up to the headline figure is how "the numbers are wrong" reports start. + + A button when there is more the API can return, so the tail opens in place. + Past the API's ceiling it stays a plain row — a control that cannot change + what you see is worse than no control. */} - {breakdown.other.rowCount > 0 && ( -
- - {`Other (${breakdown.other.rowCount} more)`} - - - {showTokensOnly - ? formatChartCompactNumber(breakdown.other.tokens) - : breakdown.other.credits.toLocaleString()} - - {trailingSlot && ( -
- )} + {breakdown.other.rowCount > 0 && + (() => { + const OtherRow = onExpandOther ? 'button' : 'div' + return ( + + + {`Other (${breakdown.other.rowCount} more)`} + + + {showTokensOnly + ? formatChartCompactNumber(breakdown.other.tokens) + : breakdown.other.credits.toLocaleString()} + + {onExpandOther ? ( + + ) : ( + trailingSlot && ( + + ) + })()}
) } diff --git a/apps/sim/ee/organization-usage/components/usage-events-view.tsx b/apps/sim/ee/organization-usage/components/usage-events-view.tsx index 7a499cc77b3..d53f203bed0 100644 --- a/apps/sim/ee/organization-usage/components/usage-events-view.tsx +++ b/apps/sim/ee/organization-usage/components/usage-events-view.tsx @@ -1,6 +1,6 @@ 'use client' -import { chipVariants, cn } from '@sim/emcn' +import { Chip, cn } from '@sim/emcn' import { ArrowLeft } from '@sim/emcn/icons' import { formatDateTime } from '@sim/utils/formatting' import { useRouter } from 'next/navigation' @@ -74,7 +74,7 @@ export function UsageEventsView({ organizationId, backHref }: UsageEventsViewPro router.push(backHref) }} title='Usage events' - description='Every credit-consuming event behind your usage.' + description="Every credit-consuming event across your organization's workspaces." >
))} {hasNextPage && ( - + )} )} diff --git a/apps/sim/ee/organization-usage/components/usage-monitoring.tsx b/apps/sim/ee/organization-usage/components/usage-monitoring.tsx index 0708d28a4cc..23a369dd0d1 100644 --- a/apps/sim/ee/organization-usage/components/usage-monitoring.tsx +++ b/apps/sim/ee/organization-usage/components/usage-monitoring.tsx @@ -6,6 +6,7 @@ import { Calendar, ChipCombobox, ChipModalTabs, + OverflowText, Popover, PopoverAnchor, PopoverContent, @@ -25,9 +26,13 @@ import { } from '@/app/workspace/[workspaceId]/settings/components/manage-credits-modal' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' +import { serializeAuditLogFilters } from '@/ee/audit-logs/search-params' import { UsageConsumers } from '@/ee/organization-usage/components/usage-consumers' +import { UsageSourceMix } from '@/ee/organization-usage/components/usage-source-mix' import { UsageSummary } from '@/ee/organization-usage/components/usage-summary' import { + COLLAPSED_ROW_COUNT, + EXPANDED_ROW_COUNT, PERIOD_OPTIONS, USAGE_OVERVIEW_TAB, USAGE_SECTION_LABELS, @@ -36,6 +41,7 @@ import { type UsageTab, } from '@/ee/organization-usage/constants' import { useUsageWindow } from '@/ee/organization-usage/hooks/use-usage-window' +import { serializeOrganizationUsageParams } from '@/ee/organization-usage/search-params' import { useOrganizationBilling } from '@/hooks/queries/organization' import { useOrganizationUsageBreakdown, @@ -72,8 +78,13 @@ function UsageSection({ interface UsageMonitoringProps { organizationId: string - /** Set by the settings section switch; the sub-route lives under this workspace. */ - workspaceId: string + /** + * Base path of the events drill-down, built by the settings switch the same way it + * builds `creditUsageHref` and `billingHref`. The panel appends its own window. + */ + eventsHref: string + /** Base path of the audit-logs section, which the workspace drill-down scopes. */ + auditLogsHref: string } /** @@ -84,11 +95,17 @@ interface UsageMonitoringProps { * Only the visible tab's dimension is fetched, which is also the performance story — * half the dimensions heap-scan the ledger, and a tab nobody opens never pays for one. */ -export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoringProps) { +export function UsageMonitoring({ + organizationId, + eventsHref: eventsBaseHref, + auditLogsHref: auditLogsBaseHref, +}: UsageMonitoringProps) { const router = useRouter() - const { window, tab, workspace, preset, startDate, endDate, periodLabel, setState } = + const { window, tab, workspace, expanded, preset, startDate, endDate, periodLabel, setState } = useUsageWindow() const [datePickerOpen, setDatePickerOpen] = useState(false) + /** Gates the Export chip so it cannot re-fire while a CSV is still downloading. */ + const [isExporting, setIsExporting] = useState(false) /** The member whose credit limit is being edited, or null when the modal is closed. */ const [creditsTarget, setCreditsTarget] = useState(null) @@ -115,11 +132,17 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring const summary = useOrganizationUsageSummary(organizationId, window) /** * Kept alive in the drill-down purely to name it. The rule is to store the id and - * derive the entity from the loaded list; arriving by click serves this from cache, - * and arriving by deep link fetches it once. + * derive the entity from the loaded list. + * + * Pinned to the full page rather than to the panel's current row limit: the id can + * come from an expanded list or from a bookmark, and a lookup that only held the + * top ten resolved nothing for either — which reads as the drill-down refusing to + * open, since `isWorkspaceDetail` gates on the name. Requesting the ceiling means a + * click from an expanded list is served from that list's own cache entry. */ const workspaceList = useOrganizationUsageBreakdown(organizationId, window, 'workspace', { enabled: isWorkspaceSelected, + limit: EXPANDED_ROW_COUNT, }) const workspaceName = workspaceList.data?.rows.find((row) => row.id === workspace)?.label /** @@ -135,17 +158,42 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring ? 'workflow' : (tab as UsageBreakdownDimension) + const rowLimit = expanded ? EXPANDED_ROW_COUNT : COLLAPSED_ROW_COUNT const breakdown = useOrganizationUsageBreakdown(organizationId, window, dimension, { + limit: rowLimit, ...(isWorkspaceDetail && workspace ? { workspaceId: workspace } : {}), }) + + /** + * Opens the tail, unless the list is already at the API's ceiling — past that the + * `Other` row is a true remainder and the control would do nothing. `undefined` + * rather than a no-op handler, so the row renders as text instead of as a button. + */ + const handleExpandOther = + rowLimit < EXPANDED_ROW_COUNT ? () => void setState({ expanded: true }) : undefined const workspaceSources = useOrganizationUsageBreakdown(organizationId, window, 'source', { enabled: isWorkspaceDetail, + limit: rowLimit, ...(workspace ? { workspaceId: workspace } : {}), }) // Already cached by Members and Billing, so the meter costs nothing extra and // cannot report a different allowance than they do. const billing = useOrganizationBilling(organizationId) + /** The organization audit feed, narrowed to the workspace being drilled into. */ + const auditLogsHref = serializeAuditLogFilters(auditLogsBaseHref, { workspace }) + + /** + * The drill-down is the same window, in more detail. Without the params it read its + * own defaults and silently showed the current period while the panel behind it + * showed a custom range — two pages disagreeing about what "this" means. + */ + const eventsHref = serializeOrganizationUsageParams(eventsBaseHref, { + preset: window.preset, + startDate: window.startDate ?? null, + endDate: window.endDate ?? null, + }) + const handlePeriodChange = (value: string) => { if (value === 'custom') { setDatePickerOpen(true) @@ -173,6 +221,8 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring } const handleExport = async () => { + if (isExporting) return + setIsExporting(true) // The organization is the path segment below; the query no longer carries a // second copy of it. const params = new URLSearchParams({ @@ -211,6 +261,8 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring URL.revokeObjectURL(url) } catch { toast.error('Failed to export usage') + } finally { + setIsExporting(false) } } @@ -221,17 +273,27 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring if (isWorkspaceDetail && workspace) { return ( void setState({ workspace: null }), + onSelect: () => void setState({ workspace: null, expanded: null }), }} title={workspaceName ?? 'Workspace usage'} actions={[ { + /* + The organization's audit feed, scoped to this workspace — not + `/workspace//logs`. Organization admin is not workspace + membership, and `WorkspaceLayout` answers a non-member with + `WorkspaceAccessDenied`, so the run-logs route was a one-way trip to + a dead end for any workspace the admin had not joined. Audit logs + live in the settings section the admin is already inside. + */ text: 'Open logs', - onSelect: () => router.push(`/workspace/${workspace}/logs`), + onSelect: () => router.push(auditLogsHref), + onPrefetch: () => router.prefetch(auditLogsHref), }, ]} > @@ -247,6 +309,7 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring breakdown={workspaceSources.data} isLoading={workspaceSources.isLoading} isError={workspaceSources.isError} + onExpandOther={handleExpandOther} /> @@ -255,6 +318,7 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring breakdown={breakdown.data} isLoading={breakdown.isLoading} isError={breakdown.isError} + onExpandOther={handleExpandOther} /> @@ -262,54 +326,67 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring } return ( - router.push(`/workspace/${workspaceId}/settings/usage/events`), - }, - { - text: 'Export', - icon: Download, - onSelect: () => void handleExport(), - }, - ]} - > -
- void setState({ tab: value as UsageTab, workspace: null })} - /> -
- {/* ChipCombobox (Radix Popover, non-modal), not ChipSelect (Radix + <> + router.push(eventsHref), + onPrefetch: () => router.prefetch(eventsHref), + }, + { + text: 'Export', + icon: Download, + onSelect: () => void handleExport(), + disabled: summary.isLoading || isExporting, + }, + ]} + > +
+ + void setState({ tab: value as UsageTab, workspace: null, expanded: null }) + } + /> +
+ {/* ChipCombobox (Radix Popover, non-modal), not ChipSelect (Radix DropdownMenu, modal by default) — a modal trigger closing in the same tick that opens the Calendar popover below traps it behind the modal's focus lock, so "Custom range" silently does nothing. */} - {periodLabel} - } - align='end' - /> - { - if (!isOpen) setDatePickerOpen(false) - }} - > - - - {/* + overlayLabel={periodLabel} + overlayContent={ + + } + align='end' + /> + { + if (!isOpen) setDatePickerOpen(false) + }} + > + + + {/* No `showTime`: the panel buckets by calendar day, so a time of day is precision it cannot render. It also emitted the end bound as an inclusive `…T23:59:59` local wall time, which the window resolver then @@ -318,87 +395,117 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring rejected. Bare `YYYY-MM-DD` bounds parse as UTC midnight, matching the rest of the window logic. */} - setDatePickerOpen(false)} - /> - - + setDatePickerOpen(false)} + /> + + +
-
- {isOverview ? ( - <> - {/* + {isOverview ? ( + <> + {/* The allowance is a per-billing-period figure, so it is only comparable to the current period's total. Against a rolling window or a custom range it measures a different span than the limit covers — a 30-day window spanning two periods could read "Over limit" while neither period was — so those windows show the figure without an allowance. */} - - {/* + + + + {/* "What kind of work was this?" belongs beside the total it explains, not behind a tab — it is the second half of the same sentence. + + Two columns of the same data: the list ranks the sources, the radar shows + whether spend is concentrated or spread. Stacked below `lg`, where two + half-columns would leave neither the list room for its figures nor the web + room for its captions. */} - +
+ + + + + + +
+ + ) : ( + + void setState({ workspace: row.id, expanded: null }, { history: 'push' }), + } + : {})} + {...(canManageCredits + ? { + rowActions: (row) => [ + { + label: 'Manage credits', + onSelect: () => setCreditsTarget({ userId: row.id, name: row.label }), + }, + ], + } + : {})} /> - - ) : ( - - void setState({ workspace: row.id }) } - : {})} - {...(canManageCredits - ? { - rowActions: (row) => [ - { - label: 'Manage credits', - onSelect: () => setCreditsTarget({ userId: row.id, name: row.label }), - }, - ], - } - : {})} - /> - - )} - + )} + {/* + A sibling of the panel, not a child. `SettingsPanel` renders its children + straight into the shell's gap-7 content column, so a modal mounted inside it + is a body slot that contributes to that spacing. + The same modal the Members settings page opens, driven by the same hooks — setting a cap here and there is one implementation, not two. */} - { - if (!open) setCreditsTarget(null) - }} - organizationId={organizationId} - member={creditsTarget} - /> - + {canManageCredits && ( + { + if (!open) setCreditsTarget(null) + }} + organizationId={organizationId} + member={creditsTarget} + /> + )} + ) } diff --git a/apps/sim/ee/organization-usage/components/usage-source-mix.tsx b/apps/sim/ee/organization-usage/components/usage-source-mix.tsx new file mode 100644 index 00000000000..3974fa03aac --- /dev/null +++ b/apps/sim/ee/organization-usage/components/usage-source-mix.tsx @@ -0,0 +1,80 @@ +'use client' + +import { useMemo } from 'react' +import { RadarChart, type RadarChartAxis } from '@/components/charts' +import type { OrganizationUsageBreakdown } from '@/lib/api/contracts/organization-usage' +import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' + +/** The same series colour the credit bars use, so one period reads as one dataset. */ +const MIX_SERIES_COLOR = 'var(--indicator-seat-filled)' + +/** + * Beyond this the web's captions overlap and the shape stops being readable, so the + * tail folds into one axis — the same treatment the list gives its `Other` row. + */ +const MAX_AXES = 6 + +interface UsageSourceMixProps { + breakdown?: OrganizationUsageBreakdown + isLoading: boolean + isError: boolean +} + +/** + * The source list's shape, beside the list itself. + * + * The rows answer "how much did each source cost"; they cannot answer "is this + * organization's spend concentrated or spread", which is the question an admin + * actually opens this tab with. Reading the same rows as a polygon makes a single + * dominant source and an even split visibly different at a glance. + */ +export function UsageSourceMix({ breakdown, isLoading, isError }: UsageSourceMixProps) { + /* + Stabilized so `RadarChart`'s `memo()` can pass — built inline it was a new array + on every render of the panel. Above the early return because hooks cannot be + called conditionally. + */ + const axes = useMemo(() => { + const rows = breakdown?.rows ?? [] + const head = rows.slice(0, MAX_AXES) + const tail = rows.slice(MAX_AXES) + /* + The folded axis carries the API's own remainder as well as the rows this chart + dropped, so the web reconciles to the same total — and reads the same label — as + the list beside it. A second presentation of one dataset that disagrees with the + first is read as a bug in both. + */ + const otherRowCount = tail.length + (breakdown?.other.rowCount ?? 0) + const otherCredits = + tail.reduce((total, row) => total + row.credits, 0) + (breakdown?.other.credits ?? 0) + return [ + ...head.map((row) => ({ + label: row.label, + value: row.credits, + display: row.credits.toLocaleString(), + })), + ...(otherRowCount > 0 + ? [ + { + label: `Other (${otherRowCount} more)`, + value: otherCredits, + display: otherCredits.toLocaleString(), + }, + ] + : []), + ] + }, [breakdown]) + + if (isError) { + return ( + + Couldn't load the source mix. + + ) + } + if (isLoading || !breakdown) { + return Loading source mix… + } + + return +} diff --git a/apps/sim/ee/organization-usage/components/usage-summary.tsx b/apps/sim/ee/organization-usage/components/usage-summary.tsx index 43cb6a1ac31..ae4ab4d5156 100644 --- a/apps/sim/ee/organization-usage/components/usage-summary.tsx +++ b/apps/sim/ee/organization-usage/components/usage-summary.tsx @@ -1,14 +1,23 @@ 'use client' +import { useMemo } from 'react' import { Badge } from '@sim/emcn' import { BarChart } from '@/components/charts' import type { OrganizationUsageSummary } from '@/lib/api/contracts/organization-usage' import { formatCreditsLabel } from '@/lib/billing/credits/conversion' +import { SegmentedMeter } from '@/app/workspace/[workspaceId]/settings/components/segmented-meter' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' /** Consumption, matching the seat meter's indicator rather than an outcome colour. */ const USAGE_SERIES_COLOR = 'var(--indicator-seat-filled)' +/** + * A credit allowance runs to the millions, so the meter cannot be countable the way + * the seat meter is — a fixed count reads as a percentage instead. Matches the seat + * overview's own fallback. + */ +const ALLOWANCE_SEGMENTS = 24 + interface UsageSummaryProps { summary?: OrganizationUsageSummary /** Pooled allowance in credits, from the organization's billing data. `null` when uncapped. */ @@ -23,6 +32,19 @@ function percentDelta(current: number, previous: number): number | null { } export function UsageSummary({ summary, limitCredits, isLoading, isError }: UsageSummaryProps) { + /* + Stabilized so `BarChart`'s `memo()` can actually pass. Built inline it was a new + array on every render of the panel — a date-picker toggle or an export click + re-rendered ninety bars for nothing. + + Above the early returns because hooks cannot be called conditionally. + */ + const series = useMemo( + () => + summary?.series.map((point) => ({ timestamp: point.timestamp, value: point.credits })) ?? [], + [summary] + ) + if (isError) { return ( @@ -47,7 +69,11 @@ export function UsageSummary({ summary, limitCredits, isLoading, isError }: Usag number twice and read as a rendering bug. */}
- + {/* + `text-base`, not `text-lg`: the shell's page title is `text-lg`, and a + metric drawn at the same size competed with the header for the first read. + */} + {formatCreditsLabel(used)} {hasLimit && ( @@ -63,22 +89,24 @@ export function UsageSummary({ summary, limitCredits, isLoading, isError }: Usag )} {isOverLimit && ( - + // `red`, not `amber`: past the pooled allowance is a violation, and the + // trend badge sitting immediately beside it is already amber. + Over limit )}
- ({ - timestamp: point.timestamp, - value: point.credits, - }))} - label='' - color={USAGE_SERIES_COLOR} - unit='credits' - height={160} - /> + {/* + The shared allowance meter, not a second bar drawn from the same token. + `SegmentedMeter` already owns the overage tone, and it is the affordance the + seat overview uses for exactly this question. + */} + {hasLimit && ( + + )} + +
) } diff --git a/apps/sim/ee/organization-usage/constants.ts b/apps/sim/ee/organization-usage/constants.ts index 4a96fd042b7..f0f97053ab0 100644 --- a/apps/sim/ee/organization-usage/constants.ts +++ b/apps/sim/ee/organization-usage/constants.ts @@ -39,7 +39,13 @@ export const USAGE_TAB_ORDER: readonly UsageTab[] = [ 'member', 'workspace', 'model', - 'byok', + /* + 'byok' is withheld, not removed. No usage has been recorded against a + bring-your-own-key provider yet, so the tab could only ever show its empty + state. Restore this entry once the ledger carries BYOK rows — the dimension, + its labels, its token unit, and the breakdown query all still work. + */ + // 'byok', ] export const USAGE_TAB_LABELS: Record = { @@ -70,5 +76,16 @@ export const USAGE_TAB_EMPTY_COPY: Record = { source: 'Nothing consumed credits in this period.', } +/** + * Rows per breakdown before and after the `Other` row is expanded. + * + * The collapsed count keeps a tab to one screen; the expanded one is the contract's + * own ceiling (`usageLimitSchema(50, 10)`), so asking for more would be refused. A + * dimension with more than {@link EXPANDED_ROW_COUNT} distinct rows still shows an + * `Other` row after expanding, which is the honest result rather than a bug. + */ +export const COLLAPSED_ROW_COUNT = 10 +export const EXPANDED_ROW_COUNT = 50 + export const DEFAULT_USAGE_PRESET = 'current-period' as const export const DEFAULT_USAGE_TAB = USAGE_OVERVIEW_TAB diff --git a/apps/sim/ee/organization-usage/hooks/use-usage-window.test.ts b/apps/sim/ee/organization-usage/hooks/use-usage-window.test.ts new file mode 100644 index 00000000000..766bac6d619 --- /dev/null +++ b/apps/sim/ee/organization-usage/hooks/use-usage-window.test.ts @@ -0,0 +1,45 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { MAX_CUSTOM_RANGE_DAYS } from '@/lib/api/contracts/organization-usage' +import { isUsableCustomRange } from '@/ee/organization-usage/hooks/use-usage-window' + +/** + * Every condition here is one the server answers with a 400. A deep link carrying + * one would otherwise be marked resolved and fail all four of the panel's queries, + * so the guard degrades the link to the default window instead — and that only + * works while these three rules match the window resolver's. + */ +describe('isUsableCustomRange', () => { + it('accepts a well-formed range inside the cap', () => { + expect(isUsableCustomRange('2026-01-01', '2026-01-31')).toBe(true) + }) + + it('accepts a single-day range', () => { + expect(isUsableCustomRange('2026-01-01', '2026-01-01')).toBe(true) + }) + + it('rejects a missing bound', () => { + expect(isUsableCustomRange('2026-01-01', null)).toBe(false) + expect(isUsableCustomRange(null, '2026-01-31')).toBe(false) + }) + + it('rejects a malformed or unreal date', () => { + expect(isUsableCustomRange('2026-1-1', '2026-01-31')).toBe(false) + expect(isUsableCustomRange('2026-02-30', '2026-03-01')).toBe(false) + expect(isUsableCustomRange('not-a-date', '2026-03-01')).toBe(false) + }) + + it('rejects an inverted pair', () => { + expect(isUsableCustomRange('2026-03-01', '2026-02-01')).toBe(false) + }) + + it('accepts a span of exactly the cap and rejects one day past it', () => { + const start = new Date('2026-01-01T00:00:00.000Z') + const at = new Date(start.getTime() + (MAX_CUSTOM_RANGE_DAYS - 1) * 86_400_000) + const past = new Date(start.getTime() + MAX_CUSTOM_RANGE_DAYS * 86_400_000) + expect(isUsableCustomRange('2026-01-01', at.toISOString().slice(0, 10))).toBe(true) + expect(isUsableCustomRange('2026-01-01', past.toISOString().slice(0, 10))).toBe(false) + }) +}) diff --git a/apps/sim/ee/organization-usage/hooks/use-usage-window.ts b/apps/sim/ee/organization-usage/hooks/use-usage-window.ts index 212c44ba596..a10ac5251b4 100644 --- a/apps/sim/ee/organization-usage/hooks/use-usage-window.ts +++ b/apps/sim/ee/organization-usage/hooks/use-usage-window.ts @@ -1,6 +1,5 @@ 'use client' -import { useMemo } from 'react' import { useQueryStates } from 'nuqs' import { MAX_CUSTOM_RANGE_DAYS, @@ -32,7 +31,7 @@ function isCalendarDate(value: string | null): value is string { * guard exists to provide. Duplicated deliberately, and narrowly: these are the three * conditions that turn a link into an error rather than into different data. */ -function isUsableCustomRange(start: string | null, end: string | null): boolean { +export function isUsableCustomRange(start: string | null, end: string | null): boolean { if (!isCalendarDate(start) || !isCalendarDate(end)) return false const from = new Date(`${start}T00:00:00.000Z`).getTime() const to = new Date(`${end}T00:00:00.000Z`).getTime() @@ -63,16 +62,18 @@ export function useUsageWindow() { const preset: UsageWindowPreset = state.preset === 'custom' && !isResolvedCustom ? DEFAULT_USAGE_PRESET : state.preset - const window = useMemo( - () => ({ - preset, - ...(isResolvedCustom - ? { startDate: state.startDate ?? undefined, endDate: state.endDate ?? undefined } - : {}), - timezone, - }), - [preset, isResolvedCustom, state.startDate, state.endDate, timezone] - ) + /* + Not memoized: this object is only ever hashed, never compared by identity — + React Query hashes a query key structurally, and the panel reads the primitive + fields off it directly. + */ + const window: OrganizationUsageWindowKey = { + preset, + ...(isResolvedCustom + ? { startDate: state.startDate ?? undefined, endDate: state.endDate ?? undefined } + : {}), + timezone, + } const periodLabel = isResolvedCustom ? `${formatDateShort(state.startDate as string)} - ${formatDateShort(state.endDate as string)}` @@ -82,6 +83,7 @@ export function useUsageWindow() { window, tab: state.tab, workspace: state.workspace, + expanded: state.expanded, /** * The *resolved* preset, not the raw URL value. A partial custom deep link * queries the current period, so surfacing `state.preset` left the picker diff --git a/apps/sim/ee/organization-usage/search-params.ts b/apps/sim/ee/organization-usage/search-params.ts index 576db83fc4b..09d22e9fd2f 100644 --- a/apps/sim/ee/organization-usage/search-params.ts +++ b/apps/sim/ee/organization-usage/search-params.ts @@ -1,4 +1,4 @@ -import { parseAsString, parseAsStringLiteral } from 'nuqs/server' +import { createSerializer, parseAsBoolean, parseAsString, parseAsStringLiteral } from 'nuqs/server' import { USAGE_WINDOW_PRESETS } from '@/lib/api/contracts/organization-usage' import { parseAsDateString } from '@/app/workspace/[workspaceId]/logs/search-params' import { @@ -26,6 +26,13 @@ export const organizationUsageParsers = { * list rather than rendering an empty drill-down. */ workspace: parseAsString, + /** + * Whether the visible breakdown's `Other` row has been opened. In the URL because + * it is shareable view-state like every other filter here — and because it changes + * which rows the page fetched, so a shared link that omitted it would not show the + * list the sender was looking at. + */ + expanded: parseAsBoolean.withDefault(false), } as const /** Filter view-state: clean URLs, no back-stack churn, kebab-case URL keys. */ @@ -38,3 +45,14 @@ export const organizationUsageUrlKeys = { endDate: 'end-date', }, } as const + +/** + * Outbound links into the usage drill-downs, serialized from the same parser map the + * destination reads. Hand-writing the wire keys duplicated the `urlKeys` remap, so + * renaming `start-date` would have silently dropped the window from every such link — + * exactly the panel/drill-down disagreement the events href exists to prevent. + */ +export const serializeOrganizationUsageParams = createSerializer(organizationUsageParsers, { + clearOnDefault: true, + urlKeys: organizationUsageUrlKeys.urlKeys, +}) diff --git a/apps/sim/hooks/queries/organization-usage.ts b/apps/sim/hooks/queries/organization-usage.ts index ef19c5f5fe2..b808c9cd3af 100644 --- a/apps/sim/hooks/queries/organization-usage.ts +++ b/apps/sim/hooks/queries/organization-usage.ts @@ -1,6 +1,6 @@ 'use client' -import { keepPreviousData, useInfiniteQuery, useQuery } from '@tanstack/react-query' +import { hashKey, keepPreviousData, useInfiniteQuery, useQuery } from '@tanstack/react-query' import { requestJson } from '@/lib/api/client/request' import { getOrganizationUsageBreakdownContract, @@ -53,6 +53,15 @@ interface UseBreakdownOptions { workspaceId?: string } +/** + * A breakdown key with its row limit removed. The key ends `dimension, limit, + * workspaceId`, so dropping the second-to-last element leaves the identity of the + * list — which is what "the same list, more rows" has to compare on. + */ +function breakdownKeyWithoutLimit(key: readonly unknown[]): unknown[] { + return key.filter((_, index) => index !== key.length - 2) +} + export function useOrganizationUsageBreakdown( organizationId: string | undefined, window: OrganizationUsageWindowKey, @@ -61,14 +70,15 @@ export function useOrganizationUsageBreakdown( ) { const limit = options.limit ?? 10 const { workspaceId } = options + const queryKey = organizationUsageKeys.breakdown( + organizationId ?? '', + window, + dimension, + limit, + workspaceId + ) return useQuery({ - queryKey: organizationUsageKeys.breakdown( - organizationId ?? '', - window, - dimension, - limit, - workspaceId - ), + queryKey, queryFn: ({ signal }): Promise => requestJson(getOrganizationUsageBreakdownContract, { params: { id: organizationId as string }, @@ -82,8 +92,20 @@ export function useOrganizationUsageBreakdown( }), enabled: Boolean(organizationId) && (options.enabled ?? true), staleTime: ORGANIZATION_USAGE_BREAKDOWN_STALE_TIME, - // Deliberately no keepPreviousData: a stale ranking under a new group label reads - // as wrong data, which is worse than a brief skeleton. + /** + * Kept only across a row-limit change — opening the `Other` row asks the same + * question of the same list, and the visible rows are a prefix of the answer, so + * dimming beats blanking. Any other key change (dimension, window, workspace) + * would put a stale ranking under a new label, which reads as wrong data and is + * worse than a brief skeleton. + */ + placeholderData: (previous, previousQuery) => + previous && + previousQuery && + hashKey(breakdownKeyWithoutLimit(previousQuery.queryKey)) === + hashKey(breakdownKeyWithoutLimit(queryKey)) + ? previous + : undefined, }) } diff --git a/apps/sim/lib/api/contracts/audit-logs.ts b/apps/sim/lib/api/contracts/audit-logs.ts index ebf09243741..df7a7dbeca4 100644 --- a/apps/sim/lib/api/contracts/audit-logs.ts +++ b/apps/sim/lib/api/contracts/audit-logs.ts @@ -1,5 +1,5 @@ import { z } from 'zod' -import { organizationIdSchema } from '@/lib/api/contracts/primitives' +import { organizationIdSchema, workspaceIdSchema } from '@/lib/api/contracts/primitives' import { defineRouteContract } from '@/lib/api/contracts/types' export const auditLogsQuerySchema = z.object({ @@ -11,6 +11,13 @@ export const auditLogsQuerySchema = z.object({ action: z.string().optional(), resourceType: z.string().optional(), actorId: z.string().optional(), + /** + * Narrows the org-scoped feed to one workspace. The use case already refuses an + * id outside the caller's organization; this only opens the door the internal + * surface had left shut while `buildFilterConditions` and the v1 contract both + * supported it. + */ + workspaceId: workspaceIdSchema.optional(), startDate: z .string() .optional() diff --git a/apps/sim/lib/billing/application/organization-usage/get-organization-usage-breakdown.ts b/apps/sim/lib/billing/application/organization-usage/get-organization-usage-breakdown.ts index 1e760bc58ac..8efa0811cfd 100644 --- a/apps/sim/lib/billing/application/organization-usage/get-organization-usage-breakdown.ts +++ b/apps/sim/lib/billing/application/organization-usage/get-organization-usage-breakdown.ts @@ -20,7 +20,7 @@ import { type InternalUsageLogSource, toBillingUsageLogSource, } from '@/lib/billing/usage-sources' -import { getProviderFromModel } from '@/providers/models' +import { getProviderFromModel, PROVIDER_DEFINITIONS } from '@/providers/models' export interface OrganizationUsageBreakdownInput { organizationId: string @@ -56,22 +56,15 @@ export interface OrganizationUsageBreakdownResult { const NAMED_DIMENSIONS = new Set(['member', 'workspace', 'workflow']) /** Provider ids that read better with their conventional casing. */ -const PROVIDER_LABELS: Readonly> = { - openai: 'OpenAI', - anthropic: 'Anthropic', - google: 'Google', - 'azure-openai': 'Azure OpenAI', - deepseek: 'DeepSeek', - xai: 'xAI', - groq: 'Groq', - cerebras: 'Cerebras', - ollama: 'Ollama', - openrouter: 'OpenRouter', - mistral: 'Mistral', -} - +/** + * The registry's own display name, not a second hand-written table. + * + * A local map had eleven of the registry's twenty-two providers, so anything newer + * — `zai`, `kimi`, `vertex` — surfaced as a raw lowercase id. This is a server + * module, so reading the registry costs nothing a client bundle would pay for. + */ function providerLabel(providerId: string): string { - return PROVIDER_LABELS[providerId] ?? providerId + return PROVIDER_DEFINITIONS[providerId]?.name ?? providerId } export const getOrganizationUsageBreakdown = defineAuthorizedOrganizationUsageUseCase({ diff --git a/packages/emcn/src/components/chip-input/chip-input.test.tsx b/packages/emcn/src/components/chip-input/chip-input.test.tsx index ed7c1a54f9f..5f55e70afbe 100644 --- a/packages/emcn/src/components/chip-input/chip-input.test.tsx +++ b/packages/emcn/src/components/chip-input/chip-input.test.tsx @@ -9,12 +9,12 @@ import { ChipInput } from './chip-input' let root: Root | null = null let container: HTMLDivElement | null = null -function mount(): HTMLInputElement { +function mount(type?: string): HTMLInputElement { ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true container = document.createElement('div') document.body.appendChild(container) root = createRoot(container) - act(() => root?.render()) + act(() => root?.render()) const input = container.querySelector('input') if (!input) throw new Error('ChipInput did not render an input') @@ -35,4 +35,21 @@ describe('ChipInput', () => { expect(input.className).toContain('-ml-1') expect(input.className).toContain('indent-1') }) + + /** + * A chip field owns its chrome, and the browser's number stepper paints a bordered + * double-arrow inside it that belongs to no design token. Suppressing it lives on + * the component so no caller re-derives the three vendor rules by hand. + */ + it('suppresses the native number spinner on a number field', () => { + const input = mount('number') + + expect(input.className).toContain('[appearance:textfield]') + expect(input.className).toContain('[&::-webkit-inner-spin-button]:appearance-none') + expect(input.className).toContain('[&::-webkit-outer-spin-button]:appearance-none') + }) + + it('leaves a text field untouched, since it has no stepper to hide', () => { + expect(mount().className).not.toContain('[appearance:textfield]') + }) }) diff --git a/packages/emcn/src/components/chip-input/chip-input.tsx b/packages/emcn/src/components/chip-input/chip-input.tsx index 3f668691bd7..53434d7f05b 100644 --- a/packages/emcn/src/components/chip-input/chip-input.tsx +++ b/packages/emcn/src/components/chip-input/chip-input.tsx @@ -28,7 +28,22 @@ */ import * as React from 'react' import { cn } from '../../lib/cn' -import { chipFieldSurfaceClass, chipFieldTextClass } from '../chip/chip-chrome' +import { chipFieldSurfaceClass, chipFieldTextClass, chipGeometryClass } from '../chip/chip-chrome' + +/** + * Suppresses the native number spinner. + * + * A `type='number'` field is still a chip field: it owns a flat 30px surface with + * no focus ring, and the browser's stepper paints its own chrome inside it — + * a bordered double-arrow that belongs to no design token and shifts the value's + * right edge on hover. Removing it belongs to the component rather than to every + * caller, or each one re-derives the same three vendor rules by hand. + * + * `appearance: textfield` covers Firefox; the WebKit pseudo-elements cover + * Chrome and Safari, which ignore it. + */ +const NUMBER_SPINNER_RESET = + '[appearance:textfield] [&::-webkit-inner-spin-button]:m-0 [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:m-0 [&::-webkit-outer-spin-button]:appearance-none' type ChipInputIcon = React.ComponentType<{ className?: string }> @@ -65,7 +80,8 @@ export const ChipInput = React.forwardRef( ) => (
( className={cn( '-ml-1 h-full w-full bg-transparent indent-1 disabled:cursor-not-allowed', chipFieldTextClass, + type === 'number' && NUMBER_SPINNER_RESET, inputClassName )} {...props} diff --git a/packages/emcn/src/icons/chart-column.tsx b/packages/emcn/src/icons/chart-column.tsx index 4f701a0805e..fdd011d0175 100644 --- a/packages/emcn/src/icons/chart-column.tsx +++ b/packages/emcn/src/icons/chart-column.tsx @@ -20,9 +20,9 @@ export function ChartColumn(props: SVGProps) { aria-hidden='true' {...props} > - - - + + + ) } From 13a18946e22ce007ea8e3aa6c25f219097eeaaf7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 27 Aug 2026 21:55:38 -0700 Subject: [PATCH 02/10] fix(usage): repair the chart family's shared geometry and the audit export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up review of the previous commit, across the whole diff. The CSV export ignored the workspace filter. The contract accepted it, the on-screen feed applied it, and the export route dropped it on the floor — every field of `AuditLogFilterParams` is optional, so omitting one still type-checks. An admin exporting a workspace-scoped feed downloaded the whole organization, under a truncation warning that blamed the date range. The route now forwards the parsed query whole and refuses an out-of-organization id the way the list route does, with tests for both. The line chart's derived hover index was not clamped. The previous commit replaced stored state with a derivation, but reproduced the clamp only for the bar chart: `padding.left` follows the axis labels and `chartWidth` follows the container, so a sidebar collapse mid-hover pushed the ratio past 1 and indexed off the end — the dot, the rule and the tooltip all vanished until the cursor moved. Per-chart gutters de-aligned the logs dashboard, where three line charts sit in one row. Deriving each from its own labels put their plot origins at 26, 27 and 32 where they had shared one. The gutter now rounds up to a step, which collapses differences that small and leaves several pixels of slack instead of the sub-pixel margin `Math.ceil` alone gave. The radar chart, reviewed against its siblings: - Its hover targets were triangles, whose far edge is the chord. Along its own spoke a triangle reaches only `reach·cos(π/n)` — at three axes, 50px against a 74px radius — so the largest value's vertex, the one a reader aims at, sat outside every target. They are arc sectors now. - The tooltip was positioned against the scroll container rather than the plot, so below the width floor it stayed nailed while the web slid under it. It now sits beside the hovered vertex through the family's own placer, instead of covering the densest part of the gradient. - Captions below the centreline rode ~3px off the ring rather than the gap they were given, and captions beside the web were misaligned from their own vertex. - Web opacities, stroke width, vertex radius, the per-theme fill relationship and the `screen` blend now match the bar and line charts rather than freelancing. - Its rings read the shared grid fractions instead of dividing into even steps that agreed with the siblings only by coincidence. Also: one `expanded` flag drove both lists in the workspace drill-down, so opening either tail silently opened the other's; the Overview and tab lists rendered an inert `Other` row while the same row two clicks away was a button; the expand chevron knocked the value column out of alignment; row hover had regressed to the chip surface where every other settings row uses the active one; and the radar and the list beside it printed two different `Other (N more)` counts under identical wording. The Overview's two readings of the source data now share one section rather than drawing two half-width rules on one line. Rendered-geometry tests cover the clipping and caption bugs against the real SVG — a unit test of the helpers could not have caught either, since both came from a callsite combining correct helpers wrongly. --- .../app/api/audit-logs/export/route.test.ts | 25 ++ apps/sim/app/api/audit-logs/export/route.ts | 33 +- .../usage-limit-field/usage-limit-field.tsx | 1 - apps/sim/components/charts/bar-chart.tsx | 15 +- .../components/charts/chart-geometry.test.ts | 35 +- apps/sim/components/charts/chart-geometry.ts | 29 +- .../components/charts/chart-layout.test.tsx | 239 +++++++++++++ .../components/charts/chart-tooltip.test.ts | 8 +- apps/sim/components/charts/line-chart.tsx | 24 +- apps/sim/components/charts/radar-chart.tsx | 335 ++++++++++-------- .../ee/audit-logs/components/audit-logs.tsx | 44 ++- .../ee/audit-logs/hooks/audit-logs.test.tsx | 6 +- apps/sim/ee/audit-logs/hooks/audit-logs.ts | 8 +- .../components/usage-consumers.test.ts | 6 +- .../components/usage-consumers.tsx | 53 ++- .../components/usage-monitoring.tsx | 62 ++-- .../components/usage-source-mix.tsx | 40 ++- .../components/usage-summary.tsx | 2 - apps/sim/ee/organization-usage/constants.ts | 9 +- .../ee/organization-usage/search-params.ts | 21 +- .../components/session-policy-settings.tsx | 1 - apps/sim/hooks/queries/organization-usage.ts | 12 +- .../queries/utils/organization-usage-keys.ts | 9 +- 23 files changed, 713 insertions(+), 304 deletions(-) create mode 100644 apps/sim/components/charts/chart-layout.test.tsx diff --git a/apps/sim/app/api/audit-logs/export/route.test.ts b/apps/sim/app/api/audit-logs/export/route.test.ts index 6f177ab7b9f..7c0667a9de7 100644 --- a/apps/sim/app/api/audit-logs/export/route.test.ts +++ b/apps/sim/app/api/audit-logs/export/route.test.ts @@ -164,4 +164,29 @@ describe('GET /api/audit-logs/export', () => { expect(response.status).toBe(400) expect(mockQueryAuditLogs).not.toHaveBeenCalled() }) + + /** + * The export has to filter by everything the on-screen feed does. It did not + * forward `workspaceId`, so an admin exporting from a workspace-scoped feed + * downloaded the whole organization — silently, because every field of + * `AuditLogFilterParams` is optional and dropping one still type-checks. + */ + it('forwards the workspace filter the on-screen feed applies', async () => { + mockGetOrgWorkspaceIds.mockResolvedValue(['workspace-1']) + + await GET(makeRequest('?workspaceId=workspace-1')) + + expect(mockBuildFilterConditions).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: 'workspace-1' }) + ) + }) + + it('rejects a workspaceId outside the organization, as the list route does', async () => { + mockGetOrgWorkspaceIds.mockResolvedValue(['workspace-1']) + + const response = await GET(makeRequest('?workspaceId=workspace-elsewhere')) + + expect(response.status).toBe(400) + expect(mockQueryAuditLogs).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/app/api/audit-logs/export/route.ts b/apps/sim/app/api/audit-logs/export/route.ts index c4860a85092..bcf02a37d72 100644 --- a/apps/sim/app/api/audit-logs/export/route.ts +++ b/apps/sim/app/api/audit-logs/export/route.ts @@ -66,8 +66,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { } const { organizationId, orgMemberIds } = authResult.context - const { search, action, resourceType, actorId, startDate, endDate, includeDeparted } = - parsed.data.query + const { actorId, workspaceId, includeDeparted } = parsed.data.query if (actorId && !orgMemberIds.includes(actorId)) { return NextResponse.json( @@ -77,20 +76,34 @@ export const GET = withRouteHandler(async (request: NextRequest) => { } const orgWorkspaceIds = await getOrgWorkspaceIds(organizationId) + /** + * The same refusal `listAuditLogs` gives. The scope predicate already makes an + * out-of-organization id return nothing, but an empty CSV and a 400 that names the + * problem are very different answers to the same bad request, and the two paths + * disagreeing about which one you get is what an audit trail cannot afford. + */ + if (workspaceId && !orgWorkspaceIds.includes(workspaceId)) { + return NextResponse.json( + { error: 'workspaceId does not belong to your organization' }, + { status: 400 } + ) + } const scopeCondition = buildOrgScopeCondition({ organizationId, orgWorkspaceIds, orgMemberIds, includeDeparted, }) - const filterConditions = buildFilterConditions({ - action, - resourceType, - actorId, - search, - startDate, - endDate, - }) + /** + * The whole parsed query, not a hand-listed subset. + * + * Every field of `AuditLogFilterParams` is optional, so dropping one type-checks + * silently — which is how `workspaceId` came to be accepted by the contract, + * honoured by the list route, and ignored here: an admin looking at one + * workspace's feed downloaded the entire organization's, under a truncation + * warning that blamed the date range. + */ + const filterConditions = buildFilterConditions(parsed.data.query) const conditions = [scopeCondition, ...filterConditions] const rows: ReturnType[] = [] diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx index 48b198dc5b0..5914b92e3c8 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/billing/components/usage-limit-field/usage-limit-field.tsx @@ -135,7 +135,6 @@ export function UsageLimitField({ : String(dollarsToCredits(currentLimit)) } disabled={!canEdit} - inputClassName='[appearance:textfield] [&::-webkit-inner-spin-button]:appearance-none [&::-webkit-outer-spin-button]:appearance-none' /> ) diff --git a/apps/sim/components/charts/bar-chart.tsx b/apps/sim/components/charts/bar-chart.tsx index c216c64490f..17acfbe4cb9 100644 --- a/apps/sim/components/charts/bar-chart.tsx +++ b/apps/sim/components/charts/bar-chart.tsx @@ -117,12 +117,9 @@ function BarChartComponent({ const barWidth = Math.max(1, Math.min(24, slot * 0.7)) /** - * The hovered bucket, derived from the stored cursor rather than stored beside it. - * - * Bars own a slot, so this is which slot the cursor is in — not the nearest sample, - * which is how a line chart resolves it. Deriving it means a resize mid-hover cannot - * leave an index that disagrees with the slot geometry it was measured against, and - * a mouse move writes one state instead of two. + * Bars own a slot, so the hovered bucket is which slot the cursor is in — not the + * nearest sample, which is how a line chart resolves it. Derived, so a resize + * mid-hover cannot leave an index disagreeing with the slot geometry. */ const hoverIndex = hoverPos === null || data.length === 0 || slot <= 0 @@ -203,12 +200,6 @@ function BarChartComponent({ contradicted the constant's own note that the chart "scrolls rather than compresses". At or above the floor there is no overflow and nothing changes. */ - /* - `overflow-y-hidden` is not redundant with `overflow-x-auto`: a computed - `overflow-x` other than `visible` promotes `overflow-y: visible` to `auto`, - so the tooltip's shadow reaching the foot of the box raised a vertical - scrollbar over the chart whenever the cursor neared the axis. - */ 'w-full overflow-x-auto overflow-y-hidden', !hasExternalWrapper && 'rounded-lg border bg-[var(--surface-1)] p-4 shadow-card' )} diff --git a/apps/sim/components/charts/chart-geometry.test.ts b/apps/sim/components/charts/chart-geometry.test.ts index 10820754e0d..6108dc3386b 100644 --- a/apps/sim/components/charts/chart-geometry.test.ts +++ b/apps/sim/components/charts/chart-geometry.test.ts @@ -83,19 +83,32 @@ describe('chartPlotBand', () => { }) describe('resolveChartPadding', () => { - /** - * The bug this exists for: a fixed 26px gutter left 18px of drawable width once - * `CHART_AXIS_LABEL_GAP` was taken out, and a right-anchored `7.3k` at 9px is wider - * than that — so its first glyph was cut off at the container's left edge. - */ it('widens the gutter until the longest label fits beside the axis', () => { const { left } = resolveChartPadding(['7.3k', '0']) expect(left).toBeGreaterThanOrEqual(estimateAxisLabelWidth('7.3k') + CHART_AXIS_LABEL_GAP) }) - it('never narrows below the shared padding, so short labels stay aligned', () => { - expect(resolveChartPadding(['0', '0']).left).toBe(CHART_PADDING.left) - expect(resolveChartPadding([]).left).toBe(CHART_PADDING.left) + it('never narrows below the shared padding', () => { + expect(resolveChartPadding(['0', '0']).left).toBeGreaterThanOrEqual(CHART_PADDING.left) + expect(resolveChartPadding([]).left).toBeGreaterThanOrEqual(CHART_PADDING.left) + }) + + /** + * Three charts sit side by side on the logs dashboard. A gutter derived exactly from + * each one's own labels put their plot origins at 26, 27 and 32 — visibly ragged + * across a row that used to share one origin. + */ + it('resolves labels of similar width to the same gutter', () => { + const gutters = [['5'], ['1.2s'], ['12.3k'], ['0'], ['7.3k']].map( + (labels) => resolveChartPadding(labels).left + ) + expect(new Set(gutters).size).toBe(1) + }) + + it('still grows for a genuinely wider label', () => { + expect(resolveChartPadding(['123456.7m']).left).toBeGreaterThan( + resolveChartPadding(['7.3k']).left + ) }) it('leaves the other three sides on the shared constant', () => { @@ -104,10 +117,4 @@ describe('resolveChartPadding', () => { expect(padding.right).toBe(CHART_PADDING.right) expect(padding.bottom).toBe(CHART_PADDING.bottom) }) - - it('grows monotonically with label length', () => { - const short = resolveChartPadding(['1.2k']).left - const long = resolveChartPadding(['123456.7m']).left - expect(long).toBeGreaterThan(short) - }) }) diff --git a/apps/sim/components/charts/chart-geometry.ts b/apps/sim/components/charts/chart-geometry.ts index 1bdfff37b30..3494543c3c2 100644 --- a/apps/sim/components/charts/chart-geometry.ts +++ b/apps/sim/components/charts/chart-geometry.ts @@ -15,16 +15,38 @@ export type ChartPadding = { top: number; right: number; bottom: number; left: n /** Matches the loader placeholders callers size themselves against. */ export const CHART_DEFAULT_HEIGHT = 166 -/** Below this the axis labels collide, so the chart scrolls rather than compresses. */ +/** + * Below this the axis labels collide, so the chart scrolls rather than compresses. + * + * Consumers pair `overflow-x-auto` with `overflow-y-hidden`: a computed `overflow-x` + * other than `visible` promotes `overflow-y: visible` to `auto`, so the tooltip's + * shadow reaching the foot of the box raised a vertical scrollbar over the chart + * whenever the cursor neared the axis. + */ export const CHART_MIN_WIDTH = 280 export const CHART_TICK_FILL = 'var(--text-tertiary)' export const CHART_TICK_FONT_SIZE = 9 export const CHART_GRID_FRACTIONS = [0.25, 0.5, 0.75] as const +/** Punctuation and whitespace, which sit near half the width of a digit or letter. */ +const NARROW_GLYPH = /[.,:\s]/ + /** Gap between a y-axis tick label's right edge and the axis rule. */ export const CHART_AXIS_LABEL_GAP = 8 +/** + * The gutter is rounded up to a multiple of this. + * + * Charts are read side by side — the logs dashboard puts three in one row — and a + * gutter derived exactly from each chart's own labels made `5`, `1.2s` and `12.3k` + * resolve to 26, 27 and 32, so three plots that used to share an origin no longer + * did. Quantizing collapses differences this small to one value while still growing + * for a genuinely wider label, and it turns the sub-pixel slack that `Math.ceil` + * alone left into several pixels. + */ +const CHART_AXIS_GUTTER_STEP = 8 + /** * Rendered width of a right-anchored y-axis tick label. * @@ -37,7 +59,7 @@ export const CHART_AXIS_LABEL_GAP = 8 export function estimateAxisLabelWidth(text: string): number { let width = 0 for (const character of text) { - width += /[.,:\s]/.test(character) ? 0.3 : 0.58 + width += NARROW_GLYPH.test(character) ? 0.3 : 0.58 } return width * CHART_TICK_FONT_SIZE } @@ -54,9 +76,10 @@ export function estimateAxisLabelWidth(text: string): number { */ export function resolveChartPadding(yAxisLabels: readonly string[]): ChartPadding { const widest = yAxisLabels.reduce((max, label) => Math.max(max, estimateAxisLabelWidth(label)), 0) + const required = Math.max(CHART_PADDING.left, widest + CHART_AXIS_LABEL_GAP) return { ...CHART_PADDING, - left: Math.max(CHART_PADDING.left, Math.ceil(widest) + CHART_AXIS_LABEL_GAP), + left: Math.ceil(required / CHART_AXIS_GUTTER_STEP) * CHART_AXIS_GUTTER_STEP, } } diff --git a/apps/sim/components/charts/chart-layout.test.tsx b/apps/sim/components/charts/chart-layout.test.tsx new file mode 100644 index 00000000000..3ab1c4e4bc8 --- /dev/null +++ b/apps/sim/components/charts/chart-layout.test.tsx @@ -0,0 +1,239 @@ +/** + * @vitest-environment jsdom + */ +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { BarChart } from '@/components/charts/bar-chart' +import { CHART_PADDING } from '@/components/charts/chart-geometry' +import { RadarChart } from '@/components/charts/radar-chart' + +/** + * Rendered-geometry guards for the chart family. + * + * These assert against the real SVG the components emit rather than against the + * geometry helpers in isolation: the two clipping bugs this file exists for — a + * y-axis label cut off at the container's left edge, and a radar caption painting + * over the section beside it — were both invisible to a unit test of the maths, + * because each came from a *callsite* combining correct helpers wrongly. + */ + +let container: HTMLDivElement +let root: Root + +/** jsdom lays nothing out, so the width the chart measures has to be supplied. */ +function mountAtWidth(width: number, element: React.ReactElement): SVGSVGElement { + container = document.createElement('div') + document.body.appendChild(container) + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({ + width, + height: 0, + top: 0, + left: 0, + right: width, + bottom: 0, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect) + root = createRoot(container) + act(() => root.render(element)) + const svg = container.querySelector('svg') + if (!svg) throw new Error('chart did not render an svg') + return svg +} + +/** Right-anchored SVG text at 9px, measured the way the chart's own estimator does. */ +function textExtent(text: string): number { + let width = 0 + for (const character of text) width += /[.,:\s]/.test(character) ? 0.3 : 0.58 + return width * 9 +} + +beforeEach(() => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + globalThis.ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + } as unknown as typeof ResizeObserver +}) + +afterEach(() => { + act(() => root?.unmount()) + container?.remove() + vi.restoreAllMocks() +}) + +function dailySeries(count: number, peak: number) { + return Array.from({ length: count }, (_, index) => ({ + timestamp: new Date(Date.UTC(2026, 0, 1 + index)).toISOString(), + value: index === 0 ? peak : peak / 10, + })) +} + +describe('BarChart rendered geometry', () => { + const widths = [280, 420, 680, 1024] + const peaks = [7300, 173_000, 1_234_567] + + it.each(widths.flatMap((width) => peaks.map((peak) => [width, peak] as const)))( + 'keeps the y-axis labels inside the box at width %i, peak %i', + (width, peak) => { + const svg = mountAtWidth( + width, + + ) + const labels = [...svg.querySelectorAll('text')].filter( + (node) => node.getAttribute('text-anchor') === 'end' + ) + expect(labels.length).toBe(2) + for (const label of labels) { + const anchorX = Number(label.getAttribute('x')) + // Right-anchored: the glyphs run leftward from the anchor. + expect(anchorX - textExtent(label.textContent ?? '')).toBeGreaterThanOrEqual(0) + } + } + ) + + it.each(widths)('keeps every bar inside the plot area at width %i', (width) => { + const svg = mountAtWidth( + width, + + ) + const bars = [...svg.querySelectorAll('rect')] + expect(bars.length).toBeGreaterThan(0) + const svgWidth = Number(svg.getAttribute('width')) + for (const bar of bars) { + const x = Number(bar.getAttribute('x')) + const right = x + Number(bar.getAttribute('width')) + expect(x).toBeGreaterThanOrEqual(CHART_PADDING.left) + expect(right).toBeLessThanOrEqual(svgWidth - CHART_PADDING.right + 0.01) + } + }) + + it('keeps the first and last x-axis tick label inside the box', () => { + const width = 680 + const svg = mountAtWidth( + width, + + ) + const ticks = [...svg.querySelectorAll('text')].filter( + (node) => node.getAttribute('text-anchor') === 'middle' + ) + expect(ticks.length).toBeGreaterThan(1) + for (const tick of ticks) { + const centre = Number(tick.getAttribute('x')) + const half = textExtent(tick.textContent ?? '') / 2 + expect(centre - half).toBeGreaterThanOrEqual(0) + expect(centre + half).toBeLessThanOrEqual(width) + } + }) +}) + +describe('RadarChart rendered geometry', () => { + const LONG = 'Knowledge Base Sync' + + /** + * Every caption long, not just the first. + * + * The first axis sits at twelve o'clock, where a caption is centred and has the + * whole half-width to spend — the one position that cannot overflow horizontally. + * A fixture that only made that one long proved nothing about the axes that + * actually run out of room. + */ + function axesOf(count: number) { + return Array.from({ length: count }, (_, index) => ({ + label: `${LONG} ${index}`, + value: 100 * (index + 1), + display: String(100 * (index + 1)), + })) + } + + it.each([ + [280, 3], + [280, 6], + [320, 4], + [420, 5], + [420, 6], + [520, 7], + [680, 6], + ])('keeps every axis caption inside the box at width %i with %i axes', (width, axisCount) => { + const svg = mountAtWidth(width, ) + const height = Number(svg.getAttribute('height')) + const captions = [...svg.querySelectorAll('text')] + expect(captions.length).toBe(axisCount) + + for (const caption of captions) { + const x = Number(caption.getAttribute('x')) + const y = Number(caption.getAttribute('y')) + const anchor = caption.getAttribute('text-anchor') + const extent = textExtent(caption.textContent ?? '') + const left = anchor === 'start' ? x : anchor === 'end' ? x - extent : x - extent / 2 + const right = left + extent + expect(left).toBeGreaterThanOrEqual(0) + expect(right).toBeLessThanOrEqual(width) + + // An 'auto' baseline sits the glyphs above y; 'middle' centres them on it. + const capHeight = 9 + const top = + caption.getAttribute('dominant-baseline') === 'middle' ? y - capHeight / 2 : y - capHeight + const bottom = top + capHeight + expect(top).toBeGreaterThanOrEqual(0) + expect(bottom).toBeLessThanOrEqual(height) + } + }) + + it('draws a positive-radius web rather than collapsing at the narrow floor', () => { + const svg = mountAtWidth(280, ) + const rings = [...svg.querySelectorAll('polygon')].filter( + (node) => node.getAttribute('fill') === 'none' + ) + expect(rings.length).toBeGreaterThan(0) + const outer = rings[rings.length - 1] + const points = (outer.getAttribute('points') ?? '') + .split(' ') + .map((pair) => pair.split(',').map(Number)) + const xs = points.map(([x]) => x) + const ys = points.map(([, y]) => y) + expect(Math.max(...xs) - Math.min(...xs)).toBeGreaterThan(40) + expect(Math.max(...ys) - Math.min(...ys)).toBeGreaterThan(40) + }) + + it('renders the empty state rather than a degenerate polygon below three axes', () => { + container = document.createElement('div') + document.body.appendChild(container) + vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect').mockReturnValue({ + width: 420, + height: 0, + top: 0, + left: 0, + right: 420, + bottom: 0, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect) + root = createRoot(container) + act(() => root.render()) + expect(container.querySelector('svg')).toBeNull() + expect(container.textContent).toContain('No data') + }) +}) diff --git a/apps/sim/components/charts/chart-tooltip.test.ts b/apps/sim/components/charts/chart-tooltip.test.ts index 36bc19467e1..b3b782cf95f 100644 --- a/apps/sim/components/charts/chart-tooltip.test.ts +++ b/apps/sim/components/charts/chart-tooltip.test.ts @@ -26,13 +26,7 @@ function place(anchorY: number, rows = 1, hasDate = true) { } describe('positionChartTooltip', () => { - /** - * The bug this exists for: the vertical clamp was a fixed inset that ignored the - * box's real height, so near the foot of the plot the tooltip hung a pixel or two - * past the bottom. The scroll container sets `overflow-x`, which promotes - * `overflow-y` to `auto`, and those pixels raised a vertical scrollbar over the - * chart the moment the cursor approached the axis. - */ + /** Guards the height-aware vertical clamp — see `positionChartTooltip`. */ it('keeps the whole box inside the chart when the cursor is at the very bottom', () => { const { top, tooltipHeight } = place(HEIGHT) expect(top + tooltipHeight).toBeLessThanOrEqual(HEIGHT) diff --git a/apps/sim/components/charts/line-chart.tsx b/apps/sim/components/charts/line-chart.tsx index 9ce78acb586..0896f1df4ee 100644 --- a/apps/sim/components/charts/line-chart.tsx +++ b/apps/sim/components/charts/line-chart.tsx @@ -192,14 +192,24 @@ function LineChartComponent({ /** * The hovered sample, derived from the stored cursor rather than stored beside it. * - * The x written on hover is the same clamped x the index was resolved from, so a - * second piece of state could only ever agree with this — or go stale after a - * resize, once the width it was measured against had changed. + * Clamped here rather than relying on the stored x having been clamped at mousemove + * time: `padding.left` follows the axis labels and `chartWidth` follows the + * container, so either can move with no pointer event at all — a sidebar collapse + * mid-hover otherwise pushed the ratio past 1 and indexed off the end, and the dot, + * the rule and the tooltip all vanished until the cursor moved again. */ const hoverIndex = hoverPos === null || scaledPoints.length === 0 ? null - : Math.round(((hoverPos.x - padding.left) / (chartWidth || 1)) * (scaledPoints.length - 1)) + : Math.max( + 0, + Math.min( + scaledPoints.length - 1, + Math.round( + ((hoverPos.x - padding.left) / (chartWidth || 1)) * (scaledPoints.length - 1) + ) + ) + ) const scaledSeries = useMemo( () => @@ -280,12 +290,6 @@ function LineChartComponent({ contradicted the constant's own note that the chart "scrolls rather than compresses". At or above the floor there is no overflow and nothing changes. */ - /* - `overflow-y-hidden` is not redundant with `overflow-x-auto`: a computed - `overflow-x` other than `visible` promotes `overflow-y: visible` to `auto`, - so the tooltip's shadow reaching the foot of the box raised a vertical - scrollbar over the chart whenever the cursor neared the axis. - */ 'w-full overflow-x-auto overflow-y-hidden', !hasExternalWrapper && 'rounded-lg border bg-[var(--surface-1)] p-4 shadow-card' )} diff --git a/apps/sim/components/charts/radar-chart.tsx b/apps/sim/components/charts/radar-chart.tsx index 25b26e1a240..0f33a02b783 100644 --- a/apps/sim/components/charts/radar-chart.tsx +++ b/apps/sim/components/charts/radar-chart.tsx @@ -13,6 +13,7 @@ import { ChartTooltipRow, estimateTooltipHeight, estimateTooltipWidth, + positionChartTooltip, } from '@/components/charts/chart-tooltip' import { useChartWidth, @@ -39,13 +40,20 @@ const LABEL_GUTTER = 52 /** Gap between the outer ring and a caption anchored beyond it. */ const LABEL_GAP = 12 +/** + * The web's rings: the family's gridline fractions plus the outer ring, which is this + * chart's axis rule. Read from the constant rather than divided into `RING_COUNT` + * even steps — the arithmetic agreed with the siblings only while the fractions + * happened to be uniform, which is exactly the drift `chart-geometry` exists to stop. + */ +const RING_FRACTIONS = [...CHART_GRID_FRACTIONS, 1] as const + /** * Caption budget. A long source name would otherwise run past the container, and the * svg paints outside its box so it would not even clip — it would overlap the section * beside it. The hover row carries the full name. */ const MAX_LABEL_LENGTH = 16 -const RING_COUNT = CHART_GRID_FRACTIONS.length + 1 /** * Polar coordinates for an axis. `-90°` puts the first axis at twelve o'clock, so a @@ -86,36 +94,41 @@ function RadarChartComponent({ axes, color, height = 200 }: RadarChartProps) { const cx = width / 2 const cy = height / 2 /* - The horizontal budget is the caption's own estimated width — the same + One memo over the whole web: hovering re-renders this component on every wedge + enter and leave, and none of this geometry can move under a hover. Guarding only + the point projection left the costlier half — a per-glyph estimate of every + caption — running on each of those renders. + + The horizontal budget is the caption's own estimated width, the same `estimateAxisLabelWidth` the sibling charts use to size a gutter around SVG text they cannot measure. A 16-glyph caption runs to ~84px, so a fixed inset let every - side caption paint past the container and over the section beside it; the svg is - `overflow-visible`, so nothing clipped it. + side caption run past the plot; budgeting the radius against the real caption + width is what keeps them inside the box the svg clips to. */ - const labelWidth = axes.reduce( - (max, axis) => Math.max(max, estimateAxisLabelWidth(truncate(axis.label, MAX_LABEL_LENGTH))), - 0 - ) - const radius = Math.max( - 0, - Math.min(width / 2 - labelWidth - LABEL_GAP, height / 2 - LABEL_GUTTER / 2) - ) - - const maxValue = Math.max(...axes.map((a) => a.value), 0) - - const points = useMemo( - () => - axes.map((axis, index) => { - const fraction = maxValue > 0 ? axis.value / maxValue : 0 + const { maxValue, radius, points } = useMemo(() => { + const labelWidth = axes.reduce( + (max, axis) => Math.max(max, estimateAxisLabelWidth(truncate(axis.label, MAX_LABEL_LENGTH))), + 0 + ) + const webRadius = Math.max( + 0, + Math.min(width / 2 - labelWidth - LABEL_GAP, height / 2 - LABEL_GUTTER / 2) + ) + const peak = Math.max(...axes.map((axis) => axis.value), 0) + return { + maxValue: peak, + radius: webRadius, + points: axes.map((axis, index) => { + const fraction = peak > 0 ? axis.value / peak : 0 return { axis, - outer: axisPoint(index, axes.length, radius, cx, cy), - value: axisPoint(index, axes.length, radius * fraction, cx, cy), - label: axisPoint(index, axes.length, radius + LABEL_GAP, cx, cy), + outer: axisPoint(index, axes.length, webRadius, cx, cy), + value: axisPoint(index, axes.length, webRadius * fraction, cx, cy), + label: axisPoint(index, axes.length, webRadius + LABEL_GAP, cx, cy), } }), - [axes, maxValue, radius, cx, cy] - ) + } + }, [axes, width, height, cx, cy]) if (containerWidth === null) { return
@@ -141,142 +154,172 @@ function RadarChartComponent({ axes, color, height = 200 }: RadarChartProps) { return ( /* - The same scroll guard as the sibling charts: `useChartWidth` floors the svg at - CHART_MIN_WIDTH, so in a narrower box it is wider than its container. The - captions are inside the box by construction — `radius` is budgeted against - `labelWidth` — so nothing legible is clipped, and `overflow-y-hidden` keeps the - `overflow-x` from promoting the vertical axis to `auto`. + Two boxes, like the siblings: the outer one scrolls, the inner one is the + positioning context. `relative` on the scroll container itself left the + absolutely-positioned tooltip anchored to the viewport of the scroll rather than + to the plot — below CHART_MIN_WIDTH it stayed nailed while the web slid under it. + + Captions are inside the plot by construction, since `radius` is budgeted against + `labelWidth`, so the horizontal scroll never cuts one off. */ -
- - - - - - - +
+
+ + + {/* + Radial rather than the siblings' vertical linear gradient — a shape with + radial symmetry lit from the top reads as a rendering error. The stop + opacities stay in the family's range, and light is the more opaque theme + because dark composites through `screen` below. + */} + + + + + - {/* The web: one ring per grid fraction plus the outer ring, and a spoke per axis. */} - {Array.from({ length: RING_COUNT }, (_, ring) => { - const ringRadius = (radius * (ring + 1)) / RING_COUNT - return ( + {RING_FRACTIONS.map((fraction) => ( axisPoint(index, axes.length, ringRadius, cx, cy)) + axes.map((_, index) => axisPoint(index, axes.length, radius * fraction, cx, cy)) )} fill='none' stroke='var(--border)' - strokeOpacity={ring === RING_COUNT - 1 ? 0.9 : 0.45} + strokeOpacity={fraction === 1 ? 1 : 0.35} strokeWidth='1' /> - ) - })} - {points.map((point, index) => ( - - ))} - - point.value))} - fill={`url(#radar-${uniqueId})`} - stroke={resolvedColor} - strokeWidth='1.5' - strokeLinejoin='round' - /> - - {points.map((point, index) => ( - - ))} - - {points.map((point, index) => ( - cx ? 'start' : 'end' - } - dominantBaseline={Math.abs(point.label.y - cy) < 1 ? 'middle' : 'auto'} - fontSize={CHART_TICK_FONT_SIZE} - fill={CHART_TICK_FILL} - > - {truncate(point.axis.label, MAX_LABEL_LENGTH)} - - ))} + ))} + {points.map((point, index) => ( + + ))} - {/* - Hit targets last so they sit above the painted web. A wedge per axis, drawn - as a transparent triangle from the centre — a vertex-sized target is far too - small to hover on a 200px chart. - */} - {points.map((point, index) => { - const half = Math.PI / axes.length - const angle = (index / axes.length) * Math.PI * 2 - Math.PI / 2 - const reach = radius + LABEL_GUTTER / 2 - const a = { - x: cx + Math.cos(angle - half) * reach, - y: cy + Math.sin(angle - half) * reach, - } - const b = { - x: cx + Math.cos(angle + half) * reach, - y: cy + Math.sin(angle + half) * reach, - } - return ( + setHoverIndex(index)} - onMouseLeave={() => setHoverIndex(null)} + points={polygon(points.map((point) => point.value))} + fill={`url(#radar-${uniqueId})`} + stroke={resolvedColor} + strokeWidth={isDark ? 1.7 : 2} + strokeLinejoin='round' /> - ) - })} - + {points.map((point, index) => ( + + ))} + - {hovered && - (() => { - const value = hovered.axis.display ?? String(hovered.axis.value) - /* - Centred on the chart rather than tracked to the cursor. The wedges all - meet at the centre, so one box there is equidistant from every target and - cannot be pushed outside a chart that — unlike its axis-bearing siblings — - has no padding to clamp against. - */ - const boxWidth = estimateTooltipWidth(Math.max(hovered.axis.label.length, value.length)) - return ( - ( + cx ? 'start' : 'end' + } + dominantBaseline={ + Math.abs(point.label.x - cx) >= 1 + ? 'middle' + : point.label.y > cy + ? 'hanging' + : 'auto' + } + fontSize={CHART_TICK_FONT_SIZE} + fill={CHART_TICK_FILL} > - - - ) - })()} + {truncate(point.axis.label, MAX_LABEL_LENGTH)} + + ))} + + {/* + Hit targets last so they sit above the painted web, and wedge-sized — a + vertex-sized target is far too small to hover on a 200px chart. + + An arc sector, not a triangle. A triangle's far edge is the chord, which + along its own spoke reaches only `reach·cos(π/n)` — at three axes that is + 50px against a 74px radius, so the largest value's vertex, the one a reader + aims at, sat outside its own target and outside every other. Sectors tile + identically and reach `reach` in every direction. The sweep flag is 1 + because SVG's y grows downward, and the arc is never a major one: 2π/n ≤ + 2π/3 < π for the three-or-more axes this chart requires. + */} + {points.map((point, index) => { + const half = Math.PI / axes.length + const angle = (index / axes.length) * Math.PI * 2 - Math.PI / 2 + const reach = radius + LABEL_GUTTER / 2 + const a = { + x: cx + Math.cos(angle - half) * reach, + y: cy + Math.sin(angle - half) * reach, + } + const b = { + x: cx + Math.cos(angle + half) * reach, + y: cy + Math.sin(angle + half) * reach, + } + return ( + setHoverIndex(index)} + onMouseLeave={() => setHoverIndex(null)} + /> + ) + })} + + + {hovered && + (() => { + const value = hovered.axis.display ?? String(hovered.axis.value) + /* + Beside the hovered vertex, through the same placer the siblings use, so + the box flips and clamps identically. Centring it on the web instead put + a filled panel over the densest part of the gradient — the concentration + this chart exists to show. The padding passed is the caption gap rather + than the axis-bearing charts' gutters: a radar has no axis rules to keep + clear of. + */ + const { left, top } = positionChartTooltip({ + anchorX: hovered.value.x, + anchorY: hovered.value.y, + width, + height, + tooltipMaxWidth: estimateTooltipWidth( + Math.max(hovered.axis.label.length, value.length) + ), + tooltipHeight: estimateTooltipHeight(1, true), + padding: { top: 0, right: LABEL_GAP, bottom: 0, left: LABEL_GAP }, + }) + return ( + + + + ) + })()} +
) } diff --git a/apps/sim/ee/audit-logs/components/audit-logs.tsx b/apps/sim/ee/audit-logs/components/audit-logs.tsx index 82166da1b65..386101a979f 100644 --- a/apps/sim/ee/audit-logs/components/audit-logs.tsx +++ b/apps/sim/ee/audit-logs/components/audit-logs.tsx @@ -270,10 +270,6 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { const [searchTerm, setSearchTerm] = useSettingsSearch() const debouncedSearch = useDebounce(searchTerm, SEARCH_DEBOUNCE_MS).trim() const [isVisuallyRefreshing, setIsVisuallyRefreshing] = useState(false) - /* - Lazy-init: `useRef(new Set())` allocates a fresh Set on every render and throws - all but the first away. - */ const refreshTimersRef = useRef | null>(null) refreshTimersRef.current ??= new Set() const refreshTimers = refreshTimersRef.current @@ -285,22 +281,18 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { } }, [refreshTimers]) - const filters = useMemo(() => { - return { - search: debouncedSearch || undefined, - resourceType: selectedTypes.length > 0 ? selectedTypes.join(',') : undefined, - workspaceId: filteredWorkspace?.id, - startDate: getStartDateFromTimeRange(timeRange, customStartDate)?.toISOString(), - endDate: getEndDateFromTimeRange(timeRange, customEndDate)?.toISOString(), - } - }, [ - debouncedSearch, - selectedTypes, - filteredWorkspace?.id, - timeRange, - customStartDate, - customEndDate, - ]) + /* + Not memoized: this object is only ever hashed, never compared by identity — React + Query hashes a query key structurally, and the export handler reads its fields + directly. The same rule `useUsageWindow` applies to its window object. + */ + const filters: AuditLogFilters = { + search: debouncedSearch || undefined, + resourceType: selectedTypes.length > 0 ? selectedTypes.join(',') : undefined, + workspaceId: filteredWorkspace?.id, + startDate: getStartDateFromTimeRange(timeRange, customStartDate)?.toISOString(), + endDate: getEndDateFromTimeRange(timeRange, customEndDate)?.toISOString(), + } /** * A deep-linked workspace scope is only resolvable once the organization's workspace @@ -449,14 +441,20 @@ export function AuditLogs({ organizationId }: AuditLogsProps) { /* A deep-linked scope, not a picker: the organization can hold hundreds of workspaces, so this narrows the feed only when a link asks it to and - offers exactly one action — take it back off. + offers exactly one action — take it back off. Trailing `X` and a bounded + width, matching the app's other removable filter chips; the label names + the dimension because a bare workspace name gives no clue what it scopes. */ void setUrlFilters({ workspace: null })} aria-label={`Clear the ${filteredWorkspace.name} workspace filter`} + className='max-w-[280px] shrink-0' > - {filteredWorkspace.name} + )}
diff --git a/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx b/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx index 8a26c7e1c5a..f06a8ff7075 100644 --- a/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx +++ b/apps/sim/ee/audit-logs/hooks/audit-logs.test.tsx @@ -137,11 +137,7 @@ describe('useAuditLogs identity transitions', () => { ) }) - /** - * The other half of the same rule. Blanking the feed on every filter change is what - * the placeholder exists to stop — and it is why the Export action's - * `isPlaceholderData` guard means anything. - */ + /** Blanking the feed on each keystroke is what the placeholder exists to stop. */ it('holds the current entries while a filter change loads, within one organization', async () => { const filteredPage = createDeferred() mockRequestJson.mockImplementation( diff --git a/apps/sim/ee/audit-logs/hooks/audit-logs.ts b/apps/sim/ee/audit-logs/hooks/audit-logs.ts index 114ac66aad4..20a2d16dcd7 100644 --- a/apps/sim/ee/audit-logs/hooks/audit-logs.ts +++ b/apps/sim/ee/audit-logs/hooks/audit-logs.ts @@ -11,8 +11,12 @@ export const auditLogKeys = { [...auditLogKeys.lists(), organizationId, filters] as const, } -/** Position of the organization id in a key built by {@link auditLogKeys.list}. */ -const AUDIT_LOG_KEY_ORGANIZATION_INDEX = 2 +/** + * Position of the organization id in a key built by {@link auditLogKeys.list} — + * derived from the factory rather than restated, so a new prefix segment cannot + * silently point this at the wrong element. + */ +const AUDIT_LOG_KEY_ORGANIZATION_INDEX = auditLogKeys.lists().length export interface AuditLogFilters { search?: string diff --git a/apps/sim/ee/organization-usage/components/usage-consumers.test.ts b/apps/sim/ee/organization-usage/components/usage-consumers.test.ts index 2060081e8b2..510d065a2a2 100644 --- a/apps/sim/ee/organization-usage/components/usage-consumers.test.ts +++ b/apps/sim/ee/organization-usage/components/usage-consumers.test.ts @@ -6,11 +6,7 @@ import { USAGE_PROVIDER_ICON_IDS } from '@/ee/organization-usage/components/usag import { PROVIDER_DEFINITIONS } from '@/providers/models' describe('PROVIDER_ICONS', () => { - /** - * The breakdown keys model rows by `getProviderFromModel`, which only ever returns - * a registry provider id. A gap here is silent — the row simply renders without a - * mark, which is how GLM shipped as the one iconless model in the list. - */ + /** A gap is silent: the row simply renders with no mark. */ it('covers every provider the model registry defines', () => { const covered = new Set(USAGE_PROVIDER_ICON_IDS) const missing = Object.keys(PROVIDER_DEFINITIONS).filter((id) => !covered.has(id)) diff --git a/apps/sim/ee/organization-usage/components/usage-consumers.tsx b/apps/sim/ee/organization-usage/components/usage-consumers.tsx index 565694d5eea..15a7a33d480 100644 --- a/apps/sim/ee/organization-usage/components/usage-consumers.tsx +++ b/apps/sim/ee/organization-usage/components/usage-consumers.tsx @@ -1,7 +1,7 @@ 'use client' import type { ComponentType } from 'react' -import { chipHoverSurfaceClass, cn } from '@sim/emcn' +import { cn, disclosureChevronClass } from '@sim/emcn' import { ArrowRight, ChevronDown } from '@sim/emcn/icons' import { formatChartCompactNumber } from '@/components/charts' import { @@ -92,6 +92,12 @@ interface UsageConsumerRowProps { showTokensOnly: boolean onSelect?: (row: OrganizationUsageBreakdownRow) => void actions?: RowAction[] + /** + * Width of the affordance some other row in this list carries, reserved here so + * every figure stays in one column — including when the only row that carries one + * is `Other`. + */ + reservedTrailing?: string } /** @@ -99,17 +105,31 @@ interface UsageConsumerRowProps { * same slot on its `Other` row and keep every figure in one column. */ const TRAILING_SLOT_CLASSES = { - /** The canonical resource-row arrow, at its own size. */ arrow: 'size-4', /** `RowActionsMenu`'s trigger: a 14px glyph in a `chipVariants()` pill. */ menu: 'size-[30px]', + /** The disclosure chevron on an expandable `Other` row, at the default icon size. */ + disclosure: 'size-[14px]', } as const +/** + * Geometry of the bespoke tabular usage row — the sanctioned exception to + * `SettingsResourceRow` in `sim-settings-pages.md`. One definition, so the breakdown + * rows, the `Other` row, and the events ledger cannot drift apart. + */ +export const USAGE_ROW_CLASSES = 'flex w-full items-center gap-2.5 rounded-lg p-2 text-left' + /** * A tabular row, not `SettingsResourceRow` — tabular columns are the sanctioned * exception in `sim-settings-pages.md`, alongside billing invoices and credit usage. */ -function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsumerRowProps) { +function UsageConsumerRow({ + row, + showTokensOnly, + onSelect, + actions, + reservedTrailing, +}: UsageConsumerRowProps) { const ProviderIcon = row.providerId ? PROVIDER_ICONS[row.providerId] : undefined const Row = onSelect ? 'button' : 'div' @@ -123,8 +143,8 @@ function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsu } : {})} className={cn( - 'flex w-full items-center gap-2.5 rounded-lg p-2 text-left', - onSelect && cn('transition-colors', chipHoverSurfaceClass) + USAGE_ROW_CLASSES, + onSelect && 'transition-colors hover-hover:bg-[var(--surface-active)]' )} > {ProviderIcon && ( @@ -148,6 +168,8 @@ function UsageConsumerRow({ row, showTokensOnly, onSelect, actions }: UsageConsu ) : actions?.length ? ( + ) : reservedTrailing ? ( +