Add requests timeout codemod is adding timeout kwarg to other methods

[LucasFaudman]

Example of Issue

The following code is part of a pull request made by pixie to a setup script in one of my projects after a correctly triggering an AddRequestsTimeouts Codemod.

versions = requests.get(versions_url, timeout=60).json(timeout=60)

This clearly would result in a TypeError since "timeout" is not a valid kwarg for JSONDecoder.__init__.

Potential Cause

I assume that this issue is caused by the OpenAI function call outputs that are used to create code edits not being properly linted on the backend before the resulting code is included in a pull request.

Potential Solution

A possible solution could be adding a valid function and argument parsing class to file_parsers that gathers the valid function names, argument names and types by checking the args in function definitions, typeing info and calls to kwargs.get/pop in each function for each file in the repo.

Then each function call in the AI written code could be linted for valid arguments+types and an error could be raised that triggers a subsequent OpenAI call requesting a revision and providing the valid args+types to the model to insure code is valid before including it a pull request.

As a backup in cases where multiple revisions produce invalid code, the valid args for functions in a given code snippet could be specified as an enum in the OpenAI function/tool definitions used to edit code.

Alternatively an Assistants API instance with the code_interpreter tool enabled could be prompted to test code snippets to prevent this issue but this would of course increase costs.

Good luck with this project absolutely awesome idea!

[drdavella]

Hi @LucasFaudman thank you so much for your feedback, it's very much appreciated!

There are actually no LLMs involved in Codemodder today (although we intend to explore that possibility for future codemods). What that means is that this is being produced by some broken logic in our code. The transformation is implemented here:

codemodder-python/src/core_codemods/add_requests_timeouts.py

Lines 14 to 22 in cb5e23a

	class TransformAddRequestsTimeouts(LibcstResultTransformer):
	# Sets an arbitrary default timeout for all requests
	DEFAULT_TIMEOUT = 60
	change_description = "Add timeout to `requests` call"
	def on_result_found(self, original_node, updated_node):
	del original_node
	return self.add_arg_to_call(updated_node, "timeout", self.DEFAULT_TIMEOUT)

This likely indicates a bug within our helper code and as a result it potentially affects multiple codemods. Your suggestion about additional post-validation is a good one but it requires us to enumerate some more (and better) test cases. We will iterate on this.

Again we really appreciate the feedback and will work to address this and release a fix ASAP. Please keep them coming!

[clavedeluna]

The add-requests-timeouts codemod relies on pattern-matching using semgrep. I played around with the codemod's pattern and discovered something surprising (maybe a bug) with the semgrep results.
Here's a semgrep playground example. The issue is that when using $CALL and metavariable, semgrep is matching the entire code line, including .json(). However, simplifying the rule without using metavariable and $CALL, the semgrep result does not include .json() in the match.

I don't know if this is intended semgrep behavior so perhaps we should report it to them? Also, I will survey other codemods that rely on semgrep's metavariable pattern to see if they have this issue.
For a quick fix here, changing the semgrep rule for this codemod to not use metavariable and just duplicate the pattern for each requests.$CALL we care about should solve this issue. Other solutions could be not relying on semgrep for this codemod.

[drdavella]

@clavedeluna thanks this is awesome. I agree that this feels like a Semgrep bug to me, especially since we are able to get the expected behavior by making the pattern explicit without metavariable patterns. It's unfortunate but at least we can get it to work.

Making a change to the Semgrep pattern for this particular codemod seems like the right fix to me.

[clavedeluna]

Reported semgrep/semgrep#9756

[LucasFaudman]

@drdavella @clavedeluna Thank you guys for checking this out. I thought the issue was similar a to param hallucination issue I ran into when implementing an OpenAI function recently but I'm thrilled it helped identify an even deeper issue in your dependencies. This was a very interesting read and i'm definitely looking forward to seeing your LLM powered codemods in the future.