Skip to content

Commit 673ac4e

Browse files
committed
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.
1 parent 085b4a7 commit 673ac4e

4 files changed

Lines changed: 148 additions & 21 deletions

File tree

src/pipeline/lsp_resolve.h

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
#define CBM_PIPELINE_LSP_RESOLVE_H
2323

2424
#include "cbm.h"
25+
#include "graph_buffer/graph_buffer.h"
2526
#include "foundation/constants.h"
2627

28+
#include <stdio.h>
2729
#include <string.h>
2830

2931
/* 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
7577
return best;
7678
}
7779

80+
/* Resolve an LSP-emitted callee_qn to a graph-buffer node.
81+
*
82+
* Per-file LSPs (notably py_lsp) sometimes emit `callee_qn` as the raw
83+
* import-module path the source code uses (e.g. `greeter.Greeter` from
84+
* `from greeter import Greeter`) rather than the project-qualified QN
85+
* the gbuf actually stores (`<project>.greeter.Greeter`). This is
86+
* unavoidable at the per-file LSP layer: the LSP cannot tell in-project
87+
* imports (qualify) from external imports (don't qualify, e.g. `os.path`)
88+
* without consulting the gbuf, which is built downstream.
89+
*
90+
* The fallback rule: try the LSP-emitted QN as-is first; on miss, retry
91+
* with `<project>.<callee_qn>`. If that also misses, the target is
92+
* external/unknown and the caller drops the edge — same as today.
93+
*
94+
* Returns the matching node, or NULL if neither lookup hits. */
95+
static inline const cbm_gbuf_node_t *
96+
cbm_pipeline_lsp_target_node(const cbm_gbuf_t *gbuf, const char *project_name,
97+
const char *callee_qn) {
98+
if (!gbuf || !callee_qn) {
99+
return NULL;
100+
}
101+
const cbm_gbuf_node_t *direct = cbm_gbuf_find_by_qn(gbuf, callee_qn);
102+
if (direct) {
103+
return direct;
104+
}
105+
if (!project_name || !project_name[0]) {
106+
return NULL;
107+
}
108+
/* Skip the prefix retry if callee_qn is already project-qualified —
109+
* avoids producing nonsense like `proj.proj.foo.Bar`. */
110+
size_t proj_len = strlen(project_name);
111+
if (strncmp(callee_qn, project_name, proj_len) == 0 && callee_qn[proj_len] == '.') {
112+
return NULL;
113+
}
114+
char buf[CBM_SZ_1K];
115+
int written = snprintf(buf, sizeof(buf), "%s.%s", project_name, callee_qn);
116+
if (written < 0 || (size_t)written >= sizeof(buf)) {
117+
return NULL;
118+
}
119+
return cbm_gbuf_find_by_qn(gbuf, buf);
120+
}
121+
78122
#endif /* CBM_PIPELINE_LSP_RESOLVE_H */

