Behavior Test Instrumentation to account for input mutation #867

aseembits93 · 2025-10-31T18:13:40Z

PR Type

Enhancement, Tests

Description

Bind call args via inspect.signature
Apply defaults to bound arguments
Route args/kwargs through wrapper in behavior mode
Add inspect import for instrumentation

Diagram Walkthrough

flowchart LR
  A["Test AST Call Node"] -- "extract args/keywords" --> B["get_call_arguments()"]
  B -- "bind to signature" --> C["inspect.signature(...).bind(...)"]
  C -- "apply_defaults()" --> D["_call__bound__arguments"]
  D -- "provide args/kwargs" --> E["codeflash_wrap(...)"]
  A -- "performance mode" --> F["pass original args/keywords"]

File Walkthrough

Relevant files

Enhancement

instrument_existing_tests.py `Bind call args and pass via wrapper` codeflash/code_utils/instrument_existing_tests.py Add dataclass `FunctionCallNodeArguments`. Bind and default call args using inspect. In behavior mode, pass bound args/kwargs to wrapper. Import `inspect` for new binding logic.	+155/-6

github-actions · 2025-10-31T18:14:39Z

PR Reviewer Guide 🔍

(Review updated until commit `ffff5f1`)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Possible Issue In behavior mode, the wrapper is passed `bound.args` and `kwargs=bound.kwargs`, but the first positional argument for `codeflash_wrap` is the original target function/expression. Ensure the bound args exclude the target callable and that the wrapper's signature matches the new calling convention; otherwise arguments may be shifted or misrouted. node.func = ast.Name(id="codeflash_wrap", ctx=ast.Load()) node.args = [ ast.Name(id=function_name, ctx=ast.Load()), ast.Constant(value=self.module_path), ast.Constant(value=test_class_name or None), ast.Constant(value=node_name), ast.Constant(value=self.function_object.qualified_name), ast.Constant(value=index), ast.Name(id="codeflash_loop_index", ctx=ast.Load()), ( [ast.Name(id="codeflash_cur", ctx=ast.Load()), ast.Name(id="codeflash_con", ctx=ast.Load())] if self.mode == TestingMode.BEHAVIOR else [] ), ( call_node.args if self.mode == TestingMode.PERFORMANCE else [ ast.Starred( value=ast.Attribute( value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), attr="args", ctx=ast.Load(), ), ctx=ast.Load(), ) ] ), ] node.keywords = ( [ ast.keyword( value=ast.Attribute( value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), attr="kwargs", ctx=ast.Load(), ) ) ] if self.mode == TestingMode.BEHAVIOR else call_node.keywords ) AST Linenos* The injected `bind_call` and `apply_defaults` nodes set `lineno` relative to `test_node`, but `end_lineno`, `end_col_offset`, and `ctx` consistency are not set. This can cause formatting or compile issues under strict AST consumers. Consider fixing line/col and calling `ast.fix_missing_locations`. # Create the signature binding statements bind_call = ast.Assign( targets=[ast.Name(id="_call__bound__arguments", ctx=ast.Store())], value=ast.Call( func=ast.Attribute( value=ast.Call( func=ast.Attribute( value=ast.Name(id="inspect", ctx=ast.Load()), attr="signature", ctx=ast.Load() ), args=[ast.Name(id=function_name, ctx=ast.Load())], keywords=[], ), attr="bind", ctx=ast.Load(), ), args=all_args.args, keywords=all_args.keywords, ), lineno=test_node.lineno, col_offset=test_node.col_offset, ) apply_defaults = ast.Expr( value=ast.Call( func=ast.Attribute( value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), attr="apply_defaults", ctx=ast.Load(), ), args=[], keywords=[], ), lineno=test_node.lineno + 1, col_offset=test_node.col_offset, ) Attribute Calls For attribute-based calls, `function_name` is reparsed with `ast.parse(..., mode="eval").body`. Re-parsing untrusted or complex expressions may diverge from the original node (loss of location/ctx and potential mismatch with bound args). Prefer using the existing `node.func` directly where possible. function_name = ast.unparse(node.func) # Create the signature binding statements bind_call = ast.Assign( targets=[ast.Name(id="_call__bound__arguments", ctx=ast.Store())], value=ast.Call( func=ast.Attribute( value=ast.Call( func=ast.Attribute( value=ast.Name(id="inspect", ctx=ast.Load()), attr="signature", ctx=ast.Load(), ), args=[ast.parse(function_name, mode="eval").body], keywords=[], ), attr="bind", ctx=ast.Load(), ), args=all_args.args, keywords=all_args.keywords, ), lineno=test_node.lineno, col_offset=test_node.col_offset, ) apply_defaults = ast.Expr( value=ast.Call( func=ast.Attribute( value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), attr="apply_defaults", ctx=ast.Load(), ), args=[], keywords=[], ), lineno=test_node.lineno + 1, col_offset=test_node.col_offset, ) node.func = ast.Name(id="codeflash_wrap", ctx=ast.Load()) node.args = [ ast.parse(function_name, mode="eval").body, ast.Constant(value=self.module_path), ast.Constant(value=test_class_name or None),

github-actions · 2025-10-31T18:14:59Z

PR Code Suggestions ✨

Latest suggestions up to ffff5f1
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Safeguard signature binding failures Guard the `inspect.signature(...).bind(...)` call to avoid runtime crashes when binding fails (e.g., varargs, missing/extra args) or when `function_name` is not introspectable. Wrap it in a try/except and fall back to original `call_node.args`/`call_node.keywords` if binding errors occur. codeflash/code_utils/instrument_existing_tests.py [99-119] -bind_call = ast.Assign( - targets=[ast.Name(id="_call__bound__arguments", ctx=ast.Store())], - value=ast.Call( - func=ast.Attribute( - value=ast.Call( - func=ast.Attribute( - value=ast.Name(id="inspect", ctx=ast.Load()), attr="signature", ctx=ast.Load() +try: + bind_call = ast.Assign( + targets=[ast.Name(id="_call__bound__arguments", ctx=ast.Store())], + value=ast.Call( + func=ast.Attribute( + value=ast.Call( + func=ast.Attribute( + value=ast.Name(id="inspect", ctx=ast.Load()), attr="signature", ctx=ast.Load() + ), + args=[ast.Name(id=function_name, ctx=ast.Load())], + keywords=[], ), - args=[ast.Name(id=function_name, ctx=ast.Load())], - keywords=[], + attr="bind", + ctx=ast.Load(), ), - attr="bind", - ctx=ast.Load(), + args=all_args.args, + keywords=all_args.keywords, ), - args=all_args.args, - keywords=all_args.keywords, - ), - lineno=test_node.lineno, - col_offset=test_node.col_offset, -) + lineno=test_node.lineno, + col_offset=test_node.col_offset, + ) + apply_defaults = ast.Expr( + value=ast.Call( + func=ast.Attribute( + value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), + attr="apply_defaults", + ctx=ast.Load(), + ), + args=[], + keywords=[], + ), + lineno=test_node.lineno + 1, + col_offset=test_node.col_offset, + ) + binding_ok = True +except Exception: + binding_ok = False Suggestion importance[1-10]: 5 __ Why: The idea to guard `inspect.signature(...).bind(...)` from runtime failures is reasonable, but introducing a Python try/except around AST construction isn't directly applicable inside AST-building code and the PR already imports `inspect`. Impact is moderate and the proposed improved_code doesn't integrate cleanly with the transformer logic.	Low
Possible issue	Add robust fallback for args Ensure a fallback to original arguments when binding is unavailable (e.g., earlier guard sets `binding_ok=False`). Use conditional construction so BEHAVIOR mode still works without `_call__bound__arguments`, preventing NameError at runtime. codeflash/code_utils/instrument_existing_tests.py [141-176] +star_args = ( + [ + ast.Starred( + value=ast.Attribute( + value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), + attr="args", + ctx=ast.Load(), + ), + ctx=ast.Load(), + ) + ] + if (self.mode == TestingMode.BEHAVIOR and 'binding_ok' in locals() and binding_ok) + else list(call_node.args) +) +kw_assign = ( + [ + ast.keyword( + value=ast.Attribute( + value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), + attr="kwargs", + ctx=ast.Load(), + ) + ) + ] + if (self.mode == TestingMode.BEHAVIOR and 'binding_ok' in locals() and binding_ok) + else list(call_node.keywords) +) node.args = [ ast.Name(id=function_name, ctx=ast.Load()), ast.Constant(value=self.module_path), ast.Constant(value=test_class_name or None), ast.Constant(value=node_name), ast.Constant(value=self.function_object.qualified_name), ast.Constant(value=index), ast.Name(id="codeflash_loop_index", ctx=ast.Load()), ( [ast.Name(id="codeflash_cur", ctx=ast.Load()), ast.Name(id="codeflash_con", ctx=ast.Load())] if self.mode == TestingMode.BEHAVIOR else [] ), - ( - call_node.args - if self.mode == TestingMode.PERFORMANCE - else [ - ast.Starred( - value=ast.Attribute( - value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), - attr="args", - ctx=ast.Load(), - ), - ctx=ast.Load(), - ) - ] - ), + *star_args, ] -node.keywords = ( - [ - ast.keyword( - value=ast.Attribute( - value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), - attr="kwargs", - ctx=ast.Load(), - ) - ) - ] - if self.mode == TestingMode.BEHAVIOR - else call_node.keywords -) +node.keywords = kw_assign Suggestion importance[1-10]: 4 __ Why: Providing a fallback to original `call_node` args/keywords if binding isn't available is sensible; however, referencing runtime `locals()`/`binding_ok` within AST construction is not coherent in this context and `existing_code` spans behavior already conditional on `TestingMode`. The concept helps robustness but the concrete patch is not accurate.	Low
General	Gate insertion on success Insert the binding nodes only when binding succeeded, and ensure they precede the modified call node in execution order. Otherwise you risk referencing `_call__bound__arguments` before assignment or inserting unused nodes. codeflash/code_utils/instrument_existing_tests.py [176-181] -apply_defaults = ast.Expr( - value=ast.Call( - func=ast.Attribute( - value=ast.Name(id="_call__bound__arguments", ctx=ast.Load()), - attr="apply_defaults", - ctx=ast.Load(), - ), - args=[], - keywords=[], - ), - lineno=test_node.lineno + 1, - col_offset=test_node.col_offset, -) -... -return_statement = ( - [bind_call, apply_defaults, test_node] if self.mode == TestingMode.BEHAVIOR else [test_node] -) +if self.mode == TestingMode.BEHAVIOR and 'binding_ok' in locals() and binding_ok: + return_statement = [bind_call, apply_defaults, test_node] +else: + return_statement = [test_node] Suggestion importance[1-10]: 5 __ Why: Conditioning the insertion of binding nodes is a reasonable guard against referencing `_call__bound__arguments` before assignment, but the suggested use of `binding_ok`/`locals()` is not defined in the current AST-building flow. The intent is valid yet the proposed code doesn't align with how the transformer returns nodes.	Low

Previous suggestions

Suggestions up to commit b91b61f

Category	Suggestion	Impact
Possible issue	Fix invalid AST argument access Using `call_node.args` and `call_node.keywords` directly is required; `ast.Call` has no `args`/`keywords` attributes on the call expression object in the AST of the test (they exist, but accessing via `Attribute(value=call_node, attr="args")` produces invalid Python). Build the bind call with the actual arg and keyword lists from `call_node`. This prevents generation of an unparsable AST and runtime failures. codeflash/code_utils/instrument_existing_tests.py [87-108] bind_call = ast.Assign( targets=[ast.Name(id="_call__bound__arguments", ctx=ast.Store())], value=ast.Call( func=ast.Attribute( value=ast.Call( func=ast.Attribute( value=ast.Name(id="inspect", ctx=ast.Load()), attr="signature", ctx=ast.Load() ), args=[ast.Name(id=function_name, ctx=ast.Load())], keywords=[] ), attr="bind", ctx=ast.Load() ), - args=[ast.Starred(value=ast.Attribute(value=call_node, attr="args", ctx=ast.Load()), ctx=ast.Load())], - keywords=[ast.keyword(arg=None, value=ast.Attribute(value=call_node, attr="keywords", ctx=ast.Load()))] + args=call_node.args, + keywords=call_node.keywords ), - lineno=test_node.lineno if hasattr(test_node, 'lineno') else 1, - col_offset=test_node.col_offset if hasattr(test_node, 'col_offset') else 0 + lineno=getattr(test_node, "lineno", 1), + col_offset=getattr(test_node, "col_offset", 0) ) Suggestion importance[1-10]: 8 __ Why: The existing code incorrectly builds `ast.Attribute(value=call_node, attr="args"/"keywords")`, which is not a valid AST for passing arguments; using `call_node.args` and `call_node.keywords` is correct and prevents unparsable ASTs.	Medium
	Correct AST args list typing Passing `args=function_name` is incorrect; `ast.Call.args` must be a list of AST nodes, not a string. Wrap the unparsed name in an `ast.parse(...).body[0].value` or keep it as the original `node.func` expression to maintain correct AST typing. This prevents malformed AST and compilation errors. codeflash/code_utils/instrument_existing_tests.py [153-174] function_name = ast.unparse(node.func) -# Create the signature binding statements bind_call = ast.Assign( targets=[ast.Name(id="_call__bound__arguments", ctx=ast.Store())], value=ast.Call( func=ast.Attribute( value=ast.Call( func=ast.Attribute( value=ast.Name(id="inspect", ctx=ast.Load()), attr="signature", ctx=ast.Load() ), - args=function_name, + args=[ast.parse(function_name, mode="eval").body], keywords=[] ), attr="bind", ctx=ast.Load() ), args=call_node.args, keywords=call_node.keywords ), lineno=test_node.lineno, col_offset=test_node.col_offset ) Suggestion importance[1-10]: 7 __ Why: The code sets `args=function_name`, a string, which is invalid for `ast.Call.args`; wrapping it as an AST node list fixes a real type error, though using the original `node.func` could be an alternative.	Medium
	Ensure return variable is defined `return_statement` is only defined in one branch; when the `ast.Name`-call path returns early, this later reference can be undefined, causing an UnboundLocalError. Initialize a default `return_statement = [test_node]` before the loop and update it where needed to ensure safe returns. codeflash/code_utils/instrument_existing_tests.py [215-218] +return_statement = [test_node] +for node in ast.walk(test_node): + ... + # in attribute branch after building bind_call/apply_defaults: + return_statement = [bind_call, apply_defaults, test_node] + break + if call_node is None: return None return return_statement Suggestion importance[1-10]: 6 __ Why: `return_statement` can be undefined if no attribute-branch path sets it; initializing a default avoids `UnboundLocalError`, improving robustness with modest impact.	Low

