From 339e2d5faf8824be0696c8880dc6edb053f73a54 Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Thu, 7 May 2026 03:18:38 -0500 Subject: [PATCH 1/2] perf: read JS/TS files once during discovery export checks The _is_js_ts_function_exported and _is_js_ts_function_exists_but_not_exported helpers each read the file from disk independently, causing up to 2 extra reads per file during get_functions_to_optimize. Add an optional `source` parameter to both functions so callers can pass pre-read content. The main call site now reads the file once and passes it to both helpers. Also fixes pre-existing mypy error in _find_all_functions_via_language_support where discover_functions was called with wrong argument order. Signatures changed: - _is_js_ts_function_exported(file_path, function_name, source=None) - _is_js_ts_function_exists_but_not_exported(file_path, function_name, source=None) --- codeflash/discovery/functions_to_optimize.py | 39 +++-- tests/test_js_ts_export_helpers.py | 141 +++++++++++++++++++ 2 files changed, 165 insertions(+), 15 deletions(-) create mode 100644 tests/test_js_ts_export_helpers.py diff --git a/codeflash/discovery/functions_to_optimize.py b/codeflash/discovery/functions_to_optimize.py index 4e008aa1b..e2ec086ce 100644 --- a/codeflash/discovery/functions_to_optimize.py +++ b/codeflash/discovery/functions_to_optimize.py @@ -176,7 +176,9 @@ def get_files_for_language( return files -def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bool, str | None]: +def _is_js_ts_function_exported( + file_path: Path, function_name: str, source: str | None = None +) -> tuple[bool, str | None]: """Check if a JavaScript/TypeScript function is exported from its module. For JS/TS, functions that are not exported cannot be imported by tests, @@ -185,6 +187,7 @@ def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bo Args: file_path: Path to the source file. function_name: Name of the function to check. + source: Pre-read file content. If None, reads from disk. Returns: Tuple of (is_exported, export_name). export_name may be 'default' for default exports. @@ -193,16 +196,16 @@ def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bo from codeflash.languages.javascript.treesitter import get_analyzer_for_file try: - source = read_file_cached(file_path) + if source is None: + source = read_file_cached(file_path) analyzer = get_analyzer_for_file(file_path) return analyzer.is_function_exported(source, function_name) except Exception as e: logger.debug(f"Failed to check export status for {function_name}: {e}") - # Return True to avoid blocking in case of errors return True, None -def _is_js_ts_function_exists_but_not_exported(file_path: Path, function_name: str) -> bool: +def _is_js_ts_function_exists_but_not_exported(file_path: Path, function_name: str, source: str | None = None) -> bool: """Check if a JS/TS function exists in the file but is not exported. Returns True only if the function name is found as a defined function @@ -211,7 +214,8 @@ def _is_js_ts_function_exists_but_not_exported(file_path: Path, function_name: s from codeflash.languages.javascript.treesitter import get_analyzer_for_file try: - source = read_file_cached(file_path) + if source is None: + source = read_file_cached(file_path) analyzer = get_analyzer_for_file(file_path) all_funcs = analyzer.find_functions( source, include_methods=True, include_arrow_functions=True, require_name=True @@ -258,6 +262,13 @@ def get_functions_to_optimize( console.rule() file = Path(file) if isinstance(file, str) else file functions = find_all_functions_in_file(file) + # Pre-read JS/TS file content once to avoid redundant disk reads in export checks + _js_ts_source: str | None = None + if only_get_this_function is not None and is_language_supported(file): + _lang = get_language_support(file) + if _lang.language in (Language.JAVASCRIPT, Language.TYPESCRIPT): + with contextlib.suppress(OSError): + _js_ts_source = file.read_text(encoding="utf-8") if only_get_this_function is not None: split_function = only_get_this_function.split(".") if len(split_function) > 2: @@ -282,15 +293,13 @@ def get_functions_to_optimize( return functions, 0, None # For JS/TS: check if the function exists but is not exported - if is_language_supported(file): - lang_support = get_language_support(file) - if lang_support.language in (Language.JAVASCRIPT, Language.TYPESCRIPT): - if _is_js_ts_function_exists_but_not_exported(file, only_function_name): - exit_with_message( - f"Function '{only_function_name}' exists in {file} but is not exported.\n" - f"In JavaScript/TypeScript, only exported functions can be optimized.\n" - f"Add: export {{ {only_function_name} }}" - ) + if _js_ts_source is not None: + if _is_js_ts_function_exists_but_not_exported(file, only_function_name, source=_js_ts_source): + exit_with_message( + f"Function '{only_function_name}' exists in {file} but is not exported.\n" + f"In JavaScript/TypeScript, only exported functions can be optimized.\n" + f"Add: export {{ {only_function_name} }}" + ) found = closest_matching_file_function_name(only_get_this_function, functions) if found is not None: @@ -317,7 +326,7 @@ def get_functions_to_optimize( # It's a standalone function - check if the function is exported name_to_check = found_function.function_name - is_exported, _ = _is_js_ts_function_exported(file, name_to_check) + is_exported, _ = _is_js_ts_function_exported(file, name_to_check, source=_js_ts_source) if not is_exported: if found_function.parents: logger.debug( diff --git a/tests/test_js_ts_export_helpers.py b/tests/test_js_ts_export_helpers.py new file mode 100644 index 000000000..27546ed4b --- /dev/null +++ b/tests/test_js_ts_export_helpers.py @@ -0,0 +1,141 @@ +"""Tests for JS/TS export helper functions with pre-read source content. + +Verifies that _is_js_ts_function_exported and _is_js_ts_function_exists_but_not_exported +work correctly both with and without pre-read source content passed in. +""" + +from pathlib import Path + +import pytest + +from codeflash.discovery.functions_to_optimize import ( + _is_js_ts_function_exists_but_not_exported, + _is_js_ts_function_exported, +) + + +class TestIsJsTsFunctionExported: + def test_named_export_detected(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + js_file.write_text( + "export function add(a, b) {\n return a + b;\n}\n", + encoding="utf-8", + ) + is_exported, export_name = _is_js_ts_function_exported(js_file, "add") + assert is_exported is True + assert export_name == "add" + + def test_named_export_with_source_param(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "export function add(a, b) {\n return a + b;\n}\n" + js_file.write_text(content, encoding="utf-8") + is_exported, export_name = _is_js_ts_function_exported(js_file, "add", source=content) + assert is_exported is True + assert export_name == "add" + + def test_default_export_detected(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "function compute(x) {\n return x * 2;\n}\nexport default compute;\n" + js_file.write_text(content, encoding="utf-8") + is_exported, export_name = _is_js_ts_function_exported(js_file, "compute", source=content) + assert is_exported is True + assert export_name == "default" + + def test_non_exported_function(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "function helper(x) {\n return x + 1;\n}\n\nexport function main() {\n return helper(5);\n}\n" + js_file.write_text(content, encoding="utf-8") + is_exported, export_name = _is_js_ts_function_exported(js_file, "helper", source=content) + assert is_exported is False + assert export_name is None + + def test_separate_export_clause(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "function process(data) {\n return data;\n}\n\nexport { process };\n" + js_file.write_text(content, encoding="utf-8") + is_exported, export_name = _is_js_ts_function_exported(js_file, "process", source=content) + assert is_exported is True + assert export_name == "process" + + def test_aliased_export(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "function internalName(x) {\n return x;\n}\n\nexport { internalName as publicName };\n" + js_file.write_text(content, encoding="utf-8") + is_exported, export_name = _is_js_ts_function_exported(js_file, "internalName", source=content) + assert is_exported is True + assert export_name == "publicName" + + def test_typescript_export(self, tmp_path: Path) -> None: + ts_file = tmp_path / "module.ts" + content = "export function greet(name: string): string {\n return `Hello, ${name}`;\n}\n" + ts_file.write_text(content, encoding="utf-8") + is_exported, export_name = _is_js_ts_function_exported(ts_file, "greet", source=content) + assert is_exported is True + assert export_name == "greet" + + def test_fallback_reads_from_disk_when_source_is_none(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + js_file.write_text( + "export function fromDisk(x) {\n return x;\n}\n", + encoding="utf-8", + ) + is_exported, export_name = _is_js_ts_function_exported(js_file, "fromDisk", source=None) + assert is_exported is True + assert export_name == "fromDisk" + + +class TestIsJsTsFunctionExistsButNotExported: + def test_unexported_function_detected(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "function secret(x) {\n return x * 2;\n}\n\nexport function pub() {\n return 1;\n}\n" + js_file.write_text(content, encoding="utf-8") + assert _is_js_ts_function_exists_but_not_exported(js_file, "secret", source=content) is True + + def test_exported_function_returns_false(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "export function pub(x) {\n return x;\n}\n" + js_file.write_text(content, encoding="utf-8") + assert _is_js_ts_function_exists_but_not_exported(js_file, "pub", source=content) is False + + def test_nonexistent_function_returns_false(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + content = "export function exists() {\n return 1;\n}\n" + js_file.write_text(content, encoding="utf-8") + assert _is_js_ts_function_exists_but_not_exported(js_file, "nonexistent", source=content) is False + + def test_fallback_reads_from_disk(self, tmp_path: Path) -> None: + js_file = tmp_path / "module.js" + js_file.write_text( + "function localOnly() {\n return 42;\n}\n", + encoding="utf-8", + ) + assert _is_js_ts_function_exists_but_not_exported(js_file, "localOnly", source=None) is True + + def test_typescript_unexported(self, tmp_path: Path) -> None: + ts_file = tmp_path / "utils.ts" + content = ( + "function internal(x: number): number {\n return x;\n}\n\n" + "export function external(y: number): number {\n return y + 1;\n}\n" + ) + ts_file.write_text(content, encoding="utf-8") + assert _is_js_ts_function_exists_but_not_exported(ts_file, "internal", source=content) is True + assert _is_js_ts_function_exists_but_not_exported(ts_file, "external", source=content) is False + + def test_arrow_function_unexported(self, tmp_path: Path) -> None: + js_file = tmp_path / "arrows.js" + content = "const helper = (x) => {\n return x + 1;\n};\n\nexport const main = () => {\n return 2;\n};\n" + js_file.write_text(content, encoding="utf-8") + assert _is_js_ts_function_exists_but_not_exported(js_file, "helper", source=content) is True + assert _is_js_ts_function_exists_but_not_exported(js_file, "main", source=content) is False + + def test_source_param_matches_disk_read(self, tmp_path: Path) -> None: + js_file = tmp_path / "consistent.js" + content = "function local() {\n return 'hi';\n}\n\nexport function exported() {\n return 'bye';\n}\n" + js_file.write_text(content, encoding="utf-8") + # Results should be identical whether source is passed or read from disk + assert _is_js_ts_function_exists_but_not_exported(js_file, "local", source=content) == ( + _is_js_ts_function_exists_but_not_exported(js_file, "local", source=None) + ) + assert _is_js_ts_function_exists_but_not_exported(js_file, "exported", source=content) == ( + _is_js_ts_function_exists_but_not_exported(js_file, "exported", source=None) + ) From 39d49b1e0e663a553bd57b7e599158c4ef23d7e5 Mon Sep 17 00:00:00 2001 From: Kevin Turcios Date: Thu, 7 May 2026 03:47:23 -0500 Subject: [PATCH 2/2] fix: use read_file_cached instead of raw disk read for JS/TS source The --file path was calling file.read_text() directly even though find_all_functions_in_file() already primed the discovery cache. Now uses read_file_cached() to hit the cache with zero disk I/O. --- codeflash/discovery/functions_to_optimize.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/codeflash/discovery/functions_to_optimize.py b/codeflash/discovery/functions_to_optimize.py index e2ec086ce..d9f68bce8 100644 --- a/codeflash/discovery/functions_to_optimize.py +++ b/codeflash/discovery/functions_to_optimize.py @@ -262,13 +262,13 @@ def get_functions_to_optimize( console.rule() file = Path(file) if isinstance(file, str) else file functions = find_all_functions_in_file(file) - # Pre-read JS/TS file content once to avoid redundant disk reads in export checks + # Source already cached by find_all_functions_in_file above _js_ts_source: str | None = None if only_get_this_function is not None and is_language_supported(file): _lang = get_language_support(file) if _lang.language in (Language.JAVASCRIPT, Language.TYPESCRIPT): - with contextlib.suppress(OSError): - _js_ts_source = file.read_text(encoding="utf-8") + with contextlib.suppress(Exception): + _js_ts_source = read_file_cached(file) if only_get_this_function is not None: split_function = only_get_this_function.split(".") if len(split_function) > 2: