fix: preserve ThinkingPart fields in OpenCode storage round-trip (#156) - #222
Conversation
ThinkingPart.id, provider_name, signature, and provider_details were silently dropped during OpenCode storage serialization. The write path (provider.py) only saved content as ReasoningPart.text, and the read path (helpers.py) only restored content — all other fields defaulted to None. This caused send-back to degrade to tags mode: pydantic-ai's _map_response_thinking_part() checks id + provider_name to decide between field mode (reasoning_content) and tags mode. Without these fields, thinking content was wrapped in <think> tags instead of sent via the proper reasoning_content field. Fix: store the extra fields in ReasoningPart.metadata during write, and restore them when constructing ThinkingPart during read. The metadata field already exists on ReasoningPart and is a dict[str, Any], making it a natural extensibility bucket without model changes. Backward compatible: old data without metadata produces ThinkingPart with None defaults, same as before.
![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 implements the preservation of ThinkingPart fields (id, provider_name, signature, and provider_details) through the OpenCode storage round-trip cycle by mapping them to and from the metadata field of ReasoningPart. It also adds comprehensive unit tests to verify this behavior. The reviewer suggests simplifying the manual extraction and construction of metadata dictionaries in both helpers.py and provider.py using loops or dictionary comprehensions to improve maintainability and idiomaticity.
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.
| thinking_kwargs: dict[str, Any] = {"content": part.text} | ||
| meta = part.metadata | ||
| if meta: | ||
| if meta.get("thinking_id") is not None: | ||
| thinking_kwargs["id"] = meta["thinking_id"] | ||
| if meta.get("provider_name") is not None: | ||
| thinking_kwargs["provider_name"] = meta["provider_name"] | ||
| if meta.get("signature") is not None: | ||
| thinking_kwargs["signature"] = meta["signature"] | ||
| if meta.get("provider_details") is not None: | ||
| thinking_kwargs["provider_details"] = meta["provider_details"] | ||
| response_parts.append(ThinkingPart(**thinking_kwargs)) |
There was a problem hiding this comment.
The manual extraction of metadata fields can be simplified and made more maintainable by using a dictionary mapping and a loop. Additionally, when processing messages loaded from storage where 'content' may be None, use defensive 'or ""' checks to normalize None to an empty string. Also, avoid implicit truthiness checks on optional dictionary/mapping parameters and use explicit 'is not None' checks instead.
| thinking_kwargs: dict[str, Any] = {"content": part.text} | |
| meta = part.metadata | |
| if meta: | |
| if meta.get("thinking_id") is not None: | |
| thinking_kwargs["id"] = meta["thinking_id"] | |
| if meta.get("provider_name") is not None: | |
| thinking_kwargs["provider_name"] = meta["provider_name"] | |
| if meta.get("signature") is not None: | |
| thinking_kwargs["signature"] = meta["signature"] | |
| if meta.get("provider_details") is not None: | |
| thinking_kwargs["provider_details"] = meta["provider_details"] | |
| response_parts.append(ThinkingPart(**thinking_kwargs)) | |
| thinking_kwargs: dict[str, Any] = {"content": part.text or ""} | |
| if part.metadata is not None: | |
| mapping = { | |
| "thinking_id": "id", | |
| "provider_name": "provider_name", | |
| "signature": "signature", | |
| "provider_details": "provider_details", | |
| } | |
| for meta_key, part_key in mapping.items(): | |
| if (val := part.metadata.get(meta_key)) is not None: | |
| thinking_kwargs[part_key] = val | |
| response_parts.append(ThinkingPart(**thinking_kwargs)) |
References
- PEP 8 recommends writing clean, idiomatic, and maintainable code, avoiding redundant or repetitive conditional blocks where a loop or comprehension is more appropriate. (link)
- When processing messages loaded from storage where 'content' may be None, use defensive 'or ""' checks to normalize None to an empty string so that falsy values can be cleanly filtered by guards.
- Avoid using implicit truthiness checks or intermediate boolean variables to check for the existence of optional dictionary/mapping parameters when type-narrowing is required. Use explicit
is not Nonechecks directly in the conditional statement so that static type checkers like mypy can correctly narrow the type.
| reasoning_metadata: dict[str, Any] = {} | ||
| if part.id is not None: | ||
| reasoning_metadata["thinking_id"] = part.id | ||
| if part.provider_name is not None: | ||
| reasoning_metadata["provider_name"] = part.provider_name | ||
| if part.signature is not None: | ||
| reasoning_metadata["signature"] = part.signature | ||
| if part.provider_details is not None: | ||
| reasoning_metadata["provider_details"] = part.provider_details |
There was a problem hiding this comment.
The manual construction of the metadata dictionary can be simplified using a dictionary comprehension. This is more idiomatic and concise.
reasoning_metadata = {
k: v
for k, v in {
"thinking_id": part.id,
"provider_name": part.provider_name,
"signature": part.signature,
"provider_details": part.provider_details,
}.items()
if v is not None
}References
- PEP 8 recommends writing clean, idiomatic, and maintainable code, avoiding redundant or repetitive conditional blocks where a loop or comprehension is more appropriate. (link)
|
Closing in favor of a new PR rebased on |
1 participant
Summary
Fixes #156
ThinkingPart.id, provider_name, signature, and provider_details were silently dropped during OpenCode storage serialization. The write path only saved content as ReasoningPart.text, and the read path only restored content — all other fields defaulted to None.
This caused send-back to degrade to tags mode: pydantic-ai checks id + provider_name to decide between field mode (reasoning_content) and tags mode. Without these fields, thinking content was wrapped in tags instead of sent via the proper reasoning_content field.
Fix
Store the extra fields in ReasoningPart.metadata during write, and restore them when constructing ThinkingPart during read. The metadata field already exists on ReasoningPart and is a dict[str, Any], making it a natural extensibility bucket without model changes.
Changes
Backward Compatibility
Old data without metadata produces ThinkingPart with None defaults — same behavior as before this fix.
Test Plan
Relationship to #155 and #174
Three independent fixes, no overlap.