src/pipeline/pass_calls.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,13 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call,
336336
/* LSP-resolved calls take precedence over registry-textual matching. */
337337
const CBMResolvedCall *lsp = cbm_pipeline_find_lsp_resolution(lsp_calls, call);
338338
if (lsp) {
339-
const cbm_gbuf_node_t *target_node = cbm_gbuf_find_by_qn(ctx->gbuf, lsp->callee_qn);
339+
const cbm_gbuf_node_t *target_node =
340+
cbm_pipeline_lsp_target_node(ctx->gbuf, ctx->project_name, lsp->callee_qn);
340341
if (target_node && source_node->id != target_node->id) {
341342
cbm_resolution_t res = {0};
342-
res.qualified_name = lsp->callee_qn;
343+
/* Use the gbuf node's QN so downstream edge props show the canonical
344+
* project-qualified form even when fallback prefixed the project. */
345+
res.qualified_name = target_node->qualified_name;
343346
res.confidence = lsp->confidence;
344347
res.strategy = lsp->strategy;
345348
res.candidate_count = 1;

src/pipeline/pass_parallel.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,11 +1477,21 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
14771477
const CBMResolvedCall *lsp =
14781478
cbm_pipeline_find_lsp_resolution(&result->resolved_calls, call);
14791479
if (lsp) {
1480-
res.qualified_name = lsp->callee_qn;
1481-
res.strategy = lsp->strategy ? lsp->strategy : "lsp_override";
1482-
res.confidence = (double)lsp->confidence;
1483-
res.candidate_count = 1;
1484-
ws->lsp_overrides++;
1480+
/* Canonicalise to the gbuf node's QN so res.qualified_name matches
1481+
* the gbuf even when the cross-file fallback had to prefix the
1482+
* project name. If neither lookup hits, leave res.qualified_name
1483+
* empty — the LSP was confident but its target isn't in the gbuf
1484+
* (external/unindexed), so drop the edge rather than fall back to
1485+
* the registry resolver, matching prior single-lookup semantics. */
1486+
const cbm_gbuf_node_t *lsp_target =
1487+
cbm_pipeline_lsp_target_node(rc->main_gbuf, rc->project_name, lsp->callee_qn);
1488+
if (lsp_target) {
1489+
res.qualified_name = lsp_target->qualified_name;
1490+
res.strategy = lsp->strategy ? lsp->strategy : "lsp_override";
1491+
res.confidence = (double)lsp->confidence;
1492+
res.candidate_count = 1;
1493+
ws->lsp_overrides++;
1494+
}
14851495
} else {
14861496
res = cbm_registry_resolve(rc->registry, call->callee_name, module_qn,
14871497
imp_keys, imp_vals, imp_count);

tests/test_parallel.c

Lines changed: 84 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -463,20 +463,14 @@ TEST(parallel_python_lsp_override_emits_lsp_strategy_edges) {
463463
SKIP("mkdtemp failed");
464464
}
465465

466-
/* Single-file scenario on purpose. cbm_run_py_lsp runs per-file in
467-
* the parallel pipeline, so cross-file type inference doesn't fire
468-
* during cbm_parallel_extract; the type registry only sees the
469-
* current file's own defs. With Greeter and main() in the same file,
470-
* py_lsp can register Greeter from the file's own defs, type the
471-
* `g = Greeter()` constructor as NAMED("…Greeter"), and resolve
472-
* `g.hello()` to Greeter.hello via attribute lookup — yielding a
473-
* resolved_calls entry whose callee_qn matches the gbuf node QN
474-
* exactly. The cross-file scenario lights up the same wiring code
475-
* (lsp_overrides counter increments, confirmed in earlier runs) but
476-
* the resulting edge gets dropped at cbm_gbuf_find_by_qn because the
477-
* per-file py_lsp emits an unprefixed module path that doesn't match
478-
* the project-qualified gbuf QN — that's a separate cross-file bug
479-
* tracked elsewhere, not what this test is pinning. */
466+
/* Single-file scenario: pins the in-file LSP path where py_lsp
467+
* registers Greeter from the file's own defs, types `g = Greeter()`
468+
* as NAMED("…Greeter"), and resolves `g.hello()` to Greeter.hello
469+
* via attribute lookup. callee_qn matches the gbuf QN directly. The
470+
* cross-file equivalent is covered by
471+
* parallel_python_lsp_override_cross_file_emits_lsp_strategy_edges,
472+
* which exercises the project-prefix fallback in
473+
* cbm_pipeline_lsp_target_node. */
480474
char fpath0[512];
481475
snprintf(fpath0, sizeof(fpath0), "%s/app.py", tmpdir);
482476
FILE *f = fopen(fpath0, "w");
@@ -520,6 +514,81 @@ TEST(parallel_python_lsp_override_emits_lsp_strategy_edges) {
520514
PASS();
521515
}
522516

517+
/* Cross-file regression for the QN-mismatch bug: py_lsp's per-file mode
518+
* emits resolved_calls.callee_qn as the raw import-module path (e.g.
519+
* `greeter.Greeter` from `from greeter import Greeter`) rather than the
520+
* project-qualified QN the gbuf stores (`<project>.greeter.Greeter`).
521+
* Before cbm_pipeline_lsp_target_node added the project-prefix fallback,
522+
* the LSP match succeeded (lsp_overrides counter incremented) but the
523+
* downstream cbm_gbuf_find_by_qn lookup missed silently, dropping the
524+
* edge. With the fallback in place, the cross-file `g.hello()` call is
525+
* attributed to <project>.greeter.Greeter.hello with an lsp_* strategy.
526+
*
527+
* Two-file scenario: greeter.py defines Greeter; app.py imports it and
528+
* calls hello() — same shape as the original failing reproduction. */
529+
TEST(parallel_python_lsp_override_cross_file_emits_lsp_strategy_edges) {
530+
char tmpdir[256];
531+
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_par_pylsp_xf_XXXXXX");
532+
if (!cbm_mkdtemp(tmpdir)) {
533+
SKIP("mkdtemp failed");
534+
}
535+
536+
char gpath[512];
537+
snprintf(gpath, sizeof(gpath), "%s/greeter.py", tmpdir);
538+
FILE *gf = fopen(gpath, "w");
539+
if (!gf) {
540+
SKIP("fopen greeter.py failed");
541+
}
542+
fprintf(gf, "class Greeter:\n"
543+
" def hello(self):\n"
544+
" return 'hi'\n");
545+
fclose(gf);
546+
547+
char apath[512];
548+
snprintf(apath, sizeof(apath), "%s/app.py", tmpdir);
549+
FILE *af = fopen(apath, "w");
550+
if (!af) {
551+
unlink(gpath);
552+
rmdir(tmpdir);
553+
SKIP("fopen app.py failed");
554+
}
555+
fprintf(af, "from greeter import Greeter\n"
556+
"\n"
557+
"def main():\n"
558+
" g = Greeter()\n"
559+
" g.hello()\n");
560+
fclose(af);
561+
562+
cbm_file_info_t files[2] = {0};
563+
files[0].path = gpath;
564+
files[0].rel_path = (char *)"greeter.py";
565+
files[0].language = CBM_LANG_PYTHON;
566+
files[1].path = apath;
567+
files[1].rel_path = (char *)"app.py";
568+
files[1].language = CBM_LANG_PYTHON;
569+
570+
cbm_gbuf_t *gbuf = run_parallel("cbm_par_pylsp_xf", tmpdir, files, 2, 2);
571+
ASSERT_NOT_NULL(gbuf);
572+
573+
lsp_edge_count_ctx_t c = {0};
574+
cbm_gbuf_foreach_edge(gbuf, count_lsp_call_edges, &c);
575+
576+
ASSERT_GT(c.total_calls, 0);
577+
/* The cross-file LSP override must produce at least one lsp_*
578+
* CALLS edge. Without the project-prefix fallback in
579+
* cbm_pipeline_lsp_target_node this assertion would fail because the
580+
* raw module-path callee_qn doesn't match the project-qualified
581+
* gbuf node QN. */
582+
ASSERT_GT(c.lsp_strategy_count, 0);
583+
584+
cbm_gbuf_free(gbuf);
585+
586+
unlink(apath);
587+
unlink(gpath);
588+
rmdir(tmpdir);
589+
PASS();
590+
}
591+
523592
/* ── Suite Registration ──────────────────────────────────────────── */
524593

525594
SUITE(parallel) {
@@ -534,6 +603,7 @@ SUITE(parallel) {
534603
/* Parallel pipeline parity tests */
535604
RUN_TEST(parallel_node_count);
536605
RUN_TEST(parallel_python_lsp_override_emits_lsp_strategy_edges);
606+
RUN_TEST(parallel_python_lsp_override_cross_file_emits_lsp_strategy_edges);
537607
RUN_TEST(parallel_calls_parity);
538608
RUN_TEST(parallel_defines_parity);
539609
RUN_TEST(parallel_defines_method_parity);

0 commit comments

Comments
 (0)