Skip to content

fix(memory): add dedicated SGLang and vLLM thinking transports - #312

Closed
LeslieWylie wants to merge 1 commit into
MemTensor:mainfrom
LeslieWylie:codex/qwen-chat-template-thinking
Closed

fix(memory): add dedicated SGLang and vLLM thinking transports#312
LeslieWylie wants to merge 1 commit into
MemTensor:mainfrom
LeslieWylie:codex/qwen-chat-template-thinking

Conversation

@LeslieWylie

@LeslieWylie LeslieWylie commented Aug 27, 2026

Copy link
Copy Markdown

Summary

Add dedicated sglang and vllm memory LLM providers for the Qwen chat_template_kwargs.enable_thinking transport.

The generic openai_compatible provider keeps its existing top-level request shape, so cloud-provider calls are unaffected.

Changes

  • accept sglang and vllm as explicit LLM provider values
  • route both providers through the existing OpenAI-compatible chat-completions client
  • move Qwen enable_thinking into chat_template_kwargs only for those dedicated providers
  • preserve caller-supplied chat_template_kwargs, including an explicit enable_thinking override
  • keep top-level enable_thinking behavior unchanged for DashScope and generic OpenAI-compatible endpoints

Validation

  • vitest run Memory/tests/llm-thinking.test.ts — 21 tests passed
  • coverage includes SGLang, vLLM, disabled/enabled thinking, preserved template kwargs, and generic-provider isolation
  • live SGLang replay previously confirmed that top-level enable_thinking=false was ignored while chat_template_kwargs.enable_thinking=false produced zero reasoning tokens and valid JSON

@Wang-Daoji

Copy link
Copy Markdown
Collaborator

Thanks for working on this! I suggest adding chat_template_kwargs later as part of dedicated SGLang/vLLM providers. Adding it to the generic OpenAI-compatible path now could affect existing cloud-provider calls and introduce unnecessary risk.

Route Qwen enable_thinking through chat_template_kwargs only for explicit sglang and vllm providers. Keep the generic OpenAI-compatible request body unchanged so cloud-provider calls retain their existing contract.
@LeslieWylie LeslieWylie closed this Sep 3, 2026
@LeslieWylie
LeslieWylie force-pushed the codex/qwen-chat-template-thinking branch from 415f802 to 5410110 Compare September 3, 2026 07:49
@LeslieWylie LeslieWylie changed the title fix(memory): route Qwen thinking through chat template kwargs fix(memory): add dedicated SGLang and vLLM thinking transports Sep 3, 2026
@LeslieWylie

Copy link
Copy Markdown
Author

Thanks — I reworked the PR around that boundary. chat_template_kwargs is now used only by explicit sglang and vllm providers; the generic openai_compatible path keeps its existing top-level request shape. I also added coverage that pins the generic-provider isolation.

@LeslieWylie

Copy link
Copy Markdown
Author

Superseded by clean-history replacement #348. An amend from a shallow checkout accidentally removed this branch's base ancestry, so GitHub closed the PR and would not reopen it after the ancestry was repaired. The replacement keeps the maintainer-requested provider isolation and the passing 21-test validation.

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.

2 participants