Fix status detection for modified files and clean after view switch

atomic-cli/src/commands/status.rs

@@ -345,6 +345,22 @@ impl Status {
             print_blank();
         }
 
+        // Hint: stale FILE_INDEX detected
+        if status.needs_reindex() {
+            println!(
+                "  {}",
+                hint(&format!(
+                    "hint: {} file(s) reported as modified due to a stale index.",
+                    status.stale_index_count()
+                ))
+            );
+            println!(
+                "  {}",
+                hint("Run \"atomic status --reindex\" to rebuild the file index.")
+            );
+            print_blank();
+        }
+
         // Print summary hint
         if has_changes {
             print_hint("Use \"atomic record\" to record your changes");

atomic-repository/src/repository/materialize.rs

@@ -146,32 +146,58 @@ impl Repository {
     /// # Returns
     ///
     /// Statistics about the materialize operation.
-    /// Populate the file index for all files in a materialize result.
+    /// Populate the file index for all tracked files after a materialize.
     ///
-    /// Stats each written file from disk and stores its mtime + size +
-    /// content hash in the pristine database.  Errors are silently ignored
+    /// Stats each tracked file from disk and stores its mtime + size +
+    /// content hash in the pristine database. Errors are silently ignored
     /// (best-effort).
-    fn populate_file_index(&self, result: &MaterializeResult) {
+    ///
+    /// Why we ignore `result.file_results` and walk the tracked set: the
+    /// `MaterializeResult.file_results` map is only populated when
+    /// `merge_file_result(_, store_result=true)` is called, and the
+    /// `materialize_view` call site in `atomic-core` passes `false`. As a
+    /// result `result.file_results.keys()` is empty in production, and the
+    /// previous implementation of this function silently no-op'd —
+    /// FILE_INDEX was never refreshed by materialize, leaving stale
+    /// per-view hashes after `view switch` and producing false `Modified`
+    /// reports from `status`.
+    ///
+    /// Walking `list_tracked_files()` is correct because materialize has
+    /// just brought the working copy into sync with the destination
+    /// view's recorded state — every tracked file's on-disk content is
+    /// the authoritative baseline FILE_INDEX should cache.
+    fn populate_file_index(&self, _result: &MaterializeResult) {
         use std::time::SystemTime;
 
-        let mut entries: Vec<(String, i64, u32, u64, Hash)> = Vec::new();
-
-        for path in result.file_results.keys() {
-            let abs_path = self.root.join(path);
-            if let Ok(metadata) = std::fs::metadata(&abs_path) {
-                let mtime = metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH);
-                let duration = mtime
-                    .duration_since(SystemTime::UNIX_EPOCH)
-                    .unwrap_or_default();
-                let secs = duration.as_secs() as i64;
-                let nanos = duration.subsec_nanos();
-                let size = metadata.len();
-                let content_hash = std::fs::read(&abs_path)
-                    .map(|bytes| Hash::of(&bytes))
-                    .unwrap_or(Hash::ZERO);
-                let normalized = path.replace('\\', "/");
-                entries.push((normalized, secs, nanos, size, content_hash));
-            }
+        let tracked = match self.list_tracked_files() {
+            Ok(t) => t,
+            Err(_) => return,
+        };
+
+        let mut entries: Vec<(String, i64, u32, u64, Hash)> = Vec::with_capacity(tracked.len());
+
+        for file in &tracked {
+            let abs_path = self.root.join(&file.path);
+            let metadata = match std::fs::metadata(&abs_path) {
+                Ok(m) if m.is_file() => m,
+                _ => continue,
+            };
+
+            let mtime = metadata.modified().unwrap_or(SystemTime::UNIX_EPOCH);
+            let duration = mtime
+                .duration_since(SystemTime::UNIX_EPOCH)
+                .unwrap_or_default();
+            let secs = duration.as_secs() as i64;
+            let nanos = duration.subsec_nanos();
+            let size = metadata.len();
+
+            let content_hash = match std::fs::read(&abs_path) {
+                Ok(bytes) => Hash::of(&bytes),
+                Err(_) => continue,
+            };
+
+            let normalized = file.path.to_string_lossy().replace('\\', "/");
+            entries.push((normalized, secs, nanos, size, content_hash));
         }
 
         if !entries.is_empty() {

atomic-repository/src/repository/status.rs

@@ -40,28 +40,31 @@ impl Repository {
 
         // ── View-aware filtering ───────────────────────────────────────
         //
-        // Fast path: for a Shared view with no parent (the common case
-        // after `atomic init` or `atomic git import`), ALL changes in
-        // GRAPH are visible.  Skip the expensive O(N) scan entirely.
+        // Always build the explicit change filter.  The TREE table is
+        // global — it contains entries from ALL views, including child
+        // views.  Without filtering, files recorded on child views leak
+        // into the parent's status as false "Deleted" entries.
         //
-        // Slow path: for Draft views or views with parents, we need the
-        // actual filter set to hide changes from other views.
-        let (current_view_change_ids, filter_is_universal) = if let Some(ref view) = txn
+        // The previous "universal" fast-path (skip filter for
+        // is_shared() && parent.is_none()) was unsound: the dev view is
+        // shared with no parent, but child/sibling views may have unique
+        // changes.  Skipping the filter caused TREE entries created by
+        // those changes to surface as phantom `Deleted` files in dev's
+        // status.
+        //
+        // The filter computation is O(C) where C is changes on the view —
+        // a single B-tree scan, fast even on large repos.
+        //
+        // None means "no current view" (a misconfigured repo); preserve
+        // the legacy "show everything" behavior in that case rather than
+        // producing an empty status.
+        let current_view_change_ids: Option<HashSet<NodeId>> = if let Some(ref view) = txn
             .get_view(&self.current_view)
             .map_err(|e| RepositoryError::Database(e.to_string()))?
         {
-            if view.kind.is_shared() && view.parent.is_none() {
-                (HashSet::new(), true)
-            } else {
-                // Build the effective view filter entirely from pristine
-                // indexes. Do not load `.change` files here: status must stay
-                // proportional to working-copy/index state, not object-store
-                // history size.
-                let ids = collect_visible_change_ids_with_deps(&txn, view)?;
-                (ids, false)
-            }
+            Some(collect_visible_change_ids_with_deps(&txn, view)?)
         } else {
-            (HashSet::new(), true)
+            None
         };
 
         let phase1_ms = overall_start.elapsed().as_millis();
@@ -86,21 +89,16 @@ impl Repository {
             let (path, inode) = result.map_err(|e| RepositoryError::Database(e.to_string()))?;
 
             // View filter: skip files whose creating change is not on
-            // the current view.
-            // Check inode_position for every file:
-            // - For non-universal views: also apply the view filter
-            // - For universal views: just determine has_graph
-            //
-            // This is correct and required — files in TREE without a
-            // graph position must be classified as Added, not Clean.
+            // the current view.  Files in TREE without a graph position
+            // must be classified as Added, not Clean.  Files whose
+            // creating change is not in the current view's filter are
+            // skipped entirely — they belong to other views and must not
+            // appear in this view's status.
             let has_graph = if let Ok(Some(position)) = txn.inode_position(inode) {
-                // View filter: skip files whose creating change is not
-                // on the current view.
-                if !filter_is_universal
-                    && !position.change.is_root()
-                    && !current_view_change_ids.contains(&position.change)
-                {
-                    continue;
+                if let Some(ref ids) = current_view_change_ids {
+                    if !position.change.is_root() && !ids.contains(&position.change) {
+                        continue;
+                    }
                 }
                 true
             } else {
@@ -307,8 +305,50 @@ impl Repository {
             }
 
             // No FILE_INDEX entry — file is tracked with graph content
-            // but was never indexed. Assume clean (can't compare without
-            // reconstructing graph content, which is expensive).
+            // but was never indexed. This happens after `atomic insert`,
+            // `atomic clone`, or `atomic view switch` materializes a file
+            // into the working copy without going through `record()` (only
+            // record/materialize_view populate FILE_INDEX today).
+            //
+            // We CANNOT silently treat this as Clean: that would let real
+            // edits to such files become invisible to status/diff/record,
+            // and `record(all=true)` would silently drop them.
+            //
+            // Conservative correctness: mark Modified so the caller can
+            // run a full diff against pristine. If the file is actually
+            // unchanged, the recording workflow produces an empty hunk
+            // and skips it (record_modified_file returns is_empty()).
+            // Subsequent records re-populate FILE_INDEX, returning the
+            // file to the fast path.
+            status.add_stale_index_hit();
+            if options.hash_contents {
+                hash_count += 1;
+                let mut entry = FileStatusEntry::new(path.clone(), FileStatus::Modified);
+                if let Some(inode) = inode {
+                    entry.set_inode(inode);
+                }
+                match hash_file_contents(&abs_path) {
+                    Ok(current_hash) => {
+                        entry.set_current_hash(current_hash);
+                    }
+                    Err(_) => {
+                        entry.set_details("Unable to read file contents".to_string());
+                    }
+                }
+                entry.set_details("FILE_INDEX entry missing".to_string());
+                status.add_entry(entry);
+            } else {
+                // Fast mode: skip the hash but still surface the entry so
+                // it isn't silently dropped. Callers using fast mode (e.g.
+                // the agent record path) re-query with hash_contents=true
+                // before recording.
+                let mut entry = FileStatusEntry::new(path.clone(), FileStatus::Modified);
+                if let Some(inode) = inode {
+                    entry.set_inode(inode);
+                }
+                entry.set_details("FILE_INDEX entry missing".to_string());
+                status.add_entry(entry);
+            }
         }
 
         let classify_ms = classify_start.elapsed().as_millis();

atomic-repository/src/repository/tests/integration_tests.rs

@@ -393,3 +393,103 @@ fn test_switch_view_shows_view_content() {
         "Content should be feature version after switching to feature"
     );
 }
+
+/// Repro for the post-switch phantom-Deleted / false-Modified bug.
+///
+/// Scenario:
+///   1. Record alpha.txt + bravo.txt on dev.
+///   2. Split feature off dev, switch to feature.
+///   3. On feature: edit alpha.txt and add delta.txt; record.
+///   4. Switch back to dev.
+///
+/// At step 4 the working copy is correct (alpha.txt has dev's original
+/// content, delta.txt is gone), and dev's recorded state is also correct.
+/// Yet `status()` reports `alpha.txt: Modified` and `delta.txt: Deleted`.
+///
+/// Cause: `switch_view` updates the disk via `materialize` and removes
+/// stale files from the working copy, but it does not reconcile TREE
+/// or FILE_INDEX with the destination view's recorded state.
+///   - delta.txt remains in TREE (it was added by feature's record on a
+///     globally-shared TREE), and dev's status filter is "universal" for
+///     a no-parent shared view, so iter_tree surfaces delta.txt → since
+///     it's not on disk, it's reported Deleted.
+///   - alpha.txt's FILE_INDEX entry still holds feature's hash; status
+///     hashes the disk content, sees it differs from the cached hash,
+///     and reports Modified.
+///
+/// The correct post-switch invariant is: if no working-copy edits have
+/// happened since the switch, `status().is_clean()` must hold and there
+/// must be no phantom Deleted or false Modified entries.
+#[test]
+fn test_status_clean_after_view_switch_with_sibling_changes() {
+    let (temp_dir, mut repo) = create_temp_repo();
+
+    // Step 1: record alpha.txt + bravo.txt on dev.
+    std::fs::write(temp_dir.path().join("alpha.txt"), b"alpha-original\n").unwrap();
+    std::fs::write(temp_dir.path().join("bravo.txt"), b"bravo-original\n").unwrap();
+    repo.add("alpha.txt", TrackingOptions::default()).unwrap();
+    repo.add("bravo.txt", TrackingOptions::default()).unwrap();
+    repo.record(
+        ChangeHeader::new("Add alpha + bravo on dev"),
+        RecordOptions::new().with_all(true),
+    )
+    .unwrap();
+
+    // Step 2: split feature off dev and switch to it.
+    repo.create_view_from("feature", "dev").unwrap();
+    repo.switch_view("feature").unwrap();
+
+    // Step 3: on feature, modify alpha.txt and add delta.txt; record.
+    std::fs::write(
+        temp_dir.path().join("alpha.txt"),
+        b"alpha-modified-on-feature\n",
+    )
+    .unwrap();
+    std::fs::write(temp_dir.path().join("delta.txt"), b"delta-feature\n").unwrap();
+    repo.add("delta.txt", TrackingOptions::default()).unwrap();
+    repo.record(
+        ChangeHeader::new("Edit alpha + add delta on feature"),
+        RecordOptions::new().with_all(true),
+    )
+    .unwrap();
+
+    // Step 4: switch back to dev.
+    repo.switch_view("dev").unwrap();
+
+    // Sanity-check the disk before checking status. The switch should
+    // have restored alpha.txt to dev's content and removed delta.txt.
+    let alpha_on_disk = std::fs::read(temp_dir.path().join("alpha.txt")).unwrap();
+    assert_eq!(
+        alpha_on_disk, b"alpha-original\n",
+        "switch_view must restore alpha.txt to dev's content"
+    );
+    assert!(
+        !temp_dir.path().join("delta.txt").exists(),
+        "switch_view must remove delta.txt (not in dev) from the working copy"
+    );
+
+    // The actual assertion: status must reflect the disk truth.
+    let status = repo
+        .status(StatusOptions::default())
+        .expect("status failed");
+
+    // Collect any non-clean entries for diagnostics.
+    let dirty: Vec<(String, crate::status::FileStatus)> = status
+        .entries()
+        .iter()
+        .filter(|e| e.status().is_dirty())
+        .map(|e| (e.path().to_string_lossy().to_string(), e.status()))
+        .collect();
+
+    assert!(
+        dirty.is_empty(),
+        "status() after view switch must be clean — disk and view state agree, \
+         but status reported phantom dirty entries: {:?}",
+        dirty
+    );
+    assert!(
+        status.is_clean(),
+        "status().is_clean() must hold immediately after a view switch with no \
+         working-copy edits"
+    );
+}