Skip to content

Conversation

iSevenDays
Copy link
Contributor

Summary

Fixes a bug in Qwen3 content extraction where aggressive whitespace cleanup was breaking proper code formatting in non-tool-call streaming output.

Problem

The qwen3::extract_content_during_parsing() function was using an aggressive regex pattern that collapsed multiple newlines:

content = std::regex_replace(content, std::regex(R"(\n\s*\n)"), "\n");

This broke:

  • PEP 8 compliance: 2 empty lines between Python functions were collapsed to 1
  • Code formatting: Any intentional whitespace structure was lost
  • Streaming output: Users requesting properly formatted code received malformed results

Root Cause

The regex R"(\n\s*\n)" replaces any sequence of newline + whitespace + newline with a single newline, which is overly aggressive for content that should preserve formatting.

Solution

Replace aggressive cleanup with gentle trimming

  • Remove std::regex_replace(content, std::regex(R"(\n\s*\n)"), "\n")
  • Replace with string_strip(content) following original llama.cpp patterns
  • Only trim leading/trailing whitespace, preserve internal formatting

Add proper include

  • Add #include "../../common/common.h" for string_strip access

Add regression test

  • New test_qwen3_whitespace_preservation() validates PEP 8 compliance
  • Ensures 2 empty lines between functions are preserved

Testing

  • PEP 8 compliance: 2 empty lines between functions preserved
  • Tool call parsing: All Qwen3 tests continue to pass
  • No regressions: Existing functionality maintained
  • Follows patterns: Matches original llama.cpp whitespace handling

Example

Before (broken):

def celsius_to_fahrenheit(celsius):
    return celsius * 9/5 + 32
def fahrenheit_to_celsius(fahrenheit):  # Missing empty lines\!
    return (fahrenheit - 32) * 5/9

After (fixed):

def celsius_to_fahrenheit(celsius):
    return celsius * 9/5 + 32


def fahrenheit_to_celsius(fahrenheit):  # Proper PEP 8 spacing preserved\!
    return (fahrenheit - 32) * 5/9

Files Changed

  • examples/server/parsers/qwen3_parser.hpp - Fix whitespace handling
  • tests/test-function-calls.cpp - Add regression test

Test Results

🧹 Testing Qwen3 Whitespace Preservation Fix:
✅ PASS: Qwen3: PEP 8 double empty lines preserved
✅ PASS: Qwen3: Content not empty after processing
✅ PASS: Qwen3: Function content preserved
✅ PASS: Qwen3: Second function preserved

Problem:
- qwen3::extract_content_during_parsing() used aggressive regex to collapse multiple newlines
- This broke proper code formatting (e.g., PEP 8's 2 empty lines between functions)
- Affected non-tool-call streaming output where formatting is critical

Solution:
- Replace aggressive std::regex_replace(R"(\n\s*\n)", "\n") with gentle string_strip()
- Follow original llama.cpp patterns: only trim leading/trailing whitespace
- Preserve internal formatting including multiple newlines
- Add proper include for common.h to access string_strip function

Changes:
- examples/server/parsers/qwen3_parser.hpp: Replace whitespace cleanup with string_strip()
- tests/test-function-calls.cpp: Add test_qwen3_whitespace_preservation() to prevent regression

Testing:
- ✅ PEP 8 compliance: 2 empty lines between functions preserved
- ✅ Tool call parsing: All Qwen3 tests continue to pass
- ✅ No regressions: Existing functionality maintained
- ✅ Follows original llama.cpp whitespace handling patterns
@gopinath87607
Copy link

gopinath87607 commented Aug 2, 2025

hi i tested this pr in vs code continue extension with recent unsolth quant qwen 30b a3b coder but its still breaking when its calling second time tool and main repo not even working

update

looks like model itself there is a error let me check again

@gopinath87607
Copy link

gopinath87607 commented Aug 2, 2025

@iSevenDays
Copy link
Contributor Author

@gopinath87607 here is my latest branch updates
#670
I found some issues and differences between llama.cpp and ik_llama.cpp and fixed them. That's probably not everything, but works much stable.

@Danmoreng
Copy link

Apparently qwen3 coder uses a different tool calling structure with XML instead of json, llama.cpp currently ads the functionality here: ggml-org/llama.cpp#15012

@gopinath87607
Copy link

@gopinath87607 here is my latest branch updates #670 I found some issues and differences between llama.cpp and ik_llama.cpp and fixed them. That's probably not everything, but works much stable.

thanks mate i will check it do we need to use any special tag for tool activation like jinja

@ubergarm
Copy link
Contributor

ubergarm commented Aug 2, 2025

Some more discussion on similar qwen with toolcalling stuff here: https://huggingface.co/bartowski/Qwen_Qwen3-235B-A22B-Instruct-2507-GGUF/discussions/1

@ikawrakow ikawrakow merged commit dee40cf into ikawrakow:main Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants