fix: preserve Built stage and detect cross-flush file overwrites

nojaf · nojaf · commit c9d0b631141c · 2026-02-20T10:17:28.000+01:00
TypecheckOnly passes (typecheck_dependents, full_typecheck) no longer
downgrade Built modules to TypeChecked when cmi/cmt hashes are
unchanged — the .cmj and JS on disk remain valid.

Cross-flush delete+create sequences (LLM file overwrites, git
checkout) are now detected via a recently_deleted set on PendingState.
When a file is deleted in one flush and created in the next, it is
routed through file_build (FullCompile) to emit JS, instead of
triggering two TypecheckOnly project rebuilds.
diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs
@@ -587,6 +587,18 @@ pub fn process_in_waves(
             "cmt",
         ));
 
+        // Extract Built hashes before taking the mutable borrow below.
+        // Used to preserve Built state when TypecheckOnly produces identical artifacts.
+        let prev_built_hashes = match prev_stage {
+            CompilationStage::Built {
+                cmi_hash,
+                cmt_hash,
+                cmj_hash,
+                ..
+            } => Some((*cmi_hash, *cmt_hash, *cmj_hash)),
+            _ => None,
+        };
+
         if let Some(Module::SourceFile(sf)) = build_state.build_state.modules.get_mut(name) {
             let compile_warnings = module_compile_warnings.remove(name);
             match (mode.emits_js(), cmi_hash, cmt_hash) {
@@ -639,18 +651,26 @@ pub fn process_in_waves(
                     }
                 }
                 (false, Some(cmi), Some(cmt)) => {
-                    sf.set_compilation_stage(CompilationStage::TypeChecked {
-                        implementation_source_hash,
-                        implementation_ast_hash,
-                        interface_source_hash,
-                        interface_ast_hash,
-                        cmi_hash: cmi,
-                        cmt_hash: cmt,
-                        compiled_at: now,
-                        implementation_parse_warnings,
-                        interface_parse_warnings,
-                        compile_warnings,
-                    });
+                    // If the module was already Built and the type artifacts
+                    // didn't change, skip — the .cmj and JS on disk are still
+                    // valid. This prevents TypecheckOnly passes from losing
+                    // the JS-produced marker.
+                    if !prev_built_hashes
+                        .is_some_and(|(old_cmi, old_cmt, _)| old_cmi == cmi && old_cmt == cmt)
+                    {
+                        sf.set_compilation_stage(CompilationStage::TypeChecked {
+                            implementation_source_hash,
+                            implementation_ast_hash,
+                            interface_source_hash,
+                            interface_ast_hash,
+                            cmi_hash: cmi,
+                            cmt_hash: cmt,
+                            compiled_at: now,
+                            implementation_parse_warnings,
+                            interface_parse_warnings,
+                            compile_warnings,
+                        });
+                    }
                 }
                 _ => {
                     sf.set_compilation_stage(CompilationStage::SourceDirty);
diff --git a/rewatch/src/lsp/queue.rs b/rewatch/src/lsp/queue.rs
@@ -130,6 +130,10 @@ struct PendingState {
     /// File changes that require full project rebuilds.
     /// Flush resolves project roots and groups these into per-project sets.
     build_projects: PendingProjectBuilds,
+    /// Files deleted in a recent flush that haven't been matched by a create.
+    /// Used to detect cross-flush delete+create (file overwrite) patterns
+    /// from LLMs and git operations.
+    recently_deleted: HashSet<PathBuf>,
 }
 
 /// Debug summary of what a flush is about to process.
@@ -357,6 +361,7 @@ impl PendingState {
             typechecks: HashMap::new(),
             compile_files: HashMap::new(),
             build_projects: PendingProjectBuilds::new(),
+            recently_deleted: HashSet::new(),
         }
     }
 
@@ -424,27 +429,37 @@ impl PendingState {
                 if let Ok(uri) = Url::from_file_path(&file_path) {
                     self.typechecks.remove(&uri);
                     self.compile_files.remove(&uri);
-                    // If this file was previously deleted in the same batch,
-                    // it was recreated (e.g. git checkout). Schedule an
-                    // incremental build to emit JS after the full rebuild.
-                    if self.build_projects.deleted_files.contains(&file_path) {
+                }
+                // Check both same-flush (build_projects.deleted_files) and
+                // cross-flush (recently_deleted) for the delete+create pattern.
+                let same_flush_delete = self.build_projects.deleted_files.remove(&file_path);
+                let cross_flush_delete = self.recently_deleted.remove(&file_path);
+                if same_flush_delete || cross_flush_delete {
+                    // Delete + create = content overwrite (LLM, git checkout).
+                    // Still need project_build to re-add the module to state,
+                    // plus file_build to emit JS with FullCompile.
+                    self.build_projects.created_files.insert(file_path.clone());
+                    if let Ok(uri) = Url::from_file_path(&file_path) {
                         self.compile_files.insert(
                             uri,
                             PendingFileBuild {
-                                file_path: file_path.clone(),
+                                file_path,
                                 buffer_content: None,
                                 generation: 0,
                             },
                         );
                     }
+                } else {
+                    // Genuinely new file — needs full project rebuild.
+                    self.build_projects.created_files.insert(file_path);
                 }
-                self.build_projects.created_files.insert(file_path);
             }
             QueueEvent::FileDeleted { file_path } => {
                 if let Ok(uri) = Url::from_file_path(&file_path) {
                     self.typechecks.remove(&uri);
                     self.compile_files.remove(&uri);
                 }
+                self.recently_deleted.insert(file_path.clone());
                 self.build_projects.deleted_files.insert(file_path);
             }
             QueueEvent::ConfigChanged { file_path } => {
@@ -560,6 +575,14 @@ async fn flush_inner(
     let _flush_guard = diagnostic_store.as_ref().map(|s| FlushGuard::new(s));
     let store_ref = diagnostic_store.as_deref();
 
+    // Rotate recently_deleted: clear old entries from previous flushes,
+    // carry forward this flush's deletes for cross-flush detection.
+    // Must happen before project_build::run takes build_projects.
+    state.recently_deleted.clear();
+    state
+        .recently_deleted
+        .extend(state.build_projects.deleted_files.iter().cloned());
+
     // Step 0: Run full builds (file creation / deletion).
     let has_full_builds = !state.build_projects.is_empty();
     if has_full_builds {
@@ -933,10 +956,10 @@ mod tests {
         assert!(state.build_projects.created_files.contains(&test_path("New.res")));
     }
 
-    // -- Recreated files (delete + create) → promoted to compile_files --
+    // -- Recreated files (delete + create) → treated as save --
 
     #[test]
-    fn delete_then_create_promotes_to_compile_files() {
+    fn delete_then_create_treated_as_overwrite() {
         let mut state = PendingState::new();
 
         // File deleted (e.g. git checkout starts)
@@ -949,9 +972,10 @@ mod tests {
             file_path: test_path("Pkmn.res"),
         });
 
-        // Should be in both build_projects sets
+        // Delete+create = overwrite. Removed from deleted_files,
+        // but still in created_files (project_build re-adds the module).
         assert!(
-            state
+            !state
                 .build_projects
                 .deleted_files
                 .contains(&test_path("Pkmn.res"))
@@ -1102,4 +1126,67 @@ mod tests {
         assert!(state.typechecks.contains_key(&uri_b));
         assert!(!state.typechecks.contains_key(&uri_a));
     }
+
+    // -- Cross-flush delete+create detection --
+
+    /// Simulate the recently_deleted rotation that flush_inner performs.
+    fn simulate_flush_rotation(state: &mut PendingState) {
+        state.recently_deleted.clear();
+        state
+            .recently_deleted
+            .extend(state.build_projects.deleted_files.iter().cloned());
+        // Simulate project_build taking build_projects
+        state.build_projects = PendingProjectBuilds::new();
+        // Simulate file_build/file_typecheck taking their maps
+        state.compile_files.clear();
+        state.typechecks.clear();
+    }
+
+    #[test]
+    fn cross_flush_delete_then_create_treated_as_overwrite() {
+        let mut state = PendingState::new();
+
+        // Flush N: file is deleted
+        state.merge(QueueEvent::FileDeleted {
+            file_path: test_path("Pkmn.res"),
+        });
+
+        // Simulate flush N completing
+        simulate_flush_rotation(&mut state);
+
+        // Flush N+1: same file is created (cross-flush overwrite)
+        state.merge(QueueEvent::FileCreated {
+            file_path: test_path("Pkmn.res"),
+        });
+
+        // Should be in created_files (project_build re-adds module to state)
+        // AND in compile_files (file_build emits JS)
+        assert!(
+            state
+                .build_projects
+                .created_files
+                .contains(&test_path("Pkmn.res"))
+        );
+        assert!(state.compile_files.contains_key(&test_uri("Pkmn.res")));
+    }
+
+    #[test]
+    fn recently_deleted_cleared_after_rotation_without_create() {
+        let mut state = PendingState::new();
+
+        // Flush N: file is deleted
+        state.merge(QueueEvent::FileDeleted {
+            file_path: test_path("Gone.res"),
+        });
+
+        // Simulate flush N completing — recently_deleted carries {Gone.res}
+        simulate_flush_rotation(&mut state);
+        assert!(state.recently_deleted.contains(&test_path("Gone.res")));
+
+        // Flush N+1: no create arrives, flush completes
+        simulate_flush_rotation(&mut state);
+
+        // recently_deleted should now be empty (no new deletes to carry forward)
+        assert!(state.recently_deleted.is_empty());
+    }
 }
diff --git a/tests/rewatch_tests/tests/lsp/__snapshots__/built-downgrade.test.mjs.snap b/tests/rewatch_tests/tests/lsp/__snapshots__/built-downgrade.test.mjs.snap
@@ -1,5 +1,94 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
+exports[`lsp compile mode preservation > external file overwrite via delete+create still produces JS 1`] = `
+[
+  "rewatch.lsp",
+  "  lsp.initialized",
+  "    lsp.register_watchers[watcher_count=3]",
+  "      lsp.discover_package[name=@rewatch-test/dep-chain]",
+  "        lsp.source_dir[dir=packages/dep-chain/src, recursive=true]",
+  "  lsp.initial_build[project=@rewatch-test/dep-chain]",
+  "    build.load_package_sources[package=@rewatch-test/dep-chain]",
+  "    packages.parse_packages",
+  "    clean.cleanup_previous_build",
+  "    build.parse[dirty_modules=4]",
+  "      build.parse_file[module=App, package=@rewatch-test/dep-chain]",
+  "      build.parse_file[module=Button, package=@rewatch-test/dep-chain]",
+  "      build.parse_file[module=Leaf, package=@rewatch-test/dep-chain]",
+  "      build.parse_file[module=Main, package=@rewatch-test/dep-chain]",
+  "    full_typecheck[module_count=4, output=lsp]",
+  "      build.typecheck",
+  "        build.typecheck_wave[file_count=2]",
+  "          build.typecheck_file[module=Button, package=@rewatch-test/dep-chain]",
+  "          build.typecheck_file[module=Leaf, package=@rewatch-test/dep-chain]",
+  "        build.typecheck_wave[file_count=1]",
+  "          build.typecheck_file[module=App, package=@rewatch-test/dep-chain]",
+  "        build.typecheck_wave[file_count=1]",
+  "          build.typecheck_file[module=Main, package=@rewatch-test/dep-chain]",
+  "  lsp.did_save[file=packages/dep-chain/src/Leaf.res]",
+  "  lsp.flush[incremental_builds=packages/dep-chain/src/Leaf.res]",
+  "    lsp.flush.file_build.batch[modules=["Leaf"]]",
+  "      lsp.flush.file_build.compile_dependencies",
+  "        build.parse[dirty_modules=1]",
+  "          build.parse_file[module=Leaf, package=@rewatch-test/dep-chain]",
+  "        compile_dependencies[module_count=4, output=lsp]",
+  "          build.compile",
+  "            build.compile_wave[file_count=1]",
+  "              build.compile_file[module=Leaf, package=@rewatch-test/dep-chain, suffix=.mjs, module_system=esmodule]",
+  "      lsp.flush.file_build.typecheck_dependents[dependent_count=2]",
+  "        typecheck_dependents[module_count=4, output=lsp]",
+  "          build.typecheck",
+  "            build.typecheck_wave[file_count=1]",
+  "              build.typecheck_file[module=App, package=@rewatch-test/dep-chain]",
+  "            build.typecheck_wave[file_count=1]",
+  "              build.typecheck_file[module=Main, package=@rewatch-test/dep-chain]",
+  "      lsp.flush.file_build.compile_resolved",
+  "  lsp.did_change_watched_files[file_count=1]",
+  "  lsp.flush[project_builds=deleted: packages/dep-chain/src/Leaf.res]",
+  "    lsp.flush.project_build[project=@rewatch-test/dep-chain]",
+  "      build.load_package_sources[package=@rewatch-test/dep-chain]",
+  "      packages.parse_packages",
+  "      clean.cleanup_previous_build",
+  "      build.parse[dirty_modules=0]",
+  "      full_typecheck[module_count=3, output=lsp]",
+  "        build.typecheck",
+  "          build.typecheck_wave[file_count=1]",
+  "            build.typecheck_file[module=App, package=@rewatch-test/dep-chain]",
+  "        build.compile_error",
+  "  lsp.did_change_watched_files[file_count=1]",
+  "  lsp.flush[project_builds=created: packages/dep-chain/src/Leaf.res, incremental_builds=packages/dep-chain/src/Leaf.res]",
+  "    lsp.flush.project_build[project=@rewatch-test/dep-chain]",
+  "      build.load_package_sources[package=@rewatch-test/dep-chain]",
+  "      packages.parse_packages",
+  "      clean.cleanup_previous_build",
+  "      build.parse[dirty_modules=1]",
+  "        build.parse_file[module=Leaf, package=@rewatch-test/dep-chain]",
+  "      full_typecheck[module_count=4, output=lsp]",
+  "        build.typecheck",
+  "          build.typecheck_wave[file_count=1]",
+  "            build.typecheck_file[module=Leaf, package=@rewatch-test/dep-chain]",
+  "          build.typecheck_wave[file_count=1]",
+  "            build.typecheck_file[module=App, package=@rewatch-test/dep-chain]",
+  "          build.typecheck_wave[file_count=1]",
+  "    lsp.flush.file_build.batch[modules=["Leaf"]]",
+  "      lsp.flush.file_build.compile_dependencies",
+  "        build.parse[dirty_modules=1]",
+  "          build.parse_file[module=Leaf, package=@rewatch-test/dep-chain]",
+  "        compile_dependencies[module_count=4, output=lsp]",
+  "          build.compile",
+  "            build.compile_wave[file_count=1]",
+  "              build.compile_file[module=Leaf, package=@rewatch-test/dep-chain, suffix=.mjs, module_system=esmodule]",
+  "      lsp.flush.file_build.typecheck_dependents[dependent_count=2]",
+  "        typecheck_dependents[module_count=4, output=lsp]",
+  "          build.typecheck",
+  "            build.typecheck_wave[file_count=1]",
+  "              build.typecheck_file[module=App, package=@rewatch-test/dep-chain]",
+  "            build.typecheck_wave[file_count=1]",
+  "              build.typecheck_file[module=Main, package=@rewatch-test/dep-chain]",
+  "      lsp.flush.file_build.compile_resolved",
+]
+`;
+
 exports[`lsp compile mode preservation > preserves FullCompile intent across intervening typecheck passes 1`] = `
 [
   "rewatch.lsp",
diff --git a/tests/rewatch_tests/tests/lsp/built-downgrade.test.mjs b/tests/rewatch_tests/tests/lsp/built-downgrade.test.mjs
@@ -1,4 +1,4 @@
-import { existsSync } from "node:fs";
+import { existsSync, readFileSync, writeFileSync } from "node:fs";
 import path from "node:path";
 import { pathToFileURL } from "node:url";
 import { describe, expect, it } from "vitest";
@@ -96,4 +96,63 @@ describe("lsp compile mode preservation", { timeout: 60_000 }, () => {
       },
       { cwd: "packages/dep-chain" },
     ));
+
+  // Regression test: cross-flush delete+create (LLM file overwrite) must
+  // produce JS output, not trigger two TypecheckOnly project rebuilds.
+  //
+  // Scenario:
+  // 1. Save Leaf.res to produce Leaf.mjs (initial build is TypecheckOnly).
+  // 2. Delete Leaf.res and notify as Deleted — triggers project_build flush.
+  // 3. Wait for the delete to land in a separate flush (200ms > debounce).
+  // 4. Write new content and notify as Created — should be treated as save.
+  // 5. Verify Leaf.mjs exists with the updated content.
+  it("external file overwrite via delete+create still produces JS", () =>
+    runLspTest(
+      async ({ lsp, lspCwd, writeFile, deleteFile }) => {
+        const rootUri = pathToFileURL(lspCwd).href;
+        await lsp.initialize(rootUri);
+        await lsp.waitForNotification("rescript/buildFinished", 30000);
+
+        // Save Leaf.res to produce Leaf.mjs (initial build is TypecheckOnly, no JS yet)
+        await writeFile("src/Leaf.res", 'let value = "hello"\n');
+        await lsp.waitForNotification("rescript/buildFinished", 30000);
+
+        const leafMjs = path.join(lspCwd, "src", "Leaf.mjs");
+        expect(existsSync(leafMjs), "Leaf.mjs should exist after save").toBe(
+          true,
+        );
+
+        // Simulate LLM-style file overwrite: delete then create in separate flushes.
+        await deleteFile("src/Leaf.res");
+        lsp.notifyWatchedFilesChanged([
+          { relativePath: "src/Leaf.res", type: 3 }, // Deleted
+        ]);
+
+        // Wait for the delete flush to complete
+        await lsp.waitForNotification("rescript/buildFinished", 30000);
+
+        // Write new content and notify as created
+        const fullPath = path.join(lspCwd, "src", "Leaf.res");
+        writeFileSync(fullPath, 'let value = "updated"\n');
+        lsp.notifyWatchedFilesChanged([
+          { relativePath: "src/Leaf.res", type: 1 }, // Created
+        ]);
+
+        // Wait for the create flush to complete
+        await lsp.waitForNotification("rescript/buildFinished", 30000);
+
+        // Leaf.mjs should exist with updated content.
+        // Without the fix, the delete triggers project_build (TypecheckOnly),
+        // and the create triggers another project_build — neither emits JS.
+        // With the fix, the cross-flush delete+create is detected and treated
+        // as a save, which goes through file_build (FullCompile) → JS emitted.
+        expect(
+          existsSync(leafMjs),
+          "Leaf.mjs must exist after delete+create overwrite",
+        ).toBe(true);
+        const content = readFileSync(leafMjs, "utf8");
+        expect(content).toContain("updated");
+      },
+      { cwd: "packages/dep-chain" },
+    ));
 });
