Skip to content

Commit 54e3f27

Browse files
icecrasher321claude
andcommitted
fix(usage): correct reconciliation, gating, and error classification
Second review round. The findings that were real, and what changed. Billing aggregates. `getBillingPeriodWorkflowRunCount` counts distinct workflow executions with no category predicate, and its own contract says executions with no billable usage are excluded. A BYOK-only run with a zero base charge writes nothing but a `model_unbilled` row, so it would newly appear in a figure that feeds the enterprise billing preview; the count now excludes unbilled categories. `recordUsage` also admits an unbilled entry only at exactly zero cost — the whole safety argument for the category is that every aggregate is `SUM(cost)`. Reconciliation. Breakdown rows and their remainder were each rounded to credits independently and compared against a separately rounded total, so with sub-credit fractions they could not add up — which is precisely what the `Other` row exists to prevent. They now go through one `apportionCredits` pass. The event list and the CSV also counted a row stamped exactly on the window end, which the summary excluded, because the ledger filter is `lte` while an analytics window is half-open; `endDateExclusive` makes the two agree. BYOK ranking. The tab is denominated in tokens and every row costs zero, so ranking by cost fell through to an alphabetical tiebreak — the "top providers" were whichever sorted first, and the hidden tail's tokens were dropped entirely. It ranks by tokens, and the remainder carries its own token total. Gating. The usage entry carried `hideWhenBillingDisabled` copied from Members, but the sidebar applies that filter before it consults `selfHostedOverride`, so it hid the section from exactly the deployment the override exists to serve. Members can carry the flag because it has no override to reach. Error classification. An over-long custom range threw past the orchestration policy and answered 500 on all four routes. A shared policy maps it to 400, the export route makes the same classification in its catch, and the picker now refuses the range up front rather than committing one the API will reject. Also: the segmented meter's overage tone was unreachable (both counts clamped to `segments`, so the comparison could never hold) and now scales both against `max(total, used)`; the allowance is only compared against the current period, since a rolling window can exceed a limit neither period did; `source` accepts a scalar and validates against the source enum instead of an unchecked cast; the bar chart's axis tick uses the same unit-aware formatter as its tooltip; and `assertValidTimezone` strips control characters before echoing a rejected value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 89c238d commit 54e3f27

19 files changed

Lines changed: 261 additions & 36 deletions

File tree

apps/sim/app/api/organizations/[id]/usage/breakdown/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { getOrganizationUsageBreakdownContract } from '@/lib/api/contracts/organization-usage'
22
import {
33
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
54
internalRateLimits,
65
internalSessionAuth,
76
} from '@/lib/api/server/routes'
87
import { getOrganizationUsageBreakdown } from '@/lib/billing/application/organization-usage/get-organization-usage-breakdown'
98
import { organizationUsageOperations } from '@/lib/billing/application/organization-usage/operations'
9+
import { organizationUsageErrorPolicy } from '@/app/api/organizations/[id]/usage/error-policy'
1010

1111
export const dynamic = 'force-dynamic'
1212

