Skip to content

fix(parser): add explicit continue for self_parameter skip (#649)#657

Merged
carlos-alm merged 1 commit intomainfrom
fix/wasm-parity-649
Mar 27, 2026
Merged

fix(parser): add explicit continue for self_parameter skip (#649)#657
carlos-alm merged 1 commit intomainfrom
fix/wasm-parity-649

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

  • Adds explicit continue in extractRustParameters when skipping self_parameter nodes, matching native engine behavior

The original parity fix (commit c898a4a on fix/wasm-engine-parity-649) contained 5 fixes. Four of the five already reached main through other PRs (#618, #637, #651, etc.). This PR lands the one remaining change: an explicit continue statement in the WASM Rust extractor's self_parameter guard.

While the empty if block was functionally equivalent (the else if prevented fall-through), the explicit continue is clearer and matches the pattern used elsewhere in the codebase.

Closes #649

Test plan

  • CI passes (no behavioral change — the else if already prevented self_parameter from being extracted)
  • Engine parity tests continue to pass

Four root causes fixed across both engines:

WASM JS/TS: extractConstantsWalk only walked root-level children,
missing constants inside top-level blocks (if, try, for). Now recurses
into all non-function scopes, matching native's recursive walk.

WASM JS/TS: query pattern only matched property_identifier for methods,
missing private #methods. Added private_property_identifier pattern.

WASM Rust: extractRustParameters included self_parameter nodes that
native correctly skips. Now skips them.

Native JS: find_parent_of_types missed generator_function_declaration
and generator_function, leaking constants from generator function bodies.
Added both types to the scope filter.

Native Rust: function_item handler emitted a duplicate entry for trait
default-impl functions already captured by trait_item handler. Added
parent guard matching WASM's existing skip logic.
@claude
Copy link
Copy Markdown

claude bot commented Mar 27, 2026

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


Comprehensive PR Review

PR #657: fix(parser): add explicit continue for self_parameter skip

Root Cause Analysis

Problem Being Solved: WASM-native engine parity gap in Rust parameter extraction. The WASM engine had functionally correct but stylistically inconsistent code compared to the native engine.

Why This Approach: Both engines must produce identical results per CLAUDE.md requirements. The native engine explicitly uses continue for self_parameter nodes (rust_lang.rs:261-263), while WASM used an empty if block. This change aligns the code style without changing behavior.

Risk Assessment: ZERO RISK - This is purely cosmetic. The else if (param.type === 'parameter') on line 232 already prevented fall-through, making the empty if functionally equivalent to explicit continue.

Configuration Changes (✓ PASS)

  • No configuration, linting, or test changes
  • No rules relaxed or timeouts increased
  • Zero files modified beyond the single line addition

Test Integrity (✓ PASS)

  • No tests modified - this cannot weaken test assertions
  • Author explicitly states "no behavioral change"
  • Engine parity tests at tests/engines/parity.test.ts:75-78 filter out self parameters, confirming both engines should skip them

Code Quality (✓ PASS)

  • Correctness: The existing else if already prevented self_parameter extraction, so this adds explicit clarity without behavior change
  • Consistency: Now matches native engine style (rust_lang.rs:261-263)
  • Readability: Explicit continue is clearer intent than empty if block

Backlog Compliance

  • Zero-dep: ✓ — No new dependencies
  • Foundation-aligned: ✓ — Aligns with dual-engine architecture principle, ensures identical output
  • Problem-fit: 3 — Prevents engine divergence, indirectly helps AI agents by ensuring consistent results
  • Breaking: No — Pure internal refactor, zero API/output changes
  • Tier: 1 (foundation-aligned ✓ + zero-dep ✓)

Critical Concerns

NONE - This is the safest possible change:

  1. Single line addition with explicit early return
  2. Functionally equivalent to existing logic
  3. Matches native engine implementation exactly
  4. Zero test modifications or behavioral changes
  5. Part of broader parity effort that already landed other fixes

Final Recommendation

  • Rating: ⭐⭐⭐⭐ (4/5) — High quality, well-justified, zero risk
  • Action: APPROVE
  • Reasoning: Textbook example of safe parity fix. Improves code clarity and engine consistency with zero behavioral impact. The explicit continue makes the intent crystal clear and aligns with the native implementation.

Note: While stylistic changes usually merit lower priority, engine parity is foundational to codegraph's dual-engine architecture. This change eliminates a minor inconsistency that could confuse future maintainers.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 27, 2026

Greptile Summary

This PR adds an explicit continue statement inside the self_parameter guard of extractRustParameters in the WASM Rust extractor, completing the final piece of the engine-parity fix originally tracked in #649.

Key points:

Confidence Score: 5/5

Safe to merge — single-line clarity fix with no behavioral change.

The change is a one-line addition of an explicit continue that was already implicitly enforced by the else if structure. No logic is altered, no data flow changes, and the PR description accurately captures the history. No issues found.

No files require special attention.

Important Files Changed

Filename Overview
src/extractors/rust.ts Adds explicit continue in extractRustParameters for the self_parameter guard — purely a clarity improvement with no behavioral change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Loop over paramListNode children] --> B{param is null?}
    B -- Yes --> C[continue]
    B -- No --> D{param.type === 'self_parameter'?}
    D -- Yes --> E["// Skip self parameter\ncontinue <- NEW explicit continue"]
    D -- No --> F{param.type === 'parameter'?}
    F -- Yes --> G[Extract pattern child]
    G --> H{pattern exists?}
    H -- Yes --> I[Push SubDeclaration to params]
    H -- No --> J[Skip]
    F -- No --> K[Skip other node types]
    I --> A
    J --> A
    K --> A
    C --> A
    E --> A
Loading

Reviews (1): Last reviewed commit: "fix(parser): close WASM–native engine pa..." | Re-trigger Greptile

@carlos-alm carlos-alm merged commit 94245f6 into main Mar 27, 2026
15 checks passed
@carlos-alm carlos-alm deleted the fix/wasm-parity-649 branch March 27, 2026 22:11
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 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.

bug: WASM engine extracts 26 fewer nodes than native (engine parity gap)

1 participant