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

PR Review: OCaml Interface Grammar Support

Todo List:

• Phase 1: Root cause analysis - understand the problem being solved
• Phase 2: Configuration change detection
• Phase 3: Test integrity review
• Phase 4: Code quality assessment
• Phase 5: Backlog compliance evaluation
• Provide final structured assessment

Analysis Summary:

Problem Being Solved: .mli (OCaml interface) files were being parsed with the generic LANGUAGE_OCAML grammar, which is designed for implementation files (.ml). This was producing error-polluted ASTs and missing interface-specific declarations like val specifications, external declarations, module type definitions, and exception definitions.

Phase 1: Root Cause Analysis ✅

• Real Problem: This addresses a genuine parsing accuracy issue. OCaml interface files have different syntax constructs than implementation files.
• Justified Approach: Using a dedicated LANGUAGE_OCAML_INTERFACE grammar is the correct solution.
• Not a Workaround: This is a proper fix, not hiding symptoms.

Phase 2: Configuration Changes ✅

• No linting rules disabled or relaxed
• No test timeouts increased or coverage thresholds lowered
• No build configuration weakening detected
• Clean addition of new language support

Phase 3: Test Integrity ✅

• All 6 existing .ml parser tests preserved unchanged
• 7 new .mli interface tests added (not weakened)
• Tests are comprehensive and properly validate the new functionality
• No test assertions weakened or removed

Phase 4: Code Quality ✅

• Both engines updated consistently (Rust + WASM)
• Smart val classification based on type signatures
• Proper interface-specific node handlers
• Good code reuse between .ml and .mli parsing

Root Cause Analysis

Problem Being Solved: OCaml interface files (.mli) contain different syntax constructs than implementation files (.ml). Using the wrong grammar produced error-polluted ASTs and missed interface-specific declarations.

Why This Approach: Using dedicated LANGUAGE_OCAML_INTERFACE grammar is the architecturally correct solution - aligns with tree-sitter's design for language-specific grammars.

Risk Assessment: Low risk. This is an additive change that doesn't affect existing .ml parsing and uses established patterns.

Backlog Compliance

• Zero-dep: ✅ No new runtime dependencies - builds on existing tree-sitter infrastructure
• Foundation-aligned: ✅ Fully complies with all principles - improves accuracy without changing core design
• Problem-fit: 4/5 - Directly improves AI agent accuracy by providing clean ASTs and catching interface-specific symbols that were previously missed
• Breaking: No - purely additive, existing .ml files unaffected
• Tier: 1 (zero-dep ✅ + foundation-aligned ✅)

Critical Concerns

None. This is a well-implemented language accuracy improvement.

Minor Notes

• hasDescendantType recursive traversal could be expensive for deeply nested types (acceptable for typical OCaml)
• Would benefit from CI confirmation of Rust compilation (Windows linker issue prevents local verification)

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE WITH CONDITIONS
• Reasoning: Solid implementation that directly improves parsing accuracy for OCaml interface files. Well-tested, follows established patterns, no architectural concerns. Minor deduction only because CI verification of Rust compilation is pending.

Conditions for approval: Verify CI build passes, confirming Rust compilation succeeds and WASM grammar builds correctly.

Greptile Summary

This PR correctly fixes OCaml .mli interface file parsing by routing them to the dedicated LANGUAGE_OCAML_INTERFACE tree-sitter grammar instead of re-using LANGUAGE_OCAML, which was producing error-polluted ASTs. The fix is applied consistently across both the Rust native engine and the WASM engine. Previous review comments about comment accuracy and exception aliases are both addressed in this revision.

• .mli extension split off into its own ocaml-interface registry entry in both parser.ts and parser_registry.rs, each pointing to the correct grammar
• tree-sitter-ocaml_interface.wasm build entry added to build-wasm.ts using the grammars/interface sub-path from tree-sitter-ocaml
• val name : type specs are classified as function/variable based on whether the type contains a function_type descendant — works correctly for top-level function types but can over-match for values whose types merely contain an arrow (e.g. val callbacks : (event -> unit) list)
• 11 new tests cover all new node handlers across both .ml and .mli grammars, including the exception alias fallback path

Confidence Score: 5/5

Safe to merge — all changes are additive, well-tested, and the only remaining finding is a P2 edge-case misclassification in the function-type heuristic.

All P0/P1 concerns from prior rounds are resolved. The single remaining note is a P2 edge case where hasDescendantType may over-classify a value like val callbacks : (event -> unit) list as a function. This does not break parsing, cause data loss, or affect the primary use-case. Per the confidence guidelines, a P2-only finding does not reduce the score below 5.

Both src/extractors/ocaml.ts and crates/codegraph-core/src/extractors/ocaml.rs share the hasDescendantType/has_descendant_kind heuristic worth revisiting for higher classification precision.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[File input] --> B{Extension?}
    B -- ".ml" --> C[LanguageKind::Ocaml\nocaml registry entry\ntree-sitter-ocaml.wasm]
    B -- ".mli" --> D[LanguageKind::OcamlInterface\nocaml-interface registry entry\ntree-sitter-ocaml_interface.wasm]
    C --> E[OcamlExtractor / extractOCamlSymbols]
    D --> E
    E --> F{Node type?}
    F -- value_definition/let_binding --> G[function / variable]
    F -- type_definition --> H[type]
    F -- module_definition --> I[module]
    F -- open_module --> K[import]
    F -- value_specification --> L{Outer type is\nfunction_type?}
    L -- yes --> M[function]
    L -- no --> N[variable]
    F -- external --> O[function]
    F -- module_type_definition --> P[interface]
    F -- exception_definition --> Q[type]

_{Reviews (2): Last reviewed commit: "fix: add ocaml-interface to LanguageId t..." | Re-trigger Greptile}

Addressed all Greptile review feedback in fd2a1fe:

1. Misleading comment (P2): Updated "Interface-specific (.mli) node types" to "Shared node types present in both .ml and .mli files" in both TS and Rust extractors.

2. Exception aliases silently skipped (P2): Added fallback to constructor_name on the exception_definition node for exception Foo = Bar alias syntax, in both TS and Rust extractors.

3. Missing .ml test coverage (outside-diff P2): Added 4 new test cases to the OCaml parser describe block covering external, exception, exception alias, and module type definitions in .ml files.

4. TS2322 type error (CI blocker): Added 'ocaml-interface' to the LanguageId union type in src/types.ts.

@greptileai