Skip to content

Commit 887e889

Browse files
icecrasher321claude
andcommitted
fix(usage): drop inline workspace attachment, tighten window and chart edges
Removes the workspace attachment from admin organization creation, on the maintainer's call. Three review rounds went into its failure semantics, and the conclusion each time was that it cannot be made honest inline: it runs its own transaction under a lock order that exists to avoid deadlocking against invitation acceptance, so it could only ever be best-effort after the organization committed. This codebase already solves the problem properly. `AdminMemberOperationView` tracks workspace moves with `pending | processing | dead_letter | applied` and per-workspace retry, and the enterprise-owner-claim path creates the workspace and the organization in one transaction and enqueues an outbox event for the rest. Provisioning belongs on one of those, not inline in a create call that also relocates the owner's billing payer as a side effect. The endpoint is back to creating an organization and its owner membership, and the contract says why. The rest are edges found in review: - A zero bucket rendered as a 3px colored bar. `chartPlotBand` clamps drawn geometry off the axis rule, but applied to zero it floored every densified empty day at the band — the opposite of what densifying zeros is for. - `preset=custom` without both dates fell back to the raw period, bypassing the unbounded-period bound added last round. It now recurses through `current-period` so there is one rule rather than two copies. - `Date.parse` accepts `2026-02-30` and rolls it forward, so a February request silently returned a window starting March 2. Dates now round-trip. - `?limit=` coerced to `0` and answered 400 instead of using the declared default. - The period picker bound to the raw URL preset, so a partial custom deep link read "Custom range" over current-period data — and suppressed the allowance that was exactly comparable to it. - The CSV rendered a sub-credit charge as the string "0 credits", losing it, and left the column unsummable. Export rows now carry unrounded credits and the CSV writes a bare number. Also regenerates the docs manifest, which CI flagged for the new docs page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e8cbcc5 commit 887e889

12 files changed

Lines changed: 115 additions & 97 deletions

File tree

apps/docs/content/docs/en/platform/enterprise/usage-tracking.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ The **BYOK** tab groups this usage by provider and reports **tokens** rather tha
102102

103103
## Exporting
104104

105-
**Export** downloads the events behind the current period and filters as a CSV with columns `Date, Source, Description, Workflow, Credits`.
105+
**Export** downloads the events behind the current period and filters as a CSV with columns `Date, Source, Description, Workflow, Credits`. Credits are exported as plain numbers so the column can be summed, and carry decimals — an individual event often costs a fraction of a credit.
106106

107107
**All events** opens the full ledger — every credit-consuming event, newest first, with its own filters and export.
108108

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import {
1212
UsageWindowRangeInvertedError,
1313
UsageWindowRangeTooLargeError,
1414
} from '@/lib/billing/core/usage-analytics'
15-
import { formatCreditsLabel } from '@/lib/billing/credits/conversion'
1615
import { ForbiddenOperationError } from '@/lib/core/application'
1716
import { formatCsvValue, toCsvRow } from '@/lib/core/utils/csv'
1817
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -21,14 +20,25 @@ const logger = createLogger('OrganizationUsageExportAPI')
2120

2221
const CSV_HEADER = toCsvRow(['Date', 'Source', 'Description', 'Workflow', 'Credits'])
2322

23+
/**
24+
* A bare number, to four decimals, with trailing zeros trimmed.
25+
*
26+
* Not `formatCreditsLabel`: that renders `"0 credits"` for any charge under half a
27+
* credit, so a real cost vanished from the export, and the `"N credits"` text made
28+
* the column unsummable in a spreadsheet — which is most of what a CSV is for.
29+
*/
30+
function formatExportCredits(credits: number): string {
31+
return String(Number(credits.toFixed(4)))
32+
}
33+
2434
/** `formatCsvValue` neutralizes formula injection — model and workflow names are user-controlled. */
2535
function toCsvLine(row: OrganizationUsageExportRow): string {
2636
return toCsvRow([
2737
formatCsvValue(row.createdAt),
2838
formatCsvValue(row.source),
2939
formatCsvValue(row.description),
3040
formatCsvValue(row.workflowName ?? ''),
31-
formatCsvValue(formatCreditsLabel(row.credits)),
41+
formatCsvValue(formatExportCredits(row.credits)),
3242
])
3343
}
3444

apps/sim/app/api/v1/admin/organizations/route.ts

Lines changed: 6 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,17 @@
1818
* - slug: string - Organization slug (optional, auto-generated from name if not provided)
1919
* - ownerId: string - User ID of the organization owner (required)
2020
*
21-
* Response: AdminSingleResponse<AdminOrganization & { memberId, attachedWorkspaceIds, workspaceAttachmentFailed }>
22-
* `attachedWorkspaceIds` reports which of the owner's workspaces moved under the
23-
* organization, and `workspaceAttachmentFailed` distinguishes an owner who had
24-
* none from an attachment that threw after the organization was committed — the
25-
* latter leaves the organization unreachable and is worth retrying.
21+
* Response: AdminSingleResponse<AdminOrganization & { memberId: string }>
22+
*
23+
* Creates the organization and its owner membership, and nothing else. Attaching
24+
* or creating a workspace for it is deliberately not done here — see the note on
25+
* `adminV1CreateOrganizationContract`.
2626
*/
2727

2828
import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
2929
import { db, dbReplica } from '@sim/db'
3030
import { member, organization, organizationColumns, user } from '@sim/db/schema'
3131
import { createLogger } from '@sim/logger'
32-
import { getErrorMessage } from '@sim/utils/errors'
3332
import { slugify } from '@sim/utils/string'
3433
import { count, eq } from 'drizzle-orm'
3534
import {
@@ -43,7 +42,6 @@ import {
4342
OrganizationSlugTakenError,
4443
} from '@/lib/billing/organizations/create-organization'
4544
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
46-
import { attachOwnedWorkspacesToOrganization } from '@/lib/workspaces/organization-workspaces'
4745
import { withAdminAuth } from '@/app/api/v1/admin/middleware'
4846
import {
4947
adminInvalidJsonResponse,
@@ -156,53 +154,6 @@ export const POST = withRouteHandler(
156154
slug,
157155
})
158156

159-
/**
160-
* Organization settings are reached only through a workspace the organization
161-
* owns, so an organization that owns none leaves its own admin with no route
162-
* to administer it. Attaching the owner's existing workspaces is what makes the
163-
* organization reachable, and matches what `POST /api/organizations` already
164-
* does — this path was the inconsistent one.
165-
*
166-
* An owner with no workspaces has nothing to attach and stays unreachable until
167-
* one exists. Creating a workspace only closes that gap once the organization
168-
* carries a usable Team/Enterprise plan; without one the creation policy still
169-
* resolves to a personal workspace.
170-
*
171-
* Attachment is a follow-on effect, not part of creating the organization, and
172-
* it is deliberately not folded into the creation transaction: it runs its own,
173-
* under a documented lock order (invitation scope, then organization, then
174-
* workspace rows) that exists to avoid deadlocking against invitation
175-
* acceptance. Re-deriving that ordering in a route is how a deadlock ships.
176-
*
177-
* So its failure must not be reported as a failure to create: the organization
178-
* is already committed, and answering 500 for state that exists left the retry
179-
* blocked by the existing-membership check above, with no way to reach the
180-
* organization at all.
181-
*
182-
* It must not be reported as an unqualified success either. Both the ids that
183-
* moved and whether the attach threw are on the response contract and on the
184-
* audit event, because an empty list alone cannot distinguish an owner who had
185-
* no workspaces from provisioning that is genuinely incomplete — only the
186-
* second is worth retrying, and only the caller can decide to.
187-
*/
188-
let attachedWorkspaceIds: string[] = []
189-
let workspaceAttachmentFailed = false
190-
try {
191-
;({ attachedWorkspaceIds } = await attachOwnedWorkspacesToOrganization({
192-
ownerUserId: ownerId,
193-
organizationId,
194-
externalMemberPolicy: 'keep-external',
195-
includeArchived: true,
196-
}))
197-
} catch (attachError) {
198-
workspaceAttachmentFailed = true
199-
logger.error('Admin API: Created organization but could not attach its workspaces', {
200-
organizationId,
201-
ownerId,
202-
error: getErrorMessage(attachError),
203-
})
204-
}
205-
206157
const [createdOrg] = await db
207158
.select(organizationColumns)
208159
.from(organization)
@@ -214,7 +165,6 @@ export const POST = withRouteHandler(
214165
slug,
215166
ownerId,
216167
memberId,
217-
attachedWorkspaceIds,
218168
})
219169

