mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix(graph): stable USAGE-edge ownership for directory-module nodes
Since #667 every file in a Java/Go package shares a directory-based module QN that collides with the pipeline's Folder/Project node. Class-level references resolved their enclosing scope to that shared node, whose file_path was clobbered by each file's always-emitted Module def — last writer wins (sequential upsert) or last-merged worker wins (parallel merge) — so USAGE-edge sources conflated across same-package files and varied run to run. Three coordinated changes: - cbm_gbuf_upsert_node: a Module def colliding with a Project/Folder node no longer updates ANY field (the #667 guard only protected the label). - merge_update_existing: same guard on the parallel worker-gbuf merge path; the ID remap stays outside the guard so worker edges still redirect onto the canonical node. - Source-node finders (pass_usages, pass_parallel, pass_calls) treat a lookup landing on a Folder/Project container as a miss and fall back to the per-file File node (new cbm_pipeline_node_is_dir_container helper). Adds tests/repro/repro_issue787.c: exact-source-set, 5-run stability, and >50-file parallel-path guards (red on the unfixed code in both sequential and parallel flavors). Distilled from #828: the author's commits lacked DCO sign-off trailers and could not be cherry-picked; content applied verbatim plus one clang-format fix in graph_buffer.c. Closes #787. Co-authored-by: Tommy Corbett <spde89@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
9871e7e086
commit
f6b9ac9d65
8 files changed
+593
-22
No files matched your search
@@ -449,6 +449,7 @@ TEST_REPRO_SRCS = \
|
||||
tests/repro/repro_new_cypher_limit_zero.c \
|
||||
tests/repro/repro_issue363.c \
|
||||
tests/repro/repro_issue581.c \
|
||||
tests/repro/repro_issue787.c \
|
||||
tests/repro/repro_invariant_calls.c \
|
||||
tests/repro/repro_invariant_graph.c \
|
||||
tests/repro/repro_invariant_breadth.c \
|
||||
|
||||
@@ -635,25 +635,30 @@ int64_t cbm_gbuf_upsert_node(cbm_gbuf_t *gb, const char *label, const char *name
|
||||
/* Check if node already exists */
|
||||
cbm_gbuf_node_t *existing = cbm_ht_get(gb->node_by_qn, qualified_name);
|
||||
if (existing) {
|
||||
/* Don't let a per-file "Module" def touch a structural directory node
|
||||
* ("Project" root or "Folder"). In a directory-based-module language
|
||||
* (Go/Java) a file's module_qn equals its directory QN: a root file →
|
||||
* the project name (== the "Project" node's QN); a file in pkg/ →
|
||||
* proj.pkg (== the "pkg/" Folder node's QN). Its always-emitted Module
|
||||
* def collides here; the directory node is the package/module container
|
||||
* and must keep its structural label AND its own name/file_path/range.
|
||||
* Updating those in place set the shared node's file_path to whichever
|
||||
* same-package file happened to be processed LAST (worker-order
|
||||
* dependent) — the nondeterministic file attribution behind #787 — and
|
||||
* left the Folder node exposed to delete-nodes-by-file on incremental
|
||||
* reindex of that file. Skip the update entirely. (Both the sequential
|
||||
* upsert and the parallel local-gbuf merge route through this function.) */
|
||||
if (existing->label && label && strcmp(label, "Module") == 0 &&
|
||||
(strcmp(existing->label, "Project") == 0 || strcmp(existing->label, "Folder") == 0)) {
|
||||
return existing->id;
|
||||
}
|
||||
/* Update in-place. name/properties are strdup'd BEFORE freeing old ones
|
||||
* (callers may pass existing->name as an argument). label/file_path are
|
||||
* interned: gb_intern returns a stable pool pointer (idempotent even when
|
||||
* label == existing->label), so the old value is replaced, never freed. */
|
||||
char *new_name = heap_strdup(name);
|
||||
char *new_props = properties_json ? heap_strdup(properties_json) : NULL;
|
||||
/* Don't let a per-file "Module" def downgrade a structural directory node
|
||||
* ("Project" root or "Folder"). In a directory-based-module language
|
||||
* (Go/Java) a file's module_qn equals its directory QN: a root file →
|
||||
* the project name (== the "Project" node's QN); a file in pkg/ →
|
||||
* proj.pkg (== the "pkg/" Folder node's QN). Its always-emitted Module
|
||||
* def collides here; the directory node is the package/module container
|
||||
* and must keep its structural label. (Both the sequential upsert and the
|
||||
* parallel local-gbuf merge route through this function.) */
|
||||
if (!(existing->label && label && strcmp(label, "Module") == 0 &&
|
||||
(strcmp(existing->label, "Project") == 0 ||
|
||||
strcmp(existing->label, "Folder") == 0))) {
|
||||
existing->label = (char *)gb_intern(gb, label);
|
||||
}
|
||||
existing->label = (char *)gb_intern(gb, label);
|
||||
free(existing->name);
|
||||
existing->name = new_name;
|
||||
existing->file_path = (char *)gb_intern(gb, file_path);
|
||||
@@ -1112,15 +1117,30 @@ static void free_remap_entry(const char *key, void *val, void *ud) {
|
||||
* label/file_path are re-interned into dst's pool (sn's pointers belong to src). */
|
||||
static void merge_update_existing(cbm_gbuf_t *dst, cbm_gbuf_node_t *existing,
|
||||
const cbm_gbuf_node_t *sn, CBMHashTable **remap) {
|
||||
existing->label = (char *)gb_intern(dst, sn->label);
|
||||
free(existing->name);
|
||||
existing->name = heap_strdup(sn->name);
|
||||
existing->file_path = (char *)gb_intern(dst, sn->file_path);
|
||||
existing->start_line = sn->start_line;
|
||||
existing->end_line = sn->end_line;
|
||||
if (sn->properties_json) {
|
||||
free(existing->properties_json);
|
||||
existing->properties_json = heap_strdup(sn->properties_json);
|
||||
/* Same guard as cbm_gbuf_upsert_node: a per-file "Module" def coming from a
|
||||
* worker-local gbuf must not touch the structural directory node ("Project"
|
||||
* root or "Folder") that shares its QN in a directory-based-module language
|
||||
* (Java/Go). pass_structure seeds Folder/Project nodes on the MAIN gbuf
|
||||
* before the parallel extract, so every worker's always-emitted Module def
|
||||
* for that package collides here; unconditional "src wins" relabelled the
|
||||
* directory node to Module and set its file_path to whichever worker merged
|
||||
* LAST — the nondeterministic USAGE-source misattribution of #787. Keep the
|
||||
* structural node intact; the ID remap below still redirects the worker's
|
||||
* edges onto the canonical node. */
|
||||
bool module_on_container =
|
||||
existing->label && sn->label && strcmp(sn->label, "Module") == 0 &&
|
||||
(strcmp(existing->label, "Project") == 0 || strcmp(existing->label, "Folder") == 0);
|
||||
if (!module_on_container) {
|
||||
existing->label = (char *)gb_intern(dst, sn->label);
|
||||
free(existing->name);
|
||||
existing->name = heap_strdup(sn->name);
|
||||
existing->file_path = (char *)gb_intern(dst, sn->file_path);
|
||||
existing->start_line = sn->start_line;
|
||||
existing->end_line = sn->end_line;
|
||||
if (sn->properties_json) {
|
||||
free(existing->properties_json);
|
||||
existing->properties_json = heap_strdup(sn->properties_json);
|
||||
}
|
||||
}
|
||||
|
||||
if (sn->id != existing->id) {
|
||||
|
||||
@@ -409,6 +409,12 @@ static const cbm_gbuf_node_t *calls_find_source(cbm_pipeline_ctx_t *ctx, const c
|
||||
const cbm_gbuf_node_t *src = NULL;
|
||||
if (enclosing_qn) {
|
||||
src = cbm_gbuf_find_by_qn(ctx->gbuf, enclosing_qn);
|
||||
/* A class-level call in a directory-module language carries the
|
||||
* DIRECTORY module QN, which hits the shared Folder/Project node —
|
||||
* attribute to this file's File node instead (#787). */
|
||||
if (cbm_pipeline_node_is_dir_container(src)) {
|
||||
src = NULL;
|
||||
}
|
||||
}
|
||||
if (!src) {
|
||||
char *fqn = cbm_pipeline_fqn_compute(ctx->project_name, rel, "__file__");
|
||||
|
||||
@@ -1828,6 +1828,12 @@ static const cbm_gbuf_node_t *find_source_node(const cbm_gbuf_t *gbuf, const cha
|
||||
const cbm_gbuf_node_t *src = NULL;
|
||||
if (enclosing_qn) {
|
||||
src = cbm_gbuf_find_by_qn(gbuf, enclosing_qn);
|
||||
/* A class-level reference in a directory-module language carries the
|
||||
* DIRECTORY module QN, which hits the shared Folder/Project node —
|
||||
* attribute to this file's File node instead (#787). */
|
||||
if (cbm_pipeline_node_is_dir_container(src)) {
|
||||
src = NULL;
|
||||
}
|
||||
}
|
||||
if (!src) {
|
||||
char *file_qn = cbm_pipeline_fqn_compute(project, rel, "__file__");
|
||||
|
||||
@@ -197,6 +197,12 @@ static const cbm_gbuf_node_t *find_enclosing_node(cbm_pipeline_ctx_t *ctx, const
|
||||
const cbm_gbuf_node_t *node = NULL;
|
||||
if (func_qn && func_qn[0]) {
|
||||
node = cbm_gbuf_find_by_qn(ctx->gbuf, func_qn);
|
||||
/* A class-level reference in a directory-module language carries the
|
||||
* DIRECTORY module QN, which hits the shared Folder/Project node —
|
||||
* attribute to this file's File node instead (#787). */
|
||||
if (cbm_pipeline_node_is_dir_container(node)) {
|
||||
node = NULL;
|
||||
}
|
||||
}
|
||||
if (!node) {
|
||||
char *file_qn = cbm_pipeline_fqn_compute(ctx->project_name, rel_path, "__file__");
|
||||
|
||||
@@ -34,6 +34,19 @@
|
||||
* out_sz >= strlen(in) + 1 always suffices. Returns out. */
|
||||
const char *cbm_route_canon_path(const char *in, char *out, size_t out_sz);
|
||||
|
||||
/* True when a graph node is a structural directory container (Folder/Project)
|
||||
* rather than a code node. In a directory-based-module language (Java/Go, see
|
||||
* cbm_lang_module_is_dir) a file's module QN equals its directory QN, so an
|
||||
* enclosing-scope lookup for a CLASS-LEVEL usage/call (enclosing_func_qn ==
|
||||
* module_qn) resolves to the ONE Folder/Project node shared by every file in
|
||||
* that package. Sourcing an edge there conflates all same-package files into a
|
||||
* single source node with an arbitrary file_path (#787). Source-node finders
|
||||
* must treat such a hit as a miss and fall back to the per-file File node. */
|
||||
static inline bool cbm_pipeline_node_is_dir_container(const cbm_gbuf_node_t *node) {
|
||||
return node && node->label &&
|
||||
(strcmp(node->label, "Folder") == 0 || strcmp(node->label, "Project") == 0);
|
||||
}
|
||||
|
||||
/* Time unit conversions */
|
||||
#define CBM_NS_PER_SEC 1000000000LL
|
||||
#define CBM_US_PER_SEC 1000000LL
|
||||
|
||||
@@ -0,0 +1,517 @@
|
||||
/*
|
||||
* repro_issue787.c -- Reproduce-first / regression guard for bug #787.
|
||||
*
|
||||
* Issue #787: "Nondeterministic Java USAGE-edge misattribution across
|
||||
* same-package files — USAGE sources vary per run"
|
||||
*
|
||||
* ROOT CAUSE (introduced by PR #667, merge commit 36d83280):
|
||||
* PR #667 switched Java/Go module QNs from filename-stem-based
|
||||
* ("proj.pkg.OwnerController") to DIRECTORY-based ("proj.pkg"), so all
|
||||
* files in the same Java package share a single module QN. This QN also
|
||||
* collides with the pipeline's Folder node for that directory.
|
||||
*
|
||||
* When a source-node finder (find_enclosing_node in pass_usages.c,
|
||||
* find_source_node in pass_parallel.c, calls_find_source in pass_calls.c)
|
||||
* resolves a CLASS-LEVEL usage (e.g., a field type reference
|
||||
* `private IRepository repo;`), the enclosing_func_qn equals the shared
|
||||
* directory module QN. Looking that QN up in the graph buffer returns the
|
||||
* ONE Folder/Project node shared by every file in the package. That shared
|
||||
* node's file_path was clobbered by each file's always-emitted Module def —
|
||||
* through TWO code paths:
|
||||
* - sequential: cbm_gbuf_upsert_node updated name/file_path/range in
|
||||
* place (its #667 guard only protected the label);
|
||||
* - parallel: merge_update_existing applied unconditional "src wins"
|
||||
* when merging worker-local gbufs, even relabelling the Folder node to
|
||||
* Module — and worker merge ORDER varies per run (the race flavor).
|
||||
* USAGE edges from all same-package class-level references then share a
|
||||
* single source node whose file_path is whichever file won the last write.
|
||||
*
|
||||
* Spring-petclinic oracle:
|
||||
* MATCH (c {name:'OwnerRepository'})<-[r:USAGE]-(m) RETURN m.file_path
|
||||
* should yield exactly 7 distinct files; HEAD (dcf98dc) returns 4-6 with
|
||||
* bogus entries varying per run.
|
||||
*
|
||||
* FIX (three co-ordinated parts):
|
||||
* 1. graph_buffer.c cbm_gbuf_upsert_node: a Module def colliding with a
|
||||
* Project/Folder node no longer updates ANY field (was: label only).
|
||||
* 2. graph_buffer.c merge_update_existing: same guard on the parallel
|
||||
* worker-gbuf merge path.
|
||||
* 3. pipeline finders (pass_usages/pass_parallel/pass_calls): a lookup that
|
||||
* lands on a structural directory container (Folder/Project — see
|
||||
* cbm_pipeline_node_is_dir_container) is treated as a miss, falling
|
||||
* through to the per-file File node, so every file's class-level usages
|
||||
* attribute to that file's unique File node.
|
||||
*
|
||||
* FIXTURE DESIGN (minimal spring-petclinic analog):
|
||||
* Package owner/ — three Java files:
|
||||
* IRepository.java defines interface IRepository (the used type)
|
||||
* ServiceA.java field `IRepository repo;` (class-level USAGE)
|
||||
* ServiceB.java field `IRepository repo;` (class-level USAGE)
|
||||
* Package web/ — one Java file:
|
||||
* WebController.java field `IRepository repo;` (cross-package USAGE)
|
||||
*
|
||||
* Correct USAGE edges to IRepository must source from EXACTLY:
|
||||
* owner/ServiceA.java, owner/ServiceB.java, web/WebController.java
|
||||
* (3 distinct source files, each appearing exactly once).
|
||||
*
|
||||
* Pre-fix: owner/ServiceA.java and owner/ServiceB.java share the same
|
||||
* source node (the "proj.owner" Folder/Module node), so the query returns
|
||||
* at most 2 distinct file_paths (and one of the two is random).
|
||||
* Post-fix: each file gets its own source → 3 distinct, stable file_paths.
|
||||
*
|
||||
* WHY RED on buggy HEAD:
|
||||
* The stability assertion (same source set across N runs) fires because the
|
||||
* shared Folder node's file_path changes between indexing passes (different
|
||||
* file write orders can produce different results each run). The exactness
|
||||
* assertion (set equals the expected three files) fires because the owner/
|
||||
* package's two callers are conflated into one node whose file_path is
|
||||
* whichever of ServiceA.java / ServiceB.java was extracted last.
|
||||
*
|
||||
* NOTE: this test uses N=5 independent full-pipeline index+query rounds so
|
||||
* that any ordering-based nondeterminism has several opportunities to
|
||||
* surface. Each round opens a fresh in-memory project (different tmpdir →
|
||||
* different project name → completely isolated store).
|
||||
*/
|
||||
|
||||
#include <foundation/compat.h>
|
||||
#include "test_framework.h"
|
||||
#include "repro_harness.h"
|
||||
|
||||
#include <string.h>
|
||||
#include <stdlib.h>
|
||||
#include <stdio.h>
|
||||
|
||||
/* ── Fixture files ──────────────────────────────────────────────────────── */
|
||||
|
||||
/* IRepository: the type that will be a USAGE target. */
|
||||
static const char k_irepository[] =
|
||||
"package org.example.owner;\n"
|
||||
"\n"
|
||||
"public interface IRepository {\n"
|
||||
" void save(Object o);\n"
|
||||
" Object findById(int id);\n"
|
||||
"}\n";
|
||||
|
||||
/* ServiceA: reference to IRepository ONLY at class level (field declaration —
|
||||
* no constructor parameter). A class-level reference's enclosing scope is the
|
||||
* MODULE QN, which for Java is the package directory QN shared by every file
|
||||
* in owner/ — the collision at the heart of #787. Keeping the field as the
|
||||
* file's only IRepository reference makes the buggy collapse deterministic:
|
||||
* ServiceA.java and ServiceB.java always conflate to ONE source node
|
||||
* regardless of file processing order, so the distinct-source-file set is
|
||||
* short in every ordering (no false green when the "right" file happens to
|
||||
* win the last-writer race). */
|
||||
static const char k_service_a[] =
|
||||
"package org.example.owner;\n"
|
||||
"\n"
|
||||
"public class ServiceA {\n"
|
||||
" private IRepository repo;\n"
|
||||
"\n"
|
||||
" public void doA() {\n"
|
||||
" repo.save(\"a\");\n"
|
||||
" }\n"
|
||||
"}\n";
|
||||
|
||||
/* ServiceB: same shape as ServiceA — class-level-only reference. */
|
||||
static const char k_service_b[] =
|
||||
"package org.example.owner;\n"
|
||||
"\n"
|
||||
"public class ServiceB {\n"
|
||||
" private IRepository repo;\n"
|
||||
"\n"
|
||||
" public void doB() {\n"
|
||||
" repo.save(\"b\");\n"
|
||||
" }\n"
|
||||
"}\n";
|
||||
|
||||
/* WebController: cross-package reference to IRepository. */
|
||||
static const char k_web_controller[] =
|
||||
"package org.example.web;\n"
|
||||
"\n"
|
||||
"import org.example.owner.IRepository;\n"
|
||||
"\n"
|
||||
"public class WebController {\n"
|
||||
" private IRepository repo;\n"
|
||||
"\n"
|
||||
" public WebController(IRepository repo) {\n"
|
||||
" this.repo = repo;\n"
|
||||
" }\n"
|
||||
"\n"
|
||||
" public void handle() {\n"
|
||||
" repo.save(\"web\");\n"
|
||||
" }\n"
|
||||
"}\n";
|
||||
|
||||
static const RFile k_files[] = {
|
||||
{"owner/IRepository.java", k_irepository},
|
||||
{"owner/ServiceA.java", k_service_a},
|
||||
{"owner/ServiceB.java", k_service_b},
|
||||
{"web/WebController.java", k_web_controller},
|
||||
};
|
||||
static const int k_nfiles = (int)(sizeof(k_files) / sizeof(k_files[0]));
|
||||
|
||||
/* The full pipeline switches to the parallel (worker) path above
|
||||
* MIN_FILES_FOR_PARALLEL=50 files (pipeline.c). The bug has TWO faces:
|
||||
* - sequential: cbm_gbuf_upsert_node clobbered the Folder node in place;
|
||||
* - parallel: merge_update_existing clobbered it during worker-local gbuf
|
||||
* merge (worker order → run-to-run nondeterminism).
|
||||
* The 4-file fixture only exercises the sequential face, so a second fixture
|
||||
* pads the same core files with FILLER_COUNT inert same-package classes to
|
||||
* push the file count over the threshold and exercise the merge face too. */
|
||||
enum { REPRO787_FILLER_COUNT = 60, REPRO787_NAME_SZ = 64, REPRO787_BODY_SZ = 192 };
|
||||
|
||||
/* Build the >50-file fixture into files[]/name_bufs[]/body_bufs[] (caller
|
||||
* arrays sized k_nfiles + REPRO787_FILLER_COUNT). Returns total file count.
|
||||
* Caller frees name_bufs/body_bufs entries [k_nfiles..total). */
|
||||
static int build_parallel_fixture(RFile *files, char **name_bufs, char **body_bufs) {
|
||||
int n = 0;
|
||||
for (int i = 0; i < k_nfiles; i++) {
|
||||
files[n++] = k_files[i];
|
||||
}
|
||||
for (int i = 0; i < REPRO787_FILLER_COUNT; i++) {
|
||||
char *name = malloc(REPRO787_NAME_SZ);
|
||||
char *body = malloc(REPRO787_BODY_SZ);
|
||||
if (!name || !body) {
|
||||
free(name);
|
||||
free(body);
|
||||
break;
|
||||
}
|
||||
snprintf(name, REPRO787_NAME_SZ, "owner/Filler%02d.java", i);
|
||||
snprintf(body, REPRO787_BODY_SZ,
|
||||
"package org.example.owner;\n"
|
||||
"\n"
|
||||
"public class Filler%02d {\n"
|
||||
" private int value%02d;\n"
|
||||
"}\n",
|
||||
i, i);
|
||||
name_bufs[n] = name;
|
||||
body_bufs[n] = body;
|
||||
files[n].name = name;
|
||||
files[n].content = body;
|
||||
n++;
|
||||
}
|
||||
return n;
|
||||
}
|
||||
|
||||
/* ── Helper: collect USAGE source file_paths for IRepository ───────────── */
|
||||
|
||||
/*
|
||||
* collect_usage_sources: index the fixture once, find the IRepository node,
|
||||
* walk all USAGE edges that target it, and write the source file_paths
|
||||
* (up to `cap`) into `out`. DISTINCT-file (set) semantics match the petclinic
|
||||
* oracle ("exactly 7 user files"): one file may legitimately contribute several
|
||||
* USAGE edges from different source nodes (a method-scoped reference sources
|
||||
* from the Method node, a class-level field reference from the file-scope
|
||||
* node) — ownership is per FILE, so duplicate paths are collapsed. Returns the
|
||||
* number of distinct sources found (may be truncated by `cap`).
|
||||
*
|
||||
* The caller is responsible for free()ing each string in `out[0..return-1]`.
|
||||
* Returns -1 on setup failure.
|
||||
*/
|
||||
static int collect_usage_sources_n(const RFile *files, int nfiles, char **out, int cap) {
|
||||
RProj lp;
|
||||
cbm_store_t *store = rh_index_files(&lp, files, nfiles);
|
||||
if (!store) {
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Locate the IRepository node by name. */
|
||||
cbm_node_t *candidates = NULL;
|
||||
int ncand = 0;
|
||||
int rc = cbm_store_find_nodes_by_name(store, lp.project, "IRepository",
|
||||
&candidates, &ncand);
|
||||
if (rc != CBM_STORE_OK || ncand == 0) {
|
||||
cbm_store_free_nodes(candidates, ncand);
|
||||
rh_cleanup(&lp, store);
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Pick the Interface / Class node (label check). */
|
||||
int64_t target_id = 0;
|
||||
for (int i = 0; i < ncand; i++) {
|
||||
const char *lbl = candidates[i].label ? candidates[i].label : "";
|
||||
if (strcmp(lbl, "Interface") == 0 || strcmp(lbl, "Class") == 0) {
|
||||
target_id = candidates[i].id;
|
||||
break;
|
||||
}
|
||||
}
|
||||
cbm_store_free_nodes(candidates, ncand);
|
||||
|
||||
if (!target_id) {
|
||||
rh_cleanup(&lp, store);
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Walk inbound USAGE edges. */
|
||||
cbm_edge_t *edges = NULL;
|
||||
int nedges = 0;
|
||||
rc = cbm_store_find_edges_by_target_type(store, target_id, "USAGE", &edges, &nedges);
|
||||
if (rc != CBM_STORE_OK) {
|
||||
rh_cleanup(&lp, store);
|
||||
return -1;
|
||||
}
|
||||
|
||||
int found = 0;
|
||||
for (int i = 0; i < nedges && found < cap; i++) {
|
||||
cbm_node_t src_node;
|
||||
if (cbm_store_find_node_by_id(store, edges[i].source_id, &src_node) != CBM_STORE_OK) {
|
||||
continue;
|
||||
}
|
||||
if (!src_node.file_path || !src_node.file_path[0]) {
|
||||
continue;
|
||||
}
|
||||
/* Set semantics: skip a file_path already collected. */
|
||||
int dup = 0;
|
||||
for (int j = 0; j < found; j++) {
|
||||
if (strcmp(out[j], src_node.file_path) == 0) {
|
||||
dup = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (dup) {
|
||||
continue;
|
||||
}
|
||||
out[found++] = strdup(src_node.file_path);
|
||||
}
|
||||
|
||||
cbm_store_free_edges(edges, nedges);
|
||||
rh_cleanup(&lp, store);
|
||||
return found;
|
||||
}
|
||||
|
||||
/* Check that paths[] contains exactly the expected file suffixes, once each.
|
||||
* Returns 1 if all expected suffixes appear exactly once, 0 otherwise.
|
||||
* Prints a diagnostic on mismatch. */
|
||||
static int check_sources_exact(char **paths, int count,
|
||||
const char **expected, int nexpected) {
|
||||
if (count != nexpected) {
|
||||
printf(" source count %d != expected %d\n", count, nexpected);
|
||||
for (int i = 0; i < count; i++) {
|
||||
printf(" got: %s\n", paths[i] ? paths[i] : "(null)");
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
for (int e = 0; e < nexpected; e++) {
|
||||
int seen = 0;
|
||||
for (int i = 0; i < count; i++) {
|
||||
if (paths[i] && strstr(paths[i], expected[e])) {
|
||||
seen++;
|
||||
}
|
||||
}
|
||||
if (seen != 1) {
|
||||
printf(" expected suffix '%s' appears %d time(s) (want 1)\n",
|
||||
expected[e], seen);
|
||||
for (int i = 0; i < count; i++) {
|
||||
printf(" got: %s\n", paths[i] ? paths[i] : "(null)");
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
return 1;
|
||||
}
|
||||
|
||||
/* Expected distinct USAGE source files (shared by all three tests). */
|
||||
static const char *k_expected[] = {
|
||||
"owner/ServiceA.java",
|
||||
"owner/ServiceB.java",
|
||||
"web/WebController.java",
|
||||
};
|
||||
static const int k_nexpected = (int)(sizeof(k_expected) / sizeof(k_expected[0]));
|
||||
|
||||
/* ── Tests ──────────────────────────────────────────────────────────────── */
|
||||
|
||||
/*
|
||||
* repro_issue787_usage_exact_sources
|
||||
*
|
||||
* Single-run correctness check (sequential pipeline path): after indexing
|
||||
* the 4-file fixture exactly once, the USAGE edges targeting IRepository
|
||||
* must source from exactly the three expected files.
|
||||
*
|
||||
* RED on buggy HEAD: the two same-package callers (ServiceA.java, ServiceB.java)
|
||||
* collapse onto a single shared Folder/Module node whose file_path is
|
||||
* whichever of the package's files was processed last. The distinct source
|
||||
* set has 2 entries instead of 3 (possibly including a bogus non-referencing
|
||||
* file such as IRepository.java itself).
|
||||
*/
|
||||
TEST(repro_issue787_usage_exact_sources) {
|
||||
#define MAX_SRCS 16
|
||||
char *paths[MAX_SRCS];
|
||||
memset(paths, 0, sizeof(paths));
|
||||
|
||||
int count = collect_usage_sources_n(k_files, k_nfiles, paths, MAX_SRCS);
|
||||
if (count < 0) {
|
||||
FAIL("fixture indexing or IRepository lookup failed");
|
||||
}
|
||||
|
||||
int ok = check_sources_exact(paths, count, k_expected, k_nexpected);
|
||||
|
||||
for (int i = 0; i < MAX_SRCS; i++) {
|
||||
free(paths[i]);
|
||||
paths[i] = NULL;
|
||||
}
|
||||
#undef MAX_SRCS
|
||||
|
||||
if (!ok) {
|
||||
FAIL("USAGE edge source file_paths do not match expected set");
|
||||
}
|
||||
PASS();
|
||||
}
|
||||
|
||||
/*
|
||||
* repro_issue787_usage_stable_across_runs
|
||||
*
|
||||
* Stability check: index the same fixture N=5 times independently (each
|
||||
* run uses a fresh tmpdir + fresh DB), collect the USAGE source file_paths,
|
||||
* and assert they are IDENTICAL across all runs.
|
||||
*
|
||||
* RED on buggy HEAD: the shared Folder/Module node for "proj.owner" is
|
||||
* upserted once per file in the package; its file_path is set to the last
|
||||
* file written. Across runs the write order may differ (filesystem readdir
|
||||
* order, thread scheduling, etc.), so the collapsed source file_path varies,
|
||||
* and the set of paths returned by the query changes run to run.
|
||||
*
|
||||
* Even if the first-run result happened to be correct, the stability
|
||||
* assertion would still catch a subsequent run that diverged.
|
||||
*
|
||||
* N=5 gives five independent opportunities to expose nondeterminism without
|
||||
* making the test prohibitively slow (each index+query round is < 1 s).
|
||||
*/
|
||||
TEST(repro_issue787_usage_stable_across_runs) {
|
||||
#define N_RUNS 5
|
||||
#define MAX_SRCS 16
|
||||
|
||||
/* Collect results across N runs. */
|
||||
char *all_paths[N_RUNS][MAX_SRCS];
|
||||
int all_counts[N_RUNS];
|
||||
memset(all_paths, 0, sizeof(all_paths));
|
||||
memset(all_counts, 0, sizeof(all_counts));
|
||||
|
||||
for (int run = 0; run < N_RUNS; run++) {
|
||||
all_counts[run] = collect_usage_sources_n(k_files, k_nfiles, all_paths[run], MAX_SRCS);
|
||||
if (all_counts[run] < 0) {
|
||||
/* Free already-allocated strings from prior runs. */
|
||||
for (int r = 0; r <= run; r++) {
|
||||
for (int i = 0; i < MAX_SRCS; i++) { free(all_paths[r][i]); }
|
||||
}
|
||||
FAIL("fixture indexing or IRepository lookup failed on one run");
|
||||
}
|
||||
}
|
||||
|
||||
/* Assert all runs agree with run 0. */
|
||||
int stable = 1;
|
||||
for (int run = 1; run < N_RUNS; run++) {
|
||||
if (all_counts[run] != all_counts[0]) {
|
||||
printf(" run %d returned %d sources, run 0 returned %d\n",
|
||||
run, all_counts[run], all_counts[0]);
|
||||
stable = 0;
|
||||
continue;
|
||||
}
|
||||
/* Each path in run 0 must appear (by suffix) in run `run`. */
|
||||
for (int i = 0; i < all_counts[0]; i++) {
|
||||
if (!all_paths[0][i]) {
|
||||
continue;
|
||||
}
|
||||
int found = 0;
|
||||
for (int j = 0; j < all_counts[run]; j++) {
|
||||
if (all_paths[run][j] &&
|
||||
strcmp(all_paths[0][i], all_paths[run][j]) == 0) {
|
||||
found = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!found) {
|
||||
printf(" run %d missing source '%s' (present in run 0)\n",
|
||||
run, all_paths[0][i]);
|
||||
stable = 0;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (int r = 0; r < N_RUNS; r++) {
|
||||
for (int i = 0; i < MAX_SRCS; i++) { free(all_paths[r][i]); }
|
||||
}
|
||||
|
||||
if (!stable) {
|
||||
FAIL("USAGE source file_paths differ across runs — nondeterministic");
|
||||
}
|
||||
|
||||
/* Also check correctness of run 0. */
|
||||
char *paths0[MAX_SRCS];
|
||||
memset(paths0, 0, sizeof(paths0));
|
||||
int count0 = collect_usage_sources_n(k_files, k_nfiles, paths0, MAX_SRCS);
|
||||
if (count0 < 0) {
|
||||
FAIL("verification re-index failed");
|
||||
}
|
||||
int ok = check_sources_exact(paths0, count0, k_expected, k_nexpected);
|
||||
for (int i = 0; i < MAX_SRCS; i++) { free(paths0[i]); }
|
||||
|
||||
if (!ok) {
|
||||
FAIL("USAGE sources are stable but do not match expected set");
|
||||
}
|
||||
|
||||
#undef N_RUNS
|
||||
#undef MAX_SRCS
|
||||
PASS();
|
||||
}
|
||||
|
||||
/*
|
||||
* repro_issue787_usage_stable_parallel
|
||||
*
|
||||
* PARALLEL-path check: same core fixture padded past MIN_FILES_FOR_PARALLEL
|
||||
* (>50 files) so index_repository takes the worker/merge path, repeated N=3
|
||||
* times. Asserts the distinct USAGE source set equals the expected files on
|
||||
* every run.
|
||||
*
|
||||
* RED on buggy HEAD via a DIFFERENT mechanism than the sequential tests:
|
||||
* merge_update_existing (graph_buffer.c) applied unconditional "src wins" when
|
||||
* a worker-local gbuf's per-file Module def (directory QN) collided with the
|
||||
* main gbuf's Folder node, relabelling it Module and setting its file_path to
|
||||
* whichever worker merged last — worker scheduling makes the collapsed source
|
||||
* file vary RUN TO RUN (the race flavor reported in #787).
|
||||
*/
|
||||
TEST(repro_issue787_usage_stable_parallel) {
|
||||
#define N_RUNS_PAR 3
|
||||
#define MAX_SRCS 80
|
||||
RFile files[(int)(sizeof(k_files) / sizeof(k_files[0])) + REPRO787_FILLER_COUNT];
|
||||
char *name_bufs[(int)(sizeof(k_files) / sizeof(k_files[0])) + REPRO787_FILLER_COUNT];
|
||||
char *body_bufs[(int)(sizeof(k_files) / sizeof(k_files[0])) + REPRO787_FILLER_COUNT];
|
||||
memset(name_bufs, 0, sizeof(name_bufs));
|
||||
memset(body_bufs, 0, sizeof(body_bufs));
|
||||
int total = build_parallel_fixture(files, name_bufs, body_bufs);
|
||||
|
||||
int all_ok = 1;
|
||||
for (int run = 0; run < N_RUNS_PAR && all_ok; run++) {
|
||||
char *paths[MAX_SRCS];
|
||||
memset(paths, 0, sizeof(paths));
|
||||
int count = collect_usage_sources_n(files, total, paths, MAX_SRCS);
|
||||
if (count < 0) {
|
||||
all_ok = 0;
|
||||
printf(" parallel run %d: indexing or lookup failed\n", run + 1);
|
||||
} else if (!check_sources_exact(paths, count, k_expected, k_nexpected)) {
|
||||
printf(" ^ parallel run %d\n", run + 1);
|
||||
all_ok = 0;
|
||||
}
|
||||
for (int i = 0; i < MAX_SRCS; i++) {
|
||||
free(paths[i]);
|
||||
}
|
||||
}
|
||||
|
||||
for (int i = 0; i < total; i++) {
|
||||
free(name_bufs[i]);
|
||||
free(body_bufs[i]);
|
||||
}
|
||||
#undef N_RUNS_PAR
|
||||
#undef MAX_SRCS
|
||||
|
||||
if (!all_ok) {
|
||||
FAIL("parallel-path USAGE sources wrong or unstable");
|
||||
}
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── Suite ──────────────────────────────────────────────────────────────── */
|
||||
SUITE(repro_issue787) {
|
||||
RUN_TEST(repro_issue787_usage_exact_sources);
|
||||
RUN_TEST(repro_issue787_usage_stable_across_runs);
|
||||
RUN_TEST(repro_issue787_usage_stable_parallel);
|
||||
}
|
||||
@@ -78,6 +78,7 @@ extern void suite_repro_issue221(void);
|
||||
extern void suite_repro_issue548(void);
|
||||
extern void suite_repro_issue363(void);
|
||||
extern void suite_repro_issue581(void);
|
||||
extern void suite_repro_issue787(void);
|
||||
/* NEW bugs found by the discovery sweep */
|
||||
extern void suite_repro_new_ts_class_field_arrow(void);
|
||||
extern void suite_repro_new_py_tuple_unpack(void);
|
||||
@@ -152,6 +153,7 @@ int main(void) {
|
||||
RUN_SUITE(repro_new_cypher_limit_zero);
|
||||
RUN_SUITE(repro_issue363);
|
||||
RUN_SUITE(repro_issue581);
|
||||
RUN_SUITE(repro_issue787);
|
||||
RUN_SUITE(repro_invariant_calls);
|
||||
RUN_SUITE(repro_invariant_graph);
|
||||
RUN_SUITE(repro_invariant_breadth);
|
||||
|
||||
Reference in new issue
Block a user