Skip to content

Commit 694cce6

Browse files
committed
fix billing, tracing, retry lifecycle issues
1 parent ed99f66 commit 694cce6

14 files changed

Lines changed: 615 additions & 87 deletions

File tree

apps/sim/app/api/billing/update-cost/route.test.ts

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@
44
import { createMockRequest } from '@sim/testing'
55
import { beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
const { mockCheckInternalApiKey, mockRecordUsage, mockCheckAndBillOverageThreshold } = vi.hoisted(
8-
() => ({
9-
mockCheckInternalApiKey: vi.fn(),
10-
mockRecordUsage: vi.fn(),
11-
mockCheckAndBillOverageThreshold: vi.fn(),
12-
})
13-
)
7+
const {
8+
mockCheckInternalApiKey,
9+
mockRecordUsage,
10+
mockRecordCumulativeUsage,
11+
mockCheckAndBillOverageThreshold,
12+
} = vi.hoisted(() => ({
13+
mockCheckInternalApiKey: vi.fn(),
14+
mockRecordUsage: vi.fn(),
15+
mockRecordCumulativeUsage: vi.fn(),
16+
mockCheckAndBillOverageThreshold: vi.fn(),
17+
}))
1418

1519
vi.mock('@/lib/copilot/request/http', () => ({
1620
checkInternalApiKey: mockCheckInternalApiKey,
@@ -27,19 +31,13 @@ vi.mock('@/lib/copilot/request/otel', () => ({
2731

2832
vi.mock('@/lib/billing/core/usage-log', () => ({
2933
recordUsage: mockRecordUsage,
34+
recordCumulativeUsage: mockRecordCumulativeUsage,
3035
}))
3136

3237
vi.mock('@/lib/billing/threshold-billing', () => ({
3338
checkAndBillOverageThreshold: mockCheckAndBillOverageThreshold,
3439
}))
3540

36-
vi.mock('@/lib/core/idempotency/service', () => ({
37-
billingIdempotency: {
38-
atomicallyClaim: vi.fn(),
39-
release: vi.fn(),
40-
},
41-
}))
42-
4341
vi.mock('@/lib/core/config/feature-flags', () => ({
4442
isBillingEnabled: true,
4543
}))
@@ -51,10 +49,11 @@ describe('POST /api/billing/update-cost — workspaceId attribution', () => {
5149
vi.clearAllMocks()
5250
mockCheckInternalApiKey.mockReturnValue({ success: true })
5351
mockRecordUsage.mockResolvedValue(undefined)
52+
mockRecordCumulativeUsage.mockResolvedValue({ billed: true, delta: 0.5, total: 0.5 })
5453
mockCheckAndBillOverageThreshold.mockResolvedValue(undefined)
5554
})
5655

57-
it('stamps workspaceId onto recorded usage when provided', async () => {
56+
it('stamps workspaceId onto recorded usage when provided (no idempotency key)', async () => {
5857
const res = await POST(
5958
createMockRequest(
6059
'POST',
@@ -70,6 +69,57 @@ describe('POST /api/billing/update-cost — workspaceId attribution', () => {
7069
})
7170
})
7271

72+
it('records cumulative cost via monotonic top-up when an idempotency key is present', async () => {
73+
const res = await POST(
74+
createMockRequest(
75+
'POST',
76+
{
77+
userId: 'user-1',
78+
cost: 0.4662453,
79+
model: 'claude-opus-4.8',
80+
source: 'workspace-chat',
81+
workspaceId: 'ws-1',
82+
idempotencyKey: 'msg-1-billing',
83+
inputTokens: 461371,
84+
outputTokens: 1686,
85+
},
86+
{ 'x-api-key': 'internal' }
87+
)
88+
)
89+
expect(res.status).toBe(200)
90+
expect(mockRecordUsage).not.toHaveBeenCalled()
91+
expect(mockRecordCumulativeUsage).toHaveBeenCalledTimes(1)
92+
expect(mockRecordCumulativeUsage.mock.calls[0][0]).toMatchObject({
93+
userId: 'user-1',
94+
workspaceId: 'ws-1',
95+
source: 'workspace-chat',
96+
model: 'claude-opus-4.8',
97+
cost: 0.4662453,
98+
eventKey: 'update-cost:msg-1-billing',
99+
})
100+
expect(mockCheckAndBillOverageThreshold).toHaveBeenCalledWith('user-1')
101+
})
102+
103+
it('returns 409 and skips overage when the cumulative is not higher (duplicate flush)', async () => {
104+
mockRecordCumulativeUsage.mockResolvedValue({ billed: false, delta: 0, total: 0.4662453 })
105+
const res = await POST(
106+
createMockRequest(
107+
'POST',
108+
{
109+
userId: 'user-1',
110+
cost: 0.4662453,
111+
model: 'claude-opus-4.8',
112+
source: 'workspace-chat',
113+
workspaceId: 'ws-1',
114+
idempotencyKey: 'msg-1-billing',
115+
},
116+
{ 'x-api-key': 'internal' }
117+
)
118+
)
119+
expect(res.status).toBe(409)
120+
expect(mockCheckAndBillOverageThreshold).not.toHaveBeenCalled()
121+
})
122+
73123
it('rejects with 400 when workspaceId is omitted (contract-required, fail loud)', async () => {
74124
const res = await POST(
75125
createMockRequest(

apps/sim/app/api/billing/update-cost/route.ts

Lines changed: 73 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@ import { toError } from '@sim/utils/errors'
44
import { type NextRequest, NextResponse } from 'next/server'
55
import { billingUpdateCostContract } from '@/lib/api/contracts/subscription'
66
import { parseRequest } from '@/lib/api/server'
7-
import { recordUsage } from '@/lib/billing/core/usage-log'
7+
import { recordCumulativeUsage, recordUsage } from '@/lib/billing/core/usage-log'
88
import { checkAndBillOverageThreshold } from '@/lib/billing/threshold-billing'
99
import { BillingRouteOutcome } from '@/lib/copilot/generated/trace-attribute-values-v1'
1010
import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
1111
import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
1212
import { checkInternalApiKey } from '@/lib/copilot/request/http'
1313
import { withIncomingGoSpan } from '@/lib/copilot/request/otel'
1414
import { isBillingEnabled } from '@/lib/core/config/feature-flags'
15-
import { type AtomicClaimResult, billingIdempotency } from '@/lib/core/idempotency/service'
1615
import { generateRequestId } from '@/lib/core/utils/request'
1716
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1817

@@ -42,8 +41,6 @@ export const POST = withRouteHandler((req: NextRequest) =>
4241
async function updateCostInner(req: NextRequest, span: Span): Promise<NextResponse> {
4342
const requestId = generateRequestId()
4443
const startTime = Date.now()
45-
let claim: AtomicClaimResult | null = null
46-
let usageCommitted = false
4744

4845
try {
4946
logger.info(`[${requestId}] Update cost request started`)
@@ -125,63 +122,92 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
125122
...(idempotencyKey ? { [TraceAttr.BillingIdempotencyKey]: idempotencyKey } : {}),
126123
})
127124

128-
claim = idempotencyKey
129-
? await billingIdempotency.atomicallyClaim('update-cost', idempotencyKey)
130-
: null
125+
logger.info(`[${requestId}] Processing cost update`, {
126+
userId,
127+
cost,
128+
model,
129+
source,
130+
})
131131

132-
if (claim && !claim.claimed) {
133-
logger.warn(`[${requestId}] Duplicate billing update rejected`, {
134-
idempotencyKey,
132+
// Go sends the request's CUMULATIVE cost, possibly more than once (a
133+
// mid-loop provider-error flush, then the recovered terminal flush, plus
134+
// abort-race duplicates). Record it as a monotonic top-up: one ledger row
135+
// per request holds the MAX cumulative and we bill only the delta, so
136+
// partial + complete flushes converge to the true total exactly once — no
137+
// under-billing on recovery, no over-billing on duplicates. When there is
138+
// no idempotency key (shouldn't happen for real requests) we fall back to a
139+
// plain append so cost is never silently dropped.
140+
let billed = true
141+
if (idempotencyKey) {
142+
const result = await recordCumulativeUsage({
143+
userId,
144+
workspaceId,
145+
source,
146+
model,
147+
cost,
148+
eventKey: `update-cost:${idempotencyKey}`,
149+
metadata: { inputTokens, outputTokens },
150+
})
151+
billed = result.billed
152+
logger.info(`[${requestId}] Cumulative cost top-up`, {
135153
userId,
136154
source,
155+
cumulativeCost: cost,
156+
billedDelta: result.delta,
157+
newTotal: result.total,
158+
billed: result.billed,
159+
})
160+
} else {
161+
await recordUsage({
162+
userId,
163+
workspaceId,
164+
entries: [
165+
{
166+
category: 'model',
167+
source,
168+
description: model,
169+
cost,
170+
sourceReference: requestId,
171+
metadata: { inputTokens, outputTokens },
172+
},
173+
],
174+
})
175+
logger.info(`[${requestId}] Recorded usage (no idempotency key)`, {
176+
userId,
177+
addedCost: cost,
178+
source,
179+
})
180+
}
181+
182+
const duration = Date.now() - startTime
183+
184+
// Same-or-lower cumulative than already recorded: nothing new to bill. Tell
185+
// the caller via 409 (its existing "duplicate" outcome) without re-running
186+
// overage billing.
187+
if (!billed) {
188+
logger.info(`[${requestId}] Duplicate/non-increasing cumulative cost; no new charge`, {
189+
idempotencyKey,
190+
userId,
191+
cost,
137192
})
138193
span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.DuplicateIdempotencyKey)
139194
span.setAttribute(TraceAttr.HttpStatusCode, 409)
195+
span.setAttribute(TraceAttr.BillingDurationMs, duration)
140196
return NextResponse.json(
141197
{
142198
success: false,
143-
error: 'Duplicate request: idempotency key already processed',
199+
error: 'Duplicate request: cumulative cost already recorded',
144200
requestId,
145201
},
146202
{ status: 409 }
147203
)
148204
}
149205

150-
logger.info(`[${requestId}] Processing cost update`, {
151-
userId,
152-
cost,
153-
model,
154-
source,
155-
})
156-
157-
await recordUsage({
158-
userId,
159-
workspaceId,
160-
entries: [
161-
{
162-
category: 'model',
163-
source,
164-
description: model,
165-
cost,
166-
eventKey: idempotencyKey ? `update-cost:${idempotencyKey}` : undefined,
167-
sourceReference: idempotencyKey ? `update-cost:${idempotencyKey}` : requestId,
168-
metadata: { inputTokens, outputTokens },
169-
},
170-
],
171-
})
172-
usageCommitted = true
173-
174-
logger.info(`[${requestId}] Recorded usage`, {
175-
userId,
176-
addedCost: cost,
177-
source,
178-
})
179-
180-
// Check if user has hit overage threshold and bill incrementally
206+
// Check if user has hit overage threshold and bill incrementally. Reads the
207+
// (now topped-up) ledger total and is idempotent against billedOverage, so
208+
// it is safe to run on every flush that records new cost.
181209
await checkAndBillOverageThreshold(userId)
182210

183-
const duration = Date.now() - startTime
184-
185211
logger.info(`[${requestId}] Cost update completed successfully`, {
186212
userId,
187213
duration,
@@ -209,22 +235,9 @@ async function updateCostInner(req: NextRequest, span: Span): Promise<NextRespon
209235
duration,
210236
})
211237

212-
if (claim?.claimed && !usageCommitted) {
213-
await billingIdempotency
214-
.release(claim.normalizedKey, claim.storageMethod)
215-
.catch((releaseErr) => {
216-
logger.warn(`[${requestId}] Failed to release idempotency claim`, {
217-
error: toError(releaseErr).message,
218-
normalizedKey: claim?.normalizedKey,
219-
})
220-
})
221-
} else if (claim?.claimed && usageCommitted) {
222-
logger.warn(
223-
`[${requestId}] Error occurred after usage committed; retaining idempotency claim to prevent double-billing`,
224-
{ normalizedKey: claim.normalizedKey }
225-
)
226-
}
227-
238+
// The cumulative top-up runs in a single transaction (and a plain append is
239+
// a single insert), so a failure here leaves nothing partially committed —
240+
// a retry re-evaluates the max idempotently. No claim to release.
228241
span.setAttribute(TraceAttr.BillingOutcome, BillingRouteOutcome.InternalError)
229242
span.setAttribute(TraceAttr.HttpStatusCode, 500)
230243
span.setAttribute(TraceAttr.BillingDurationMs, duration)

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ export function AgentGroup({
6060
(item): item is Extract<AgentGroupItem, { type: 'tool' }> => item.type === 'tool'
6161
)
6262
const allDone = toolItems.length > 0 && toolItems.every((t) => isToolDone(t.data.status))
63-
const showDelegatingSpinner = isDelegating && !allDone
63+
// Only a live turn can be delegating. Once the turn is terminal (complete,
64+
// errored, or stopped) no subagent should spin — even one aborted before its
65+
// first tool call, where `allDone` is false because there are no tools yet.
66+
const showDelegatingSpinner = isStreaming && isDelegating && !allDone
6467

6568
// Expand only while the turn is live and the group is still open or working.
6669
// Once the turn ends (isStreaming false) — or a subagent closes mid-turn — the

apps/sim/app/workspace/[workspaceId]/home/components/message-content/message-content.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44
import { describe, expect, it } from 'vitest'
55
import type { ContentBlock } from '../../types'
6-
import { parseBlocks } from './message-content'
6+
import { parseBlocks, shouldSmoothTextSegment } from './message-content'
77

88
function subagentStart(name: string, spanId: string, parentSpanId: string): ContentBlock {
99
return { type: 'subagent', content: name, spanId, parentSpanId, timestamp: 1 }
@@ -182,3 +182,20 @@ describe('parseBlocks span-identity tree', () => {
182182
expect(groups[0].agentName).toBe('workflow')
183183
})
184184
})
185+
186+
describe('shouldSmoothTextSegment', () => {
187+
it('only smooths the trailing text segment of a live stream', () => {
188+
expect(shouldSmoothTextSegment({ isStreaming: true, segmentIndex: 0, segmentCount: 2 })).toBe(
189+
false
190+
)
191+
expect(shouldSmoothTextSegment({ isStreaming: true, segmentIndex: 1, segmentCount: 2 })).toBe(
192+
true
193+
)
194+
})
195+
196+
it('never smooths completed messages', () => {
197+
expect(shouldSmoothTextSegment({ isStreaming: false, segmentIndex: 0, segmentCount: 1 })).toBe(
198+
false
199+
)
200+
})
201+
})

apps/sim/app/workspace/[workspaceId]/home/components/message-content/message-content.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,18 @@ export function assistantMessageHasRenderableContent(
659659
return segments.length > 0
660660
}
661661

662+
export function shouldSmoothTextSegment({
663+
isStreaming,
664+
segmentIndex,
665+
segmentCount,
666+
}: {
667+
isStreaming: boolean
668+
segmentIndex: number
669+
segmentCount: number
670+
}): boolean {
671+
return isStreaming && segmentIndex === segmentCount - 1
672+
}
673+
662674
interface MessageContentProps {
663675
blocks: ContentBlock[]
664676
fallbackContent: string
@@ -719,7 +731,11 @@ function MessageContentInner({
719731
<ChatContent
720732
key={`text-${i}`}
721733
content={segment.content}
722-
isStreaming={isStreaming}
734+
isStreaming={shouldSmoothTextSegment({
735+
isStreaming,
736+
segmentIndex: i,
737+
segmentCount: segments.length,
738+
})}
723739
onOptionSelect={onOptionSelect}
724740
onWorkspaceResourceSelect={onWorkspaceResourceSelect}
725741
/>

apps/sim/hooks/use-smooth-text.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ export function useSmoothText(
8080
// trips React's max-update-depth guard. The running tick reads the latest content
8181
// from `contentRef`, so new chunks are absorbed without per-chunk teardown;
8282
// `hasBacklog` only flips when the reveal falls behind or catches up.
83+
if (!isStreaming && effectiveRevealed !== content.length) {
84+
effectiveRevealed = content.length
85+
revealedRef.current = content.length
86+
}
87+
8388
const hasBacklog = effectiveRevealed < content.length
8489

8590
useEffect(() => {

0 commit comments

Comments
 (0)