Skip to content

fix(registry): index a symbol under its own name and its QN's tail - #2308

Closed
BobbieBarker wants to merge 1 commit into
DeusData:mainfrom
BobbieBarker:pr/registry-index-by-name-and-tail
Closed

fix(registry): index a symbol under its own name and its QN's tail#2308
BobbieBarker wants to merge 1 commit into
DeusData:mainfrom
BobbieBarker:pr/registry-index-by-name-and-tail

Conversation

@BobbieBarker

@BobbieBarker BobbieBarker commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

The defect

cbm_registry_add keys the shared by-name index on simple_name(qualified_name) — the text after the last dot. For most languages the definition's own name and that tail are the same string, so nothing shows. They are not always the same:

  • an HCL/Terraform block's name is dotted where its QN tail is not: find_hcl_block_name returns resource.aws_instance.web, and the QN tail is web
  • a Rust #[cfg(...)] twin carries a qualifier in its QN (add#cfg(test)) that its name does not

Whichever of the two forms a caller presents, only one of them resolves. registry_indexes_by_passed_name_not_qn_tail fails on main today for the cfg case.

The fix

Index under both the passed name and the QN's tail when they differ, so a bare-name lookup resolves either way. The by-name bucket already tolerates several entries per key and receiver_chain_admits still gates every candidate, so this widens what can be found without widening what is accepted.

':' is deliberately not treated as a separator: XML namespaced elements (ns:root) do not mismatch.

Verification

  • tests/test_registry.c: registry_indexes_by_passed_name_not_qn_tail — fails on main, passes with this change.
  • tests/test_registry.c: registry_indexes_a_dotted_name_under_its_tail_too and tests/test_pipeline.c: pipeline_hcl_block_reference_resolves_to_its_block pass on main and are here as guards: they pin the shape a regression in this index would take, which is what the dotted-vs-tail mismatch makes possible.
  • A two-resource Terraform fixture indexed end to end produces byte-identical nodes and edges before and after, confirming the change is additive for HCL rather than a behaviour change.
  • Full suite green.

Reported and fixed against main.

@BobbieBarker @claude
fix(registry): index a symbol under its own name and its QN's tail
707f6f0 
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 e783f73 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 e783f73 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) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 25, 2026

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@BobbieBarker BobbieBarker mentioned this pull request Sep 25, 2026
Closed
@BobbieBarker

BobbieBarker commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of #2310, which carries this commit plus the rest of the set.

I split this out first, then realised the pieces cannot land independently: this change creates nodes for guarded functions, which makes a pre-existing phantom self-edge in the calls walk reachable — the regression documented in this PR's own body. It is fixed by a later commit in the set. Merging this alone would ship that regression.

Same commit, same tests, unchanged, now in #2310 with the commits it depends on. Sorry for the noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DeusData DeusData Awaiting requested review from DeusData DeusData is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@BobbieBarker