Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses three main concerns: fixing conversation title generation to handle special characters, updating Kong CORS configuration to support additional origins and headers, and updating the prompt orchestration documentation to reflect recent system changes.
Key Changes:
- Adds comprehensive content sanitization for conversation titles to handle URLs, special characters, and markdown links
- Extends Kong CORS configuration to include production domains and additional MCP-related headers
- Updates prompt orchestration documentation with expanded module descriptions and configuration details
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| services/llm-api/internal/interfaces/httpserver/handlers/chathandler/chat_handler.go | Introduces sanitizeTitleContent function to remove URLs, emails, and special characters from conversation titles while preserving readable text |
| kong/kong.yml | Adds production domains (jan.ai) and development ports to CORS origins; adds "mcp-protocol-version" and "Accept" headers to allowed CORS headers across multiple services |
| kong/kong-dev-full.yml | Same CORS configuration updates as kong.yml for development environment with full service setup |
| docs/guides/prompt-orchestration.md | Comprehensive documentation update covering new modules (Deep Research, Project Instructions, User Profile), updated priority system, template service integration, and refined configuration examples |
Comments suppressed due to low confidence (1)
services/llm-api/internal/interfaces/httpserver/handlers/chathandler/chat_handler.go:803
- There's a potential byte vs. rune indexing issue when truncating the title. The code uses
content[:60]which operates on bytes, but for strings containing multi-byte Unicode characters, this could cut in the middle of a character and cause invalid UTF-8. Consider using rune-aware truncation, for example by converting to a rune slice first or usingutf8.RuneCountInStringto properly count characters.
if len(content) > 60 {
// Find a good breaking point (end of word)
truncated := content[:60]
if lastSpace := strings.LastIndex(truncated, " "); lastSpace > 30 {
content = content[:lastSpace] + "..."
} else {
content = truncated + "..."
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["POST", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "mcp-protocol-version", "X-Tool-Call-ID", "X-Conversation-ID"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "mcp-protocol-version", "X-Tool-Call-ID", "X-Conversation-ID", "Accept"] |
There was a problem hiding this comment.
The header "mcp-protocol-version" uses all lowercase, which is inconsistent with the title case convention used for other custom headers like "X-API-Key", "X-Request-Id", "X-Tool-Call-ID", and "X-Conversation-ID". Consider using "Mcp-Protocol-Version" for consistency.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "mcp-protocol-version", "X-Tool-Call-ID", "X-Conversation-ID", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Protocol-Version", "X-Tool-Call-ID", "X-Conversation-ID", "Accept"] |
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] |
There was a problem hiding this comment.
The header naming convention is inconsistent. The existing "Mcp-Session-Id" uses title case for each word, but the newly added "mcp-protocol-version" uses all lowercase. Consider using "Mcp-Protocol-Version" for consistency with other headers.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "Mcp-Protocol-Version", "Accept"] |
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] |
There was a problem hiding this comment.
Same inconsistency: "mcp-protocol-version" uses all lowercase while "Mcp-Session-Id" uses title case. Consider using "Mcp-Protocol-Version" for consistency.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "Mcp-Protocol-Version", "Accept"] |
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["POST", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "mcp-protocol-version"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "mcp-protocol-version", "X-Tool-Call-ID", "X-Conversation-ID", "Accept"] |
There was a problem hiding this comment.
The header "mcp-protocol-version" uses all lowercase, which is inconsistent with the title case convention used for other custom headers like "X-API-Key", "X-Request-Id", "X-Tool-Call-ID", and "X-Conversation-ID". Consider using "Mcp-Protocol-Version" for consistency.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "mcp-protocol-version", "X-Tool-Call-ID", "X-Conversation-ID", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Protocol-Version", "X-Tool-Call-ID", "X-Conversation-ID", "Accept"] |
| func sanitizeTitleContent(content string) string { | ||
| // Remove URLs (http, https, ftp, and www patterns) | ||
| urlPattern := regexp.MustCompile(`(?i)(https?://|ftp://|www\.)[^\s]+`) | ||
| content = urlPattern.ReplaceAllString(content, "") | ||
|
|
||
| // Remove markdown links [text](url) | ||
| markdownLinkPattern := regexp.MustCompile(`\[([^\]]*)\]\([^)]+\)`) | ||
| content = markdownLinkPattern.ReplaceAllString(content, "$1") | ||
|
|
||
| // Remove email addresses | ||
| emailPattern := regexp.MustCompile(`[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}`) | ||
| content = emailPattern.ReplaceAllString(content, "") | ||
|
|
||
| // Remove special characters but keep basic punctuation (.,!?-') and unicode letters/numbers | ||
| var result strings.Builder | ||
| for _, r := range content { | ||
| if unicode.IsLetter(r) || unicode.IsNumber(r) || unicode.IsSpace(r) || | ||
| r == '.' || r == ',' || r == '!' || r == '?' || r == '-' || r == '\'' { | ||
| result.WriteRune(r) | ||
| } | ||
| } | ||
| content = result.String() | ||
|
|
||
| // Replace multiple spaces with single space (after special char removal) | ||
| multiSpacePattern := regexp.MustCompile(`\s+`) | ||
| content = multiSpacePattern.ReplaceAllString(content, " ") | ||
|
|
||
| // Trim whitespace and trailing punctuation for cleaner titles | ||
| content = strings.TrimSpace(content) | ||
| content = strings.TrimRight(content, " .,!?-'") | ||
|
|
||
| return content | ||
| } |
There was a problem hiding this comment.
The sanitizeTitleContent function compiles three regular expressions on every call. Since this function is called for every conversation title generation, this creates unnecessary overhead. Consider compiling these regex patterns once at package level or as struct fields to improve performance.
The regex patterns that should be pre-compiled:
- urlPattern (line 814)
- markdownLinkPattern (line 818)
- emailPattern (line 822)
- multiSpacePattern (line 836)
| func sanitizeTitleContent(content string) string { | ||
| // Remove URLs (http, https, ftp, and www patterns) | ||
| urlPattern := regexp.MustCompile(`(?i)(https?://|ftp://|www\.)[^\s]+`) | ||
| content = urlPattern.ReplaceAllString(content, "") | ||
|
|
||
| // Remove markdown links [text](url) | ||
| markdownLinkPattern := regexp.MustCompile(`\[([^\]]*)\]\([^)]+\)`) | ||
| content = markdownLinkPattern.ReplaceAllString(content, "$1") | ||
|
|
||
| // Remove email addresses | ||
| emailPattern := regexp.MustCompile(`[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}`) | ||
| content = emailPattern.ReplaceAllString(content, "") | ||
|
|
||
| // Remove special characters but keep basic punctuation (.,!?-') and unicode letters/numbers | ||
| var result strings.Builder | ||
| for _, r := range content { | ||
| if unicode.IsLetter(r) || unicode.IsNumber(r) || unicode.IsSpace(r) || | ||
| r == '.' || r == ',' || r == '!' || r == '?' || r == '-' || r == '\'' { | ||
| result.WriteRune(r) | ||
| } | ||
| } | ||
| content = result.String() | ||
|
|
||
| // Replace multiple spaces with single space (after special char removal) | ||
| multiSpacePattern := regexp.MustCompile(`\s+`) | ||
| content = multiSpacePattern.ReplaceAllString(content, " ") | ||
|
|
||
| // Trim whitespace and trailing punctuation for cleaner titles | ||
| content = strings.TrimSpace(content) | ||
| content = strings.TrimRight(content, " .,!?-'") | ||
|
|
||
| return content | ||
| } |
There was a problem hiding this comment.
The sanitizeTitleContent function implements complex sanitization logic with multiple regex patterns and character filtering rules, but lacks test coverage. Consider adding tests to verify:
- URL removal (http, https, ftp, www patterns)
- Markdown link handling (preserving link text, removing URLs)
- Email address removal
- Special character filtering while preserving basic punctuation
- Multiple space normalization
- Edge cases like empty strings, only special characters, only URLs, etc.
- Unicode character handling
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] |
There was a problem hiding this comment.
Same inconsistency as line 92: "mcp-protocol-version" uses all lowercase while "Mcp-Session-Id" uses title case. Consider using "Mcp-Protocol-Version" for consistency.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "Mcp-Protocol-Version", "Accept"] |
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] |
There was a problem hiding this comment.
Same inconsistency: "mcp-protocol-version" uses all lowercase while "Mcp-Session-Id" uses title case. Consider using "Mcp-Protocol-Version" for consistency.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "Mcp-Protocol-Version", "Accept"] |
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] |
There was a problem hiding this comment.
The header naming convention is inconsistent. The existing "Mcp-Session-Id" uses title case for each word, but the newly added "mcp-protocol-version" uses all lowercase. While HTTP headers are case-insensitive and both will work, consider using consistent casing for better maintainability. The convention in the codebase appears to favor title case (e.g., "X-API-Key", "X-Request-Id", "Mcp-Session-Id"), so "Mcp-Protocol-Version" might be more consistent.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "Mcp-Protocol-Version", "Accept"] |
| origins: ["http://localhost", "http://localhost:3000", "http://localhost:3001", "http://127.0.0.1", "http://127.0.0.1:3000", "http://127.0.0.1:3001", "http://127.0.0.1:8080", "http://localhost:8080", "https://chat-dev.jan.ai", "https://platform-dev.jan.ai", "https://api-gateway-dev.jan.ai", "https://chat.jan.ai", "https://platform.jan.ai"] | ||
| methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id"] | ||
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] |
There was a problem hiding this comment.
Same inconsistency as lines 92 and 131: "mcp-protocol-version" uses all lowercase while "Mcp-Session-Id" uses title case. Consider using "Mcp-Protocol-Version" for consistency.
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "mcp-protocol-version", "Accept"] | |
| headers: ["Authorization", "Content-Type", "X-API-Key", "Idempotency-Key", "X-Request-Id", "Mcp-Session-Id", "Mcp-Protocol-Version", "Accept"] |
Uh oh!
There was an error while loading. Please reload this page.