From 466da48382a30e5bf1b384e6ecd1bc9ea76e363e Mon Sep 17 00:00:00 2001 From: DevelopmentCats Date: Thu, 5 Nov 2026 05:34:14 +0000 Subject: [PATCH 1/2] hf: use the reasoning parser a card's SGLang command names Qwen3's hybrid cards name --reasoning-parser deepseek_r1 in their vllm serve line and qwen3 in their SGLang line. Served with deepseek_r1 on vLLM 0.27.1, a non-streaming request with enable_thinking false returns content null and the whole answer in reasoning; qwen3 returns it in content. With thinking on, and when streaming, both parsers behave the same. Measured on a DGX Spark with Qwen3-0.6B. When the SGLang command names a parser the installed vLLM registers under that exact name, use it in place of the vllm serve line's, and say so in inspect, new and the panel. Cards whose lines agree, such as the Thinking-2507 ones (deepseek_r1 and deepseek-r1) and GLM-4.5 (glm45), are unchanged. Fixes #23 --- CHANGELOG.md | 11 ++++++ docs/models.md | 2 + internal/catalog/catalog.go | 14 ++++++- internal/cli/search.go | 3 ++ internal/hf/hf.go | 4 ++ internal/hf/hf_internal_test.go | 16 ++++++++ internal/hf/snippet.go | 35 +++++++++++++++++ internal/hf/snippet_internal_test.go | 21 +++++++++++ internal/hf/testdata/README_qwen3_hybrid.md | 9 +++++ internal/vllm/cardparser.go | 42 +++++++++++++++++++++ internal/vllm/cardparser_internal_test.go | 38 +++++++++++++++++++ internal/web/static/models.js | 5 ++- 12 files changed, 197 insertions(+), 3 deletions(-) create mode 100644 internal/hf/testdata/README_qwen3_hybrid.md create mode 100644 internal/vllm/cardparser.go create mode 100644 internal/vllm/cardparser_internal_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d11aaf..a5cc3c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,17 @@ the fact. ## [Unreleased] +### Fixed + +- Profiles for Qwen3 hybrid models got `--reasoning-parser deepseek_r1` from + the card's `vllm serve` line. With thinking turned off + (`enable_thinking: false`), a non-streaming reply then came back with + `content` empty and the answer in `reasoning`. When the card's SGLang + command names a different parser and the installed vLLM has one by that + name (Qwen3's cards name `qwen3`), that parser is used, and `inspect` and + `new` say so. Existing profiles keep their parser; change one with + `llm-helper set`. + ## [0.6.0] - 2026-11-04 ### Added diff --git a/docs/models.md b/docs/models.md index e0dae7f..842ffe2 100644 --- a/docs/models.md +++ b/docs/models.md @@ -12,6 +12,8 @@ llm-helper reads a model's HuggingFace card before anything is saved: it checks - `llm-helper inspect ` shows one repo's license, its weight size against this machine's budget, and the vLLM flags its card suggests — without pulling anything. `search`, `inspect`, and `new` pre-fill known vLLM serving flags from the model card: tool call parsers, reasoning parsers, and context length. +When the card's SGLang command names a different reasoning parser than its `vllm serve` line, and the installed vLLM has a parser by that exact name, that one is used and `inspect` says so. +Qwen3's cards are the case this covers: their `vllm serve` line names `deepseek_r1`, which puts the whole answer in `reasoning` when a request turns thinking off. `search` marks what is too big for this machine and what cannot load, and preselects the biggest quant that fits. `new` refuses a quant that cannot load rather than serving it — activating a profile restarts the service, so a model that cannot load would take the running one down with it. diff --git a/internal/catalog/catalog.go b/internal/catalog/catalog.go index 2405840..e6dfc69 100644 --- a/internal/catalog/catalog.go +++ b/internal/catalog/catalog.go @@ -102,6 +102,7 @@ type Detail struct { // vLLM won't take, so it's visible before pulling rather than as a // crash loop afterwards. DroppedFlags []string `json:"dropped_flags,omitempty"` + FlagNotes []string `json:"flag_notes,omitempty"` // KVBytesPerToken is what one token of context costs in KV cache, and // MaxContext the longest the model was trained for. Both 0 when the // config could not be read. @@ -230,7 +231,11 @@ func Inspect(ctx context.Context, m hf.Model, budget detect.WeightsBudget, caps if flags, err := hf.DetectFlags(ctx, m.ID); err == nil { // Validate here too: seeing that a card's parser won't work on this // machine is most useful before pulling tens of gigabytes. - d.DetectedFlags, d.DroppedFlags = vllm.Check(flags.ExtraArgs, caps) + args, note := vllm.PreferSGLangParser(flags.ExtraArgs, flags.SGLangReasoningParser, caps) + if note != "" { + d.FlagNotes = append(d.FlagNotes, note) + } + d.DetectedFlags, d.DroppedFlags = vllm.Check(args, caps) d.MaxModelLen = flags.MaxModelLen } return d @@ -255,6 +260,7 @@ func ForProvider(d Detail, prov provider.Provider) Detail { d.ArchBlocked = "" d.DetectedFlags = "" d.DroppedFlags = nil + d.FlagNotes = nil if !caps.GGUFQuant && d.NeedsGGUF { d.ArchBlocked = "only published as GGUF here, which " + prov.Name() + " does not load" } @@ -357,7 +363,11 @@ func PrepareProfile(ctx context.Context, dirs config.Dirs, repo string, choice * // author prescribed, the other is what this tool added. Merging them // before reporting attributed our own --tokenizer to the card, and said // "flags auto-detected from README" about a repo whose README had none. - cardArgs, dropped := vllm.Check(detected.ExtraArgs, caps) + cardArgs, note := vllm.PreferSGLangParser(detected.ExtraArgs, detected.SGLangReasoningParser, caps) + if note != "" { + p.Notes = append(p.Notes, note) + } + cardArgs, dropped := vllm.Check(cardArgs, caps) p.Notes = append(p.Notes, dropped...) if patched, needsPatch, err := hf.DetectChatTemplatePatch(ctx, repo); err != nil { diff --git a/internal/cli/search.go b/internal/cli/search.go index 0284e6f..6804f3b 100644 --- a/internal/cli/search.go +++ b/internal/cli/search.go @@ -260,6 +260,9 @@ func printModelDetail(d catalog.Detail, cfg *config.Config) { } else { fmt.Println(" vllm flags: no vllm serve snippet in README -- set flags by hand after pulling") } + for _, n := range d.FlagNotes { + fmt.Printf(" changed: %s\n", n) + } for _, w := range d.DroppedFlags { fmt.Printf(" dropped: %s\n", w) } diff --git a/internal/hf/hf.go b/internal/hf/hf.go index 6644572..396a5d8 100644 --- a/internal/hf/hf.go +++ b/internal/hf/hf.go @@ -690,6 +690,9 @@ func nextPageURL(header, want string) string { type DetectedFlags struct { ExtraArgs string // e.g. "--enable-auto-tool-choice --tool-call-parser qwen3_coder" MaxModelLen string // from --max-model-len in the snippet, "" if absent + // SGLangReasoningParser is the --reasoning-parser the card's SGLang + // command names, "" if none or if its SGLang commands disagree. + SGLangReasoningParser string } // DetectFlags fetches a model's README and pulls known vLLM serving flags @@ -720,6 +723,7 @@ func DetectFlags(ctx context.Context, repo string) (DetectedFlags, error) { if snippet := snippetRe.FindString(string(body)); snippet != "" { d.ExtraArgs, d.MaxModelLen = flagsFromSnippet(snippet) } + d.SGLangReasoningParser = sglangReasoningParser(string(body)) // Model cards routinely paste a snippet from a bigger sibling model, so // the advertised context can exceed what this model actually supports. diff --git a/internal/hf/hf_internal_test.go b/internal/hf/hf_internal_test.go index 148f985..530c7a8 100644 --- a/internal/hf/hf_internal_test.go +++ b/internal/hf/hf_internal_test.go @@ -1138,3 +1138,19 @@ func TestGGUFServable(t *testing.T) { t.Error("an unreadable config was treated as a refusal") } } + +func TestDetectFlagsReadsTheSGLangParser(t *testing.T) { + serveTestdata(t, map[string]string{ + "/" + repo + "/raw/main/README.md": "README_qwen3_hybrid.md", + }) + d, err := DetectFlags(context.Background(), repo) + if err != nil { + t.Fatalf("DetectFlags: %v", err) + } + if d.ExtraArgs != "--enable-reasoning --reasoning-parser deepseek_r1" { + t.Errorf("ExtraArgs = %q", d.ExtraArgs) + } + if d.SGLangReasoningParser != "qwen3" { + t.Errorf("SGLangReasoningParser = %q, want qwen3", d.SGLangReasoningParser) + } +} diff --git a/internal/hf/snippet.go b/internal/hf/snippet.go index 1e32764..562bff4 100644 --- a/internal/hf/snippet.go +++ b/internal/hf/snippet.go @@ -107,6 +107,41 @@ func flagsFromSnippet(snippet string) (extraArgs, maxModelLen string) { return strings.Join(kept, " "), maxModelLen } +var sglangSnippetRe = regexp.MustCompile(`sglang[._](?:launch_server|serve)(?:.*\\\n)*.*`) + +func sglangReasoningParser(card string) string { + found := "" + for _, snippet := range sglangSnippetRe.FindAllString(card, -1) { + name := reasoningParserIn(snippet) + if name == "" { + continue + } + if found != "" && found != name { + return "" + } + found = name + } + return found +} + +func reasoningParserIn(snippet string) string { + flat := strings.ReplaceAll(snippet, "\\\n", " ") + flat = strings.NewReplacer("\r", " ", "\n", " ", "\t", " ").Replace(flat) + args, err := config.SplitArgs(flat) + if err != nil { + return "" + } + for i, a := range args { + if v, ok := strings.CutPrefix(a, "--reasoning-parser="); ok { + return v + } + if a == "--reasoning-parser" && i+1 < len(args) && !isFlagToken(args[i+1]) { + return args[i+1] + } + } + return "" +} + // looksSecret keeps a credential pasted into a card out of a profile file. // Matching the trailing word spares real flags like --tokenizer-mode. func looksSecret(flag string) bool { diff --git a/internal/hf/snippet_internal_test.go b/internal/hf/snippet_internal_test.go index de15e59..a6be244 100644 --- a/internal/hf/snippet_internal_test.go +++ b/internal/hf/snippet_internal_test.go @@ -93,3 +93,24 @@ func TestFlagsFromSnippetUnparseable(t *testing.T) { t.Errorf("got %q, want nothing from an unparseable snippet", got) } } + +func TestSGLangReasoningParser(t *testing.T) { + t.Parallel() + tests := []struct { + name, card, want string + }{ + {"qwen3 hybrid card", "- SGLang:\n ```shell\n python -m sglang.launch_server --model-path Qwen/Qwen3-0.6B --reasoning-parser qwen3\n ```\n- vLLM:\n ```shell\n vllm serve Qwen/Qwen3-0.6B --enable-reasoning --reasoning-parser deepseek_r1\n ```\n", "qwen3"}, + {"continued lines", "python3 -m sglang.launch_server \\\n --model-path ./m \\\n --reasoning-parser=deepseek-r1\n", "deepseek-r1"}, + {"commands disagree", "sglang.launch_server --reasoning-parser qwen3\nsglang.launch_server --reasoning-parser deepseek-r1\n", ""}, + {"no parser", "python -m sglang.launch_server --model-path m\n", ""}, + {"no sglang command", "vllm serve m --reasoning-parser deepseek_r1\n", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := sglangReasoningParser(tt.card); got != tt.want { + t.Errorf("sglangReasoningParser() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/hf/testdata/README_qwen3_hybrid.md b/internal/hf/testdata/README_qwen3_hybrid.md new file mode 100644 index 0000000..244bc7b --- /dev/null +++ b/internal/hf/testdata/README_qwen3_hybrid.md @@ -0,0 +1,9 @@ +For deployment, you can use `sglang>=0.4.6.post1` or `vllm>=0.8.5` or to create an OpenAI-compatible API endpoint: +- SGLang: + ```shell + python -m sglang.launch_server --model-path Qwen/Qwen3-0.6B --reasoning-parser qwen3 + ``` +- vLLM: + ```shell + vllm serve Qwen/Qwen3-0.6B --enable-reasoning --reasoning-parser deepseek_r1 + ``` diff --git a/internal/vllm/cardparser.go b/internal/vllm/cardparser.go new file mode 100644 index 0000000..6ef4443 --- /dev/null +++ b/internal/vllm/cardparser.go @@ -0,0 +1,42 @@ +package vllm + +import ( + "fmt" + "strings" + + "github.com/coder/vllm-helper/internal/config" +) + +// PreferSGLangParser replaces the card's vLLM --reasoning-parser with the one +// its SGLang command names, when this vLLM registers that exact name. +func PreferSGLangParser(extraArgs, sglangParser string, caps Caps) (string, string) { + if sglangParser == "" || !caps.Known() || !caps.HasReasoningParser(sglangParser) { + return extraArgs, "" + } + args, err := config.SplitArgs(extraArgs) + if err != nil { + return extraArgs, "" + } + var kept []string + old := "" + for i := 0; i < len(args); i++ { + flag, value, ok := splitFlag(args, &i) + switch { + case !ok: + kept = append(kept, args[i]) + case flag == "--reasoning-parser" && value != "" && value != sglangParser: + old = value + kept = append(kept, flag+" "+config.Quote(sglangParser)) + case value == "": + kept = append(kept, flag) + default: + kept = append(kept, flag+" "+config.Quote(value)) + } + } + if old == "" { + return extraArgs, "" + } + return strings.Join(kept, " "), fmt.Sprintf( + "using --reasoning-parser %s instead of the card's %s: the card's SGLang command names %s, and vLLM %s has it", + sglangParser, old, sglangParser, caps.Version) +} diff --git a/internal/vllm/cardparser_internal_test.go b/internal/vllm/cardparser_internal_test.go new file mode 100644 index 0000000..7f4f20b --- /dev/null +++ b/internal/vllm/cardparser_internal_test.go @@ -0,0 +1,38 @@ +package vllm + +import ( + "strings" + "testing" +) + +func TestPreferSGLangParser(t *testing.T) { + t.Parallel() + tests := []struct { + name, args, sglang, want string + caps Caps + changed bool + }{ + {"qwen3 hybrid", "--enable-reasoning --reasoning-parser deepseek_r1", "qwen3", "--enable-reasoning --reasoning-parser qwen3", caps0271, true}, + {"equals form", "--reasoning-parser=deepseek_r1 --enforce-eager", "qwen3", "--reasoning-parser qwen3 --enforce-eager", caps0271, true}, + {"sglang name not registered", "--reasoning-parser deepseek_r1", "deepseek-r1", "--reasoning-parser deepseek_r1", caps0271, false}, + {"same parser", "--reasoning-parser qwen3", "qwen3", "--reasoning-parser qwen3", caps0271, false}, + {"no reasoning parser", "--enforce-eager", "qwen3", "--enforce-eager", caps0271, false}, + {"no sglang parser", "--reasoning-parser deepseek_r1", "", "--reasoning-parser deepseek_r1", caps0271, false}, + {"caps unknown", "--reasoning-parser deepseek_r1", "qwen3", "--reasoning-parser deepseek_r1", Caps{}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, note := PreferSGLangParser(tt.args, tt.sglang, tt.caps) + if got != tt.want { + t.Errorf("args = %q, want %q", got, tt.want) + } + if tt.changed != (note != "") { + t.Errorf("note = %q, want changed=%v", note, tt.changed) + } + if tt.changed && !strings.Contains(note, "deepseek_r1") { + t.Errorf("note %q does not name the card's parser", note) + } + }) + } +} diff --git a/internal/web/static/models.js b/internal/web/static/models.js index 600b8fa..117a6a2 100644 --- a/internal/web/static/models.js +++ b/internal/web/static/models.js @@ -460,7 +460,10 @@ async function inspect(repo) { el.flags.hidden = !d.detected_flags; if (d.detected_flags) { - el.flags.textContent = `Serving flags found on the model card and accepted by your vLLM: ${d.detected_flags}`; + el.flags.textContent = [ + `Serving flags found on the model card and accepted by your vLLM: ${d.detected_flags}`, + ...(d.flag_notes ?? []), + ].join("\n"); } el.dropped.hidden = !d.dropped_flags?.length; if (d.dropped_flags?.length) { From 63032721f80c039f094d470a9df9370b9ddff0c5 Mon Sep 17 00:00:00 2001 From: DevelopmentCats Date: Thu, 5 Nov 2026 05:45:31 +0000 Subject: [PATCH 2/2] hf: apply parser defaults after picking the SGLang parser Replacing the vllm serve line's --reasoning-parser after the snippet was parsed skipped the --default-chat-template-kwargs {"thinking":true} that deepseek_v3 and poolside_v1 need. DetectFlags now rebuilds the card's flags with the SGLang parser substituted, so those defaults follow the parser that is actually used. --- internal/catalog/catalog.go | 15 ++++++-- internal/catalog/catalog_internal_test.go | 37 ++++++++++++++++++++ internal/hf/hf.go | 12 +++++-- internal/hf/hf_internal_test.go | 6 ++++ internal/hf/snippet.go | 14 ++++++-- internal/hf/snippet_internal_test.go | 21 ++++++++++++ internal/vllm/cardparser.go | 42 ----------------------- internal/vllm/cardparser_internal_test.go | 38 -------------------- 8 files changed, 99 insertions(+), 86 deletions(-) delete mode 100644 internal/vllm/cardparser.go delete mode 100644 internal/vllm/cardparser_internal_test.go diff --git a/internal/catalog/catalog.go b/internal/catalog/catalog.go index e6dfc69..1d56d1a 100644 --- a/internal/catalog/catalog.go +++ b/internal/catalog/catalog.go @@ -231,7 +231,7 @@ func Inspect(ctx context.Context, m hf.Model, budget detect.WeightsBudget, caps if flags, err := hf.DetectFlags(ctx, m.ID); err == nil { // Validate here too: seeing that a card's parser won't work on this // machine is most useful before pulling tens of gigabytes. - args, note := vllm.PreferSGLangParser(flags.ExtraArgs, flags.SGLangReasoningParser, caps) + args, note := cardFlags(flags, caps) if note != "" { d.FlagNotes = append(d.FlagNotes, note) } @@ -241,6 +241,17 @@ func Inspect(ctx context.Context, m hf.Model, budget detect.WeightsBudget, caps return d } +// cardFlags returns the card's vllm serve flags, rebuilt around the parser its +// SGLang command names when this vLLM registers that name. +func cardFlags(flags hf.DetectedFlags, caps vllm.Caps) (args, note string) { + if flags.SGLangExtraArgs == "" || !caps.Known() || !caps.HasReasoningParser(flags.SGLangReasoningParser) { + return flags.ExtraArgs, "" + } + return flags.SGLangExtraArgs, fmt.Sprintf( + "using --reasoning-parser %s instead of the card's %s: the card's SGLang command names %s, and vLLM %s has it", + flags.SGLangReasoningParser, flags.ReasoningParser, flags.SGLangReasoningParser, caps.Version) +} + // ForProvider reshapes what Inspect found for the server that will load the // model, since Inspect judges GGUF by vLLM's plugin. A server that picks its // own GGUF quant has none of the plugin's limits, so split and subfolder @@ -363,7 +374,7 @@ func PrepareProfile(ctx context.Context, dirs config.Dirs, repo string, choice * // author prescribed, the other is what this tool added. Merging them // before reporting attributed our own --tokenizer to the card, and said // "flags auto-detected from README" about a repo whose README had none. - cardArgs, note := vllm.PreferSGLangParser(detected.ExtraArgs, detected.SGLangReasoningParser, caps) + cardArgs, note := cardFlags(detected, caps) if note != "" { p.Notes = append(p.Notes, note) } diff --git a/internal/catalog/catalog_internal_test.go b/internal/catalog/catalog_internal_test.go index 129382f..4e059cb 100644 --- a/internal/catalog/catalog_internal_test.go +++ b/internal/catalog/catalog_internal_test.go @@ -763,3 +763,40 @@ func TestAPullWithoutAQuantChoiceStillMeasuresTheWeights(t *testing.T) { t.Error("the pull planned a KV cache without ever measuring the model it has to fit around") } } + +func TestCardFlagsPrefersARegisteredSGLangParser(t *testing.T) { + t.Parallel() + qwen := hf.DetectedFlags{ + ExtraArgs: "--reasoning-parser deepseek_r1", + ReasoningParser: "deepseek_r1", + SGLangReasoningParser: "qwen3", + SGLangExtraArgs: "--reasoning-parser qwen3", + } + unregistered := qwen + unregistered.SGLangReasoningParser = "deepseek-r1" + unregistered.SGLangExtraArgs = "--reasoning-parser deepseek-r1" + tests := []struct { + name string + flags hf.DetectedFlags + caps vllm.Caps + want string + changed bool + }{ + {"registered", qwen, spark, "--reasoning-parser qwen3", true}, + {"not registered", unregistered, spark, "--reasoning-parser deepseek_r1", false}, + {"caps unknown", qwen, vllm.Caps{}, "--reasoning-parser deepseek_r1", false}, + {"parsers agree", hf.DetectedFlags{ExtraArgs: "--reasoning-parser qwen3", SGLangReasoningParser: "qwen3"}, spark, "--reasoning-parser qwen3", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, note := cardFlags(tt.flags, tt.caps) + if got != tt.want { + t.Errorf("args = %q, want %q", got, tt.want) + } + if tt.changed != (note != "") { + t.Errorf("note = %q, want changed=%v", note, tt.changed) + } + }) + } +} diff --git a/internal/hf/hf.go b/internal/hf/hf.go index 396a5d8..a5f897f 100644 --- a/internal/hf/hf.go +++ b/internal/hf/hf.go @@ -693,6 +693,11 @@ type DetectedFlags struct { // SGLangReasoningParser is the --reasoning-parser the card's SGLang // command names, "" if none or if its SGLang commands disagree. SGLangReasoningParser string + // ReasoningParser is the --reasoning-parser the vllm serve line names. + ReasoningParser string + // SGLangExtraArgs is ExtraArgs rebuilt with SGLangReasoningParser, set + // only when the two parsers differ. + SGLangExtraArgs string } // DetectFlags fetches a model's README and pulls known vLLM serving flags @@ -720,10 +725,13 @@ func DetectFlags(ctx context.Context, repo string) (DetectedFlags, error) { // context clamp below -- those models are exactly the ones whose real // context is far under the generic default. snippetRe := regexp.MustCompile(`vllm serve(?:.*\\\n)*.*`) + d.SGLangReasoningParser = sglangReasoningParser(string(body)) if snippet := snippetRe.FindString(string(body)); snippet != "" { - d.ExtraArgs, d.MaxModelLen = flagsFromSnippet(snippet) + d.ExtraArgs, d.MaxModelLen, d.ReasoningParser = parseSnippet(snippet, "") + if d.ReasoningParser != "" && d.SGLangReasoningParser != "" && d.ReasoningParser != d.SGLangReasoningParser { + d.SGLangExtraArgs, _, _ = parseSnippet(snippet, d.SGLangReasoningParser) + } } - d.SGLangReasoningParser = sglangReasoningParser(string(body)) // Model cards routinely paste a snippet from a bigger sibling model, so // the advertised context can exceed what this model actually supports. diff --git a/internal/hf/hf_internal_test.go b/internal/hf/hf_internal_test.go index 530c7a8..a5bc2da 100644 --- a/internal/hf/hf_internal_test.go +++ b/internal/hf/hf_internal_test.go @@ -1153,4 +1153,10 @@ func TestDetectFlagsReadsTheSGLangParser(t *testing.T) { if d.SGLangReasoningParser != "qwen3" { t.Errorf("SGLangReasoningParser = %q, want qwen3", d.SGLangReasoningParser) } + if d.ReasoningParser != "deepseek_r1" { + t.Errorf("ReasoningParser = %q, want deepseek_r1", d.ReasoningParser) + } + if d.SGLangExtraArgs != "--enable-reasoning --reasoning-parser qwen3" { + t.Errorf("SGLangExtraArgs = %q", d.SGLangExtraArgs) + } } diff --git a/internal/hf/snippet.go b/internal/hf/snippet.go index 562bff4..ea41475 100644 --- a/internal/hf/snippet.go +++ b/internal/hf/snippet.go @@ -53,6 +53,13 @@ var thinkingParsers = regexp.MustCompile(`^(deepseek_v3|poolside_v1)$`) // name is real depends on the vLLM installed on this machine, which is not // something a HuggingFace client should know about. func flagsFromSnippet(snippet string) (extraArgs, maxModelLen string) { + extraArgs, maxModelLen, _ = parseSnippet(snippet, "") + return extraArgs, maxModelLen +} + +// parseSnippet is flagsFromSnippet that also returns the --reasoning-parser it +// kept, replaced by parser when that is set. +func parseSnippet(snippet, parser string) (extraArgs, maxModelLen, reasoningParser string) { // Fold shell line continuations, then flatten what is left. The backslash // and the newline are separate concerns: a card can wrap without one, and // a bare newline left in place gets read as the previous flag's value. @@ -62,7 +69,7 @@ func flagsFromSnippet(snippet string) (extraArgs, maxModelLen string) { if err != nil { // A card whose snippet isn't valid shell (prose wrapped mid-line, // an unbalanced quote) yields nothing rather than half of it. - return "", "" + return "", "", "" } var kept []string @@ -92,6 +99,9 @@ func flagsFromSnippet(snippet string) (extraArgs, maxModelLen string) { continue } if flag == "--reasoning-parser" { + if parser != "" { + value = parser + } sawParser = value } if value == "" { @@ -104,7 +114,7 @@ func flagsFromSnippet(snippet string) (extraArgs, maxModelLen string) { if thinkingParsers.MatchString(sawParser) { kept = append(kept, `--default-chat-template-kwargs {"thinking":true}`) } - return strings.Join(kept, " "), maxModelLen + return strings.Join(kept, " "), maxModelLen, sawParser } var sglangSnippetRe = regexp.MustCompile(`sglang[._](?:launch_server|serve)(?:.*\\\n)*.*`) diff --git a/internal/hf/snippet_internal_test.go b/internal/hf/snippet_internal_test.go index a6be244..286f803 100644 --- a/internal/hf/snippet_internal_test.go +++ b/internal/hf/snippet_internal_test.go @@ -94,6 +94,27 @@ func TestFlagsFromSnippetUnparseable(t *testing.T) { } } +func TestParseSnippetReplacesTheParser(t *testing.T) { + t.Parallel() + tests := []struct { + name, snippet, parser, want string + }{ + {"into deepseek_v3", `vllm serve org/m --reasoning-parser deepseek_r1`, "deepseek_v3", `--reasoning-parser deepseek_v3 --default-chat-template-kwargs {"thinking":true}`}, + {"away from deepseek_v3", `vllm serve org/m --reasoning-parser deepseek_v3`, "qwen3", "--reasoning-parser qwen3"}, + {"equals form", `vllm serve org/m --reasoning-parser=deepseek_r1 --enforce-eager`, "qwen3", "--reasoning-parser qwen3 --enforce-eager"}, + {"no parser in the snippet", `vllm serve org/m --enforce-eager`, "qwen3", "--enforce-eager"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, _, _ := parseSnippet(tt.snippet, tt.parser) + if got != tt.want { + t.Errorf("parseSnippet() = %q, want %q", got, tt.want) + } + }) + } +} + func TestSGLangReasoningParser(t *testing.T) { t.Parallel() tests := []struct { diff --git a/internal/vllm/cardparser.go b/internal/vllm/cardparser.go deleted file mode 100644 index 6ef4443..0000000 --- a/internal/vllm/cardparser.go +++ /dev/null @@ -1,42 +0,0 @@ -package vllm - -import ( - "fmt" - "strings" - - "github.com/coder/vllm-helper/internal/config" -) - -// PreferSGLangParser replaces the card's vLLM --reasoning-parser with the one -// its SGLang command names, when this vLLM registers that exact name. -func PreferSGLangParser(extraArgs, sglangParser string, caps Caps) (string, string) { - if sglangParser == "" || !caps.Known() || !caps.HasReasoningParser(sglangParser) { - return extraArgs, "" - } - args, err := config.SplitArgs(extraArgs) - if err != nil { - return extraArgs, "" - } - var kept []string - old := "" - for i := 0; i < len(args); i++ { - flag, value, ok := splitFlag(args, &i) - switch { - case !ok: - kept = append(kept, args[i]) - case flag == "--reasoning-parser" && value != "" && value != sglangParser: - old = value - kept = append(kept, flag+" "+config.Quote(sglangParser)) - case value == "": - kept = append(kept, flag) - default: - kept = append(kept, flag+" "+config.Quote(value)) - } - } - if old == "" { - return extraArgs, "" - } - return strings.Join(kept, " "), fmt.Sprintf( - "using --reasoning-parser %s instead of the card's %s: the card's SGLang command names %s, and vLLM %s has it", - sglangParser, old, sglangParser, caps.Version) -} diff --git a/internal/vllm/cardparser_internal_test.go b/internal/vllm/cardparser_internal_test.go deleted file mode 100644 index 7f4f20b..0000000 --- a/internal/vllm/cardparser_internal_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package vllm - -import ( - "strings" - "testing" -) - -func TestPreferSGLangParser(t *testing.T) { - t.Parallel() - tests := []struct { - name, args, sglang, want string - caps Caps - changed bool - }{ - {"qwen3 hybrid", "--enable-reasoning --reasoning-parser deepseek_r1", "qwen3", "--enable-reasoning --reasoning-parser qwen3", caps0271, true}, - {"equals form", "--reasoning-parser=deepseek_r1 --enforce-eager", "qwen3", "--reasoning-parser qwen3 --enforce-eager", caps0271, true}, - {"sglang name not registered", "--reasoning-parser deepseek_r1", "deepseek-r1", "--reasoning-parser deepseek_r1", caps0271, false}, - {"same parser", "--reasoning-parser qwen3", "qwen3", "--reasoning-parser qwen3", caps0271, false}, - {"no reasoning parser", "--enforce-eager", "qwen3", "--enforce-eager", caps0271, false}, - {"no sglang parser", "--reasoning-parser deepseek_r1", "", "--reasoning-parser deepseek_r1", caps0271, false}, - {"caps unknown", "--reasoning-parser deepseek_r1", "qwen3", "--reasoning-parser deepseek_r1", Caps{}, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - got, note := PreferSGLangParser(tt.args, tt.sglang, tt.caps) - if got != tt.want { - t.Errorf("args = %q, want %q", got, tt.want) - } - if tt.changed != (note != "") { - t.Errorf("note = %q, want changed=%v", note, tt.changed) - } - if tt.changed && !strings.Contains(note, "deepseek_r1") { - t.Errorf("note %q does not name the card's parser", note) - } - }) - } -}