fix(pipeline): resolve LSP cross-file QN mismatch via project-prefix fallback
Per-file py_lsp emits resolved_calls.callee_qn as the raw import-module path (e.g. `greeter.Greeter` from `from greeter import Greeter`) rather than the project-qualified QN the gbuf stores (`<project>.greeter.Greeter`). Before this change, the LSP match succeeded (lsp_overrides counter incremented) but the downstream cbm_gbuf_find_by_qn lookup missed silently and the edge was dropped — the failure mode the new lsp_overrides telemetry was designed to surface. The LSP layer can't tell in-project imports (qualify) from external imports (don't qualify, e.g. `os.path`) without consulting the gbuf, which is built downstream. So normalise at the consumer instead: try the LSP-emitted QN as-is first; on miss, retry with `<project>.<callee_qn>`. If that also misses, drop the edge — same as before, target is external/unindexed. New helper cbm_pipeline_lsp_target_node in lsp_resolve.h is shared by both pipelines (sequential pass_calls.c and parallel pass_parallel.c) so they continue to admit identical sets of LSP overrides. The helper also canonicalises res.qualified_name to the gbuf node's QN, so downstream edge property serialisation shows the project-qualified form even when fallback resolution kicked in. Cross-file regression test parallel_python_lsp_override_cross_file_- emits_lsp_strategy_edges pins the two-file Greeter scenario that originally exposed the bug.
This commit is contained in:
@@ -22,8 +22,10 @@
|
||||
#define CBM_PIPELINE_LSP_RESOLVE_H
|
||||
|
||||
#include "cbm.h"
|
||||
#include "graph_buffer/graph_buffer.h"
|
||||
#include "foundation/constants.h"
|
||||
|
||||
#include <stdio.h>
|
||||
#include <string.h>
|
||||
|
||||
/* Confidence floor below which LSP-resolved calls are ignored and the
|
||||
@@ -75,4 +77,46 @@ cbm_pipeline_find_lsp_resolution(const CBMResolvedCallArray *arr, const CBMCall
|
||||
return best;
|
||||
}
|
||||
|
||||
/* Resolve an LSP-emitted callee_qn to a graph-buffer node.
|
||||
*
|
||||
* Per-file LSPs (notably py_lsp) sometimes emit `callee_qn` as the raw
|
||||
* import-module path the source code uses (e.g. `greeter.Greeter` from
|
||||
* `from greeter import Greeter`) rather than the project-qualified QN
|
||||
* the gbuf actually stores (`<project>.greeter.Greeter`). This is
|
||||
* unavoidable at the per-file LSP layer: the LSP cannot tell in-project
|
||||
* imports (qualify) from external imports (don't qualify, e.g. `os.path`)
|
||||
* without consulting the gbuf, which is built downstream.
|
||||
*
|
||||
* The fallback rule: try the LSP-emitted QN as-is first; on miss, retry
|
||||
* with `<project>.<callee_qn>`. If that also misses, the target is
|
||||
* external/unknown and the caller drops the edge — same as today.
|
||||
*
|
||||
* Returns the matching node, or NULL if neither lookup hits. */
|
||||
static inline const cbm_gbuf_node_t *
|
||||
cbm_pipeline_lsp_target_node(const cbm_gbuf_t *gbuf, const char *project_name,
|
||||
const char *callee_qn) {
|
||||
if (!gbuf || !callee_qn) {
|
||||
return NULL;
|
||||
}
|
||||
const cbm_gbuf_node_t *direct = cbm_gbuf_find_by_qn(gbuf, callee_qn);
|
||||
if (direct) {
|
||||
return direct;
|
||||
}
|
||||
if (!project_name || !project_name[0]) {
|
||||
return NULL;
|
||||
}
|
||||
/* Skip the prefix retry if callee_qn is already project-qualified —
|
||||
* avoids producing nonsense like `proj.proj.foo.Bar`. */
|
||||
size_t proj_len = strlen(project_name);
|
||||
if (strncmp(callee_qn, project_name, proj_len) == 0 && callee_qn[proj_len] == '.') {
|
||||
return NULL;
|
||||
}
|
||||
char buf[CBM_SZ_1K];
|
||||
int written = snprintf(buf, sizeof(buf), "%s.%s", project_name, callee_qn);
|
||||
if (written < 0 || (size_t)written >= sizeof(buf)) {
|
||||
return NULL;
|
||||
}
|
||||
return cbm_gbuf_find_by_qn(gbuf, buf);
|
||||
}
|
||||
|
||||
#endif /* CBM_PIPELINE_LSP_RESOLVE_H */
|
||||
|
||||
@@ -336,10 +336,13 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call,
|
||||
/* LSP-resolved calls take precedence over registry-textual matching. */
|
||||
const CBMResolvedCall *lsp = cbm_pipeline_find_lsp_resolution(lsp_calls, call);
|
||||
if (lsp) {
|
||||
const cbm_gbuf_node_t *target_node = cbm_gbuf_find_by_qn(ctx->gbuf, lsp->callee_qn);
|
||||
const cbm_gbuf_node_t *target_node =
|
||||
cbm_pipeline_lsp_target_node(ctx->gbuf, ctx->project_name, lsp->callee_qn);
|
||||
if (target_node && source_node->id != target_node->id) {
|
||||
cbm_resolution_t res = {0};
|
||||
res.qualified_name = lsp->callee_qn;
|
||||
/* Use the gbuf node's QN so downstream edge props show the canonical
|
||||
* project-qualified form even when fallback prefixed the project. */
|
||||
res.qualified_name = target_node->qualified_name;
|
||||
res.confidence = lsp->confidence;
|
||||
res.strategy = lsp->strategy;
|
||||
res.candidate_count = 1;
|
||||
|
||||
@@ -1477,11 +1477,21 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
|
||||
const CBMResolvedCall *lsp =
|
||||
cbm_pipeline_find_lsp_resolution(&result->resolved_calls, call);
|
||||
if (lsp) {
|
||||
res.qualified_name = lsp->callee_qn;
|
||||
res.strategy = lsp->strategy ? lsp->strategy : "lsp_override";
|
||||
res.confidence = (double)lsp->confidence;
|
||||
res.candidate_count = 1;
|
||||
ws->lsp_overrides++;
|
||||
/* Canonicalise to the gbuf node's QN so res.qualified_name matches
|
||||
* the gbuf even when the cross-file fallback had to prefix the
|
||||
* project name. If neither lookup hits, leave res.qualified_name
|
||||
* empty — the LSP was confident but its target isn't in the gbuf
|
||||
* (external/unindexed), so drop the edge rather than fall back to
|
||||
* the registry resolver, matching prior single-lookup semantics. */
|
||||
const cbm_gbuf_node_t *lsp_target =
|
||||
cbm_pipeline_lsp_target_node(rc->main_gbuf, rc->project_name, lsp->callee_qn);
|
||||
if (lsp_target) {
|
||||
res.qualified_name = lsp_target->qualified_name;
|
||||
res.strategy = lsp->strategy ? lsp->strategy : "lsp_override";
|
||||
res.confidence = (double)lsp->confidence;
|
||||
res.candidate_count = 1;
|
||||
ws->lsp_overrides++;
|
||||
}
|
||||
} else {
|
||||
res = cbm_registry_resolve(rc->registry, call->callee_name, module_qn,
|
||||
imp_keys, imp_vals, imp_count);
|
||||
|
||||
+84
-14
@@ -463,20 +463,14 @@ TEST(parallel_python_lsp_override_emits_lsp_strategy_edges) {
|
||||
SKIP("mkdtemp failed");
|
||||
}
|
||||
|
||||
/* Single-file scenario on purpose. cbm_run_py_lsp runs per-file in
|
||||
* the parallel pipeline, so cross-file type inference doesn't fire
|
||||
* during cbm_parallel_extract; the type registry only sees the
|
||||
* current file's own defs. With Greeter and main() in the same file,
|
||||
* py_lsp can register Greeter from the file's own defs, type the
|
||||
* `g = Greeter()` constructor as NAMED("…Greeter"), and resolve
|
||||
* `g.hello()` to Greeter.hello via attribute lookup — yielding a
|
||||
* resolved_calls entry whose callee_qn matches the gbuf node QN
|
||||
* exactly. The cross-file scenario lights up the same wiring code
|
||||
* (lsp_overrides counter increments, confirmed in earlier runs) but
|
||||
* the resulting edge gets dropped at cbm_gbuf_find_by_qn because the
|
||||
* per-file py_lsp emits an unprefixed module path that doesn't match
|
||||
* the project-qualified gbuf QN — that's a separate cross-file bug
|
||||
* tracked elsewhere, not what this test is pinning. */
|
||||
/* Single-file scenario: pins the in-file LSP path where py_lsp
|
||||
* registers Greeter from the file's own defs, types `g = Greeter()`
|
||||
* as NAMED("…Greeter"), and resolves `g.hello()` to Greeter.hello
|
||||
* via attribute lookup. callee_qn matches the gbuf QN directly. The
|
||||
* cross-file equivalent is covered by
|
||||
* parallel_python_lsp_override_cross_file_emits_lsp_strategy_edges,
|
||||
* which exercises the project-prefix fallback in
|
||||
* cbm_pipeline_lsp_target_node. */
|
||||
char fpath0[512];
|
||||
snprintf(fpath0, sizeof(fpath0), "%s/app.py", tmpdir);
|
||||
FILE *f = fopen(fpath0, "w");
|
||||
@@ -520,6 +514,81 @@ TEST(parallel_python_lsp_override_emits_lsp_strategy_edges) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Cross-file regression for the QN-mismatch bug: py_lsp's per-file mode
|
||||
* emits resolved_calls.callee_qn as the raw import-module path (e.g.
|
||||
* `greeter.Greeter` from `from greeter import Greeter`) rather than the
|
||||
* project-qualified QN the gbuf stores (`<project>.greeter.Greeter`).
|
||||
* Before cbm_pipeline_lsp_target_node added the project-prefix fallback,
|
||||
* the LSP match succeeded (lsp_overrides counter incremented) but the
|
||||
* downstream cbm_gbuf_find_by_qn lookup missed silently, dropping the
|
||||
* edge. With the fallback in place, the cross-file `g.hello()` call is
|
||||
* attributed to <project>.greeter.Greeter.hello with an lsp_* strategy.
|
||||
*
|
||||
* Two-file scenario: greeter.py defines Greeter; app.py imports it and
|
||||
* calls hello() — same shape as the original failing reproduction. */
|
||||
TEST(parallel_python_lsp_override_cross_file_emits_lsp_strategy_edges) {
|
||||
char tmpdir[256];
|
||||
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_par_pylsp_xf_XXXXXX");
|
||||
if (!cbm_mkdtemp(tmpdir)) {
|
||||
SKIP("mkdtemp failed");
|
||||
}
|
||||
|
||||
char gpath[512];
|
||||
snprintf(gpath, sizeof(gpath), "%s/greeter.py", tmpdir);
|
||||
FILE *gf = fopen(gpath, "w");
|
||||
if (!gf) {
|
||||
SKIP("fopen greeter.py failed");
|
||||
}
|
||||
fprintf(gf, "class Greeter:\n"
|
||||
" def hello(self):\n"
|
||||
" return 'hi'\n");
|
||||
fclose(gf);
|
||||
|
||||
char apath[512];
|
||||
snprintf(apath, sizeof(apath), "%s/app.py", tmpdir);
|
||||
FILE *af = fopen(apath, "w");
|
||||
if (!af) {
|
||||
unlink(gpath);
|
||||
rmdir(tmpdir);
|
||||
SKIP("fopen app.py failed");
|
||||
}
|
||||
fprintf(af, "from greeter import Greeter\n"
|
||||
"\n"
|
||||
"def main():\n"
|
||||
" g = Greeter()\n"
|
||||
" g.hello()\n");
|
||||
fclose(af);
|
||||
|
||||
cbm_file_info_t files[2] = {0};
|
||||
files[0].path = gpath;
|
||||
files[0].rel_path = (char *)"greeter.py";
|
||||
files[0].language = CBM_LANG_PYTHON;
|
||||
files[1].path = apath;
|
||||
files[1].rel_path = (char *)"app.py";
|
||||
files[1].language = CBM_LANG_PYTHON;
|
||||
|
||||
cbm_gbuf_t *gbuf = run_parallel("cbm_par_pylsp_xf", tmpdir, files, 2, 2);
|
||||
ASSERT_NOT_NULL(gbuf);
|
||||
|
||||
lsp_edge_count_ctx_t c = {0};
|
||||
cbm_gbuf_foreach_edge(gbuf, count_lsp_call_edges, &c);
|
||||
|
||||
ASSERT_GT(c.total_calls, 0);
|
||||
/* The cross-file LSP override must produce at least one lsp_*
|
||||
* CALLS edge. Without the project-prefix fallback in
|
||||
* cbm_pipeline_lsp_target_node this assertion would fail because the
|
||||
* raw module-path callee_qn doesn't match the project-qualified
|
||||
* gbuf node QN. */
|
||||
ASSERT_GT(c.lsp_strategy_count, 0);
|
||||
|
||||
cbm_gbuf_free(gbuf);
|
||||
|
||||
unlink(apath);
|
||||
unlink(gpath);
|
||||
rmdir(tmpdir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── Suite Registration ──────────────────────────────────────────── */
|
||||
|
||||
SUITE(parallel) {
|
||||
@@ -534,6 +603,7 @@ SUITE(parallel) {
|
||||
/* Parallel pipeline parity tests */
|
||||
RUN_TEST(parallel_node_count);
|
||||
RUN_TEST(parallel_python_lsp_override_emits_lsp_strategy_edges);
|
||||
RUN_TEST(parallel_python_lsp_override_cross_file_emits_lsp_strategy_edges);
|
||||
RUN_TEST(parallel_calls_parity);
|
||||
RUN_TEST(parallel_defines_parity);
|
||||
RUN_TEST(parallel_defines_method_parity);
|
||||
|
||||
Reference in New Issue
Block a user