refactor(opencode): split json-schema normalize into named rewrite steps

[dxz05]

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue:

Closes #110

Full path to the refactored file:

packages/opencode/src/tool/json-schema.ts

What do you think this file does?

It converts a tool's Effect Schema into the JSON Schema document that is sent to LLM providers as the tool's parameters. fromSchema asks Effect for a draft-2020-12 document, then normalize rewrites it into the "wire shape" providers accept (drops additionalProperties: true, strips null from optional fields, collapses trivial anyOf/allOf unions, bounds integers to the safe range) and finally inlines local $defs references.

What is the scope of your refactoring within that file?

Only normalize() (previously lines 28–79). Its logic is now split into normalize (happy path) plus four helpers directly below it: normalizeChildren, collapseAnyOf, flattenAllOf, boundIntegerRange, and a small isNonFiniteEnum predicate. fromSchema, fromTool, the $ref inlining and the existing predicates are untouched.

Which Qlty‑reported issue did you address?

Function with high complexity (count = 29): normalize at packages/opencode/src/tool/json-schema.ts:28 (the same function was also flagged Function with many returns (count = 9)). File total complexity BEFORE: 60.

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?

normalize was one 50-line function that did six unrelated rewrites in a single pass with 9 exit points, a for loop that mutated required bookkeeping, and several deeply nested if chains. To know which rule produced a given output you had to trace the whole function, and adding a new rewrite meant threading another branch into the same block.

What changes did you make to resolve the issue?

• normalize now reads as the happy path: recurse into arrays/non-objects, normalize children, drop additionalProperties: true, apply the first matching anyOf/allOf rewrite (and re-normalize its result), else bound integers.
• normalizeChildren owns the "which properties are optional" bookkeeping (required set + stripNull per property) using Object.fromEntries/map instead of a mutable loop.
• collapseAnyOf holds the three union simplifications (null stripping, non-finite-number enum collapse, empty struct union, single-member union) and returns either a replacement schema or undefined.
• flattenAllOf and boundIntegerRange hold the remaining two rules.
• isNonFiniteEnum replaces a repeated inline isRecord(x) && Array.isArray(x.enum) && x.enum.every(isNonFiniteNumber) expression.

Behavior is unchanged: every existing snapshot in test/tool/parameters.test.ts still matches, and I diffed ToolJsonSchema.fromSchema output for representative schemas (optional/nullable fields, Schema.Number, empty structs, allOf constraints, integers, nested arrays) between main and this branch before deleting the probe.

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

Each rewrite rule now has a name, a single responsibility and at most three exits, so a reader can find "why did null disappear" by opening collapseAnyOf instead of the whole function, and a new rule is one more helper in the ?? chain. Qlty no longer reports any function-level smell in the file and total complexity drops 60 → 54. I considered (a) a table of [predicate, rewrite] pairs iterated in order, which was more abstract than the five concrete rules warrant, and (b) only extracting the anyOf block, which would have left normalize at ~7 returns; the current split matches the repo's "happy path + helpers below" style guide.

3. Validation

How did you validate that the change is correct?

• bun test test/tool/parameters.test.ts test/tool/registry.test.ts from packages/opencode: 79 pass, 0 fail. The existing 16 JSON Schema snapshots for every built-in tool exercise normalize end to end; these are the tests that would break if any rewrite changed output for real tool schemas.
• Added six focused tests in test/tool/parameters.test.ts → describe("JSON Schema (wire shape)"), one per extracted rule: null stripping only on optional fields, non-finite number union collapse, empty-struct union collapse, allOf flattening, explicit integer maximum preserved, and pass-through of non-object schema values. Together with the snapshots these cover every line of normalize and its helpers (see coverage below), so a reviewer can trust that both the common tool schemas and each individual rule still produce the same wire shape.
• bun typecheck and bun lint from packages/opencode pass; bun prettier --check passes.
• The two test files are also run as an explicit Run tool JSON Schema tests step in .github/workflows/test.yml (the existing bun turbo test step already runs them but uses --only-failures, which hides passing test names in the CI log).

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

bun test test/tool/parameters.test.ts --coverage --coverage-reporter=lcov + genhtml: json-schema.ts 95.7% lines; every line of normalize and the new helpers (28–87) is hit. The only uncovered lines are fromTool (covered by registry.test.ts) and two $ref edge branches unrelated to this change.

Attach a screenshot showing the tests that cover the change passing during CI

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