Skip to content

fix: The tag based thinking process does not output #2968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shaohuzhang1 merged 1 commit into from
Apr 24, 2025
Merged

fix: The tag based thinking process does not output #2968

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apps/setting/models_provider/impl/base_chat_open_ai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ def _convert_delta_to_message_chunk(
_dict: Mapping[str, Any], default_class: type[BaseMessageChunk]
) -> BaseMessageChunk:
id_ = _dict.get("id")
reasoning_content = cast(str, _dict.get("reasoning_content") or "")
role = cast(str, _dict.get("role"))
content = cast(str, _dict.get("content") or "")
additional_kwargs: dict = {'reasoning_content': reasoning_content}
additional_kwargs: dict = {}
if 'reasoning_content' in _dict:
additional_kwargs['reasoning_content'] = _dict.get('reasoning_content')
if _dict.get("function_call"):
function_call = dict(_dict["function_call"])
if "name" in function_call and function_call["name"] is None:
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Apr 24, 2025

Choose a reason for hiding this comment

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

The provided code snippet seems to be part of a larger function _convert_delta_to_message_chunk responsible for converting various types into BaseMessageChunk objects. Here's an analysis along with some suggestions:

  1. Variable Naming:

    • The variable named additional_kwargs might be confusing, since it could potentially include other keyword arguments related to different message chunks types.

  2. Conditional Check:

    • The condition 'reasoning_content' in _dict: checks if the key is present before attempting to access the value. This is good practice as it prevents accessing keys that do not exist.

  3. Default Keyword Argument:

    • Adding "default_class=type(BaseMessageChunk) as an argument to the function does no harm but might obscure intent slightly. If this line was removed, clarity would increase slightly without changing behavior.

  4. Dictionary Initialization:

    • Initializing additional_kwargs as {} at the top ensures that dictionary creation occurs only once, which is preferable for performance reasons when dealing with large collections.

  5. Function Call Handling:

    • Ensuring that the function_call.name is either None or properly handled should ideally be done more robustly within the broader logic context where messages are being constructed.

  6. Overall Clarity:

    • The use of explicit casts (cast(str,)) and inline comments can help maintain clarity on data conversion and handling.

Here’s a revised version of the code incorporating these suggestions:

@@ -24,10 +24,12 @@ def _convert_delta_to_message_chunk(
     base_cls: type[BaseMessageChunk],
 ) -> BaseMessageChunk:
     """Converts delta-dictionary data structures into corresponding MessageChunk instances."""
     id_ = _dict.get("id", None)  # Default value set to None instead of "" for clarity
     role = cast(str, _dict.get("role"))
     content = cast(str, _dict.get("content", ""))
    
    addition_kwargs: dict = {}
    # Only add reasoning_content if it exists in the dictionary
    if "reasoning_content" in _dict:
        addition_kwargs["reasoning_content"] = _dict.get("reasoning_content")
    
    if "function_call" in _dict:
        function_call_data = _dict["function_call"].copy()  # Copy to avoid mutating original
        function_name = function_call_data.pop("name", None)
        
        # Additional processing or validation here based on requirements
        
        function_call = {"name": function_name}  # Create new dictionary containing name
        addition_kwargs.update(function_call)
    
    chunk_type = "regular" if not function_call else "function"
    
    try:
        return base_cls(id=id_, role=role, content=content, **addition_kwargs)
    except TypeError as e:
        raise ValueError(f"Failed to create {chunk_type} message due to invalid parameters.") from e

This revision improves readability, adds missing documentation where necessary, fixes unnecessary initializations, and clarifies certain details such as handling default values and creating new dictionaries.

Expand Down