feat(providers): add reasoning_echo_policy field#90
Merged
Conversation
…asoning_content handling Refs librefang/librefang#4842 — long-term replacement for the substring match that the OpenAI driver currently uses to decide how to handle `reasoning_content` on historical assistant turns. Three provider-specific behaviours that the driver must distinguish at wire time, now expressed as catalog metadata: * `strip` — DeepSeek R1 / deepseek-reasoner. The API rejects requests that carry reasoning_content on previous assistant messages. * `echo` — DeepSeek V4 Flash. Thinking mode is on by default and the API rejects multi-turn requests when assistant turns containing tool_calls don't echo back the original reasoning text. This is the bug surfaced in librefang/librefang#4842. * `empty_string` — Moonshot / Kimi K2 family. The field must be present (empty string) on tool_calls turns, with thinking disabled wire-side for multi-turn compatibility. * `none` (default) — most providers; field is omitted entirely. V4 Pro is intentionally NOT marked `echo` — librefang#4842 reports it working out-of-the-box; flip when there's an empirical reproducer. Marks affected models: providers/deepseek.toml deepseek-v4-flash → echo deepseek-reasoner → strip providers/moonshot.toml kimi-k2.6, kimi-k2.5, kimi-k2 → empty_string providers/kimi-coding.toml kimi-for-coding → empty_string providers/byteplus-coding.toml kimi-k2.5 → empty_string providers/novita.toml moonshotai/kimi-k2-thinking → empty_string Tooling: * schema.toml registers the field with the four enum options and a `none` default so existing TOML files keep parsing unchanged. * scripts/validate.py rejects unknown enum values; verified with a hand-crafted negative case (`reasoning_echo_policy = "bogus"` → validation fails with the expected message). * `python3 scripts/validate.py` passes (267 models). The librefang side that consumes this field will land in a follow-up PR — until then, registry consumers ignore the field via `#[serde(default)]` and the existing substring fallback continues to work, so this commit is safe to ship independently.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs librefang/librefang#4842.
Summary
Long-term replacement for the substring match in the LibreFang OpenAI driver that decides how to handle
reasoning_contenton historical assistant turns. Three provider-specific behaviours, now expressed as catalog metadata:stripdeepseek-reasonerreasoning_contenton previous assistant messagesechotool_callsturns don't echo back the original reasoning text (the librefang#4842 bug)empty_stringtool_callsturns, with thinking disabled wire-sidenone(default)V4 Pro is intentionally NOT marked
echo— librefang#4842 reports it working out-of-the-box; the substring guard is narrow on purpose. Flip later when there's an empirical reproducer.Files
schema.toml— registers the field, fouroptions, default"none".#[serde(default)]semantics on the consumer side keep existing TOML parsing unchanged.providers/deepseek.toml—deepseek-v4-flash→echo;deepseek-reasoner→stripproviders/moonshot.toml—kimi-k2.6,kimi-k2.5,kimi-k2→empty_stringproviders/kimi-coding.toml—kimi-for-coding→empty_stringproviders/byteplus-coding.toml—kimi-k2.5→empty_stringproviders/novita.toml—moonshotai/kimi-k2-thinking→empty_stringscripts/validate.py—VALID_REASONING_ECHO_POLICIESconstant + per-model enum checkTest plan
python3 scripts/validate.py— passes (267 models, no regressions)reasoning_echo_policy = "bogus"ondeepseek-v4-flash→ validator fails withModel 'deepseek-v4-flash' invalid reasoning_echo_policy 'bogus' (valid: echo, empty_string, none, strip)Backward compatibility
required = falsewithdefault = "none"and consumers will read it via#[serde(default)]. Older librefang releases that don't know the field will simply ignore it; their existing substring-based driver behaviour is unchanged.Follow-up (in librefang/librefang)
A separate PR will add a matching
ReasoningEchoPolicyenum tolibrefang-types::ModelCatalogEntry, plumb it throughCompletionRequest, and replace the OpenAI driver'sis_deepseek_reasoner/is_deepseek_v4_thinking_with_tools/kimi_needs_reasoning_contentsubstring helpers with a single match on the catalog field. Substring matching is kept as a fallback for unknown / user-defined models.