fix: add missing serializer registration to 6 chat adapters (#1477)#1496
fix: add missing serializer registration to 6 chat adapters (#1477)#1496EvanYao826 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds static initialization blocks to automatically register several chat model adapters, including Amazon Bedrock, Azure OpenAI, Google Vertex, LangChain, OpenAI, and Watsonx. The reviewer recommends using 'this.register()' instead of referencing the class names directly inside the static blocks, which is more idiomatic, robust against future class renames, and consistent with other adapters in the codebase.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| static { | ||
| AmazonBedrockChatModel.register(); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using this.register() inside the static block is more idiomatic and robust than referencing the class name directly. In a static block, this refers to the class constructor itself, which prevents potential bugs if the class is renamed in the future. Additionally, this aligns with the pattern already used by other adapters in the codebase (such as anthropic, dummy, groq, and ollama). This suggestion also adds a trailing newline to the file.
| static { | |
| AmazonBedrockChatModel.register(); | |
| } | |
| } | |
| static { | |
| this.register(); | |
| } | |
| } | |
| static { | ||
| AzureOpenAIChatModel.register(); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using this.register() inside the static block is more idiomatic and robust than referencing the class name directly. In a static block, this refers to the class constructor itself, which prevents potential bugs if the class is renamed in the future. Additionally, this aligns with the pattern already used by other adapters in the codebase (such as anthropic, dummy, groq, and ollama). This suggestion also adds a trailing newline to the file.
| static { | |
| AzureOpenAIChatModel.register(); | |
| } | |
| } | |
| static { | |
| this.register(); | |
| } | |
| } | |
| static { | ||
| GoogleVertexChatModel.register(); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using this.register() inside the static block is more idiomatic and robust than referencing the class name directly. In a static block, this refers to the class constructor itself, which prevents potential bugs if the class is renamed in the future. Additionally, this aligns with the pattern already used by other adapters in the codebase (such as anthropic, dummy, groq, and ollama). This suggestion also adds a trailing newline to the file.
| static { | |
| GoogleVertexChatModel.register(); | |
| } | |
| } | |
| static { | |
| this.register(); | |
| } | |
| } | |
| static { | ||
| LangChainChatModel.register(); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using this.register() inside the static block is more idiomatic and robust than referencing the class name directly. In a static block, this refers to the class constructor itself, which prevents potential bugs if the class is renamed in the future. Additionally, this aligns with the pattern already used by other adapters in the codebase (such as anthropic, dummy, groq, and ollama). This suggestion also adds a trailing newline to the file.
| static { | |
| LangChainChatModel.register(); | |
| } | |
| } | |
| static { | |
| this.register(); | |
| } | |
| } | |
| static { | ||
| OpenAIChatModel.register(); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using this.register() inside the static block is more idiomatic and robust than referencing the class name directly. In a static block, this refers to the class constructor itself, which prevents potential bugs if the class is renamed in the future. Additionally, this aligns with the pattern already used by other adapters in the codebase (such as anthropic, dummy, groq, and ollama). This suggestion also adds a trailing newline to the file.
| static { | |
| OpenAIChatModel.register(); | |
| } | |
| } | |
| static { | |
| this.register(); | |
| } | |
| } | |
| static { | ||
| WatsonxChatModel.register(); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Using this.register() inside the static block is more idiomatic and robust than referencing the class name directly. In a static block, this refers to the class constructor itself, which prevents potential bugs if the class is renamed in the future. Additionally, this aligns with the pattern already used by other adapters in the codebase (such as anthropic, dummy, groq, and ollama). This suggestion also adds a trailing newline to the file.
| static { | |
| WatsonxChatModel.register(); | |
| } | |
| } | |
| static { | |
| this.register(); | |
| } | |
| } | |
|
Hi @EvanYao826 — the change itself is approved (correct and complete fix for #1477), but it's blocked by a failing DCO check: neither of the two commits is signed off. To sign off both commits and refresh the branch in one go: git fetch origin
git rebase --signoff origin/main
git push --force-with-lease(The |
Fixes i-am-bee#1477 6 of 12 TypeScript chat adapters were missing the static register() call, which is needed for proper serializer/deserializer support: - amazon-bedrock: AmazonBedrockChatModel.register() - azure-openai: AzureOpenAIChatModel.register() - google-vertex: GoogleVertexChatModel.register() - langchain: LangChainChatModel.register() - openai: OpenAIChatModel.register() - watsonx: WatsonxChatModel.register() The following adapters already had registration: - anthropic, dummy, groq, ollama (this.register()) - xai (XAIChatModel.register()) Note: vercel is abstract and does not need registration. Signed-off-by: EvanYao826 <2869018789@qq.com>
Signed-off-by: EvanYao826 <2869018789@qq.com>
506d981 to
3485a2c
Compare
|
Hi @Tomas2D — both commits have been signed off and force-pushed. Could you trigger the DCO check again? Thanks for the review! |
Summary
Fixes #1477
6 of 12 TypeScript chat adapters were missing the
static { ClassName.register() }call needed for proper serializer/deserializer support. This PR adds the missing registration to all concrete adapter classes.Changes
AmazonBedrockChatModelAzureOpenAIChatModelGoogleVertexChatModelLangChainChatModelOpenAIChatModelWatsonxChatModelVercelChatModelAlready registered (no changes needed)
this.register()XAIChatModel.register()Verification
Each adapter now has a static initializer block at the end of the class:
This matches the pattern used by the 4 already-registered adapters (anthropic, dummy, groq, ollama) and ensures that
ChatModel.fromName()can properly deserialize instances of these adapters.