Skip to content

fix(groovy): emit ClassRelation for interface inheritance in both engines#1158

Merged
carlos-alm merged 6 commits into
mainfrom
fix/1115-groovy-interface-inheritance
May 19, 2026
Merged

fix(groovy): emit ClassRelation for interface inheritance in both engines#1158
carlos-alm merged 6 commits into
mainfrom
fix/1115-groovy-interface-inheritance

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • interface X extends Y, Z produced no class-graph edges because both Groovy extractors pushed a single Definition for the interface but never inspected the unnamed extends_interfaces child.
  • The Rust handler now reuses the existing collect_interfaces helper (which already recurses into type_list); the TS handler gains an equivalent collectGroovyParentInterfaces helper that mirrors the same traversal.
  • Both engines emit ClassRelation { name: X, implements: Y, line } for each parent interface — consistent with the existing collect_interfaces helper, with class-implements behavior, and with the downstream IMPLEMENTS_TARGET_KINDS filter that already accepts interface targets.

Notes

  • tree-sitter-groovy 0.1.x exposes parent interfaces as an unnamed extends_interfaces child of interface_declaration (not a field), distinct from class declarations which use the interfaces field that wraps super_interfaces → type_list.
  • This PR fixes the extraction layer only. The downstream HIERARCHY_SOURCE_KINDS set in src/domain/graph/builder/stages/build-edges.ts does not currently include interface, so emitted relations still won't materialize as graph edges until that filter is extended — that's a broader cross-language change worth tracking separately.

Closes #1115

Test plan

  • cargo test -p codegraph-core --lib extractors::groovy — new extracts_interface_inheritance test passes alongside existing 8 tests
  • npx vitest run tests/parsers/groovy.test.ts — new extracts interface inheritance (extends_interfaces) test passes alongside existing 6 tests
  • npx biome check src/extractors/groovy.ts tests/parsers/groovy.test.ts — clean

…ines

`interface X extends Y, Z` produced no class-graph edges because both
extractors pushed only a Definition for the interface and never inspected
the unnamed `extends_interfaces` child. The Rust handler now reuses
`collect_interfaces` (which already recurses into `type_list`); the TS
handler gains an equivalent helper that mirrors the same traversal.

Closes #1115
@claude
Copy link
Copy Markdown

claude Bot commented May 18, 2026

Claude finished @carlos-alm's task —— View job


Reviewing PR: fix(groovy): emit ClassRelation for interface inheritance in both engines

Todo List

  • Phase 1: Root cause analysis and approach justification
  • Phase 2: Check for configuration changes
  • Phase 3: Verify test integrity
  • Phase 4: Code quality review
  • Phase 5: Backlog compliance assessment
  • Final: Provide structured review feedback

Root Cause Analysis

Problem Being Solved: interface X extends Y, Z was producing no class-graph edges because both Groovy extractors only emitted a Definition for the interface but never inspected the unnamed extends_interfaces child that tree-sitter-groovy uses to represent parent interfaces.

Why This Approach:

  • Correct fix at source: Addresses the extraction layer where the bug actually occurs
  • Consistent pattern: Rust handler reuses existing collect_interfaces helper; TypeScript gets equivalent collectGroovyParentInterfaces helper
  • Grammar-aware: Properly handles tree-sitter-groovy's distinction between interface inheritance (extends_interfaces unnamed child) vs class implements (interfaces field)
  • Engine parity: Both extractors now emit identical ClassRelation structures

Risk Assessment: LOW - Purely additive change to extraction logic, no behavioral modifications to existing functionality.

Backlog Compliance

  • Zero-dep: ✓ — no new runtime dependencies, builds on existing tree-sitter infrastructure
  • Foundation-aligned: ✓ — improves accuracy of dependency graph extraction, core to the tool's mission
  • Problem-fit: 4/5 — directly prevents AI hallucination of interface relationships by ensuring accurate extraction
  • Breaking: No — purely additive extraction, no API/output format changes
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Code Quality Assessment

