Skip to content

refactor(opencode): cut returns in packages/opencode/src/acp/content.ts - #104

Open
JakePJakeP wants to merge 1 commit into
CMU-313:mainfrom
JakePJakeP:acp-content-pr
Open

refactor(opencode): cut returns in packages/opencode/src/acp/content.ts#104
JakePJakeP wants to merge 1 commit into
CMU-313:mainfrom
JakePJakeP:acp-content-pr

Conversation

@JakePJakeP

Copy link
Copy Markdown

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue:
#77

Full path to the refactored file:
packages/opencode/src/acp/content.ts

What do you think this file does?
It converts Agent Client Protocol (ACP) content blocks into OpenCode prompt parts, and the reverse for replay: text, images, resource links, and embedded resources become session text/file parts (and later ACP content chunks).

What is the scope of your refactoring within that file?
Only contentBlockToParts and helpers extracted below it: textBlockToParts, imageBlockToParts, imageBlockUrl, resourceLinkBlockToParts, resourceBlockToParts, resourceTextToParts, resourceBlobToParts, plus the typed handlers map. partToContentChunks and the rest of the file were not changed.

Which Qlty‑reported issue did you address?
Function with many returns (count = 11) on contentBlockToParts (line 30). The same function also had Function with high complexity (count = 33); the file also showed High total complexity (count = 61).

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
Eleven return paths in one switch (text, several image URL shapes, resource_link, nested resource text/blob, default) made the conversion hard to scan and risky to change, because a mime or URL tweak could easily break a sibling branch.

What changes did you make to resolve the issue?
I kept the public API contentBlockToParts(block): PromptPart[] and copied each case into helpers below it, then collapsed the export to one dispatch: return handlers[block.type]?.(block as never) ?? []. URL handling, mime fallbacks, Windows slash normalize, and data-URL behavior were left as-is.

How do your changes improve maintainability? Did you consider alternatives?
Each block type is now a small named function, so a future ACP type or image-URL rule can be edited without rereading the whole converter. I considered leaving a 5-return switch, but Qlty’s bar was “many returns,” so I used the typed handler map instead of any or a still-smelly switch.

3. Validation

How did you validate that the change is correct?
Locally: bun lint from the repo root; bun test test/acp/content.test.ts from packages/opencode (19 pass, including new cases that hit the extracted helpers); coverage via bun test --coverage --coverage-dir=./coverage test/acp/content.test.ts (src/acp/content.ts 100% functions / 83.84% lines, with remaining misses in unchanged replay helpers); and qlty smells --no-snippets packages/opencode/src/acp/content.ts afterward reported no smells. CI on this branch adds an acp content job that runs only that test file.

Attach a screenshot of the test coverage showing the lines were executed by the tests.
Screenshot 2026-09-07 at 10 38 43 AM
Screenshot 2026-09-07 at 10 38 59 AM

Attach a screenshot showing the tests that cover the change passing during CI.
Screenshot 2026-09-07 at 11 27 28 AM

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-07 at 10 50 38 AM
After Changes:
Screenshot 2026-09-07 at 10 51 06 AM

…count

Co-authored-by: Cursor <cursoragent@cursor.com>
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