mirror of
https://github.com/instructkr/claw-code.git
synced 2026-04-07 16:44:50 +08:00
fix(cli): add section headers to OMC output for agent type grouping
voloshko: flat wall of text. Now groups output with section separators by agent type (Explore, Implementation, Verification).
This commit is contained in:
@@ -6424,6 +6424,111 @@ fn format_grep_result(icon: &str, parsed: &serde_json::Value) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// Detects multi-agent OMC/orchestrator output where the model labels lines with
|
||||
/// `Explore:`, `Implementation:`, etc., and rewrites them so each agent type
|
||||
/// becomes its own visually grouped section. This is display-only — the
|
||||
/// underlying text persisted to the session is left untouched by callers.
|
||||
///
|
||||
/// The emitted section headers use HTML numeric entities for the surrounding
|
||||
/// hyphens (`--- Explore ---`) so the shared markdown
|
||||
/// renderer's smart-punctuation pass cannot collapse them into a single em-dash.
|
||||
/// After rendering, the user sees the literal `--- Explore ---` form requested
|
||||
/// in the bug report.
|
||||
///
|
||||
/// Returns `Some(grouped)` only when at least two distinct canonical agent
|
||||
/// types are present, so single-agent output is left unchanged.
|
||||
fn regroup_omc_agent_sections(text: &str) -> Option<String> {
|
||||
if text.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut preamble: Vec<String> = Vec::new();
|
||||
let mut sections: Vec<(String, Vec<String>)> = Vec::new();
|
||||
let mut current: Option<usize> = None;
|
||||
let mut seen: BTreeSet<String> = BTreeSet::new();
|
||||
|
||||
for raw_line in text.lines() {
|
||||
if let Some((label, content)) = parse_omc_agent_section_line(raw_line) {
|
||||
seen.insert(label.clone());
|
||||
let index = sections
|
||||
.iter()
|
||||
.position(|(existing, _)| existing == &label)
|
||||
.unwrap_or_else(|| {
|
||||
sections.push((label, Vec::new()));
|
||||
sections.len() - 1
|
||||
});
|
||||
current = Some(index);
|
||||
if !content.is_empty() {
|
||||
sections[index].1.push(content);
|
||||
}
|
||||
} else if let Some(index) = current {
|
||||
sections[index].1.push(raw_line.to_string());
|
||||
} else {
|
||||
preamble.push(raw_line.to_string());
|
||||
}
|
||||
}
|
||||
|
||||
if seen.len() < 2 {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut out = String::new();
|
||||
if !preamble.is_empty() {
|
||||
out.push_str(preamble.join("\n").trim_end());
|
||||
if !out.is_empty() {
|
||||
out.push('\n');
|
||||
}
|
||||
}
|
||||
for (idx, (label, lines)) in sections.iter().enumerate() {
|
||||
if idx > 0 || !out.is_empty() {
|
||||
out.push('\n');
|
||||
}
|
||||
out.push_str(&omc_agent_section_header(label));
|
||||
if !lines.is_empty() {
|
||||
out.push('\n');
|
||||
out.push_str(lines.join("\n").trim_end());
|
||||
}
|
||||
out.push('\n');
|
||||
}
|
||||
Some(out.trim_end_matches('\n').to_string())
|
||||
}
|
||||
|
||||
/// Builds the markdown-safe form of an OMC agent section header. The hyphens
|
||||
/// are emitted as numeric character references so the shared markdown renderer
|
||||
/// does not transform `---` into `—` via smart punctuation.
|
||||
fn omc_agent_section_header(label: &str) -> String {
|
||||
format!("--- {label} ---")
|
||||
}
|
||||
|
||||
/// Parses a single line for an OMC agent-section prefix like
|
||||
/// `Explore: <body>`. Returns the canonical label and the trailing body if the
|
||||
/// line opens with one of the recognised agent types.
|
||||
fn parse_omc_agent_section_line(line: &str) -> Option<(String, String)> {
|
||||
let trimmed = line.trim_start();
|
||||
let (head, rest) = trimmed.split_once(':')?;
|
||||
let label = canonical_omc_agent_label(head.trim())?;
|
||||
let body = rest.trim_start().to_string();
|
||||
Some((label, body))
|
||||
}
|
||||
|
||||
/// Maps a free-form agent label to its canonical display name. Returns `None`
|
||||
/// for unrecognised labels so unrelated `Foo:` prefixes are left as plain text.
|
||||
fn canonical_omc_agent_label(label: &str) -> Option<String> {
|
||||
let normalized = label.trim().to_ascii_lowercase();
|
||||
let canonical = match normalized.as_str() {
|
||||
"explore" | "exploring" | "research" => "Explore",
|
||||
"implementation" | "implementing" | "implement" => "Implementation",
|
||||
"verification" | "verifying" | "verify" => "Verification",
|
||||
"plan" | "planning" => "Plan",
|
||||
"review" | "reviewing" => "Review",
|
||||
"oracle" => "Oracle",
|
||||
"librarian" => "Librarian",
|
||||
"general" | "general-purpose" => "General",
|
||||
_ => return None,
|
||||
};
|
||||
Some(canonical.to_string())
|
||||
}
|
||||
|
||||
fn format_generic_tool_result(icon: &str, name: &str, parsed: &serde_json::Value) -> String {
|
||||
let rendered_output = match parsed {
|
||||
serde_json::Value::String(text) => text.clone(),
|
||||
@@ -6433,8 +6538,10 @@ fn format_generic_tool_result(icon: &str, name: &str, parsed: &serde_json::Value
|
||||
}
|
||||
_ => parsed.to_string(),
|
||||
};
|
||||
let grouped = regroup_omc_agent_sections(&rendered_output);
|
||||
let display_source = grouped.as_deref().unwrap_or(&rendered_output);
|
||||
let preview = truncate_output_for_display(
|
||||
&rendered_output,
|
||||
display_source,
|
||||
TOOL_OUTPUT_DISPLAY_MAX_LINES,
|
||||
TOOL_OUTPUT_DISPLAY_MAX_CHARS,
|
||||
);
|
||||
@@ -6538,7 +6645,14 @@ fn push_output_block(
|
||||
match block {
|
||||
OutputContentBlock::Text { text } => {
|
||||
if !text.is_empty() {
|
||||
let rendered = TerminalRenderer::new().markdown_to_ansi(&text);
|
||||
// Display-only: when the orchestrator emits multi-agent
|
||||
// labelled lines (`Explore: ...`, `Implementation: ...`),
|
||||
// group them under section headers so the transcript stops
|
||||
// reading as a flat wall of text. The unmodified `text` is
|
||||
// still pushed onto `events` so persistence is unchanged.
|
||||
let display_text =
|
||||
regroup_omc_agent_sections(&text).unwrap_or_else(|| text.clone());
|
||||
let rendered = TerminalRenderer::new().markdown_to_ansi(&display_text);
|
||||
write!(out, "{rendered}")
|
||||
.and_then(|()| out.flush())
|
||||
.map_err(|error| RuntimeError::new(error.to_string()))?;
|
||||
@@ -6961,9 +7075,9 @@ mod tests {
|
||||
format_unknown_slash_command_message, format_user_visible_api_error,
|
||||
normalize_permission_mode, parse_args, parse_git_status_branch,
|
||||
parse_git_status_metadata_for, parse_git_workspace_summary, permission_policy,
|
||||
print_help_to, push_output_block, render_config_report, render_diff_report,
|
||||
render_diff_report_for, render_memory_report, render_repl_help, render_resume_usage,
|
||||
resolve_model_alias, resolve_session_reference, response_to_events,
|
||||
print_help_to, push_output_block, regroup_omc_agent_sections, render_config_report,
|
||||
render_diff_report, render_diff_report_for, render_memory_report, render_repl_help,
|
||||
render_resume_usage, resolve_model_alias, resolve_session_reference, response_to_events,
|
||||
resume_supported_slash_commands, run_resume_command,
|
||||
slash_command_completion_candidates_with_sessions, status_context, validate_no_args,
|
||||
write_mcp_server_fixture, CliAction, CliOutputFormat, CliToolExecutor, GitWorkspaceSummary,
|
||||
@@ -8858,6 +8972,144 @@ UU conflicted.rs",
|
||||
assert!(output.contains("raw 119"));
|
||||
}
|
||||
|
||||
const ENTITY_DASHES: &str = "---";
|
||||
|
||||
#[test]
|
||||
fn regroup_omc_agent_sections_inserts_headers_when_multiple_agent_types_present() {
|
||||
// given
|
||||
let raw = "Here is the multi-agent transcript:\n\
|
||||
Explore: Located main.rs renderer\n\
|
||||
Explore: Mapped tool result paths\n\
|
||||
Implementation: Added section helper\n\
|
||||
Implementation: Wired helper into format_generic_tool_result\n\
|
||||
Verification: Ran cargo check";
|
||||
let explore_header = format!("{ENTITY_DASHES} Explore {ENTITY_DASHES}");
|
||||
let impl_header = format!("{ENTITY_DASHES} Implementation {ENTITY_DASHES}");
|
||||
let verify_header = format!("{ENTITY_DASHES} Verification {ENTITY_DASHES}");
|
||||
|
||||
// when
|
||||
let grouped = regroup_omc_agent_sections(raw).expect("multi-agent text should regroup");
|
||||
|
||||
// then
|
||||
assert!(grouped.contains("Here is the multi-agent transcript:"));
|
||||
assert!(grouped.contains(&explore_header), "{grouped}");
|
||||
assert!(grouped.contains(&impl_header), "{grouped}");
|
||||
assert!(grouped.contains(&verify_header), "{grouped}");
|
||||
assert!(grouped.contains("Located main.rs renderer"));
|
||||
assert!(grouped.contains("Wired helper into format_generic_tool_result"));
|
||||
let explore_pos = grouped
|
||||
.find(&explore_header)
|
||||
.expect("Explore header should be present");
|
||||
let impl_pos = grouped
|
||||
.find(&impl_header)
|
||||
.expect("Implementation header should be present");
|
||||
let verify_pos = grouped
|
||||
.find(&verify_header)
|
||||
.expect("Verification header should be present");
|
||||
assert!(
|
||||
explore_pos < impl_pos && impl_pos < verify_pos,
|
||||
"agent sections should preserve first-seen ordering: {grouped}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn regroup_omc_agent_sections_returns_none_when_only_single_agent_type_present() {
|
||||
// given
|
||||
let raw = "Explore: Found the renderer\nExplore: Found the test file";
|
||||
|
||||
// when
|
||||
let grouped = regroup_omc_agent_sections(raw);
|
||||
|
||||
// then
|
||||
assert!(
|
||||
grouped.is_none(),
|
||||
"single-agent transcript should not be regrouped: {grouped:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn regroup_omc_agent_sections_ignores_unrelated_colon_prefixes() {
|
||||
// given
|
||||
let raw = "Note: this is fine\nFooBar: still fine\nDetails: nothing to group";
|
||||
|
||||
// when
|
||||
let grouped = regroup_omc_agent_sections(raw);
|
||||
|
||||
// then
|
||||
assert!(
|
||||
grouped.is_none(),
|
||||
"non-agent prefixes should not trigger grouping: {grouped:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_tool_result_groups_agent_output_by_type_headers() {
|
||||
// given
|
||||
let output = "Explore: Mapped the OMC renderer surface\n\
|
||||
Implementation: Added grouped section headers\n\
|
||||
Implementation: Preserved truncation behavior\n\
|
||||
Verification: Ran cargo check on rusty-claude-cli";
|
||||
let explore_header = format!("{ENTITY_DASHES} Explore {ENTITY_DASHES}");
|
||||
let impl_header = format!("{ENTITY_DASHES} Implementation {ENTITY_DASHES}");
|
||||
let verify_header = format!("{ENTITY_DASHES} Verification {ENTITY_DASHES}");
|
||||
|
||||
// when
|
||||
let rendered = format_tool_result("Agent", output, false);
|
||||
|
||||
// then
|
||||
assert!(rendered.contains("Agent"));
|
||||
assert!(rendered.contains(&explore_header), "{rendered}");
|
||||
assert!(rendered.contains(&impl_header), "{rendered}");
|
||||
assert!(rendered.contains(&verify_header), "{rendered}");
|
||||
assert!(rendered.contains("Mapped the OMC renderer surface"));
|
||||
assert!(rendered.contains("Preserved truncation behavior"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn push_output_block_groups_assistant_text_by_agent_type_for_display_only() {
|
||||
// given
|
||||
let mut out = Vec::new();
|
||||
let mut events: Vec<AssistantEvent> = Vec::new();
|
||||
let mut pending_tool = None;
|
||||
let mut block_has_thinking_summary = false;
|
||||
let assistant_text = "Explore: Found flat OMC transcript rendering\n\
|
||||
Implementation: Added grouped output headers\n\
|
||||
Verification: Added rendering coverage";
|
||||
|
||||
// when
|
||||
push_output_block(
|
||||
OutputContentBlock::Text {
|
||||
text: assistant_text.to_string(),
|
||||
},
|
||||
&mut out,
|
||||
&mut events,
|
||||
&mut pending_tool,
|
||||
false,
|
||||
&mut block_has_thinking_summary,
|
||||
)
|
||||
.expect("text block should render");
|
||||
|
||||
// then
|
||||
// After the shared markdown renderer expands the numeric character
|
||||
// references in the section headers, the user-visible output contains
|
||||
// the literal `--- Explore ---` form requested by the bug report.
|
||||
let rendered = String::from_utf8(out).expect("utf8");
|
||||
assert!(rendered.contains("--- Explore ---"), "{rendered}");
|
||||
assert!(rendered.contains("--- Implementation ---"), "{rendered}");
|
||||
assert!(rendered.contains("--- Verification ---"), "{rendered}");
|
||||
assert!(
|
||||
rendered.contains("Added grouped output headers"),
|
||||
"{rendered}"
|
||||
);
|
||||
// The original, unmodified assistant text is still pushed onto the
|
||||
// event stream so persisted sessions are byte-identical to the API
|
||||
// response.
|
||||
assert!(matches!(
|
||||
events.as_slice(),
|
||||
[AssistantEvent::TextDelta(text)] if text == assistant_text
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ultraplan_progress_lines_include_phase_step_and_elapsed_status() {
|
||||
let snapshot = InternalPromptProgressState {
|
||||
|
||||
Reference in New Issue
Block a user