feat(ai): thinking mode switch, vision image input, and API host normalization fixes - #1596
lb1038678031 wants to merge 5 commits into
Conversation
- Unify host normalization between model listing and chat client so both hosts with or without the /v1 suffix resolve identically (fixes upstream NOT_FOUND when a full /v1 base URL is configured). - Trim trailing slashes before building the models URL. - Summarize upstream error bodies instead of echoing the full response. - Prefer the chosen provider's stored key over host-based key lookup. - Return nil client when AI is disabled instead of an invalid client. - Fix default provider list: use Gemini's OpenAI-compatible endpoint and drop Anthropic (not supported by the OpenAI-compatible backend).
Thinking mode (per provider): - New thinking_mode field on the AI provider config. - When enabled, a custom http.RoundTripper merges enable_thinking=true into chat completion request bodies before they are sent. Vision input (per provider): - New vision_enabled field; surfaced to clients via site info ai_vision_enabled so the front end can show the attach button. - Chat requests accept images on the first user message: base64 data URLs or HTTPS links, max 4 per message and 4MB decoded each. - Images are converted into MultiContent parts for vision models. - Conversation history keeps an '[图片]' placeholder instead of image data. Also stops logging full request bodies that contain image data.
- Admin AI settings page gains deep-thinking and image-input switches per provider, saved with the rest of the provider config. - Site settings store tracks ai_vision_enabled. - AiAssistant sender shows an attach-image action when vision is on, previews thumbnails, validates count/type/size client-side and sends base64 images with the first message. New-conversation first hop now carries images through as well. - zh_CN/en_US translations for all new strings.
The embedded base64 constant in the vision test was mistaken for a leaked credential. Build the placeholder image from the canonical PNG signature bytes at runtime instead, so no base64 blob appears in the source.
LinkinStars
left a comment
There was a problem hiding this comment.
Thanks for the substantial work here. I found several blocking issues that should be addressed before merge.
-
Image validation/logging can be bypassed through later messages. The handler validates and redacts only
messages[0].images. A signed-in user can put a large base64 data URL inmessages[1].images: it is neither validated nor sent to the model, but the full request is JSON-marshaled into the server log. BecauseShouldBindruns before the per-image size check and this route does not applyhttp.MaxBytesReader, this also permits avoidable memory and log-volume DoS. Please cap the complete request body before binding, rejectimageson all non-first messages, and redact attachments across every message before logging. -
enable_thinkingis not a provider-neutral OpenAI-compatible parameter. The transport unconditionally injects top-levelenable_thinking: truefor every configured provider/model. This matches DashScope/Qwen compatibility mode, but it does not match DeepSeek's documented Chat Completions API. DeepSeek documentsthinking: {"type": "enabled"}(plusreasoning_effort), notenable_thinking; its documentation also specifies howreasoning_contentmust be preserved when tools are used. The PR description specifically names DeepSeek, so direct DeepSeek configurations will have an unsupported or ignored switch. Please make this provider/model-specific (or expose a safely validated extra-body configuration) rather than injecting one gateway-specific field globally.
Official references:
- DeepSeek Thinking Mode: https://api-docs.deepseek.com/guides/thinking_mode/
- DashScope/Qwen
enable_thinking: https://help.aliyun.com/en/model-studio/qwen-api-via-dashscope
-
The Gemini default-host fix does not reach existing installations. This changes init data and the already-released AI migration, but adds no new migration. Existing deployments retain
https://generativelanguage.googleapis.comin theirai_config.providerrow, which normalizes to the still-invalid/v1path rather than Gemini's OpenAI-compatible/v1beta/openaipath. Please add a forward migration that updates only the unchanged legacy default, without overwriting administrator-customized hosts. The new default itself is correct per Gemini's documentation: https://ai.google.dev/gemini-api/docs/openai -
Historical
reasoning_contentis discarded on a later turn even though tools are always included. Conversation records persist reasoning content, but rebuildingConversationContextand converting it to OpenAI messages copies onlyRoleandContent. DeepSeek explicitly requires historical reasoning content to be passed back in tool-bearing requests. Please preserve it end-to-end when restoring records and building subsequent requests.
There is also a smaller validation gap: MIME prefixes plus base64 decoding do not prove that an attachment is a PNG/JPEG/WebP; the test intentionally uses a non-decodable PNG-shaped byte sequence. Please validate actual image content (at least magic bytes, preferably decoder/config validation) before forwarding it.
…s and gemini host migration - cap the whole chat request body before binding, reject images on non-first messages and redact attachments across every message before logging - inject the provider-documented thinking parameter instead of a global enable_thinking flag (DashScope: enable_thinking, DeepSeek: thinking object, hosts without a documented parameter: none) - add a v2.0.4 migration upgrading only the unchanged legacy Gemini default host, and restore the Anthropic provider entry - preserve reasoning_content end-to-end when rebuilding conversation context for follow-up turns - validate image magic bytes for base64 attachments
|
Thanks for the careful review! All items are addressed in commit 8de3fbd:
The route now caps the complete request body with http.MaxBytesReader (32MB: 4 x 4MB images with base64 expansion is ~21.3MB, plus text history) before binding, with an early ContentLength rejection, so oversized payloads are no longer buffered or logged. Agreed — the flag is gateway-specific. Injection is now decided per configured host (thinkingParamForHost): DashScope/Qwen hosts -> top-level enable_thinking: true (documented compatibility mode) Added forward migration v2.0.4. It upgrades only the unchanged legacy default (https://generativelanguage.googleapis.com -> https://generativelanguage.googleapis.com/v1beta/openai) in both places the stale default can live: the ai_config.provider config row (the provider list behind the admin UI) and the saved site AI configuration (SaveSiteAI backfills provider entries with the list default at save time, so existing installs carry the stale host in their ai_providers entries). Administrator-customized hosts are never overwritten, and unparseable content is skipped. While re-verifying the provider list I also restored the Anthropic entry that was accidentally dropped from v31.go/init_data.go in an earlier commit of this PR.
initializeConversationContext now copies ReasoningContent from the persisted records when rebuilding the conversation, and ConversationContext.GetOpenAIMessages includes it in the OpenAI messages, so tool-bearing follow-up requests pass prior reasoning back end-to-end.
Base64 attachments are now sniffed against PNG/JPEG/WebP magic bytes after decoding (hasImageMagicBytes); a declared image MIME type with non-image content is rejected. HTTPS URL attachments are forwarded as URLs for the provider to fetch and validate on its side (the server does not proxy those bytes), so content sniffing there would require fetching them server-side — happy to add that if you prefer. Tests added/updated: per-host thinking parameter dispatch (DashScope/DeepSeek/unknown), transport merge + cross-provider no-leak + no-op paths, magic-byte accept/reject cases, and migration cases covering the legacy upgrade, customized-host preservation and invalid content. go vet, full build and all controller/migrations/schema tests pass. Thanks again for the thorough review — happy to adjust if anything else needs changing. |
Resubmit of #1594, addressing @LinkinStars's feedback there. Implements #1595 (feature clarification).
What
Three improvements to the AI feature, shipped as independent commits:
fix(ai): normalize API host and harden AI model listing
api_host + "/v1/models"while the chatclient auto-appends
/v1. Configuring a base URL that already contains/v1(common with OpenAI-compatible gateways such as SenseNova, SiliconFlow,
OneAPI relays...) produced
/v1/v1/models, and the gateway's gRPC-styleNOT_FOUNDerror was passed through to the UI.schema.NormalizeAPIHost()trims slashes/whitespace,appends
/v1when missing, and keeps/v1beta/*endpoints intact.error.code/message) instead of beingechoed verbatim; image data is never logged.
matching.
Anthropic removed because the backend only speaks the OpenAI protocol.
feat(ai): per-provider thinking mode
thinking_modeswitch on each provider in Admin → AI settings.http.RoundTrippermerges"enable_thinking": trueinto chat completion request bodies — the convention used by
reasoning-capable OpenAI-compatible gateways (DeepSeek V4, Qwen/DashScope,
SenseNova, vLLM/SGLang, ...). The existing
reasoning_contentstreamingpath renders the thought process in the bubble UI unchanged.
feat(ai): vision image input behind an admin switch
vision_enabledswitch per provider; exposed to clients via site infoas
ai_vision_enabled.first message of a turn; images travel as base64 data URLs or HTTPS links
and are converted into MultiContent parts.
(no DB schema change).
Security note (from #1594)
The base64 constant flagged during review was the encoding of a standard 1×1
pixel PNG placeholder used in a unit test, not a credential. To avoid any
misreading,
158516bconstructs the placeholder at runtime from the canonicalPNG signature bytes; the source no longer contains any base64 blob.
Testing
go test ./internal/schema/ ./internal/controller/ ./internal/service/siteinfo/ ./internal/migrations/— new table tests for host normalization, transport injection, and image
validation (count/type/size).
succeeds with both root and
/v1base URLs;reasoning_contentappears withthinking enabled; image questions answered by a vision-capable model.