fix(neo4j): carry module.source and span byte offsets so the graph can resolve text (#255)

Canonical decision D1 makes `module.source` the primary text and every narrower
node's text a byte-slice of it — it dropped per-callable `code` from
`analysis.json` on exactly that ground. The JSON projection obeys it. The graph
projection inverted it: `:JModule` carried no `source`, while `:JCallable` kept
the one derivation D1 deleted.

The source was already in hand where the graph is written — `V2GraphProjector`
calls `module.getSource()` to slice `code` out of it — so this was an omission
rather than missing data.

The flattened span was the other half of the same defect. `putLines` emitted
`start_line` / `end_line` only, dropping the columns and the UTF-8 byte offsets,
though the JSON schema defines `span.bytes` as "[from, to) UTF-8 offsets into
module.source, so node text is an O(1) slice". Carrying `source` without the
offsets would not have made anything sliceable.

Together the two gaps meant nothing narrower than a callable resolved to text at
all: python-sdk's reconstruction reports `source=""` and hardwires
`_body_source` to `None`, both for want of these facts.

- `:JModule` carries `source`, coalesced from null to `""` so it is always
  present — `RowBuilder.prune` drops nulls, and the projections would otherwise
  disagree about whether the property exists.
- The flattened span carries `start_column`, `end_column`, `start_byte`,
  `end_byte` beside the line pair. These four names are new shared vocabulary:
  codeanalyzer-python and codeanalyzer-typescript both flatten to the line pair
  alone and must adopt these spellings rather than coin their own.
- A conformance test asserts every `:JModule`'s `source` equals the
  `analysis.json` module source per module, not by presence, so a truncated or
  placeholder value fails.

Verified on daytrader8: 141 of 141 modules carry `source`, each hashing to the
`content_hash` its own row declares, which proves the value survives Cypher
escaping; and `source[start_byte:end_byte]` equals the node's existing `code`
byte-for-byte for all 662 callables where both are present, zero mismatches.

`JCallable.code` is now provably redundant but stays — python-sdk reconstructs
from it, so removing it is a separate breaking change. `schema_version` stays at
the held 2.0.0 baseline, so this is not detectable from the payload.

Closes #254

Author: rahlk

schema.neo4j.json

@@ -28,7 +28,8 @@
         "id": "string",
         "file_key": "string",
         "package": "string",
-        "content_hash": "string"
+        "content_hash": "string",
+        "source": "string"
       }
     },
     {
@@ -46,7 +47,11 @@
         "is_entrypoint": "boolean",
         "entrypoint_frameworks": "string[]",
         "start_line": "integer",
-        "end_line": "integer"
+        "start_column": "integer",
+        "end_line": "integer",
+        "end_column": "integer",
+        "start_byte": "integer",
+        "end_byte": "integer"
       }
     },
     {
@@ -72,7 +77,11 @@
         "is_entrypoint": "boolean",
         "entrypoint_frameworks": "string[]",
         "start_line": "integer",
-        "end_line": "integer"
+        "start_column": "integer",
+        "end_line": "integer",
+        "end_column": "integer",
+        "start_byte": "integer",
+        "end_byte": "integer"
       }
     },
     {
@@ -98,7 +107,11 @@
         "modifiers": "string[]",
         "docstring": "string",
         "start_line": "integer",
-        "end_line": "integer"
+        "start_column": "integer",
+        "end_line": "integer",
+        "end_column": "integer",
+        "start_byte": "integer",
+        "end_byte": "integer"
       }
     },
     {
@@ -111,7 +124,11 @@
         "type": "string",
         "initializer": "string",
         "start_line": "integer",
-        "end_line": "integer"
+        "start_column": "integer",
+        "end_line": "integer",
+        "end_column": "integer",
+        "start_byte": "integer",
+        "end_byte": "integer"
       }
     },
     {
@@ -157,7 +174,11 @@
         "var": "string",
         "call_node": "string",
         "start_line": "integer",
-        "end_line": "integer"
+        "start_column": "integer",
+        "end_line": "integer",
+        "end_column": "integer",
+        "start_byte": "integer",
+        "end_byte": "integer"
       }
     },
     {
@@ -214,7 +235,11 @@
         "value": "string",
         "references": "string[]",
         "start_line": "integer",
-        "end_line": "integer"
+        "start_column": "integer",
+        "end_line": "integer",
+        "end_column": "integer",
+        "start_byte": "integer",
+        "end_byte": "integer"
       }
     }
   ],

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

