Simplify completePatchContents in packages/session-ui/src/components/session-diff.ts

[anish-jai]

Refactors completePatchContents to remove the Qlty "Function with high complexity (count = 37)" smell with no behavior change.

1. Issue

Link to the associated GitHub issue:
Closes #181

Full path to the refactored file:
packages/session-ui/src/components/session-diff.ts

What do you think this file does?
It normalizes the different diff shapes the session UI receives (snapshot, VCS, tool patch, legacy before/after) into one ViewDiff for the review components. For unified patches it decides whether the patch covers the whole file (render full before/after text) or only part of it (keep a partial hunk view).

What is the scope of your refactoring within that file?
completePatchContents (the target) and patchInput, which duplicated the same try { parsePatch(patch)[0] } catch block; that parse now lives in a small shared parseFirstPatch helper. No exported function or signature changed.

Which Qlty-reported issue did you address?
Function with high complexity (count = 37): completePatchContents at line 74. Fixing it also cleared Function with many returns (count = 7) and the file-level High total complexity (count = 53); Qlty now reports nothing for this file.

2. Refactoring

How did the specific issue you chose impact the codebase's maintainability?
The function mixed four validation rules, a try/catch, and a stateful loop that remembered the previous line's marker and reached back to mutate earlier entries on a \ No newline at end of file marker. Following the complete-vs-partial decision meant tracing seven exit points plus that hidden state, and the parse block was copy-pasted into patchInput.

What changes did you make to resolve the issue?
Replaced the stateful loop with a patchSides lookup table (marker to side) and a single map/filter pass that looks one line ahead for the no-newline marker instead of mutating earlier entries. Moved the duplicated parse into parseFirstPatch, used by both functions, and flattened the guards from seven exits to five.

How do your changes improve maintainability? Did you consider alternatives?
Each rule is now one readable line, the data flow is a pure map/filter, and the marker semantics live in one table instead of four if branches. I considered splitting into several single-use helpers to push the number lower, but the repo's AGENTS.md asks not to extract single-use helpers preemptively, so I only extracted the one piece that is genuinely reused.

3. Validation

How did you validate that the change is correct?

• The existing session-diff.test.ts (10 tests) exercises completePatchContents and patchInput through the public resolveFileDiff/normalize API and still passes unchanged.
• I added 3 tests for previously unexercised branches (single hunk starting after line 1, no-newline marker after a context line, blank line inside a hunk). They also pass against the original code, so they pin down existing behavior.
• bun test --coverage reports 100% functions and 100% lines for session-diff.ts; bun run typecheck passes; bun lint findings are identical before and after (2 pre-existing errors in unrelated files, 0 in the touched files).
• packages/session-ui is already wired into bun turbo test, so these tests run in the test / unit job on this PR.

Attach a screenshot of the test coverage showing the lines were executed by the tests.
cd packages/session-ui && bun test --coverage --coverage-dir=./coverage: session-diff.ts 100% functions, 100% lines, 86 tests pass.

Attach a screenshot showing the tests that cover the change passing during CI
All 3 checks green on the final commit b84504e; the test / unit job runs session-diff.test.ts.

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.ts> showing fewer reported issues after the changes.
BEFORE, on main:

AFTER, on refactor-session-diff, same command and file (nothing reported):

Additional local evidence
cd packages/session-ui && bun test: 86 pass, 0 fail. The red parseLineType lines are console output from the @pierre/diffs fallback parser during the blank-line test, not a failure.

bunx oxlint on the two changed files: 0 warnings, 0 errors. bun lint repo-wide: 698 warnings, 2 errors, identical on untouched main (in packages/web and prompt-input/index.tsx, neither touched here).

Linked issue #181 with bot assignment: