Conversation
- Add `mock` feature flag (gated behind `cfg(any(test, feature = "mock"))`) - Add `Streamed<T>` wrapper converting non-streaming models to streamable - Add `tool_then_text` helper for two-turn tool-call→text mock pattern - Add `with_model_override` on `AgentBuildContext` to inject mock models - Gate model override logic behind `#[cfg(any(test, feature = "mock"))]` - Re-export `FunctionModel`, `MockModel`, `TestModel` from mock module - Add `extract_tool_return_text` helper for rendering tool results
|
Warning Review limit reached
More reviews will be available in 53 minutes and 37 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis pull request adds mock model support to the SerdesAI agent runtime. It introduces a 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/reloaded-code-serdesai/src/mock.rs (2)
8-14: 💤 Low valueClarify the example as pseudo-code.
The example is marked as
textand references an undefinedagentvariable. Consider either:
- Marking it explicitly as pseudo-code in prose, or
- Expanding it to a complete
rust,no_runexample that shows building an agent withAgentBuildContext.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/reloaded-code-serdesai/src/mock.rs` around lines 8 - 14, The doc example in mock.rs uses a ```text``` block and references an undefined `agent`, which is confusing; update the example to either label it clearly as pseudocode in prose (e.g., add a sentence before the code saying "The following is pseudocode") or convert it into a runnable snippet by changing the fence to `rust,no_run` and showing how to build an agent (use `AgentBuildContext` to construct an `agent`) before calling `agent.run_stream(...)`; ensure symbols `Streamed`, `tool_then_text`, `agent.run_stream`, and `AgentBuildContext` are mentioned so readers can locate the related API.
204-235: ⚖️ Poor tradeoffConsider handling the Multiple variant explicitly.
The function doesn't explicitly extract items from the Multiple variant (
{"type":"multiple","items":[...]}). It falls through to the pretty-print fallback, which works but may not produce the most readable output.For better readability, consider iterating over the
itemsarray and recursively extracting text from each item.💡 Optional enhancement
// Error message field. if let Some(msg) = val.get("message").and_then(|v| v.as_str()) { return format!("[error] {msg}"); } + // Multiple items field. + if let Some(items) = val.get("items").and_then(|v| v.as_array()) { + let extracted: Vec<String> = items + .iter() + .filter_map(|item| { + // Each item might have content/message fields + item.get("content") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + .or_else(|| { + item.get("message") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + }) + }) + .collect(); + if !extracted.is_empty() { + return extracted.join("\n"); + } + } + // Fallback: pretty-print the whole thing. serde_json::to_string_pretty(&val).unwrap_or_else(|_| format!("{:?}", tr.content)) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/reloaded-code-serdesai/src/mock.rs` around lines 204 - 235, extract_tool_return_text currently falls back to pretty-print for the "multiple" variant; add explicit handling for {"type":"multiple","items":[...]} by checking val.get("items") for an array, iterating the items, and concatenating a user-friendly representation (e.g., newline-separated) of each item’s extracted text. Implement this by either converting each serde_json::Value item into the same shape inspected by extract_tool_return_text (or add a small helper that accepts a Value and reuses the existing extraction logic) so you recursively extract text for nested items from the ToolReturnPart's content.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/reloaded-code-serdesai/src/mock.rs`:
- Around line 8-14: The doc example in mock.rs uses a ```text``` block and
references an undefined `agent`, which is confusing; update the example to
either label it clearly as pseudocode in prose (e.g., add a sentence before the
code saying "The following is pseudocode") or convert it into a runnable snippet
by changing the fence to `rust,no_run` and showing how to build an agent (use
`AgentBuildContext` to construct an `agent`) before calling
`agent.run_stream(...)`; ensure symbols `Streamed`, `tool_then_text`,
`agent.run_stream`, and `AgentBuildContext` are mentioned so readers can locate
the related API.
- Around line 204-235: extract_tool_return_text currently falls back to
pretty-print for the "multiple" variant; add explicit handling for
{"type":"multiple","items":[...]} by checking val.get("items") for an array,
iterating the items, and concatenating a user-friendly representation (e.g.,
newline-separated) of each item’s extracted text. Implement this by either
converting each serde_json::Value item into the same shape inspected by
extract_tool_return_text (or add a small helper that accepts a Value and reuses
the existing extraction logic) so you recursively extract text for nested items
from the ToolReturnPart's content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08060a1e-61ab-493f-bcba-fb3172301102
📒 Files selected for processing (4)
src/reloaded-code-serdesai/Cargo.tomlsrc/reloaded-code-serdesai/src/agent_runtime/task.rssrc/reloaded-code-serdesai/src/lib.rssrc/reloaded-code-serdesai/src/mock.rs
1 participant
Add mock model infra. Test agents without real LLM provider.
Used for hook tests and hook examples.
Cargo.toml- newmockfeature flagmock.rs-Streamed<T>wrapper makes non-streaming models streamable.tool_then_texthelper: first turn calls tool, second turn returns text + real tool outputtask.rs-model_overridefield +with_model_override()onAgentBuildContext. Builder picks override over catalog model whenmockenabledlib.rs- exposepub mod mockbehind feature gateNo real provider needed. CI-safe.