Skip to content

refactor(core): drop redundant String casts on Model.Ref in to-llm-message - #57887

Merged
rekram1-node merged 2 commits into
devfrom
core-to-llm-message-cleanup
Oct 21, 2026
Merged

rekram1-node merged 2 commits into
devfrom
core-to-llm-message-cleanup

Conversation

@rekram1-node

@rekram1-node rekram1-node commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

assistant in to-llm-message.ts wraps message.model and model fields in String(...) even though both are typed as Model.Ref (unlike modelSwitched in the same file, which compares providerID and id directly).

Change

Compare Model.Ref fields directly in assistant.

 assistant(message, model)
-  sameProvider = String(message.model.providerID) === String(model.providerID)
-  sameModel = sameProvider && String(message.model.id) === String(model.id)
+  sameProvider = message.model.providerID === model.providerID
+  sameModel = sameProvider && message.model.id === model.id

Out of scope

  • Changing provider metadata reuse rules.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The change looks sound and leaves behaviour unchanged. With fewer than two eligible inline mentioned images, the dedupe loop can't find a duplicate, so it returns the same parts as the removed early-return path. Model.Ref.id and providerID are branded strings, so === matches the old String(...) comparison. In packages/core, session-runner-message.test.ts passes, including the dedupe and provider-metadata reuse cases, and typecheck passes.

The public API is unchanged. I have no concerns.

@GiuseppeBellamacina GiuseppeBellamacina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With fewer than two eligible inline mentioned images, the dedupe loop can't find a duplicate, so dropping the pre-filter returns the same parts as the old early return. The direct providerID / id comparisons match what modelSwitched already does in this file. Looks good to me.

@rekram1-node rekram1-node changed the title refactor(core): drop redundant String casts and pre-filter pass in to-llm-message refactor(core): drop redundant String casts on Model.Ref in to-llm-message Oct 21, 2026
@rekram1-node
rekram1-node merged commit e98ab91 into dev Oct 21, 2026
13 of 14 checks passed
@rekram1-node
rekram1-node deleted the core-to-llm-message-cleanup branch October 21, 2026 21:01
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.

2 participants