fix: close remaining unified message protocol gaps#105
Conversation
Add tests for thinking, code, image, and refusal content blocks in mapAssistantMessage, verifying the mapper already handles all 7 content types correctly (ISSUE 4).
… adapter Add image, code, and refusal variants to the ContentBlock union type. Handle these content types in the Claude adapter's translateAssistant() switch instead of dropping them to empty text (ISSUE 1). Add tests for passthrough of all three new types plus verification that truly unknown block types are still dropped with metadata tracking.
Add canonical 'model' key to OpenCode adapter's assistant metadata so the consumer mapper can resolve it (ISSUE 2). Add canonical 'tool_name' key to Codex adapter's function_call metadata in both translateItemAdded and translateItemDone (ISSUE 2).
Expand the comment in handleStreamEvent() to explain why "running" status inference is Claude-specific and why generalizing it was rejected (ISSUE 3).
…erage
Remove redundant `(block as { budget_tokens?: number })` cast in the
thinking case — TypeScript now narrows correctly after the ContentBlock
union was extended.
Split the thinking test into two: one verifying budget_tokens is
forwarded when present, and one asserting undefined when absent.
Rename test description to match the naming convention of the other
content type tests.
Summary of ChangesHello @teng-lin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the unified message protocol by addressing several identified gaps. It ensures proper handling and consistent metadata for various content types across different adapters, improving the robustness and interoperability of message translation. The changes primarily focus on enhancing content block processing, standardizing metadata keys, and clarifying status inference logic, all while maintaining existing functionality and adding comprehensive test coverage. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request does an excellent job of closing gaps in the unified message protocol. The changes are well-reasoned and address several inconsistencies across different adapters, such as handling new content block types (image, code, refusal) and standardizing metadata keys like model and tool_name. The addition of comprehensive tests for the new functionality and for previously untested parts of the consumer message mapper significantly improves the robustness of the codebase. The documentation update in unified-message-router.ts is also a valuable clarification for future development. Additionally, consider extracting repeated logic blocks into a private helper method to improve maintainability and avoid code duplication, especially in tests.
| it("maps thinking content blocks", () => { | ||
| const msg = createUnifiedMessage({ | ||
| type: "assistant", | ||
| role: "assistant", | ||
| content: [{ type: "thinking", thinking: "Let me analyze...", budget_tokens: 5000 }], | ||
| metadata: { | ||
| message_id: "msg-007", | ||
| model: "claude-sonnet-4-5-20250929", | ||
| stop_reason: null, | ||
| usage: { | ||
| input_tokens: 0, | ||
| output_tokens: 0, | ||
| cache_creation_input_tokens: 0, | ||
| cache_read_input_tokens: 0, | ||
| }, | ||
| parent_tool_use_id: null, | ||
| }, | ||
| }); | ||
|
|
||
| const result = mapAssistantMessage(msg); | ||
| const assistant = result as Extract<typeof result, { type: "assistant" }>; | ||
| expect(assistant.message.content).toEqual([ | ||
| { type: "thinking", thinking: "Let me analyze...", budget_tokens: 5000 }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("maps code content blocks", () => { | ||
| const msg = createUnifiedMessage({ | ||
| type: "assistant", | ||
| role: "assistant", | ||
| content: [{ type: "code", language: "typescript", code: "const x = 1;" }], | ||
| metadata: { | ||
| message_id: "msg-008", | ||
| model: "claude-sonnet-4-5-20250929", | ||
| stop_reason: null, | ||
| usage: { | ||
| input_tokens: 0, | ||
| output_tokens: 0, | ||
| cache_creation_input_tokens: 0, | ||
| cache_read_input_tokens: 0, | ||
| }, | ||
| parent_tool_use_id: null, | ||
| }, | ||
| }); | ||
|
|
||
| const result = mapAssistantMessage(msg); | ||
| const assistant = result as Extract<typeof result, { type: "assistant" }>; | ||
| expect(assistant.message.content).toEqual([ | ||
| { type: "code", language: "typescript", code: "const x = 1;" }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("maps image content blocks with flattened source", () => { | ||
| const msg = createUnifiedMessage({ | ||
| type: "assistant", | ||
| role: "assistant", | ||
| content: [ | ||
| { | ||
| type: "image", | ||
| source: { type: "base64", media_type: "image/png", data: "iVBOR..." }, | ||
| }, | ||
| ], | ||
| metadata: { | ||
| message_id: "msg-009", | ||
| model: "claude-sonnet-4-5-20250929", | ||
| stop_reason: null, | ||
| usage: { | ||
| input_tokens: 0, | ||
| output_tokens: 0, | ||
| cache_creation_input_tokens: 0, | ||
| cache_read_input_tokens: 0, | ||
| }, | ||
| parent_tool_use_id: null, | ||
| }, | ||
| }); | ||
|
|
||
| const result = mapAssistantMessage(msg); | ||
| const assistant = result as Extract<typeof result, { type: "assistant" }>; | ||
| expect(assistant.message.content).toEqual([ | ||
| { type: "image", media_type: "image/png", data: "iVBOR..." }, | ||
| ]); | ||
| }); | ||
|
|
||
| it("maps refusal content blocks", () => { | ||
| const msg = createUnifiedMessage({ | ||
| type: "assistant", | ||
| role: "assistant", | ||
| content: [{ type: "refusal", refusal: "I cannot assist with that." }], | ||
| metadata: { | ||
| message_id: "msg-010", | ||
| model: "claude-sonnet-4-5-20250929", | ||
| stop_reason: null, | ||
| usage: { | ||
| input_tokens: 0, | ||
| output_tokens: 0, | ||
| cache_creation_input_tokens: 0, | ||
| cache_read_input_tokens: 0, | ||
| }, | ||
| parent_tool_use_id: null, | ||
| }, | ||
| }); | ||
|
|
||
| const result = mapAssistantMessage(msg); | ||
| const assistant = result as Extract<typeof result, { type: "assistant" }>; | ||
| expect(assistant.message.content).toEqual([ | ||
| { type: "refusal", refusal: "I cannot assist with that." }, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
These new tests for different content blocks are great additions. However, there's a significant amount of duplication in creating the test UnifiedMessage objects, specifically the metadata block which is repeated across four new tests.
To improve maintainability and reduce boilerplate, consider creating a helper function to generate these test messages. This would make the tests cleaner, easier to read, and simpler to update in the future.
For example:
function createTestAssistantMessage(content: UnifiedContent[], messageId: string) {
return createUnifiedMessage({
type: "assistant",
role: "assistant",
content,
metadata: {
message_id: messageId,
model: "claude-sonnet-4-5-20250929",
stop_reason: null,
usage: {
input_tokens: 0,
output_tokens: 0,
cache_creation_input_tokens: 0,
cache_read_input_tokens: 0,
},
parent_tool_use_id: null,
},
});
}
// Then in your test:
it("maps thinking content blocks", () => {
const msg = createTestAssistantMessage(
[{ type: "thinking", thinking: "Let me analyze...", budget_tokens: 5000 }],
"msg-007"
);
const result = mapAssistantMessage(msg);
const assistant = result as Extract<typeof result, { type: "assistant" }>;
expect(assistant.message.content).toEqual([
{ type: "thinking", thinking: "Let me analyze...", budget_tokens: 5000 },
]);
});References
- To improve maintainability and avoid code duplication, extract repeated logic blocks into a private helper method.
There was a problem hiding this comment.
Addressed in commit 34600bb: extracted makeAssistantMsg(content, messageId) helper to eliminate repeated metadata boilerplate across the 4 new content type tests. Net -40 lines.
… content types Exercises the full pipeline (CLIMessage → translate → router → consumer) for newly supported content block types. Verifies image source flattening, code block passthrough, refusal passthrough, and mixed content handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback from gemini-code-assist on PR #105.
Address the broader Gemini review feedback from PR #105 by converting pre-existing mapAssistantMessage tests to use the makeAssistantMsg helper, adding optional metadata overrides support. Net -49 lines.
Address the broader Gemini review feedback from PR #105 by converting pre-existing mapAssistantMessage tests to use the makeAssistantMsg helper, adding optional metadata overrides support. Net -49 lines.
1 participant
Summary
Closes 4 remaining open issues from the unified message protocol audit (Section 8):
ISSUE 1 — Claude adapter dropped
image/code/refusalcontent blocks to empty text. ExtendedContentBlockunion with 3 new variants and added switch cases intranslateAssistant()to pass them through instead of falling to thedefaultdrop path.ISSUE 2 — Metadata key divergence across adapters. Added canonical
modelkey to OpenCode adapter (alongside existingmodel_id) so the consumer mapper resolves it. Added canonicaltool_namekey to Codex adapter'sfunction_callbranches (bothtranslateItemAddedandtranslateItemDone).ISSUE 3 — Status "running" inference is Claude-specific. Expanded the comment in
handleStreamEvent()to document why generalizing it was rejected (false positives from sub-agent streams). No code change.ISSUE 4 — Consumer mapper tests were missing for 4 of 7 content types. Added tests for
thinking,code,image(with flattened source verification), andrefusalblocks. All pass immediately — the mapper already implemented these.Additionally cleaned up an unnecessary type cast in the thinking block handler now that TypeScript narrows correctly with the extended
ContentBlockunion.Files changed (9 files, +223/-4)
src/types/cli-messages.tsContentBlockunionsrc/adapters/claude/message-translator.tssrc/adapters/claude/message-translator.test.tssrc/adapters/opencode/opencode-message-translator.tsmodelkeysrc/adapters/opencode/opencode-message-translator.test.tsmodelkeysrc/adapters/codex/codex-message-translator.tstool_nameto 2 function_call metadata objectssrc/adapters/codex/codex-message-translator.test.tstool_namein both pathssrc/core/consumer-message-mapper.test.tssrc/core/unified-message-router.tsTest plan
npx tsc --noEmit— cleannpx vitest run— all 2441 tests pass (158 files)🤖 Generated with Claude Code