Merge pull request #528 from win4r/fix/incremental-inbound-cross-file-edges

fix(pipeline): preserve inbound cross-file edges on incremental reindex

src/pipeline/pipeline_incremental.c

@@ -296,6 +296,141 @@ static void free_mode_skipped(cbm_file_hash_t *ms, int count) {
     free(ms);
 }
 
+/* ── Inbound cross-file edge preservation (incremental correctness) ──
+ *
+ * The purge step (cbm_gbuf_delete_by_file) removes a changed file's nodes,
+ * and the cascade then drops every edge referencing them — INCLUDING inbound
+ * edges whose source lives in an UNCHANGED file (e.g. StudyService.grade ->
+ * SM2.review, or a Folder -> File containment edge). Because incremental only
+ * re-parses the changed files, the resolution passes never regenerate those
+ * inbound edges, so the graph silently loses cross-file CALLS / USAGE /
+ * CONTAINS_FILE / INHERITS / ... edges on every edit and diverges from a
+ * clean full reindex (which resolves every file).
+ *
+ * Fix: snapshot the inbound cross-file edges into changed files BEFORE the
+ * purge, keyed by endpoint qualified_name (stable across re-parse), then
+ * re-link them AFTER re-resolution + post-passes. Notes:
+ *   - Only edges whose target is in a changed file and whose source is NOT
+ *     are snapshotted; edges out of a changed file are regenerated when that
+ *     file is re-resolved.
+ *   - Edge types recomputed wholesale by post-passes (SIMILAR_TO,
+ *     SEMANTICALLY_RELATED) are skipped — re-linking a stale snapshot could
+ *     add edges a full reindex would not produce.
+ *   - cbm_gbuf_insert_edge dedups, so re-linking an edge the resolver already
+ *     recreated is a harmless no-op.
+ *   - A target whose qualified_name no longer exists (symbol deleted or
+ *     renamed by the edit) is dropped — matching full-reindex semantics. */
+
+typedef struct {
+    char *source_qn;
+    char *target_qn;
+    char *type;
+    char *props;
+} cbm_saved_edge_t;
+
+typedef struct {
+    cbm_gbuf_t *gbuf;
+    CBMHashTable *changed_paths; /* rel_path -> non-NULL sentinel (membership set) */
+    cbm_saved_edge_t *items;
+    int count;
+    int cap;
+} cbm_edge_capture_t;
+
+/* Edge types that must NOT be re-linked from the pre-purge snapshot, because a
+ * full reindex (re)computes them via a pass whose result can differ from the
+ * snapshot — restoring a stale copy could leave wrong properties or even an
+ * edge a full reindex would not produce:
+ *   - SIMILAR_TO / SEMANTICALLY_RELATED: rebuilt wholesale by the incremental
+ *     post-passes (similarity / semantic_edges) over a drifting corpus.
+ *   - FILE_CHANGES_WITH (git-history coupling) and DATA_FLOWS (route data flow):
+ *     produced only by full-pipeline post-passes (githistory / route_nodes)
+ *     that do NOT run during incremental; they remain a known incremental
+ *     limitation rather than something to restore stale.
+ * Every other edge type IS safe to re-link, by one of two routes that both
+ * match a full reindex: edges re-emitted by the per-file resolution passes that
+ * run incrementally (CALLS, USAGE, DEFINES, DEFINES_METHOD, INHERITS,
+ * IMPLEMENTS) are deduped on re-link, while structural containment edges
+ * (CONTAINS_FILE, CONTAINS_FOLDER) — which the full-only structure pass does
+ * NOT regenerate incrementally — are preserved precisely by this snapshot. */
+static bool incr_edge_type_is_recomputed(const char *type) {
+    return type && (strcmp(type, "SIMILAR_TO") == 0 || strcmp(type, "SEMANTICALLY_RELATED") == 0 ||
+                    strcmp(type, "FILE_CHANGES_WITH") == 0 || strcmp(type, "DATA_FLOWS") == 0);
+}
+
+/* cbm_gbuf_foreach_edge visitor: snapshot inbound cross-file edges into
+ * changed files so they survive the purge and can be re-linked afterward. */
+static void incr_capture_inbound_edge(const cbm_gbuf_edge_t *edge, void *userdata) {
+    cbm_edge_capture_t *cap = (cbm_edge_capture_t *)userdata;
+    if (incr_edge_type_is_recomputed(edge->type)) {
+        return;
+    }
+    const cbm_gbuf_node_t *src = cbm_gbuf_find_by_id(cap->gbuf, edge->source_id);
+    const cbm_gbuf_node_t *tgt = cbm_gbuf_find_by_id(cap->gbuf, edge->target_id);
+    if (!src || !tgt || !src->qualified_name || !tgt->qualified_name || !src->file_path ||
+        !tgt->file_path) {
+        return;
+    }
+    /* Keep only edges that the purge would orphan permanently: target is in a
+     * changed file (its node is deleted + re-created), source is NOT (its file
+     * is never re-parsed, so the resolver won't regenerate the edge). */
+    if (!cbm_ht_get(cap->changed_paths, tgt->file_path) ||
+        cbm_ht_get(cap->changed_paths, src->file_path)) {
+        return;
+    }
+    if (cap->count >= cap->cap) {
+        int ncap = (cap->cap > 0) ? cap->cap * PAIR_LEN : CBM_SZ_64;
+        cbm_saved_edge_t *tmp = realloc(cap->items, (size_t)ncap * sizeof(*tmp));
+        if (!tmp) {
+            cbm_log_warn("incremental.edge_snapshot_oom", "captured", itoa_buf(cap->count));
+            return; /* best-effort: stop capturing, keep what we have */
+        }
+        cap->items = tmp;
+        cap->cap = ncap;
+    }
+    cbm_saved_edge_t *s = &cap->items[cap->count];
+    s->source_qn = strdup(src->qualified_name);
+    s->target_qn = strdup(tgt->qualified_name);
+    s->type = strdup(edge->type);
+    s->props = strdup(edge->properties_json ? edge->properties_json : "{}");
+    if (!s->source_qn || !s->target_qn || !s->type || !s->props) {
+        free(s->source_qn);
+        free(s->target_qn);
+        free(s->type);
+        free(s->props);
+        return;
+    }
+    cap->count++;
+}
+
+/* Re-link snapshotted inbound edges to the freshly re-created target nodes.
+ * Returns the number of edges re-linked. */
+static int incr_restore_inbound_edges(cbm_gbuf_t *gbuf, cbm_edge_capture_t *cap) {
+    int restored = 0;
+    for (int i = 0; i < cap->count; i++) {
+        cbm_saved_edge_t *s = &cap->items[i];
+        const cbm_gbuf_node_t *src = cbm_gbuf_find_by_qn(gbuf, s->source_qn);
+        const cbm_gbuf_node_t *tgt = cbm_gbuf_find_by_qn(gbuf, s->target_qn);
+        if (src && tgt) {
+            cbm_gbuf_insert_edge(gbuf, src->id, tgt->id, s->type, s->props);
+            restored++;
+        }
+    }
+    return restored;
+}
+
+static void incr_free_edge_capture(cbm_edge_capture_t *cap) {
+    for (int i = 0; i < cap->count; i++) {
+        free(cap->items[i].source_qn);
+        free(cap->items[i].target_qn);
+        free(cap->items[i].type);
+        free(cap->items[i].props);
+    }
+    free(cap->items);
+    cap->items = NULL;
+    cap->count = 0;
+    cap->cap = 0;
+}
+
 /* ── Persist file hashes ─────────────────────────────────────────── */
 
 /* Persist file hash rows for the current discovery and any mode-skipped
@@ -367,10 +502,27 @@ static void persist_hashes(cbm_store_t *store, const char *project, cbm_file_inf
 
 /* ── Registry seed visitor ────────────────────────────────────────── */
 
