Skip to content

refactor: simplify retry#161

Merged
Disviel merged 6 commits intoEmaFanClub:mainfrom
Myriad-Dreamin:refactor-llm
Mar 1, 2026
Merged

refactor: simplify retry#161
Disviel merged 6 commits intoEmaFanClub:mainfrom
Myriad-Dreamin:refactor-llm

Conversation

@Myriad-Dreamin
Copy link
Copy Markdown
Contributor

@Myriad-Dreamin Myriad-Dreamin commented Feb 27, 2026

This PR refactors the retry mechanism by simplifying the implementation and reorganizing the module location. The main changes consolidate the retry logic from a decorator-based approach to a simpler function wrapper approach, while moving the retry module into the llm subdirectory to better reflect its primary usage context.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the retry mechanism by simplifying the implementation and reorganizing the module location. The main changes consolidate the retry logic from a decorator-based approach to a simpler function wrapper approach, while moving the retry module into the llm subdirectory to better reflect its primary usage context.

Changes:

  • Removed the asyncRetry decorator function and simplified wrapWithRetry to directly wrap functions without decorator indirection
  • Moved the retry module from packages/ema/src/retry.ts to packages/ema/src/llm/retry.ts
  • Updated all import paths across openai_client.ts, google_client.ts, config.ts, and agent.ts to reference the new location

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/ema/src/llm/retry.ts Removed decorator pattern, simplified wrapWithRetry to directly wrap functions, and added validation for max_retries
packages/ema/src/llm/openai_client.ts Updated import path from ../retry to ./retry
packages/ema/src/llm/google_client.ts Updated import path from ../retry to ./retry
packages/ema/src/config.ts Updated import and export paths from ./retry to ./llm/retry
packages/ema/src/agent.ts Updated import path from ./retry to ./llm/retry
Comments suppressed due to low confidence (2)

packages/ema/src/llm/retry.ts:99

  • The validation if (config.max_retries <= 0) is overly restrictive. Setting max_retries to 0 is a valid use case meaning "no retries, just one attempt". The original code would have supported this (the loop runs with attempt = 0 and throws on first failure). This validation breaks backward compatibility for users who might want to disable retries by setting max_retries: 0 while keeping enabled: true. Consider either removing this validation or changing it to if (config.max_retries < 0) to only reject negative values.
    packages/ema/src/llm/retry.ts:123
  • The error messages no longer include the function name (previously used propertyKey from the decorator). This makes debugging more difficult as it's harder to identify which function's retry failed. Consider adding an optional functionName parameter to wrapWithRetry that can be included in error messages, or document that users should provide this context in their onRetry callback if needed for debugging.

@Disviel Disviel merged commit 204f4d0 into EmaFanClub:main Mar 1, 2026
7 checks passed
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.

3 participants