Skip to content

Commit 450f5ed

Browse files
committed
test(ollama): improve patch coverage for native thinking support
Add tests covering previously uncovered branches in native-ollama.ts: - Anthropic-protocol thinking block round-tripping (block.type === thinking) - Concatenation of multiple reasoning/thinking blocks with newline joins - Empty reasoning/thinking blocks (length > 0 false branch + reasoningText || undefined) - Plain assistant message without reasoning blocks (falsy reasoningText branch) - Unknown reasoningEffort value (default switch branch returns undefined) - Stream processing error wrapping (catch streamError branch + Unknown error fallback) - Non-ECONNREFUSED non-404 error rethrow (fall-through throw error branch) Also strengthen the Ollama.spec.tsx toggle-off assertion per CodeRabbit nitpick: assert no call with reasoningEffort as the first argument at all, instead of only ruling out undefined. Patch coverage for native-ollama.ts rises from 71.05% to ~100% of new lines, clearing the codecov/patch 80% threshold.
1 parent 0b0a3df commit 450f5ed

2 files changed

Lines changed: 226 additions & 1 deletion

File tree

src/api/providers/__tests__/native-ollama.spec.ts

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,179 @@ describe("NativeOllamaHandler", () => {
479479
}),
480480
)
481481
})
482+
it("should round-trip Anthropic-protocol thinking blocks as the thinking field on assistant messages", async () => {
483+
// Covers the `block.type === "thinking"` branch in the assistant
484+
// message converter. Anthropic-protocol thinking blocks carry the
485+
// reasoning text in a `thinking` field (not `text`).
486+
mockChat.mockImplementation(async function* () {
487+
yield { message: { content: "ok" } }
488+
})
489+
490+
const messages: Anthropic.Messages.MessageParam[] = [
491+
{
492+
role: "assistant",
493+
content: [
494+
{ type: "thinking", thinking: "Anthropic thinking text" } as any,
495+
{ type: "text", text: "Prior answer" },
496+
],
497+
},
498+
{ role: "user" as const, content: "Follow up" },
499+
]
500+
501+
const stream = handler.createMessage("System", messages)
502+
for await (const _ of stream) {
503+
// consume
504+
}
505+
506+
expect(mockChat).toHaveBeenCalledWith(
507+
expect.objectContaining({
508+
messages: expect.arrayContaining([
509+
expect.objectContaining({
510+
role: "assistant",
511+
thinking: "Anthropic thinking text",
512+
}),
513+
]),
514+
}),
515+
)
516+
})
517+
518+
it("should concatenate multiple reasoning and thinking blocks into the thinking field", async () => {
519+
// Multiple reasoning/thinking blocks are joined with newlines so the
520+
// full thinking context is preserved across turns.
521+
mockChat.mockImplementation(async function* () {
522+
yield { message: { content: "ok" } }
523+
})
524+
525+
const messages: Anthropic.Messages.MessageParam[] = [
526+
{
527+
role: "assistant",
528+
content: [
529+
{ type: "reasoning", text: "First reasoning", summary: [] } as any,
530+
{ type: "thinking", thinking: "Second thinking" } as any,
531+
{ type: "text", text: "Answer" },
532+
],
533+
},
534+
{ role: "user" as const, content: "Follow up" },
535+
]
536+
537+
const stream = handler.createMessage("System", messages)
538+
for await (const _ of stream) {
539+
// consume
540+
}
541+
542+
expect(mockChat).toHaveBeenCalledWith(
543+
expect.objectContaining({
544+
messages: expect.arrayContaining([
545+
expect.objectContaining({
546+
role: "assistant",
547+
thinking: "First reasoning\nSecond thinking",
548+
}),
549+
]),
550+
}),
551+
)
552+
})
553+
554+
it("should not set thinking field when assistant reasoning/thinking blocks are empty", async () => {
555+
// Covers the `block.text.length > 0` and `block.thinking.length > 0`
556+
// false branches, and the `reasoningText || undefined` falsy branch.
557+
mockChat.mockImplementation(async function* () {
558+
yield { message: { content: "ok" } }
559+
})
560+
561+
const messages: Anthropic.Messages.MessageParam[] = [
562+
{
563+
role: "assistant",
564+
content: [
565+
{ type: "reasoning", text: "", summary: [] } as any,
566+
{ type: "thinking", thinking: "" } as any,
567+
{ type: "text", text: "Answer" },
568+
],
569+
},
570+
{ role: "user" as const, content: "Follow up" },
571+
]
572+
573+
const stream = handler.createMessage("System", messages)
574+
for await (const _ of stream) {
575+
// consume
576+
}
577+
578+
expect(mockChat).toHaveBeenCalledWith(
579+
expect.objectContaining({
580+
messages: expect.arrayContaining([
581+
expect.objectContaining({
582+
role: "assistant",
583+
thinking: undefined,
584+
}),
585+
]),
586+
}),
587+
)
588+
})
589+
590+
it("should not set thinking field on assistant messages without reasoning blocks", async () => {
591+
// Covers the `reasoningText || undefined` falsy branch for a plain
592+
// assistant text+tool_use message (no reasoning/thinking blocks).
593+
mockChat.mockImplementation(async function* () {
594+
yield { message: { content: "ok" } }
595+
})
596+
597+
const messages: Anthropic.Messages.MessageParam[] = [
598+
{
599+
role: "assistant",
600+
content: [
601+
{ type: "text", text: "Answer" },
602+
{
603+
type: "tool_use",
604+
id: "tool-1",
605+
name: "get_weather",
606+
input: { location: "SF" },
607+
},
608+
],
609+
},
610+
{ role: "user" as const, content: "Follow up" },
611+
]
612+
613+
const stream = handler.createMessage("System", messages)
614+
for await (const _ of stream) {
615+
// consume
616+
}
617+
618+
expect(mockChat).toHaveBeenCalledWith(
619+
expect.objectContaining({
620+
messages: expect.arrayContaining([
621+
expect.objectContaining({
622+
role: "assistant",
623+
thinking: undefined,
624+
}),
625+
]),
626+
}),
627+
)
628+
})
629+
630+
it("should not send think parameter for an unknown reasoningEffort value", async () => {
631+
// Covers the `default` branch of getOllamaThinkParam's switch,
632+
// which returns undefined for unrecognized effort values.
633+
const options: ApiHandlerOptions = {
634+
apiModelId: "qwen3",
635+
ollamaModelId: "qwen3",
636+
ollamaBaseUrl: "http://localhost:11434",
637+
enableReasoningEffort: true,
638+
reasoningEffort: "bogus" as any,
639+
}
640+
641+
handler = new NativeOllamaHandler(options)
642+
643+
mockChat.mockImplementation(async function* () {
644+
yield { message: { content: "ok" } }
645+
})
646+
647+
const stream = handler.createMessage("System", [{ role: "user" as const, content: "Hi" }])
648+
for await (const _ of stream) {
649+
// consume
650+
}
651+
652+
const callArgs = mockChat.mock.calls[0][0] as Record<string, unknown>
653+
expect(callArgs.think).toBeUndefined()
654+
})
482655
})
483656

