Skip to content

[Fix] Normalizer and expand its scope#1780

Merged
Saga4 merged 1 commit intomainfrom
fix/normalizer
Mar 6, 2026
Merged

[Fix] Normalizer and expand its scope#1780
Saga4 merged 1 commit intomainfrom
fix/normalizer

Conversation

@Saga4
Copy link
Contributor

@Saga4 Saga4 commented Mar 6, 2026

No description provided.

@claude
Copy link
Contributor

claude bot commented Mar 6, 2026

Claude finished @Saga4's task in 2m 59s —— View job


PR Review Summary


Prek Checks

All checks pass — ruff check and ruff format both clean.

Mypy errors in support.py (missing type params for dict, TypeScriptSupport decorator type mismatch) are pre-existing and not introduced by this PR.


Code Review

What this PR does:

  • Wires normalize_js_code (tree-sitter-based AST normalizer) into JavaScriptSupport.normalize_code, replacing a simple line-stripping fallback
  • Expands find_function_node / find_function_at_line to handle var-assigned function expressions, generator functions, and multi-line arrow declarations
  • Adds tests for all new code paths

Findings:

1. Silent exception swallowing in normalize_code without logging (support.py:1712)

try:
    is_ts = self.treesitter_language == TreeSitterLanguage.TYPESCRIPT
    return normalize_js_code(source, typescript=is_ts)
except Exception:
    return source  # no log here

When the normalizer fails, it silently returns the original source unchanged. This means deduplication silently degrades to "no deduplication at all" with no observability. The inner normalize_js_code already falls back to _basic_normalize_js on tree-sitter failures — so this outer except only catches truly unexpected errors. A logger.debug or logger.warning would be worth adding.

Fix this →

2. Dead CodeNormalizer ABC kept in normalizer.py (normalizer.py:23–43)

The abstract class CodeNormalizer is retained "for reference" per the docstring, but it has no implementations anywhere in the codebase, is never imported, and just adds noise. If the interface contract needs documenting, a comment would be cleaner than a live ABC.

Fix this →

3. find_function_node and find_function_at_line share nearly identical logic (support.py:1199, 1286)

Both inner functions now have identical 3-branch trees (method_definition → function_declaration family → lexical/variable_declaration). This is pre-existing duplication that the PR extends. Not a blocker, but worth noting as a refactor target.

4. Multiline arrow function line-matching correctness (support.py:1319–1322)

if name == target_name and (
    (node.start_point[0] + 1) == target_line
    or (value_node.start_point[0] + 1) == target_line
):

The fix for multi-line const calculate =\n (a, b) => { ... } by also checking value_node.start_point is correct and well-tested. Good catch.

5. generator_function in named declaration check (support.py:1214)

"generator_function" is a tree-sitter node type for expressions (not named declarations). The code adds it to the named function declaration check alongside "generator_function_declaration". This is safe (it won't cause false positives since name matching still applies) but technically it belongs in the variable-assigned branch, not the named declaration branch. Low risk.


Duplicate Detection

No duplicates detected. The two inner functions (find_function_node / find_function_at_line) are near-duplicates of each other within the same file, but that's pre-existing design — they serve different call sites (body extraction vs. body replacement) and differ by the target_line parameter.


Test Coverage

All 140 tests pass. New tests cover:

  • var-assigned function expressions
  • Generator function expressions
  • Multi-line arrow functions
  • Async arrow functions
  • JS/TS deduplication via the normalizer

normalizer.py coverage: 71% (new file tested adequately for its public API)
support.py coverage: 46% (large existing file, acceptable given scope)


Optimization PRs

Merged #1779 (⚡️ Speed up InitDecorator.visit_ClassDef by 19%, CI was passing).


@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Mar 6, 2026

⚡️ Codeflash found optimizations for this PR

📄 11% (0.11x) speedup for JavaScriptSupport._find_and_extract_body in codeflash/languages/javascript/support.py

⏱️ Runtime : 1.91 milliseconds 1.72 milliseconds (best of 30 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch fix/normalizer).

Static Badge

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Mar 6, 2026

⚡️ Codeflash found optimizations for this PR

📄 114% (1.14x) speedup for JavaScriptSupport.normalize_code in codeflash/languages/javascript/support.py

⏱️ Runtime : 680 microseconds 317 microseconds (best of 5 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch fix/normalizer).

Static Badge

@Saga4 Saga4 merged commit 417623f into main Mar 6, 2026
19 of 27 checks passed
@Saga4 Saga4 deleted the fix/normalizer branch March 6, 2026 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant