fix: add max_retries param to ModuleLLM to bound retry attempts#275
fix: add max_retries param to ModuleLLM to bound retry attempts#275
Conversation
Previously generate() and agenerate() used unbounded exponential backoff with no stop condition, causing simulations to appear hung indefinitely on persistent API errors (issue mesa#266). Add max_retries: int = 5 parameter to ModuleLLM.__init__. Refactor generate() from class-level @Retry decorator to inline Retrying() so self.max_retries is accessible at call time. Update agenerate() to pass stop_after_attempt(self.max_retries) to AsyncRetrying. Update existing tests to use max_retries=1 (bypassing decorator __wrapped__ which no longer exists). Add test verifying generate() raises after exactly max_retries attempts.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing in favour of #267 which was opened earlier and covers the same fix. Sorry for the duplicate. |
Pre-PR Checklist
Summary
ModuleLLM.generate()andagenerate()retried transient errors (APIConnectionError,Timeout,RateLimitError) with exponential backoff but no upper bound, causing simulations to appear hung indefinitely on persistent API failures.Bug / Issue
Closes #266.
The
@retrydecorator inmodule_llm.pyusedwait_exponentialwith nostopcondition. A persistentTimeoutorRateLimitErrorwould retry forever, blocking the simulation with no indication of what was happening.Implementation
max_retries: int = 5parameter toModuleLLM.__init__generate()from a class-level@retrydecorator to inlineRetrying()soself.max_retriesis accessible at call timestop=stop_after_attempt(self.max_retries)to bothgenerate()andagenerate()max_retriesTesting
generate.__wrapped__(which no longer exists since the decorator was removed); replaced withmax_retries=1to bypass retrytest_generate_stops_after_max_retriesverifying thatgenerate()raises after exactlymax_retriesattemptspytest tests/ --ignore=tests/test_integration)blackandruffcleanAdditional Notes
The
agenerate()path already used inlineAsyncRetrying()so the change there is a single line addingstop=stop_after_attempt(self.max_retries).