chatGPTBox icon indicating copy to clipboard operation
chatGPTBox copied to clipboard

Add reasoning content support (for DeepSeek format API)

Open PasserbyAlpha opened this issue 9 months ago • 4 comments

For customAPI, try to load delta.reasoning_content and message.reasoning_content if it exists.

For DeepSeek-R1 reasoning_model

For GPT-4o common_model

Currently only delta part (streaming) has been tested (I don't know how to trigger a single-request chat completion with the repo).

I'm not familiar with nodejs. Please help me to improve it if possible, thank you!

PasserbyAlpha avatar Feb 19 '25 19:02 PasserbyAlpha

/review

PeterDaveHello avatar Jun 02 '25 18:06 PeterDaveHello

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Logic Flow

The streaming handling logic may have an issue where delta content is only added when reasoning content exists. Check if this could cause regular content to be missed when no reasoning is present.

if (delta) {
  if (with_reasoning && !has_reasoning_end) {
    answer += REASONING_END_SIGN
    has_reasoning_end = true
  }
  answer += delta
}
Edge Case

The code doesn't handle the case where reasoning_content exists but is empty. This could lead to unnecessary reasoning markers being added to the output.

if (delta_reasoning) {
  with_reasoning = true
  if (!has_reasoning_start) {
    answer += REASONING_START_SIGN
    has_reasoning_start = true
  }
  answer += delta_reasoning

qodo-merge-pro[bot] avatar Jun 02 '25 18:06 qodo-merge-pro[bot]

/improve

PeterDaveHello avatar Jun 02 '25 18:06 PeterDaveHello

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix reasoning marker logic

The current implementation adds the reasoning end sign for every delta chunk
when reasoning is present, but it should only be added once before transitioning
to the actual response. Add a check to ensure the reasoning end marker is only
added when transitioning from reasoning to response content.

src/services/apis/custom-api.mjs [91-107]

 if (delta !== undefined) {
   // streaming handling
   if (delta_reasoning) {
     with_reasoning = true
     if (!has_reasoning_start) {
       answer += REASONING_START_SIGN
       has_reasoning_start = true
     }
     answer += delta_reasoning
   }
   if (delta) {
-    if (with_reasoning && !has_reasoning_end) {
+    if (with_reasoning && !has_reasoning_end && delta_reasoning === undefined) {
       answer += REASONING_END_SIGN
       has_reasoning_end = true
     }
     answer += delta
   }
 }
  • [ ] Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a logical issue where the REASONING_END_SIGN could be added prematurely if both delta_reasoning and delta are present in the same chunk. Adding the delta_reasoning === undefined condition ensures the end marker is only added when transitioning from reasoning to response content.

Medium
  • [ ] More

qodo-merge-pro[bot] avatar Jun 02 '25 18:06 qodo-merge-pro[bot]