Feature/bug fix of thinking llm in vllm #3510

dog-last · 2025-09-27T09:00:45Z

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Please delete options that are not relevant.

Unit Test

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
I have checked my code and corrected any misspellings

Maintainer Checklist

closes #xxxx (Replace xxxx with the GitHub issue number)
Made sure Checks passed

CLAassistant · 2025-09-27T09:00:51Z

All committers have signed the CLA.

parshvadaftari

Hey thanks for raising the PR, can you please incorporate the requested changes?

parshvadaftari · 2025-09-29T19:23:12Z

mem0/memory/main.py

        )
        try:
            response = remove_code_blocks(response)
+            if '</think>' in response:


Hey instead of creating a new utils for removing "think" blocks, it would be better if you incorporate it into the remove_code_blocks. This will reduce the code redundancy and latency.

Hi, thanks for the review!
I've remove the remove_thinking_tagsfunction, and adjust the remove_code_blocks.
Is that OK?

parshvadaftari · 2025-09-29T19:24:35Z

tests/memory/test_thinking_tag.py

@@ -0,0 +1,135 @@
+from unittest.mock import MagicMock, patch


A bit skeptical about a entire separate test for removing the "thinking tags". Can we add this to test_vllm?

Move the test to test_vllm.py

parshvadaftari · 2025-09-29T19:24:58Z

bug__fix.md

@@ -0,0 +1,64 @@
+# Ensure when using vllm, the <think></think> tags are handled instead of throw an error


Could you please remove this?

dog-last · 2025-09-30T07:47:55Z

The latest change makes sure:

All feedback addressed.
Follows project style guide.
Tests passed.

Thanks again for your attention😊

parshvadaftari · 2025-10-01T08:37:15Z

tests/llms/test_vllm.py

+
+
+
+def create_mocked_memory():


This test is not required and also doesn't cover any vllm aspect.

parshvadaftari · 2025-10-01T08:37:20Z

tests/llms/test_vllm.py

+
+
+def create_mocked_async_memory():
+    """Create a fully mocked AsyncMemory instance for testing."""


This test is not required and also doesn't cover any vllm aspect.

parshvadaftari · 2025-10-01T08:38:25Z

tests/llms/test_vllm.py

+
+
+@pytest.mark.asyncio
+async def test_async_thinking_tags_in_add_to_vector_store():


A bit nit picking over here, but the test function name can be bitter, which doesn't have this long sentence like name.

parshvadaftari · 2025-10-01T08:38:40Z

tests/llms/test_vllm.py

+        return memory, mock_llm, mock_vector_store
+
+
+def test_thinking_tags_in_add_to_vector_store():


The name of the function seems to be a bit too long.

…hink></think> tags output by the LLM, instead of raise a JSON format error.

…the test to test_vllm.py instead of a entire separate test.

dog-last · 2025-10-02T08:46:36Z

Thank you for the valuable feedback! I've made the following changes as suggested:

Removed the test function that was recommended for deletion.
Renamed the two test functions with shorter names.
A new commit has been submitted with these updates. Please let me know if this meets the requirements or if further adjustments are needed.

parshvadaftari

Looks good to me.

parshvadaftari · 2025-10-03T13:43:25Z

Thanks for contributing to mem0.

parshvadaftari reviewed Sep 29, 2025

View reviewed changes

dog-last force-pushed the feature/bug-fix-of-thinking-llm-in-vllm branch from 3204212 to 0dad8d3 Compare September 30, 2025 07:21

parshvadaftari reviewed Oct 1, 2025

View reviewed changes

dog-last added 3 commits October 2, 2025 16:35

Add support for Thinking LLMs while using vllm. It will handle the <t…

c71b4b0

…hink></think> tags output by the LLM, instead of raise a JSON format error.

incorporate my feature into the remove_code_blocks function. And add …

86b8440

…the test to test_vllm.py instead of a entire separate test.

remove the useless test and rename two

4b671a9

dog-last force-pushed the feature/bug-fix-of-thinking-llm-in-vllm branch from 0dad8d3 to 4b671a9 Compare October 2, 2025 08:39

parshvadaftari approved these changes Oct 3, 2025

View reviewed changes

parshvadaftari merged commit 51ce6f1 into mem0ai:main Oct 3, 2025
6 of 7 checks passed

parshvadaftari mentioned this pull request Oct 7, 2025

Add support for deep-thinking models to avoid JSON parsing failures c… #3548

Closed

		@@ -0,0 +1,64 @@
		# Ensure when using vllm, the <think></think> tags are handled instead of throw an error



		def create_mocked_async_memory():
		"""Create a fully mocked AsyncMemory instance for testing."""



		@pytest.mark.asyncio
		async def test_async_thinking_tags_in_add_to_vector_store():

		return memory, mock_llm, mock_vector_store


		def test_thinking_tags_in_add_to_vector_store():

Feature/bug fix of thinking llm in vllm #3510

Feature/bug fix of thinking llm in vllm #3510

Uh oh!

Conversation

dog-last commented Sep 27, 2025

Description

Type of change

How Has This Been Tested?

Checklist:

Maintainer Checklist

Uh oh!

CLAassistant commented Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parshvadaftari left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dog-last commented Sep 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dog-last commented Oct 2, 2025

Uh oh!

parshvadaftari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

parshvadaftari commented Oct 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CLAassistant commented Sep 27, 2025 •

edited

Loading