Repository navigation
Improvements to the MCP tools service and enhances LLM API parameter handling #291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -13,6 +13,7 @@ import ( | |||||
| "go.opentelemetry.io/otel/codes" | ||||||
|
|
||||||
| "jan-server/services/llm-api/internal/domain/conversation" | ||||||
| domainmodel "jan-server/services/llm-api/internal/domain/model" | ||||||
| "jan-server/services/llm-api/internal/domain/project" | ||||||
| "jan-server/services/llm-api/internal/domain/prompt" | ||||||
| "jan-server/services/llm-api/internal/domain/usersettings" | ||||||
|
|
@@ -28,6 +29,8 @@ import ( | |||||
| "jan-server/services/llm-api/internal/utils/httpclients/chat" | ||||||
| "jan-server/services/llm-api/internal/utils/idgen" | ||||||
| "jan-server/services/llm-api/internal/utils/platformerrors" | ||||||
|
|
||||||
| "github.com/shopspring/decimal" | ||||||
| ) | ||||||
|
|
||||||
| const ConversationReferrerContextKey = "conversation_referrer" | ||||||
|
|
@@ -201,6 +204,16 @@ func (h *ChatHandler) CreateChatCompletion( | |||||
| // Override the request model with the provider's original model ID | ||||||
| request.Model = selectedProviderModel.ProviderOriginalModelID | ||||||
|
|
||||||
| // Optionally load model catalog (used later to apply default parameters) | ||||||
| var modelCatalog *domainmodel.ModelCatalog | ||||||
| if selectedProviderModel.ModelCatalogID != nil { | ||||||
| modelCatalog, err = h.providerHandler.GetModelCatalogByID(ctx, *selectedProviderModel.ModelCatalogID) | ||||||
| if err != nil { | ||||||
| log := logger.GetLogger() | ||||||
| log.Warn().Err(err).Uint("model_catalog_id", *selectedProviderModel.ModelCatalogID).Msg("failed to load model catalog defaults") | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Resolve jan_* media placeholders (best-effort) | ||||||
| request.Messages = h.resolveMediaPlaceholders(ctx, reqCtx, request.Messages) | ||||||
|
|
||||||
|
|
@@ -283,12 +296,20 @@ func (h *ChatHandler) CreateChatCompletion( | |||||
| } | ||||||
|
|
||||||
| // Handle streaming vs non-streaming | ||||||
| llmRequest := chat.CompletionRequest{ | ||||||
| ChatCompletionRequest: request.ChatCompletionRequest, | ||||||
| TopK: request.TopK, | ||||||
| RepetitionPenalty: request.RepetitionPenalty, | ||||||
| } | ||||||
| if modelCatalog != nil { | ||||||
| h.applyModelDefaultsFromCatalog(&llmRequest, modelCatalog) | ||||||
| } | ||||||
| observability.AddSpanEvent(ctx, "calling_llm") | ||||||
| llmStartTime := time.Now() | ||||||
| if request.Stream { | ||||||
| response, err = h.streamCompletion(ctx, reqCtx, chatClient, conv, request.ChatCompletionRequest) | ||||||
| response, err = h.streamCompletion(ctx, reqCtx, chatClient, conv, llmRequest) | ||||||
| } else { | ||||||
| response, err = h.callCompletion(ctx, chatClient, request.ChatCompletionRequest) | ||||||
| response, err = h.callCompletion(ctx, chatClient, llmRequest) | ||||||
| } | ||||||
| llmDuration := time.Since(llmStartTime) | ||||||
|
|
||||||
|
|
@@ -419,7 +440,7 @@ func (h *ChatHandler) CreateChatCompletion( | |||||
| func (h *ChatHandler) callCompletion( | ||||||
| ctx context.Context, | ||||||
| chatClient *chat.ChatCompletionClient, | ||||||
| request openai.ChatCompletionRequest, | ||||||
| request chat.CompletionRequest, | ||||||
| ) (*openai.ChatCompletionResponse, error) { | ||||||
| chatCompletion, err := chatClient.CreateChatCompletion(ctx, "", request) | ||||||
| if err != nil { | ||||||
|
|
@@ -435,7 +456,7 @@ func (h *ChatHandler) streamCompletion( | |||||
| reqCtx *gin.Context, | ||||||
| chatClient *chat.ChatCompletionClient, | ||||||
| conv *conversation.Conversation, | ||||||
| request openai.ChatCompletionRequest, | ||||||
| request chat.CompletionRequest, | ||||||
| ) (*openai.ChatCompletionResponse, error) { | ||||||
| // Create callback to send conversation data before [DONE] | ||||||
| var beforeDoneCallback chat.BeforeDoneCallback | ||||||
|
|
@@ -525,6 +546,69 @@ func (h *ChatHandler) resolveMediaPlaceholders(ctx context.Context, reqCtx *gin. | |||||
| return messages | ||||||
| } | ||||||
|
|
||||||
| // applyModelDefaultsFromCatalog fills in missing request parameters using defaults from the model catalog. | ||||||
| func (h *ChatHandler) applyModelDefaultsFromCatalog(req *chat.CompletionRequest, catalog *domainmodel.ModelCatalog) { | ||||||
| if req == nil || catalog == nil { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| defaults := catalog.SupportedParameters.Default | ||||||
| if len(defaults) == 0 { | ||||||
| return | ||||||
| } | ||||||
|
|
||||||
| if req.Temperature == 0 { | ||||||
| if val, ok := decimalToFloat32(defaults["temperature"]); ok { | ||||||
| req.Temperature = val | ||||||
| } | ||||||
| } | ||||||
| if req.TopP == 0 { | ||||||
| if val, ok := decimalToFloat32(defaults["top_p"]); ok { | ||||||
| req.TopP = val | ||||||
| } | ||||||
| } | ||||||
| if req.PresencePenalty == 0 { | ||||||
| if val, ok := decimalToFloat32(defaults["presence_penalty"]); ok { | ||||||
| req.PresencePenalty = val | ||||||
| } | ||||||
| } | ||||||
| if req.FrequencyPenalty == 0 { | ||||||
| if val, ok := decimalToFloat32(defaults["frequency_penalty"]); ok { | ||||||
| req.FrequencyPenalty = val | ||||||
| } | ||||||
| } | ||||||
| if req.MaxTokens == 0 { | ||||||
| if val, ok := decimalToInt(defaults["max_tokens"]); ok { | ||||||
| req.MaxTokens = val | ||||||
| } | ||||||
| } | ||||||
| if req.TopK == nil || (req.TopK != nil && *req.TopK == 0) { | ||||||
| if val, ok := decimalToInt(defaults["top_k"]); ok { | ||||||
| req.TopK = &val | ||||||
| } | ||||||
| } | ||||||
| if req.RepetitionPenalty == nil || (req.RepetitionPenalty != nil && *req.RepetitionPenalty == 0) { | ||||||
|
||||||
| if req.RepetitionPenalty == nil || (req.RepetitionPenalty != nil && *req.RepetitionPenalty == 0) { | |
| if req.RepetitionPenalty == nil || *req.RepetitionPenalty == 0 { |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error returned from Float64() conversion is being silently discarded with _. If the decimal-to-float conversion fails, this function will return 0 without indication. Consider checking and handling the conversion error, or at least document why it's safe to ignore.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -78,6 +78,13 @@ type ChatCompletionClient struct { | |||||
| name string | ||||||
| } | ||||||
|
|
||||||
| // CompletionRequest extends the OpenAI chat request with provider-specific fields. | ||||||
| type CompletionRequest struct { | ||||||
| openai.ChatCompletionRequest | ||||||
| TopK *int `json:"top_k,omitempty"` | ||||||
| RepetitionPenalty *float32 `json:"repetition_penalty,omitempty"` | ||||||
| } | ||||||
|
|
||||||
| type functionCallAccumulator struct { | ||||||
| Name string | ||||||
| Arguments string | ||||||
|
|
@@ -103,7 +110,7 @@ func NewChatCompletionClient(client *resty.Client, name, baseURL string) *ChatCo | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| func (c *ChatCompletionClient) CreateChatCompletion(ctx context.Context, apiKey string, request openai.ChatCompletionRequest) (*openai.ChatCompletionResponse, error) { | ||||||
| func (c *ChatCompletionClient) CreateChatCompletion(ctx context.Context, apiKey string, request CompletionRequest) (*openai.ChatCompletionResponse, error) { | ||||||
| // Start OpenTelemetry span for tracking | ||||||
| ctx, span := otel.Tracer("chat-completion-client").Start(ctx, "CreateChatCompletion", | ||||||
| trace.WithSpanKind(trace.SpanKindClient), | ||||||
|
|
@@ -126,9 +133,41 @@ func (c *ChatCompletionClient) CreateChatCompletion(ctx context.Context, apiKey | |||||
| if request.TopP != 0 { | ||||||
| span.SetAttributes(attribute.Float64("llm.top_p", float64(request.TopP))) | ||||||
| } | ||||||
| if request.PresencePenalty != 0 { | ||||||
| span.SetAttributes(attribute.Float64("llm.presence_penalty", float64(request.PresencePenalty))) | ||||||
| } | ||||||
| if request.FrequencyPenalty != 0 { | ||||||
| span.SetAttributes(attribute.Float64("llm.frequency_penalty", float64(request.FrequencyPenalty))) | ||||||
| } | ||||||
|
|
||||||
| start := time.Now() | ||||||
|
|
||||||
| // Debug logging: Log request details before sending | ||||||
| log := logger.GetLogger() | ||||||
| topK := 0 | ||||||
| if request.TopK != nil { | ||||||
| topK = *request.TopK | ||||||
| } | ||||||
| repetitionPenalty := float32(0) | ||||||
| if request.RepetitionPenalty != nil { | ||||||
| repetitionPenalty = *request.RepetitionPenalty | ||||||
| } | ||||||
| log.Info(). | ||||||
| Str("provider", c.name). | ||||||
| Str("model", request.Model). | ||||||
| Int("messages", len(request.Messages)). | ||||||
| Bool("stream", request.Stream). | ||||||
| Float32("temperature", request.Temperature). | ||||||
| Int("max_tokens", request.MaxTokens). | ||||||
| Float32("top_p", request.TopP). | ||||||
| Int("top_k", topK). | ||||||
| Float32("presence_penalty", request.PresencePenalty). | ||||||
| Float32("frequency_penalty", request.FrequencyPenalty). | ||||||
| Float32("repetition_penalty", repetitionPenalty). | ||||||
| Msg("[ChatCompletion] Sending request to inference server") | ||||||
|
|
||||||
| log.Info().Interface("request", request).Msg("[ChatCompletion] Request body") | ||||||
|
||||||
| log.Info().Interface("request", request).Msg("[ChatCompletion] Request body") | |
| log.Debug().Interface("request", request).Msg("[ChatCompletion] Request body") |
Copilot
AI
Dec 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logging the entire request object at Info level (line 557) could expose sensitive data like API keys, user messages, or PII in logs. The request may contain conversation history with personal information. Consider using Debug level or sanitizing/redacting sensitive fields before logging.
| log.Info().Interface("request", request).Msg("[ChatCompletion][Stream] Request body") | |
| log.Debug().Interface("request", request).Msg("[ChatCompletion][Stream] Request body") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition
req.TopK == nil || (req.TopK != nil && *req.TopK == 0)is redundant. Ifreq.TopK == nilis true, the second part of the OR will never be evaluated. Simplify toreq.TopK == nil || *req.TopK == 0.