220170
recordAudit({
@@ -225,21 +175,13 @@ export const POST = withRouteHandler(
225175
resourceId: organizationId,
226176
resourceName: name,
227177
description: `Admin API created organization "${name}"`,
228-
metadata: {
229-
slug,
230-
ownerId,
231-
memberId,
232-
attachedWorkspaceIds,
233-
...(workspaceAttachmentFailed ? { workspaceAttachmentFailed: true } : {}),
234-
},
178+
metadata: { slug, ownerId, memberId },
235179
request,
236180
})
237181

238182
return singleResponse({
239183
...toAdminOrganization(createdOrg),
240184
memberId,
241-
attachedWorkspaceIds,
242-
workspaceAttachmentFailed,
243185
})
244186
} catch (error) {
245187
if (error instanceof OrganizationSlugInvalidError) {

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,19 @@ function BarChartComponent({
106106
const x = padding.left + slot * index + (slot - barWidth) / 2
107107
const rawY = padding.top + chartHeight - (point.value / maxValue) * chartHeight
108108
const y = Math.max(yMin, Math.min(yMax, rawY))
109-
return { x, y, height: Math.max(0, height - padding.bottom - y), point }
109+
return {
110+
x,
111+
y,
112+
/*
113+
* A zero bucket draws nothing. The clamp above keeps a *drawn* bar off the
114+
* axis rule, but applied to zero it floored the bar at the 3px band and
115+
* every empty day rendered as a small amount of usage — the densified zeros
116+
* this chart exists to show honestly. Only the track represents an empty
117+
* bucket.
118+
*/
119+
height: point.value > 0 ? Math.max(0, height - padding.bottom - y) : 0,
120+
point,
121+
}
110122
}),
111123
[data, slot, barWidth, maxValue, chartHeight, height, padding.left, padding.top, yMin, yMax]
112124
)

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,12 @@ export function UsageMonitoring({ organizationId, workspaceId }: UsageMonitoring
280280
options={PERIOD_OPTIONS}
281281
value={preset}
282282
onChange={handlePeriodChange}
283+
/*
284+
The visible layer is masked, so the interactive layer owns the one
285+
reachable tooltip — a custom range's label truncates, and without
286+
`overlayLabel` its full value was unreadable.
287+
*/
288+
overlayLabel={periodLabel}
283289
overlayContent={
284290
<span className='truncate text-[var(--text-primary)]'>{periodLabel}</span>
285291
}

apps/sim/ee/organization-usage/hooks/use-usage-window.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,13 @@ export function useUsageWindow() {
4646
window,
4747
tab: state.tab,
4848
workspace: state.workspace,
49-
preset: state.preset,
49+
/**
50+
* The *resolved* preset, not the raw URL value. A partial custom deep link
51+
* queries the current period, so surfacing `state.preset` left the picker
52+
* reading "Custom range" over data that was not custom — and the allowance
53+
* gate, which keys on `current-period`, disagreed with the window too.
54+
*/
55+
preset,
5056
startDate: state.startDate,
5157
endDate: state.endDate,
5258
periodLabel,

apps/sim/lib/api/contracts/organization-usage.ts

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,41 @@ export type UsageBreakdownDimension = z.output<typeof usageBreakdownDimensionSch
4040
*/
4141
export const MAX_CUSTOM_RANGE_DAYS = 92
4242

43+
/**
44+
* A calendar date the window resolver can trust.
45+
*
46+
* `Date.parse` alone accepts `2026-02-30` and rolls it forward, so a request for
47+
* February silently returned a window starting on March 2 — worse than a rejection,
48+
* because the response looks authoritative. The round-trip is what makes the check
49+
* a calendar one: a date that does not survive re-serialization did not exist.
50+
*/
4351
const isoDateSchema = z
4452
.string()
4553
.optional()
46-
.refine((value) => !value || !Number.isNaN(Date.parse(value)), {
47-
message: 'Expected an ISO date such as 2026-08-01',
48-
})
54+
.refine(
55+
(value) => {
56+
if (!value) return true
57+
const parsed = new Date(value)
58+
if (Number.isNaN(parsed.getTime())) return false
59+
// Bare `YYYY-MM-DD` parses as UTC, so compare against the UTC serialization.
60+
return !/^\d{4}-\d{2}-\d{2}$/.test(value) || parsed.toISOString().slice(0, 10) === value
61+
},
62+
{ message: 'Expected a real calendar date such as 2026-08-01' }
63+
)
64+
65+
/**
66+
* A page size that treats an empty or absent parameter as omitted.
67+
*
68+
* `z.coerce.number()` turns `''` into `0`, which then fails `.min(1)` — so a client
69+
* that serializes an unset filter as `?limit=` got a 400 instead of the default the
70+
* schema declares. Explicit numeric values still validate normally.
71+
*/
72+
function usageLimitSchema(max: number, fallback: number) {
73+
return z.preprocess(
74+
(value) => (value === '' || value === null ? undefined : value),
75+
z.coerce.number().int().min(1).max(max).default(fallback)
76+
)
77+
}
4978

5079
/**
5180
* Shared by all four contracts so the four surfaces cannot describe different
@@ -79,7 +108,7 @@ export const organizationUsageBreakdownQuerySchema = organizationUsageWindowQuer
79108
dimension: usageBreakdownDimensionSchema,
80109
/** Narrows the breakdown to one workspace, for the Workspaces drill-down. */
81110
workspaceId: workspaceIdSchema.optional(),
82-
limit: z.coerce.number().int().min(1).max(50).default(10),
111+
limit: usageLimitSchema(50, 10),
83112
})
84113
export type OrganizationUsageBreakdownQuery = z.input<typeof organizationUsageBreakdownQuerySchema>
85114

@@ -100,7 +129,7 @@ const usageLogSourceFilterSchema = z
100129

101130
export const organizationUsageEventsQuerySchema = organizationUsageWindowQuerySchema.extend({
102131
source: usageLogSourceFilterSchema.optional(),
103-
limit: z.coerce.number().int().min(1).max(100).default(50),
132+
limit: usageLimitSchema(100, 50),
104133
cursor: z.string().min(1).optional(),
105134
})
106135
export type OrganizationUsageEventsQuery = z.input<typeof organizationUsageEventsQuerySchema>

apps/sim/lib/api/contracts/v1/admin/organizations.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -210,30 +210,30 @@ export const adminV1ListOrganizationsContract = defineRouteContract({
210210
})
211211

212212
/**
213-
* Creating an organization also attaches the owner's existing workspaces, so that
214-
* the organization is reachable through the workspace-scoped settings it is
215-
* administered from. That attachment runs in its own transaction and can fail after
216-
* the organization is already committed, so the outcome is part of the response
217-
* rather than something only the logs know.
213+
* Creates the organization and its owner membership, and deliberately nothing else.
218214
*
219-
* `attachedWorkspaceIds` alone is ambiguous — empty means both "the owner had none"
220-
* and "the attach threw" — which is why the failure carries its own flag. A caller
221-
* seeing `workspaceAttachmentFailed` can retry the attachment; a caller seeing an
222-
* empty list without it has nothing outstanding.
215+
* Organization settings are reached through a workspace the organization owns, so a
216+
* brand-new organization with none is not yet administrable. Closing that inside
217+
* this call was tried and removed: attaching the owner's existing workspaces cannot
218+
* join the creation transaction (it runs its own, under a lock order that exists to
219+
* avoid deadlocking against invitation acceptance), so it could only ever be
220+
* best-effort — leaving a committed organization, a response that could not honestly
221+
* report the outcome, and a retry blocked by the existing-membership check.
222+
*
223+
* This codebase already solves it properly elsewhere. `AdminMemberOperationView`
224+
* tracks workspace moves with `pending | processing | dead_letter | applied` and
225+
* per-workspace retry, and the enterprise-owner-claim path creates the workspace and
226+
* the organization in one transaction and enqueues an outbox event for the rest.
227+
* Provisioning a workspace for an organization belongs on one of those paths, not
228+
* inline here.
223229
*/
224-
const adminV1CreatedOrganizationSchema = adminV1OrganizationSchema.extend({
225-
memberId: z.string(),
226-
attachedWorkspaceIds: z.array(z.string()),
227-
workspaceAttachmentFailed: z.boolean(),
228-
})
229-
230230
export const adminV1CreateOrganizationContract = defineRouteContract({
231231
method: 'POST',
232232
path: '/api/v1/admin/organizations',
233233
body: adminV1CreateOrganizationBodySchema,
234234
response: {
235235
mode: 'json',
236-
schema: adminV1SingleResponseSchema(adminV1CreatedOrganizationSchema),
236+
schema: adminV1SingleResponseSchema(adminV1OrganizationSchema.extend({ memberId: z.string() })),
237237
},
238238
})
239239

apps/sim/lib/billing/application/organization-usage/export-organization-usage-events.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
usageWindowLedgerFilter,
77
} from '@/lib/billing/core/usage-analytics'
88
import { getBillingEntityUsageLogs } from '@/lib/billing/core/usage-log'
9-
import { dollarsToCredits } from '@/lib/billing/credits/conversion'
9+
import { CREDIT_MULTIPLIER } from '@/lib/billing/credits/conversion'
1010
import type { InternalUsageLogSource } from '@/lib/billing/usage-sources'
1111

