fix(devin): support ATIF v1.7 transcripts#526
Conversation
|
|
||
| User-input steps (`metadata.is_user_input === true`) are skipped. Non-user | ||
| steps are included only if they have positive ACU usage or positive token usage. | ||
| CodeBurn supports ATIF transcript variants used by Devin across **ATIF-v1.5**, **ATIF-v1.6**, and **ATIF-v1.7** (and remains backward-compatible with older Devin transcripts that still use v1.4-style field names). |
There was a problem hiding this comment.
As it is I would not frame as "CodeBurn supports ATIF". The existing code is tied to devin. For that claim to be true. The code would need to be on a separate place and be shared by other providers, which at this point is not.
There was a problem hiding this comment.
I changed the wording, let me know your thoughts.
|
|
||
| `metadata.committed_acu_cost` is per step, not cumulative. The provider converts | ||
| each step with: | ||
| `costUSD` is always provider-supplied and uses configured ACU conversion: |
There was a problem hiding this comment.
Still per step and not cumulative, right?
| using Devin's current export convention: | ||
|
|
||
| ```text | ||
| committed_acu_cost = committed_credit_cost / 10000 |
There was a problem hiding this comment.
What is the source of this? Can you provide a reference?
Also IMO this should not be hardcoded, but rather configurable. Because Devin can change it at any time, and most likely on enterprise accounts can even depend on the agreed plan.
| `devin:<sessionId>:<step.step_id>` | ||
|
|
||
| The provider name is part of the key via the `devin:` prefix. | ||
| When `step_id` is missing, parser falls back to 1-based step index. |
There was a problem hiding this comment.
step_id accoding to ATIF RFC has never been optional and is always mandatory. So it will never be missing
| name?: string; | ||
| version?: string; | ||
| model_name?: string; | ||
| tool_definitions?: unknown; |
There was a problem hiding this comment.
not unknown, but rather an array following openai functional calling schema
There was a problem hiding this comment.
set name, version to required.
set tool_definitions to array.
| type ToolCall = { | ||
| tool_call_id: string; | ||
| function_name: string; | ||
| tool_call_id?: string; |
There was a problem hiding this comment.
field is mandatory, not optional
| tool_call_id: string; | ||
| function_name: string; | ||
| tool_call_id?: string; | ||
| function_name?: string; |
There was a problem hiding this comment.
field is mandatory, not optional
| function_name: string; | ||
| tool_call_id?: string; | ||
| function_name?: string; | ||
| function?: { |
There was a problem hiding this comment.
does not exist. only exists on tool definition
| type Step = { | ||
| step_id: number; | ||
| source: string; | ||
| step_id?: number | string; |
There was a problem hiding this comment.
step_id is a number according to ATIF RFC
| step_id: number; | ||
| source: string; | ||
| step_id?: number | string; | ||
| source?: string; |
There was a problem hiding this comment.
source is required and not optional
| timestamp?: string; | ||
| model_name?: string; | ||
| message: string; | ||
| message?: unknown; |
There was a problem hiding this comment.
message is mandatory and not optional.
The type if we want to accept versions above 1.6+ should be array or a string.
When is an array then it has the following schema
{ type: string, text?: string, source?: unknown }
| model_name?: string; | ||
| message: string; | ||
| message?: unknown; | ||
| metrics?: DevinMetrics; |
There was a problem hiding this comment.
should not be tied to DevinMetrics. Step type is supposed to be generic and unrelated with Devin.
| message: string; | ||
| message?: unknown; | ||
| metrics?: DevinMetrics; | ||
| metadata?: DevinMetadata; |
There was a problem hiding this comment.
metadata is not part of ATIF at step level and this seems to be custom to devin. Hence should not be declared in this type.
| message?: unknown; | ||
| metrics?: DevinMetrics; | ||
| metadata?: DevinMetadata; | ||
| extra?: DevinStepExtra; |
There was a problem hiding this comment.
extra is part of the spec but is an object with no bound contract.
Since Step type is supposed to be generic for ATIF it should not be tied to devin in any way
| }; | ||
|
|
||
| type DevinStep = Step & { metadata?: DevinMetadata }; | ||
| type DevinStep = Step; |
There was a problem hiding this comment.
The preivous version was intented since this type was defined to provide the custom DevinStep that extends Step with custom attributes and information
| const DEVIN_PROVIDER_DISPLAY_NAME = "Devin"; | ||
| const DEVIN_TRANSCRIPTS_SUBDIR = "transcripts"; | ||
| const DEVIN_SESSIONS_DB = "sessions.db"; | ||
| const DEVIN_CREDITS_PER_ACU = 10_000; |
There was a problem hiding this comment.
What is the source for this? IMO this should be configurable, since Devin can change, and also might depend on subscription plan
| return JSON.parse(raw) as DevinAgentTrajectory; | ||
| const parsed = JSON.parse(raw) as unknown; | ||
| if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { | ||
| return null; | ||
| } | ||
| return parsed as DevinAgentTrajectory; |
There was a problem hiding this comment.
No need for the complex defensive programming. The transcript must be allways from ATIF format
| const tools: string[] = []; | ||
| const toolCalls = Array.isArray(step.tool_calls) ? step.tool_calls : []; | ||
| for (const call of toolCalls) { | ||
| const toolName = call.function_name ?? call.function?.name; | ||
| if (toolName) tools.push(toolName); | ||
| } | ||
| return tools; |
There was a problem hiding this comment.
- tool_calls is always an array
- function.name is not part of the ATIF schema for tool_calls. only function_name is the valid attribute. So no need for the extra logic
|
|
||
| const transcript = parseTranscript(raw); | ||
| if (!transcript?.steps) return; | ||
| if (!transcript?.steps || !Array.isArray(transcript.steps)) return; |
There was a problem hiding this comment.
steps is always an array so no need for the check
| if (!step || typeof step !== "object" || Array.isArray(step)) continue; | ||
| if (isUserInputStep(step)) continue; |
There was a problem hiding this comment.
step is always defined and is an object with several mandatory fields. So no need for the check. It can be simplified to
| if (!step || typeof step !== "object" || Array.isArray(step)) continue; | |
| if (isUserInputStep(step)) continue; | |
| if (isUserInputStep(step)) continue; |
| const timestamp = getTimestamp(step, session) ?? ""; | ||
|
|
||
| const deduplicationKey = `devin:${sessionId}:${step.step_id}`; | ||
| const stepId = `${step.step_id ?? index + 1}`; |
There was a problem hiding this comment.
step id is mandatory so no need for this
| const metadataAcuCost = safeNumber(step.metadata?.committed_acu_cost); | ||
| if (isPositiveNumber(metadataAcuCost)) return metadataAcuCost; | ||
|
|
||
| const extraAcuCost = safeNumber(step.extra?.committed_acu_cost); | ||
| if (isPositiveNumber(extraAcuCost)) return extraAcuCost; | ||
|
|
||
| const metadataCreditCost = safeNumber(step.metadata?.committed_credit_cost); | ||
| if (isPositiveNumber(metadataCreditCost)) { | ||
| return metadataCreditCost / DEVIN_CREDITS_PER_ACU; | ||
| } | ||
|
|
||
| const extraCreditCost = safeNumber(step.extra?.committed_credit_cost); | ||
| if (isPositiveNumber(extraCreditCost)) { | ||
| return extraCreditCost / DEVIN_CREDITS_PER_ACU; | ||
| } | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
The logic can be grouped in two blocks. ACU costs and Credit costs. The computational effort vs readability in my opinion does not pay all the repetition for the early returns. Also isPositiveNumber subsumes safeNumber.
| const metadataAcuCost = safeNumber(step.metadata?.committed_acu_cost); | |
| if (isPositiveNumber(metadataAcuCost)) return metadataAcuCost; | |
| const extraAcuCost = safeNumber(step.extra?.committed_acu_cost); | |
| if (isPositiveNumber(extraAcuCost)) return extraAcuCost; | |
| const metadataCreditCost = safeNumber(step.metadata?.committed_credit_cost); | |
| if (isPositiveNumber(metadataCreditCost)) { | |
| return metadataCreditCost / DEVIN_CREDITS_PER_ACU; | |
| } | |
| const extraCreditCost = safeNumber(step.extra?.committed_credit_cost); | |
| if (isPositiveNumber(extraCreditCost)) { | |
| return extraCreditCost / DEVIN_CREDITS_PER_ACU; | |
| } | |
| return 0; | |
| const acuCost = [ | |
| step.metadata?.committed_acu_cost, | |
| step.extra?.committed_acu_cost, | |
| ].filter(cost => isPositiveNumber(cost)); | |
| const creditCost = [ | |
| step.metadata?.committed_credit_cost, | |
| step.extra?.committed_credit_cost | |
| ].filter(cost => isPositiveNumber(cost)) | |
| .map(cost => cost / DEVIN_CREDITS_PER_ACU); | |
| return acuCost.shift() || creditCost.shift() || 0; |
| const cacheCreationInputTokens = safeNumber( | ||
| metrics?.cache_creation_tokens ?? | ||
| metrics?.cache_creation_input_tokens ?? | ||
| metrics?.extra?.cache_creation_input_tokens, | ||
| ); | ||
|
|
||
| const cacheReadInputTokens = safeNumber( | ||
| metrics?.cache_read_tokens ?? | ||
| metrics?.cache_read_input_tokens ?? | ||
| metrics?.cached_tokens ?? | ||
| metrics?.extra?.cache_read_input_tokens, | ||
| ); | ||
|
|
||
| const promptTokens = safeNumber( | ||
| metrics?.prompt_tokens ?? metrics?.total_input_tokens, | ||
| ); | ||
|
|
||
| let inputTokens = safeNumber(metrics?.input_tokens); | ||
| if (inputTokens === 0 && promptTokens > 0) { | ||
| inputTokens = Math.max( | ||
| 0, | ||
| promptTokens - cacheReadInputTokens - cacheCreationInputTokens, | ||
| ); | ||
| } | ||
|
|
||
| const outputTokens = safeNumber( | ||
| metrics?.output_tokens ?? metrics?.completion_tokens, | ||
| ); |
There was a problem hiding this comment.
I would suggest to extract each one to a separate method for maintanability and also separation of concerns, making also this method getUsage easier to read.
| ): DevinUsage | null { | ||
| if (!metadata) return null; | ||
| const metrics = metadata.metrics; | ||
| function normalizeMessageText(message: unknown): string { |
There was a problem hiding this comment.
I suggest first to check the schema from ATIF to avoid all this complex code. and to a proper message, taking into account ATIF schema.
tvcsantos
left a comment
tvcsantos
left a comment
There was a problem hiding this comment.
Thanks for the PR.
In order to better match ATIF format you can check:
|
converted to [DRAFT] while I work on the issues identified in the excellent review by tvcsantos |
2 participants
What
Why
Recent Devin CLI transcripts use newer ATIF shapes (including v1.7) that are not strictly metadata-scoped. This change ensures codeburn can ingest these sessions end-to-end while preserving existing behavior.
Testing-
RUN v3.2.6 /Users/william.mcdonough/github/bmcdonough/codeburn
✓ tests/providers/devin.test.ts (14 tests) 60ms
Test Files 1 passed (1)
Tests 14 passed (14)
Start at 15:18:26
Duration 600ms (transform 180ms, setup 0ms, collect 244ms, tests 60ms, environment 0ms, prepare 111ms)