fix(neo4j)!: scope the v2 purge on the can:// id prefix; retire _module from v2

schema.neo4j.json

@@ -1,8 +1,9 @@
 {
-  "schema_version": "2.2.0",
+  "schema_version": "3.0.0",
   "generator": "codeanalyzer-java",
   "marker_labels": [
-    "JEntrypoint"
+    "JEntrypoint",
+    "JCanNode"
   ],
   "node_labels": [
     {
@@ -24,8 +25,7 @@
         "id": "string",
         "file_key": "string",
         "package": "string",
-        "content_hash": "string",
-        "_module": "string"
+        "content_hash": "string"
       }
     },
     {
@@ -41,7 +41,6 @@
         "interfaces": "string[]",
         "docstring": "string",
         "is_entrypoint": "boolean",
-        "_module": "string",
         "start_line": "integer",
         "end_line": "integer"
       }
@@ -67,7 +66,6 @@
         "accessed_fields": "string[]",
         "is_implicit": "boolean",
         "is_entrypoint": "boolean",
-        "_module": "string",
         "start_line": "integer",
         "end_line": "integer"
       }
@@ -94,7 +92,6 @@
         "initializer": "string",
         "modifiers": "string[]",
         "docstring": "string",
-        "_module": "string",
         "start_line": "integer",
         "end_line": "integer"
       }
@@ -108,7 +105,6 @@
         "name": "string",
         "type": "string",
         "initializer": "string",
-        "_module": "string",
         "start_line": "integer",
         "end_line": "integer"
       }
@@ -121,8 +117,7 @@
         "id": "string",
         "name": "string",
         "arguments": "string[]",
-        "docstring": "string",
-        "_module": "string"
+        "docstring": "string"
       }
     },
     {
@@ -135,8 +130,7 @@
         "type": "string",
         "modifiers": "string[]",
         "is_variadic": "boolean",
-        "docstring": "string",
-        "_module": "string"
+        "docstring": "string"
       }
     },
     {
@@ -155,7 +149,6 @@
         "is_static_call": "boolean",
         "argument_types": "string[]",
         "argument_expr": "string[]",
-        "_module": "string",
         "var": "string",
         "call_node": "string",
         "start_line": "integer",
@@ -518,6 +511,7 @@
     "CREATE INDEX j_callable_name IF NOT EXISTS FOR (c:JCallable) ON (c.name)",
     "CREATE INDEX j_type_name IF NOT EXISTS FOR (t:JType) ON (t.name)",
     "CREATE INDEX j_annotation_name_idx IF NOT EXISTS FOR (an:JAnnotation) ON (an.name)",
-    "CREATE FULLTEXT INDEX j_code_fts IF NOT EXISTS FOR (c:JCallable) ON EACH [c.code, c.docstring]"
+    "CREATE FULLTEXT INDEX j_code_fts IF NOT EXISTS FOR (c:JCallable) ON EACH [c.code, c.docstring]",
+    "CREATE INDEX j_can_node_id IF NOT EXISTS FOR (n:JCanNode) ON (n.id)"
   ]
 }

src/main/java/com/ibm/cldk/neo4j/BoltWriter.java

@@ -49,27 +49,54 @@
  */
 public final class BoltWriter implements BoltSink {
     /**
-     * Purge the edges a changed module owns, and the declarations that vanished from it.
+     * The v2 purge: scoped by the module's own {@code can://} id rather than by a bare file key.
      *
-     * <p>Both are anchored on {@link CypherWriter#MODULE_OWNED}, and that anchor is the whole point:
-     * {@code _module} is a shared convention rather than a java-private property, so the sibling
-     * analyzers set it on their own nodes with the same value (the module's file key). The
-     * unlabelled {@code MATCH (x {_module: $m})} these replaced therefore matched a sibling's nodes
-     * wherever a file key collided across languages in a shared database, and deleted a graph this
-     * tool did not write and cannot see -- silently, unless the delete happened to be large enough
-     * to exhaust the transaction limit and roll back (#213).
+     * <p>The id is a path — {@code can://java/<app>/<file>} — so matching the module itself by
+     * equality and its declarations by {@code id STARTS WITH <id> + '/'} is containment, and it is
+     * simultaneously scoped to one language, one application and one module. That last one is what
+     * neither a label anchor nor {@code _module} could give: two java applications sharing
+     * {@code src/main/java/Foo.java} have identical labels and an identical file key, and only the
+     * id distinguishes them (.github#50).
      *
-     * <p>Constants rather than inline strings so the anchoring is assertable without a live
-     * database; {@code BoltWriterPurgeTest} pins it.
+     * <p>The trailing slash is not cosmetic. A bare {@code STARTS WITH <id>} would also match
+     * {@code <id>Xtra}, so descendants are matched on the separator and the module on equality.
+     *
+     * <p>{@code :JCanNode} is present only so the prefix predicate can seek: Neo4j property indexes
+     * are label-scoped, and without a label this scans the whole node store.
      */
-    static final String PURGE_MODULE_EDGES =
-            "MATCH (x:" + CypherWriter.MODULE_OWNED + ") WHERE x._module = $m "
+    static final String PURGE_MODULE_EDGES_V2 =
+            "MATCH (x:" + RowBuilder.CAN_NODE + ") WHERE x.id = $mid OR x.id STARTS WITH $pre "
                     + "MATCH (x)-[r]->() DELETE r";
 
-    /** @see #PURGE_MODULE_EDGES */
-    static final String PURGE_VANISHED_NODES =
-            "MATCH (x:" + CypherWriter.MODULE_OWNED + ") WHERE x._module = $m "
-                    + "AND NOT coalesce(x.id, x.file_key) IN $keys DETACH DELETE x";
+    /** @see #PURGE_MODULE_EDGES_V2 */
+    static final String PURGE_VANISHED_NODES_V2 =
+            "MATCH (x:" + RowBuilder.CAN_NODE + ") WHERE (x.id = $mid OR x.id STARTS WITH $pre) "
+                    + "AND NOT x.id IN $keys DETACH DELETE x";
+
+    /**
+     * The prefix that matches a module's declarations but not a sibling module whose path merely
+     * starts the same way. The separator is the whole point: {@code can://java/app/src/Foo.java}
+     * is also a prefix of {@code can://java/app/src/Foo.javaX}, so a bare {@code STARTS WITH} on
+     * the module id would purge a different module's nodes. The module itself is matched by
+     * equality instead, since its own id does not end in a separator.
+     */
+    static String descendantPrefix(String moduleId) {
+        return moduleId + "/";
+    }
+
+    /**
+     * The {@code can://} id of the module these rows belong to, or {@code null} on the v1 path.
+     * Taken from the module's own row rather than parsed out of a declaration's id, because a file
+     * key may itself contain {@code /} and so cannot be recovered by splitting.
+     */
+    private static String canModuleIdOf(List<NodeRow> nodes) {
+        for (NodeRow n : nodes) {
+            if (n.labels.contains("JModule") && RowBuilder.isCanId(n.value)) {
+                return n.value;
+            }
+        }
+        return null;
+    }
 
 
     private static final int BATCH = 1000;
@@ -115,10 +142,9 @@ void run(GraphRows rows, boolean fullRun) {
             List<NodeRow> shared = new ArrayList<>();
             Map<String, String> unitOf = new HashMap<>(); // node value → owning unit
             for (NodeRow n : rows.nodes) {
-                Object m = n.props.get("_module");
-                if (m instanceof String) {
-                    byUnit.computeIfAbsent((String) m, x -> new ArrayList<>()).add(n);
-                    unitOf.put(n.value, (String) m);
+                if (n.moduleKey != null) {
+                    byUnit.computeIfAbsent(n.moduleKey, x -> new ArrayList<>()).add(n);
+                    unitOf.put(n.value, n.moduleKey);
                 } else {
                     shared.add(n);
                 }
@@ -155,8 +181,27 @@ void run(GraphRows rows, boolean fullRun) {
                 }
                 try (Session s = session()) {
                     s.writeTransaction(tx -> {
-                        tx.run(PURGE_MODULE_EDGES, Values.parameters("m", unit));
-                        tx.run(PURGE_VANISHED_NODES, Values.parameters("m", unit, "keys", keys));
+                        String moduleId = canModuleIdOf(nodes);
+                        if (moduleId == null) {
+                            // Legacy v1 rows: FQN-shaped ids carry no application segment, so there
+                            // is no prefix to scope by. v2 is the schema as of 3.0.0, so rather than
+                            // carry a second scoping mechanism for a legacy path, the purge is
+                            // skipped and said so. Nodes are still upserted -- a v1 push updates, it
+                            // just never deletes.
+                            Log.info("neo4j(bolt): legacy v1 rows for " + unit
+                                    + " - purge skipped (v1 ids carry no application scope)");
+                            return null;
+                        }
+                        // The module's own can:// id scopes this to one module of one application of
+                        // one language at once, because the id is a path and a prefix match on it is
+                        // containment. `_module` could not: it is a bare project-relative file key,
+                        // so two applications sharing a source path purged each other (.github#50).
+                        Map<String, Object> args = new LinkedHashMap<>();
+                        args.put("mid", moduleId);
+                        args.put("pre", descendantPrefix(moduleId));
+                        args.put("keys", keys);
+                        tx.run(PURGE_MODULE_EDGES_V2, args);
+                        tx.run(PURGE_VANISHED_NODES_V2, args);
                         return null;
                     });
                 }

src/main/java/com/ibm/cldk/neo4j/CypherWriter.java

@@ -17,7 +17,6 @@
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -56,38 +55,6 @@ public final class CypherWriter {
             + "|J_HAS_CRUD_OPERATION|J_HAS_CRUD_QUERY|J_HAS_COMMENT"
             + "|J_DECLARES|J_HAS_METHOD|J_HAS_BODY_NODE*1..]";
 
-    /**
-     * The labels this analyzer owns per module -- every label in either generation's catalog that
-     * carries the internal {@code _module} provenance property, as a Cypher label disjunction
-     * ({@code JModule|JType|JCallable|...}).
-     *
-     * <p>{@code _module} is NOT java-private. The sibling analyzers set the same property on their
-     * own nodes, using the same value (the module's file key), so an unlabelled
-     * {@code MATCH (x {_module: $m})} matches THEIR nodes wherever a file key collides across
-     * languages in a shared database -- and a delete built on it destroys a graph this tool did not
-     * write and cannot see. Every statement that matches on {@code _module} must be anchored here.
-     * The hazard is the same one {@link #DESCENDANTS} is written to avoid, one property over.
-     *
-     * <p>Derived from the catalogs rather than hand-listed, so a label that gains {@code _module}
-     * later is covered without a second list to maintain, and spans BOTH generations for the same
-     * reason {@code DESCENDANTS} does: {@link BoltWriter} is schema-agnostic and may be pushing
-     * either one.
-     */
-    static final String MODULE_OWNED = moduleOwnedPattern();
-
-    private static String moduleOwnedPattern() {
-        List<String> labels = new ArrayList<>();
-        for (List<SchemaCatalog.NodeLabel> catalog :
-                Arrays.asList(SchemaCatalog.NODE_LABELS, V2SchemaCatalog.NODE_LABELS)) {
-            for (SchemaCatalog.NodeLabel nl : catalog) {
-                if (nl.properties.containsKey("_module") && !labels.contains(nl.label)) {
-                    labels.add(nl.label);
-                }
-            }
-        }
-        return String.join("|", labels);
-    }
-
     private CypherWriter() {}
 
     /**

src/main/java/com/ibm/cldk/neo4j/GraphRows.java

@@ -56,8 +56,25 @@ public static final class NodeRow {
         public final String keyProp;
         public final String value;
         public final Map<String, Object> props;
+        /**
+         * The file key of the module owning this node, or {@code null} for a shared node.
+         *
+         * <p>In memory only — deliberately NOT a property, so it is never written to the graph. The
+         * incremental writer needs to group nodes by module; the graph does not need to be told,
+         * because a node's owning module is already the prefix of its {@code can://} id. Emitting it
+         * was how a delete came to be scoped by a bare file key that carries no language and no
+         * application, which is what made two applications sharing a path delete each other's nodes
+         * (codellm-devkit/.github#50).
+         */
+        public final String moduleKey;
 
         public NodeRow(List<String> labels, String keyProp, String value, Map<String, Object> props) {
+            this(labels, keyProp, value, props, null);
+        }
+
+        public NodeRow(List<String> labels, String keyProp, String value, Map<String, Object> props,
+                String moduleKey) {
+            this.moduleKey = moduleKey;
             this.labels = labels;
             this.keyProp = keyProp;
             this.value = value;

src/main/java/com/ibm/cldk/neo4j/RowBuilder.java

@@ -78,13 +78,47 @@ public NodeRef node(List<String> labels, String keyProp, String value, Map<Strin
                     existing.labels.add(l);
                 }
             }
+            // putAll may have re-introduced the lifted property; strip it again on the v2 path only,
+            // so a re-merged v1 node keeps the property its contract still includes.
+            if (isCanId(value)) {
+                existing.props.remove(MODULE_PROP);
+            }
         } else {
-            nodes.put(id, new NodeRow(new ArrayList<>(labels), keyProp, value, new LinkedHashMap<>(props)));
+            Map<String, Object> p = new LinkedHashMap<>(props);
+            // Lift the owning module off the emitted properties and onto the row. The writer needs
+            // it to group nodes; the graph does not, because a v2 node's owning module is already
+            // the prefix of its can:// id. See NodeRow.moduleKey.
+            Object m = p.remove(MODULE_PROP);
+            String moduleKey = m instanceof String ? (String) m : null;
+            List<String> allLabels = new ArrayList<>(labels);
+            if (isCanId(value)) {
+                // The index anchor for prefix-scoped statements: Neo4j property indexes are
+                // label-scoped, so `WHERE n.id STARTS WITH $p` needs a label to seek on. Carries no
+                // safety claim of its own -- the prefix does that -- and is java-namespaced so an
+                // anchoring mistake still cannot reach another language's nodes.
+                allLabels.add(CAN_NODE);
+            } else if (moduleKey != null) {
+                // v1 ids are FQN-shaped and carry no application segment, so they cannot be
+                // prefix-scoped. That path keeps the property it has always had; only the v2
+                // contract drops it.
+                p.put(MODULE_PROP, moduleKey);
+            }
+            nodes.put(id, new NodeRow(allLabels, keyProp, value, p, moduleKey));
         }
         keys.add(value);
         return new NodeRef(labels.get(0), keyProp, value);
     }
 
+    /** The marker label carried by every node keyed on a {@code can://} id. */
+    public static final String CAN_NODE = "JCanNode";
+
+    private static final String MODULE_PROP = "_module";
+
+    /** Whether a node key is a v2 {@code can://} id, as opposed to a v1 FQN-shaped one. */
+    static boolean isCanId(String value) {
+        return value != null && value.startsWith("can://");
+    }
+
     /** An edge whose endpoints are known to exist (both ends emitted as nodes this run). */
     public void edge(String type, NodeRef from, NodeRef to, Map<String, Object> props) {
         edges.add(new EdgeRow(type, from, to, props));

src/main/java/com/ibm/cldk/neo4j/Schema.java

@@ -57,5 +57,10 @@ private Schema() {}
             "CREATE INDEX j_callable_name IF NOT EXISTS FOR (c:JCallable) ON (c.name)",
             "CREATE INDEX j_type_name IF NOT EXISTS FOR (t:JType) ON (t.name)",
             "CREATE INDEX j_annotation_name_idx IF NOT EXISTS FOR (an:JAnnotation) ON (an.name)",
-            "CREATE FULLTEXT INDEX j_code_fts IF NOT EXISTS FOR (c:JCallable) ON EACH [c.code, c.docstring]");
+            "CREATE FULLTEXT INDEX j_code_fts IF NOT EXISTS FOR (c:JCallable) ON EACH [c.code, c.docstring]",
+            // The incremental purge matches `id STARTS WITH <module-id>`. Neo4j property indexes are
+            // label-scoped, so without this the prefix seek degrades to a scan of every node in the
+            // store, once per changed module. STARTS WITH is index-backed on a range index; CONTAINS
+            // and ENDS WITH are not, which is why the predicate is written this way.
+            "CREATE INDEX j_can_node_id IF NOT EXISTS FOR (n:" + RowBuilder.CAN_NODE + ") ON (n.id)");
 }