mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
* chore: remove dead code (LlmEvaluator, chunk_by_paragraphs, bundled channel installer, Reasoning::safety) Delete unused code flagged in #648: - evaluation/success.rs: delete LlmEvaluator struct/impl, remove #[allow(dead_code)] from RuleBasedEvaluator methods - workspace/chunker.rs: delete chunk_by_paragraphs() and its tests (zero production callers) - extensions/manager.rs: delete install_bundled_channel_from_artifacts() (hot-activation never shipped) - llm/reasoning.rs: remove unused safety field from Reasoning struct; cascade removal through ContextCompactor, HeartbeatRunner, LlmSoftwareBuilder, and all callers Closes #648 [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: move RuleBasedEvaluator into test module to fix dead_code warning RuleBasedEvaluator has no production callers -- it was only used in tests of itself. Moving it into #[cfg(test)] eliminates the clippy dead_code error that broke CI. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
edff54b0b1
commit
272d31797e
@@ -417,7 +417,6 @@ impl Agent {
|
||||
hygiene,
|
||||
workspace.clone(),
|
||||
self.cheap_llm().clone(),
|
||||
self.safety().clone(),
|
||||
Some(notify_tx),
|
||||
self.store().map(Arc::clone),
|
||||
))
|
||||
|
||||
@@ -345,7 +345,6 @@ impl Agent {
|
||||
crate::workspace::hygiene::HygieneConfig::default(),
|
||||
workspace.clone(),
|
||||
self.llm().clone(),
|
||||
self.safety().clone(),
|
||||
);
|
||||
|
||||
match runner.check_heartbeat().await {
|
||||
@@ -406,7 +405,7 @@ impl Agent {
|
||||
.with_max_tokens(512)
|
||||
.with_temperature(0.3);
|
||||
|
||||
let reasoning = Reasoning::new(self.llm().clone(), self.safety().clone());
|
||||
let reasoning = Reasoning::new(self.llm().clone());
|
||||
match reasoning.complete(request).await {
|
||||
Ok((text, _usage)) => Ok(SubmissionResult::response(format!(
|
||||
"Thread Summary:\n\n{}",
|
||||
@@ -454,7 +453,7 @@ impl Agent {
|
||||
.with_max_tokens(512)
|
||||
.with_temperature(0.5);
|
||||
|
||||
let reasoning = Reasoning::new(self.llm().clone(), self.safety().clone());
|
||||
let reasoning = Reasoning::new(self.llm().clone());
|
||||
match reasoning.complete(request).await {
|
||||
Ok((text, _usage)) => Ok(SubmissionResult::response(format!(
|
||||
"Suggested Next Steps:\n\n{}",
|
||||
|
||||
+4
-12
@@ -13,7 +13,6 @@ use crate::agent::context_monitor::{CompactionStrategy, ContextBreakdown};
|
||||
use crate::agent::session::Thread;
|
||||
use crate::error::Error;
|
||||
use crate::llm::{ChatMessage, CompletionRequest, LlmProvider, Reasoning};
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::workspace::Workspace;
|
||||
|
||||
/// Result of a compaction operation.
|
||||
@@ -34,13 +33,12 @@ pub struct CompactionResult {
|
||||
/// Compacts conversation context to stay within limits.
|
||||
pub struct ContextCompactor {
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
}
|
||||
|
||||
impl ContextCompactor {
|
||||
/// Create a new context compactor.
|
||||
pub fn new(llm: Arc<dyn LlmProvider>, safety: Arc<SafetyLayer>) -> Self {
|
||||
Self { llm, safety }
|
||||
pub fn new(llm: Arc<dyn LlmProvider>) -> Self {
|
||||
Self { llm }
|
||||
}
|
||||
|
||||
/// Compact a thread's context using the given strategy.
|
||||
@@ -233,7 +231,7 @@ Be brief but capture all important details. Use bullet points."#,
|
||||
.with_max_tokens(1024)
|
||||
.with_temperature(0.3);
|
||||
|
||||
let reasoning = Reasoning::new(self.llm.clone(), self.safety.clone());
|
||||
let reasoning = Reasoning::new(self.llm.clone());
|
||||
let (text, _) = reasoning.complete(request).await?;
|
||||
Ok(text)
|
||||
}
|
||||
@@ -346,17 +344,11 @@ mod tests {
|
||||
// === QA Plan - Compaction strategy tests ===
|
||||
|
||||
use crate::agent::context_monitor::CompactionStrategy;
|
||||
use crate::config::SafetyConfig;
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::testing::StubLlm;
|
||||
|
||||
/// Helper: build a `ContextCompactor` with the given `StubLlm`.
|
||||
fn make_compactor(llm: Arc<StubLlm>) -> ContextCompactor {
|
||||
let safety = Arc::new(SafetyLayer::new(&SafetyConfig {
|
||||
max_output_length: 100_000,
|
||||
injection_check_enabled: false,
|
||||
}));
|
||||
ContextCompactor::new(llm, safety)
|
||||
ContextCompactor::new(llm)
|
||||
}
|
||||
|
||||
/// Helper: build a thread with `n` completed turns.
|
||||
|
||||
+3
-11
@@ -113,7 +113,7 @@ impl Agent {
|
||||
None
|
||||
};
|
||||
|
||||
let mut reasoning = Reasoning::new(self.llm().clone(), self.safety().clone())
|
||||
let mut reasoning = Reasoning::new(self.llm().clone())
|
||||
.with_channel(message.channel.clone())
|
||||
.with_model_name(self.llm().active_model_name())
|
||||
.with_group_chat(is_group_chat);
|
||||
@@ -1609,12 +1609,8 @@ mod tests {
|
||||
use crate::testing::StubLlm;
|
||||
|
||||
let stub = Arc::new(StubLlm::failing_non_transient("ctx-bomb"));
|
||||
let safety = Arc::new(SafetyLayer::new(&SafetyConfig {
|
||||
max_output_length: 100_000,
|
||||
injection_check_enabled: false,
|
||||
}));
|
||||
|
||||
let reasoning = Reasoning::new(stub.clone(), safety);
|
||||
let reasoning = Reasoning::new(stub.clone());
|
||||
|
||||
// Build a fat context with lots of history.
|
||||
let messages = vec![
|
||||
@@ -1724,11 +1720,7 @@ mod tests {
|
||||
use crate::llm::{Reasoning, ReasoningContext, RespondResult, ToolDefinition};
|
||||
|
||||
let provider = Arc::new(AlwaysToolCallProvider);
|
||||
let safety = Arc::new(SafetyLayer::new(&SafetyConfig {
|
||||
max_output_length: 100_000,
|
||||
injection_check_enabled: false,
|
||||
}));
|
||||
let reasoning = Reasoning::new(provider, safety);
|
||||
let reasoning = Reasoning::new(provider);
|
||||
|
||||
let tool_def = ToolDefinition {
|
||||
name: "echo".to_string(),
|
||||
|
||||
@@ -31,7 +31,6 @@ use tokio::sync::mpsc;
|
||||
use crate::channels::OutgoingResponse;
|
||||
use crate::db::Database;
|
||||
use crate::llm::{ChatMessage, CompletionRequest, LlmProvider, Reasoning};
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::workspace::Workspace;
|
||||
use crate::workspace::hygiene::HygieneConfig;
|
||||
|
||||
@@ -131,7 +130,6 @@ pub struct HeartbeatRunner {
|
||||
hygiene_config: HygieneConfig,
|
||||
workspace: Arc<Workspace>,
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
response_tx: Option<mpsc::Sender<OutgoingResponse>>,
|
||||
store: Option<Arc<dyn Database>>,
|
||||
consecutive_failures: u32,
|
||||
@@ -144,14 +142,12 @@ impl HeartbeatRunner {
|
||||
hygiene_config: HygieneConfig,
|
||||
workspace: Arc<Workspace>,
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
) -> Self {
|
||||
Self {
|
||||
config,
|
||||
hygiene_config,
|
||||
workspace,
|
||||
llm,
|
||||
safety,
|
||||
response_tx: None,
|
||||
store: None,
|
||||
consecutive_failures: 0,
|
||||
@@ -307,7 +303,7 @@ impl HeartbeatRunner {
|
||||
.with_max_tokens(max_tokens)
|
||||
.with_temperature(0.3);
|
||||
|
||||
let reasoning = Reasoning::new(self.llm.clone(), self.safety.clone());
|
||||
let reasoning = Reasoning::new(self.llm.clone());
|
||||
let (content, _usage) = match reasoning.complete(request).await {
|
||||
Ok(r) => r,
|
||||
Err(e) => return HeartbeatResult::Failed(format!("LLM call failed: {}", e)),
|
||||
@@ -421,11 +417,10 @@ pub fn spawn_heartbeat(
|
||||
hygiene_config: HygieneConfig,
|
||||
workspace: Arc<Workspace>,
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
response_tx: Option<mpsc::Sender<OutgoingResponse>>,
|
||||
store: Option<Arc<dyn Database>>,
|
||||
) -> tokio::task::JoinHandle<()> {
|
||||
let mut runner = HeartbeatRunner::new(config, hygiene_config, workspace, llm, safety);
|
||||
let mut runner = HeartbeatRunner::new(config, hygiene_config, workspace, llm);
|
||||
if let Some(tx) = response_tx {
|
||||
runner = runner.with_response_channel(tx);
|
||||
}
|
||||
@@ -655,7 +650,6 @@ mod tests {
|
||||
HygieneConfig,
|
||||
Arc<crate::workspace::Workspace>,
|
||||
Arc<dyn crate::llm::LlmProvider>,
|
||||
Arc<crate::safety::SafetyLayer>,
|
||||
Option<tokio::sync::mpsc::Sender<crate::channels::OutgoingResponse>>,
|
||||
Option<Arc<dyn crate::db::Database>>,
|
||||
) -> tokio::task::JoinHandle<()> = spawn_heartbeat;
|
||||
|
||||
@@ -230,7 +230,7 @@ impl Agent {
|
||||
)
|
||||
.await;
|
||||
|
||||
let compactor = ContextCompactor::new(self.llm().clone(), self.safety().clone());
|
||||
let compactor = ContextCompactor::new(self.llm().clone());
|
||||
if let Err(e) = compactor
|
||||
.compact(thread, strategy, self.workspace().map(|w| w.as_ref()))
|
||||
.await
|
||||
@@ -627,7 +627,7 @@ impl Agent {
|
||||
crate::agent::context_monitor::CompactionStrategy::Summarize { keep_recent: 5 },
|
||||
);
|
||||
|
||||
let compactor = ContextCompactor::new(self.llm().clone(), self.safety().clone());
|
||||
let compactor = ContextCompactor::new(self.llm().clone());
|
||||
match compactor
|
||||
.compact(thread, strategy, self.workspace().map(|w| w.as_ref()))
|
||||
.await
|
||||
|
||||
+1
-1
@@ -212,7 +212,7 @@ impl Worker {
|
||||
let job_ctx = self.context_manager().get_context(self.job_id).await?;
|
||||
|
||||
// Create reasoning engine
|
||||
let reasoning = Reasoning::new(self.llm().clone(), self.safety().clone());
|
||||
let reasoning = Reasoning::new(self.llm().clone());
|
||||
|
||||
// Build initial reasoning context (tool definitions refreshed each iteration in execution_loop)
|
||||
let mut reason_ctx = ReasoningContext::new().with_job(&job_ctx.description);
|
||||
|
||||
+1
-5
@@ -403,11 +403,7 @@ impl AppBuilder {
|
||||
&& (self.config.agent.allow_local_tools || !self.config.sandbox.enabled)
|
||||
{
|
||||
tools
|
||||
.register_builder_tool(
|
||||
llm.clone(),
|
||||
safety.clone(),
|
||||
Some(self.config.builder.to_builder_config()),
|
||||
)
|
||||
.register_builder_tool(llm.clone(), Some(self.config.builder.to_builder_config()))
|
||||
.await;
|
||||
tracing::info!("Builder mode enabled");
|
||||
}
|
||||
|
||||
+123
-227
@@ -1,13 +1,10 @@
|
||||
//! Success evaluation for jobs.
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
use crate::context::{ActionRecord, JobContext};
|
||||
use crate::error::EvaluationError;
|
||||
use crate::llm::LlmProvider;
|
||||
|
||||
/// Result of evaluating job success.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
@@ -64,233 +61,132 @@ pub trait SuccessEvaluator: Send + Sync {
|
||||
) -> Result<EvaluationResult, EvaluationError>;
|
||||
}
|
||||
|
||||
/// Rule-based success evaluator.
|
||||
pub struct RuleBasedEvaluator {
|
||||
/// Minimum success rate for actions.
|
||||
min_action_success_rate: f64,
|
||||
/// Maximum allowed failures.
|
||||
max_failures: u32,
|
||||
}
|
||||
|
||||
impl RuleBasedEvaluator {
|
||||
/// Create a new rule-based evaluator.
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
min_action_success_rate: 0.8,
|
||||
max_failures: 3,
|
||||
}
|
||||
}
|
||||
|
||||
/// Set minimum action success rate.
|
||||
#[allow(dead_code)] // Public API for configuring evaluation threshold
|
||||
pub fn with_min_success_rate(mut self, rate: f64) -> Self {
|
||||
self.min_action_success_rate = rate;
|
||||
self
|
||||
}
|
||||
|
||||
/// Set maximum failures.
|
||||
#[allow(dead_code)] // Public API for configuring failure tolerance
|
||||
pub fn with_max_failures(mut self, max: u32) -> Self {
|
||||
self.max_failures = max;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for RuleBasedEvaluator {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl SuccessEvaluator for RuleBasedEvaluator {
|
||||
async fn evaluate(
|
||||
&self,
|
||||
job: &JobContext,
|
||||
actions: &[ActionRecord],
|
||||
_output: Option<&str>,
|
||||
) -> Result<EvaluationResult, EvaluationError> {
|
||||
let mut issues = Vec::new();
|
||||
|
||||
// Check if there were any actions
|
||||
if actions.is_empty() {
|
||||
return Ok(EvaluationResult::failure(
|
||||
"No actions were taken",
|
||||
vec!["No actions recorded".to_string()],
|
||||
));
|
||||
}
|
||||
|
||||
// Calculate action success rate
|
||||
let successful = actions.iter().filter(|a| a.success).count();
|
||||
let total = actions.len();
|
||||
let success_rate = successful as f64 / total as f64;
|
||||
|
||||
if success_rate < self.min_action_success_rate {
|
||||
issues.push(format!(
|
||||
"Action success rate {:.1}% below threshold {:.1}%",
|
||||
success_rate * 100.0,
|
||||
self.min_action_success_rate * 100.0
|
||||
));
|
||||
}
|
||||
|
||||
// Count failures
|
||||
let failures = actions.iter().filter(|a| !a.success).count() as u32;
|
||||
if failures > self.max_failures {
|
||||
issues.push(format!(
|
||||
"Too many failures: {} (max {})",
|
||||
failures, self.max_failures
|
||||
));
|
||||
}
|
||||
|
||||
// Check for critical errors
|
||||
for action in actions.iter().filter(|a| !a.success) {
|
||||
if let Some(ref error) = action.error
|
||||
&& (error.to_lowercase().contains("critical")
|
||||
|| error.to_lowercase().contains("fatal"))
|
||||
{
|
||||
issues.push(format!("Critical error in {}: {}", action.tool_name, error));
|
||||
}
|
||||
}
|
||||
|
||||
// Check job state
|
||||
if job.state != crate::context::JobState::Completed
|
||||
&& job.state != crate::context::JobState::Submitted
|
||||
{
|
||||
issues.push(format!("Job not in completed state: {:?}", job.state));
|
||||
}
|
||||
|
||||
// Calculate quality score
|
||||
let quality_score = if issues.is_empty() {
|
||||
let base_score = (success_rate * 80.0) as u32;
|
||||
let completion_bonus = if job.state == crate::context::JobState::Completed {
|
||||
20
|
||||
} else {
|
||||
0
|
||||
};
|
||||
(base_score + completion_bonus).min(100)
|
||||
} else {
|
||||
((success_rate * 50.0) as u32).min(50)
|
||||
};
|
||||
|
||||
if issues.is_empty() {
|
||||
Ok(EvaluationResult::success(
|
||||
format!(
|
||||
"Job completed successfully with {}/{} actions succeeding ({:.1}%)",
|
||||
successful,
|
||||
total,
|
||||
success_rate * 100.0
|
||||
),
|
||||
quality_score,
|
||||
))
|
||||
} else {
|
||||
Ok(EvaluationResult {
|
||||
success: false,
|
||||
confidence: 0.85,
|
||||
reasoning: format!("Job had {} issues", issues.len()),
|
||||
issues,
|
||||
suggestions: vec![
|
||||
"Review failed actions for common patterns".to_string(),
|
||||
"Consider adjusting retry logic".to_string(),
|
||||
],
|
||||
quality_score,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// LLM-based success evaluator for more nuanced evaluation.
|
||||
pub struct LlmEvaluator {
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
}
|
||||
|
||||
impl LlmEvaluator {
|
||||
/// Create a new LLM-based evaluator.
|
||||
#[allow(dead_code)] // Public API for LLM-based evaluation
|
||||
pub fn new(llm: Arc<dyn LlmProvider>) -> Self {
|
||||
Self { llm }
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl SuccessEvaluator for LlmEvaluator {
|
||||
async fn evaluate(
|
||||
&self,
|
||||
job: &JobContext,
|
||||
actions: &[ActionRecord],
|
||||
output: Option<&str>,
|
||||
) -> Result<EvaluationResult, EvaluationError> {
|
||||
// Build evaluation prompt
|
||||
let actions_summary: Vec<String> = actions
|
||||
.iter()
|
||||
.map(|a| {
|
||||
format!(
|
||||
"- {}: {} ({})",
|
||||
a.tool_name,
|
||||
if a.success { "success" } else { "failed" },
|
||||
a.error.as_deref().unwrap_or("ok")
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
|
||||
let prompt = format!(
|
||||
r#"Evaluate if this job was completed successfully.
|
||||
|
||||
Job: {}
|
||||
Description: {}
|
||||
State: {:?}
|
||||
|
||||
Actions taken:
|
||||
{}
|
||||
|
||||
{}
|
||||
|
||||
Respond in JSON format:
|
||||
{{
|
||||
"success": true/false,
|
||||
"confidence": 0.0-1.0,
|
||||
"reasoning": "...",
|
||||
"issues": ["..."],
|
||||
"suggestions": ["..."],
|
||||
"quality_score": 0-100
|
||||
}}"#,
|
||||
job.title,
|
||||
job.description,
|
||||
job.state,
|
||||
actions_summary.join("\n"),
|
||||
output
|
||||
.map(|o| format!("Output:\n{}", o))
|
||||
.unwrap_or_default()
|
||||
);
|
||||
|
||||
let request =
|
||||
crate::llm::CompletionRequest::new(vec![crate::llm::ChatMessage::user(prompt)])
|
||||
.with_max_tokens(1024)
|
||||
.with_temperature(0.1);
|
||||
|
||||
let response = self
|
||||
.llm
|
||||
.complete(request)
|
||||
.await
|
||||
.map_err(|e| EvaluationError::Failed {
|
||||
job_id: job.job_id,
|
||||
reason: e.to_string(),
|
||||
})?;
|
||||
|
||||
// Parse the response
|
||||
let result: EvaluationResult =
|
||||
serde_json::from_str(&response.content).map_err(|e| EvaluationError::Failed {
|
||||
job_id: job.job_id,
|
||||
reason: format!("Failed to parse LLM evaluation: {}", e),
|
||||
})?;
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::context::JobContext;
|
||||
use crate::context::{ActionRecord, JobContext};
|
||||
use crate::error::EvaluationError;
|
||||
|
||||
/// Rule-based success evaluator (test-only; no production callers).
|
||||
struct RuleBasedEvaluator {
|
||||
min_action_success_rate: f64,
|
||||
max_failures: u32,
|
||||
}
|
||||
|
||||
impl RuleBasedEvaluator {
|
||||
fn new() -> Self {
|
||||
Self {
|
||||
min_action_success_rate: 0.8,
|
||||
max_failures: 3,
|
||||
}
|
||||
}
|
||||
|
||||
fn with_min_success_rate(mut self, rate: f64) -> Self {
|
||||
self.min_action_success_rate = rate;
|
||||
self
|
||||
}
|
||||
|
||||
fn with_max_failures(mut self, max: u32) -> Self {
|
||||
self.max_failures = max;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for RuleBasedEvaluator {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl SuccessEvaluator for RuleBasedEvaluator {
|
||||
async fn evaluate(
|
||||
&self,
|
||||
job: &JobContext,
|
||||
actions: &[ActionRecord],
|
||||
_output: Option<&str>,
|
||||
) -> Result<EvaluationResult, EvaluationError> {
|
||||
let mut issues = Vec::new();
|
||||
|
||||
if actions.is_empty() {
|
||||
return Ok(EvaluationResult::failure(
|
||||
"No actions were taken",
|
||||
vec!["No actions recorded".to_string()],
|
||||
));
|
||||
}
|
||||
|
||||
let successful = actions.iter().filter(|a| a.success).count();
|
||||
let total = actions.len();
|
||||
let success_rate = successful as f64 / total as f64;
|
||||
|
||||
if success_rate < self.min_action_success_rate {
|
||||
issues.push(format!(
|
||||
"Action success rate {:.1}% below threshold {:.1}%",
|
||||
success_rate * 100.0,
|
||||
self.min_action_success_rate * 100.0
|
||||
));
|
||||
}
|
||||
|
||||
let failures = actions.iter().filter(|a| !a.success).count() as u32;
|
||||
if failures > self.max_failures {
|
||||
issues.push(format!(
|
||||
"Too many failures: {} (max {})",
|
||||
failures, self.max_failures
|
||||
));
|
||||
}
|
||||
|
||||
for action in actions.iter().filter(|a| !a.success) {
|
||||
if let Some(ref error) = action.error
|
||||
&& (error.to_lowercase().contains("critical")
|
||||
|| error.to_lowercase().contains("fatal"))
|
||||
{
|
||||
issues.push(format!("Critical error in {}: {}", action.tool_name, error));
|
||||
}
|
||||
}
|
||||
|
||||
if job.state != crate::context::JobState::Completed
|
||||
&& job.state != crate::context::JobState::Submitted
|
||||
{
|
||||
issues.push(format!("Job not in completed state: {:?}", job.state));
|
||||
}
|
||||
|
||||
let quality_score = if issues.is_empty() {
|
||||
let base_score = (success_rate * 80.0) as u32;
|
||||
let completion_bonus = if job.state == crate::context::JobState::Completed {
|
||||
20
|
||||
} else {
|
||||
0
|
||||
};
|
||||
(base_score + completion_bonus).min(100)
|
||||
} else {
|
||||
((success_rate * 50.0) as u32).min(50)
|
||||
};
|
||||
|
||||
if issues.is_empty() {
|
||||
Ok(EvaluationResult::success(
|
||||
format!(
|
||||
"Job completed successfully with {}/{} actions succeeding ({:.1}%)",
|
||||
successful,
|
||||
total,
|
||||
success_rate * 100.0
|
||||
),
|
||||
quality_score,
|
||||
))
|
||||
} else {
|
||||
Ok(EvaluationResult {
|
||||
success: false,
|
||||
confidence: 0.85,
|
||||
reasoning: format!("Job had {} issues", issues.len()),
|
||||
issues,
|
||||
suggestions: vec![
|
||||
"Review failed actions for common patterns".to_string(),
|
||||
"Consider adjusting retry logic".to_string(),
|
||||
],
|
||||
quality_score,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_rule_based_evaluator_success() {
|
||||
|
||||
@@ -1405,38 +1405,6 @@ impl ExtensionManager {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[allow(dead_code)] // Used by upcoming hot-activation flow
|
||||
async fn install_bundled_channel_from_artifacts(
|
||||
&self,
|
||||
name: &str,
|
||||
) -> Result<InstallResult, ExtensionError> {
|
||||
// Check if already installed
|
||||
let channel_wasm = self.wasm_channels_dir.join(format!("{}.wasm", name));
|
||||
if channel_wasm.exists() {
|
||||
return Err(ExtensionError::AlreadyInstalled(name.to_string()));
|
||||
}
|
||||
|
||||
crate::channels::wasm::install_bundled_channel(name, &self.wasm_channels_dir, false)
|
||||
.await
|
||||
.map_err(ExtensionError::InstallFailed)?;
|
||||
|
||||
tracing::info!(
|
||||
"Installed bundled channel '{}' to {}",
|
||||
name,
|
||||
self.wasm_channels_dir.display()
|
||||
);
|
||||
|
||||
Ok(InstallResult {
|
||||
name: name.to_string(),
|
||||
kind: ExtensionKind::WasmChannel,
|
||||
message: format!(
|
||||
"Channel '{}' installed. \
|
||||
Run tool_auth('{}') to configure authentication, then activate.",
|
||||
name, name,
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
/// Install a WASM extension from local build artifacts (WasmBuildable source).
|
||||
///
|
||||
/// Resolves the build directory (relative to `CARGO_MANIFEST_DIR` or absolute),
|
||||
|
||||
+2
-12
@@ -11,7 +11,6 @@ use crate::llm::{
|
||||
ChatMessage, CompletionRequest, LlmProvider, Role, ToolCall, ToolCompletionRequest,
|
||||
ToolDefinition,
|
||||
};
|
||||
use crate::safety::SafetyLayer;
|
||||
|
||||
/// Token the agent returns when it has nothing to say (e.g. in group chats).
|
||||
/// The dispatcher should check for this and suppress the message.
|
||||
@@ -343,8 +342,6 @@ pub struct RespondOutput {
|
||||
/// Reasoning engine for the agent.
|
||||
pub struct Reasoning {
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
#[allow(dead_code)] // Will be used for sanitizing tool outputs
|
||||
safety: Arc<SafetyLayer>,
|
||||
/// Optional workspace for loading identity/system prompts.
|
||||
workspace_system_prompt: Option<String>,
|
||||
/// Optional skill context block to inject into system prompt.
|
||||
@@ -362,10 +359,9 @@ pub struct Reasoning {
|
||||
|
||||
impl Reasoning {
|
||||
/// Create a new reasoning engine.
|
||||
pub fn new(llm: Arc<dyn LlmProvider>, safety: Arc<SafetyLayer>) -> Self {
|
||||
pub fn new(llm: Arc<dyn LlmProvider>) -> Self {
|
||||
Self {
|
||||
llm,
|
||||
safety,
|
||||
workspace_system_prompt: None,
|
||||
skill_context: None,
|
||||
channel: None,
|
||||
@@ -2117,15 +2113,9 @@ That's my plan."#;
|
||||
// ---- System prompt building tests (issue #565) ----
|
||||
|
||||
fn make_test_reasoning() -> Reasoning {
|
||||
use crate::config::SafetyConfig;
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::testing::StubLlm;
|
||||
let llm = Arc::new(StubLlm::new("test"));
|
||||
let safety = Arc::new(SafetyLayer::new(&SafetyConfig {
|
||||
max_output_length: 100_000,
|
||||
injection_check_enabled: false,
|
||||
}));
|
||||
Reasoning::new(llm, safety)
|
||||
Reasoning::new(llm)
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -43,7 +43,6 @@ use crate::error::ToolError as AgentToolError;
|
||||
use crate::llm::{
|
||||
ChatMessage, LlmProvider, Reasoning, ReasoningContext, RespondResult, ToolDefinition,
|
||||
};
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::tools::ToolRegistry;
|
||||
use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput};
|
||||
|
||||
@@ -251,29 +250,18 @@ pub trait SoftwareBuilder: Send + Sync {
|
||||
pub struct LlmSoftwareBuilder {
|
||||
config: BuilderConfig,
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
tools: Arc<ToolRegistry>,
|
||||
}
|
||||
|
||||
impl LlmSoftwareBuilder {
|
||||
/// Create a new LLM-based software builder.
|
||||
pub fn new(
|
||||
config: BuilderConfig,
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
tools: Arc<ToolRegistry>,
|
||||
) -> Self {
|
||||
pub fn new(config: BuilderConfig, llm: Arc<dyn LlmProvider>, tools: Arc<ToolRegistry>) -> Self {
|
||||
// Ensure build directory exists
|
||||
if let Err(e) = std::fs::create_dir_all(&config.build_dir) {
|
||||
tracing::warn!("Failed to create build directory: {}", e);
|
||||
}
|
||||
|
||||
Self {
|
||||
config,
|
||||
llm,
|
||||
safety,
|
||||
tools,
|
||||
}
|
||||
Self { config, llm, tools }
|
||||
}
|
||||
|
||||
/// Get the build tools available for the build loop.
|
||||
@@ -521,7 +509,7 @@ Create alongside the .wasm file to grant capabilities:
|
||||
let mut iteration = 0;
|
||||
|
||||
// Create reasoning engine
|
||||
let reasoning = Reasoning::new(self.llm.clone(), self.safety.clone());
|
||||
let reasoning = Reasoning::new(self.llm.clone());
|
||||
|
||||
// Build initial context
|
||||
let tool_defs = self.get_build_tools().await;
|
||||
@@ -822,7 +810,7 @@ Create alongside the .wasm file to grant capabilities:
|
||||
impl SoftwareBuilder for LlmSoftwareBuilder {
|
||||
async fn analyze(&self, description: &str) -> Result<BuildRequirement, AgentToolError> {
|
||||
// Use LLM to parse the description
|
||||
let reasoning = Reasoning::new(self.llm.clone(), self.safety.clone());
|
||||
let reasoning = Reasoning::new(self.llm.clone());
|
||||
|
||||
let prompt = format!(
|
||||
r#"Analyze this software requirement and extract structured information.
|
||||
|
||||
@@ -10,7 +10,6 @@ use crate::db::Database;
|
||||
use crate::extensions::ExtensionManager;
|
||||
use crate::llm::{LlmProvider, ToolDefinition};
|
||||
use crate::orchestrator::job_manager::ContainerJobManager;
|
||||
use crate::safety::SafetyLayer;
|
||||
use crate::secrets::SecretsStore;
|
||||
use crate::skills::catalog::SkillCatalog;
|
||||
use crate::skills::registry::SkillRegistry;
|
||||
@@ -485,17 +484,15 @@ impl ToolRegistry {
|
||||
pub async fn register_builder_tool(
|
||||
self: &Arc<Self>,
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
safety: Arc<SafetyLayer>,
|
||||
config: Option<BuilderConfig>,
|
||||
) {
|
||||
// First register dev tools needed by the builder
|
||||
self.register_dev_tools();
|
||||
|
||||
// Create the builder (arg order: config, llm, safety, tools)
|
||||
// Create the builder (arg order: config, llm, tools)
|
||||
let builder = Arc::new(LlmSoftwareBuilder::new(
|
||||
config.unwrap_or_default(),
|
||||
llm,
|
||||
safety,
|
||||
Arc::clone(self),
|
||||
));
|
||||
|
||||
|
||||
@@ -133,7 +133,7 @@ impl WorkerRuntime {
|
||||
.await?;
|
||||
|
||||
// Create reasoning engine
|
||||
let reasoning = Reasoning::new(self.llm.clone(), self.safety.clone());
|
||||
let reasoning = Reasoning::new(self.llm.clone());
|
||||
|
||||
// Build initial context
|
||||
let mut reason_ctx = ReasoningContext::new().with_job(&job.description);
|
||||
|
||||
@@ -113,79 +113,6 @@ pub fn chunk_document(content: &str, config: ChunkConfig) -> Vec<String> {
|
||||
chunks
|
||||
}
|
||||
|
||||
/// Split content by paragraphs first, then chunk.
|
||||
///
|
||||
/// This is better for preserving semantic boundaries.
|
||||
#[allow(dead_code)] // Alternative chunking strategy for paragraph-aware indexing
|
||||
pub fn chunk_by_paragraphs(content: &str, config: ChunkConfig) -> Vec<String> {
|
||||
if content.is_empty() {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
// Split by double newlines (paragraphs)
|
||||
let paragraphs: Vec<&str> = content
|
||||
.split("\n\n")
|
||||
.map(|p| p.trim())
|
||||
.filter(|p| !p.is_empty())
|
||||
.collect();
|
||||
|
||||
if paragraphs.is_empty() {
|
||||
return chunk_document(content, config);
|
||||
}
|
||||
|
||||
let mut chunks = Vec::new();
|
||||
let mut current_chunk = String::new();
|
||||
let mut current_word_count = 0;
|
||||
|
||||
for paragraph in paragraphs {
|
||||
let para_words = paragraph.split_whitespace().count();
|
||||
|
||||
// If this paragraph alone exceeds chunk size, chunk it separately
|
||||
if para_words > config.chunk_size {
|
||||
// Flush current chunk first
|
||||
if !current_chunk.is_empty() {
|
||||
chunks.push(current_chunk.trim().to_string());
|
||||
current_chunk = String::new();
|
||||
current_word_count = 0;
|
||||
}
|
||||
// Chunk the large paragraph
|
||||
let para_chunks = chunk_document(paragraph, config.clone());
|
||||
chunks.extend(para_chunks);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check if adding this paragraph would exceed chunk size
|
||||
if current_word_count + para_words > config.chunk_size {
|
||||
// Flush current chunk
|
||||
if !current_chunk.is_empty() {
|
||||
chunks.push(current_chunk.trim().to_string());
|
||||
}
|
||||
current_chunk = paragraph.to_string();
|
||||
current_word_count = para_words;
|
||||
} else {
|
||||
// Add paragraph to current chunk
|
||||
if !current_chunk.is_empty() {
|
||||
current_chunk.push_str("\n\n");
|
||||
}
|
||||
current_chunk.push_str(paragraph);
|
||||
current_word_count += para_words;
|
||||
}
|
||||
}
|
||||
|
||||
// Flush remaining content
|
||||
if !current_chunk.is_empty() {
|
||||
// If too small, merge with previous chunk if possible
|
||||
if current_word_count < config.min_chunk_size && !chunks.is_empty() {
|
||||
let last = chunks.pop().unwrap();
|
||||
chunks.push(format!("{}\n\n{}", last, current_chunk.trim()));
|
||||
} else {
|
||||
chunks.push(current_chunk.trim().to_string());
|
||||
}
|
||||
}
|
||||
|
||||
chunks
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -253,49 +180,6 @@ mod tests {
|
||||
assert_eq!(config.step_size(), 85);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_paragraph_chunking() {
|
||||
let config = ChunkConfig::default().with_chunk_size(20);
|
||||
|
||||
let content = "First paragraph with some words.\n\nSecond paragraph with different content.\n\nThird paragraph here.";
|
||||
let chunks = chunk_by_paragraphs(content, config);
|
||||
|
||||
// Should preserve paragraph boundaries
|
||||
assert!(!chunks.is_empty());
|
||||
for chunk in &chunks {
|
||||
// No chunk should start or end with \n\n
|
||||
assert!(!chunk.starts_with("\n"));
|
||||
assert!(!chunk.ends_with("\n"));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_large_paragraph_handling() {
|
||||
let config = ChunkConfig {
|
||||
chunk_size: 10,
|
||||
overlap_percent: 0.15,
|
||||
min_chunk_size: 3, // Low threshold for test
|
||||
};
|
||||
|
||||
// Create a paragraph with 30 words
|
||||
let large_para = (1..=30)
|
||||
.map(|i| format!("word{}", i))
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
let content = format!("Short intro.\n\n{}\n\nShort outro.", large_para);
|
||||
|
||||
let chunks = chunk_by_paragraphs(&content, config);
|
||||
|
||||
// Should have multiple chunks due to large paragraph
|
||||
// 30 words + 2 intro + 2 outro = 34 words, chunk_size=10
|
||||
// Expect at least 3 chunks
|
||||
assert!(
|
||||
chunks.len() >= 3,
|
||||
"Expected at least 3 chunks for 34 words with chunk_size=10, got {}",
|
||||
chunks.len()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_min_chunk_size_merging() {
|
||||
let config = ChunkConfig {
|
||||
|
||||
Reference in New Issue
Block a user