fix(attachment): hash full file content instead of range content and consolidate FileInfo (#2578)

Author: tusharmath

Cargo.lock

@@ -111,10 +111,7 @@ mod tests {
                     let hash = compute_hash(content);
                     ReadOutput {
                         content: Content::file(content.clone()),
-                        start_line: 1,
-                        end_line: 1,
-                        total_lines: 1,
-                        content_hash: hash,
+                        info: forge_domain::FileInfo::new(1, 1, 1, hash),
                     }
                 })
                 .ok_or_else(|| anyhow::anyhow!(std::io::Error::from(std::io::ErrorKind::NotFound)))

crates/forge_app/src/changed_files.rs

@@ -62,14 +62,14 @@ impl<F: FsReadService> FileChangeDetector<F> {
                 async move {
                     // Get current hash from the full raw file content (not the
                     // truncated/formatted content returned to the LLM).
-                    // ReadOutput.content_hash is always computed from the
+                    // ReadOutput.info.content_hash is always computed from the
                     // unprocessed file, so it is directly comparable with the
                     // stored hash.
                     let current_hash = fs
                         .read(file_path.to_string_lossy().to_string(), None, None)
                         .await
                         .ok()
-                        .map(|o| o.content_hash);
+                        .map(|o| o.info.content_hash);
 
                     // Check if hash has changed
                     if current_hash != last_hash {
@@ -182,10 +182,7 @@ mod tests {
             if let Some(file) = self.files.get(&path) {
                 Ok(crate::ReadOutput {
                     content: Content::File(file.displayed_content.clone()),
-                    start_line: 1,
-                    end_line: 1,
-                    total_lines: 1,
-                    content_hash: compute_hash(&file.raw_content),
+                    info: forge_domain::FileInfo::new(1, 1, 1, compute_hash(&file.raw_content)),
                 })
             } else {
                 Err(anyhow::anyhow!(std::io::Error::from(

crates/forge_app/src/file_tracking.rs

@@ -56,7 +56,7 @@ mod tests {
 
     use console::strip_ansi_codes;
     use forge_display::DiffFormat;
-    use forge_domain::{ChatResponseContent, Environment};
+    use forge_domain::{ChatResponseContent, Environment, FileInfo};
     use insta::assert_snapshot;
     use pretty_assertions::assert_eq;
 
@@ -97,10 +97,7 @@ mod tests {
             },
             output: ReadOutput {
                 content: Content::file(content),
-                start_line: 1,
-                end_line: 1,
-                total_lines: 5,
-                content_hash: crate::compute_hash(content),
+                info: FileInfo::new(1, 1, 5, crate::compute_hash(content)),
             },
         };
         let env = fixture_environment();
@@ -123,10 +120,7 @@ mod tests {
             },
             output: ReadOutput {
                 content: Content::file(content),
-                start_line: 2,
-                end_line: 4,
-                total_lines: 10,
-                content_hash: crate::compute_hash(content),
+                info: FileInfo::new(2, 4, 10, crate::compute_hash(content)),
             },
         };
         let env = fixture_environment();

crates/forge_app/src/fmt/fmt_output.rs

@@ -78,6 +78,9 @@ pub trait FileReaderInfra: Send + Sync {
     /// - FileInfo.start_line: starting line position
     /// - FileInfo.end_line: ending line position
     /// - FileInfo.total_lines: total line count in file
+    /// - FileInfo.content_hash: SHA-256 hash of the **full** file content,
+    ///   allowing callers to store a stable hash that matches what a whole-file
+    ///   read produces (used by the external-change detector)
     async fn range_read_utf8(
         &self,
         path: &Path,

crates/forge_app/src/infra.rs

@@ -238,7 +238,7 @@ impl ToolOperation {
                     *metrics = metrics.clone().insert(
                         input.file_path.clone(),
                         FileOperation::new(tool_kind)
-                            .content_hash(Some(output.content_hash.clone())),
+                            .content_hash(Some(output.info.content_hash.clone())),
                     );
 
                     return forge_domain::ToolOutput::image(image.clone());
@@ -248,7 +248,7 @@ impl ToolOperation {
                 let content = output.content.file_content();
                 let content = if input.show_line_numbers {
                     content
-                        .to_numbered_from(output.start_line as usize)
+                        .to_numbered_from(output.info.start_line as usize)
                         .to_string()
                 } else {
                     content.to_string()
@@ -257,7 +257,7 @@ impl ToolOperation {
                     .attr("path", &input.file_path)
                     .attr(
                         "display_lines",
-                        format!("{}-{}", output.start_line, output.end_line),
+                        format!("{}-{}", output.info.start_line, output.info.end_line),
                     )
                     .attr("total_lines", content.lines().count())
                     .cdata(content);
@@ -270,7 +270,8 @@ impl ToolOperation {
                 );
                 *metrics = metrics.clone().insert(
                     input.file_path.clone(),
-                    FileOperation::new(tool_kind).content_hash(Some(output.content_hash.clone())),
+                    FileOperation::new(tool_kind)
+                        .content_hash(Some(output.info.content_hash.clone())),
                 );
 
                 forge_domain::ToolOutput::text(elm)
@@ -708,7 +709,7 @@ mod tests {
     use std::fmt::Write;
     use std::path::PathBuf;
 
-    use forge_domain::{FSRead, ToolValue};
+    use forge_domain::{FSRead, FileInfo, ToolValue};
 
     use super::*;
     use crate::{Content, Match, MatchResult};
@@ -828,10 +829,7 @@ mod tests {
             },
             output: ReadOutput {
                 content: Content::file(content),
-                start_line: 1,
-                end_line: 2,
-                total_lines: 2,
-                content_hash: hash,
+                info: FileInfo::new(1, 2, 2, hash),
             },
         };
 
@@ -860,10 +858,7 @@ mod tests {
             },
             output: ReadOutput {
                 content: Content::file(content),
-                start_line: 1,
-                end_line: 1,
-                total_lines: 1,
-                content_hash: hash,
+                info: FileInfo::new(1, 1, 1, hash),
             },
         };
 
@@ -891,10 +886,7 @@ mod tests {
             },
             output: ReadOutput {
                 content: Content::file(content),
-                start_line: 2,
-                end_line: 3,
-                total_lines: 5,
-                content_hash: hash,
+                info: FileInfo::new(2, 3, 5, hash),
             },
         };
 
@@ -923,10 +915,7 @@ mod tests {
             },
             output: ReadOutput {
                 content: Content::file(content),
-                start_line: 1,
-                end_line: 100,
-                total_lines: 200,
-                content_hash: hash,
+                info: FileInfo::new(1, 100, 200, hash),
             },
         };
 
@@ -2499,10 +2488,7 @@ mod tests {
                     "base64_image_data".to_string(),
                     "image/png",
                 )),
-                start_line: 1,
-                end_line: 1,
-                total_lines: 1,
-                content_hash: "hash123".to_string(),
+                info: FileInfo::new(1, 1, 1, "hash123".to_string()),
             },
         };
 

crates/forge_app/src/operation.rs

@@ -6,8 +6,8 @@ use derive_setters::Setters;
 use forge_domain::{
     AgentId, AnyProvider, Attachment, AuthContextRequest, AuthContextResponse, AuthMethod,
     ChatCompletionMessage, CommandOutput, Context, Conversation, ConversationId, Environment, File,
-    FileStatus, Image, InitAuth, LoginInfo, McpConfig, McpServers, Model, ModelId, Node, Provider,
-    ProviderId, ResultStream, Scope, SearchParams, SyncProgress, SyntaxError, Template,
+    FileInfo, FileStatus, Image, InitAuth, LoginInfo, McpConfig, McpServers, Model, ModelId, Node,
+    Provider, ProviderId, ResultStream, Scope, SearchParams, SyncProgress, SyntaxError, Template,
     ToolCallFull, ToolOutput, Workflow, WorkspaceAuth, WorkspaceId, WorkspaceInfo,
 };
 use merge::Merge;
@@ -38,10 +38,7 @@ pub struct PatchOutput {
 #[setters(into)]
 pub struct ReadOutput {
     pub content: Content,
-    pub start_line: u64,
-    pub end_line: u64,
-    pub total_lines: u64,
-    pub content_hash: String,
+    pub info: FileInfo,
 }
 
 #[derive(Debug)]

crates/forge_app/src/services.rs

@@ -222,10 +222,11 @@ impl<S: AttachmentService> UserPromptGenerator<S> {
             // Use the raw content_hash (computed before line-numbering) so that the
             // external-change detector, which hashes the raw file on disk, sees a
             // matching hash and does not raise a false "modified externally" warning.
-            if let AttachmentContent::FileContent { content_hash, .. } = &attachment.content {
+            if let AttachmentContent::FileContent { info, .. } = &attachment.content {
                 metrics = metrics.insert(
                     attachment.path.clone(),
-                    FileOperation::new(ToolKind::Read).content_hash(Some(content_hash.clone())),
+                    FileOperation::new(ToolKind::Read)
+                        .content_hash(Some(info.content_hash.clone())),
                 );
             }
         }
@@ -240,8 +241,8 @@ impl<S: AttachmentService> UserPromptGenerator<S> {
 #[cfg(test)]
 mod tests {
     use forge_domain::{
-        AgentId, AttachmentContent, Context, ContextMessage, ConversationId, ModelId, ProviderId,
-        ToolKind,
+        AgentId, AttachmentContent, Context, ContextMessage, ConversationId, FileInfo, ModelId,
+        ProviderId, ToolKind,
     };
     use pretty_assertions::assert_eq;
 
@@ -390,20 +391,14 @@ mod tests {
                         path: "/test/file1.rs".to_string(),
                         content: AttachmentContent::FileContent {
                             content: "fn main() {}".to_string(),
-                            start_line: 1,
-                            end_line: 1,
-                            total_lines: 1,
-                            content_hash: "hash1".to_string(),
+                            info: FileInfo::new(1, 1, 1, "hash1".to_string()),
                         },
                     },
                     Attachment {
                         path: "/test/file2.rs".to_string(),
                         content: AttachmentContent::FileContent {
                             content: "fn test() {}".to_string(),
-                            start_line: 1,
-                            end_line: 1,
-                            total_lines: 1,
-                            content_hash: "hash2".to_string(),
+                            info: FileInfo::new(1, 1, 1, "hash2".to_string()),
                         },
                     },
                 ])

crates/forge_app/src/user_prompt.rs

@@ -1,7 +1,7 @@
 use nom::Parser;
 use nom::bytes::complete::tag;
 
-use crate::Image;
+use crate::{FileInfo, Image};
 
 /// A file or directory attachment included in a chat message.
 #[derive(Debug, serde::Deserialize, serde::Serialize, Clone, PartialEq, Eq)]
@@ -24,16 +24,9 @@ pub enum AttachmentContent {
         /// Line-numbered display text shown to the model. May represent only a
         /// slice of the full file when a range was requested.
         content: String,
-        /// First line of the displayed range (1-based, inclusive).
-        start_line: u64,
-        /// Last line of the displayed range (1-based, inclusive).
-        end_line: u64,
-        /// Total number of lines in the full file on disk.
-        total_lines: u64,
-        /// SHA-256 hash of the raw (unformatted, untruncated) file content.
-        /// Used for external-change detection so the stored hash matches what
-        /// the detector reads back from disk, avoiding false-positive warnings.
-        content_hash: String,
+        /// Metadata about the file read: line positions and full-file content
+        /// hash for external-change detection.
+        info: FileInfo,
     },
     /// A directory listing showing the immediate children of a directory.
     DirectoryListing {
@@ -76,8 +69,8 @@ impl AttachmentContent {
 
     pub fn range_info(&self) -> Option<(u64, u64, u64)> {
         match self {
-            AttachmentContent::FileContent { start_line, end_line, total_lines, .. } => {
-                Some((*start_line, *end_line, *total_lines))
+            AttachmentContent::FileContent { info, .. } => {
+                Some((info.start_line, info.end_line, info.total_lines))
             }
             _ => None,
         }

crates/forge_domain/src/attachment.rs

@@ -461,18 +461,12 @@ impl Context {
         attachments.into_iter().fold(self, |ctx, attachment| {
             ctx.add_message(match attachment.content {
                 AttachmentContent::Image(image) => ContextMessage::Image(image),
-                AttachmentContent::FileContent {
-                    content,
-                    start_line,
-                    end_line,
-                    total_lines,
-                    ..
-                } => {
+                AttachmentContent::FileContent { content, info } => {
                     let elm = Element::new("file_content")
                         .attr("path", attachment.path)
-                        .attr("start_line", start_line)
-                        .attr("end_line", end_line)
-                        .attr("total_lines", total_lines)
+                        .attr("start_line", info.start_line)
+                        .attr("end_line", info.end_line)
+                        .attr("total_lines", info.total_lines)
                         .cdata(content);
 
                     let mut message = TextMessage::new(Role::User, elm.to_string()).droppable(true);
@@ -734,7 +728,7 @@ mod tests {
 
     use super::*;
     use crate::transformer::Transformer;
-    use crate::{DirectoryEntry, estimate_token_count};
+    use crate::{DirectoryEntry, FileInfo, estimate_token_count};
 
     #[test]
     fn test_override_system_message() {
@@ -1134,10 +1128,7 @@ mod tests {
             path: "/path/to/file.rs".to_string(),
             content: AttachmentContent::FileContent {
                 content: "fn main() {}\n".to_string(),
-                start_line: 1,
-                end_line: 1,
-                total_lines: 1,
-                content_hash: "hash".to_string(),
+                info: FileInfo::new(1, 1, 1, "hash".to_string()),
             },
         }];
 
@@ -1187,20 +1178,14 @@ mod tests {
                 path: "/path/to/file1.rs".to_string(),
                 content: AttachmentContent::FileContent {
                     content: "fn foo() {}\n".to_string(),
-                    start_line: 1,
-                    end_line: 1,
-                    total_lines: 1,
-                    content_hash: "hash1".to_string(),
+                    info: FileInfo::new(1, 1, 1, "hash1".to_string()),
                 },
             },
             Attachment {
                 path: "/path/to/file2.rs".to_string(),
                 content: AttachmentContent::FileContent {
                     content: "fn bar() {}\n".to_string(),
-                    start_line: 1,
-                    end_line: 1,
-                    total_lines: 1,
-                    content_hash: "hash2".to_string(),
+                    info: FileInfo::new(1, 1, 1, "hash2".to_string()),
                 },
             },
         ];