fix(callgraph): extract module-top-level calls to fix rc undercount (closes #210)

docs/design/0210-module-level-call-extraction.md

@@ -0,0 +1,174 @@
+# Design Doc: Module-top-level Call Extraction (Issue #210)
+
+> **Status:** Proposed
+> **Date:** 2026-07-12
+> **Author:** Wolfvin
+> **Related issues:** #210
+> **Related PRs:** (this PR)
+
+---
+
+## Problem
+
+CodeLens `reference_count` (`rc` field in `search --mode symbol` output) was
+severely undercounting cross-file calls for any function only called via
+the two patterns below — both extremely common in modern Express/Router
+modular backends:
+
+1. **Middleware-factory argument pattern.** A function call passed as an
+   argument to another call at module top-level, e.g.
+   `router.post(path, requirePermission('admin'), handler)`. The
+   `requirePermission('admin')` call is inside `router.post(...)`'s
+   argument list — at module top-level, not inside any function body.
+
+2. **Inline arrow callback body pattern.** A function call inside an
+   inline arrow function passed as a callback, e.g. the route handler
+   `(req, res) => { if (hasPermission(...)) {...} }`. The arrow function
+   is NOT assigned to a variable, so it's not registered as a function
+   declaration, and its body is never walked for call extraction.
+
+Concrete cases reported in issue #210 (verified against the
+Coretax-Auto-Downloader / KDS backend):
+
+- `requirePermission` (middleware factory in `src/middleware/permission-gate.ts`)
+  was reported with `rc: 0`. Ground truth: 13+ call sites across route files
+  via `router.post(path, requirePermission('admin'), handler)` at module top-level.
+  Verified via SQLite: `SELECT * FROM graph_edges WHERE target_id = ...`
+  returned 0 rows.
+
+- `hasPermission` (utility in `src/lib/permissions.ts`) was reported with
+  `rc: 1`. Ground truth: 7 call sites in 3 files (3 in `assignments.ts`,
+  3 in `task-templates.ts`, 1 in `permission-gate.ts`). Only the call
+  inside `requirePermission`'s direct body was extracted.
+
+Both bugs have the same root cause: the TS/JS backend parsers' per-function
+pass only walks bodies of registered function/class declarations. Module-
+top-level calls and inline arrow callback bodies are invisible.
+
+## Goal
+
+`search --mode symbol` reports `ref_count` that matches the actual number
+of call sites for functions called via the two patterns above, without
+regressing any existing callgraph consumer (dead-code, impact, trace).
+
+## Changes
+
+### Architecture / Data Model
+
+Introduce a **module-level pass** in the TS/JS backend parsers, run after
+the per-function pass. The pass walks the AST root (`program` node) and
+extracts every `call_expression` / `new_expression` that hasn't already
+been covered by the per-function pass, by skipping the subtrees of
+registered declarations:
+
+- `function_declaration` / `generator_function_declaration`
+- `class_declaration` (its `class_body` is walked by the per-function pass)
+- `variable_declarator` whose value is `arrow_function` /
+  `function_expression` / a `defineStore(...)` call (Pinia store pattern)
+
+Everything else — including inline arrow functions, call arguments, and
+non-function `variable_declarator` values like `const router = Router()` —
+is walked normally, so calls inside them are extracted.
+
+Edges from module-level calls use a **synthetic source_id** of the form
+`<file>:0:<module>`, with NO corresponding `graph_nodes` entry. This
+mirrors the convention already used by
+`hybrid_type_resolver._file_node_id` for IMPORTS edges. Benefits:
+
+- `ref_count` (computed from the target side) is correct — each call
+  site produces one edge, incrementing the target's count.
+- `cmd_list` and `search --mode symbol` output remains clean — no fake
+  `<module>` function entries appear in user-facing output.
+- `dead-code` correctly marks previously-falsely-dead functions as
+  active (their `ref_count > 0` now).
+- `trace` / `impact` continue to work without crash — their JOINs on
+  `source_id = node_id` simply produce no rows for `<module>` sources,
+  same as the existing `<rel_path>:0:file` sources for IMPORTS edges.
+  This is NOT a regression: before the fix, these commands also saw 0
+  callers for `requirePermission` because no CALLS edges existed at all.
+
+### Modified Files
+
+- `scripts/parsers/ts_backend_parser.py` — add `_find_module_level_calls`
+  method; call it from `extract_references` after the per-function pass;
+  update docstring.
+- `scripts/parsers/js_backend_parser.py` — add `_find_module_level_calls`
+  method (using the parser's iterative DFS + `keep_alive` pattern from
+  issue #116/#163); call it from `extract_references` after the main
+  walk loop; update docstring.
+- `scripts/parsers/fallback_js_backend.py` — add module-level scan for
+  lines not covered by any function's approximate scope; emit edges with
+  synthetic `<module>` source.
+
+### Tests
+
+- `tests/test_js_backend_parser.py::TestIssue210ModuleLevelCalls` — 6 new
+  tests covering middleware-factory argument, inline arrow callback body,
+  multi-site accumulation, no-double-count for function bodies, sanity
+  extraction of `router.post` itself.
+- `tests/test_ts_backend_parser.py` — new file with 10 tests covering
+  baseline TS parsing plus the same issue #210 patterns (TS is the
+  primary fix target since the KDS backend is TypeScript).
+
+## Trade-offs
+
+### Alternative A: Create a synthetic `<module>` node per file
+
+- **Pros:** Trace/impact commands would see real caller counts via
+  SQLite JOINs (source_id would match a real graph_nodes row).
+- **Cons:** Adds fake function entries to the backend registry. Would
+  require modifying `commands/list.py` and `search_engine.py` to filter
+  them out — violating the issue constraint "Jangan ubah command files
+  (commands/*.py) kecuali memang diperlukan". The `test_list_backend_only`
+  test would also need updating.
+- **Why rejected:** Violates issue constraint. The fix should stay scoped
+  to parsers.
+
+### Alternative B: Modify the per-function pass to also walk module top-level
+
+- **Pros:** Single-pass design; no separate module-level walk needed.
+- **Cons:** Would require invasive changes to the existing walk logic in
+  both parsers. The TS parser's two-pass design (declarations then calls)
+  and the JS parser's single-pass design (with issue #116 SIGSEGV
+  mitigation) both have non-trivial structure that doesn't naturally
+  accommodate "also extract calls outside function bodies".
+- **Why rejected:** Higher regression risk; larger blast radius.
+
+### Alternative C: Register inline arrow callbacks as anonymous function declarations
+
+- **Pros:** Per-function pass would naturally walk their bodies.
+- **Cons:** Anonymous functions have no name — can't be registered as
+  function nodes with a meaningful `fn` field. Would require generating
+  synthetic names, polluting the registry.
+- **Why rejected:** Pollutes the function registry with anonymous entries.
+
+### Chosen approach: Separate module-level pass with synthetic source_id
+
+- **Why:** Minimal blast radius (parsers only). Follows existing precedent
+  for synthetic source_ids (hybrid_type_resolver._file_node_id). No
+  changes to command files or the registry shape. Satisfies the issue's
+  DoD points 1 and 2 (correct `rc` in `search --mode symbol`).
+
+## Open Questions
+
+- [ ] Should `trace` and `impact` eventually be updated to follow
+  `source_id = '<file>:0:<module>'` edges and report module-level callers?
+  Currently they skip these dangling sources (same as IMPORTS edges).
+  This is a follow-up enhancement, not a regression — filed as a future
+  improvement once issue #210 lands.
+
+## Migration / Rollout
+
+No migration impact — additive change. Existing callgraphs gain new edges
+for previously-missed call sites; `ref_count` values increase for affected
+functions. Functions previously marked "dead" by `audit --check dead-code`
+may now be correctly marked "active" if their only callers were module-
+top-level. This is a correctness improvement, not a breaking change.
+
+## References
+
+- Issue: #210
+- Prior art: `scripts/hybrid_type_resolver.py:_file_node_id` — synthetic
+  source_id convention for IMPORTS edges.
+- Related design docs: [0004-graph-model](0004-graph-model.md) (graph_edges
+  schema, source_id semantics)

scripts/parsers/fallback_js_backend.py

@@ -170,16 +170,37 @@ def parse_js_backend_fallback(content, file_path):
     # Detect function calls (simplified — within function bodies)
     # For each function found, scan its approximate scope for calls
     lines = content.split('\n')
+    covered_lines = [False] * len(lines)
     for node in nodes:
         start_line = node["line"] - 1  # 0-indexed
         # Approximate: scan from function start to next function or 50 lines
         end_line = min(start_line + 50, len(lines))
         for i in range(start_line, end_line):
             if i >= len(lines):
                 break
+            covered_lines[i] = True
             for m in re.finditer(r'\b([a-zA-Z_]\w*)\s*\(', lines[i]):
                 call_name = m.group(1)
                 if call_name not in skip_names and call_name != node["fn"]:
                     edges.append({"from": node["id"], "to_fn": call_name})
 
+    # Issue #210: module-top-level calls. The per-function loop above only
+    # scans lines inside an approximate function scope. Calls at file root
+    # (e.g., `router.post(path, requirePermission('admin'), handler)` in an
+    # Express modular route file) and calls inside inline arrow-function
+    # callbacks that aren't assigned to a variable are missed entirely,
+    # undercounting reference_count for any function only called via those
+    # patterns. Scan remaining uncovered lines and emit edges with a
+    # synthetic `<module>` caller (no node entry — mirrors the IMPORTS-edge
+    # convention in hybrid_type_resolver._file_node_id).
+    module_node_id = f"{file_path}:0:<module>"
+    for i, line in enumerate(lines):
+        if covered_lines[i]:
+            continue
+        for m in re.finditer(r'\b([a-zA-Z_]\w*)\s*\(', line):
+            call_name = m.group(1)
+            if call_name in skip_names:
+                continue
+            edges.append({"from": module_node_id, "to_fn": call_name})
+
     return {"nodes": nodes, "edges": edges}

scripts/parsers/js_backend_parser.py

@@ -9,7 +9,10 @@
 - async variants of all above
 - Method calls: obj.method() → tracked as "method"
 - Member expression calls: HttpClient.new()
-- Anonymous/inline callbacks → IGNORED
+- Module-top-level calls (issue #210): router.post(...) at file root
+- Calls inside inline arrow/function-expression callbacks (issue #210):
+  e.g., the (req, res) => {...} handler passed to router.post(...)
+- Anonymous/inline callbacks -> body calls now extracted (issue #210)
 - Built-in keywords filtered out
 """
 
@@ -202,6 +205,39 @@
                         keep_alive.append(child)
                         stack.append((child, depth + 1))
 
+            # Issue #210: module-top-level pass.
+            # The main walk above only extracts calls via _parse_and_collect_calls,
+            # which is invoked on declaration nodes (function_declaration,
+            # variable_declarator, class_declaration, export_statement children).
+            # Calls that live outside any registered declaration body are missed:
+            #   - Module-top-level calls (e.g. `router.post(path, fn, handler)` at
+            #     file root) — including calls passed as arguments like
+            #     `requirePermission('admin')` inside `router.post(...)`.
+            #   - Calls inside inline arrow/function-expression callbacks (e.g.
+            #     the `(req, res) => { hasPermission(...) }` handler passed to
+            #     `router.post(...)`).
+            # Walk the root again, skipping subtrees of registered declarations
+            # to avoid double-count, and extract remaining call_expression /
+            # new_expression nodes with a synthetic `<module>` caller.
+            module_calls = self._find_module_level_calls(root, source, file_path)
+            if module_calls:
+                # Synthetic source_id (no corresponding node entry) — mirrors
+                # the convention used by hybrid_type_resolver._file_node_id for
+                # IMPORTS edges. CALLS-traversal code naturally skips dangling
+                # source_ids, and ref_count is computed from the target side,
+                # so no node is needed. This keeps `list`/`search` output free
+                # of fake `<module>` function entries.
+                module_node_id = f"{file_path}:0:<module>"
+                for call_info in module_calls:
+                    edge = {
+                        "from": module_node_id,
+                        "to_fn": call_info["fn_name"],
+                        "via_self": call_info.get("via_self", False)
+                    }
+                    if call_info.get("is_ipc_call"):
+                        edge["is_ipc_call"] = True
+                    edges.append(edge)
+
             return {"nodes": nodes, "edges": edges, "skipped": []}
         finally:
             if _gc_was_enabled:
@@ -505,6 +541,105 @@
 
         return calls
 
+    def _find_module_level_calls(self, root: Node, source: bytes,
+                                  file_path: str) -> List[Dict]:
+        """Find call sites that live outside any registered function declaration.
+
+        Issue #210: the main walk in :meth:`extract_references` only extracts
+        calls via :meth:`_parse_and_collect_calls`, which is invoked on
+        declaration nodes. Two patterns slip through and cause
+        reference_count undercounting:
+
+        1. Module-top-level calls. Express/Router modular route files typically
+           do `router.post(path, requirePermission('admin'), handler)` at the
+           file root — not inside any function. The whole call expression
+           (including the `requirePermission('admin')` argument) is invisible
+           to the per-function pass.
+        2. Calls inside inline arrow/function-expression callbacks. Route
+           handlers like `(req, res) => { hasPermission(...) }` are passed as
+           arguments to `router.post(...)` and are NOT registered as function
+           declarations (they're not assigned to a name), so the per-function
+           pass never visits their bodies.
+
+        This pass walks the AST root and extracts every call_expression /
+        new_expression that hasn't already been covered, by skipping the
+        subtrees of registered declarations:
+
+          - function_declaration / generator_function_declaration
+          - class_declaration (its method_definition bodies are walked by the
+            per-function pass via the class_body)
+          - variable_declarator whose value is arrow_function /
+            function_expression / defineStore(...) call (registered as a
+            function node or Pinia store)
+
+        Everything else — including inline arrow functions and the argument
+        lists of call_expressions — is walked normally, so calls inside them
+        are extracted with a synthetic `<module>` caller.
+
+        Uses the same iterative DFS + ``keep_alive`` pattern as
+        :meth:`_find_calls_in_scope` (issue #116/#163) so tree-sitter Node
+        references don't dangle mid-walk.
+        """
+        calls: List[Dict] = []
+        MAX_DEPTH = 200
+
+        def _is_registered_value(child: Node) -> bool:
+            """Return True if a variable_declarator child is a registered value.
+
+            Registered values are arrow_function, function_expression, or a
+            defineStore(...) call_expression (Pinia store pattern).
+            """
+            if child.type in ('arrow_function', 'function_expression'):
+                return True
+            if child.type == 'call_expression':
+                func_node = child.child_by_field_name('function')
+                if func_node:
+                    fn_text = source[func_node.start_byte:func_node.end_byte].decode(
+                        'utf-8', errors='replace')
+                    if fn_text == 'defineStore':
+                        return True
+            return False
+
+        keep_alive: List[Node] = [root]
+        stack: List[Tuple[Node, int]] = [(root, 0)]
+        while stack:
+            node, depth = stack.pop()
+            if depth > MAX_DEPTH:
+                continue
+
+            # Skip subtrees already covered by the per-function pass to
+            # avoid double-counting call sites.
+            skip_subtree = False
+            if node.type in ('function_declaration',
+                             'generator_function_declaration',
+                             'class_declaration'):
+                skip_subtree = True
+            elif node.type == 'variable_declarator':
+                for child in node.children:
+                    if _is_registered_value(child):
+                        skip_subtree = True
+                        break
+
+            if skip_subtree:
+                continue
+
+            if node.type == 'call_expression':
+                call_info = self._parse_call(node, source)
+                if call_info:
+                    calls.append(call_info)
+            elif node.type == 'new_expression':
+                call_info = self._parse_new_expression(node, source)
+                if call_info:
+                    calls.append(call_info)
+
+            if depth + 1 <= MAX_DEPTH:
+                children = node.children
+                for child in reversed(children):
+                    keep_alive.append(child)
+                    stack.append((child, depth + 1))
+
+        return calls
+
     def _parse_call(self, node: Node, source: bytes) -> Optional[Dict]:
         """Parse a call_expression node to extract the called function name.