fix: classify destructured bindings as constant, not function

crates/codegraph-core/src/extractors/javascript.rs

@@ -2507,6 +2507,19 @@ fn handle_object_literal_shorthand_value_ref(node: &Node, source: &[u8], calls:
     });
 }
 
+/// Extract definitions from destructured object bindings: `const { handleToken,
+/// checkPermissions } = initAuth(...)` creates definitions for `handleToken`
+/// and `checkPermissions`, kind `constant` — matching the convention for plain
+/// `const x = <literal>` bindings and array-pattern destructuring.
+///
+/// Every call site of this function is already gated to `const` declarations
+/// (never `let`/`var`), so `constant` is unconditionally correct here. Prior to
+/// #1773 this used `kind: "function"` on the theory that destructured names
+/// are usually callbacks, but that miscategorized every non-function
+/// destructured value (e.g. `const { dbPath } = workerData`). `constant`-kind
+/// nodes remain fully resolvable as call targets — call-target resolution is
+/// kind-agnostic — so callback-style destructured bindings still resolve.
+/// Mirrors the TS extractor's `extractDestructuredBindings`.
 fn extract_destructured_bindings(
     pattern: &Node,
     source: &[u8],
@@ -2520,7 +2533,7 @@ fn extract_destructured_bindings(
             "shorthand_property_identifier_pattern" | "shorthand_property_identifier" => {
                 definitions.push(Definition {
                     name: node_text(&child, source).to_string(),
-                    kind: "function".to_string(),
+                    kind: "constant".to_string(),
                     line,
                     end_line: Some(end_line),
                     decorators: None,
@@ -2536,7 +2549,7 @@ fn extract_destructured_bindings(
                     {
                         definitions.push(Definition {
                             name: node_text(&value, source).to_string(),
-                            kind: "function".to_string(),
+                            kind: "constant".to_string(),
                             line,
                             end_line: Some(end_line),
                             decorators: None,
@@ -4565,12 +4578,33 @@ mod tests {
 
     #[test]
     fn extracts_destructured_const_bindings() {
+        // kind is "constant" (#1773), not "function" — matches the plain
+        // `const x = <literal>` and array-pattern destructuring convention.
+        // Destructured names remain resolvable as call targets regardless of
+        // kind (call-target resolution is kind-agnostic), so callback-style
+        // destructured bindings like `handleToken` still resolve when called.
         let s = parse_js("const { handleToken, checkPermissions } = initAuth(config);");
         let names: Vec<&str> = s.definitions.iter().map(|d| d.name.as_str()).collect();
         assert!(names.contains(&"handleToken"), "should extract handleToken definition");
         assert!(names.contains(&"checkPermissions"), "should extract checkPermissions definition");
         let ht = s.definitions.iter().find(|d| d.name == "handleToken").unwrap();
-        assert_eq!(ht.kind, "function");
+        assert_eq!(ht.kind, "constant");
+    }
+
+    #[test]
+    fn extracts_non_renamed_destructured_bindings_with_kind_constant() {
+        // Regression guard for issue #1773: plain (non-renamed) destructured
+        // bindings from a non-call RHS (e.g. `workerData`) must not default to
+        // kind "function" — they hold arbitrary values, not callables.
+        let s = parse_js("const { dbPath, name, force } = workerData;");
+        for expected in ["dbPath", "name", "force"] {
+            let def = s
+                .definitions
+                .iter()
+                .find(|d| d.name == expected)
+                .unwrap_or_else(|| panic!("should extract {expected} definition"));
+            assert_eq!(def.kind, "constant");
+        }
     }
 
     #[test]
@@ -4611,7 +4645,13 @@ mod tests {
     #[test]
     fn extracts_renamed_destructured_binding() {
         let s = parse_js("const { original: renamed } = initAuth();");
-        assert!(s.definitions.iter().any(|d| d.name == "renamed"), "should use the local alias");
+        let renamed = s
+            .definitions
+            .iter()
+            .find(|d| d.name == "renamed")
+            .expect("should use the local alias");
+        // kind is "constant" (#1773) — see comment on extracts_destructured_const_bindings.
+        assert_eq!(renamed.kind, "constant");
         assert!(!s.definitions.iter().any(|d| d.name == "original"), "should not use the original key");
     }
 

src/extractors/javascript.ts

@@ -1112,7 +1112,23 @@ function handleTypeAliasDecl(node: TreeSitterNode, ctx: ExtractorOutput): void {
 /**
  * Extract definitions from destructured object bindings.
  * `const { handleToken, checkPermissions } = initAuth(...)` creates definitions
- * for handleToken and checkPermissions so they can be resolved as call targets.
+ * for handleToken and checkPermissions, kind `constant` — matching the
+ * convention for plain `const x = <literal>` bindings (handleConstIdentifierAssignment)
+ * and array-pattern destructuring (the sibling branch in the callers below).
+ *
+ * Every call site of this function is already gated to `const` declarations
+ * (never `let`/`var`), so `constant` is unconditionally correct here — there is
+ * no live binding-mutability to branch on. Prior to #1773 this used `kind:
+ * 'function'` on the theory that destructured names are usually callbacks, but
+ * that miscategorized every non-function destructured value (e.g. `const {
+ * dbPath } = workerData`), which polluted `--kind function` queries and caused
+ * the dead-code classifier to misjudge them via the wrong kind's heuristics.
+ * `constant`-kind nodes remain fully resolvable as call targets — call-target
+ * resolution (`resolveByGlobal`'s exact by-name lookup) is kind-agnostic, and
+ * `constant` is already in the caller-attribution fallback tier
+ * (`TOP_LEVEL_BINDING_KINDS` in call-resolver.ts) — so callback-style
+ * destructured bindings (`const { handleToken } = router; handleToken(req)`)
+ * still resolve correctly.
  */
 function extractDestructuredBindings(
   pattern: TreeSitterNode,
@@ -1128,15 +1144,15 @@ function extractDestructuredBindings(
       child.type === 'shorthand_property_identifier'
     ) {
       // { handleToken } — shorthand binding
-      definitions.push({ name: child.text, kind: 'function', line, endLine });
+      definitions.push({ name: child.text, kind: 'constant', line, endLine });
     } else if (child.type === 'pair_pattern' || child.type === 'pair') {
       // { original: renamed } — renamed binding, use the local alias
       const value = child.childForFieldName('value');
       if (
         value &&
         (value.type === 'identifier' || value.type === 'shorthand_property_identifier_pattern')
       ) {
-        definitions.push({ name: value.text, kind: 'function', line, endLine });
+        definitions.push({ name: value.text, kind: 'constant', line, endLine });
       }
     }
   }
@@ -1259,8 +1275,9 @@ function handleConstObjectPatternAssignment(
   ctx: ExtractorOutput,
 ): void {
   // Destructured bindings: const { handleToken, checkPermissions } = initAuth(...)
-  // Each destructured property becomes a function definition so it can be
-  // resolved when passed as a callback (e.g. router.use(handleToken)).
+  // Each destructured property becomes a constant definition (#1773) — still
+  // resolvable when passed as a callback (e.g. router.use(handleToken)), since
+  // call-target resolution is kind-agnostic (see extractDestructuredBindings).
   // Restricted to const to avoid creating spurious definitions for
   // transient let/var destructuring (e.g. let { userId } = parseRequest(req)).
   // Scope guard mirrors extractDestructuredBindingsWalk (query path) and

tests/integration/issue-1773-destructured-binding-kind.test.ts

@@ -0,0 +1,185 @@
+/**
+ * Integration test for #1773: destructured object-pattern binding targets
+ * (renamed and non-renamed) were classified with `kind: "function"` instead
+ * of `kind: "constant"`, regardless of what the destructured value actually
+ * held. Because these bindings had no call-graph edges pointing at them by
+ * name, `codegraph roles --role dead` risked flagging them `dead-unresolved`
+ * (the "genuinely dead callable" bucket) even when read repeatedly elsewhere.
+ *
+ * Root cause: `extractDestructuredBindings` (src/extractors/javascript.ts,
+ * shared by both the walk and query extraction paths) and its native mirror
+ * `extract_destructured_bindings` (crates/codegraph-core/src/extractors/
+ * javascript.rs) hardcoded `kind: 'function'` for every object-pattern
+ * binding target, on the theory that destructured names are usually
+ * callbacks. That miscategorized any destructured value that wasn't a
+ * function (e.g. `const { dbPath } = workerData`).
+ *
+ * Fix: both engines now emit `kind: 'constant'`, matching the existing
+ * convention for plain `const x = <literal>` bindings and array-pattern
+ * destructuring. Constants remain fully resolvable as call targets (call-
+ * target resolution is kind-agnostic), so callback-style destructured
+ * bindings still resolve; and constants with active call-graph-connected
+ * siblings in the same file are classified `leaf`, not `dead-unresolved` —
+ * exactly like any other same-file constant.
+ */
+
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import Database from 'better-sqlite3';
+import { afterAll, beforeAll, describe, expect, it } from 'vitest';
+import { buildGraph } from '../../src/domain/graph/builder.js';
+import { isNativeAvailable } from '../../src/infrastructure/native.js';
+
+// Repro 1 (renamed destructuring, issue #1773): a file with other real,
+// call-graph-connected functions (giving it "active file siblings") plus a
+// renamed destructured binding read repeatedly afterward — mirrors
+// scripts/token-benchmark.ts's `const { values: flags } = parseArgs(...)`.
+// Repro 2 (non-renamed destructuring, issue #1773): an isolated worker-style
+// file with *only* a destructured binding from a non-call RHS and no other
+// callables — mirrors tests/unit/snapshot-race-worker.mjs's
+// `const { dbPath, name, force } = workerData`.
+const FIXTURE = {
+  'renamed-repro.js': `
+function parseArgs(opts) { return { values: computeDefaults(opts) }; }
+function computeDefaults(opts) { return { runs: 3, model: opts.model }; }
+
+const { values: flags } = parseArgs({ model: 'x' });
+
+function main() {
+  console.log(flags.runs, flags.model);
+}
+main();
+`,
+  'worker-repro.mjs': `
+const { dbPath, name, force } = workerData;
+save(name, { dbPath, force });
+`,
+};
+
+function readNode(dbPath: string, file: string, name: string) {
+  const db = new Database(dbPath, { readonly: true });
+  try {
+    return db
+      .prepare('SELECT name, kind, role FROM nodes WHERE file = ? AND name = ?')
+      .get(file, name) as { name: string; kind: string; role: string | null } | undefined;
+  } finally {
+    db.close();
+  }
+}
+
+function expectFixedKindAndRole(dbPath: string) {
+  // Repro 1: renamed destructured binding, active file siblings present.
+  const flags = readNode(dbPath, 'renamed-repro.js', 'flags');
+  expect(flags, 'flags node not found').toBeDefined();
+  expect(flags!.kind, 'flags must be kind constant, not function').toBe('constant');
+  expect(
+    flags!.role,
+    `flags was classified as ${flags!.role} — must not be dead-unresolved`,
+  ).not.toBe('dead-unresolved');
+
+  // Repro 2: non-renamed destructured bindings, no active file siblings.
+  for (const varName of ['dbPath', 'name', 'force']) {
+    const node = readNode(dbPath, 'worker-repro.mjs', varName);
+    expect(node, `${varName} node not found`).toBeDefined();
+    expect(node!.kind, `${varName} must be kind constant, not function`).toBe('constant');
+    // With no other call-graph-connected callable in the file, these fall
+    // back to 'dead-leaf' — the same honest classification any isolated,
+    // unreferenced-by-calls constant gets (properties/constants are leaf
+    // nodes by definition; call-graph reachability can't prove liveness for
+    // pure value bindings). The bug this test guards against is the far more
+    // misleading 'dead-unresolved' ("genuinely dead callable") label that a
+    // wrong kind: 'function' classification used to produce.
+    expect(
+      node!.role,
+      `${varName} was classified as ${node!.role} — must not be dead-unresolved`,
+    ).not.toBe('dead-unresolved');
+  }
+}
+
+describe('destructured binding kind classification (#1773) — WASM', () => {
+  let tmpDir: string;
+
+  beforeAll(async () => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1773-'));
+    for (const [rel, content] of Object.entries(FIXTURE)) {
+      fs.writeFileSync(path.join(tmpDir, rel), content);
+    }
+    await buildGraph(tmpDir, { engine: 'wasm', incremental: false, skipRegistry: true });
+  });
+
+  afterAll(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+  });
+
+  it('classifies renamed and non-renamed destructured bindings as kind constant, not dead-unresolved', () => {
+    expectFixedKindAndRole(path.join(tmpDir, '.codegraph', 'graph.db'));
+  });
+
+  it('still resolves calls made through a destructured callback-style binding', () => {
+    // `flags.runs`/`flags.model` are property reads, not calls, but `flags`
+    // itself must still show up as the attributed caller of `parseArgs` — the
+    // fix must not break caller attribution for the top-level binding.
+    const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
+    const db = new Database(dbPath, { readonly: true });
+    try {
+      const row = db
+        .prepare(
+          `SELECT COUNT(*) AS cnt
+           FROM edges e
+           JOIN nodes s ON e.source_id = s.id
+           JOIN nodes t ON e.target_id = t.id
+           WHERE s.name = 'flags' AND t.name = 'parseArgs' AND e.kind = 'calls'`,
+        )
+        .get() as { cnt: number };
+      expect(row.cnt, 'expected flags -> parseArgs calls edge to survive the kind fix').toBe(1);
+    } finally {
+      db.close();
+    }
+  });
+});
+
+describe.skipIf(!isNativeAvailable())(
+  'destructured binding kind classification (#1773) — native',
+  () => {
+    let tmpDir: string;
+
+    beforeAll(async () => {
+      tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-1773-native-'));
+      for (const [rel, content] of Object.entries(FIXTURE)) {
+        fs.writeFileSync(path.join(tmpDir, rel), content);
+      }
+      await buildGraph(tmpDir, { engine: 'native', incremental: false, skipRegistry: true });
+    }, 60_000);
+
+    afterAll(() => {
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    });
+
+    it('classifies renamed and non-renamed destructured bindings as kind constant, not dead-unresolved', () => {
+      expectFixedKindAndRole(path.join(tmpDir, '.codegraph', 'graph.db'));
+    });
+
+    it('still resolves calls made through a destructured callback-style binding', () => {
+      // `flags.runs`/`flags.model` are property reads, not calls, but `flags`
+      // itself must still show up as the attributed caller of `parseArgs` — the
+      // fix must not break caller attribution for the top-level binding.
+      const dbPath = path.join(tmpDir, '.codegraph', 'graph.db');
+      const db = new Database(dbPath, { readonly: true });
+      try {
+        const row = db
+          .prepare(
+            `SELECT COUNT(*) AS cnt
+             FROM edges e
+             JOIN nodes s ON e.source_id = s.id
+             JOIN nodes t ON e.target_id = t.id
+             WHERE s.name = 'flags' AND t.name = 'parseArgs' AND e.kind = 'calls'`,
+          )
+          .get() as { cnt: number };
+        expect(row.cnt, 'expected flags -> parseArgs calls edge to survive the kind fix').toBe(1);
+      } finally {
+        db.close();
+      }
+    });
+  },
+);

tests/parsers/javascript.test.ts

@@ -1002,22 +1002,43 @@ describe('JavaScript parser', () => {
 
     // Destructured bindings
     it('extracts definitions from destructured const bindings', () => {
+      // kind is 'constant' (#1773), not 'function' — matches the plain
+      // `const x = <literal>` and array-pattern destructuring convention.
+      // Destructured names remain resolvable as call targets regardless of
+      // kind (call-target resolution is kind-agnostic), so callback-style
+      // destructured bindings like `handleToken` still resolve when called.
       const symbols = parseJS(`const { handleToken, checkPermissions } = initAuth(config);`);
       expect(symbols.definitions).toContainEqual(
-        expect.objectContaining({ name: 'handleToken', kind: 'function' }),
+        expect.objectContaining({ name: 'handleToken', kind: 'constant' }),
       );
       expect(symbols.definitions).toContainEqual(
-        expect.objectContaining({ name: 'checkPermissions', kind: 'function' }),
+        expect.objectContaining({ name: 'checkPermissions', kind: 'constant' }),
       );
     });
 
     it('extracts definitions from exported destructured const bindings', () => {
       const symbols = parseJS(`export const { handleToken, checkPermissions } = initAuth(config);`);
       expect(symbols.definitions).toContainEqual(
-        expect.objectContaining({ name: 'handleToken', kind: 'function' }),
+        expect.objectContaining({ name: 'handleToken', kind: 'constant' }),
       );
       expect(symbols.definitions).toContainEqual(
-        expect.objectContaining({ name: 'checkPermissions', kind: 'function' }),
+        expect.objectContaining({ name: 'checkPermissions', kind: 'constant' }),
+      );
+    });
+
+    it('extracts non-renamed destructured const bindings with kind constant (#1773)', () => {
+      // Regression guard for issue #1773: plain (non-renamed) destructured
+      // bindings from a non-call RHS (e.g. `workerData`) must not default to
+      // kind 'function' — they hold arbitrary values, not callables.
+      const symbols = parseJS(`const { dbPath, name, force } = workerData;`);
+      expect(symbols.definitions).toContainEqual(
+        expect.objectContaining({ name: 'dbPath', kind: 'constant' }),
+      );
+      expect(symbols.definitions).toContainEqual(
+        expect.objectContaining({ name: 'name', kind: 'constant' }),
+      );
+      expect(symbols.definitions).toContainEqual(
+        expect.objectContaining({ name: 'force', kind: 'constant' }),
       );
     });
 
@@ -1034,9 +1055,10 @@ describe('JavaScript parser', () => {
     });
 
     it('extracts renamed destructured const binding under its local alias', () => {
+      // kind is 'constant' (#1773) — see comment on the non-renamed case above.
       const symbols = parseJS(`const { original: renamed } = initAuth();`);
       expect(symbols.definitions).toContainEqual(
-        expect.objectContaining({ name: 'renamed', kind: 'function' }),
+        expect.objectContaining({ name: 'renamed', kind: 'constant' }),
       );
       expect(symbols.definitions).not.toContainEqual(expect.objectContaining({ name: 'original' }));
     });