Skip to content

improvement(trace): billing trace span typing#4375

Merged
icecrasher321 merged 1 commit intostagingfrom
improvement/tracespan-billing-types
May 1, 2026
Merged

improvement(trace): billing trace span typing#4375
icecrasher321 merged 1 commit intostagingfrom
improvement/tracespan-billing-types

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

Minor typing improvement to make invariants clearer for trace spans.

Type of Change

  • Other: Code quality

Testing

N/A

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 1, 2026 7:30am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 1, 2026

PR Summary

Medium Risk
Touches billing-related cost aggregation and BYOK/segment cost handling, so mistakes could affect what users are charged. Changes are small and covered by targeted unit tests around toolCost and segment recursion.

Overview
Improves trace-span cost handling and typing by making calculateCostSummary operate on a narrowed TraceSpan shape and using type guards to distinguish billable spans and model-breakdown children.

Adds first-class support for toolCost in TraceSpan.cost, preserves it when building spans from provider outputs, and ensures cost summaries attribute tool charges to the parent span while still skipping model-breakdown child spans to avoid double counting. Tests are updated/added to verify toolCost propagation and that non-model (e.g., tool) segment costs are not zeroed for BYOK.

Reviewed by Cursor Bugbot for commit dd71d11. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 1, 2026

Greptile Summary

This PR extends TraceSpan.cost with an optional toolCost field and threads it through the trace-span pipeline: span-factory.ts now propagates it from provider output (guarded to positive values), logging-factory.ts aggregates it per-model in calculateCostSummary, and both the factory and test files are tightened from any[] to the new CostTraceSpan/BillableTraceSpan narrowed types. The BYOK test is updated to actively exercise tool-segment cost preservation (previously just a future-proofing comment).

Confidence Score: 5/5

Safe to merge — all changes are additive typing improvements with no logic mutations except toolCost propagation, which is gated by a > 0 guard consistent with the provider side.

No bugs identified. The type narrowing is correct, the toolCost > 0 guard is consistent with the existing sumToolCosts guard in providers/index.ts, the BYOK zeroing correctly leaves tool segments intact, and all new behaviour is covered by tests.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/lib/logs/types.ts Adds toolCost?: number to TraceSpan.cost — minimal, correct addition to the existing optional-fields shape.
apps/sim/lib/logs/execution/trace-spans/span-factory.ts Destructures and conditionally propagates toolCost from provider output into the trace span cost; guarded correctly to only write positive values, consistent with the provider-side sumToolCosts guard.
apps/sim/lib/logs/execution/logging-factory.ts Replaces any[] in calculateCostSummary with CostTraceSpan/BillableTraceSpan narrowed types; adds hasBillableCost and isModelBreakdownSpan helpers; accepts undefined input gracefully; logic is unchanged.
apps/sim/providers/index.test.ts Test updated to give the tool time-segment an actual cost and assert it is preserved under BYOK zeroing — correct behavioural test.
apps/sim/lib/logs/execution/logging-factory.test.ts New test verifies that parent toolCost is preserved in cost summary while model-breakdown children are correctly skipped; assertions are accurate.
apps/sim/lib/logs/execution/trace-spans/trace-spans.test.ts New concurrent test validates end-to-end that toolCost from a block output is faithfully round-tripped onto the resulting TraceSpan.cost.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Provider response\n(response.cost.toolCost)"] -->|"sumToolCosts()"| B["providers/index.ts\nattaches toolCost to response.cost"]
    B -->|"BYOK: zeroModelSegmentCosts()\n(model segments only — tool preserved)"| C["ProviderTiming.timeSegments"]
    B -->|"enrichWithProviderMetadata()"| D["span-factory.ts\nspan.cost = {input, output, total, toolCost?}"]
    D -->|"toolCost > 0 guard"| E["TraceSpan.cost.toolCost\n(new field in types.ts)"]
    E -->|"calculateCostSummary()"| F["logging-factory.ts\nmodels[model].toolCost accumulator"]
    F --> G["Cost summary\n(totalCost, models[x].toolCost)"]
Loading

Reviews (1): Last reviewed commit: "improvement(trace): billing trace span t..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant