Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/forge_api/src/forge_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ impl<
}

async fn update_config(&self, ops: Vec<forge_domain::ConfigOperation>) -> anyhow::Result<()> {
// Determine whether any op affects provider/model resolution before writing,
// so we can invalidate the agent cache afterwards.
// Determine whether any op affects provider/model resolution before
// writing, so we can invalidate the agent cache afterwards.
let needs_agent_reload = ops
.iter()
.any(|op| matches!(op, forge_domain::ConfigOperation::SetSessionConfig(_)));
Expand Down
12 changes: 7 additions & 5 deletions crates/forge_app/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,9 @@ impl AgentExt for Agent {

// Apply workflow compact configuration to agents
if let Some(ref workflow_compact) = config.compact {
// Convert forge_config::Compact to forge_domain::Compact, then merge.
// Agent settings take priority over workflow settings.
// Convert forge_config::Compact to forge_domain::Compact, then
// merge. Agent settings take priority over workflow
// settings.
let mut merged_compact = Compact {
retention_window: workflow_compact.retention_window,
eviction_window: workflow_compact.eviction_window.value(),
Expand Down Expand Up @@ -169,7 +170,8 @@ impl AgentExt for Agent {
exclude: config_reasoning.exclude,
enabled: config_reasoning.enabled,
};
// Start from the agent's own settings and fill unset fields from config.
// Start from the agent's own settings and fill unset fields from
// config.
let mut merged = agent.reasoning.clone().unwrap_or_default();
merged.merge(config_as_domain);
// If the config explicitly disables reasoning, honour that override
Expand Down Expand Up @@ -302,8 +304,8 @@ mod tests {

// CURRENT BEHAVIOR: Due to merge order (workflow_compact merged with
// agent.compact), agent's retention_window=0 overwrites workflow's 10
// This is the documented behavior: "Agent settings take priority over workflow
// settings"
// This is the documented behavior: "Agent settings take priority over
// workflow settings"

// Agent default has retention_window=0, which overwrites workflow's 10
assert_eq!(
Expand Down
5 changes: 3 additions & 2 deletions crates/forge_app/src/agent_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> AgentEx
.await?
.ok_or(Error::ConversationNotFound { id: conversation_id })?
} else {
// Create context with agent initiator since it's spawned by a parent agent
// This is crucial for GitHub Copilot billing optimization
// Create context with agent initiator since it's spawned by a
// parent agent This is crucial for GitHub Copilot
// billing optimization
let context = forge_domain::Context::default().initiator("agent".to_string());
let conversation = Conversation::generate()
.title(task.clone())
Expand Down
3 changes: 2 additions & 1 deletion crates/forge_app/src/agent_provider_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ where
// Load all agent definitions and find the one we need

if let Some(agent) = self.0.get_agent(&agent_id).await? {
// If the agent definition has a provider, use it; otherwise use default
// If the agent definition has a provider, use it; otherwise use
// default
agent.provider
} else {
// TODO: Needs review, should we throw an err here?
Expand Down
10 changes: 6 additions & 4 deletions crates/forge_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeAp
let tracing_handler = TracingHandler::new();
let title_handler = TitleGenerationHandler::new(services.clone());

// Build the on_end hook, conditionally adding PendingTodosHandler based on
// config
// Build the on_end hook, conditionally adding PendingTodosHandler based
// on config
let on_end_hook = if forge_config.verify_todos {
tracing_handler
.clone()
Expand Down Expand Up @@ -194,7 +194,8 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeAp
let conversation = orch.get_conversation().clone();
let save_result = services.upsert_conversation(conversation).await;

// Send any error to the stream (prioritize dispatch error over save error)
// Send any error to the stream (prioritize dispatch error
// over save error)
#[allow(clippy::collapsible_if)]
if let Some(err) = dispatch_result.err().or(save_result.err()) {
if let Err(e) = tx.send(Err(err)).await {
Expand Down Expand Up @@ -306,7 +307,8 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeAp
pub async fn get_all_provider_models(&self) -> Result<Vec<ProviderModels>> {
let all_providers = self.services.get_all_providers().await?;

// Build one future per configured provider, preserving the error on failure.
// Build one future per configured provider, preserving the error on
// failure.
let futures: Vec<_> = all_providers
.into_iter()
.filter_map(|any_provider| any_provider.into_configured())
Expand Down
3 changes: 2 additions & 1 deletion crates/forge_app/src/command_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,8 @@ mod tests {
.map(|(path, is_dir)| File { path: path.clone(), is_dir: *is_dir })
.collect();

// Sort: directories first (alphabetically), then files (alphabetically)
// Sort: directories first (alphabetically), then files
// (alphabetically)
files.sort_by(|a, b| match (a.is_dir, b.is_dir) {
(true, false) => std::cmp::Ordering::Less,
(false, true) => std::cmp::Ordering::Greater,
Expand Down
42 changes: 24 additions & 18 deletions crates/forge_app/src/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ impl Compactor {
// chains. After compaction, this consistency can break if the first
// remaining assistant lacks reasoning.
//
// Solution: Extract the LAST reasoning from compacted messages and inject it
// into the first assistant message after compaction. This preserves
// chain continuity while preventing exponential accumulation across
// multiple compactions.
// Solution: Extract the LAST reasoning from compacted messages and
// inject it into the first assistant message after compaction.
// This preserves chain continuity while preventing exponential
// accumulation across multiple compactions.
//
// Example: [U, A+r, U, A+r, U, A] → compact → [U-summary, A+r, U, A]
// └─from last
Expand All @@ -133,8 +133,8 @@ impl Compactor {
_ => None,
});

// Accumulate usage from all messages in the compaction range before they are
// destroyed
// Accumulate usage from all messages in the compaction range before
// they are destroyed
let compacted_usage = context.messages.get(start..=end).and_then(|slice| {
slice
.iter()
Expand All @@ -143,7 +143,8 @@ impl Compactor {
.reduce(|a, b| a.accumulate(&b))
});

// Replace the range with the summary, transferring the accumulated usage
// Replace the range with the summary, transferring the accumulated
// usage
let mut summary_entry = MessageEntry::from(ContextMessage::user(summary, None));
summary_entry.usage = compacted_usage;
context
Expand Down Expand Up @@ -289,7 +290,8 @@ mod tests {

let context = compactor.compress_single_sequence(context, (0, 2)).unwrap();

// Verify reasoning didn't accumulate - should still be just 1 reasoning block
// Verify reasoning didn't accumulate - should still be just 1 reasoning
// block
let first_assistant = context
.messages
.iter()
Expand Down Expand Up @@ -318,7 +320,8 @@ mod tests {
..Default::default()
}];

// Most recent message in range has empty reasoning, earlier has non-empty
// Most recent message in range has empty reasoning, earlier has
// non-empty
let context = Context::default()
.add_message(ContextMessage::user("M1", None))
.add_message(ContextMessage::assistant(
Expand Down Expand Up @@ -647,8 +650,9 @@ mod tests {
"Summary message should carry accumulated usage from compacted messages"
);

// accumulate_usage() must sum both the compacted range usage (on the summary
// message) and the surviving outside_usage — total = inside + inside2 + outside
// accumulate_usage() must sum both the compacted range usage (on the
// summary message) and the surviving outside_usage — total =
// inside + inside2 + outside
let expected_total_usage = Usage {
total_tokens: TokenCount::Actual(100000),
prompt_tokens: TokenCount::Actual(90000),
Expand Down Expand Up @@ -855,17 +859,19 @@ mod tests {
// - Safe threshold (89.6K): ~95K tokens, SHOULD compact (true)
//
// At turn 2:
// - Unsafe threshold (100K): ~110K tokens, SHOULD compact (true) - but too
// late!
// - Unsafe threshold (100K): ~110K tokens, SHOULD compact (true) - but
// too late!
// - Safe threshold (89.6K): ~110K tokens, already compacted at turn 1

// Verify that safe threshold triggers at turn 1 (providing early warning)
// Verify that safe threshold triggers at turn 1 (providing early
// warning)
let safe_token_count_turn1 = 95_000; // Approximate
let safe_should_compact_turn1 =
safe_compact.should_compact(&safe_context, safe_token_count_turn1);

// The key fix: safe threshold (89.6K) triggers at ~95K, while unsafe (100K)
// doesn't This provides a safety margin before we hit the 128K limit
// The key fix: safe threshold (89.6K) triggers at ~95K, while unsafe
// (100K) doesn't This provides a safety margin before we hit
// the 128K limit
assert!(
safe_should_compact_turn1 || safe_token_count_turn1 < 89_600,
"Safe threshold (89.6K) should trigger compaction at ~95K tokens to provide safety margin"
Expand All @@ -876,8 +882,8 @@ mod tests {
let final_unsafe = unsafe_context.token_count_approx();
let final_safe = safe_context.token_count_approx();

// Both should be identical since we're just testing threshold logic, not actual
// compaction
// Both should be identical since we're just testing threshold logic,
// not actual compaction
assert_eq!(
final_unsafe, final_safe,
"Both contexts should have same token count"
Expand Down
23 changes: 14 additions & 9 deletions crates/forge_app/src/dto/anthropic/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ impl TryFrom<forge_domain::Context> for Request {
// a positive effort / `max_tokens` still emit reasoning on the wire.
let reasoning_on = request.is_reasoning_supported();
let (thinking, output_config) = if reasoning_on && let Some(reasoning) = request.reasoning {
// Adaptive thinking on 4.7 hides reasoning content by default; opting
// into reasoning should surface it unless the caller set `exclude`.
// Adaptive thinking on 4.7 hides reasoning content by default;
// opting into reasoning should surface it unless the
// caller set `exclude`.
let adaptive_display = if reasoning.exclude == Some(true) {
Some(ThinkingDisplay::Omitted)
} else {
Expand Down Expand Up @@ -185,7 +186,8 @@ impl TryFrom<forge_domain::Context> for Request {
output_config,
output_format: request.response_format.and_then(|rf| match rf {
forge_domain::ResponseFormat::Text => {
// Anthropic doesn't have a "text" output format, so we skip it
// Anthropic doesn't have a "text" output format, so we skip
// it
None
}
forge_domain::ResponseFormat::JsonSchema(schema) => {
Expand Down Expand Up @@ -260,7 +262,8 @@ impl TryFrom<ContextMessage> for Message {
forge_domain::Role::User => Message { role: Role::User, content },
forge_domain::Role::Assistant => Message { role: Role::Assistant, content },
forge_domain::Role::System => {
// note: Anthropic doesn't support system role messages and they're already
// note: Anthropic doesn't support system role messages
// and they're already
// filtered out. so this state is unreachable.
return Err(
forge_domain::Error::UnsupportedRole("System".to_string()).into()
Expand All @@ -285,7 +288,8 @@ impl Message {
*content = std::mem::take(content).cached(false);
}

// If enabling cache, set cache control on the last cacheable content item
// If enabling cache, set cache control on the last cacheable content
// item
if enable_cache
&& let Some(last_cacheable_idx) =
self.content
Expand Down Expand Up @@ -592,7 +596,8 @@ mod tests {

#[test]
fn test_reasoning_max_tokens_and_effort_emit_both() {
// Effort and budget are independent knobs — neither should hide the other.
// Effort and budget are independent knobs — neither should hide the
// other.
let fixture = Context::default().reasoning(ReasoningConfig {
effort: Some(forge_domain::Effort::Low),
enabled: Some(true),
Expand Down Expand Up @@ -735,9 +740,9 @@ mod tests {

#[test]
fn test_reasoning_enabled_none_with_max_tokens_still_emits_thinking() {
// Matches the domain's `is_reasoning_supported` rule: enabled: None with a
// positive budget counts as on, so inherited/merged configs don't silently
// disable reasoning on the wire.
// Matches the domain's `is_reasoning_supported` rule: enabled: None
// with a positive budget counts as on, so inherited/merged
// configs don't silently disable reasoning on the wire.
let fixture = Context::default().reasoning(ReasoningConfig {
enabled: None,
max_tokens: Some(8000),
Expand Down
21 changes: 13 additions & 8 deletions crates/forge_app/src/dto/anthropic/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,11 @@ impl From<Usage> for forge_domain::Usage {
fn from(usage: Usage) -> Self {
// Anthropic token breakdown:
// - input_tokens: tokens NOT from cache (billed at full price)
// - cache_creation_input_tokens: tokens written to cache (billed at full price
// - cache_creation_input_tokens: tokens written to cache (billed at
// full price
// + write cost)
// - cache_read_input_tokens: tokens read from cache (billed at 90% discount)
// - cache_read_input_tokens: tokens read from cache (billed at 90%
// discount)
// Total input = input_tokens + cache_creation_input_tokens +
// cache_read_input_tokens

Expand Down Expand Up @@ -320,7 +322,8 @@ impl TryFrom<Event> for ChatCompletionMessage {
ChatCompletionMessage::try_from(content_block)?
}
Event::MessageStart { message } => {
// Extract usage from MessageStart - this contains input token counts
// Extract usage from MessageStart - this contains input token
// counts
ChatCompletionMessage::assistant(Content::part("")).usage(message.usage)
}
Event::MessageDelta { delta, usage } => {
Expand All @@ -332,7 +335,8 @@ impl TryFrom<Event> for ChatCompletionMessage {
return Err(error.into());
}
Event::Ping { cost: Some(cost) } => {
// OpenCode Zen sends cost in a ping event at the end of the stream
// OpenCode Zen sends cost in a ping event at the end of the
// stream
let cost_value = match cost {
StringOrF64::Number(n) => n,
StringOrF64::String(s) => s.parse().unwrap_or(0.0),
Expand Down Expand Up @@ -395,8 +399,9 @@ impl TryFrom<ContentBlock> for ChatCompletionMessage {
)
}
ContentBlock::ToolUse { id, name, input } => {
// note: We've to check if the input is empty or null. else we end up adding
// empty object `{}` as prefix to tool args.
// note: We've to check if the input is empty or null. else we
// end up adding empty object `{}` as prefix to
// tool args.
let is_empty =
input.is_null() || input.as_object().is_some_and(|map| map.is_empty());
ChatCompletionMessage::assistant(Content::part("")).add_tool_call(ToolCallPart {
Expand Down Expand Up @@ -565,8 +570,8 @@ mod tests {
let expected_prompt = TokenCount::Actual(100 + 200 + 300);
assert_eq!(actual.prompt_tokens, expected_prompt);

// cached_tokens should only include cache reads (tokens that benefited from
// caching)
// cached_tokens should only include cache reads (tokens that benefited
// from caching)
let expected_cached = TokenCount::Actual(300);
assert_eq!(actual.cached_tokens, expected_cached);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ impl Transformer for EnforceStrictObjectSchema {
if let Some(OutputFormat::JsonSchema { schema }) = request.output_format.take() {
// Convert schema to JSON value for normalization
if let Ok(mut schema_value) = serde_json::to_value(&schema) {
// Use non-strict mode (false) for Anthropic - only adds additionalProperties
// Use non-strict mode (false) for Anthropic - only adds
// additionalProperties
enforce_strict_schema(&mut schema_value, false);

// Convert back to RootSchema
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,17 @@ mod tests {
#[test]
fn test_handles_server_name_containing_tool_substring() {
// Server named "my_tool_server", tool named "action"
// rfind gives the LAST `_tool_`, so server = "my_tool_server", tool = "action"
// rfind gives the LAST `_tool_`, so server = "my_tool_server", tool =
// "action"
let actual = to_claude_code_format("mcp_my_tool_server_tool_action");
let expected = "mcp__my_tool_server__action";
assert_eq!(actual, expected);
}

#[test]
fn test_leaves_already_converted_names_unchanged() {
// mcp__ prefix means it was already converted; no `_tool_` in sanitized names
// mcp__ prefix means it was already converted; no `_tool_` in sanitized
// names
let actual = to_claude_code_format("mcp__github__create_issue");
let expected = "mcp__github__create_issue";
assert_eq!(actual, expected);
Expand Down
3 changes: 2 additions & 1 deletion crates/forge_app/src/dto/anthropic/transforms/set_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ mod tests {
) -> String {
let mut messages = Vec::new();

// Add system messages to the regular messages array for Anthropic format
// Add system messages to the regular messages array for Anthropic
// format
for c in system_messages.chars() {
match c {
's' => messages.push(
Expand Down
Loading
Loading