fix: C/C++ function name extraction and remove startLine from nodeId

[Alice523]

• Fix findEnclosingFunction unable to extract C/C++ function/method names from nested AST structure (function_declarator -> qualified_identifier)
• Fix missing PHP built-in filtering in main-thread mode (call-processor.ts had incomplete BUILT_IN_NAMES, causing PHP calls like date() to be tracked)
• Consolidate FUNCTION_NODE_TYPES, extractFunctionName, BUILT_IN_NAMES, isBuiltInOrNoise into utils.ts, reducing duplicate code between call-processor.ts and parse-worker.ts
• Remove startLine from nodeId to match web backend format (startLine is already stored as node property and not used elsewhere)

[vercel]

Someone is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[magyargergo]

Can you please resolve the merge conflicts and add unit and integration tests to cover your changes? 🙏

[Alice523]

Can you please resolve the merge conflicts and add unit and integration tests to cover your changes? 🙏

I've solved the conflicts.

[magyargergo]

Thank you! I made a change recently, could you please merge origin/main into this PR? 🙏

[github-actions]

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	success	tsc --noEmit
✅ Unit Tests	success	3 platforms
✅ Integration	success	3 OS x 4 groups = 12 jobs

Test Results

✅ 1015 passed
· 293 suites · 1015 total
· ⏱️ 18s
· 📊 865 unit + 150 integration

Code Coverage

Combined (Unit + Integration)

Metric	Coverage	Covered	Threshold	Status
Statements	38.25%	2328/6085	26%	🟢 ███████░░░░░░░░░░░░░
Branches	31.17%	1270/4074	23%	🟢 ██████░░░░░░░░░░░░░░
Functions	41.85%	280/669	28%	🟢 ████████░░░░░░░░░░░░
Lines	39.52%	2191/5543	27%	🟢 ███████░░░░░░░░░░░░░

Coverage breakdown by test suite

Unit Tests

Metric	Coverage	Covered	Threshold	Status
Statements	27.06%	1647/6085	26%	🟢 █████░░░░░░░░░░░░░░░
Branches	24.52%	999/4074	23%	🟢 ████░░░░░░░░░░░░░░░░
Functions	29.14%	195/669	28%	🟢 █████░░░░░░░░░░░░░░░
Lines	27.94%	1549/5543	27%	🟢 █████░░░░░░░░░░░░░░░

Integration Tests

Metric	Coverage	Covered	Threshold	Status
Statements	20%	1217/6085	26%	🔴 ████░░░░░░░░░░░░░░░░
Branches	14.75%	601/4074	23%	🔴 ██░░░░░░░░░░░░░░░░░░
Functions	22.57%	151/669	28%	🔴 ████░░░░░░░░░░░░░░░░
Lines	20.72%	1149/5543	27%	🔴 ████░░░░░░░░░░░░░░░░

Coverage thresholds are auto-ratcheted — they only go up

Vitest thresholds.autoUpdate bumps the floor whenever local coverage exceeds it.
CI enforces the current thresholds; developers commit the ratcheted values.

---

_{📋 View full run · Generated by CI}

[magyargergo]

@claude

[claude]

Claude finished @magyargergo's task in 2m 11s —— View job

---

Code Review

• Gather context and read changed files
• Review utils.ts consolidation
• Review call-processor.ts changes
• Review parse-worker.ts changes
• Review test coverage
• Post final review

---

Overall this is a well-structured PR. The C/C++ extraction fix is correct, the consolidation reduces duplication meaningfully, and the PHP built-in filtering fix is a real bug fix. A few issues to address before merging:

Bug: Dead variable in parse-worker.ts

gitnexus/src/core/ingestion/workers/parse-worker.ts, line 276

const { funcName, label } = extractFunctionName(current);
if (funcName) {
  const startLine = current.startPosition?.row ?? 0;  // ← computed but never used
  return generateId(label, `${filePath}:${funcName}`);
}

