Skip to content

fix: update model aliases and enhance event parsing for Copilot provider#220

Open
NihalJain wants to merge 1 commit intogetagentseal:mainfrom
NihalJain:fix_copilot_auto
Open

fix: update model aliases and enhance event parsing for Copilot provider#220
NihalJain wants to merge 1 commit intogetagentseal:mainfrom
NihalJain:fix_copilot_auto

Conversation

@NihalJain
Copy link
Copy Markdown
Contributor

@iamtoruk
Copy link
Copy Markdown
Member

iamtoruk commented May 5, 2026

Thanks for the PR @NihalJain, good find on the model classification issue. Reviewed the changes in detail:

Model aliases in models.ts -- These are correct and needed. claude-sonnet-4.6 (dot-version, tier-first) is a different format from what getCanonicalName produces, so explicit aliases are the right approach.

Catch-all type on LegacyCopilotEvent -- Good, consistent with the transcript format which already has this.

toolu_ prefix in tool call hints -- Correct and properly ordered after the more specific toolu_bdrk_ and toolu_vrtx_ prefixes, so specificity is preserved.

Two things I'd like adjusted before merging:

  1. The data.model extraction in parseLegacyEvents (lines 89-92 of the diff) -- Casting event.data broadly and extracting model from ANY event type is fragile. If a future event type has a model field meaning something different (e.g. a "3D model" reference), it would silently corrupt currentModel. Safer to scope this to specific event types, or at minimum only apply it when currentModel is still empty (i.e. as a fallback, not an override).

  2. Fallback change from 'copilot-auto' to COPILOT_OPENAI_AUTO -- This changes cost attribution from Claude pricing ($3/$15 per MTok) to OpenAI GPT pricing when the model can't be inferred. This is a meaningful behavioral change. copilot-auto was intentionally a neutral catch-all. If a user is on Anthropic models with no tool calls (so no ID prefixes to infer from), their costs would now be calculated at OpenAI rates. Can you share the reasoning here? If the intent is "Copilot defaults to OpenAI", that's reasonable for the fallback in inferModelFromToolCallIds, but should be paired with a similar default in parseLegacyEvents for consistency.

The data.model extraction in inferModelFromToolCallIds with weight 100 is fine -- explicit model IDs should outweigh heuristic inference.

Copy link
Copy Markdown
Member

@iamtoruk iamtoruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retracting my earlier comment -- I jumped to review without checking with you first. Please disregard the previous feedback; we'll follow up separately once we've aligned internally on what we want from this PR.

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.

Copilot: Models like claude-sonnet-4.6 are classified as Copilot (auto)

2 participants