Skip to content

Refactor (packages/opencode/src/session/retry.ts): Function with high complexity - #38

Open
yswcyswc wants to merge 2 commits into
CMU-313:mainfrom
yswcyswc:simplify-retry-delay
Open

Refactor (packages/opencode/src/session/retry.ts): Function with high complexity#38
yswcyswc wants to merge 2 commits into
CMU-313:mainfrom
yswcyswc:simplify-retry-delay

Conversation

@yswcyswc

@yswcyswc yswcyswc commented Sep 3, 2026

Copy link
Copy Markdown

P1B: Starter Task: Refactoring PR

Use this pull request template to briefly answer the questions below in one to two sentences each.
Feel free to delete this text at the top after filling out the template.

1. Issue

Link to the associated GitHub issue: #35

Full path to the refactored file: packages/opencode/src/session/retry.ts

What do you think this file does?
(Your answer does not have to be 100% correct; give a reasonable, evidence‑based guess.)
I think this file defines the retry policy for failed session and provider requests. It determines whether an error is retryable, calculates the waiting time from provider response headers or exponential backoff, and updates the retry status and attempt count before the next request.

What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
I refactored the delay() function and extracted its retry-header parsing and exponential-backoff calculations into the retryAfter() and backoff() helper functions. I also removed an unused import identified by the linter because there was a warning there.

Which Qlty‑reported issue did you address?
(Name the rule/metric and include the BEFORE value; e.g., “Cognitive Complexity 18 in render()”.)
I addressed the Qlty issue Function with high complexity (count = 22): delay, originally reported at line 44.

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The original delay() combined header detection, parsing of multiple header formats and timer limits through several conditional branches. This made its control flow difficult to understand and modify safely.

What changes did you make to resolve the issue?
I changed delay() into a short coordinator that selects either a provider-specified retry delay or exponential backoff. I moved those calculations into the focused retryAfter() and backoff() helper functions and used early returns to reduce nesting.

How do your changes improve maintainability? Did you consider alternatives?

The new helper functions separate header parsing from exponential-backoff calculation, giving each function a single, clearly named responsibility. This reduces nesting, makes each calculation easier to test and modify independently, and keeps delay() easy to follow and I considered only flattening the original conditionals with early returns, but extraction created clearer boundaries between the two calculations.

3. Validation

How did you validate that the change is correct?
I ran the focused session.retry.delay tests covering missing headers, retry-after-ms, numeric and HTTP-date retry-after values, invalid hints, exponential backoff, and timer caps. I also ran focused linting, type checking, coverage, Qlty, and GitHub CI to confirm that behavior was preserved and the targeted delay() complexity smell was removed.

Attach a screenshot of the test coverage showing the lines were executed by the tests.

Line-level HTML coverage for retry.ts; the refactored delay(), backoff(), and retryAfter() lines are shown as executed:
coverage

Bun coverage summary showing 100% function coverage and 98.55% line coverage for src/session/retry.ts; the only uncovered lines are 162–163, outside the refactored code:
coverage-retry

Attach a screenshot showing the tests that cover the change passing during CI
GitHub Actions passed all three checks for the latest commit, including unit tests, smoke tests, and type checking:
ci-pass

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.
Before refactoring, Qlty reported Function with high complexity (count = 22): delay:
before-qlty-retry
After refactoring, the complexity smell for delay() is no longer reported, and the file’s total complexity decreased from 84 to 70:
after-qlty-retry

Additional local validation: bun lint and bun test passing locally
Local lint completed with 0 warnings and 0 errors for the refactored file:
local-lint-retry
The targeted retry test suite passed all 52 tests with 0 failures:
local-test
The focused session.retry.delay tests passed with 0 failures:
focused

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.

1 participant