-/* Callback for cbm_gbuf_foreach_node: add each node to the registry
- * so the resolver can find cross-file symbols during incremental. */
+/* Labels the full-index definition pass seeds into the registry
+ * (pass_definitions.c — KEEP IN SYNC). Incremental re-resolution must see the
+ * SAME symbol set, or it diverges from a clean full reindex: seeding extra
+ * container nodes (File / Module / Folder / ...) lets a type usage like `Word`
+ * resolve to the same-named Module node instead of the Class node. Only
+ * callable / declared symbols belong in the registry. */
+static bool incr_label_is_registry_symbol(const char *label) {
+    return label && (strcmp(label, "Function") == 0 || strcmp(label, "Method") == 0 ||
+                     strcmp(label, "Class") == 0 || strcmp(label, "Interface") == 0 ||
+                     strcmp(label, "Variable") == 0 || strcmp(label, "Field") == 0);
+}
+
+/* Callback for cbm_gbuf_foreach_node: seed the registry with the existing
+ * project's definition symbols so the resolver can match cross-file symbols
+ * during incremental. Mirrors the full-index registry contents exactly so an
+ * incremental re-resolve picks the same nodes a full reindex would. */
 static void registry_visitor(const cbm_gbuf_node_t *node, void *userdata) {
     cbm_registry_t *r = (cbm_registry_t *)userdata;
+    if (!incr_label_is_registry_symbol(node->label)) {
+        return;
+    }
     cbm_registry_add(r, node->name, node->qualified_name, node->label);
 }
 
