Skip to content

Commit cbba8a6

Browse files
committed
feat(reviewer): propagate provider errors; pass prompts dir to review tool
Change Reviewer::new to async fn new() -> Result<Self>, replacing .expect() with ? for error propagation on BaselineRegistry and create_provider_cached. Update main.rs call site to match on Result. Add --prompts arg to run_review_tool using settings.get_prompts_dir(). Update src/bin/review.rs to use ToolBox::with_config, PromptRegistry::with_settings, and Option<PathBuf> for --prompts arg. Signed-off-by: derekbarbosa <derekasobrab@gmail.com>
1 parent 600cabe commit cbba8a6

3 files changed

Lines changed: 39 additions & 30 deletions

File tree

src/bin/review.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ struct Args {
4343
#[arg(long)]
4444
worktree_dir: Option<PathBuf>,
4545

46-
#[arg(long, default_value = "third_party/prompts/kernel")]
47-
prompts: PathBuf,
46+
#[arg(long)]
47+
prompts: Option<PathBuf>,
4848

4949
/// If set, only review the patch with this index (1-based usually).
5050
/// Previous patches (with lower index) will be applied but not reviewed.
@@ -366,11 +366,25 @@ async fn main() -> Result<()> {
366366
let provider = sashiko::ai::create_provider(&settings).expect("Failed to create AI provider");
367367

368368
// Enable read_prompt tool only if explicit caching is NOT used.
369-
let prompts_dir = PathBuf::from("third_party/prompts/kernel");
369+
let prompts_dir = args
370+
.prompts
371+
.clone()
372+
.unwrap_or_else(|| PathBuf::from(settings.get_prompts_dir()));
370373
let prompts_tool_path = Some(prompts_dir.join("tool.md"));
371374

372-
let tools = ToolBox::new(worktree.path.clone(), prompts_tool_path);
373-
let prompts = PromptRegistry::new(args.prompts.clone());
375+
let tools = ToolBox::with_config(
376+
worktree.path.clone(),
377+
prompts_tool_path,
378+
settings.tools.as_ref(),
379+
);
380+
381+
let prompts = if settings.prompts.is_some() {
382+
PromptRegistry::with_settings(settings.prompts.as_ref())
383+
.await
384+
.expect("Failed to load prompts with settings")
385+
} else {
386+
PromptRegistry::new(prompts_dir)
387+
};
374388

375389
// Calculate series range (baseline..last_patch)
376390
let series_range = calculate_series_range(

src/main.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -570,10 +570,16 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
570570
}
571571

572572
// Start Reviewer Service
573-
let reviewer = Reviewer::new(db.clone(), settings.clone()).await;
574-
tokio::spawn(async move {
575-
reviewer.start().await;
576-
});
573+
match Reviewer::new(db.clone(), settings.clone()).await {
574+
Ok(reviewer) => {
575+
tokio::spawn(async move {
576+
reviewer.start().await;
577+
});
578+
}
579+
Err(e) => {
580+
error!("Failed to initialize Reviewer service: {}", e);
581+
}
582+
}
577583

578584
let metrics_db = db.clone();
579585
tokio::spawn(async move {

src/reviewer.rs

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -90,43 +90,30 @@ impl Reviewer {
9090
///
9191
/// * `db` - The database connection.
9292
/// * `settings` - Application settings.
93-
pub async fn new(db: Arc<Database>, settings: Settings) -> Self {
93+
pub async fn new(db: Arc<Database>, settings: Settings) -> Result<Self> {
9494
let concurrency = settings.review.concurrency;
9595
let repo_path = PathBuf::from(&settings.git.repository_path);
9696

97-
let baseline_registry =
98-
match BaselineRegistry::new(&repo_path, settings.git.custom_remotes.clone()) {
99-
Ok(r) => Arc::new(r),
100-
Err(e) => {
101-
error!(
102-
"Failed to initialize BaselineRegistry: {}. Using empty registry.",
103-
e
104-
);
105-
Arc::new(
106-
BaselineRegistry::new(&repo_path, settings.git.custom_remotes.clone())
107-
.unwrap_or_else(|_| {
108-
panic!("Critical error initializing BaselineRegistry: {}", e)
109-
}),
110-
)
111-
}
112-
};
97+
let baseline_registry = Arc::new(BaselineRegistry::new(
98+
&repo_path,
99+
settings.git.custom_remotes.clone(),
100+
)?);
113101

114102
let provider = create_provider_cached(
115103
&settings,
116104
settings.ai.response_cache,
117105
settings.ai.response_cache_ttl_days,
118106
)
119-
.await
120-
.expect("Failed to create AI provider");
107+
.await?;
121108

122-
Self {
109+
Ok(Self {
123110
db,
124111
settings,
125112
semaphore: Arc::new(Semaphore::new(concurrency)),
126113
baseline_registry,
127114
quota_manager: Arc::new(QuotaManager::new()),
128115
provider,
129-
}
116+
})
130117
}
131118

132119
/// Starts the reviewer service loop.
@@ -1553,6 +1540,8 @@ async fn run_review_tool(
15531540
cmd.arg("--stages").arg(stages_str);
15541541
}
15551542

1543+
cmd.arg("--prompts").arg(settings.get_prompts_dir());
1544+
15561545
cmd.stdin(Stdio::piped());
15571546
cmd.stdout(Stdio::piped());
15581547
cmd.stderr(Stdio::piped());

0 commit comments

Comments
 (0)