Implementation Strengths

  1. Engine Parity: Both Rust and TypeScript implementations follow identical logic patterns
  2. Helper Reuse: Rust cleverly reuses existing collect_interfaces that already handles type_list recursion
  3. Grammar Correctness: Proper distinction between extends_interfaces (interface inheritance) vs interfaces field (class implements)
  4. Test Coverage: Comprehensive tests in both engines validate the fix end-to-end

Technical Analysis

Rust Implementation (groovy.rs:173-184):

for i in 0..node.child_count() {
    let Some(child) = node.child(i) else { continue };
    if child.kind() == "extends_interfaces" {
        collect_interfaces(&child, &iface_name, source, symbols);
        break;
    }
}

✅ Efficient early termination with break
✅ Reuses battle-tested collect_interfaces helper
✅ Proper error handling with let Some

TypeScript Implementation (groovy.ts:157-163):

for (let i = 0; i < node.childCount; i++) {
    const child = node.child(i);
    if (child && child.type === 'extends_interfaces') {
        collectGroovyParentInterfaces(child, ifaceName, node.startPosition.row + 1, ctx);
        break;
    }
}

✅ Mirrors Rust pattern exactly
✅ New collectGroovyParentInterfaces helper follows same recursion logic
✅ Proper null checks and early termination

Minor Observations

  1. Architecture Note: The PR correctly notes that HIERARCHY_SOURCE_KINDS doesn't include 'interface', so emitted relations won't materialize as graph edges until that filter is updated. This is appropriately framed as a separate broader change.

  2. Test Quality: Both test suites use identical test cases (interface Serializable extends Comparable, Cloneable) ensuring engine behavior consistency.

  3. Documentation: Excellent inline comments explaining tree-sitter grammar specifics and why the approach works.

Critical Concerns

None found. This is a well-executed bug fix with proper testing and engine parity.

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: Clean, well-tested fix that properly addresses interface inheritance extraction. The implementation correctly handles tree-sitter grammar specifics, maintains engine parity, and includes comprehensive test coverage. The only minor issue is the broader architectural question about HIERARCHY_SOURCE_KINDS, which is correctly identified as a separate concern.

Excellent work - this demonstrates deep understanding of both the tree-sitter grammar and the codegraph extraction pipeline.


@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR fixes a missing-edge bug where interface X extends Y, Z declarations produced no ClassRelation entries in either the Rust or TypeScript Groovy extractors. The fix identifies the unnamed extends_interfaces child of interface_declaration and routes it through the existing collect_interfaces helper (Rust) or a new, equivalent collectGroovyParentInterfaces helper (TS).

  • Rust (groovy.rs): handle_interface_decl now walks children to find extends_interfaces and delegates to the existing collect_interfaces, which already recurses through type_list correctly.
  • TypeScript (groovy.ts): New collectGroovyParentInterfaces mirrors collect_interfaces exactly, including re-evaluating startPosition.row + 1 at each recursion level to stay line-accurate.
  • Tests: Both suites gain a basic inheritance test and a multi-line engine-parity guard ensuring line tracks the extends clause position consistently across Rust and WASM.

Confidence Score: 5/5

Safe to merge — a focused, additive extraction fix with no changes to downstream consumers or the graph builder.

The change is purely additive: it adds child scanning in two handlers and a new TS helper, both narrowly scoped to extends_interfaces nodes in Groovy interface declarations. The Rust path reuses the battle-tested collect_interfaces helper; the TS path is a faithful mirror. Both engines are covered by new unit tests including a multi-line parity guard. No existing code paths are altered, and the downstream HIERARCHY_SOURCE_KINDS filter deliberately excludes interface for now, so the new ClassRelation entries are safely inert until that filter is extended.

No files require special attention.

Important Files Changed