@@ -612,6 +764,25 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil
 
     cbm_store_close(store);
 
+    /* Snapshot inbound cross-file edges into changed files BEFORE purging, so
+     * the cascade delete doesn't permanently drop edges whose source lives in
+     * an unchanged (never-re-parsed) file. Re-linked after re-resolution. */
+    cbm_edge_capture_t edge_cap = {0};
+    edge_cap.gbuf = existing;
+    {
+        CBMHashTable *changed_paths = cbm_ht_create(ci > 0 ? (size_t)ci * PAIR_LEN : CBM_SZ_64);
+        for (int i = 0; i < ci; i++) {
+            cbm_ht_set(changed_paths, changed_files[i].rel_path, &changed_files[i]);
+        }
+        edge_cap.changed_paths = changed_paths;
+        cbm_clock_gettime(CLOCK_MONOTONIC, &t);
+        cbm_gbuf_foreach_edge(existing, incr_capture_inbound_edge, &edge_cap);
+        edge_cap.changed_paths = NULL;
+        cbm_ht_free(changed_paths); /* keys borrowed from changed_files; not freed here */
+    }
+    cbm_log_info("incremental.edge_snapshot", "captured", itoa_buf(edge_cap.count), "elapsed_ms",
+                 itoa_buf((int)elapsed_ms(t)));
+
     /* Step 2: Purge stale nodes */
     cbm_clock_gettime(CLOCK_MONOTONIC, &t);
     for (int i = 0; i < ci; i++) {
@@ -660,6 +831,16 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil
     cbm_registry_free(registry);
     cbm_path_alias_collection_free(path_aliases);
 
+    /* Re-link inbound cross-file edges that the purge orphaned. Runs after
+     * re-resolution AND post-passes so the freshly re-created target nodes
+     * exist and nothing downstream clobbers the restored edges; insert_edge
+     * dedups, so any edge the resolver already recreated is a no-op. */
+    cbm_clock_gettime(CLOCK_MONOTONIC, &t);
+    int relinked = incr_restore_inbound_edges(existing, &edge_cap);
+    cbm_log_info("incremental.edge_relink", "relinked", itoa_buf(relinked), "captured",
+                 itoa_buf(edge_cap.count), "elapsed_ms", itoa_buf((int)elapsed_ms(t)));
+    incr_free_edge_capture(&edge_cap);
+
     /* Step 7: Dump to disk (preserves mode-skipped hash rows so the next
      * reindex can correctly classify those files instead of seeing them
      * as never-existed; also exports a fast-mode artifact when one is

tests/test_pipeline.c

@@ -743,6 +743,100 @@ TEST(pipeline_calls_resolution) {
     PASS();
 }
 
+/* True iff a CALLS edge exists from a node named src_name to a node named
+ * tgt_name. Used to assert cross-file call resolution survives a reindex. */
+static bool cross_file_call_exists(cbm_store_t *s, const char *project, const char *src_name,
+                                   const char *tgt_name) {
+    cbm_node_t *srcs = NULL;
+    cbm_node_t *tgts = NULL;
+    int sc = 0;
+    int tc = 0;
+    cbm_store_find_nodes_by_name(s, project, src_name, &srcs, &sc);
+    cbm_store_find_nodes_by_name(s, project, tgt_name, &tgts, &tc);
+    bool found = false;
+    for (int i = 0; i < sc && !found; i++) {
+        cbm_edge_t *edges = NULL;
+        int ec = 0;
+        cbm_store_find_edges_by_source_type(s, srcs[i].id, "CALLS", &edges, &ec);
+        for (int j = 0; j < ec && !found; j++) {
+            for (int k = 0; k < tc; k++) {
+                if (edges[j].target_id == tgts[k].id) {
+                    found = true;
+                    break;
+                }
+            }
+        }
+        if (edges) {
+            cbm_store_free_edges(edges, ec);
+        }
+    }
+    if (srcs) {
+        cbm_store_free_nodes(srcs, sc);
+    }
+    if (tgts) {
+        cbm_store_free_nodes(tgts, tc);
+    }
+    return found;
+}
+
+/* Regression: incremental re-index of an edited file must NOT drop inbound
+ * cross-file CALLS edges whose source lives in an UNCHANGED file.
+ *
+ * Repro: Serve() (pkg/service.go) CALLS Help() (pkg/util/helper.go). Editing
+ * helper.go purges Help's node; before the fix the cascade deleted the
+ * Serve->Help edge and never regenerated it (helper.go's callers are not
+ * re-parsed), so the edge silently vanished on every edit and the incremental
+ * graph diverged from a full reindex. */
+TEST(pipeline_incremental_preserves_cross_file_calls) {
+    if (setup_test_repo() != 0) {
+        FAIL("failed to create temp dir");
+    }
+
+    char db_path[512];
+    snprintf(db_path, sizeof(db_path), "%s/test_incr_calls.db", g_tmpdir);
+
+    /* 1. Full index. */
+    cbm_pipeline_t *p1 = cbm_pipeline_new(g_tmpdir, db_path, CBM_MODE_FULL);
+    ASSERT_NOT_NULL(p1);
+    ASSERT_EQ(cbm_pipeline_run(p1), 0);
+    const char *project = cbm_pipeline_project_name(p1);
+
+    cbm_store_t *s1 = cbm_store_open_path(db_path);
+    ASSERT_NOT_NULL(s1);
+    int calls_before = cbm_store_count_edges_by_type(s1, project, "CALLS");
+    ASSERT_GTE(calls_before, 2); /* main->Serve and Serve->Help at minimum */
+    ASSERT_TRUE(cross_file_call_exists(s1, project, "Serve", "Help"));
+    cbm_store_close(s1);
+    cbm_pipeline_free(p1);
+
+    /* 2. Edit the callee's file so the incremental classifier marks it changed
+     *    (mtime+size differ). Help's symbol + qualified name are unchanged. */
+    char helper[512];
+    snprintf(helper, sizeof(helper), "%s/pkg/util/helper.go", g_tmpdir);
+    ASSERT_EQ(th_append_file(helper, "\n// incremental regression marker\n"), 0);
+
+    /* 3. Re-run on the SAME db_path → auto-routes to incremental re-index. */
+    cbm_pipeline_t *p2 = cbm_pipeline_new(g_tmpdir, db_path, CBM_MODE_FULL);
+    ASSERT_NOT_NULL(p2);
+    ASSERT_EQ(cbm_pipeline_run(p2), 0);
+
+    /* 4. The inbound cross-file CALLS edge must survive and the total CALLS
+     *    count must not regress. (Before the fix: Serve->Help is dropped.)
+     *    NOTE: query with p2's project name — p1 (and the `project` pointer it
+     *    owned) was freed above; p1 and p2 derive the same name from g_tmpdir. */
+    const char *project2 = cbm_pipeline_project_name(p2);
+    cbm_store_t *s2 = cbm_store_open_path(db_path);
+    ASSERT_NOT_NULL(s2);
+    int calls_after = cbm_store_count_edges_by_type(s2, project2, "CALLS");
+    ASSERT_EQ(calls_after, calls_before);
+    ASSERT_TRUE(cross_file_call_exists(s2, project2, "Serve", "Help"));
+    cbm_store_close(s2);
+    cbm_pipeline_free(p2);
+
+    teardown_test_repo();
+    PASS();
+}
+
 /* ── Git history pass tests ─────────────────────────────────────── */
 
 TEST(githistory_is_trackable) {
@@ -5968,6 +6062,7 @@ SUITE(pipeline) {
     RUN_TEST(pipeline_complexity_transitive_loop_depth);
     /* Calls pass */
     RUN_TEST(pipeline_calls_resolution);
+    RUN_TEST(pipeline_incremental_preserves_cross_file_calls);
     /* Git history pass */
     RUN_TEST(githistory_is_trackable);
     RUN_TEST(githistory_compute_coupling);