Skip to content

Refactor (script/beta.ts): Function with high complexity (count = 36): main - #66

Open
horsepaperfish wants to merge 2 commits into
CMU-313:mainfrom
horsepaperfish:fix-beta-complexity
Open

Refactor (script/beta.ts): Function with high complexity (count = 36): main#66
horsepaperfish wants to merge 2 commits into
CMU-313:mainfrom
horsepaperfish:fix-beta-complexity

Conversation

@horsepaperfish

Copy link
Copy Markdown

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue: #58

Full path to the refactored file: /script/beta.ts

What do you think this file does?
Beta.ts collects all open PRs labeled beta, merges them one at a time onto a new branch, delegates any merge conflicts to opencode, and force-pushes the result after a typecheck-and-build smoke check.

What is the scope of your refactoring within that file?
The refactoring is limited to main() and the functions extracted from it. I pulled the PR fetching, branch reset, per-PR merge loop, summary output, and beta sync check out of main() into fetchPRs, reset, merge, commit, apply, run, report, and syncState, and replaced the repeated inline failure handling with an Outcome union and a fail() constructor. This helped to reduce its cyclomatic complexity below the threshold.

Which Qlty‑reported issue did you address?
script/beta.ts:219: Function with high complexity (count = 36): main()

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The main() function did everything inline: fetching PRs, resetting the branch, the per-PR merge loop, all six failure paths within that loop, the summary output, and the beta sync check. Moreover, every failure case repeated the same four-line pattern (log, push to failed, comment on the PR, continue) so adding a new failure mode meant duplicating that block again and getting all four steps right.

What changes did you make to resolve the issue?
I extracted the complexities into sub-functions: fetchPRs, reset, merge, commit, apply, run, report, and syncState. The repeated failure handling became an Outcome discriminated union (applied / skipped / failed) with a fail() constructor, so each failure path returns a value instead of duplicating the same four-line pattern.

How do your changes improve maintainability? Did you consider alternatives?
The main() went from 36 decision points to 6, with no extracted function above 8. The Outcome union makes the reason/comment distinction explicit in the type rather than implicit in duplicated code, so adding a new failure mode is a single return fail(...).
Some alternatives: I considered only extracting the duplicated sync check and leaving the loop in main, but that wouldn't have cleared the threshold of reducing the complexity by a marginal amount or removed the repeated failure blocks. I also considered throwing a custom error type instead of returning a value, but rejected it since these are expected outcomes the loop recovers from, not exceptions.

3. Validation

How did you validate that the change is correct?
I Added script/beta.test.ts with 13 unit tests. Each of the unit tests lead to each Outcome variant through a contract apply and assert the resulting applied/failed arrays. I then confirmed these tests passed locally with bun coverage and bun lint unchanged from main, and qlty smells no longer reports the issue.

Attach a screenshot of the test coverage showing the lines were executed by the tests.
Test coverage of beta.test.ts (unit tests I wrote):
Screenshot 2026-09-05 at 6 03 15 PM
Test coverage of bun test coverage:
Screenshot 2026-09-05 at 6 03 28 PM

Attach a screenshot showing the tests that cover the change passing during CI
Screenshot 2026-09-05 at 6 41 39 PM

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.
Before changes:
Screenshot 2026-09-05 at 4 27 00 PM
After changes:
Screenshot 2026-09-05 at 4 28 56 PM

Screenshots of bun lint & bun test passing locally
Bun lint On main branch:
Screenshot 2026-09-05 at 5 36 12 PM
Bun lint On fix-beta-complexity branch:
Screenshot 2026-09-05 at 5 36 41 PM
Bun test:
Screenshot 2026-09-05 at 5 38 01 PM

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