Filename Overview
crates/codegraph-core/src/extractors/groovy.rs Adds extends_interfaces child scan in handle_interface_decl and reuses the existing collect_interfaces helper; two new tests cover basic inheritance and multi-line engine-parity.
src/extractors/groovy.ts Adds collectGroovyParentInterfaces that faithfully mirrors the Rust collect_interfaces helper, and wires it into handleGroovyInterfaceDecl via an extends_interfaces child scan.
tests/parsers/groovy.test.ts Adds two TS tests: one for single-line interface inheritance and one multi-line engine-parity guard asserting line === 2.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["interface_declaration node"] --> B{find child\nkind == 'extends_interfaces'}
    B -- found --> C["extends_interfaces node"]
    B -- not found --> Z["emit Definition only"]
    C --> D["collect_interfaces / collectGroovyParentInterfaces"]
    D --> E{child.type}
    E -- type_identifier\nidentifier\nscoped_type_identifier --> F["emit ClassRelation\n{ name: X, implements: Y, line }"]
    E -- generic_type --> G["read child(0).text\nemit ClassRelation"]
    E -- type_list --> H["recurse with child\n(re-evaluates start line)"]
    H --> E
    E -- other --> I["skip"]
Loading

Reviews (7): Last reviewed commit: "Merge branch 'main' into fix/1115-groovy..." | Re-trigger Greptile

Comment thread src/extractors/groovy.ts
Comment on lines +166 to +191
function collectGroovyParentInterfaces(
parent: TreeSitterNode,
name: string,
line: number,
ctx: ExtractorOutput,
): void {
for (let i = 0; i < parent.childCount; i++) {
const child = parent.child(i);
if (!child) continue;
switch (child.type) {
case 'type_identifier':
case 'identifier':
case 'scoped_type_identifier': {
ctx.classes.push({ name, implements: child.text, line });
break;
}
case 'generic_type': {
const inner = child.child(0)?.text;
if (inner) ctx.classes.push({ name, implements: inner, line });
break;
}
case 'type_list':
collectGroovyParentInterfaces(child, name, line, ctx);
break;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Line number diverges from Rust engine on multi-line declarations

collectGroovyParentInterfaces receives line once (the interface declaration's startPosition.row + 1) and forwards the same value through every recursive call into type_list children. The Rust collect_interfaces instead evaluates start_line(interfaces) at each recursive invocation, so it tracks the actual start line of whatever node (extends_interfacestype_list) is being processed at that moment.

For a multi-line interface — e.g. interface X\n extends Y, Z {} — the Rust engine would emit line 2 for the relation, while the TS engine always emits line 1. This is currently benign (the downstream HIERARCHY_SOURCE_KINDS filter doesn't include interface), but it means the two engines will produce divergent ClassRelation.line values once that filter is extended.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 732c82dcollectGroovyParentInterfaces no longer accepts a fixed line parameter; it now derives line = parent.startPosition.row + 1 at each recursion level, matching Rust's collect_interfaces which re-evaluates start_line(interfaces) per call. Added an engine-parity guard test (interface X\n extends Y, Z {} → line 2) in both Rust and WASM test suites so this divergence cannot regress.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

Codegraph Impact Analysis

5 functions changed4 callers affected across 2 files

  • handle_interface_decl in crates/codegraph-core/src/extractors/groovy.rs:159 (1 transitive callers)
  • extracts_interface_inheritance in crates/codegraph-core/src/extractors/groovy.rs:541 (0 transitive callers)
  • interface_inheritance_line_tracks_extends_clause in crates/codegraph-core/src/extractors/groovy.rs:560 (0 transitive callers)
  • handleGroovyInterfaceDecl in src/extractors/groovy.ts:141 (2 transitive callers)
  • collectGroovyParentInterfaces in src/extractors/groovy.ts:166 (3 transitive callers)

carlos-alm and others added 2 commits May 18, 2026 18:51
…1158)

Align the TypeScript collectGroovyParentInterfaces helper with the Rust
collect_interfaces helper, which re-evaluates start_line(interfaces) at
each recursive invocation. Previously the WASM extractor forwarded the
interface declaration's start line through every recursion level, so a
multi-line interface (e.g. interface X\n  extends Y, Z {}) produced
line=1 in WASM but line=2 in native — the two engines must produce
identical results.

Add a multi-line line-parity test in both engines to guard against
future regressions.
@carlos-alm
Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit 81150e4 into main May 19, 2026
27 checks passed
@carlos-alm carlos-alm deleted the fix/1115-groovy-interface-inheritance branch May 19, 2026 03:54
@github-actions github-actions Bot locked and limited conversation to collaborators May 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

follow-up: emit ClassRelation for Groovy interface inheritance (both engines)

1 participant