fix(extract): Nix definitions are dropped in any file whose root expression is a function by sini · Pull Request #1304 · DeusData/codebase-memory-mcp

sini · 2026-07-27T20:33:33Z

What does this PR do?

Fixes total definition loss for Nix files whose root expression is a function — { pkgs, lib, ... }: <body> — and rewrites the Nix extraction test so the suite can actually fail for it.

Filing without a prior issue under the CONTRIBUTING carve-out for focused bug fixes and test additions. Happy to open one first if you'd rather.

The bug

A Nix library or module file almost always opens with a function pattern. That root node is a function_expression, so it matches nix_func_types. walk_defs hands it to extract_func_def, which resolves no name for it — the Nix resolver at internal/cbm/extract_defs.c:827 requires the function's parent to be a binding, and the root lambda's parent is the file root source_code — so nothing is minted. Control then reaches:

if (!descend_into_func) {
    continue;
}

Nix is not in descend_into_func, so the walk abandons the subtree. No binding below the header is ever visited.

Files opening with a bare let or an attrset are walked normally, which is why the language looked supported: some files produced definitions, so the failure read as sparse coverage rather than a structural miss.

Minimal reproduction — the last two shapes yield nothing on main:

file root	shape	defs on `main`
`let … in`	`alpha = x: x + 1;`	found
`{ … }`	`epsilon = x: x + 1;`	found
`let` inside `let`	`theta = x: x + 1;`	found
`{ prelude }:` then `let … in`	`gamma`, `delta`	none
`{ prelude }:` then `{ … }`	`eta`	none

Nesting depth is not the trigger — nested_let is fine. The trigger is solely whether the file's root expression is a function.

Why the suite didn't catch it

TEST(nix_function) was the only Nix extraction test, and it could not fail for this, for three independent reasons:

its fixture is function-headed — the exact broken shape;
its binding, hello = pkgs.writeShellScriptBin "hello" ''…'', is an application rather than a function, so no Function def is expected from it either way;
it asserts only ASSERT_NOT_NULL and ASSERT_FALSE(r->has_error), omitting the has_def check that every adjacent language test makes.

So the suite reported green while definitions were dropped for the dominant file shape in the ecosystem. I've left nix_function as-is — it usefully pins the "parses, yields no def" case — and added coverage around it.

The fix

One line: add CBM_LANG_NIX to descend_into_func.

Descending does not over-mint from curried lambdas. In f = a: b: … the inner b: has a function_expression parent, resolves no name, and stays out. nix_curried_lambda_mints_one_def pins that with a counter assertion so a future change that starts minting the inner arm is caught.

The change is gated on ctx->language, so no other language's walk is affected.

Tests

Seven cases, one per root shape, each asserting its own definitions so a regression names the shape it broke:

nix_defs_in_let_rooted_file                 let … in
nix_defs_in_attrset_rooted_file             { … }
nix_defs_in_nested_let                      let inside let
nix_defs_survive_function_header_let        { prelude }: let … in
nix_defs_survive_function_header_attrset    { prelude }: { … }
nix_defs_survive_curried_header             final: prev: { … }  (nixpkgs overlay)
nix_curried_lambda_mints_one_def            guards the other direction

Verified as a negative control: with the one-line change reverted, exactly the three new function-header tests fail, each on its own assertion (gamma, eta, iota), while the three pre-existing shapes stay green. That is what makes them real tests rather than tautologies.

Impact

Twelve real Nix repositories, same binary and mode=full on both sides, so the delta is the fix alone. Function / CALLS / files-with-definitions:

repo	.nix	before	after
library A	6	27 / 5 / 3	34 / 7 / 4
library B	76	36 / 24 / 14	390 / 287 / 59
library C	146	12 / 11 / 5	141 / 272 / 55
library D	31	10 / 17 / 7	113 / 114 / 28
library E	29	9 / 9 / 4	156 / 191 / 22
library F	27	2 / 0 / 2	87 / 149 / 22
library G	58	6 / 4 / 4	135 / 118 / 34
library H	37	2 / 0 / 2	163 / 309 / 32
library I	29	2 / 0 / 2	48 / 31 / 22
library J	34	24 / 5 / 10	140 / 75 / 30
framework	320	48 / 56 / 8	1900 / 1443 / 258
system config	499	293 / 204 / 181	654 / 325 / 320

The size of the gain tracks how much of a repository uses the function header, which is what you would expect if the diagnosis is right. Library A barely moves — its entry point opens with a bare attrset, so it was already being walked. The framework repository, where 253 of 320 files carry a header, moves 39-fold. Nothing here regresses.

Checked for quality, not just volume:

One file's extracted definitions match exactly what nil's textDocument/documentSymbol reports for the same file.
Sampled names from a second repository are all real public API — nameValuePair, hasInfix, reverseList, listDfs, toposort, findFirstIndex — plus outputs from flake.nix, which is correct since outputs = inputs@{ … }: … is genuinely a function binding.
No anonymous or empty names appear among the 1900 definitions in the largest repository, so descending past the header does not mint defs for unnamed lambdas.

Incidental finding while confirming node names: nix_module_types in lang_specs.c declares "source_expression", which does not exist in the vendored grammar (the root node is source_code). module_node_types is not consumed anywhere, so it is inert today — but it is misleading, and it is where I first took the wrong name from. Left alone here as out of scope.

Constructs checked for over-minting and clean: map/foldl' arguments, with bodies, inherit, parenthesized lambdas. Correctly minted: rec attrsets, nested attrsets, let inside a function body, factory-returned attrsets.

The { pkgs, lib, ... }: header is the near-universal idiom for Nix libraries, NixOS modules, home-manager modules, and package definitions, so the shape this fixes is the common case rather than an edge case.

Checklist

Every commit is signed off (git commit -s) — required, CI rejects
unsigned commits (DCO, see CONTRIBUTING.md)
Tests pass locally (make -f Makefile.cbm test) — see the note below
Lint passes (make -f Makefile.cbm lint-ci)
New behavior is covered by a test (reproduce-first for bug fixes)

The extraction suite is 260 passed, 0 failed. The full make test target exits
non-zero for me, but it does so on main as well, identically: LeakSanitizer
reports 3679 byte(s) leaked in 5 allocation(s) from strdup in
cbm_project_name_from_path (src/pipeline/fqn.c:511), reached from the test
harness itself — ei_index_files (tests/test_edge_imports.c:108) and
lrp_open_indexed (tests/test_lsp_resolution_probe.c:125). I checked this out at
d587dea and ran it untouched to be sure: same byte count, same allocation count,
same frames. Nothing here contributes to it. Mentioning it in case it is news —
happy to send a separate PR freeing those if you want one.

Pre-existing limitations this fix makes visible

None of these are introduced here — they live in the name resolver, which this PR does not touch. But they were previously unreachable for most files, so this change raises their exposure by roughly the same factor as the coverage gain. Flagging them rather than letting you discover them as a regression. Happy to open issues, or follow-up PRs, for whichever you want.

1. Attrpath names are the first attr child only, so leaf names collide. A definition's qualified name is <project>.<module>.<leaf attr> with no attrpath prefix. Two bindings with the same leaf name in different attrsets in one file collapse to a single node — the second definition, and any CALLS edge sourced from it, is silently dropped. Minimal case, four function bindings yielding three nodes:

{
  setA = { dupName = x: x + 1; };   # }- one node, one def lost
  setB = { dupName = y: y + 2; };   # }
  dotted.path.fn = z: z + 3;        # minted as `dotted`
  "kebab-case" = a: a;              # minted as `"kebab-case"`, quotes included
}

dotted.path.fn mints dotted because the resolver returns the first attr child of the attrpath. Quoted and interpolated attrpaths keep their delimiters, so "then" = vals: … mints a name of "then" with the quote characters in it. On the 320-file framework repository above, a conservative regex scan finds at least 54 files containing colliding leaf names, at least 275 bindings collapsed, and 73 dotted attrpaths. Attrpath-qualified names would address all three at once.

2. Nix bindings mint no Variable nodes. nix_var_types = {"binding"} is declared in lang_specs.c, but there is no Nix name resolver for a plain binding, so a non-function binding produces nothing. The Variable count was unchanged across this fix. A fix would mirror the existing function resolver — child_by_field_name(node, "attrpath"), then its attr — and would inherit the same collision caveat as (1).

… are reached A Nix library or module file's root expression is normally itself a function — `{ pkgs, lib, ... }: <body>` — the near-universal header for the ecosystem. That node matches nix_func_types, so walk_defs handed it to extract_func_def, which resolved no name for it (the Nix resolver requires the function's parent to be a `binding`; the root lambda's parent is the file root, `source_code`) and minted nothing. walk_defs then hit `if (!descend_into_func) continue;` and abandoned the whole subtree, so no binding below the header was ever visited. The effect was total for the dominant file shape, and invisible for the rest: a file opening with a bare `let` or attrset was walked normally, so the language looked supported. Measured on a 320-file Nix repository, 253 of whose files carry a function header: before after Function 48 1900 CALLS 56 1443 files w/defs 8 258 Adding CBM_LANG_NIX to descend_into_func is sufficient. Descending does not over-mint from curried lambdas: in `f = a: b: ...` the inner `b:` has a function_expression parent, resolves no name, and stays out. Checked clean for over-minting on map/foldl' arguments, `with` bodies, `inherit`, and parenthesized lambdas; rec attrsets, nested attrsets, `let` inside a function body, and factory-returned attrsets all mint correctly. Call attribution is unaffected. CALLS scope is computed by a separate traversal (push_boundary_scopes -> compute_func_qn) that calls the same shared cbm_resolve_func_name this path uses, so definition names and call-scope names agree by construction, and a scope is pushed only when that name resolves non-NULL. The edge-count rise is previously-dropped edges being written, not re-attributed ones: before the fix those edges named a source node that did not exist. The change is gated on ctx->language, so no other language's walk is affected. Signed-off-by: Jason Bowman <jason@json64.dev>

