fix: classify direct report artifacts by status
Treat direct skill report-artifact payloads as task outcomes so partial and empty reports stay successful while blocked and error statuses fail explicitly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -10,13 +10,19 @@ use crate::compat::runtime::CompatTaskContext;
|
||||
use crate::config::SgClawSettings;
|
||||
use crate::pipe::{BrowserPipeTool, PipeError, Transport};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct DirectSubmitOutcome {
|
||||
pub success: bool,
|
||||
pub summary: String,
|
||||
}
|
||||
|
||||
pub fn execute_direct_submit_skill<T: Transport + 'static>(
|
||||
browser_tool: BrowserPipeTool<T>,
|
||||
instruction: &str,
|
||||
task_context: &CompatTaskContext,
|
||||
workspace_root: &Path,
|
||||
settings: &SgClawSettings,
|
||||
) -> Result<String, PipeError> {
|
||||
) -> Result<DirectSubmitOutcome, PipeError> {
|
||||
let configured_tool = settings
|
||||
.direct_submit_skill
|
||||
.as_deref()
|
||||
@@ -80,7 +86,7 @@ pub fn execute_direct_submit_skill<T: Transport + 'static>(
|
||||
.map_err(|err| PipeError::Protocol(err.to_string()))?;
|
||||
|
||||
if result.success {
|
||||
Ok(result.output)
|
||||
Ok(interpret_direct_submit_output(&result.output))
|
||||
} else {
|
||||
Err(PipeError::Protocol(
|
||||
result
|
||||
@@ -90,6 +96,104 @@ pub fn execute_direct_submit_skill<T: Transport + 'static>(
|
||||
}
|
||||
}
|
||||
|
||||
fn interpret_direct_submit_output(output: &str) -> DirectSubmitOutcome {
|
||||
let Some(payload) = serde_json::from_str::<Value>(output).ok() else {
|
||||
return DirectSubmitOutcome {
|
||||
success: true,
|
||||
summary: output.to_string(),
|
||||
};
|
||||
};
|
||||
|
||||
let Some(artifact) = payload.as_object() else {
|
||||
return DirectSubmitOutcome {
|
||||
success: true,
|
||||
summary: output.to_string(),
|
||||
};
|
||||
};
|
||||
|
||||
if artifact.get("type").and_then(Value::as_str) != Some("report-artifact") {
|
||||
return DirectSubmitOutcome {
|
||||
success: true,
|
||||
summary: output.to_string(),
|
||||
};
|
||||
}
|
||||
|
||||
let status = artifact
|
||||
.get("status")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("ok");
|
||||
let success = matches!(status, "ok" | "partial" | "empty");
|
||||
let report_name = artifact
|
||||
.get("report_name")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("report-artifact");
|
||||
let period = artifact
|
||||
.get("period")
|
||||
.and_then(Value::as_str)
|
||||
.unwrap_or("");
|
||||
let detail_rows = count_rows(artifact.get("counts"), artifact.get("rows"), "detail_rows");
|
||||
let summary_rows = count_summary_rows(artifact.get("counts"), artifact.get("sections"));
|
||||
let partial_reasons = artifact
|
||||
.get("partial_reasons")
|
||||
.and_then(Value::as_array)
|
||||
.map(|reasons| {
|
||||
reasons
|
||||
.iter()
|
||||
.filter_map(Value::as_str)
|
||||
.filter(|value| !value.trim().is_empty())
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
|
||||
let mut parts = vec![report_name.to_string()];
|
||||
if !period.trim().is_empty() {
|
||||
parts.push(period.to_string());
|
||||
}
|
||||
parts.push(format!("status={status}"));
|
||||
parts.push(format!("detail_rows={detail_rows}"));
|
||||
parts.push(format!("summary_rows={summary_rows}"));
|
||||
if !partial_reasons.is_empty() {
|
||||
parts.push(format!("partial_reasons={}", partial_reasons.join(",")));
|
||||
}
|
||||
|
||||
DirectSubmitOutcome {
|
||||
success,
|
||||
summary: parts.join(" "),
|
||||
}
|
||||
}
|
||||
|
||||
fn count_rows(counts: Option<&Value>, rows: Option<&Value>, key: &str) -> usize {
|
||||
counts
|
||||
.and_then(Value::as_object)
|
||||
.and_then(|counts| counts.get(key))
|
||||
.and_then(Value::as_u64)
|
||||
.map(|count| count as usize)
|
||||
.or_else(|| rows.and_then(Value::as_array).map(Vec::len))
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
fn count_summary_rows(counts: Option<&Value>, sections: Option<&Value>) -> usize {
|
||||
counts
|
||||
.and_then(Value::as_object)
|
||||
.and_then(|counts| counts.get("summary_rows"))
|
||||
.and_then(Value::as_u64)
|
||||
.map(|count| count as usize)
|
||||
.or_else(|| {
|
||||
sections
|
||||
.and_then(Value::as_array)
|
||||
.and_then(|sections| {
|
||||
sections.iter().find_map(|section| {
|
||||
section
|
||||
.as_object()
|
||||
.and_then(|section| section.get("rows"))
|
||||
.and_then(Value::as_array)
|
||||
.map(Vec::len)
|
||||
})
|
||||
})
|
||||
})
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
fn parse_configured_tool_name(configured_tool: &str) -> Result<(&str, &str), PipeError> {
|
||||
let (skill_name, tool_name) = configured_tool.split_once('.').ok_or_else(|| {
|
||||
PipeError::Protocol(format!(
|
||||
@@ -162,7 +266,11 @@ fn is_year_month(candidate: &str) -> bool {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{derive_period, is_year_month, parse_configured_tool_name};
|
||||
use super::{
|
||||
count_rows, count_summary_rows, derive_period, interpret_direct_submit_output,
|
||||
is_year_month, parse_configured_tool_name,
|
||||
};
|
||||
use serde_json::json;
|
||||
|
||||
#[test]
|
||||
fn parse_configured_tool_name_requires_skill_and_tool() {
|
||||
@@ -186,4 +294,46 @@ mod tests {
|
||||
assert!(!is_year_month("2026-00"));
|
||||
assert!(!is_year_month("2026-13"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interpret_direct_submit_output_maps_report_artifact_statuses() {
|
||||
let partial = interpret_direct_submit_output(
|
||||
&json!({
|
||||
"type": "report-artifact",
|
||||
"report_name": "fault-details-report",
|
||||
"period": "2026-03",
|
||||
"counts": { "detail_rows": 1, "summary_rows": 1 },
|
||||
"status": "partial",
|
||||
"partial_reasons": ["report_log_failed"]
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
assert!(partial.success);
|
||||
assert!(partial.summary.contains("status=partial"));
|
||||
assert!(partial.summary.contains("report_log_failed"));
|
||||
|
||||
let blocked = interpret_direct_submit_output(
|
||||
&json!({
|
||||
"type": "report-artifact",
|
||||
"report_name": "fault-details-report",
|
||||
"status": "blocked",
|
||||
"partial_reasons": ["selected_range_unavailable"]
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
assert!(!blocked.success);
|
||||
assert!(blocked.summary.contains("status=blocked"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn row_count_helpers_fall_back_to_payload_shapes() {
|
||||
assert_eq!(
|
||||
count_rows(None, Some(&json!([{ "qxdbh": "QX-1" }, { "qxdbh": "QX-2" }])), "detail_rows"),
|
||||
2
|
||||
);
|
||||
assert_eq!(
|
||||
count_summary_rows(None, Some(&json!([{ "name": "summary-sheet", "rows": [{ "index": 1 }] }]))),
|
||||
1
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user