Skip to content

fix(parity): gate astTypeMap lookup with Object.hasOwn#1041

Merged
carlos-alm merged 1 commit intomainfrom
fix/haskell-resolver-1039
May 1, 2026
Merged

fix(parity): gate astTypeMap lookup with Object.hasOwn#1041
carlos-alm merged 1 commit intomainfrom
fix/haskell-resolver-1039

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • AST-store visitor looked up astTypeMap[node.type] with plain bracket notation; tree-sitter node type constructor (Haskell sum-types: Left, Right, Just, …) walked the prototype chain to Object.prototype.constructor, dropping the Object() function into the astNodes row's kind field.
  • The non-cloneable function then crashed the worker boundary with function Object() { [native code] } could not be cloned, skipping every Haskell file containing constructors and dragging the resolver from 100%/33% precision/recall in v3.9.4 to 0%/0% in v3.9.6.
  • Gate the lookup with Object.hasOwn so prototype-derived members never leak into the result. Restores the four expected truePositives in the Haskell fixture; metrics return to the v3.9.4 baseline (precision=1.0, recall=0.333).

The KNOWN_REGRESSIONS entries stay until v3.9.7 benchmark data is captured (matching the established pattern for the other 3.9.6 entries).

Closes #1039

Test plan

  • npx vitest run tests/benchmarks/resolution/resolution-benchmark.test.ts — all 170 pass; Haskell now parses 4/4 files (was 2/4).
  • Direct precision/recall check on the Haskell fixture: precision=1.000 (4/4), recall=0.333 (4/12) — matches v3.9.4 baseline.
  • npx vitest run tests/parsers/ tests/integration/ast.test.ts tests/engines/ast-parity.test.ts — 493 pass, no regressions.
  • npx vitest run tests/benchmarks/regression-guard.test.ts — 17 pass.

The AST-store visitor looked up `astTypeMap[node.type]` with plain bracket
notation. When tree-sitter emits a node type that collides with an
Object.prototype member — Haskell sum-type constructors are reported as
node type `constructor` — the lookup walks the prototype chain and
returns `Object.prototype.constructor` (the Object() function). The
function then lands in the astNodes row's `kind` field and the row
fails to structured-clone across the worker boundary, surfacing as
"function Object() { [native code] } could not be cloned" and skipping
the file entirely.

Every Haskell file containing `Left`, `Right`, `Just`, etc. was being
silently dropped, which is what dragged the resolver from
precision=1.0/recall=0.333 in v3.9.4 to 0/0 in v3.9.6. With the gate in
place the four expected truePositives in the Haskell fixture come back
and the metrics restore to the v3.9.4 baseline.

Closes #1039
@claude
Copy link
Copy Markdown

claude Bot commented Apr 30, 2026

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


Reviewing PR: fix(parity): gate astTypeMap lookup with Object.hasOwn

Todo List:

  • Read and analyze changed files
  • Phase 1: Root Cause Analysis
  • Phase 2: Configuration Change Detection
  • Phase 3: Test Integrity Analysis
  • Phase 4: Code Quality Review
  • Phase 5: Backlog Compliance Assessment
  • Provide final rating and recommendation

Phase 1: Root Cause Analysis ✅

Problem Being Solved: The AST-store visitor in src/ast-analysis/visitors/ast-store-visitor.ts:278 was using bracket notation astTypeMap[node.type] to look up AST node types. When tree-sitter parses Haskell code with sum-type constructors like Left, Right, Just, etc., it emits nodes with type constructor. This caused the lookup to walk the prototype chain and return Object.prototype.constructor (the Object() function) instead of undefined.

Why This Approach: The fix uses Object.hasOwn(astTypeMap, node.type) to guard the lookup, preventing prototype chain traversal. This is the correct approach because:

  1. Root cause: Plain object property access obj[key] can return inherited properties
  2. Specific trigger: Haskell node type constructor collides with Object.prototype.constructor
  3. Failure mode: Function objects cannot be structured-cloned across worker boundaries
  4. Impact: Files containing these constructors were silently skipped, dropping resolution precision/recall from 1.0/0.333 to 0/0

Risk Assessment: ✅ LOW RISK - This is fixing a genuine bug, not masking symptoms. The approach is minimal, targeted, and preserves existing behavior for valid lookups while preventing prototype pollution issues.

Phase 2: Configuration Change Detection ✅

No configuration changes detected. This PR only modifies:

  1. Core logic in ast-store-visitor.ts (1 guard condition added)
  2. Test documentation in regression-guard.test.ts (expanded comment explaining the root cause)

No biome/linting rules, test timeouts, coverage thresholds, build config, or TypeScript settings modified.

Phase 3: Test Integrity Analysis ✅

Test changes are appropriate and strengthen validation:

  1. Regression documentation updated (lines 93-101): The change expands the existing comment for 3.9.6:resolution haskell precision/recall in KNOWN_REGRESSIONS to provide detailed root-cause analysis. This is good practice — documenting the exact failure mode helps future maintainers.

  2. No test assertions weakened: The actual test logic and thresholds remain unchanged. The KNOWN_REGRESSIONS set still excludes these entries from regression detection until v3.9.7+ data is captured (which matches the established pattern).

  3. Test coverage maintained: Per the PR description, all 170 resolution benchmark tests pass, and Haskell parsing is restored (4/4 files vs 2/4 previously).

