feat: add max_retries parameter to Writer client initialization - AB-931

[as-flow]

Summary by CodeRabbit

• Improvements
  • Enhanced request resilience with automatic retry mechanism for failed requests, improving reliability during temporary network issues.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

A single configuration change has been made to the Writer HTTP client initialization in the AI module. The max_retries parameter has been set to 10 in the Writer constructor call, increasing the default retry count for failed requests.

Changes

Cohort / File(s)	Summary
HTTP Client Configuration src/writer/ai/__init__.py	Added max_retries=10 parameter to the Writer(...) constructor to increase retry attempts for failed HTTP requests from the default value.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 With whiskers twitched and hope held high,
Ten retries now shall never die,
When networks fail, we bounce right back,
No 429 shall block our track,
The Writer hops with steadfast grace! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR only adds max_retries parameter but does not implement exponential backoff retry logic as required by AB-931. The implementation is incomplete.	Implement exponential backoff retry logic in addition to the max_retries parameter to fully address AB-931 requirements.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: adding a max_retries parameter to Writer client initialization.
Out of Scope Changes check	✅ Passed	All changes in the PR are directly related to the max_retries initialization objective and stay within scope of addressing retry behavior.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[jackcviers]

Need to also adjust the MAX_RETRY_DELAY constant, which is capped at 8 seconds. Circuit breaker cooldowns last at minimum 30 seconds, so this alone won't help.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/writer/ai/__init__.py (1)

320-325: Consider making max_retries configurable rather than hardcoded.

Issue AB-931 requests to "make it configurable via a parameter," but the current implementation hardcodes max_retries=10. While 10 is a reasonable default, consider allowing configuration to support different use cases.

Potential approach to make max_retries configurable

You could add an optional parameter to WriterAIManager.__init__ and acquire_client:

 class WriterAIManager:
-    def __init__(self, token: Optional[str] = None):
+    def __init__(self, token: Optional[str] = None, max_retries: int = 10):
         """
         Initializes a WriterAIManager instance.
 
 
         :param token: Optional; the default token for API authentication used
         if WRITER_API_KEY environment variable is not set up.
+        :param max_retries: Maximum number of retry attempts for API requests.
         :raises RuntimeError: If an API key was not provided to initialize
         SDK client properly.
         """
         self.token = token
+        self.max_retries = max_retries

Then use instance.max_retries instead of the hardcoded value:

             client = Writer(
                 api_key=instance.token,
                 default_headers=custom_headers,
                 http_client=custom_httpx_client,
-                max_retries=10,
+                max_retries=instance.max_retries,
                 )

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a550ac2 and 0cb6564.

📒 Files selected for processing (1)

• src/writer/ai/__init__.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)

• GitHub Check: build (3.9)
• GitHub Check: build (3.11)
• GitHub Check: build (3.13)
• GitHub Check: tests (webkit)
• GitHub Check: tests (chromium)
• GitHub Check: tests (firefox)
• GitHub Check: build (3.12)
• GitHub Check: build (3.10)

[pullrequest]

HackerOne Code Security Review

🟢 Scan Complete: 1 Issue(s)
🟢 Validation Complete: Any Issues detected were validated by one of our engineers. None were determined to require immediate action.

Here's how the code changes were interpreted and info about the tools used for scanning.

ℹ️ Issues Detected

NOTE: These may not require action!

Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.

How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:

• Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
• Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.

File & Line

Issue

src/writer/ai/__init__.py Line 324

The code change adds a 'max_retries' parameter with a value of 10 to the Writer client initialization. This high retry count could lead to resource exhaustion or denial of service if the service being contacted is experiencing issues. When a service is down or returning errors, aggressive retry logic can exacerbate the problem by continuing to send requests, potentially causing cascading failures or resource depletion on both client and server sides.

🧰 Analysis tools

• [ ✅ ] HackerOne AI Code Analysis
• [ ✅ ] HackerOne AI Code Validation
• [ ✅ ] semgrep
• [ ✅ ] bandit

⏱️ Latest scan covered changes up to commit 0cb6564 (latest)

[pullrequest]

✅ Graham C reviewed all the included code changes and associated automation findings and determined that there were no immediately actionable security flaws. Note that they will continue to be notified of any new commits or comments and follow up as needed throughout the duration of this pull request's lifecycle.

Graham C

---

Reviewed with ❤️ by PullRequest