Skip to content

重构ollama渠道 - #1811

Merged
Calcium-Ion merged 11 commits into
QuantumNous:mainfrom
somnifex:main
Sep 28, 2025
Merged

重构ollama渠道#1811
Calcium-Ion merged 11 commits into
QuantumNous:mainfrom
somnifex:main

Conversation

@somnifex

@somnifex somnifex commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

已经完成了
/api/chat 流式和非流式测试、多模态测试
/api/generate prompt生成测试
/api/embed 单/多嵌入测试

Summary by CodeRabbit

  • New Features

    • OpenAI-compatible streaming and non-stream responses for Ollama chat/completions.
    • Support for tool calls, images in messages, and JSON/JSON Schema response formats.
  • Improvements

    • More reliable embeddings with per-item outputs and token usage from prompt evaluation.
    • More consistent handling of stop sequences and model options (temperature, top_p/top_k, penalties, seed, max tokens).
    • Enhanced robustness and error handling across chat, generate, and embedding requests.

@coderabbitai

coderabbitai Bot commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Refactors Ollama routing and translation: updates path mapping in adaptor, replaces monolithic DTOs with specialized chat/generate/embed types, reworks OpenAI-to-Ollama conversions, and adds streaming/non-stream handlers that translate Ollama responses to OpenAI-like outputs. Embedding request/response handling is revised with new DTOs and usage mapping.

Changes

Cohort / File(s) Summary
Routing & Response Dispatch
relay/channel/ollama/adaptor.go
Simplifies URL routing (embed → /api/embed, completions → /api/generate, default → /api/chat), switches non-completion and Claude paths to chat translator, and directly dispatches to Ollama handlers for embeddings, streaming, and chat. Adds string path checks and import updates.
DTO Redesign
relay/channel/ollama/dto.go
Removes OllamaRequest and Options; introduces structured types: OllamaChatMessage, tool DTOs, OllamaChatRequest, OllamaGenerateRequest, OllamaEmbeddingRequest, and OllamaEmbeddingResponse. Standardizes flexible options maps and explicit fields (messages, tools, think, embeddings).
OpenAI→Ollama Translation
relay/channel/ollama/relay-ollama.go
Splits translation into openAIChatToOllamaChat and openAIToGenerate. Maps response formats (json/json_schema), options (temperature/top_p/top_k/penalties/seed/max_tokens), stop forms, tools, images, and reasoning. Reworks embedding request building and response handling with prompt eval usage.
Streaming & Non-Stream Handlers
relay/channel/ollama/stream.go
Adds line-delimited JSON stream parsing for chat/generate, emitting OpenAI-like start/delta/stop/usage events. Implements non-stream aggregation path. Includes internal structs, timestamp parsing, tool-call deltas, and error normalization.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant Adaptor as Adaptor (Ollama)
  participant Router as URL Router
  participant Translator as OpenAI→Ollama
  participant Ollama as Ollama API
  participant Streamer as Stream/Response Handler

  Client->>Adaptor: HTTP request (OpenAI-compatible)
  Adaptor->>Router: GetRequestURL(path, mode)
  Router-->>Adaptor: /api/embed or /api/generate or /api/chat
  Adaptor->>Translator: Convert request (chat/generate/embed)
  Translator-->>Adaptor: Ollama*Request (chat/generate/embed)
  Adaptor->>Ollama: POST /api/...
  alt stream=true
    Ollama-->>Adaptor: line-delimited JSON
    Adaptor->>Streamer: ollamaStreamHandler
    Streamer-->>Client: start → deltas → stop → usage → [DONE]
  else stream=false
    Ollama-->>Adaptor: full response (lines or single)
    Adaptor->>Streamer: ollamaChatHandler / embedding handler
    Streamer-->>Client: OpenAI-style JSON response
  end
Loading
sequenceDiagram
  autonumber
  participant Ollama as Ollama Stream
  participant Handler as ollamaStreamHandler
  participant Client

  Ollama-->>Handler: {model, created_at, message{content|thinking|tool_calls}, done:false}*
  loop For each non-final chunk
    Handler->>Handler: Build delta (content/reasoning/tool_calls)
    Handler-->>Client: chat.completions.chunk (delta)
  end
  Ollama-->>Handler: {done:true, prompt_eval_count, eval_count,...}
  Handler->>Handler: Compute usage, finish_reason
  Handler-->>Client: final delta (stop) + usage + [DONE]
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • seeFs001
  • creamlike1024

Poem

I thump my paws: new rivers flow,
From chat to gen, embeddings grow.
I nibble bytes, then stream in lines—
Small deltas hop like springtime signs.
Tools in tow, we route with glee;
Carrots, code, and JSON tea. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "重构ollama渠道" ("Refactor Ollama channel") is concise and accurately describes the primary intent of the changeset, which performs a broad refactor of the Ollama channel's routing, DTOs, and streaming/response logic across multiple files; it is clear and relevant for reviewers scanning the repo history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (9)
relay/channel/ollama/dto.go (2)

34-43: Strongly type Tools to avoid accidental schema drift

Tools is declared as interface{} but is always populated with []OllamaTool. Strongly typing it improves compile‑time safety and JSON stability.

 type OllamaChatRequest struct {
   Model     string              `json:"model"`
   Messages  []OllamaChatMessage `json:"messages"`
-  Tools     interface{}         `json:"tools,omitempty"`
+  Tools     []OllamaTool        `json:"tools,omitempty"`
   Format    interface{}         `json:"format,omitempty"`
   Stream    bool                `json:"stream,omitempty"`
   Options   map[string]any      `json:"options,omitempty"`
   KeepAlive interface{}         `json:"keep_alive,omitempty"`
   Think     json.RawMessage     `json:"think,omitempty"`
 }

16-20: Prefer json.RawMessage for tool parameters

Parameters interface{} invites inconsistent encodings. json.RawMessage preserves caller‑provided JSON verbatim and avoids double marshaling.

 type OllamaToolFunction struct {
   Name        string      `json:"name"`
   Description string      `json:"description,omitempty"`
-  Parameters  interface{} `json:"parameters,omitempty"`
+  Parameters  json.RawMessage `json:"parameters,omitempty"`
 }
relay/channel/ollama/adaptor.go (2)

43-47: URL routing logic looks good; consider Responses path guard later

Embedding to /api/embed and completions to /api/generate read cleanly. If/when OpenAI “/v1/responses” support is added, mirror the completions guard here.

Would you like me to add the /v1/responses route now (mapping to chat by default)?


72-72: Unimplemented OpenAI Responses conversion

Stub is fine for now, but callers will receive 501‑like behavior. If the router can reach here, gate by feature flag or implement a minimal passthrough to chat.

I can wire a minimal responses→chat mapping consistent with the OpenAI adaptor; want me to push a patch?

relay/channel/ollama/stream.go (2)

64-71: Minor: created timestamp consistency across frames

Start frame uses time.Now(); subsequent frames switch to toUnix(chunk.CreatedAt). This can yield differing created values per chunk. If stability matters for clients, cache the first non‑zero created and reuse it.

Also applies to: 123-136


141-208: Non‑stream aggregator is robust; small fallback improvement

Parsing multi‑line then falling back to single JSON is good. Consider trimming “\r” for Windows newlines and preserving tool_calls in non‑stream (if upstream includes them in final frame).

If tool_calls are present in non‑stream responses from your Ollama version, I can extend ollamaChatHandler to populate choices[].message.tool_calls.

relay/channel/ollama/relay-ollama.go (3)

37-45: Store numeric option values, not pointers

temperature is currently stored as *float64. Prefer concrete numbers for JSON stability and consistency with other options.

- if r.Temperature != nil { chatReq.Options["temperature"] = r.Temperature }
+ if r.Temperature != nil { chatReq.Options["temperature"] = *r.Temperature }

145-151: Apply same temperature fix in generate path

Mirror the chat path change for generate.

- if r.Temperature != nil { gen.Options["temperature"] = r.Temperature }
+ if r.Temperature != nil { gen.Options["temperature"] = *r.Temperature }

46-57: Stop sequences mapping duplicated

stop conversion logic is duplicated for chat and generate. Extract a small helper (local to this file) to reduce drift.

I can factor a fillStops(opts map[string]any, stop any) helper and apply it to both paths if you want.

Also applies to: 121-159

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18a385f and f19b5b8.

📒 Files selected for processing (4)
  • relay/channel/ollama/adaptor.go (5 hunks)
  • relay/channel/ollama/dto.go (1 hunks)
  • relay/channel/ollama/relay-ollama.go (2 hunks)
  • relay/channel/ollama/stream.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T06:31:11.073Z
Learnt from: jiajunly
PR: QuantumNous/new-api#1629
File: relay/channel/openai/relay-openai.go:170-174
Timestamp: 2025-08-21T06:31:11.073Z
Learning: In relay/channel/openai/relay-openai.go, the streaming logic for the AddThinkFirst feature is designed so that only the first chunk of a stream gets the "<think>\n" prefix. The final flush in the streaming handler intentionally uses addThink=false because the last chunk should never receive the prefix, even in single-chunk streams where the prefix would have been applied during normal processing.

Applied to files:

  • relay/channel/ollama/stream.go
📚 Learning: 2025-08-08T17:12:43.157Z
Learnt from: RedwindA
PR: QuantumNous/new-api#1537
File: relay/gemini_handler.go:330-342
Timestamp: 2025-08-08T17:12:43.157Z
Learning: In the new-api repository, the `GeminiEmbeddingHandler` function in `relay/gemini_handler.go` is designed specifically for native Gemini embedding requests and therefore does not require the `ConvertGeminiRequest` step that is used in the chat handler. The embedding requests are already in the native Gemini format and don't need conversion.

Applied to files:

  • relay/channel/ollama/adaptor.go
🧬 Code graph analysis (3)
relay/channel/ollama/stream.go (9)
dto/openai_request.go (1)
  • Message (259-270)
relay/helper/common.go (6)
  • Done (92-94)
  • SetEventStreamHeaders (27-41)
  • GenerateStartEmptyResponse (140-156)
  • StringData (67-73)
  • GenerateStopResponse (158-171)
  • GenerateFinalUsageResponse (173-183)
dto/openai_response.go (8)
  • Usage (217-230)
  • ChatCompletionsStreamResponse (136-144)
  • ChatCompletionsStreamResponseChoice (75-80)
  • ChatCompletionsStreamResponseChoiceDelta (82-88)
  • ToolCallResponse (116-122)
  • FunctionResponse (128-134)
  • OpenAITextResponse (34-42)
  • OpenAITextResponseChoice (28-32)
types/error.go (5)
  • NewAPIError (82-90)
  • NewOpenAIError (209-232)
  • ErrorCodeBadResponse (67-67)
  • ErrorCodeBadResponseBody (68-68)
  • ErrorCodeReadResponseBodyFailed (65-65)
service/http.go (2)
  • CloseResponseBodyGracefully (14-22)
  • IOCopyBytesGracefully (24-59)
common/utils.go (1)
  • GetUUID (155-159)
common/json.go (2)
  • Marshal (20-22)
  • Unmarshal (8-10)
logger/logger.go (1)
  • LogError (63-65)
common/constants.go (1)
  • DebugEnabled (70-70)
relay/channel/ollama/adaptor.go (3)
relay/common/relay_info.go (1)
  • RelayInfo (74-120)
dto/openai_request.go (2)
  • GeneralOpenAIRequest (25-78)
  • OpenAIResponsesRequest (768-790)
relay/constant/relay_mode.go (2)
  • RelayModeEmbeddings (12-12)
  • RelayModeCompletions (11-11)
relay/channel/ollama/dto.go (1)
dto/claude.go (1)
  • Thinking (399-402)
🔇 Additional comments (5)
relay/channel/ollama/adaptor.go (2)

49-53: Headers setup LGTM

Bearer propagation via channel.SetupApiRequestHeader + Authorization is consistent with other channels.


55-62: OpenAI request conversion branching is sound

Nil‑guard + completions/chat branching is correct.

relay/channel/ollama/stream.go (2)

210-210: Helper is fine

contentPtr is concise and correct.


87-101: Helpers exist — do not apply the suggested refactor

dto/openai_response.go defines SetContentString (ln 90), SetReasoningContent (ln 111) and ToolCallResponse.SetIndex (ln 124); common.GetPointer is implemented at common/utils.go (ln 226). Leave current setter usage as-is.

Likely an incorrect or invalid review comment.

relay/channel/ollama/relay-ollama.go (1)

59-66: Message/tool mapping reads well

String vs multi‑part content handling, image fetch/base64 normalization, and tool call argument JSON are all sound.

Confirm service.GetFileBase64FromUrl enforces per‑request timeouts and size limits to avoid slowloris/oversized downloads on untrusted URLs.

Also applies to: 96-116

Comment thread relay/channel/ollama/dto.go
Comment on lines +162 to 173
func requestOpenAI2Embeddings(r dto.EmbeddingRequest) *OllamaEmbeddingRequest {
opts := map[string]any{}
if r.Temperature != nil { opts["temperature"] = r.Temperature }
if r.TopP != 0 { opts["top_p"] = r.TopP }
if r.FrequencyPenalty != 0 { opts["frequency_penalty"] = r.FrequencyPenalty }
if r.PresencePenalty != 0 { opts["presence_penalty"] = r.PresencePenalty }
if r.Seed != 0 { opts["seed"] = int(r.Seed) }
if r.Dimensions != 0 { opts["dimensions"] = r.Dimensions }
input := r.ParseInput()
if len(input)==1 { return &OllamaEmbeddingRequest{Model:r.Model, Input: input[0], Options: opts, Dimensions:r.Dimensions} }
return &OllamaEmbeddingRequest{Model:r.Model, Input: input, Options: opts, Dimensions:r.Dimensions}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Make embeddings handler backward‑compatible with “embedding” payloads

If upstream returns {"embedding":[...]}, current logic yields an empty data slice. Add a fallback path.

 func ollamaEmbeddingHandler(c *gin.Context, info *relaycommon.RelayInfo, resp *http.Response) (*dto.Usage, *types.NewAPIError) {
   var oResp OllamaEmbeddingResponse
   body, err := io.ReadAll(resp.Body)
   if err != nil { return nil, types.NewOpenAIError(err, types.ErrorCodeBadResponseBody, http.StatusInternalServerError) }
   service.CloseResponseBodyGracefully(resp)
   if err = common.Unmarshal(body, &oResp); err != nil { return nil, types.NewOpenAIError(err, types.ErrorCodeBadResponseBody, http.StatusInternalServerError) }
-  if oResp.Error != "" { return nil, types.NewOpenAIError(fmt.Errorf("ollama error: %s", oResp.Error), types.ErrorCodeBadResponseBody, http.StatusInternalServerError) }
+  if oResp.Error != "" { return nil, types.NewOpenAIError(fmt.Errorf("ollama error: %s", oResp.Error), types.ErrorCodeBadResponseBody, http.StatusInternalServerError) }
+
+  // Fallback: older Ollama returns a single vector as "embedding"
+  if len(oResp.Embeddings) == 0 && len(oResp.Embedding) > 0 {
+    oResp.Embeddings = [][]float64{oResp.Embedding}
+  }
   data := make([]dto.OpenAIEmbeddingResponseItem,0,len(oResp.Embeddings))
   for i, emb := range oResp.Embeddings { data = append(data, dto.OpenAIEmbeddingResponseItem{Index:i,Object:"embedding",Embedding:emb}) }
   usage := &dto.Usage{PromptTokens: oResp.PromptEvalCount, CompletionTokens:0, TotalTokens: oResp.PromptEvalCount}
   embResp := &dto.OpenAIEmbeddingResponse{Object:"list", Data:data, Model: info.UpstreamModelName, Usage:*usage}
   out, _ := common.Marshal(embResp)
   service.IOCopyBytesGracefully(c, resp, out)
   return usage, nil
 }

Also applies to: 175-189

🤖 Prompt for AI Agents
In relay/channel/ollama/relay-ollama.go around lines 162 to 173 (and likewise
adjust lines 175 to 189), the current Embedding request/response logic doesn't
handle upstream payloads that use "embedding" (singular) instead of
"embeddings", causing empty data slices; update the mapping to accept both
shapes by checking for "embedding" when "embeddings" is absent and treat a
single embedding vector as the first item in the data slice (i.e., if response
contains "embedding": [...] create a single-element data slice using that
vector), and mirror the same fallback in the other block at 175-189 so both
parsing paths are backward-compatible.

Comment on lines +61 to +63
helper.SetEventStreamHeaders(c)
scanner := bufio.NewScanner(resp.Body)
usage := &dto.Usage{}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Scanner token limit can truncate long chunks

bufio.Scanner defaults to 64K tokens; long JSON lines (e.g., large reasoning/thinking frames) will error. Increase the buffer or switch to bufio.Reader.

-    scanner := bufio.NewScanner(resp.Body)
+    scanner := bufio.NewScanner(resp.Body)
+    // bump scan buffer to handle large JSON lines (e.g., long reasoning content)
+    buf := make([]byte, 64*1024)
+    scanner.Buffer(buf, 4<<20) // 4MB max token
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
helper.SetEventStreamHeaders(c)
scanner := bufio.NewScanner(resp.Body)
usage := &dto.Usage{}
helper.SetEventStreamHeaders(c)
scanner := bufio.NewScanner(resp.Body)
// bump scan buffer to handle large JSON lines (e.g., long reasoning content)
buf := make([]byte, 64*1024)
scanner.Buffer(buf, 4<<20) // 4MB max token
usage := &dto.Usage{}
🤖 Prompt for AI Agents
In relay/channel/ollama/stream.go around lines 61 to 63, bufio.Scanner is used
which defaults to a 64KB token limit and can truncate/err on long JSON lines;
replace the Scanner with a bufio.Reader (or if you must keep Scanner, call
scanner.Buffer(make([]byte, initial), maxTokenSize) with a sufficiently large
max) and read the stream with reader.ReadBytes('\n') or use json.Decoder to
stream-decode JSON chunks, handling partial reads, EOF, and errors so long
reasoning/thinking frames are not truncated.

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.

3 participants