tool support for local llms #1440
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughConverts LLM streaming from raw text to structured Response items via a new StreamingParser, updates the LLaMA crate and local provider to use Response variants (TextDelta, Reasoning, ToolCall), expands desktop tool gating/assembly to include HyprLocal and baseTools, adjusts Cargo dev-deps, fixes template Jinja tags, and removes an unused import in the tracing plugin. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Desktop_UI as Desktop UI
participant LocalProvider
participant LLaMA
participant Parser as StreamingParser
User->>Desktop_UI: submit prompt (with tools)
Desktop_UI->>LocalProvider: createChatCompletion(request)
LocalProvider->>LLaMA: generate_stream()
rect rgb(240,248,255)
note right of LLaMA: raw chunks are parsed into structured Responses
LLaMA->>Parser: feed raw chunk
Parser-->>LLaMA: Response (TextDelta / ToolCall / Reasoning)
end
loop per Response
LLaMA-->>LocalProvider: Response
alt TextDelta
LocalProvider-->>Desktop_UI: stream delta (content)
else ToolCall
LocalProvider-->>Desktop_UI: stream delta (tool_call)
else Reasoning
LocalProvider->>LocalProvider: log / drop
end
end
LocalProvider-->>Desktop_UI: final aggregated result (non-stream: optional content + optional tool_calls)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/local-llm/src/server.rs (1)
229-247: Fix: UnboundedReceiver::recv is not async; current select arm won’t compile/work.Use UnboundedReceiverStream and poll next() in select.
- let (progress_sender, mut progress_receiver) = mpsc::unbounded_channel::<f64>(); + let (progress_sender, progress_receiver) = mpsc::unbounded_channel::<f64>(); + let mut progress_stream = + tokio_stream::wrappers::UnboundedReceiverStream::new(progress_receiver); @@ - tokio::pin!(response_stream); + tokio::pin!(response_stream); + tokio::pin!(progress_stream); @@ - progress_result = progress_receiver.recv() => { + progress_result = progress_stream.next() => { match progress_result { Some(progress) => yield StreamEvent::Progress(progress), None => {} } }
🧹 Nitpick comments (5)
plugins/tracing/src/lib.rs (1)
65-67: Prefer &Path over &PathBuf in function signaturesIdiomatic Rust accepts &Path to avoid tying callers to PathBuf.
Apply within this range:
-fn make_file_writer_if_enabled( - enabled: bool, - logs_dir: &PathBuf, -) -> Option<(tracing_appender::non_blocking::NonBlocking, WorkerGuard)> { +fn make_file_writer_if_enabled( + enabled: bool, + logs_dir: &std::path::Path, +) -> Option<(tracing_appender::non_blocking::NonBlocking, WorkerGuard)> {apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (2)
309-313: Base tools now always exposed — confirm intent across providersMaking search_sessions_* tools unconditional means they’re offered to all models, including HyprLocal and any non-tool-capable remotes. If that’s intentional for “local tool support,” LGTM; otherwise, gate behind capability (e.g., shouldUseTools or a model capability flag).
331-334: Tool set merge order and collisionsMerging baseTools with MCP-derived tools can silently override keys if names collide. If collisions are possible, consider namespacing MCP tools or validating uniqueness before merge.
crates/llama/src/parser.rs (1)
112-131: Avoid double JSON parse for tool_callYou parse the same JSON twice: once in parse_tool_call_block and again in try_parse_next. Return a typed structure once to reduce overhead.
Example refactor:
-fn parse_tool_call_block(input: &str) -> IResult<&str, (String, String)> { +fn parse_tool_call_block(input: &str) -> IResult<&str, (String, HashMap<String, serde_json::Value>)> { let (input, _) = tag("<tool_call>")(input)?; let (input, _) = multispace0(input)?; let (input, json_content) = take_until("</tool_call>")(input)?; let (input, _) = tag("</tool_call>")(input)?; let (input, _) = multispace0(input)?; let json_trimmed = json_content.trim(); - let parsed: serde_json::Value = serde_json::from_str(json_trimmed) + let parsed: serde_json::Value = serde_json::from_str(json_trimmed) .map_err(|_| nom::Err::Error(nom::error::Error::new(input, nom::error::ErrorKind::Tag)))?; let name = parsed["name"] .as_str() .ok_or_else(|| nom::Err::Error(nom::error::Error::new(input, nom::error::ErrorKind::Tag)))? .to_string(); - let arguments = parsed["arguments"].to_string(); + let arguments = parsed + .get("arguments") + .cloned() + .unwrap_or(serde_json::json!({})); + let arguments: HashMap<String, serde_json::Value> = + serde_json::from_value(arguments).unwrap_or_default(); - Ok((input, (name, arguments))) + Ok((input, (name, arguments))) }Then simplify try_parse_next accordingly.
crates/llama/src/lib.rs (1)
409-426: Avoid unnecessary clone of Response in tests.Response is owned; cloning is redundant and may require Clone impl.
- while let Some(response) = stream.next().await { - responses.push(response.clone()); - println!("{:?}", response); - } + while let Some(response) = stream.next().await { + println!("{:?}", response); + responses.push(response); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(2 hunks)crates/llama/Cargo.toml(1 hunks)crates/llama/src/lib.rs(10 hunks)crates/llama/src/parser.rs(1 hunks)plugins/local-llm/src/server.rs(5 hunks)plugins/tracing/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/tracing/src/lib.rsapps/desktop/src/components/right-panel/hooks/useChatLogic.tscrates/llama/src/parser.rsplugins/local-llm/src/server.rscrates/llama/src/lib.rs
🧬 Code graph analysis (1)
crates/llama/src/lib.rs (1)
crates/llama/src/parser.rs (2)
new(25-29)serde_json(61-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (9)
plugins/tracing/src/lib.rs (1)
8-8: Import cleanup looks goodSlimming to PathBuf only is correct given current usage.
crates/llama/Cargo.toml (1)
36-36: Correct dependency scope for nomnom is used by library code (not tests only), so adding it under [dependencies] is appropriate.
crates/llama/src/parser.rs (1)
10-18: Derive(Eq) will not compile with serde_json::Valueserde_json::Value does not implement Eq; deriving Eq for Response (which contains HashMap<String, Value>) will fail.
Apply:
-#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq)] pub enum Response {Likely an incorrect or invalid review comment.
crates/llama/src/lib.rs (3)
18-23: Publicly re-exporting parser types looks good.API surface aligns with downstream usage.
553-566: Cancellation test logic looks sound.Count-based early cancel should reliably stop the stream well before the upper bound.
41-48: Approve code changes. No staleString-based call sites detected by the verification script.plugins/local-llm/src/server.rs (3)
221-227: Switch to model.generate_stream_with_callback is correct.Plumbs progress via callback and registers token.
300-304: StreamEvent payload switch to hypr_llama::Response is consistent.Enum change matches upstream stream payloads.
370-415: Non-stream aggregation of text and tool calls looks correct.Reasoning dropped from output and logged at debug; content/tool_calls set only if non-empty.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
crates/llama/src/parser.rs (2)
42-48: Chunk-boundary bug: avoid flushing trailing partial tag prefixesIf a chunk ends with a partial "<think" or "<tool_call", this path emits it as text and breaks the block on the next chunk. Retain the trailing prefix in the buffer.
- if !self.buffer.is_empty() && !self.looks_like_block_start() { - let text = self.buffer.clone(); - self.buffer.clear(); - if !text.trim().is_empty() { - responses.push(Response::TextDelta(text)); - } - } + if !self.buffer.is_empty() && !self.looks_like_block_start() { + if let Some(idx) = self.trailing_incomplete_tag_prefix() { + let emit = self.buffer[..idx].to_string(); + self.buffer.drain(..idx); + if !emit.trim().is_empty() { + responses.push(Response::TextDelta(emit)); + } + } else { + let text = std::mem::take(&mut self.buffer); + if !text.trim().is_empty() { + responses.push(Response::TextDelta(text)); + } + } + }Add this helper inside
impl StreamingParser:impl StreamingParser { // Keep trailing prefix of a known tag in the buffer so we don't split it across chunks. fn trailing_incomplete_tag_prefix(&self) -> Option<usize> { let candidates = ["<think>", "<tool_call>"]; if let Some(start) = self.buffer.rfind('<') { let tail = &self.buffer[start..]; if candidates.iter().any(|t| t.starts_with(tail)) { return Some(start); } } None } }
53-79: Parser can stall on closed <tool_call> blocks with invalid JSON — make forward progressWhen a complete
<tool_call>...</tool_call>is present but JSON fails inparse_tool_call_block, the buffer still starts with<tool_call>and nothing is emitted, so parsing stalls. Demote that block toTextDeltato advance.fn try_parse_next(&mut self) -> Option<Response> { if let Ok((remaining, content)) = parse_think_block(&self.buffer) { self.buffer = remaining.to_string(); return Some(Response::Reasoning(content)); } if let Ok((remaining, (name, arguments))) = parse_tool_call_block(&self.buffer) { let arguments: HashMap<String, serde_json::Value> = serde_json::from_str(&arguments).unwrap_or_default(); self.buffer = remaining.to_string(); return Some(Response::ToolCall { name, arguments }); } + // Closed <tool_call> block present but unparseable JSON: emit as text to avoid stalling. + if self.buffer.starts_with("<tool_call>") { + if let Some(end) = self.buffer.find("</tool_call>") { + let end_idx = end + "</tool_call>".len(); + let text = self.buffer[..end_idx].to_string(); + self.buffer = self.buffer[end_idx..].to_string(); + if !text.trim().is_empty() { + return Some(Response::TextDelta(text)); + } + } + } + if let Some(pos) = self.find_next_block_start() { if pos > 0 { let text = self.buffer[..pos].to_string(); self.buffer = self.buffer[pos..].to_string(); if !text.trim().is_empty() { return Some(Response::TextDelta(text)); } } } None }plugins/local-llm/src/server.rs (1)
420-505: Tool-call chunk index should count tool calls only (not all events)
enumerate()mixes text/progress with tool-call indexing, producing wrong indices. Keep a counter that increments only onToolCall.- let stream = Box::pin( - source_stream - .enumerate() - .map(move |(index, event)| { + let stream = Box::pin( + source_stream + .map({ + let mut tool_call_idx: u32 = 0; + move |event| { let delta_template = empty_stream_response_delta.clone(); let response_template = base_stream_response_template.clone(); - match event { + match event { StreamEvent::Response(llama_response) => match llama_response { hypr_llama::Response::TextDelta(chunk) => { Some(Ok(CreateChatCompletionStreamResponse { choices: vec![ChatChoiceStream { index: 0, delta: ChatCompletionStreamResponseDelta { content: Some(chunk), ..delta_template }, finish_reason: None, logprobs: None, }], ..response_template })) } hypr_llama::Response::Reasoning(_) => None, hypr_llama::Response::ToolCall { name, arguments } => { + let this_idx = tool_call_idx; + tool_call_idx = tool_call_idx.saturating_add(1); Some(Ok(CreateChatCompletionStreamResponse { choices: vec![ChatChoiceStream { index: 0, delta: ChatCompletionStreamResponseDelta { tool_calls: Some(vec![ ChatCompletionMessageToolCallChunk { - index: index.try_into().unwrap_or(0), + index: this_idx, id: Some(uuid::Uuid::new_v4().to_string()), r#type: Some(ChatCompletionToolType::Function), function: Some(FunctionCallStream { name: Some(name), arguments: Some( serde_json::to_string(&arguments) .unwrap_or_default(), ), }), }, ]), ..delta_template }, finish_reason: None, logprobs: None, }], ..response_template })) } }, StreamEvent::Progress(progress) => { Some(Ok(CreateChatCompletionStreamResponse { choices: vec![ChatChoiceStream { index: 0, delta: ChatCompletionStreamResponseDelta { tool_calls: Some(vec![ ChatCompletionMessageToolCallChunk { - index: index.try_into().unwrap_or(0), + index: 0, id: Some("progress_update".to_string()), r#type: Some(ChatCompletionToolType::Function), function: Some(FunctionCallStream { name: Some("update_progress".to_string()), arguments: Some( serde_json::to_string(&serde_json::json!({ "progress": progress })) .unwrap(), ), }), }, ]), ..delta_template }, finish_reason: None, logprobs: None, }], ..response_template })) } } - }) + }) .filter_map(|x| async move { x }), );
🧹 Nitpick comments (1)
crates/llama/src/parser.rs (1)
130-234: Add a regression test for partial tag prefixes across chunk boundariesCover the case where a chunk ends with "<thi" (or "<tool_ca") to ensure no partial prefixes are emitted as text and the subsequent block parses correctly.
#[test] fn test_partial_prefix_boundary() { let mut p = StreamingParser::new(); let mut out = vec![]; out.extend(p.process_chunk("Hi <thi")); out.extend(p.process_chunk("nk>x</think> there")); assert_eq!( out, vec![ Response::TextDelta("Hi ".into()), Response::Reasoning("x".into()), Response::TextDelta(" there".into()), ] ); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
crates/llama/src/lib.rs(10 hunks)crates/llama/src/parser.rs(1 hunks)plugins/local-llm/src/server.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/llama/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/local-llm/src/server.rscrates/llama/src/parser.rs
🧬 Code graph analysis (1)
crates/llama/src/parser.rs (1)
crates/llama/src/lib.rs (1)
new(309-366)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/template/assets/chat.system.jinja (1)
152-171: Fix typos and grammar in Tool Calling guidanceUser-facing text has multiple spelling/grammar issues. Proposed cleanup keeps meaning intact.
Apply this diff:
-- custom MCP tools: user will be able to request you to call MCP tools. Often times, they are related to other producitvity tools like Slack, Notion, Google workspace, etc. - Below is the list of tools that you have access to. +- custom MCP tools: users can request you to call MCP tools. Often, they relate to productivity tools like Slack, Notion, or Google Workspace. + Below is the list of tools you have access to. - when you return the response for a request that used search_sessions_multi_keywords, you should smartly combined the information from the tool call results, instead of naively returning all raw results (oftentime thses could be very long). + when you return the response for a request that used search_sessions_multi_keywords, smartly combine the information from the tool-call results instead of naively returning all raw results (often these can be very long). -(what you should don't do) : "Here are the search results for .... (goes on to list all the results)" -(what you should do) : "It seems that for the keyword 'product', there are notes 'Q1 Meeting', 'Apple Client Meeting','Daily all handes'....It seems taht (your analysis and insights + references - where the information came from)" +(what you should not do): "Here are the search results for ... (lists all results)" +(what you should do): "For the keyword 'product', relevant notes include 'Q1 Meeting', 'Apple Client Meeting', 'Daily All Hands' ... It seems that (your analysis and insights + references — where the information came from)"
🧹 Nitpick comments (5)
crates/template/assets/chat.system.jinja (3)
59-63: Minor copy edits in HyprLocal example headerSmall clarity/spelling fixes.
Apply this diff:
-[Response Example] -either one of the two. +[Response Example] +Either of the two. -- informative description: - This meeting was about a VC funding round where participants expressed their opinoins on.... +- informative description: + This meeting was about a VC funding round where participants expressed their opinions on ...
90-92: Clarify guidance around code blocksBackticks are mismatched; sentence is hard to parse.
Apply this diff:
-- Information (when it's not rewriting the note, it shouldn't be inside the `blocks. Only re-written version of the note should be inside the` blocks.) +- Information (when it's not rewriting the note) should not be inside ``` blocks. Only the re-written version of the note should be inside ``` blocks.
21-21: Polish date lineTiny grammar fix for readability.
Apply this diff:
-Right now date : {{ date }} +Current date: {{ date }}apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (2)
311-315: Base tools assembly reads wellIncluding
edit_enhanced_noteonly whenselectionDatais present prevents surfacing an unusable tool. Optional micro: computebaseToolsinside theshouldUseToolsbranch to skip a tiny bit of work when tools are disabled.
333-336: Place HyprLocal’s update_progress spread last
Moving the internalupdate_progressafter thehyprMcpTools/newMcpToolsandbaseToolsspreads prevents any external tool namedupdate_progressfrom overriding the HyprLocal implementation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/desktop/src/components/right-panel/hooks/useChatLogic.ts(3 hunks)apps/desktop/src/components/right-panel/utils/chat-utils.ts(1 hunks)crates/template/assets/chat.system.jinja(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/right-panel/utils/chat-utils.tsapps/desktop/src/components/right-panel/hooks/useChatLogic.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (3)
crates/template/assets/chat.system.jinja (1)
146-146: Balanced Jinja conditionals — LGTMClosing the "Custom" block here prevents the Tool Calling section from being unintentionally nested. Looks correct.
apps/desktop/src/components/right-panel/utils/chat-utils.ts (1)
72-81: 'HyprLocal' usage confirmed
Ripgrep across all .ts files shows only the exact case-sensitive"HyprLocal"literal in every gating check and no variants.apps/desktop/src/components/right-panel/hooks/useChatLogic.ts (1)
209-216: Broaden tool gating to HyprLocal — LGTMThis matches
prepareMessageHistoryand unlocks tools for local models.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/llama/Cargo.toml (1)
38-46: Movedirsto[dependencies].
Theget_modelfunction in crates/llama/src/lib.rs (around lines 432–434) callsdirs::data_dir(), sodirsis required by the library itself—not just tests—and must be listed under[dependencies]in Cargo.toml.
♻️ Duplicate comments (1)
crates/llama/src/parser.rs (1)
31-49: Fix chunk-boundary bug: trailing partial tag prefixes flushed as text.
If a chunk ends with "<thi" or "<tool_call", this path emits it as TextDelta and breaks the block.Apply:
- if !self.buffer.is_empty() && !self.looks_like_block_start() { - let text = self.buffer.clone(); - self.buffer.clear(); - responses.push(Response::TextDelta(text)); - } + if !self.buffer.is_empty() && !self.looks_like_block_start() { + if let Some(idx) = self.trailing_incomplete_tag_prefix() { + let emit = self.buffer[..idx].to_string(); + self.buffer.drain(..idx); + if !emit.trim().is_empty() { + responses.push(Response::TextDelta(emit)); + } + } else { + let text = std::mem::take(&mut self.buffer); + if !text.trim().is_empty() { + responses.push(Response::TextDelta(text)); + } + } + }Add (outside this range, within impl StreamingParser):
impl StreamingParser { fn trailing_incomplete_tag_prefix(&self) -> Option<usize> { let candidates = ["<think>", "<tool_call>"]; if let Some(start) = self.buffer.rfind('<') { let tail = &self.buffer[start..]; if candidates.iter().any(|t| t.starts_with(tail)) { return Some(start); } } None } }
🧹 Nitpick comments (6)
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (2)
11-11: Nit: use an en dash or “to” for ranges in user-facing text.
“1–5” or “1 to 5” reads better than “1 ~ 5”.Apply this diff:
- "List of 1 ~ 5 keywords to search for, each keyword should be concise", + "List of 1–5 keywords to search for, each keyword should be concise",
31-38: Guard against duplicate pushes to matchedKeywords.
If duplicates slip past, avoid double-counting matches.Apply this diff:
- if (combinedResults.has(session.id)) { - combinedResults.get(session.id).matchedKeywords.push(keyword); - } else { - combinedResults.set(session.id, { - ...session, - matchedKeywords: [keyword], - }); - } + const existing = combinedResults.get(session.id); + if (existing) { + if (!existing.matchedKeywords.includes(keyword)) { + existing.matchedKeywords.push(keyword); + } + } else { + combinedResults.set(session.id, { ...session, matchedKeywords: [keyword] }); + }crates/llama/Cargo.toml (1)
21-22: Pin git deps to a commit to avoid branch drift.
Branches can move and break reproducible builds.Example:
-llama-cpp-2 = { git = "https://github.com/utilityai/llama-cpp-rs", default-features = false, branch = "update-llama-cpp-2025-08-21" } -llama-cpp-sys-2 = { git = "https://github.com/utilityai/llama-cpp-rs", default-features = false, branch = "update-llama-cpp-2025-08-21" } +llama-cpp-2 = { git = "https://github.com/utilityai/llama-cpp-rs", default-features = false, rev = "<commit>" } +llama-cpp-sys-2 = { git = "https://github.com/utilityai/llama-cpp-rs", default-features = false, rev = "<commit>" }crates/llama/src/parser.rs (3)
65-71: Avoid extra allocation when slicing plain text before next block.
Use drain to move out the prefix without cloning twice.- let text = self.buffer[..pos].to_string(); - self.buffer = self.buffer[pos..].to_string(); + let text: String = self.buffer.drain(..pos).collect(); return Some(Response::TextDelta(text));
105-124: Double-parsing JSON; consider parsing once.
You parse JSON insideparse_tool_call_blockto extractname, then reparseargumentsintry_parse_next. Returning the arguments as aserde_json::Map(or the whole raw JSON for single-pass parse intry_parse_next) would avoid the second parse.Minimal direction:
- Change return to
(String, serde_json::Map<String, serde_json::Value>).- In
try_parse_next, convert the map toHashMapwithout reparsing:Response::ToolCall { name, arguments: arguments.into_iter().collect() }
267-283: Make tests deterministic by seeding RNG.
Random chunking can be flaky; seed the RNG for reproducibility.Example:
- use rand::Rng; - let mut rng = rand::rng(); + use rand::{Rng, SeedableRng}; + let mut rng = rand::rngs::StdRng::seed_from_u64(42);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts(1 hunks)crates/llama/Cargo.toml(1 hunks)crates/llama/src/parser.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.tscrates/llama/src/parser.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (5)
apps/desktop/src/components/right-panel/utils/tools/search_session_multi_keywords.ts (1)
10-11: Relaxing min keywords to 1 makes sense for local LLM tool calls.
Looks good and aligns with enabling tools for HyprLocal.crates/llama/Cargo.toml (1)
19-20: nom 8.0.0 addition looks correct for the new parser.
Ties directly to parser.rs usage; no unused deps introduced.crates/llama/src/parser.rs (3)
10-18: Response derives are correct (Eq removed).
Avoids theserde_json::Value: !Eqcompile error.
57-63: Good: arguments JSON uses unwrap_or_default.
Prevents stalls on malformed/non-object arguments and ensures forward progress.
150-171: Add a test for a chunk ending with a partial tag prefix.
Covers the bug fixed above (e.g., chunk ends with "<thi" and next chunk starts with "nk>...").I can draft this test if you want it included.
Also applies to: 173-197, 199-229, 308-333
No description provided.