fix: enable extended kinds in parity tests#327
Conversation
Three optimizations to recover build speed after CFG/dataflow default-on:
1. Eliminate redundant WASM parsing (biggest win):
- Remove complexity.js clearing _tree after each file (builder already
clears all trees after all phases complete)
- Add ensureWasmTrees() in parser.js for a single WASM pre-parse pass
before CFG/dataflow, replacing two independent parse passes
- Memoize createParsers() so repeated calls return cached parsers
2. Filter CFG/dataflow to changed files only:
- Build changedSymbols map excluding reverse-dep-only files
- Pass changedSymbols (not allSymbols) to buildCFGData/buildDataflowEdges
- Reverse-dep files only need edge rebuilding, not AST re-analysis
3. Report wasmPreMs in phase timing output for visibility
Impact: 4 functions changed, 28 affected
Impact: 4 functions changed, 10 affected
…ty tests Update parity tests to compare children (parameters, properties, constants) and call receivers now that the native binary supports them. Document known native gaps: implicit self parameters (Python/Rust) and local constant scope (extracts function-local const as top-level).
|
Claude finished @carlos-alm's task —— View job Reviewing PR: Enable Extended Kinds in Parity TestsTodo List:
Phase 1: Root Cause Analysis ✅Problem Being Solved: This PR addresses a legitimate technical debt issue where parity tests between WASM and native engines were artificially limited due to an outdated native binary (v2.5.35-dev). The tests were correctly excluding "children" (parameters/properties) and "call receivers" from comparison because the native engine was missing these features. Why This Approach: The approach is justified and proper:
Risk Assessment: ✅ LOW RISK - This is fixing real issues, not hiding symptoms. Phase 2: Configuration Change Detection ✅Package Updates: ✅ LEGITIMATE
No High-Risk Changes Found: No disabled linting rules, relaxed coverage thresholds, or build configuration modifications. Phase 3: Test Integrity ✅Assertions STRENGTHENED, Not Weakened:
Gap Documentation: Both test files now include explicit comments explaining why certain comparisons are filtered, making the technical debt visible rather than hidden. Phase 4: Code Quality Review ✅Associated Performance Changes (from previous commit):
Code Quality Assessment:
No Security Concerns: Package updates are from official registry with proper integrity hashes. Root Cause AnalysisProblem Being Solved: Outdated native binary (48 commits behind) artificially limiting test coverage Critical ConcernsNone. This PR demonstrates excellent engineering practices:
Final Recommendation
|
The native engine's JS extractor walked the entire tree recursively and extracted `const x = <literal>` as top-level constants regardless of scope. The WASM extractor correctly only walks program-level children. Add a `find_parent_of_types` check to skip constants nested inside function_declaration, arrow_function, function_expression, or method_definition nodes. Impact: 2 functions changed, 1 affected
|
Claude finished @carlos-alm's task —— View job Reviewing PR: Enable Extended Kinds in Parity TestsTodo List:
Phase 1: Root Cause Analysis ✅Problem Being Solved: This PR addresses a legitimate technical debt issue where parity tests between WASM and native engines were artificially limited due to an outdated native binary (v2.5.35-dev). The tests were correctly excluding "children" (parameters/properties) and "call receivers" from comparison because the native engine was missing these features. Why This Approach: The approach is justified and proper:
Risk Assessment: ✅ LOW RISK - This is fixing real issues, not hiding symptoms. Phase 2: Configuration Change Detection ✅Package Updates: ✅ LEGITIMATE
No High-Risk Changes Found: No disabled linting rules, relaxed coverage thresholds, or build configuration modifications. Phase 3: Test Integrity ✅Assertions STRENGTHENED, Not Weakened:
Gap Documentation: Both test files now include explicit comments explaining why certain comparisons are filtered, making the technical debt visible rather than hidden. Phase 4: Code Quality Review ✅Associated Performance Changes (from previous commit):
Code Quality Assessment:
No Security Concerns: Package updates are from official registry with proper integrity hashes. Root Cause AnalysisProblem Being Solved: Outdated native binary (48 commits behind) artificially limiting test coverage Critical ConcernsNone. This PR demonstrates excellent engineering practices:
Final Recommendation
|
Greptile SummaryThis PR updates the native binary from v2.5.35-dev to v3.0.1 and enables extended kinds (children/receivers) in parity tests. Key changes:
All changes are well-documented and the PR description clearly outlines known native engine limitations with appropriate workarounds. Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildGraph starts] --> B[Native engine parses files]
B --> C[Store symbols in allSymbols map]
C --> D{Incremental build?}
D -->|Yes| E[Filter to astComplexitySymbols<br/>excluding reverse-dep-only files]
D -->|No| F[astComplexitySymbols = allSymbols]
E --> G[AST node extraction]
F --> G
G --> H[Complexity analysis<br/>tree remains cached]
H --> I{CFG or dataflow enabled?}
I -->|Yes| J[ensureWasmTrees<br/>pre-parse files missing _tree]
I -->|No| N
J --> K[CFG analysis<br/>reuses cached trees]
K --> L[Dataflow analysis<br/>reuses cached trees]
L --> M[All phases done]
M --> N[Clean up: symbols._tree = null]
N --> O[Build complete]
style J fill:#d4f1d4
style H fill:#d4f1d4
style K fill:#d4f1d4
style L fill:#d4f1d4
Last reviewed commit: 14a82bf |
1 participant
Summary
npm installtests/engines/parity.test.jsto compare children (parameters) and call receivers — previously excluded pending native binary rebuildtests/integration/build-parity.test.jsto filter outconstantnodes from comparison due to known native scope bugKnown native gaps documented
selfparameters: Native engine doesn't extractself/&selffor Python/Rust methods — filtered in parity normalizeconstvariables as top-level constants (WASM correctly limits to program-level) — excluded from build-parity comparisonTest plan
npx vitest run tests/engines/parity.test.js— 10 pass, 3 skipped (Ruby/PHP/HCL known gaps)npx vitest run tests/integration/build-parity.test.js— 2 passnpm test— 1431 pass, 0 failures