CLAassistant · 2025-10-31T23:01:42Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Codeflash Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

github-actions · 2025-10-31T23:03:59Z

Persistent review updated to latest commit ffff5f1

The optimized code achieves a **22% speedup** through two main optimizations that reduce overhead in AST traversal and attribute lookups: **1. Custom AST traversal replaces expensive `ast.walk()`** The original code uses `ast.walk()` which creates recursive stack frames for every AST node. The optimized version implements `iter_ast_calls()` - a manual iterative traversal that only visits `ast.Call` nodes using a single stack. This eliminates Python's recursion overhead and reduces the O(N) stack frame creation to a single stack operation. **2. Reduced attribute lookups in hot paths** - In `node_in_call_position()`: Uses `getattr()` with defaults to cache node attributes (`node_lineno`, `node_end_lineno`, etc.) instead of repeated `hasattr()` + attribute access - In `find_and_update_line_node()`: Hoists frequently-accessed object attributes (`fn_obj.qualified_name`, `self.mode`, etc.) to local variables before the loop - Pre-creates reusable AST nodes (`codeflash_loop_index`, `codeflash_cur`, `codeflash_con`) instead of recreating them in each iteration **Performance characteristics:** - **Small AST trees** (basic function calls): 5-28% faster due to reduced attribute lookups - **Large AST trees** (deeply nested calls): 18-26% faster due to more efficient traversal avoiding `ast.walk()` - **Large call position lists**: 26% faster due to optimized position checking with cached attributes The optimizations are most effective for complex test instrumentation scenarios with large AST trees or many call positions to check, which is typical in code analysis and transformation workflows.

codeflash-ai · 2025-11-01T00:02:11Z

⚡️ Codeflash found optimizations for this PR

📄 22% (0.22x) speedup for `InjectPerfOnly.find_and_update_line_node` in `codeflash/code_utils/instrument_existing_tests.py`

⏱️ Runtime : 2.01 milliseconds → 1.64 milliseconds (best of 113 runs)

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

⚡️ Speed up method InjectPerfOnly.find_and_update_line_node by 22% in PR #867 (inspect-signature-issue) #870

