Compaction rework

.gitattributes

@@ -1 +1,14 @@
+# Normalize all auto-detected text files to LF on checkout and in the index,
+# regardless of the developer's core.autocrlf setting. Windows developers
+# otherwise see tests fail because `include_str!` embeds CRLF bytes from the
+# autocrlf'd working tree and snapshot tests then byte-compare against LF
+# snapshots.
+* text=auto eol=lf
+
+# Shell scripts and zsh rc fragments must always be LF.
 *.zsh eol=lf
+
+# Binary snapshot kinds emitted by insta (e.g. HTML snapshots) should never be
+# line-ending converted.
+*.snap.html -text
+*.snap.new.html -text

Cargo.toml

@@ -53,7 +53,7 @@ ignore = "0.4.23"
 is_ci = "1.2.0"
 indexmap = "2.13.0"
 infer = "0.19.0"
-insta = { version = "1.47.2", features = ["json", "yaml"] }
+insta = { version = "1.47.2", features = ["json", "yaml", "redactions"] }
 lazy_static = "1.4.0"
 machineid-rs = "1.2.4"
 mockito = "1.7.2"
@@ -163,3 +163,4 @@ forge_test_kit = { path = "crates/forge_test_kit" }
 
 forge_markdown_stream = { path = "crates/forge_markdown_stream" }
 forge_config = { path = "crates/forge_config" }
+forge_compact = { path = "crates/forge_compact" }

crates/forge_api/src/api.rs

@@ -93,14 +93,6 @@ pub trait API: Sync + Send {
         title: String,
     ) -> Result<()>;
 
-    /// Compacts the context of the main agent for the given conversation and
-    /// persists it. Returns metrics about the compaction (original vs.
-    /// compacted tokens and messages).
-    async fn compact_conversation(
-        &self,
-        conversation_id: &ConversationId,
-    ) -> Result<CompactionResult>;
-
     /// Executes a shell command using the shell tool infrastructure
     async fn execute_shell_command(
         &self,

crates/forge_api/src/forge_api.rs

@@ -145,20 +145,6 @@ impl<
         self.services.upsert_conversation(conversation).await
     }
 
-    async fn compact_conversation(
-        &self,
-        conversation_id: &ConversationId,
-    ) -> anyhow::Result<CompactionResult> {
-        let agent_id = self
-            .services
-            .get_active_agent_id()
-            .await?
-            .unwrap_or_default();
-        self.app()
-            .compact_conversation(agent_id, conversation_id)
-            .await
-    }
-
     fn environment(&self) -> Environment {
         self.services.get_environment().clone()
     }

crates/forge_app/Cargo.toml

@@ -7,6 +7,7 @@ rust-version.workspace = true
 [dependencies]
 forge_domain.workspace = true
 forge_config.workspace = true
+forge_compact.workspace = true
 forge_stream.workspace = true
 async-trait.workspace = true
 anyhow.workspace = true

crates/forge_app/src/agent.rs

