Skip to content

Conversation

@rolandtannous
Copy link
Collaborator

PR Description:

This resolves PR #2334

Problem

In the current code, the files variable is created as a generator expression:

files = (os.path.split(x)[-1] for x in files)

Generators can only be consumed once. After iteration, they become empty. While the current code works fine (since sum() only consumes the generator once), this creates a fragile situation that could lead to bugs if future modifications use the files variable multiple times.

Example of potential issue

# This works fine
if sum(x == "adapter_config.json" or x == "config.json" for x in files) >= 2:
    both_exist = True

# But if someone later adds this, it would fail silently:
print("Files found:", list(files))  # Would print: [] (empty!)

Solution

Convert the generator expression to a list immediately:

files = list(os.path.split(x)[-1] for x in files)

This change:

  • Maintains current functionality (no behavior changes)
  • Makes the code more robust against future modifications
  • Prevents potential silent bugs
  • Has negligible performance impact (the file list is typically small)

Testing

The change maintains identical behavior to the current implementation but makes the code more maintainable and less error-prone.

… files variable is used multiple times in future modifications.
@danielhanchen danielhanchen merged commit b7fdfef into unslothai:main Aug 18, 2025
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.

2 participants