Fix: Trim AI "thinking" tags from Deepseek output #59
Conversation
Signed-off-by: fyzanshaik <[email protected]>
Signed-off-by: fyzanshaik <[email protected]>
Signed-off-by: fyzanshaik <[email protected]>
WalkthroughThe changes introduce a new configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DraftCommand
participant Config
participant AI
User->>DraftCommand: execute()
DraftCommand->>Config: Load draft_config
DraftCommand->>AI: Generate draft (with commit_types as JSON)
AI-->>DraftCommand: Returns draft output (may include <think> tags)
alt trim_thinking_tags enabled
DraftCommand->>DraftCommand: Remove <think>...</think> blocks
DraftCommand->>DraftCommand: Trim whitespace
end
DraftCommand->>User: Print final draft output
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.gitignore (1)
2-2: Confirm ignoring the primary config file
Ignoringlumen.config.jsonwill prevent committing the actual configuration that your code depends on. If the intent is to let users maintain their own local settings, consider instead:
- Committing a template (e.g.,
lumen.config.example.json) with default values.- Ignoring only the real file (e.g., using
/lumen.config.json).
This ensures new contributors have a starting point while keeping personal overrides out of VCS.Cargo.toml (1)
22-23: Consider using std::sync::OnceLock instead of lazy_static for better performance.The dependencies are appropriate for the new functionality. However, consider replacing
lazy_staticwithstd::sync::OnceLock(stable since Rust 1.70) orstd::sync::LazyLock(stable since Rust 1.80) for better performance and to reduce dependencies.-lazy_static = "1.5.0"Then in
src/command/draft.rs, replace the lazy_static usage with:use std::sync::OnceLock; static THINK_TAG_REGEX: OnceLock<Regex> = OnceLock::new(); fn get_think_tag_regex() -> &'static Regex { THINK_TAG_REGEX.get_or_init(|| Regex::new(r"(?s)<think>.*?</think>\s*").unwrap()) }src/config/configuration.rs (1)
80-86: Remove commented-out code.The commented-out
deserialize_commit_typesfunction should be removed rather than left as comments, as it's no longer needed after the type change.-// fn deserialize_commit_types<'de, D>(deserializer: D) -> Result<String, D::Error> -// where -// D: Deserializer<'de>, -// { -// let commit_types_map: HashMap<String, String> = HashMap::deserialize(deserializer)?; -// serde_json::to_string(&commit_types_map).map_err(serde::de::Error::custom) -// }src/command/draft.rs (1)
27-31: Optimize string operations for better performance.The thinking tag removal logic is correct, but can be optimized to reduce unnecessary string allocations.
- let mut result = provider.draft(self).await?; - if self.draft_config.trim_thinking_tags { - result = THINK_TAG_REGEX.replace_all(&result, "").to_string(); - result = result.trim().to_string(); - } + let mut result = provider.draft(self).await?; + if self.draft_config.trim_thinking_tags { + result = THINK_TAG_REGEX.replace_all(&result, "").trim().to_string(); + }This combines the operations and eliminates an unnecessary intermediate string allocation.
README.md (1)
55-55: Fix macOS capitalization.The operating system should be written as "macOS" not "MacOS" according to Apple's official branding.
-#### Using Homebrew (MacOS and Linux) +#### Using Homebrew (macOS and Linux)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
.gitignore(1 hunks)Cargo.toml(1 hunks)README.md(7 hunks)src/ai_prompt.rs(5 hunks)src/command/draft.rs(3 hunks)src/config/configuration.rs(3 hunks)src/main.rs(0 hunks)
💤 Files with no reviewable changes (1)
- src/main.rs
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~55-~55: The operating system from Apple is written “macOS”.
Context: ... ### Installation #### Using Homebrew (MacOS and Linux) ```bash brew install jnsaha...
(MAC_OS)
🔇 Additional comments (7)
src/config/configuration.rs (2)
36-38: Excellent type safety improvement.Changing
commit_typesfromStringtoHashMap<String, String>improves type safety and eliminates the need for custom deserialization. The newtrim_thinking_tagsfield is properly configured with a sensible default.
56-70: Well-structured default commit types.The implementation properly initializes the HashMap with all standard conventional commit types. The structure is clear and maintainable.
src/ai_prompt.rs (2)
7-7: Good addition of required import.The
serde_jsonimport is appropriately added to support the new serialization functionality.
126-127: Proper error handling for serialization.The serialization of the HashMap back to JSON is correctly implemented with appropriate error handling. This maintains the existing prompt format while benefiting from the improved type safety in the configuration.
src/command/draft.rs (2)
4-5: Appropriate imports for the new functionality.The regex and lazy_static imports are correctly added to support the thinking tag removal feature.
14-16: Well-crafted regex pattern.The regex pattern
(?s)<think>.*?</think>\s*is well-designed:
(?s)enables dotall mode to match newlines within thinking blocks.*?uses non-greedy matching to avoid over-matching\s*removes trailing whitespace after each tagREADME.md (1)
224-226: Excellent documentation of new feature.The documentation clearly explains the new
trim_thinking_tagsconfiguration option with appropriate context and default value. The comment explaining its purpose is helpful for users.
|
Fixed : #60 |
|
Trimming of thinking needs to be reflected for all commands just checked :(, if this is good to be merged I can work on making it globally executed in another PR! Else I am fine continuing in this |
1 participant
When using Deepseek models with lumen draft, the AI's internal "thinking" process (enclosed in ... tags) was included in the final output, cluttering the commit message.
To address this, I've implemented the following:
Added trim_thinking_tags option: A new boolean field trim_thinking_tags has been added to the draft section of lumen.config.json. Setting this to true enables the trimming functionality.
Robust commit_types deserialization: The commit_types field in DraftConfig was changed from String to HashMap<String, String>. This corrects a type mismatch between the JSON config and the Rust struct, improving config loading reliability. serde_json::to_string is now used to convert the HashMap back to JSON when building the AI prompt.
Output post-processing: If trim_thinking_tags is true, a regular expression now identifies and removes the ... blocks from the AI's raw response before it's displayed to the user.
These changes were specifically aimed at cleaning the AI's output format. No existing functionality of lumen is broken by these modifications; in fact, config loading should be more robust.
Summary by CodeRabbit
New Features
<think>...</think>tags from AI output.Bug Fixes
Documentation
Chores
.gitignoreentries for better project management.