@@ -134,8 +134,6 @@ impl AgentExt for Agent {
             // 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(),
-                max_tokens: workflow_compact.max_tokens,
                 token_threshold: workflow_compact.token_threshold,
                 token_threshold_percentage: workflow_compact
                     .token_threshold_percentage
@@ -144,6 +142,7 @@ impl AgentExt for Agent {
                 message_threshold: workflow_compact.message_threshold,
                 model: workflow_compact.model.as_deref().map(ModelId::new),
                 on_turn_end: workflow_compact.on_turn_end,
+                max_prepended_summaries: workflow_compact.max_prepended_summaries,
             };
             merged_compact.merge(agent.compact.clone());
             agent.compact = merged_compact;
@@ -273,82 +272,44 @@ mod tests {
 
     /// Tests the current behavior: agent compact settings take priority over
     /// workflow config.
-    ///
-    /// CURRENT BEHAVIOR: When agent has compact settings, they override
-    /// workflow settings. This means user's .forge.toml compact settings
-    /// are ignored if agent has ANY compact config.
-    ///
-    /// Note: The apply_config comment says "Agent settings take priority over
-    /// workflow settings", which is implemented via the merge() call that
-    /// overwrites workflow values with agent values.
+    /// When the agent leaves a compact field unset, the workflow's
+    /// value must survive the merge. Every field uses the `option`
+    /// merge strategy so `None` on the agent side falls through.
     #[test]
-    fn test_compact_agent_settings_take_priority_over_workflow_config() {
-        use forge_config::Percentage;
-
-        // Workflow config with custom compact settings (from .forge.toml)
+    fn test_workflow_compact_applies_when_agent_leaves_fields_unset() {
         let workflow_compact = forge_config::Compact::default()
             .retention_window(10_usize)
-            .eviction_window(Percentage::new(0.3).unwrap())
-            .max_tokens(5000_usize)
             .token_threshold(80000_usize)
             .token_threshold_percentage(0.65_f64);
 
         let config = ForgeConfig::default().compact(workflow_compact);
-
-        // Agent with default compact config - retention_window=0 from Default
         let agent = fixture_agent();
 
         let actual = agent.apply_config(&config).compact;
 
-        // 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"
-
-        // Agent default has retention_window=0, which overwrites workflow's 10
-        assert_eq!(
-            actual.retention_window, 0,
-            "Agent's retention_window (0) takes priority over workflow's (10). \
-             This is the CURRENT behavior per apply_config comment. \
-             If user wants workflow settings to apply, agent should have no compact config set."
-        );
-
-        // Agent default has token_threshold=None, workflow's 80000 should apply
-        assert_eq!(
-            actual.token_threshold,
-            Some(80000),
-            "Workflow token_threshold applies because agent default has None"
-        );
         assert_eq!(
-            actual.token_threshold_percentage,
-            Some(0.65),
-            "Workflow context-window percentage applies because agent default has None"
+            actual.retention_window,
+            Some(10),
+            "workflow retention_window must survive when the agent leaves it unset"
         );
+        assert_eq!(actual.token_threshold, Some(80000));
+        assert_eq!(actual.token_threshold_percentage, Some(0.65));
     }
 
-    /// Tests the current behavior when agent has partial compact config:
-    /// those agent values override workflow values.
-    ///
-    /// CURRENT BEHAVIOR: If agent sets ANY compact field, that value wins over
-    /// workflow config. Only fields where agent has None will get workflow
-    /// values.
+    /// Fields the agent *does* set win over the workflow defaults;
+    /// fields the agent leaves `None` inherit from the workflow.
     #[test]
-    fn test_compact_partial_agent_settings_override_workflow_values() {
-        use forge_config::Percentage;
+    fn test_compact_partial_agent_settings_win_per_field() {
         use forge_domain::Compact as DomainCompact;
 
-        // Workflow config with ALL settings
         let workflow_compact = forge_config::Compact::default()
             .retention_window(15_usize)
-            .eviction_window(Percentage::new(0.25).unwrap())
-            .max_tokens(6000_usize)
             .token_threshold(90000_usize)
             .token_threshold_percentage(0.4_f64)
             .turn_threshold(20_usize);
 
         let config = ForgeConfig::default().compact(workflow_compact);
 
-        // Agent with PARTIAL compact config (only retention_window set to 5)
         let agent = fixture_agent().compact(
             DomainCompact::new()
                 .retention_window(5_usize)
@@ -357,29 +318,9 @@ mod tests {
 
         let actual = agent.apply_config(&config).compact;
 
-        // CURRENT BEHAVIOR: Agent's retention_window=5 overwrites workflow's 15
-        assert_eq!(
-            actual.retention_window, 5,
-            "Agent's retention_window (5) takes priority. \
-             This is CURRENT behavior: agent.compact.retention_window is Some(5), \
-             so merge() overwrites workflow's Some(15) with agent's Some(5)."
-        );
-
-        // Fields where agent had None get workflow values
-        assert_eq!(
-            actual.token_threshold,
-            Some(90000),
-            "Workflow token_threshold applies (agent had None)"
-        );
-        assert_eq!(
-            actual.token_threshold_percentage,
-            Some(0.25),
-            "Agent's context-window percentage takes priority over workflow's 0.4"
-        );
-        assert_eq!(
-            actual.turn_threshold,
-            Some(20),
-            "Workflow turn_threshold applies (agent had None)"
-        );
+        assert_eq!(actual.retention_window, Some(5), "agent value wins when set");
+        assert_eq!(actual.token_threshold_percentage, Some(0.25));
+        assert_eq!(actual.token_threshold, Some(90000), "workflow fills unset agent field");
+        assert_eq!(actual.turn_threshold, Some(20));
     }
 }

crates/forge_app/src/app.rs

@@ -10,7 +10,7 @@ use crate::apply_tunable_parameters::ApplyTunableParameters;
 use crate::changed_files::ChangedFiles;
 use crate::dto::ToolsOverview;
 use crate::hooks::{
-    CompactionHandler, DoomLoopDetector, PendingTodosHandler, TitleGenerationHandler,
+    DoomLoopDetector, PendingTodosHandler, TitleGenerationHandler,
     TracingHandler,
 };
 use crate::init_conversation_metrics::InitConversationMetrics;
@@ -124,14 +124,14 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeAp
                 .add_system_message(conversation)
                 .await?;
 
-        // Insert user prompt
-        let conversation = UserPromptGenerator::new(
+        // Build pending-turn messages; canonical stays untouched.
+        let (conversation, pending) = UserPromptGenerator::new(
             self.services.clone(),
             agent.clone(),
             chat.event.clone(),
             current_time,
         )
-        .add_user_prompt(conversation)
+        .generate(conversation)
         .await?;
 
         // Detect and render externally changed files notification
@@ -162,18 +162,15 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeAp
         let hook = Hook::default()
             .on_start(tracing_handler.clone().and(title_handler))
             .on_request(tracing_handler.clone().and(DoomLoopDetector::default()))
-            .on_response(
-                tracing_handler
-                    .clone()
-                    .and(CompactionHandler::new(agent.clone(), environment.clone())),
-            )
+            .on_response(tracing_handler.clone())
             .on_toolcall_start(tracing_handler.clone())
             .on_toolcall_end(tracing_handler)
             .on_end(on_end_hook);
 
         let orch = Orchestrator::new(
             services.clone(),
             conversation,
+            pending,
             agent,
             self.services.get_config()?,
         )
@@ -208,77 +205,6 @@ impl<S: Services + EnvironmentInfra<Config = forge_config::ForgeConfig>> ForgeAp
         Ok(stream)
     }
 
-    /// Compacts the context of the main agent for the given conversation and
-    /// persists it. Returns metrics about the compaction (original vs.
-    /// compacted tokens and messages).
-    pub async fn compact_conversation(
-        &self,
-        active_agent_id: AgentId,
-        conversation_id: &ConversationId,
-    ) -> Result<CompactionResult> {
-        use crate::compact::Compactor;
-
-        // Get the conversation
-        let mut conversation = self
-            .services
-            .find_conversation(conversation_id)
-            .await?
-            .ok_or_else(|| forge_domain::Error::ConversationNotFound(*conversation_id))?;
-
-        // Get the context from the conversation
-        let context = match conversation.context.as_ref() {
-            Some(context) => context.clone(),
-            None => {
-                // No context to compact, return zero metrics
-                return Ok(CompactionResult::new(0, 0, 0, 0));
-            }
-        };
-
-        // Calculate original metrics
-        let original_messages = context.messages.len();
-        let original_token_count = *context.token_count();
-
-        let forge_config = self.services.get_config()?;
-
-        // Get agent and apply workflow config
-        let agent = self.services.get_agent(&active_agent_id).await?;
-
-        let Some(agent) = agent else {
-            return Ok(CompactionResult::new(
-                original_token_count,
-                0,
-                original_messages,
-                0,
-            ));
-        };
-
-        // Get compact config from the agent
-        let compact = agent
-            .apply_config(&forge_config)
-            .set_compact_model_if_none()
-            .compact;
-
-        // Apply compaction using the Compactor
-        let environment = self.services.get_environment();
-        let compacted_context = Compactor::new(compact, environment).compact(context, true)?;
-
-        let compacted_messages = compacted_context.messages.len();
-        let compacted_tokens = *compacted_context.token_count();
-
-        // Update the conversation with the compacted context
-        conversation.context = Some(compacted_context);
-
-        // Save the updated conversation
-        self.services.upsert_conversation(conversation).await?;
-
-        Ok(CompactionResult::new(
-            original_token_count,
-            compacted_tokens,
-            original_messages,
-            compacted_messages,
-        ))
-    }
-
     pub async fn list_tools(&self) -> Result<ToolsOverview> {
         self.tool_registry.tools_overview().await
     }

crates/forge_app/src/command_generator.rs

@@ -338,7 +338,7 @@ mod tests {
 
         assert_eq!(actual, "ls -la");
         let captured_context = fixture.captured_context.lock().await.clone().unwrap();
-        insta::assert_yaml_snapshot!(captured_context);
+        insta::assert_yaml_snapshot!(captured_context, { ".**.id" => "[id]" });
     }
 
     #[tokio::test]
@@ -353,7 +353,7 @@ mod tests {
 
         assert_eq!(actual, "pwd");
         let captured_context = fixture.captured_context.lock().await.clone().unwrap();
-        insta::assert_yaml_snapshot!(captured_context);
+        insta::assert_yaml_snapshot!(captured_context, { ".**.id" => "[id]" });
     }
 
     #[tokio::test]