484657
it("should not send think parameter when enableReasoningEffort is true but reasoningEffort is undefined", async () => {
@@ -663,6 +836,58 @@ describe("NativeOllamaHandler", () => {
663836
}
664837
}).rejects.toThrow("Model llama2 not found in Ollama")
665838
})
839+
840+
it("should wrap stream processing errors with a descriptive message", async () => {
841+
// Covers the `catch (streamError)` branch: the chat() call
842+
// resolves and returns an async iterable, but iterating it throws.
843+
// The handler must wrap the error with "Ollama stream processing
844+
// error: ..." and rethrow.
845+
mockChat.mockImplementation(async function* () {
846+
yield { message: { content: "partial" } }
847+
throw new Error("stream blew up")
848+
})
849+
850+
const stream = handler.createMessage("System", [{ role: "user" as const, content: "Test" }])
851+
852+
await expect(async () => {
853+
for await (const _ of stream) {
854+
// consume stream
855+
}
856+
}).rejects.toThrow("Ollama stream processing error: stream blew up")
857+
})
858+
859+
it("should wrap stream processing errors with unknown message fallback", async () => {
860+
// Covers the `streamError.message || "Unknown error"` fallback in
861+
// the stream processing catch block when the error has no message.
862+
mockChat.mockImplementation(async function* () {
863+
yield { message: { content: "partial" } }
864+
throw {}
865+
})
866+
867+
const stream = handler.createMessage("System", [{ role: "user" as const, content: "Test" }])
868+
869+
await expect(async () => {
870+
for await (const _ of stream) {
871+
// consume stream
872+
}
873+
}).rejects.toThrow("Ollama stream processing error: Unknown error")
874+
})
875+
876+
it("should rethrow non-ECONNREFUSED non-404 errors from chat()", async () => {
877+
// Covers the fall-through `throw error` branch in the outer catch
878+
// when the error is neither ECONNREFUSED nor a 404.
879+
const error = new Error("something else") as any
880+
error.status = 500
881+
mockChat.mockRejectedValue(error)
882+
883+
const stream = handler.createMessage("System", [{ role: "user" as const, content: "Test" }])
884+
885+
await expect(async () => {
886+
for await (const _ of stream) {
887+
// consume stream
888+
}
889+
}).rejects.toThrow("something else")
890+
})
666891
})
667892

668893
describe("getModel", () => {

webview-ui/src/components/settings/providers/__tests__/Ollama.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ describe("Ollama Component - thinking setting", () => {
186186
expect(mockSetApiConfigurationField).toHaveBeenCalledWith("enableReasoningEffort", false)
187187
// reasoningEffort is intentionally left untouched so the user's prior
188188
// selection survives across toggles.
189-
expect(mockSetApiConfigurationField).not.toHaveBeenCalledWith("reasoningEffort", undefined)
189+
expect(mockSetApiConfigurationField).not.toHaveBeenCalledWith("reasoningEffort", expect.anything())
190190
})
191191

192192
it("should render ThinkingBudget with supportsReasoningEffort when thinking is enabled", () => {

0 commit comments

Comments
 (0)