From 8f20d29a6a973bae2c6e9502fc2a1d944cd73686 Mon Sep 17 00:00:00 2001
From: Mura Li <2606021+typeless@users.noreply.github.com>
Date: Wed, 12 Aug 2026 10:42:01 +0800
Subject: [PATCH] Delete the ghost-paths fallback the record reader now makes
 loud

merge_out_of_scope_commands scanned the new index for entries whose
non-empty path was missing from path_to_id, and its path lookup fell back
to that scan. Every producer of such an entry registers it: the two
serialization arms that skip registration are the ones that write an empty
path deliberately, keeping a slot so ids stay dense, and the loop's own
guard excluded exactly those. The two predicates cannot drift because
interning the empty string yields the empty handle the guard tests for --
which is now stated beside is_empty and pinned, since the guards rest on it
permanently while the comment that mentioned it goes with the map.

The map is not merely unexercised in practice but dominated in principle.
It can only matter on a run where a lookup consults path_to_id for an
unregistered non-empty path, and on exactly those runs deleting it is
better: resolve_file takes its miss branch instead, mints a second entry
for that path copying the old entry's type, and prior_paths has refused a
record naming one path twice since #382 -- usually through its per-list
repeats scan rather than the cross-class overlaps one, because the mint
copies the type of the entry it duplicates. So the deletion never reduces
detection and strictly increases it: what the fallback absorbed in silence
now reaches a reader.

The qualification worth stating: that loudness arrives one build late, and
as a record reported unreadable rather than as the producer that forgot to
register. It is a better failure than none, not a diagnosis.

Measured rather than argued: the function runs 234 times across the test
suite, walking 3126 file entries, and the map receives an entry zero times.
It is entered on every build that has a prior record -- the condition is
`if (old_index)`, with no scope test -- so despite its name it is not
scoped-build-only machinery, and the incremental scenarios exercise it far
more broadly than the issue's suggested net implies.

Closes #391

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Xkm9ce9gsgq41k4k6pvc3
---
 include/pup/core/string_id.hpp |  1 +
 src/cli/cmd_build.cpp          | 12 ------------
 test/unit/test_string_pool.cpp |  9 +++++++++
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/include/pup/core/string_id.hpp b/include/pup/core/string_id.hpp
index 8a797cdf..3d27b83a 100644
--- a/include/pup/core/string_id.hpp
+++ b/include/pup/core/string_id.hpp
@@ -13,6 +13,7 @@ namespace pup {
 enum class StringId : std::uint32_t { Empty = 0 };
 
 /// Check if StringId represents the empty string
+/// Only the empty string interns to Empty, so a guard may test the handle instead (#391).
 [[nodiscard]]
 constexpr auto is_empty(StringId id) -> bool
 {
diff --git a/src/cli/cmd_build.cpp b/src/cli/cmd_build.cpp
index 02d2ddf9..085fecd7 100644
--- a/src/cli/cmd_build.cpp
+++ b/src/cli/cmd_build.cpp
@@ -1287,22 +1287,10 @@ auto merge_out_of_scope_commands(
 
     auto new_lookup = index_command_lookup(ctx.index);
 
-    // Vestigial since the ghost arm began registering its own path (#386): every entry with a
-    // path is now in path_to_id, so this finds nothing. Kept pending its own removal.
-    auto ghost_paths = PathIdMap {};
-    for (auto const& file : ctx.index.files()) {
-        if (!pup::is_empty(file.path) && !ctx.path_to_id.find(file.path)) {
-            ghost_paths.insert(file.path, file.id);
-        }
-    }
-
     auto find_new_id_by_path = [&](StringId path) -> pup::NodeId {
         if (auto found = ctx.path_to_id.find(path)) {
             return *found;
         }
-        if (auto found = ghost_paths.find(path)) {
-            return *found;
-        }
         return pup::INVALID_NODE_ID;
     };
 
diff --git a/test/unit/test_string_pool.cpp b/test/unit/test_string_pool.cpp
index 0c1a63cf..52107324 100644
--- a/test/unit/test_string_pool.cpp
+++ b/test/unit/test_string_pool.cpp
@@ -2,6 +2,7 @@
 // Copyright (c) 2024 Putup authors
 
 #include "catch_amalgamated.hpp"
+#include "pup/core/string_id.hpp"
 #include "pup/core/string_pool.hpp"
 
 #include <cstdio>
@@ -112,3 +113,11 @@ TEST_CASE("StringPool Robin Hood index", "[string_pool]")
         REQUIRE(moved.find("before_move") == id);
     }
 }
+
+TEST_CASE("Interning the empty string yields the empty handle", "[string_pool]")
+{
+    auto pool = pup::StringPool {};
+    REQUIRE(pup::is_empty(pool.intern("")));
+    REQUIRE_FALSE(pup::is_empty(pool.intern("a")));
+    REQUIRE(pup::is_empty(pool.intern("")));
+}