Phase 4: Code Quality Review ✅

Implementation Quality:
Excellent fix - The Object.hasOwn() guard at line 278 is:

  1. Precise: Targets the exact problem (prototype chain pollution)
  2. Minimal: Single line change, no side effects
  3. Standard practice: Object.hasOwn() is the modern, recommended way to check own properties
  4. Well-documented: Clear comment explaining the rationale (lines 272-277)

Potential Issues:
None identified - The fix:

  • Preserves existing behavior for valid node types
  • Has no performance impact (hasOwn is O(1))
  • Uses ES2022 syntax (Object.hasOwn) which is appropriate for a modern codebase
  • Follows the same pattern as other guard conditions in the visitor

Code Style:
Follows established patterns: The guard condition matches the existing style (early returns, clear condition logic) and the comment format matches other explanatory comments in the file.

Phase 5: Backlog Compliance Assessment ✅

Root Cause Analysis

Problem Being Solved: Critical bug where Haskell files with constructor nodes (Left, Right, Just) were silently skipped due to prototype chain pollution, causing resolution metrics to drop from 1.0/0.333 to 0/0.
Why This Approach: Uses Object.hasOwn() to prevent prototype pollution — the correct, minimal fix for the root cause.
Risk Assessment: LOW RISK — fixes genuine bug without side effects.

Backlog Compliance

  • Zero-dep: ✓ — no new dependencies, uses native ES2022 Object.hasOwn
  • Foundation-aligned: ✓ — improves parsing accuracy, supports the core dependency analysis mission
  • Problem-fit: 5 — directly prevents AI hallucination by ensuring accurate Haskell symbol extraction
  • Breaking: No — purely additive fix, no API/output format changes
  • Tier: 1

Critical Concerns

None identified. This is a textbook example of a surgical bug fix.

Final Recommendation

  • Rating: ⭐⭐⭐⭐⭐ (5/5)
  • Action: APPROVE
  • Reasoning: Exemplary fix — identifies root cause precisely, implements minimal targeted solution, includes excellent documentation, validates with comprehensive tests, and directly improves accuracy for AI coding assistants working with Haskell codebases. Zero risks, clear benefits.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 30, 2026

Greptile Summary

This PR gates the astTypeMap[node.type] lookup in ast-store-visitor.ts with Object.hasOwn, preventing JavaScript's prototype chain from surfacing Object.prototype.constructor (and other inherited members) as valid AST-node kinds when tree-sitter uses names like constructor for Haskell sum-type nodes. The companion change updates the KNOWN_REGRESSIONS comment to document the root cause and fix, leaving the benchmark entries in place until v3.9.7 data is captured.

Confidence Score: 5/5

Safe to merge — targeted one-line guard with a clear root-cause explanation and passing test suite.

The fix is minimal and correct: Object.hasOwn is the standard, spec-compliant way to prevent prototype-chain lookups on plain objects. The existing if (!kind) return guard below remains useful for genuinely falsy own-property values. Comments and regression-guard documentation are accurate. No new logic paths, no security implications, no regressions observed.

No files require special attention.

Important Files Changed

Filename Overview
src/ast-analysis/visitors/ast-store-visitor.ts Adds a single Object.hasOwn guard before the astTypeMap bracket-notation lookup; correctly prevents prototype-chain pollution (e.g. constructorObject.prototype.constructor) from leaking a non-cloneable function into the structured-clone boundary.
tests/benchmarks/regression-guard.test.ts Updates the KNOWN_REGRESSIONS comment for the Haskell precision/recall entry to describe the root cause and fix; the Set entries themselves are intentionally retained until v3.9.7 benchmark data is captured.

Sequence Diagram

sequenceDiagram
    participant Walker as AST Walker
    participant Visitor as ast-store-visitor
    participant Map as astTypeMap (plain object)
    participant Worker as Worker boundary (structured-clone)

    Walker->>Visitor: enterNode(node{type:"constructor"})
    Note over Visitor: Before fix
    Visitor->>Map: astTypeMap["constructor"]
    Map-->>Visitor: Object.prototype.constructor (function)
    Visitor->>Worker: structured-clone(row{kind: Object()})
    Worker-->>Visitor: ❌ DataCloneError — function could not be cloned

    Walker->>Visitor: enterNode(node{type:"constructor"})
    Note over Visitor: After fix (this PR)
    Visitor->>Map: Object.hasOwn(astTypeMap, "constructor")
    Map-->>Visitor: false → return early
    Note over Visitor: Node skipped cleanly, no clone attempted
Loading

Reviews (1): Last reviewed commit: "fix(parity): gate astTypeMap lookup with..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

Codegraph Impact Analysis

2 functions changed9 callers affected across 3 files

  • createAstStoreVisitor in src/ast-analysis/visitors/ast-store-visitor.ts:184 (9 transitive callers)
  • enterNode in src/ast-analysis/visitors/ast-store-visitor.ts:266 (0 transitive callers)

@carlos-alm carlos-alm merged commit bb501e1 into main May 1, 2026
23 checks passed
@carlos-alm carlos-alm deleted the fix/haskell-resolver-1039 branch May 1, 2026 01:53
@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 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.

fix(parity): haskell resolver precision/recall dropped to 0% in 3.9.6

1 participant