mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(harness): always emit a final answer before a turn ends (#4386)
This commit is contained in:
@@ -957,10 +957,11 @@ impl Agent {
|
||||
// cycle. Fold the extra call's usage into the turn accounting.
|
||||
let base = self.tool_dispatcher.to_provider_messages(&self.history);
|
||||
let (summary, summary_usage) = self
|
||||
.summarize_iteration_checkpoint(
|
||||
.summarize_turn_wrapup(
|
||||
&base,
|
||||
effective_model,
|
||||
outcome.model_calls as u32 + 1,
|
||||
super::super::turn_checkpoint::MAX_ITER_CHECKPOINT_INSTRUCTION,
|
||||
)
|
||||
.await;
|
||||
if let Some(u) = summary_usage {
|
||||
@@ -991,6 +992,61 @@ impl Agent {
|
||||
iteration: outcome.model_calls,
|
||||
},
|
||||
));
|
||||
} else if outcome.text.trim().is_empty() {
|
||||
// #4093: the loop ran tool calls (tool_calls > 0, so the branch
|
||||
// above did not fire) and then yielded a terminating response with
|
||||
// no final text — the turn did work but would otherwise end
|
||||
// silently, leaving the user with nothing. Enforce the
|
||||
// "must produce a final response" terminal step: re-prompt the
|
||||
// model (tools disabled) for a closing summary of what it did,
|
||||
// falling back to a deterministic summary of the tool calls so the
|
||||
// synthesized message is never itself empty. Fold the extra call's
|
||||
// usage into the turn accounting, exactly like the cap path above.
|
||||
let base = self.tool_dispatcher.to_provider_messages(&self.history);
|
||||
let (summary, summary_usage) = self
|
||||
.summarize_turn_wrapup(
|
||||
&base,
|
||||
effective_model,
|
||||
outcome.model_calls as u32 + 1,
|
||||
super::super::turn_checkpoint::FINAL_ANSWER_INSTRUCTION,
|
||||
)
|
||||
.await;
|
||||
if let Some(u) = summary_usage {
|
||||
input_tokens += u.input_tokens;
|
||||
output_tokens += u.output_tokens;
|
||||
cached_input_tokens += u.cached_input_tokens;
|
||||
charged_amount_usd += u.charged_amount_usd;
|
||||
}
|
||||
let final_answer = if summary.trim().is_empty() {
|
||||
super::super::turn_checkpoint::build_deterministic_final_summary(
|
||||
&tool_records_from_conversation(&outcome.conversation, &outcome.tool_outcomes),
|
||||
)
|
||||
} else {
|
||||
summary
|
||||
};
|
||||
log::info!(
|
||||
"[agent_loop] turn produced no final text after {} tool call(s); synthesized a closing summary ({} chars) — #4093",
|
||||
outcome.tool_calls,
|
||||
final_answer.chars().count()
|
||||
);
|
||||
// The empty terminal assistant response was already folded into
|
||||
// `self.history` via `outcome.conversation` above (an empty
|
||||
// `Chat(assistant(""))` — see `messages_to_conversation`). Drop that
|
||||
// blank turn before appending the synthesized answer so the
|
||||
// transcript and the next prompt don't carry a dangling empty
|
||||
// assistant message immediately before the real reply (Codex review).
|
||||
if matches!(
|
||||
self.history.last(),
|
||||
Some(ConversationMessage::Chat(msg))
|
||||
if msg.role == "assistant" && msg.content.trim().is_empty()
|
||||
) {
|
||||
self.history.pop();
|
||||
}
|
||||
self.history
|
||||
.push(ConversationMessage::Chat(ChatMessage::assistant(
|
||||
final_answer.clone(),
|
||||
)));
|
||||
final_answer
|
||||
} else {
|
||||
outcome.text.clone()
|
||||
};
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
//! Session persistence: transcript loading, checkpointing, and background tasks.
|
||||
|
||||
use super::super::transcript;
|
||||
use super::super::turn_checkpoint::MAX_ITER_CHECKPOINT_INSTRUCTION;
|
||||
use super::super::types::Agent;
|
||||
use crate::openhuman::agent::harness;
|
||||
use crate::openhuman::agent::progress::AgentProgress;
|
||||
@@ -63,27 +62,30 @@ impl Agent {
|
||||
}
|
||||
}
|
||||
|
||||
/// Ask the provider for a resumable checkpoint summary when a turn
|
||||
/// hits the tool-call iteration cap, with native tools **disabled** so
|
||||
/// the model returns prose rather than another tool call. Streams text
|
||||
/// deltas to the progress sink (when attached) so the checkpoint
|
||||
/// Ask the provider for a short wrap-up message with native tools
|
||||
/// **disabled** so the model returns prose rather than another tool call.
|
||||
/// Streams text deltas to the progress sink (when attached) so the summary
|
||||
/// appears in the UI like any other reply.
|
||||
///
|
||||
/// `instruction` is the synthetic user turn that steers the wrap-up — the
|
||||
/// tool-call-cap checkpoint (`MAX_ITER_CHECKPOINT_INSTRUCTION`) or the
|
||||
/// no-final-answer close (`FINAL_ANSWER_INSTRUCTION`, issue #4093).
|
||||
///
|
||||
/// Returns the summary text (empty when the provider call fails or
|
||||
/// yields nothing — the caller then falls back to
|
||||
/// [`build_deterministic_checkpoint`] so the thread is never left on an
|
||||
/// unterminated tool cycle, bug-report-2026-05-26 A1) **paired with the
|
||||
/// provider usage** for this extra call, so the caller can fold it into
|
||||
/// the turn's cumulative token/cost accounting instead of silently
|
||||
/// dropping it.
|
||||
pub(super) async fn summarize_iteration_checkpoint(
|
||||
/// yields nothing — the caller then falls back to a deterministic builder
|
||||
/// so the turn is never left without a well-formed assistant message,
|
||||
/// bug-report-2026-05-26 A1 / issue #4093) **paired with the provider
|
||||
/// usage** for this extra call, so the caller can fold it into the turn's
|
||||
/// cumulative token/cost accounting instead of silently dropping it.
|
||||
pub(super) async fn summarize_turn_wrapup(
|
||||
&self,
|
||||
base_messages: &[ChatMessage],
|
||||
effective_model: &str,
|
||||
iteration_for_stream: u32,
|
||||
instruction: &str,
|
||||
) -> (String, Option<UsageInfo>) {
|
||||
let mut messages = base_messages.to_vec();
|
||||
messages.push(ChatMessage::user(MAX_ITER_CHECKPOINT_INSTRUCTION));
|
||||
messages.push(ChatMessage::user(instruction));
|
||||
|
||||
// Mirror the main loop's streaming sink so the checkpoint renders
|
||||
// incrementally. Only text deltas are relevant here (tools are
|
||||
|
||||
@@ -74,3 +74,32 @@ pub(super) fn build_deterministic_checkpoint(
|
||||
);
|
||||
out
|
||||
}
|
||||
|
||||
/// Instruction appended (as a synthetic user turn) when a turn finished its
|
||||
/// tool work but the model produced **no final answer** — it yielded a
|
||||
/// terminating response with empty text after running tools (issue #4093).
|
||||
/// Native tools are disabled for this call so the model wraps up in prose
|
||||
/// instead of requesting more tools.
|
||||
pub(super) const FINAL_ANSWER_INSTRUCTION: &str = "\
|
||||
You have finished using tools for this turn but have not yet written a reply to the user. \
|
||||
Do not call any more tools. Write a short, self-contained final message that summarises what you did and \
|
||||
what you found or accomplished, grounded in the tool results above. If nothing conclusive resulted, say so plainly.";
|
||||
|
||||
/// Build a deterministic final answer from this turn's tool-call records.
|
||||
/// Used as the guaranteed non-empty fallback when a turn ran tools but the
|
||||
/// model produced no closing message and the re-prompt for one also came
|
||||
/// back empty — so a turn that did work can never end silently (issue #4093).
|
||||
/// Distinct from [`build_deterministic_checkpoint`]: the turn did NOT hit the
|
||||
/// iteration cap, so this reads as a completed summary, not a paused one.
|
||||
pub(super) fn build_deterministic_final_summary(records: &[ToolCallRecord]) -> String {
|
||||
if records.is_empty() {
|
||||
return "I finished this turn but produced no result to report.".to_string();
|
||||
}
|
||||
let mut out = String::from("Here's a summary of what I did this turn:\n\n");
|
||||
for r in records {
|
||||
let status = if r.success { "ok" } else { "failed" };
|
||||
out.push_str(&format!("- `{}` — {}\n", r.name, status));
|
||||
}
|
||||
out.push_str("\nLet me know if you'd like me to go further.");
|
||||
out
|
||||
}
|
||||
|
||||
@@ -1038,6 +1038,165 @@ async fn turn_checkpoint_falls_back_to_deterministic_summary_when_model_summary_
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_synthesizes_final_answer_when_tool_turn_yields_no_text() {
|
||||
// #4093: the model runs a tool and then yields a terminating response with
|
||||
// NO text and NO further tool calls — the turn did work but would end
|
||||
// silently. Because the cap was not hit, this is not a checkpoint case; the
|
||||
// harness must enforce the "must produce a final response" terminal step by
|
||||
// re-prompting the model (tools disabled) for a closing summary and
|
||||
// returning that instead of a blank reply.
|
||||
let provider: Arc<dyn Provider> = Arc::new(SequenceProvider {
|
||||
responses: AsyncMutex::new(vec![
|
||||
// Tool iteration (well under the cap).
|
||||
Ok(ChatResponse {
|
||||
text: Some("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
reasoning_content: None,
|
||||
}),
|
||||
// Terminal response with no text and no tool calls — the silent end.
|
||||
Ok(ChatResponse {
|
||||
text: Some(String::new()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
reasoning_content: None,
|
||||
}),
|
||||
// The harness's forced final-answer re-prompt (tools disabled).
|
||||
Ok(ChatResponse {
|
||||
text: Some("All done — I ran echo and it succeeded.".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
reasoning_content: None,
|
||||
}),
|
||||
]),
|
||||
requests: AsyncMutex::new(Vec::new()),
|
||||
});
|
||||
let mut agent = make_agent_with_builder(
|
||||
provider,
|
||||
vec![Box::new(EchoTool)],
|
||||
Box::new(FixedMemoryLoader {
|
||||
context: String::new(),
|
||||
}),
|
||||
vec![],
|
||||
crate::openhuman::config::AgentConfig {
|
||||
max_tool_iterations: 5,
|
||||
..crate::openhuman::config::AgentConfig::default()
|
||||
},
|
||||
crate::openhuman::config::ContextConfig::default(),
|
||||
);
|
||||
|
||||
let reply = agent
|
||||
.turn("hello")
|
||||
.await
|
||||
.expect("a tool-only turn with no final text should synthesize one, not error");
|
||||
assert!(
|
||||
!reply.trim().is_empty(),
|
||||
"turn must never end with an empty final message (#4093), got: {reply:?}"
|
||||
);
|
||||
assert!(
|
||||
reply.contains("I ran echo"),
|
||||
"the synthesized final message should be the model's wrap-up, got: {reply}"
|
||||
);
|
||||
// The transcript must end on the assistant's final message, not a dangling
|
||||
// tool cycle.
|
||||
assert!(
|
||||
matches!(
|
||||
agent.history.last(),
|
||||
Some(ConversationMessage::Chat(msg))
|
||||
if msg.role == "assistant" && !msg.content.trim().is_empty()
|
||||
),
|
||||
"history should end on a non-empty assistant message, got: {:?}",
|
||||
agent.history.last()
|
||||
);
|
||||
// ...and the blank terminal assistant response (folded in from the turn
|
||||
// outcome) must have been dropped, not left dangling before the synthesized
|
||||
// answer (Codex review).
|
||||
assert!(
|
||||
!agent.history.iter().any(|m| matches!(
|
||||
m,
|
||||
ConversationMessage::Chat(msg)
|
||||
if msg.role == "assistant" && msg.content.trim().is_empty()
|
||||
)),
|
||||
"no blank assistant turn should remain in history, got: {:?}",
|
||||
agent.history
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_final_answer_falls_back_to_deterministic_summary_when_reprompt_empty() {
|
||||
// #4093 safety net: the tool ran, the model yielded no final text, and the
|
||||
// forced final-answer re-prompt ALSO came back empty. The harness must fall
|
||||
// back to a deterministic summary of the tool calls so the turn is never
|
||||
// blank — and, unlike the cap path, it must read as a completed summary
|
||||
// rather than a paused "tool-call limit" checkpoint.
|
||||
let provider: Arc<dyn Provider> = Arc::new(SequenceProvider {
|
||||
responses: AsyncMutex::new(vec![
|
||||
Ok(ChatResponse {
|
||||
text: Some("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".into()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
reasoning_content: None,
|
||||
}),
|
||||
Ok(ChatResponse {
|
||||
text: Some(String::new()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
reasoning_content: None,
|
||||
}),
|
||||
// Re-prompt for a final answer also returns empty.
|
||||
Ok(ChatResponse {
|
||||
text: Some(String::new()),
|
||||
tool_calls: vec![],
|
||||
usage: None,
|
||||
reasoning_content: None,
|
||||
}),
|
||||
]),
|
||||
requests: AsyncMutex::new(Vec::new()),
|
||||
});
|
||||
let mut agent = make_agent_with_builder(
|
||||
provider,
|
||||
vec![Box::new(EchoTool)],
|
||||
Box::new(FixedMemoryLoader {
|
||||
context: String::new(),
|
||||
}),
|
||||
vec![],
|
||||
crate::openhuman::config::AgentConfig {
|
||||
max_tool_iterations: 5,
|
||||
..crate::openhuman::config::AgentConfig::default()
|
||||
},
|
||||
crate::openhuman::config::ContextConfig::default(),
|
||||
);
|
||||
|
||||
let reply = agent
|
||||
.turn("hello")
|
||||
.await
|
||||
.expect("empty final re-prompt should fall back deterministically, not error");
|
||||
assert!(
|
||||
!reply.trim().is_empty(),
|
||||
"deterministic fallback must be non-empty (#4093), got: {reply:?}"
|
||||
);
|
||||
assert!(
|
||||
reply.contains("echo"),
|
||||
"fallback should list the tool that ran, got: {reply}"
|
||||
);
|
||||
assert!(
|
||||
!reply.contains("tool-call limit"),
|
||||
"a non-capped turn must not claim it hit the tool-call limit, got: {reply}"
|
||||
);
|
||||
// The blank terminal assistant response must not linger before the
|
||||
// deterministic summary (Codex review).
|
||||
assert!(
|
||||
!agent.history.iter().any(|m| matches!(
|
||||
m,
|
||||
ConversationMessage::Chat(msg)
|
||||
if msg.role == "assistant" && msg.content.trim().is_empty()
|
||||
)),
|
||||
"no blank assistant turn should remain in history, got: {:?}",
|
||||
agent.history
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_checkpoint_usage_is_folded_into_transcript_accounting() {
|
||||
// The extra checkpoint provider call costs tokens; those must land in
|
||||
|
||||
Reference in New Issue
Block a user