Skip to content
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.

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

fix: When the thinking process label is empty, do not process the thinking process #2179

Merged
merged 1 commit into from
Feb 9, 2025

Conversation

shaohuzhang1
Copy link
Contributor

fix: When the thinking process label is empty, do not process the thinking process

Copy link

f2c-ci-robot bot commented Feb 9, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

f2c-ci-robot bot commented Feb 9, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

return {'content': chunk.content, 'reasoning_content': ''}
# 如果没有结束思考过程标签那么就全部是思考过程
if self.reasoning_content_end_tag is None or len(self.reasoning_content_end_tag) == 0:
return {'content': '', 'reasoning_content': chunk.content}
self.all_content += chunk.content
if not self.reasoning_content_is_start and len(self.all_content) >= self.reasoning_content_start_tag_len:
if self.all_content.startswith(self.reasoning_content_start_tag):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided Python code includes some minor improvements, such as handling cases where either start or end tags might be missing by setting appropriate attributes to zero or an empty string when necessary. However, there is room for further optimization and clarity:

  1. Simplify Logic: The logic for determining start/end of the reasoning content can be simplified by checking conditions once and directly returning results based on these checks.

  2. Avoid Redundant Checks: The redundant len() calls for both tags within the same condition can be removed.

  3. Use Default Parameters: Consider using default parameters in method signatures that accept optional arguments with meaningful names, which improves readability and maintainability.

Here's an improved version of the code:

def __init__(self, reasoning_content_start="", reasoning_content_end=""):
        self.all_content = ""
        self.reasoning_content_start_tag = reasoning_content_start
        self.reasoning_content_end_tag = reasoning_content_end

        self.start_prefix_length = len(reasoning_content_start) if reasoning_content_start else 0
        self.end_suffix_length = len(reasoning_content_end) if reasoning_content_end else 0
        self.end_tag_prefix = reasoning_content_end[0] if self.end_suffix_length > 0 else ''

    def get_reasoning_content(self, chunk):
        all_content = self.all_content + chunk.content

        # Start of reasoning content detected
        if all_content.startswith(self.reasoning_content_start_tag):
            result = {
                'content': '',
                'reasoning_content': ''.join(all_content.split(chunk.content)[1:])
            }
            return result

        # End of reasoning content detected
        elif all_content.endswith(self.reasoning_content_end_tag):
            reason_text = "".join(all_content.split(chunk.content))
            return {
                'content': chunk.content,
                'reasoning_content': reason_text[len(self.reasoning_content_end_tag):].strip()
            }

        # No tag matches (either at the beginning or the middle)
        else:
            self.all_content = ""
            return {'content': chunk.content, 'reasoning_content': ''}

Key Improvements:

  • Single Check for Tags: The code now uses only one check (startswith followed by endswith) instead of two separate checks, reducing redundancy.
  • Default Parameter Names: Added descriptive parameter names (reasoning_content_start, reasoning_content_end) to improve understanding of their purpose.
  • String Manipulation Simplified: Used .split() and slicing to simplify extracting text between tags.
  • Return Statement Clarity: Each branch of the logic returns a dictionary with clear keys indicating whether it contains main content or reasoning content.

@shaohuzhang1 shaohuzhang1 merged commit e1f6502 into main Feb 9, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@fix_reasoning branch February 9, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant