From 707f6f0db58a16b706b29d249bced477094ef92c Mon Sep 17 00:00:00 2001 From: Chad <4307099+BobbieBarker@users.noreply.github.com> Date: Thu, 24 Sep 2026 18:13:11 -0700 Subject: [PATCH] fix(registry): index a symbol under its own name and its QN's tail cbm_registry_add takes the symbol's `name` and discards it, deriving the bare-name lookup key from the QN's last dot segment instead. That makes the QN's tail load-bearing for the by-name index every language shares, and it is already wrong for one of them: rust_cfg_qualified_name mints a `#[cfg(test)]` twin as "proj.lib.add#cfg(test)", simple_name() has no '#' handling, so that function is indexed under the literal string "add#cfg(test)" where no bare `add` callee can reach it. The same hole would swallow every arity-fenced Elixir definition the later commits in this stack mint. The caller always knows the name; it is the argument. But the two keys disagree in BOTH directions, so swapping one for the other only moves the hole. A name can equally carry segments the QN's tail drops: grammar def->name QN tail HCL resource.aws_instance.web web TOML tool.poetry.dependencies dependencies INI tool.isort isort Markdown 1.2 Scope 2-Scope Elixir Fx.Store Store all 162 helper.py helper (the file's Module node) The last row is the broad one: a file's Module node is named with the basename including its extension in every grammar, while its QN tail is the extensionless stem a bare module reference is written as. The HCL row is the one with a live reference syntax behind it -- `instance = aws_instance.web.id` reaches the block through the tail `web`, and a name-only key drops that edge outright. So the index carries both keys, and pays for the second only where they differ. No key is removed, so no lookup that resolved before stops resolving. `name` is NULL or empty only for callers that have no symbol name to give; those have the derived key and nothing else. Swept rather than assumed: a throwaway probe walked tests/grammar_cases.h (one fixture per grammar, all 162) comparing def->name with simple_name(def->qualified_name), then targeted fixtures for the dotted shapes above. The Module row appears in all 162; HCL is the only fixture-level mismatch beyond it; TOML, INI, Markdown and Elixir mismatch on the targeted fixtures. XML namespaced elements (`ns:root`) do NOT mismatch: ':' is not a QN separator. tests/test_registry.c: registry_indexes_by_passed_name_not_qn_tail resolves a bare `add` against a cfg-fenced QN. registry_indexes_a_dotted_name_under_its_tail_too resolves an HCL block by its tail and a Module node by its stem. tests/test_pipeline.c: pipeline_hcl_block_reference_resolves_to_its_block indexes a two-resource Terraform file end to end and asserts the surviving USAGE edge. registry_indexes_by_passed_name_not_qn_tail fails without this commit. The other two pass on e783f73d and are here as guards: the by-name index this stack re-keys is what would break them, and an HCL block name is dotted where its QN tail is not, so they pin the shape that regression would take. Opens on #N+4, the branch below it in the delivered chain. It has no content dependency on #N..#N+4: it touches src/pipeline/registry.c plus two test files that none of those commits touch, and it cherry-picks onto e783f73d cleanly. It is ordered before #N+6 because the arity-fenced definitions that commit mints need the passed-name key to stay reachable from a bare callee. Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) --- src/pipeline/registry.c | 76 +++++++++++++++++++++++++++-------------- tests/test_pipeline.c | 55 +++++++++++++++++++++++++++++ tests/test_registry.c | 50 +++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 25 deletions(-) diff --git a/src/pipeline/registry.c b/src/pipeline/registry.c index c99c72d8d..42346515c 100644 --- a/src/pipeline/registry.c +++ b/src/pipeline/registry.c @@ -858,9 +858,38 @@ void cbm_registry_free(cbm_registry_t *r) { /* ── Registration ────────────────────────────────────────────────── */ +/* Record `owned_qn` in the by-name bucket for `key`, keeping the bucket's + * is_test flags in step with its entries. + * + * No array dedup needed: cbm_registry_add's exact-map check guarantees the QN + * is new, and it calls this at most once per distinct key. */ +static void index_under_name(cbm_registry_t *r, const char *key, const char *owned_qn) { + qn_array_t *arr = cbm_ht_get(r->by_name, key); + if (!arr) { + arr = calloc(CBM_ALLOC_ONE, sizeof(qn_array_t)); + cbm_ht_set(r->by_name, strdup(key), arr); + } + int before = arr->count; + cbm_da_push(arr, (char *)owned_qn); + if (arr->count == before) { + return; /* the name could not be recorded: no verdict to cache */ + } + if (arr->count > arr->is_test_cap) { + int want = arr->cap > 0 ? arr->cap : arr->count; + uint8_t *grown = + cbm_realloc(CBM_MEM_CLASS_DYN_ARRAY, arr->is_test, (size_t)want * sizeof(uint8_t)); + if (grown) { + arr->is_test = grown; + arr->is_test_cap = want; + } + } + if (arr->count <= arr->is_test_cap) { + arr->is_test[arr->count - SKIP_ONE] = is_test_qn(owned_qn) ? 1 : 0; + } +} + void cbm_registry_add(cbm_registry_t *r, const char *name, const char *qualified_name, const char *label) { - (void)name; if (!r || !qualified_name || !label) { return; } @@ -893,30 +922,27 @@ void cbm_registry_add(cbm_registry_t *r, const char *name, const char *qualified cbm_ht_set(r->exact, strdup(qualified_name), (void *)interned); const char *owned_qn = cbm_ht_get_key(r->exact, qualified_name); - /* Index by simple name. - * No array dedup needed: exact-map check above guarantees uniqueness. */ - const char *simple = simple_name(qualified_name); - qn_array_t *arr = cbm_ht_get(r->by_name, simple); - if (!arr) { - arr = calloc(CBM_ALLOC_ONE, sizeof(qn_array_t)); - cbm_ht_set(r->by_name, strdup(simple), arr); - } - int before = arr->count; - cbm_da_push(arr, (char *)owned_qn); - if (arr->count == before) { - return; /* the name could not be recorded: no verdict to cache */ - } - if (arr->count > arr->is_test_cap) { - int want = arr->cap > 0 ? arr->cap : arr->count; - uint8_t *grown = - cbm_realloc(CBM_MEM_CLASS_DYN_ARRAY, arr->is_test, (size_t)want * sizeof(uint8_t)); - if (grown) { - arr->is_test = grown; - arr->is_test_cap = want; - } - } - if (arr->count <= arr->is_test_cap) { - arr->is_test[arr->count - SKIP_ONE] = is_test_qn(owned_qn) ? 1 : 0; + /* Index the symbol under the name its caller passed AND under the QN's + * last dot segment, whenever the two differ. + * + * Neither key alone covers every language, because the two disagree in + * both directions. A QN can carry a discriminator the name does not: a + * Rust cfg twin is minted as "add#cfg(test)" (rust_cfg_qualified_name) + * and simple_name() has no '#' handling, so the derived key alone filed + * that function under the literal string "add#cfg(test)", where no bare + * `add` callee could reach it. A name can equally carry dots the QN's + * tail drops: an HCL block names itself "resource.aws_instance.web" + * (find_hcl_block_name) and a TOML dotted table key names itself + * "tool.poetry.dependencies", so the passed key alone loses the tail + * lookup that an `aws_instance.web.id` reference resolves through. + * + * `name` is NULL or empty only for callers that have no symbol name to + * give; those have the derived key and nothing else. */ + const char *derived = simple_name(qualified_name); + const char *given = (name && name[0]) ? name : derived; + index_under_name(r, given, owned_qn); + if (strcmp(given, derived) != 0) { + index_under_name(r, derived, owned_qn); } } diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index d32ab3538..059d911aa 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -1117,6 +1117,60 @@ TEST(pipeline_nix_scoped_binding_calls_resolve) { PASS(); } +/* Terraform reference resolution, end to end. + * + * An HCL block names itself with its labels appended -- find_hcl_block_name + * mints "resource.aws_instance.web" -- while its QN's last dot segment is + * bare "web", and a reference written `aws_instance.web.id` reaches the + * by-name index through that tail. HCL is therefore a language where the + * definition name and the QN tail are different strings, and an index keyed on + * either one alone drops every cross-resource reference in the file. + * + * This has to be a pipeline test: the registry is shared by all 162 languages + * and nothing in the HCL extractor mentions the index key, so the two halves + * can disagree with every extraction-level assertion still green. + */ +TEST(pipeline_hcl_block_reference_resolves_to_its_block) { + if (setup_test_repo() != 0) { + FAIL("failed to create temp dir"); + } + + char tf_path[512]; + snprintf(tf_path, sizeof(tf_path), "%s/main.tf", g_tmpdir); + FILE *tf = fopen(tf_path, "w"); + if (!tf) { + teardown_test_repo(); + FAIL("failed to write terraform fixture"); + } + fprintf(tf, "resource \"aws_instance\" \"web\" {\n" + " ami = \"ami-0c55b159cbfafe1f0\"\n" + " instance_type = \"t2.micro\"\n" + "}\n" + "\n" + "resource \"aws_eip\" \"ip\" {\n" + " instance = aws_instance.web.id\n" + "}\n"); + fclose(tf); + + char tf_db[512]; + snprintf(tf_db, sizeof(tf_db), "%s/test_hcl_refs.db", g_tmpdir); + + cbm_pipeline_t *tp = cbm_pipeline_new(g_tmpdir, tf_db, CBM_MODE_FULL); + ASSERT_NOT_NULL(tp); + ASSERT_EQ(cbm_pipeline_run(tp), 0); + + cbm_store_t *ts = cbm_store_open_path(tf_db); + ASSERT_NOT_NULL(ts); + const char *tf_project = cbm_pipeline_project_name(tp); + + ASSERT(cross_file_edge_exists(ts, tf_project, "main", "resource.aws_instance.web", "USAGE")); + + cbm_store_close(ts); + cbm_pipeline_free(tp); + teardown_test_repo(); + PASS(); +} + /* Regression: incremental re-index of an edited file must NOT drop inbound * cross-file CALLS edges whose source lives in an UNCHANGED file. * @@ -15098,6 +15152,7 @@ SUITE(pipeline) { /* Calls pass */ RUN_TEST(pipeline_calls_resolution); RUN_TEST(pipeline_nix_scoped_binding_calls_resolve); + RUN_TEST(pipeline_hcl_block_reference_resolves_to_its_block); RUN_TEST(pipeline_incremental_preserves_cross_file_calls); RUN_TEST(pipeline_objectscript_export_preserves_calls_sequential_parallel); RUN_TEST(pipeline_objectscript_export_incremental_matches_full_relationships); diff --git a/tests/test_registry.c b/tests/test_registry.c index 5f751a899..2161fc8c2 100644 --- a/tests/test_registry.c +++ b/tests/test_registry.c @@ -325,6 +325,54 @@ TEST(resolve_qualified_ambiguous_tail_falls_through) { PASS(); } +/* cbm_registry_add used to discard its `name` argument and re-derive the + * lookup key from the QN's last dot segment. That made the QN's tail load + * bearing for the bare-name index every language shares: a Rust cfg twin, + * minted as "add#cfg(test)", was indexed under that literal string and no bare + * `add` callee could ever reach it. */ +TEST(registry_indexes_by_passed_name_not_qn_tail) { + cbm_registry_t *r = cbm_registry_new(); + cbm_registry_add(r, "add", "proj.lib.add#cfg(test)", "Function"); + + cbm_resolution_t res = cbm_registry_resolve(r, "add", "proj.lib.caller", NULL, NULL, 0); + ASSERT_STR_EQ(res.qualified_name, "proj.lib.add#cfg(test)"); + + cbm_registry_free(r); + PASS(); +} + +/* The other direction of the same disagreement: a name may carry segments that + * the QN's tail drops, and then the QN tail is the key callers actually spell. + * + * Two shapes, both taken from what the extractors emit today. An HCL block is + * named "resource.aws_instance.web" by find_hcl_block_name while its QN tail + * is bare "web", which is how a `aws_instance.web.id` reference reaches it. A + * file's Module node is named with the basename INCLUDING its extension in + * every one of the 162 languages, while its QN tail is the extensionless stem + * a bare module reference is written as. + * + * Keying the index on the passed name ALONE loses both lookups, so the index + * carries both keys. */ +TEST(registry_indexes_a_dotted_name_under_its_tail_too) { + cbm_registry_t *r = cbm_registry_new(); + cbm_registry_add(r, "resource.aws_instance.web", "proj.main.resource.aws_instance.web", + "Class"); + cbm_registry_add(r, "helper.py", "proj.pkg.helper", "Module"); + + cbm_resolution_t tail = cbm_registry_resolve(r, "web", "proj.main", NULL, NULL, 0); + ASSERT_STR_EQ(tail.qualified_name, "proj.main.resource.aws_instance.web"); + + cbm_resolution_t whole = + cbm_registry_resolve(r, "resource.aws_instance.web", "proj.main", NULL, NULL, 0); + ASSERT_STR_EQ(whole.qualified_name, "proj.main.resource.aws_instance.web"); + + cbm_resolution_t stem = cbm_registry_resolve(r, "helper", "proj.pkg.caller", NULL, NULL, 0); + ASSERT_STR_EQ(stem.qualified_name, "proj.pkg.helper"); + + cbm_registry_free(r); + PASS(); +} + TEST(resolve_import_map) { cbm_registry_t *r = cbm_registry_new(); cbm_registry_add(r, "Process", "proj.pkg.worker.Process", "Function"); @@ -1201,6 +1249,8 @@ SUITE(registry) { RUN_TEST(resolve_same_module); RUN_TEST(resolve_qualified_disambiguates_same_name); RUN_TEST(resolve_qualified_ambiguous_tail_falls_through); + RUN_TEST(registry_indexes_by_passed_name_not_qn_tail); + RUN_TEST(registry_indexes_a_dotted_name_under_its_tail_too); RUN_TEST(resolve_import_map); RUN_TEST(resolve_import_map_bare_function); RUN_TEST(resolve_import_map_bare_alias);