Skip to content

⚡️ Speed up function _insert_declaration_after_dependencies by 1,230% in PR #1546 (follow-up-reference-graph) #1549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
codeflash-ai wants to merge 1 commit into from
Closed

⚡️ Speed up function _insert_declaration_after_dependencies by 1,230% in PR #1546 (follow-up-reference-graph) #1549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions codeflash/languages/javascript/code_replacer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,8 @@ def _insert_declaration_after_dependencies(
if insertion_line > 0 and lines[insertion_line - 1].strip():
decl_code = "\n" + decl_code

before = lines[:insertion_line]
after = lines[insertion_line:]

return "".join([*before, decl_code, *after])
# Directly construct the result without intermediate list
return "".join(lines[:insertion_line]) + decl_code + "".join(lines[insertion_line:])


# Author: ali <mohammed18200118@gmail.com>
Expand All @@ -174,10 +172,11 @@ def _find_insertion_line_for_declaration(

"""
# Find the maximum end line among referenced declarations
max_dependency_line = 0
for name in referenced_names:
if name in existing_decl_end_lines:
max_dependency_line = max(max_dependency_line, existing_decl_end_lines[name])
max_dependency_line = max(
(existing_decl_end_lines[name] for name in referenced_names if name in existing_decl_end_lines),
default=0
)


if max_dependency_line > 0:
# Insert after the last dependency (end_line is 1-indexed, we need 0-indexed)
Expand Down
14 changes: 14 additions & 0 deletions codeflash/languages/javascript/treesitter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

from tree_sitter import Node, Tree

_PARSER_CACHE: dict[TreeSitterLanguage, Parser] = {}
Copy link
Contributor

@claude claude bot Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Cache stores uninitialized parsers

_PARSER_CACHE is populated with Parser() (no language) at line 1780. These parsers have no grammar loaded and will fail when used to parse code.

If parser caching is the goal, the cache should store properly initialized parsers:

_PARSER_CACHE[self.language] = Parser(_get_language(self.language))

However, since the duplicate parser property that populates this cache should be removed (see other comment), this cache variable becomes unused and should also be removed.


logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -1770,6 +1772,18 @@ def _extract_type_definition(
)


@property
def parser(self) -> Parser:
"""Get or create the cached parser for this language."""
if self._parser is None:
# Check if we have a cached parser for this language
if self.language not in _PARSER_CACHE:
_PARSER_CACHE[self.language] = Parser()
# Assuming parser setup happens elsewhere or in subclass
self._parser = _PARSER_CACHE[self.language]
Comment on lines 1774 to +1783
Copy link
Contributor

@claude claude bot Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical Bug: Duplicate parser property shadows the correct implementation

This property redefines the existing parser property at line 149-154. In Python, the last definition wins, so this replaces the working implementation.

The original creates Parser(_get_language(self.language)) (correctly initialized with language grammar), but this version creates Parser() with no language argument, producing an uninitialized parser that cannot parse anything.

This also causes ruff check to fail with F811 (redefinition of unused name).

Suggested change
@property
def parser(self) -> Parser:
"""Get or create the cached parser for this language."""
if self._parser is None:
# Check if we have a cached parser for this language
if self.language not in _PARSER_CACHE:
_PARSER_CACHE[self.language] = Parser()
# Assuming parser setup happens elsewhere or in subclass
self._parser = _PARSER_CACHE[self.language]

The duplicate should be removed entirely. If parser caching is desired, modify the existing parser property at line 149-154 instead.

return self._parser


def get_analyzer_for_file(file_path: Path) -> TreeSitterAnalyzer:
"""Get the appropriate TreeSitterAnalyzer for a file based on its extension.

Expand Down
Loading