If you approve, it will be merged into this PR (branch inspect-signature-issue).

codeflash/discovery/discover_unit_tests.py

…25-11-01T00.02.02 ⚡️ Speed up method `InjectPerfOnly.find_and_update_line_node` by 22% in PR #867 (`inspect-signature-issue`)

codeflash-ai · 2025-11-01T00:18:15Z

This PR is now faster! 🚀 @misrasaurabh1 accepted my optimizations from:

⚡️ Speed up method InjectPerfOnly.find_and_update_line_node by 22% in PR #867 (inspect-signature-issue) #870

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>

codeflash-ai · 2025-11-01T00:19:36Z

This PR is now faster! 🚀 Aseem Saxena accepted my code suggestion above.

wip

b91b61f

github-actions bot added the Review effort 3/5 label Oct 31, 2025

all tests fixed

ffff5f1

aseembits93 marked this pull request as ready for review October 31, 2025 23:03

Codeflash Bot and others added 2 commits October 31, 2025 16:55

tests modified now

ecbceec

codeflash-ai bot mentioned this pull request Nov 1, 2025

⚡️ Speed up method InjectPerfOnly.find_and_update_line_node by 22% in PR #867 (inspect-signature-issue) #870

Merged

tests work now

c2817f9

aseembits93 requested review from KRRT7, misrasaurabh1 and mohammedahmed18 and removed request for KRRT7, misrasaurabh1 and mohammedahmed18 November 1, 2025 00:11

codeflash-ai bot reviewed Nov 1, 2025

View reviewed changes

codeflash/discovery/discover_unit_tests.py Outdated Show resolved Hide resolved

Merge pull request #870 from codeflash-ai/codeflash/optimize-pr867-20…

40e82e2

…25-11-01T00.02.02 ⚡️ Speed up method `InjectPerfOnly.find_and_update_line_node` by 22% in PR #867 (`inspect-signature-issue`)

Update codeflash/discovery/discover_unit_tests.py

92da986

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Behavior Test Instrumentation to account for input mutation #867

Behavior Test Instrumentation to account for input mutation #867

aseembits93 commented Oct 31, 2025 •

edited by github-actions bot

Loading

Uh oh!

github-actions bot commented Oct 31, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 31, 2025 •

edited

Loading

Uh oh!

CLAassistant commented Oct 31, 2025

Uh oh!

github-actions bot commented Oct 31, 2025

Uh oh!

codeflash-ai bot commented Nov 1, 2025

⚡️ Speed up method `InjectPerfOnly.find_and_update_line_node` by 22% in PR #867 (`inspect-signature-issue`) #870

Uh oh!

Uh oh!

codeflash-ai bot commented Nov 1, 2025

Uh oh!

codeflash-ai bot commented Nov 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Behavior Test Instrumentation to account for input mutation #867

Are you sure you want to change the base?

Behavior Test Instrumentation to account for input mutation #867

Conversation

aseembits93 commented Oct 31, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit ffff5f1)

Uh oh!

github-actions bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

CLAassistant commented Oct 31, 2025

Uh oh!

github-actions bot commented Oct 31, 2025

Uh oh!

codeflash-ai bot commented Nov 1, 2025

⚡️ Codeflash found optimizations for this PR

📄 22% (0.22x) speedup for InjectPerfOnly.find_and_update_line_node in codeflash/code_utils/instrument_existing_tests.py

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

⚡️ Speed up method InjectPerfOnly.find_and_update_line_node by 22% in PR #867 (inspect-signature-issue) #870

Uh oh!

Uh oh!

codeflash-ai bot commented Nov 1, 2025

Uh oh!

codeflash-ai bot commented Nov 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aseembits93 commented Oct 31, 2025 •

edited by github-actions bot

Loading

github-actions bot commented Oct 31, 2025 •

edited

Loading

(Review updated until commit `ffff5f1`)

github-actions bot commented Oct 31, 2025 •

edited

Loading

📄 22% (0.22x) speedup for `InjectPerfOnly.find_and_update_line_node` in `codeflash/code_utils/instrument_existing_tests.py`

⚡️ Speed up method `InjectPerfOnly.find_and_update_line_node` by 22% in PR #867 (`inspect-signature-issue`) #870