feat(prompt): tokenizer-style prompt stitching with thinking-prefix support#255
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 433a3a877d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| thinkPrefix := closeThinkMarker | ||
| if thinkingEnabled { | ||
| thinkPrefix = openThinkMarker | ||
| } | ||
| parts = append(parts, assistantMarker+thinkPrefix) |
There was a problem hiding this comment.
Strip think delimiters from visible model output
This change starts every generation with raw <think>/</think> delimiters, but output cleanup still only strips DeepSeek role/end markers in sanitizeLeakedOutput and does not remove think tags. When upstream echoes prompt-template tokens (the same class of leakage already handled for <|Assistant|>/<|end...|>), those delimiters will now be returned to clients as literal answer text, polluting responses and exposing internal reasoning boundaries. Please add <think>/</think> (and the new BOS marker if needed) to output sanitization or avoid emitting them into user-visible channels.
Useful? React with 👍 / 👎.
Motivation
Description
<|begin▁of▁sentence|>,<think>, and</think>, and apply them when assembling prompts.MessagesPrepareWithThinking(messages, thinkingEnabled)and keepMessagesPrepareas the non-thinking default wrapper for compatibility.</think>{content}<|end▁of▁sentence|>, and append an assistant prefix (<think>for thinking models or</think>for non-thinking models) when the last role is not assistant.thinkingEnabledthrough OpenAI/Claude/Gemini prompt normalization (via deepseek) so resolved model capabilities select the appropriate suffix/prefix behavior, and update unit tests to assert the new markers and think-prefix semantics.Testing
go test ./internal/prompt ./internal/util ./internal/adapter/openai ./internal/adapter/claude ./internal/adapter/geminiand all affected unit tests passed../scripts/lint.sh,./tests/scripts/check-refactor-line-gate.sh,./tests/scripts/run-unit-all.sh, andnpm run build --prefix webui, and these checks completed successfully.Codex Task