1212
/**
@@ -31,6 +31,14 @@ export interface OrganizationUsageExportRow {
3131
source: string
3232
description: string
3333
workflowName: string | null
34+
/**
35+
* Unrounded, unlike the event list's integer credits.
36+
*
37+
* A CSV is an analysis surface, and rounding each row to a whole credit printed a
38+
* real sub-credit charge as `0` — the charge disappearing rather than reading as
39+
* small. Full precision here also makes the column summable, which the previous
40+
* `"N credits"` label never was.
41+
*/
3442
credits: number
3543
}
3644

@@ -81,7 +89,7 @@ export const exportOrganizationUsageEvents = defineAuthorizedOrganizationUsageUs
8189
source: log.source,
8290
description: log.description,
8391
workflowName: log.workflowName ?? null,
84-
credits: dollarsToCredits(log.cost),
92+
credits: log.cost * CREDIT_MULTIPLIER,
8593
})
8694
}
8795

apps/sim/lib/billing/core/usage-analytics.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,9 @@ describe('foldUsageBreakdown', () => {
430430
expect(fold.rows[0].share).toBe(0)
431431
})
432432

433-
it('carries the omitted rows tokens, so a zero-cost dimension still adds up', () => {
434-
// BYOK ranks by a cost that is zero for every row, so which rows land in the
435-
// visible slice is effectively arbitrary — dropping the tail's tokens would
436-
// hide real volume behind an em dash.
433+
it('measures share in the ranking unit, so a zero-cost list still draws bars', () => {
434+
// BYOK ranks by a cost that is zero for every row, so a cost-based share is 0/0
435+
// for all of them and every bar renders at the same minimum width.
437436
const byokRows = [
438437
{ key: 'gpt-4o', cost: '0', events: 1, inputTokens: 100, outputTokens: 50 },
439438
{ key: 'claude', cost: '0', events: 1, inputTokens: 700, outputTokens: 300 },

0 commit comments

Comments
 (0)