Refactor tools into pipeline module #14
Conversation
📝 WalkthroughWalkthroughThis pull request restructures the codebase by consolidating modular code into flattened structures. The optimizer function and three tool implementations (Deconstructor, PromptReviewer, WebSearcher) migrate from separate modules into a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Merge activity
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib.rs (1)
47-47: Fix typo in log message.The log message contains a typo: "prompte" should be "prompt".
🔎 Proposed fix
- eprintln!("Optimize prompte cached to: {:?}", path); + eprintln!("Optimized prompt cached to: {:?}", path);
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (19)
.gitignoreCargo.tomlsrc/agents/mod.rssrc/agents/optimizer.rssrc/error.rssrc/lib.rssrc/main.rssrc/pipline.rssrc/tools/deconstructor.rssrc/tools/mod.rssrc/tools/prompt_reviewer.rssrc/tools/web_searcher.rssrc/types.rssrc/types/artifact.rssrc/types/common.rssrc/types/config.rssrc/types/mod.rssrc/types/pipeline.rssrc/utilities.rs
💤 Files with no reviewable changes (13)
- src/tools/mod.rs
- src/agents/mod.rs
- src/agents/optimizer.rs
- src/types/mod.rs
- .gitignore
- src/tools/web_searcher.rs
- src/types/common.rs
- src/error.rs
- src/types/config.rs
- src/tools/deconstructor.rs
- src/types/artifact.rs
- src/tools/prompt_reviewer.rs
- src/types/pipeline.rs
🧰 Additional context used
🧬 Code graph analysis (2)
src/lib.rs (1)
src/pipline.rs (1)
optimizer(131-168)
src/pipline.rs (2)
src/error.rs (1)
map_provider_error(29-36)src/utilities.rs (2)
read_artifact(47-60)require_env(10-16)
🔇 Additional comments (2)
src/pipline.rs (1)
36-55: LGTM! Clean tool implementation with proper error handling.The Deconstructor tool correctly validates the environment variable, constructs the agent, and handles errors appropriately through the
?operator.src/utilities.rs (1)
10-16: Breaking change confirmed: require_env now returns Result<()> instead of Result.All call sites have been properly updated. The function is used for validation only, with callers retrieving the actual environment variable value separately via
Client::from_env()(for GEMINI_API_KEY) orstd::env::var()(for SERPER_API_KEY). This pattern is consistently applied across all three usages in src/pipline.rs.
| [package] | ||
| name = "rigscribe" | ||
| version = "0.1.4" | ||
| version = "0.1.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat Cargo.tomlRepository: hghalebi/rigscribe
Length of output: 405
🏁 Script executed:
#!/bin/bash
# Check if there are any dependents or published versions
echo "Checking for published versions on crates.io..."
curl -s "https://crates.io/api/v1/crates/rigscribe" | jq '.versions[] | .num' 2>/dev/null || echo "Not found or not published"
# Check for any references in Cargo.lock (if exists)
if [ -f "Cargo.lock" ]; then
echo -e "\nChecking Cargo.lock for version references..."
grep -A2 "name = \"rigscribe\"" Cargo.lock || echo "No references found in Cargo.lock"
fi
# Check git history for version changes
echo -e "\nChecking git history for version changes in Cargo.toml..."
git log --oneline -n 20 -- Cargo.toml 2>/dev/null | head -5 || echo "Git history not available"Repository: hghalebi/rigscribe
Length of output: 353
Version downgrade violates semantic versioning conventions.
The package is already published on crates.io with version 0.1.4 as the most recent release. Reverting to 0.1.0 will create version conflicts and confusion for dependents. Semantic versioning requires version numbers to move forward, not backward. Either bump the version to 0.1.5 or higher, or clarify the versioning strategy if this is intentional.
🤖 Prompt for AI Agents
In Cargo.toml around line 3, the version was changed back to 0.1.0 which
downgrades the published crate (latest on crates.io is 0.1.4); update the
version to a forward-moving release (e.g., 0.1.5 or higher) to respect semantic
versioning and avoid conflicts with dependents, or add a comment/metadata
explaining an intentional alternative versioning strategy if this rollback is
deliberate.
| mod error; | ||
| pub mod agents; | ||
| pub mod tools; | ||
| pub mod pipline; |
There was a problem hiding this comment.
Fix module name typo.
The module name "pipline" appears to be a typo for "pipeline". This affects the public API surface and should be corrected before release.
🔎 Proposed fix
Rename the file from src/pipline.rs to src/pipeline.rs, then update the module declaration:
-pub mod pipline;
+pub mod pipeline;And update the import:
-use pipline::optimizer;
+use pipeline::optimizer;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub mod pipline; | |
| pub mod pipeline; |
🤖 Prompt for AI Agents
In src/lib.rs around line 2, the module declaration "pub mod pipline;" is a typo
and should be "pipeline"; rename the physical file src/pipline.rs to
src/pipeline.rs, update the module declaration in src/lib.rs to use the
corrected name, and update any use/import paths across the codebase that
reference "pipline" to "pipeline" so the public API and internal references
remain consistent.
| #[tokio::main] | ||
| async fn main() -> Result<()> { | ||
| // create caching path | ||
| let cache_path = PathBuf::from("./.prompts_perssitense_cache"); |
There was a problem hiding this comment.
Fix typo in variable name.
The variable name contains a typo: "perssitense" should be "persistence".
🔎 Proposed fix
- let cache_path = PathBuf::from("./.prompts_perssitense_cache");
+ let cache_path = PathBuf::from("./.prompts_persistence_cache");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let cache_path = PathBuf::from("./.prompts_perssitense_cache"); | |
| let cache_path = PathBuf::from("./.prompts_persistence_cache"); |
🤖 Prompt for AI Agents
In src/main.rs around line 9, the PathBuf is created with a typo in the path
variable name "perssitense" — change the string and any variable names
referencing it to use "persistence" (e.g., ".prompts_persistence_cache") so the
path and identifier read correctly; update any other occurrences in the
file/project to match the corrected spelling to avoid runtime or logical errors.
| let id = ScopeId(2032); | ||
| let id = ScopeId(2028); | ||
| // Input: The raw, often vague user intent. | ||
| let raw_prompt = "write a python fonction"; |
There was a problem hiding this comment.
Fix typo in comment.
The comment contains a typo: "fonction" should be "function".
🔎 Proposed fix
- let raw_prompt = "write a python fonction";
+ let raw_prompt = "write a python function";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let raw_prompt = "write a python fonction"; | |
| let raw_prompt = "write a python function"; |
🤖 Prompt for AI Agents
In src/main.rs around line 13, the raw_prompt string contains a typo "fonction";
update the string to use the correct English word "function" (i.e., change
"write a python fonction" to "write a python function") so the prompt reads
correctly.
| let parameters = serde_json::to_value(schema).unwrap(); | ||
| ToolDefinition { | ||
| name: "Deconstructor".to_string(), | ||
| description: "this tools take a raw prompte and give back it Specification include goal and constrian".to_string(), |
There was a problem hiding this comment.
Fix typos in tool description.
The description contains typos: "prompte" should be "prompt", and "constrian" should be "constraint".
🔎 Proposed fix
- description: "this tools take a raw prompte and give back it Specification include goal and constrian".to_string(),
+ description: "This tool takes a raw prompt and returns its Specification including goal and constraints".to_string(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: "this tools take a raw prompte and give back it Specification include goal and constrian".to_string(), | |
| description: "This tool takes a raw prompt and returns its Specification including goal and constraints".to_string(), |
🤖 Prompt for AI Agents
In src/pipline.rs around line 31, the tool description string contains typos:
change "this tools take a raw prompte and give back it Specification include
goal and constrian" to a corrected, grammatical sentence such as "this tool
takes a raw prompt and returns its specification including goal and constraint"
(adjust capitalization/punctuation to match surrounding code style).
| async fn call(&self, args: Self::Args) -> Result<Self::Output> { | ||
| println!("[Tool Calling]-> WebSearcher"); | ||
| let api_key = std::env::var("SERPER_API_KEY").map_err( | ||
| |e| | ||
| ScribeError::Config(format!("SERPER_API_KEY not set: {}", e)) | ||
| ); | ||
| let markdown = get_markdown_for_query(&args.query, &api_key?.to_string()).await; | ||
| Ok(markdown.unwrap().to_string()) | ||
| } |
There was a problem hiding this comment.
Inconsistent environment variable validation and error handling.
The WebSearcher tool uses a different pattern for environment variable validation compared to other tools. It also uses .unwrap() on line 128 which can panic. Consider using require_env("SERPER_API_KEY")? for consistency, then std::env::var to get the value.
🔎 Proposed fix
async fn call(&self, args: Self::Args) -> Result<Self::Output> {
println!("[Tool Calling]-> WebSearcher");
- let api_key = std::env::var("SERPER_API_KEY").map_err(
- |e|
- ScribeError::Config(format!("SERPER_API_KEY not set: {}", e))
- );
- let markdown = get_markdown_for_query(&args.query, &api_key?.to_string()).await;
- Ok(markdown.unwrap().to_string())
+ require_env("SERPER_API_KEY")?;
+ let api_key = std::env::var("SERPER_API_KEY")
+ .map_err(|e| ScribeError::Config(format!("SERPER_API_KEY not set: {}", e)))?;
+ let markdown = get_markdown_for_query(&args.query, &api_key)
+ .await
+ .map_err(|e| ScribeError::Config(format!("Web search failed: {:?}", e)))?;
+ Ok(markdown)Note: This assumes get_markdown_for_query returns a Result. If it returns Option, adjust accordingly.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pipline.rs around lines 121 to 129, the code validates the SERPER_API_KEY
inconsistently and calls unwrap() which can panic; replace the ad-hoc
std::env::var/map_err and the subsequent api_key?.to_string() with the project's
helper require_env("SERPER_API_KEY")? to obtain a validated String, then pass
that String (or &str) into get_markdown_for_query and handle its Result/Option
without unwrap by propagating errors with ? or mapping None to a ScribeError,
returning Ok(markdown) only after safely converting to String.
| pub async fn optimizer(prompt: Intent) -> Result<Artifact> { | ||
| require_env("GEMINI_API_KEY")?; | ||
| let client = Client::from_env(); | ||
| let system_prompt = read_artifact("data/optimizer.json") | ||
| .await | ||
| .unwrap() | ||
| .system_prompt; | ||
|
|
||
| let prompt_officer = client | ||
| .agent(MODEL) | ||
| .preamble(system_prompt.as_str()) | ||
| .tool(Deconstructor) | ||
| .tool(PromptReviewer) | ||
| .tool(WebSearcher) | ||
| .build(); | ||
|
|
||
| let input = format!( | ||
| "Follow this workflow to optimize the prompt: | ||
| 1. Use the Deconstructor tool to analyze the goal and constraints of: '{}' | ||
| 2. Use the PromptReviewer to check and refine the draft. | ||
| 3. Use the WebSearcher to find the best practice related task/goal. | ||
| 4. Finally, provide the optimized system prompt. | ||
| Constraint: The final output must be the system prompt only, but you MUST use your tools first to arrive at that result.", | ||
| prompt.text | ||
| ); | ||
| let optimized_prompt = prompt_officer | ||
| .prompt(input) | ||
| .multi_turn(10) | ||
| .await | ||
| .map_err(map_provider_error)?; | ||
| let artifact = Artifact { | ||
| system_prompt: optimized_prompt, | ||
| signed_by: "".to_string(), | ||
| }; | ||
|
|
||
| Ok(artifact) | ||
| } |
There was a problem hiding this comment.
Unwrap can cause panic - use proper error handling.
Lines 134-137 use .unwrap() when reading the optimizer artifact, which will panic if the file is missing or malformed. Use the ? operator to propagate errors properly.
🔎 Proposed fix
let system_prompt = read_artifact("data/optimizer.json")
.await
- .unwrap()
+ ?
.system_prompt;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn optimizer(prompt: Intent) -> Result<Artifact> { | |
| require_env("GEMINI_API_KEY")?; | |
| let client = Client::from_env(); | |
| let system_prompt = read_artifact("data/optimizer.json") | |
| .await | |
| .unwrap() | |
| .system_prompt; | |
| let prompt_officer = client | |
| .agent(MODEL) | |
| .preamble(system_prompt.as_str()) | |
| .tool(Deconstructor) | |
| .tool(PromptReviewer) | |
| .tool(WebSearcher) | |
| .build(); | |
| let input = format!( | |
| "Follow this workflow to optimize the prompt: | |
| 1. Use the Deconstructor tool to analyze the goal and constraints of: '{}' | |
| 2. Use the PromptReviewer to check and refine the draft. | |
| 3. Use the WebSearcher to find the best practice related task/goal. | |
| 4. Finally, provide the optimized system prompt. | |
| Constraint: The final output must be the system prompt only, but you MUST use your tools first to arrive at that result.", | |
| prompt.text | |
| ); | |
| let optimized_prompt = prompt_officer | |
| .prompt(input) | |
| .multi_turn(10) | |
| .await | |
| .map_err(map_provider_error)?; | |
| let artifact = Artifact { | |
| system_prompt: optimized_prompt, | |
| signed_by: "".to_string(), | |
| }; | |
| Ok(artifact) | |
| } | |
| pub async fn optimizer(prompt: Intent) -> Result<Artifact> { | |
| require_env("GEMINI_API_KEY")?; | |
| let client = Client::from_env(); | |
| let system_prompt = read_artifact("data/optimizer.json") | |
| .await? | |
| .system_prompt; | |
| let prompt_officer = client | |
| .agent(MODEL) | |
| .preamble(system_prompt.as_str()) | |
| .tool(Deconstructor) | |
| .tool(PromptReviewer) | |
| .tool(WebSearcher) | |
| .build(); | |
| let input = format!( | |
| "Follow this workflow to optimize the prompt: | |
| 1. Use the Deconstructor tool to analyze the goal and constraints of: '{}' | |
| 2. Use the PromptReviewer to check and refine the draft. | |
| 3. Use the WebSearcher to find the best practice related task/goal. | |
| 4. Finally, provide the optimized system prompt. | |
| Constraint: The final output must be the system prompt only, but you MUST use your tools first to arrive at that result.", | |
| prompt.text | |
| ); | |
| let optimized_prompt = prompt_officer | |
| .prompt(input) | |
| .multi_turn(10) | |
| .await | |
| .map_err(map_provider_error)?; | |
| let artifact = Artifact { | |
| system_prompt: optimized_prompt, | |
| signed_by: "".to_string(), | |
| }; | |
| Ok(artifact) | |
| } |
🤖 Prompt for AI Agents
In src/pipline.rs around lines 131 to 168, the code uses .await.unwrap() when
reading the optimizer artifact which can panic; replace the unwrap with proper
error propagation by using the ? operator: await the read_artifact call into a
local variable (e.g. let opt_artifact =
read_artifact("data/optimizer.json").await?;) and then use
opt_artifact.system_prompt, so the function returns an Err instead of panicking
when the file is missing or malformed.
| //pub const MODEL: &str = "gemini-3-pro-preview"; // does not work | ||
| //pub const MODEL: &str = "gemini-3-flash-preview"; // does not work | ||
| //pub const MODEL: &str = "gemini-2.5-flash-lite"; //does not work | ||
| pub const MODEL: &str = "gemini-2.5-pro"; // it works | ||
| //pub const MODEL: &str = "gemini-2.5-flash"; //it works | ||
| //pub const MODEL: &str = "gemini-2.0-flash-lite"; // it works but out come is so low quality | ||
| //pub const MODEL: &str = "gemini-1.5-pro"; // it does not works |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
Multiple commented MODEL constants clutter the code. If these are for documentation purposes, consider moving them to a comment block or removing them entirely.
🔎 Proposed fix
-//pub const MODEL: &str = "gemini-3-pro-preview"; // does not work
-//pub const MODEL: &str = "gemini-3-flash-preview"; // does not work
-//pub const MODEL: &str = "gemini-2.5-flash-lite"; //does not work
pub const MODEL: &str = "gemini-2.5-pro"; // it works
-//pub const MODEL: &str = "gemini-2.5-flash"; //it works
-//pub const MODEL: &str = "gemini-2.0-flash-lite"; // it works but out come is so low quality
-//pub const MODEL: &str = "gemini-1.5-pro"; // it does not worksIf you want to keep a record of tested models, consider adding a comment block:
+// Tested models:
+// - "gemini-2.5-pro" ✓ (currently in use)
+// - "gemini-2.5-flash" ✓
+// - "gemini-2.0-flash-lite" ✓ (low quality)
+// Models that didn't work: gemini-3-pro-preview, gemini-3-flash-preview, gemini-2.5-flash-lite, gemini-1.5-pro
pub const MODEL: &str = "gemini-2.5-pro"; // it works📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| //pub const MODEL: &str = "gemini-3-pro-preview"; // does not work | |
| //pub const MODEL: &str = "gemini-3-flash-preview"; // does not work | |
| //pub const MODEL: &str = "gemini-2.5-flash-lite"; //does not work | |
| pub const MODEL: &str = "gemini-2.5-pro"; // it works | |
| //pub const MODEL: &str = "gemini-2.5-flash"; //it works | |
| //pub const MODEL: &str = "gemini-2.0-flash-lite"; // it works but out come is so low quality | |
| //pub const MODEL: &str = "gemini-1.5-pro"; // it does not works | |
| pub const MODEL: &str = "gemini-2.5-pro"; // it works |
| //pub const MODEL: &str = "gemini-3-pro-preview"; // does not work | |
| //pub const MODEL: &str = "gemini-3-flash-preview"; // does not work | |
| //pub const MODEL: &str = "gemini-2.5-flash-lite"; //does not work | |
| pub const MODEL: &str = "gemini-2.5-pro"; // it works | |
| //pub const MODEL: &str = "gemini-2.5-flash"; //it works | |
| //pub const MODEL: &str = "gemini-2.0-flash-lite"; // it works but out come is so low quality | |
| //pub const MODEL: &str = "gemini-1.5-pro"; // it does not works | |
| // Tested models: | |
| // - "gemini-2.5-pro" ✓ (currently in use) | |
| // - "gemini-2.5-flash" ✓ | |
| // - "gemini-2.0-flash-lite" ✓ (low quality) | |
| // Models that didn't work: gemini-3-pro-preview, gemini-3-flash-preview, gemini-2.5-flash-lite, gemini-1.5-pro | |
| pub const MODEL: &str = "gemini-2.5-pro"; // it works |
🤖 Prompt for AI Agents
In src/types.rs around lines 4 to 10, there are multiple commented-out MODEL
constants cluttering the file; remove all unused commented constant lines and
keep only the single active pub const MODEL: &str = "gemini-2.5-pro"; if you
want to retain a record of tested model names, collapse them into a single short
comment block or move them to README/changelog instead of leaving many commented
constants inline.
| #[derive(Debug, Clone)] | ||
| pub struct RigScribeConfig { | ||
| pub model: &'static str, | ||
| } | ||
| impl RigScribeConfig { | ||
| fn set_model(&mut self, model: &'static str) { | ||
| self.model = model; | ||
| } | ||
| } | ||
| impl Default for RigScribeConfig { | ||
| fn default() -> Self { | ||
| Self { model: MODEL } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent visibility in RigScribeConfig API.
The model field is public, but the set_model method is private. This creates an inconsistent API where users can directly modify the field but cannot use the setter. Either make set_model public or make the field private with a public getter.
🔎 Proposed fixes
Option 1: Make the setter public:
- fn set_model(&mut self, model: &'static str) {
+ pub fn set_model(&mut self, model: &'static str) {Option 2: Make the field private with a getter:
- pub model: &'static str,
+ model: &'static str,
}
impl RigScribeConfig {
+ pub fn model(&self) -> &'static str {
+ self.model
+ }
+ pub fn set_model(&mut self, model: &'static str) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[derive(Debug, Clone)] | |
| pub struct RigScribeConfig { | |
| pub model: &'static str, | |
| } | |
| impl RigScribeConfig { | |
| fn set_model(&mut self, model: &'static str) { | |
| self.model = model; | |
| } | |
| } | |
| impl Default for RigScribeConfig { | |
| fn default() -> Self { | |
| Self { model: MODEL } | |
| } | |
| } | |
| #[derive(Debug, Clone)] | |
| pub struct RigScribeConfig { | |
| pub model: &'static str, | |
| } | |
| impl RigScribeConfig { | |
| pub fn set_model(&mut self, model: &'static str) { | |
| self.model = model; | |
| } | |
| } | |
| impl Default for RigScribeConfig { | |
| fn default() -> Self { | |
| Self { model: MODEL } | |
| } | |
| } |
| #[derive(Debug, Clone)] | |
| pub struct RigScribeConfig { | |
| pub model: &'static str, | |
| } | |
| impl RigScribeConfig { | |
| fn set_model(&mut self, model: &'static str) { | |
| self.model = model; | |
| } | |
| } | |
| impl Default for RigScribeConfig { | |
| fn default() -> Self { | |
| Self { model: MODEL } | |
| } | |
| } | |
| #[derive(Debug, Clone)] | |
| pub struct RigScribeConfig { | |
| model: &'static str, | |
| } | |
| impl RigScribeConfig { | |
| pub fn model(&self) -> &'static str { | |
| self.model | |
| } | |
| pub fn set_model(&mut self, model: &'static str) { | |
| self.model = model; | |
| } | |
| } | |
| impl Default for RigScribeConfig { | |
| fn default() -> Self { | |
| Self { model: MODEL } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/types.rs around lines 11 to 24, the API is inconsistent because the field
`model` is public while `set_model` is private; fix by either making the setter
public (change `fn set_model` to `pub fn set_model`) so callers can use the
setter while keeping the field public, or make the field private (change `pub
model` to `model`) and add a public getter (e.g., `pub fn model(&self) ->
&'static str`) to expose read access and keep `set_model` private or public
depending on desired mutability.
| #[schemars( | ||
| description = "he raw user intent. You must analyze this to extract technical constraints." | ||
| )] | ||
| pub text: String, |
There was a problem hiding this comment.
Fix typo in schema description.
The description has a typo: "he raw user intent" should be "The raw user intent".
🔎 Proposed fix
#[schemars(
- description = "he raw user intent. You must analyze this to extract technical constraints."
+ description = "The raw user intent. You must analyze this to extract technical constraints."
)]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[schemars( | |
| description = "he raw user intent. You must analyze this to extract technical constraints." | |
| )] | |
| pub text: String, | |
| #[schemars( | |
| description = "The raw user intent. You must analyze this to extract technical constraints." | |
| )] | |
| pub text: String, |
🤖 Prompt for AI Agents
In src/types.rs around lines 27 to 30, the schemars description has a typo ("he
raw user intent"); update the string to "The raw user intent. You must analyze
this to extract technical constraints." so the sentence starts with a capital T
and reads correctly.
![kody-ai[bot]](https://avatars.githubusercontent.com/in/413034?s=60&v=4){kind=small}
| let system_prompt = read_artifact("data/prompt_officer.json") | ||
| .await | ||
| .unwrap() | ||
| .system_prompt; |
There was a problem hiding this comment.
let system_prompt = read_artifact("data/prompt_officer.json")
.await?
.system_prompt;Multiple instances of .unwrap() being used on Result types in src/pipline.rs (lines 86-89, 127-128, 134-137) which can cause the application to panic and crash if the operation fails. This bypasses the established error handling pattern.
This issue appears in multiple locations:
- src/pipline.rs: Lines 86-89
- src/pipline.rs: Lines 127-128
- src/pipline.rs: Lines 134-137
- src/pipline.rs: Lines 86-89
Replace all instances of.unwrap()with the?operator to properly propagate errors through the existingResult/ScribeErrorsystem.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| [package] | ||
| name = "rigscribe" | ||
| version = "0.1.4" | ||
| version = "0.1.0" |
There was a problem hiding this comment.
version = "0.1.5"The package version has been reverted from 0.1.4 to 0.1.0. This is a version downgrade, which violates the principles of semantic versioning and prevents publishing to a package registry.
Why this is a problem:
Package registries like crates.io enforce that new versions must be greater than all previously published versions. Attempting to publish version 0.1.0 after 0.1.4 has been released will fail, blocking all future releases.
How to fix:
To maintain a valid release history, the version should be incremented based on the nature of the changes. Since this PR contains significant refactoring, incrementing to at least 0.1.5 (for a patch) or 0.2.0 (if there are breaking API changes) is recommended.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| let api_key = std::env::var("SERPER_API_KEY").map_err( | ||
| |e| | ||
| ScribeError::Config(format!("SERPER_API_KEY not set: {}", e)) | ||
| ); | ||
| let markdown = get_markdown_for_query(&args.query, &api_key?.to_string()).await; |
There was a problem hiding this comment.
require_env("SERPER_API_KEY")?;
// .unwrap() is safe here because require_env just checked for presence.
let api_key = std::env::var("SERPER_API_KEY").unwrap();
let markdown = get_markdown_for_query(&args.query, &api_key).await;I noticed the check for SERPER_API_KEY in WebSearcher::call (src/pipline.rs:123-127) is implemented manually, while other parts of the file use the require_env utility from src/utilities.rs. This creates a separate error handling path for a similar task. To keep the codebase consistent and easier to maintain, could we switch this to use the shared require_env utility? This centralizes the validation logic and ensures all configuration error messages are uniform.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| pub fn require_env(name: &str) -> Result<()> { | ||
| if std::env::var(name).is_ok() { | ||
| Ok(()) | ||
| } else { | ||
| Err(ScribeError::Config(format!("{name} is missing!"))) | ||
| } | ||
| } |
There was a problem hiding this comment.
pub fn require_env(name: &str) -> Result<()> {
match std::env::var(name) {
Ok(val) if !val.is_empty() => Ok(()),
_ => Err(ScribeError::Config(format!(
"{name} is missing or empty!"
))),
}
}The require_env function is intended to validate the presence of a required environment variable. However, the current implementation using std::env::var(name).is_ok() will incorrectly return Ok(()) if the environment variable is present but set to an empty string (e.g., API_KEY="").
This is a silent failure. Downstream code that later reads this variable will receive an empty string, likely causing difficult-to-diagnose runtime errors like authentication failures or invalid configurations. The validation should ensure the variable is not just present, but also contains a non-empty value.
To fix this, fetch the value and explicitly check if it's empty.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
What changed
agents/andtools/directories into a singlepipline.rsmoduletypes/artifact.rs,types/pipeline.rs,types/common.rs,types/config.rs) into a singletypes.rsfileClientErrorvariant from error handling enumrequire_envutility to returnResult<()>instead ofResult<String>read_artifact().multi_turn(10)to optimizer agent for extended conversation capability.gitignoreformatting and modifiedScopeIdin main exampleCargo.tomlversion from 0.1.4 to 0.1.0 and removed package metadataWhy
This refactoring simplifies the codebase architecture by reducing directory nesting and consolidating related functionality. The changes improve maintainability by having all pipeline logic (optimizer agent and tools: Deconstructor, PromptReviewer, WebSearcher) in one location, and all type definitions in a single file. The switch to file-based artifact loading provides more flexibility for configuration management.
Note: All documentation in this codebase is auto-generated by Gemini.
This PR was created by Graphite Agent
Summary by CodeRabbit
Release Notes
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.
This pull request introduces a significant refactoring of the project's architecture, primarily by consolidating agent and tool definitions into a new
pipelinemodule and centralizing all core data types.Key changes include:
optimizeragent and its associated tools (Deconstructor,PromptReviewer,WebSearcher) have been moved from their previousagentsandtoolsmodules into a newsrc/pipline.rsmodule. This streamlines the definition and management of the core agentic workflow.Intent,Specification,Artifact,ScopeId,Webquery, and configuration constants, are now defined in a singlesrc/types.rsfile, simplifying type management and reducing file fragmentation.optimizerfunction now utilizesClient::from_env()for client initialization, loads system prompts dynamically from files usingread_artifact, and incorporates amulti_turn(10)option for the prompt officer, suggesting improved conversational capabilities.require_envutility function has been updated to solely validate the presence of environment variables, with value retrieval now handled directly by therigclient (e.g.,Client::from_env()).Cargo.tomlfile has been updated to reflect a new version (0.1.0) and simplify project metadata by removing description, license, and repository fields..gitignorefile has been updated, and a hardcodedScopeIdvalue insrc/main.rshas been adjusted.This refactoring aims to improve code organization, maintainability, and clarity of the core agentic pipeline.