@@ -118,6 +118,13 @@ public static GraphRows project(Analysis analysis, String appName) {
             mp.put("file_key", fileKey);
             mp.put("package", module.getPackageName());
             mp.put("content_hash", module.getContentHash());
+            // The whole file, as `analysis.json` carries it. Canonical decision D1 makes
+            // `module.source` the primary text and every narrower node's text a byte-slice of it, so
+            // dropping it here left the graph holding only the ONE derivation it happens to cache
+            // (`JCallable.code`) and nothing to slice for a body node, field, parameter or the module
+            // itself. Never null: `analysis.json` requires `source`, and `prune` would silently drop
+            // a null, so the two projections would disagree on whether the property exists.
+            mp.put("source", module.getSource() == null ? "" : module.getSource());
             mp.put("_module", fileKey);
             NodeRef mod = b.node(Arrays.asList("JModule"), "id", module.getId(), RowBuilder.prune(mp));
             b.edge("J_HAS_MODULE", app, mod);
@@ -717,12 +724,33 @@ private static String slice(String source, Span span) {
         return new String(bytes, start, end - start, StandardCharsets.UTF_8);
     }
 
+    /**
+     * Flatten a {@code span} the way {@code analysis.json} carries it: line, column and UTF-8 byte
+     * offset for both ends. The byte offsets are the load-bearing pair — the JSON schema defines them
+     * as "[from, to) UTF-8 offsets into module.source, so node text is an O(1) slice", which is only
+     * true on the graph now that {@code :JModule} carries {@code source}. Emitting the line pair
+     * alone left every span resolvable to a line range and to no text, and left a column-precise
+     * position (two nodes on one line) unrepresentable.
+     */
     private static void putLines(Map<String, Object> p, Span span) {
-        if (span != null && span.getStart() != null && span.getStart().length > 0) {
+        if (span == null) {
+            return;
+        }
+        if (span.getStart() != null && span.getStart().length > 0) {
             p.put("start_line", span.getStart()[0]);
+            if (span.getStart().length > 1) {
+                p.put("start_column", span.getStart()[1]);
+            }
         }
-        if (span != null && span.getEnd() != null && span.getEnd().length > 0) {
+        if (span.getEnd() != null && span.getEnd().length > 0) {
             p.put("end_line", span.getEnd()[0]);
+            if (span.getEnd().length > 1) {
+                p.put("end_column", span.getEnd()[1]);
+            }
+        }
+        if (span.getBytes() != null && span.getBytes().length > 1) {
+            p.put("start_byte", span.getBytes()[0]);
+            p.put("end_byte", span.getBytes()[1]);
         }
     }
 

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

@@ -88,8 +88,19 @@ Map<String, String> done() {
         }
     }
 
+    /**
+     * The flattened {@code span} every located node carries. Mirrors {@code analysis.json}'s
+     * {@code span} in full: both ends as line, column and UTF-8 byte offset. {@code start_byte} /
+     * {@code end_byte} are what make a span resolvable to text — the JSON schema defines them as
+     * "[from, to) UTF-8 offsets into module.source, so node text is an O(1) slice" — and they are
+     * only useful because {@code :JModule} now carries {@code source}. codeanalyzer-python and
+     * codeanalyzer-typescript both flatten to the line pair alone today; these four property names
+     * are the ones they must adopt.
+     */
     private static Map<String, String> lines(P p) {
-        return p.put("start_line", "integer").put("end_line", "integer").done();
+        return p.put("start_line", "integer").put("start_column", "integer")
+                .put("end_line", "integer").put("end_column", "integer")
+                .put("start_byte", "integer").put("end_byte", "integer").done();
     }
 
     public static final List<NodeLabel> NODE_LABELS = buildNodeLabels();
@@ -113,9 +124,13 @@ private static List<NodeLabel> buildNodeLabels() {
                         .put("entrypoint_frameworks", "string[]")
                         .put("entrypoint_report_json", "string").done()));
 
+        // `source` is the whole file, matching `analysis.json`'s required `module.source`. It is the
+        // primary text under canonical decision D1 (per-callable `code` was dropped from the JSON on
+        // the grounds that the SDK slices `module.source[span.bytes]`), so the graph must carry it or
+        // no span narrower than a callable can be resolved to text at all.
         n.add(node("JModule", "JModule", "id",
                 new P().put("id", "string").put("file_key", "string").put("package", "string")
-                        .put("content_hash", "string").done()));
+                        .put("content_hash", "string").put("source", "string").done()));
 
         n.add(node("JType", "JSymbol", "id",
                 lines(new P().put("id", "string").put("name", "string").put("kind", "string")

src/test/java/com/ibm/cldk/neo4j/V2Neo4jSchemaConformanceTest.java

@@ -77,6 +77,7 @@ public class V2Neo4jSchemaConformanceTest {
     static Path ARTIFACT_TMP;
 
     private static GraphRows rows;
+    private static Analysis analysis;
 
     private static final Map<String, NodeLabel> BY_LABEL = new HashMap<>();
     private static final Map<String, String> MERGE_OF = new HashMap<>();
@@ -121,10 +122,11 @@ static void project() throws Exception {
             }
         }
 
-        Analysis analysis = V2Emitter.emit(
+        Analysis emitted = V2Emitter.emit(
                 APP_NAME, 3, modules, "test", l2.callGraph(), l2.externalSymbols(),
                 sdg.paramIn, sdg.paramOut, artifacts, dependencies);
-        rows = V2GraphProjector.project(analysis, APP_NAME);
+        rows = V2GraphProjector.project(emitted, APP_NAME);
+        analysis = emitted;
     }
 
     private static String specificLabel(List<String> labels) {
@@ -282,6 +284,33 @@ void l4OverlayProjectsParamAndSummaryEdges() {
     // Repository-artifact layer (Task 7): Artifact/Package/ConfigKey.
     // ------------------------------------------------------------------------------------------
 
+    /**
+     * Every {@code :JModule} carries the same whole-file {@code source} that {@code analysis.json}
+     * carries, because canonical decision D1 makes {@code module.source} the primary text and every
+     * narrower node's text a byte-slice of it. The graph once held only the one derivation it caches
+     * ({@code JCallable.code}) and dropped the primary, which left a body node, field or parameter
+     * span with nothing to resolve against. Asserted per module rather than by presence so the
+     * inversion cannot come back as a truncated or placeholder value.
+     */
+    @Test
+    void everyModuleCarriesTheSameSourceAnalysisJsonCarries() {
+        Map<String, JModule> jsonModules = analysis.getApplication().getSymbolTable();
+        assertFalse(jsonModules.isEmpty(), "fixture projected no modules");
+
+        int checked = 0;
+        for (JModule m : jsonModules.values()) {
+            NodeRow row = findNode("JModule", m.getId());
+            assertNotNull(row, "no :JModule row for " + m.getId());
+            Object projected = row.props.get("source");
+            assertNotNull(projected, "no `source` on :JModule " + m.getId()
+                    + " -- analysis.json requires module.source, so the graph must carry it too");
+            assertEquals(m.getSource(), projected,
+                    "graph `source` differs from analysis.json module.source for " + m.getId());
+            checked++;
+        }
+        assertEquals(jsonModules.size(), checked);
+    }
+
     @Test
     void artifactLayerNodesAreEmitted() {
         boolean sawArtifact = false;