GitNexus/compound-engineering.local.md
Gergő Magyar 604b575e4b
feat: Phase 7 type resolution — return-aware loop inference & PHP class-property iterables (#341)
* feat(type-resolution): Phase 7.1+7.2 foundation — ReturnTypeLookup, context object, pendingCallResults

- Move extractReturnTypeName + helpers from call-processor.ts to type-extractors/shared.ts
  (breaks circular import risk: call-processor → type-env → type-extractors → call-processor)
- Add SymbolTable.lookupFuzzyCallable(name) — lazy callable-only index, O(1) per call,
  invalidated on add(); avoids per-call .filter() on lookupFuzzy results
- Add ReturnTypeLookup interface (conservative: undefined when 0 or 2+ callables match)
- Add ForLoopExtractorContext interface — replaces 4 positional params with context object;
  update all 10 language extractor implementations (go, ts, py, jvm×2, cs, rs, rb, php, c-cpp)
- Add PendingAssignment discriminated union (kind: 'copy' | 'callResult');
  update PendingAssignmentExtractor in all 9 language extractors that implement it
- Wire buildTypeEnv: build ReturnTypeLookup from optional symbolTable; split pendingAssignments
  into pendingCopies + pendingCallResults; add Tier 2b call-result propagation loop
- Update call-processor.test.ts to import extractReturnTypeName from shared.ts

* feat(type-resolution): Phase 7.3 — call_expression iterables in for-loop extractors (7 languages)

Extends for-loop type extraction in all 7 typed-iteration languages to
resolve element types when the iterable is a direct function call.

**New capability**: `for (var u : getUsers())` in Java, `for u in get_users()`
in Python, `for user in getUsers()` in TypeScript, etc. now resolve
`u`/`user` to the callee's return element type via lookupRawReturnType +
extractElementTypeFromString.

Changes per language:
- types.ts: extend ReturnTypeLookup with lookupRawReturnType (raw return
  string for container-type extraction); update ForLoopExtractorContext
  with returnTypeLookup field
- type-env.ts: implement lookupRawReturnType on the concrete ReturnTypeLookup
  built in buildTypeEnv (same guards as lookupReturnType, no extractReturnTypeName)
- go.ts: call_expression branch in range_clause — identifier func or
  selector_expression method; existing isChannelType guards updated
- typescript.ts: identifier fn branch inside call_expression handler
- python.ts: identifier fn branch inside call handler
- jvm.ts (Java): method_invocation without object field in enhanced_for_statement
- jvm.ts (Kotlin): simple_identifier callee branch in call_expression node
- csharp.ts: identifier fn branch in invocation_expression handler
- rust.ts: identifier func branch in call_expression handler (alongside
  existing field_expression/method-call path)

All branches follow the same conservative pattern:
  lookupRawReturnType(callee) → extractElementTypeFromString → bind loop var

* feat(type-resolution): Phase 7.4 — PHP \$this->property iterable via @var class property scan

Adds Strategy C to PHP's extractForLoopBinding for the pattern:

  foreach (\$this->property as \$item)

when Strategy A (resolveIterableElementType) and Strategy B (scopeEnv lookup)
both fail to find the element type.

Strategy C: when the iterable is a member_access_expression with object '$this',
walk up the AST to the enclosing class_declaration, scan its declaration_list
for a property_declaration whose variable_name matches the property, and extract
the element type from:
  1. PHPDoc @var annotation on a preceding comment sibling (/** @var User[] */)
  2. PHP 7.4+ native type field (e.g. UserRepo \$repo — skips generic 'array')

This eliminates the @param workaround that was previously required in the
php-foreach-member-access fixture (which used @param User[] \$users on the method
to populate the method's scopeEnv with a \$users binding).

New helpers in php.ts:
- PHPDOC_VAR_RE: regex for @var extraction
- extractClassPropertyElementType: reads @var or native type from a property_declaration
- findClassPropertyElementType: scans class body for a named property

Tests added (type-env.test.ts):
- PHP: resolves from @var User[] without @param workaround
- PHP: conservative — no binding for unknown property
- PHP: multi-class file — both classes resolve independently

Fixture updated (php-foreach-member-access/App.php):
- Removed the @param User[] \$users workaround from processMembers()
- Test now validates the natural class-property-based resolution path

* docs: mark Phase 7 complete in type-resolution-roadmap.md

Records that 7A (call_expression iterables, 7 languages), 7B (PHP
$this->property via @var scan), and 7C (ReturnTypeLookup + context object)
are all shipped. Adds implementation notes and strikethroughs on resolved
language-specific gaps.

* fix(docs): update project references to feat-phase7-type-resolution in AGENTS.md and CLAUDE.md

* feat(type-resolution): Phase 7.5 — PHP call_expression foreach + integration tests for 7 languages

Add integration test coverage for Phase 7.3's call_expression iterable
resolution across all 7 languages (Go, TypeScript, Python, Java, Kotlin,
PHP, Rust). Each test creates a fixture with competing User/Repo classes
that both define save(), then verifies for-loop iteration over a function
call's return value resolves to the correct class.

PHP was missing function_call_expression support in its for-loop extractor.
Three changes fix this:
- php.ts extractForLoopBinding: handle function_call_expression and
  member_call_expression iterables via returnTypeLookup
- php.ts normalizePhpReturnType: preserve array notation (User[]) in
  SymbolTable so lookupRawReturnType returns useful container types
- parse-worker.ts + parsing-processor.ts: upgrade uninformative AST
  return types (array, iterable) with PHPDoc @return annotations

35 new integration tests (5 per language), 2525 total tests passing.

* fix(type-resolution): address PR #341 review findings — PHP asymmetry + dormant infrastructure docs

- Replace normalizePhpType with extractElementTypeFromString in PHP call-expression
  foreach paths, aligning with all 6 other language extractors and preventing
  incorrect binding of bare non-container types like User
- Add NOTE comments clarifying pendingCallResults Tier 2b is infrastructure-ready
  but no extractor populates it yet
- Expand Go channel-type comments explaining why non-channel assumption is safe

* fix(type-resolution): address verification review — docs accuracy + PHP fallback guard

- Roadmap lines 86/100: correct pendingCallResults from "active" to "dormant infrastructure (Phase 9)"
- type-resolution-system.md line 363: update to reflect Phase 7.3 loop inference is delivered
- type-resolution-system.md line 409: clarify for-loop call-expression resolution (done) vs general assignment propagation (pending)
- php.ts:127: add declaration_list type guard on fallback to prevent silent wrong results
2026-03-18 08:39:38 +00:00

4.5 KiB
Raw Permalink Blame History

review_agents plan_review_agents voltagent_agents
kieran-typescript-reviewer
pattern-recognition-specialist
architecture-strategist
data-integrity-guardian
security-sentinel
performance-oracle
code-simplicity-reviewer
kieran-typescript-reviewer
architecture-strategist
code-simplicity-reviewer
voltagent-lang:typescript-pro
voltagent-qa-sec:security-auditor
voltagent-data-ai:database-optimizer

Review Context

Project Overview

GitNexus is a code intelligence tool that builds a knowledge graph from source code using tree-sitter AST parsing across 12 languages and KuzuDB for graph storage. Two packages: gitnexus/ (CLI/MCP, TypeScript) and gitnexus-web/ (browser).

Cross-Language Pattern Consistency (pattern-recognition-specialist)

  • 12 language-specific type extractors in gitnexus/src/core/ingestion/type-extractors/ must follow identical patterns for: async unwrapping, constructor binding, namespace handling, nullable type stripping, for-loop element typing.
  • Past bugs: C#/Rust missing await_expression unwrapping that TypeScript handled correctly; PHP backslash namespace splitting inconsistent with other languages' :: / . splitting.
  • When reviewing type extractor changes, verify the same pattern exists in ALL applicable language files — asymmetry is the #1 source of bugs.

Data Integrity (data-integrity-guardian)

  • KuzuDB graph operations: schema in gitnexus/src/core/kuzu/schema.ts, adapter in kuzu-adapter.ts.
  • The ingestion pipeline writes symbols and relationships to the graph — changes to node/relation schemas or the ingestion pipeline can corrupt the index.
  • Known issue: KuzuDB close() hangs on Linux due to C++ destructor — use detachKuzu() pattern.
  • lbug-adapter.ts fallback path needs quote/newline escaping for Cypher injection prevention.

Security (security-sentinel)

  • Cypher query construction in lbug-adapter.ts and kuzu-adapter.ts — watch for injection via unescaped user-provided symbol names.
  • CLI accepts --repo parameter and file paths — validate against path traversal.
  • MCP server exposes tools to external AI agents — all tool inputs are untrusted.

Performance (performance-oracle)

  • Tree-sitter buffer size is adaptive (512KB32MB) via getTreeSitterBufferSize() in constants.ts.
  • The ingestion pipeline processes entire repositories — O(n) per file with potential O(n²) in cross-file resolution.
  • KuzuDB batch inserts vs individual inserts matter for large repos.

Architecture (architecture-strategist)

  • Ingestion pipeline phases: structure → parsing → imports → calls → heritage → processes → type resolution.
  • Shared modules: export-detection.ts, constants.ts, utils.ts — changes here have wide blast radius.
  • gitnexus-web package drifts behind CLI — flag if a change should be mirrored.

Voltagent Supplementary Agents

Invoke these via the Agent tool alongside /ce:review for deeper specialist analysis. These cover gaps that compound-engineering agents don't:

voltagent-lang:typescript-pro

When: Changes touch type-resolution logic, generics, conditional types, or complex type-level programming in type-env.ts, type-extractors/*.ts, or types.ts. Why: The type resolution system uses advanced TypeScript patterns (discriminated unions, mapped types, recursive generics) that benefit from deep TS type-system review beyond what kieran-typescript-reviewer covers.

voltagent-qa-sec:security-auditor

When: Changes touch MCP tool handlers, Cypher query construction, CLI argument parsing, or any code that processes external input. Why: GitNexus is an MCP server — all tool inputs come from untrusted AI agents. Systematic OWASP-level audit catches injection vectors that spot-checking misses. Past finding: lbug-adapter.ts fallback path had unescaped newlines in Cypher queries.

voltagent-data-ai:database-optimizer

When: Changes touch kuzu-adapter.ts, schema.ts, lbug-adapter.ts, or any Cypher query construction/execution. Why: No CE agent specializes in graph database optimization. KuzuDB batch insert patterns, index usage, and query planning directly affect analysis speed on large repos.

Review Tooling

  • Use gitnexus_impact() before approving changes to any symbol — check d=1 (WILL BREAK) callers.
  • Use gitnexus_detect_changes({scope: "compare", base_ref: "main"}) to map PR diffs to affected execution flows.
  • Use claude-mem to surface past architectural decisions relevant to the code under review.