startLine is assigned but never referenced after removing it from the generateId call. This is dead code introduced by the refactor and should be removed. Fix this →

---

Observation: extractFunctionName silently returns null for several node types in FUNCTION_NODE_TYPES

The following types are in FUNCTION_NODE_TYPES but have no handling branch in extractFunctionName in utils.ts:

• local_function_statement (C#)
• anonymous_function (PHP Kotlin)
• lambda_literal (Kotlin)
• async_arrow_function

These fall through all branches and return { funcName: null, label: 'Function' }. This matches the old behavior (they were also unhandled before), so it's not a regression — just worth a comment in extractFunctionName to make the intent clear. Something like:

// Note: local_function_statement (C#), anonymous_function, lambda_literal, async_arrow_function
// are in FUNCTION_NODE_TYPES but are intentionally unnamed here — they walk up to the parent.

---

Observation: nodeId format change is a breaking migration

Removing startLine from the nodeId in both parsing-processor.ts and parse-worker.ts is the right long-term direction (matching the web backend format), but any existing persisted graph data will have stale IDs. This is fine as long as npx gitnexus analyze is re-run after upgrading. Worth a note in the changelog/release notes.

---

Minor: extractFunctionName is not exported from call-processor.ts but is used internally

This is fine — it's imported from utils.ts and used only in findEnclosingFunction. Just noting the import line correctly includes it: import { ..., extractFunctionName, ... } from './utils.js'. No action needed.

---

Tests: Good coverage, a few gaps

The tests in ingestion-utils.test.ts are solid and well-structured. A few cases that would improve confidence:

1. extractFunctionName with Python function_definition — the new code adds a function_declarator lookup path first, which Python doesn't have. The fallback to childForFieldName('name') should still work, but a test would confirm there's no regression.
2. extractFunctionName for parenthesized_declarator (pointer-to-function, e.g. void (*MyClass::func)()) — this path is added but untested.
3. Arrow function / function expression — const foo = () => {} anonymous naming via parent variable_declarator.

These are enhancement suggestions, not blockers given CI passes with 1007 tests.

---

Summary

#	Severity	Issue
1	🔴 Minor bug	Dead startLine variable in parse-worker.ts:276
2	🟡 Nit	Silent null returns for some FUNCTION_NODE_TYPES could use a comment
3	🟡 Info	nodeId format change requires re-indexing existing graphs
4	🟢 Enhancement	Additional test cases for Python, arrow functions, parenthesized_declarator

The dead variable (#1) should be fixed before merging. The rest are optional improvements.

[magyargergo]

Hi @Alice523,

I merged master into this PR's forked branch. I reviewed claude's comments and I would like ask you address those comments.

Thank you!

[Alice523]

Hi @Alice523,

I merged master into this PR's forked branch. I reviewed claude's comments and I would like ask you address those comments.

Thank you!

Thanks for merging master into this branch. I've addressed all the comments, including removing the deadline and adding the requested notes in utils.ts. Ready for another look!

[Copilot]

Pull request overview

This PR addresses several issues in the code ingestion pipeline: fixing C/C++ function name extraction from nested AST structures, fixing missing PHP built-in filtering in the main-thread processing path, consolidating duplicated constants and utility functions into a single shared location, and removing startLine from node IDs to match the web backend format.

Changes:

• Centralized FUNCTION_NODE_TYPES, BUILT_IN_NAMES, extractFunctionName, and isBuiltInOrNoise from call-processor.ts and parse-worker.ts into utils.ts, eliminating a significant duplication that previously caused PHP built-ins to be missing in the main-thread call-processor.
• Rewrote extractFunctionName in utils.ts to properly traverse the C/C++ AST structure (function_definition → function_declarator → qualified_identifier) to extract both function name and the correct Method/Function label.
• Removed startLine from node IDs in both parse-worker.ts and parsing-processor.ts (and correspondingly in findEnclosingFunctionId/findEnclosingFunction), ensuring IDs now consistently match the format used by the web backend.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
gitnexus/src/core/ingestion/utils.ts	Adds FUNCTION_NODE_TYPES, BUILT_IN_NAMES, isBuiltInOrNoise, and extractFunctionName exports, including the new C/C++ nested AST traversal logic
gitnexus/src/core/ingestion/workers/parse-worker.ts	Removes local FUNCTION_NODE_TYPES and BUILT_INS definitions, imports from utils.ts, and removes startLine from node IDs
gitnexus/src/core/ingestion/parsing-processor.ts	Removes startLine from node ID generation to match the new format
gitnexus/src/core/ingestion/call-processor.ts	Removes local FUNCTION_NODE_TYPES, BUILT_IN_NAMES, and isBuiltInOrNoise definitions, imports the shared versions from utils.ts
gitnexus/test/unit/ingestion-utils.test.ts	Adds tests for the newly exported isBuiltInOrNoise and extractFunctionName functions

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[magyargergo]

@claude

[claude]

Claude finished @magyargergo's task in 2m 45s —— View job

---

Code Review

• Gather context from PR comments and previous reviews
• Read changed files
• Check if previous review issues were addressed
• Fix incorrect Kotlin/PHP comment attribution (copilot's finding)
• Add backward-compatibility warning comment (magyargergo's CHANGES_REQUESTED)
• Post final review

---

I reviewed the current state of the PR after Alice's updates and made two fixes directly on the branch.

What was already addressed ✅

• Dead startLine variable (parse-worker.ts:276) — removed. findEnclosingFunctionId now cleanly returns generateId(label, ...) with no dead assignments.
• Comment for unnamed function types (utils.ts:243–244) — the note about local_function_statement, anonymous_function, lambda_literal, and async_arrow_function walking up to the parent is present.
• PHP built-in filtering — BUILT_IN_NAMES is now shared from utils.ts and includes the full PHP built-in list (date, json_encode, etc.), used by both call-processor.ts and parse-worker.ts.

---

What I fixed in this review

1. Incorrect comment attribution in utils.ts — Fixed (commit 08358f5)

The comment on line 31 said // Kotlin (function_declaration already included above via JS/TS) but it preceded anonymous_function, which is PHP's anonymous function AST node type. Only lambda_literal belongs to Kotlin. Corrected to:

// PHP anonymous functions (anonymous_function is a PHP AST node type)
'anonymous_function',
// Kotlin (function_declaration already included above via JS/TS; lambda_literal is Kotlin-specific)
'lambda_literal',

2. Breaking-change comment in parsing-processor.ts — Fixed (commit 08358f5)

The nodeId format change (removing startLine) is intentional and correct for matching the web backend, but it is a breaking migration for persisted graphs. I added an explicit inline comment to document this:

// NOTE: startLine is intentionally excluded from nodeId to match the web backend format.
// This is a breaking change for persisted graphs — re-run `npx gitnexus analyze` after upgrading.
const nodeId = generateId(nodeLabel, `${file.path}:${nodeName}`);

The same format is already used in parse-worker.ts (the worker path), so both code paths are now consistent and documented.

---

Remaining optional enhancements

These were noted in the previous review and are not blockers given CI passes with 1007 tests:

• Test for extractFunctionName with Python function_definition (no regression risk but would confirm fallback path)
• Test for parenthesized_declarator (pointer-to-function, e.g. void (*MyClass::func)())
• Test for arrow function / function expression anonymous naming via variable_declarator

---

The PR is now in good shape. The two issues from previous reviews (dead variable + Kotlin/PHP comment) are resolved, the breaking change is documented inline, and CI is passing.
· Branch: bugfix/caller

[magyargergo]

Good stuff overall — the consolidation into utils.ts is well done and the C/C++ qualified_identifier fix is the right approach. A few things I noticed going through the changes.

What works well:

• The startLine removal fixes a real inconsistency — every other generateId call already used path:name, so parsing-processor.ts was the odd one out causing orphaned CALLS edges
• PHP built-in filtering is now consistent across main thread and workers
• Tests using real tree-sitter parsers instead of mocks

Follow-up items (not blocking):

• DEFINITION_CAPTURE_KEYS + getDefinitionNodeFromCaptures are still duplicated between parsing-processor.ts and parse-worker.ts — pure data/functions, no thread boundary issue. Same for getLabelFromCaptures. Could consolidate later.
• gitnexus-web/src/core/ingestion/call-processor.ts still has the old code without these fixes — worth an issue to port over.

See inline comments for specifics.

[magyargergo]

C/C++ Coverage Analysis

Did a deep dive into the current C/C++ support after your PR. Here's where things stand:

What's Working Well

Area	C	C++
Language detection	.c, .h	.cpp, .cc, .cxx, .hpp, .hxx, .hh
Tree-sitter parsing	tree-sitter-c loads ✅	tree-sitter-cpp loads ✅
Symbol extraction	Functions, structs, unions, enums, typedefs, macros	Classes, structs, namespaces, enums, functions, methods, templates
Call extraction	identifier, field_expression	+ qualified_identifier, template_function
#include → import graph	preproc_include captured	Same
Heritage	N/A	base_class_clause → EXTENDS edges
Built-in filtering	~50 entries (stdlib + kernel macros)	Same set (shared)
Framework detection	main.c, app.c	main.cpp, main.cc, app.cpp
Export detection	Always false	Always false (correct — no language-level export)
Function name extraction	function_declarator → identifier	+ qualified_identifier (Class::Method) — your PR fixed this

Test Coverage

Your PR added good coverage in ingestion-utils.test.ts:

• extractFunctionName — 5 tests covering C plain func, C with params, C++ qualified method, C++ namespace method, C++ standalone
• isBuiltInOrNoise — C/C++ stdlib + kernel macros
• getLanguageFromFilename — all C/C++ extensions

Plus existing tests in tree-sitter-queries.test.ts, tree-sitter-languages.test.ts (integration with real parsers), parsing.test.ts (isNodeExported), framework-detection.test.ts, and parser-loader.test.ts.

Gaps to be Aware Of

Gap	Severity	Notes
gitnexus-web not updated	Medium	gitnexus-web/src/core/ingestion/call-processor.ts still has the old inline isBuiltInOrNoise (line 714) and lacks extractFunctionName. The C++ qualified_identifier fix isn't in the web package.
No C++ forward declarations	Low	(declaration declarator: ...) is in C_QUERIES but not CPP_QUERIES — header forward decls won't be captured
No #include path resolution	Low	System <...> includes always fail to resolve (expected). Local "..." includes work if file exists in repo. No header search path logic.
Simple test fixtures	Low	simple.c has 3 functions, simple.cpp has 1 class + 1 free function. No templates, namespaces, macros, or multi-file include chains.
No operator overloading	Low	operator+, operator<< etc. won't be captured
No function pointer typedefs	Low	typedef void (*callback_t)(int) — typedef captured but signature lost

Verdict

C/C++ coverage is solid for the core use case (indexing typical application/library code). Your qualified_identifier fix was the biggest gap and it's now handled correctly. The main follow-up would be porting these improvements to gitnexus-web so the web and CLI packages stay in sync.

[magyargergo]

@Alice523 would you be interested in covering those gaps? :)

[magyargergo]

Since I understand why you extracted the duplicated const fields in the utils.ts file, I'd like to ask you to move it back to reduce the impact of the change and less people have to rebase their changes. 🙏

[magyargergo]

Superseded by #237 which consolidates this PR along with #170, #178, #216, #227, #234 into a single coherent branch with all C/C++/C#/Rust fixes, shared modules, and comprehensive tests.

[magyargergo]

Closing in favor of #237. Thank you for your contribution — your work on the ingestion pipeline refactor was essential and is fully incorporated in the consolidated PR!