@@ -23,7 +23,7 @@ export const GET = defineInternalJsonRoute({
2323
reason:
2424
'Authenticated org-admin settings read, gated on enterprise entitlement and billing authority',
2525
}),
26-
errorPolicy: internalOrchestrationErrorPolicy,
26+
errorPolicy: organizationUsageErrorPolicy,
2727
mapInput: ({ params, query }) => ({
2828
organizationId: params.id,
2929
dimension: query.dimension,
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import {
2+
extendInternalErrorPolicy,
3+
internalErrorResponse,
4+
internalOrchestrationErrorPolicy,
5+
} from '@/lib/api/server/routes'
6+
import { UsageWindowRangeTooLargeError } from '@/lib/billing/core/usage-analytics'
7+
8+
/**
9+
* The window resolver throws when a custom range exceeds its cap, which is a
10+
* caller-fixable input error rather than a fault. Without this it fell through to
11+
* the orchestration policy's `unhandled` branch and every over-long range answered
12+
* `500 Internal server error`, so the client could neither surface the real reason
13+
* nor tell the two apart.
14+
*
15+
* Shared by all four usage routes so they cannot classify the same throw differently.
16+
*/
17+
export const organizationUsageErrorPolicy = extendInternalErrorPolicy(
18+
internalOrchestrationErrorPolicy,
19+
(error) =>
20+
error instanceof UsageWindowRangeTooLargeError
21+
? internalErrorResponse(400, { error: error.message })
22+
: null
23+
)

apps/sim/app/api/organizations/[id]/usage/events/route.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
import { listOrganizationUsageEventsContract } from '@/lib/api/contracts/organization-usage'
22
import {
33
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
54
internalRateLimits,
65
internalSessionAuth,
76
} from '@/lib/api/server/routes'
87
import { listOrganizationUsageEvents } from '@/lib/billing/application/organization-usage/list-organization-usage-events'
98
import { organizationUsageOperations } from '@/lib/billing/application/organization-usage/operations'
10-
import type { InternalUsageLogSource } from '@/lib/billing/usage-sources'
9+
import { organizationUsageErrorPolicy } from '@/app/api/organizations/[id]/usage/error-policy'
1110

1211
export const dynamic = 'force-dynamic'
1312

@@ -24,13 +23,13 @@ export const GET = defineInternalJsonRoute({
2423
reason:
2524
'Authenticated org-admin settings read, gated on enterprise entitlement and billing authority',
2625
}),
27-
errorPolicy: internalOrchestrationErrorPolicy,
26+
errorPolicy: organizationUsageErrorPolicy,
2827
mapInput: ({ params, query }) => ({
2928
organizationId: params.id,
3029
preset: query.preset,
3130
startDate: query.startDate ? new Date(query.startDate) : undefined,
3231
endDate: query.endDate ? new Date(query.endDate) : undefined,
33-
source: query.source as InternalUsageLogSource[] | undefined,
32+
source: query.source,
3433
limit: query.limit,
3534
cursor: query.cursor,
3635
}),

apps/sim/app/api/organizations/[id]/usage/export/route.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import {
88
exportOrganizationUsageEvents,
99
type OrganizationUsageExportRow,
1010
} from '@/lib/billing/application/organization-usage/export-organization-usage-events'
11+
import { UsageWindowRangeTooLargeError } from '@/lib/billing/core/usage-analytics'
1112
import { formatCreditsLabel } from '@/lib/billing/credits/conversion'
12-
import type { InternalUsageLogSource } from '@/lib/billing/usage-sources'
1313
import { ForbiddenOperationError } from '@/lib/core/application'
1414
import { formatCsvValue, toCsvRow } from '@/lib/core/utils/csv'
1515
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -59,7 +59,7 @@ export const GET = withRouteHandler(async (request: NextRequest, context) => {
5959
preset: query.preset,
6060
startDate: query.startDate ? new Date(query.startDate) : undefined,
6161
endDate: query.endDate ? new Date(query.endDate) : undefined,
62-
source: query.source as InternalUsageLogSource[] | undefined,
62+
source: query.source,
6363
},
6464
})
6565

@@ -75,6 +75,11 @@ export const GET = withRouteHandler(async (request: NextRequest, context) => {
7575
if (error instanceof ForbiddenOperationError) {
7676
return NextResponse.json({ error: error.message }, { status: 403 })
7777
}
78+
// A range over the cap is the caller's input, not a fault — the same
79+
// classification the three JSON routes make through `organizationUsageErrorPolicy`.
80+
if (error instanceof UsageWindowRangeTooLargeError) {
81+
return NextResponse.json({ error: error.message }, { status: 400 })
82+
}
7883
logger.error('Failed to export organization usage', { error: getErrorMessage(error) })
7984
return NextResponse.json({ error: 'Failed to export usage' }, { status: 500 })
8085
}

apps/sim/app/api/organizations/[id]/usage/summary/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { getOrganizationUsageSummaryContract } from '@/lib/api/contracts/organization-usage'
22
import {
33
defineInternalJsonRoute,
4-
internalOrchestrationErrorPolicy,
54
internalRateLimits,
65
internalSessionAuth,
76
} from '@/lib/api/server/routes'
87
import { getOrganizationUsageSummary } from '@/lib/billing/application/organization-usage/get-organization-usage-summary'
98
import { organizationUsageOperations } from '@/lib/billing/application/organization-usage/operations'
9+
import { organizationUsageErrorPolicy } from '@/app/api/organizations/[id]/usage/error-policy'
1010

1111
export const dynamic = 'force-dynamic'
1212

@@ -23,7 +23,7 @@ export const GET = defineInternalJsonRoute({
2323
reason:
2424
'Authenticated org-admin settings read, gated on enterprise entitlement and billing authority',
2525
}),
26-
errorPolicy: internalOrchestrationErrorPolicy,
26+
errorPolicy: organizationUsageErrorPolicy,
2727
mapInput: ({ params, query }) => ({
2828
organizationId: params.id,
2929
preset: query.preset,

apps/sim/app/workspace/[workspaceId]/settings/[section]/page.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ vi.mock('@/components/settings/navigation', () => ({
5252
sessions: 'sessions',
5353
'data-retention': 'data-retention',
5454
'data-drains': 'data-drains',
55+
usage: 'usage',
5556
whitelabeling: 'whitelabeling',
5657
},
5758
workspaceSectionUsesPermissionConfig: vi.fn((section: string) =>

apps/sim/app/workspace/[workspaceId]/settings/components/segmented-meter/segmented-meter.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,16 @@ interface SegmentedMeterProps {
2121
* affordance answering the same question, and previously only one existed.
2222
*/
2323
export function SegmentedMeter({ used, total, segments, className }: SegmentedMeterProps) {
24-
const filledSegments =
25-
total > 0 ? Math.min(segments, Math.round((used / total) * segments)) : used > 0 ? segments : 0
26-
const allowedSegments = total > 0 ? segments : 0
24+
/**
25+
* Both counts are measured against the larger of the two, so an overage has
26+
* somewhere to render. Scaling the fill by `total` and clamping it meant
27+
* `filledSegments` and the allowance were both `segments` whenever usage exceeded
28+
* the limit, and the overage tone below could never be reached — the meter simply
29+
* showed full.
30+
*/
31+
const scale = Math.max(total, used)
32+
const filledSegments = scale > 0 ? Math.min(segments, Math.round((used / scale) * segments)) : 0
33+
const allowedSegments = scale > 0 ? Math.round((total / scale) * segments) : 0
2734

2835
return (
2936
<div className={cn('flex items-center gap-1', className)} aria-hidden='true'>

apps/sim/components/charts/bar-chart.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,9 @@ function BarChartComponent({
286286
fontSize={CHART_TICK_FONT_SIZE}
287287
fill={CHART_TICK_FILL}
288288
>
289-
{formatChartCompactNumber(maxValue)}
289+
{/* Same formatter the tooltip uses, or the axis and the hover disagree
290+
about what the numbers mean on any non-`credits` unit. */}
291+
{formatValue(maxValue)}
290292
</text>
291293
<text
292294
x={padding.left - 8}

apps/sim/components/settings/navigation.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,16 @@ export const SETTINGS_SECTION_REGISTRY: readonly SettingsSectionRegistryEntry[]
504504
description: 'Monitor credit usage across your organization.',
505505
group: 'organization',
506506
order: 1,
507-
hideWhenBillingDisabled: true,
507+
/**
508+
* Deliberately no `hideWhenBillingDisabled`, unlike Members above.
509+
*
510+
* The sidebar applies that filter *before* it consults `selfHostedOverride`,
511+
* so pairing the two hid this section from exactly the deployment the
512+
* override exists to serve: self-hosted, billing off, `USAGE_MONITORING_ENABLED`
513+
* on. Members can carry the flag because it has no override to reach. Here the
514+
* two gates below already answer both cases — hosted needs the plan, and
515+
* self-hosted needs the flag.
516+
*/
508517
requiresHosted: true,
509518
requiresEnterprise: true,
510519
selfHostedOverride: SETTINGS_SELF_HOSTED_OVERRIDES.usageMonitoring,

apps/sim/ee/organization-usage/components/usage-consumers.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,9 @@ export function UsageConsumers({
181181
{`Other (${breakdown.other.rowCount} more)`}
182182
</span>
183183
<span className='w-[72px] flex-shrink-0 text-right text-[var(--text-muted)] text-caption tabular-nums'>
184-
{showTokensOnly ? '—' : breakdown.other.credits.toLocaleString()}
184+
{showTokensOnly
185+
? formatChartCompactNumber(breakdown.other.tokens)
186+
: breakdown.other.credits.toLocaleString()}
185187
</span>
186188
{trailingSlot && (
187189
<span className={cn(trailingSlot, 'flex-shrink-0')} aria-hidden='true' />

0 commit comments

Comments
 (0)