… of error `nix_function` was the only Nix extraction test, and it could not fail for the bug the previous commit fixes. Three separate reasons: - its fixture is function-headed — the exact broken shape; - its binding, `hello = pkgs.writeShellScriptBin "hello" ''...''`, is an application, not a function, so no Function def is expected from it either way; - it asserts only ASSERT_NOT_NULL and ASSERT_FALSE(has_error), omitting the has_def check every adjacent language test makes. So the suite reported green while definitions were being dropped for the dominant file shape in the ecosystem. This keeps `nix_function` as-is — it usefully pins the "parses, yields no def" case — and adds one test per root shape, each asserting its own definitions so a regression names the shape it broke: nix_defs_in_let_rooted_file let ... in nix_defs_in_attrset_rooted_file { ... } nix_defs_in_nested_let let inside let nix_defs_survive_function_header_let { prelude }: let ... in nix_defs_survive_function_header_attrset { prelude }: { ... } nix_defs_survive_curried_header final: prev: { ... } nix_curried_lambda_mints_one_def guards the other direction The curried-header case is the nixpkgs overlay signature, the most common multi-arm header in the ecosystem: two nested function_expressions sit between the file root and the body. The last test exists because the fix descends past the header — it pins that `iota = a: b: a + b` mints exactly one Function, so a change that starts minting the inner arm as a second def is caught. Verified as a negative control — with the previous commit's one-line change reverted, the three function-header tests fail on their own assertions (gamma, eta, iota) while the pre-existing shapes stay green. With it applied the extraction suite is 260 passed, 0 failed. Signed-off-by: Jason Bowman <jason@json64.dev>

DeusData · 2026-07-28T19:04:11Z

Thank you for the contribution and for isolating the Nix function-root extraction gap with fail-before coverage and real-repository validation. This is now triaged as a high-priority parsing-quality bug for 0.9.1-rc. Our community PR queue is currently quite full, so it may take a little time before we can complete the review and, if approved, merge it. We are doing our best to support community contributions and will return with code-grounded feedback as capacity opens.

…emory-mcp Adds a second carry-patch beside the function-header one, kept as separate files so each can be dropped independently as its upstream PR lands. A Nix binding's name is a PATH, and the extractor took only its first segment. Three defects followed. `setA = { dup = …; }` and `setB = { dup = …; }` collapsed onto a single node — the second definition, and every CALLS edge sourced from it, was silently discarded at write. `a.b.fn = …` was named `a`, colliding with every other binding whose path began `a`. `"kebab-case" = …` kept its quote characters in the name. The fix adopts the convention already used for C++ namespaces: name is the leaf segment, qualified_name is enclosing scope plus leaf. It also mints Variable nodes for Nix bindings, which were unconditionally absent — `nix_var_types` declared `binding` but nothing could name one. Scope is bounded to file level (the `let` bindings and the returned attrset), matching how extract_variables treats every other language: C++ mints file-scope declarations and never locals. Without that bound every `enable = true` in an aspect's settings tree would become a node. Measured here: 226 Nix Variables across 499 .nix files, at most 9 in any one file. Cumulative with the function-header patch, on this repository at mode=full: Function 293 -> 684, CALLS 204 -> 327, Nix Variables 0 -> 226. On den-hoag: Function 48 -> 2213, CALLS 56 -> 1461. The Function rise from 1900 to 2213 there is the collision fix alone — definitions that previously shared a qualified name with a sibling and were dropped. Upstream PRs: DeusData/codebase-memory-mcp#1304 (function header) and #1305 (attrpath and Variables), from fork sini/codebase-memory-mcp. Verified end to end by indexing with the store-path binary this overlay produces, not the dev build.

sini requested a review from DeusData as a code owner July 27, 2026 20:33

sini force-pushed the spike/nix-function-header-defs branch from 115811e to ecde318 Compare July 27, 2026 20:41

sini mentioned this pull request Jul 27, 2026

fix(extract): model Nix attrpaths (leaf name, scoped QN) and mint Variables for module-level bindings #1305

Open

4 tasks

sini added 2 commits July 27, 2026 17:14

sini force-pushed the spike/nix-function-header-defs branch from ecde318 to c7cb8c4 Compare July 28, 2026 00:14

DeusData added the bug Something isn't working label Jul 28, 2026

DeusData added this to the 0.9.1-rc milestone Jul 28, 2026

DeusData added parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 28, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(extract): Nix definitions are dropped in any file whose root expression is a function - #1304

fix(extract): Nix definitions are dropped in any file whose root expression is a function#1304
sini wants to merge 2 commits into
DeusData:mainfrom
sini:spike/nix-function-header-defs

sini commented Jul 27, 2026 •

edited

Loading

Uh oh!

DeusData commented Jul 28, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sini commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

The bug

Why the suite didn't catch it

The fix

Tests

Impact

Checklist

Pre-existing limitations this fix makes visible

Uh oh!

DeusData commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sini commented Jul 27, 2026 •

edited

Loading

DeusData commented Jul 28, 2026 •

edited

Loading