Refactor (packages/codemode/src/tool-schema.ts:43): Function with high complexity (count = 18): hasUnresolvedRef

[vaishnavipalas]

P1B: Starter Task: Refactoring PR

1. Issue

Link to the associated GitHub issue: #68

Full path to the refactored file: packages/codemode/src/tool-schema.ts

What do you think this file does?
I think tool-schema.ts is responsible for converting JSON Schema (and Effect Schema) definitions into TypeScript-style type signatures. For example, it renders something like tools.orders.lookup(input: {...}): Promise<{...}> so that a tool catalog can display readable type signatures. To do this, the file has to handle $ref/$defs resolution, unions, and nested properties, and it needs to safely detect cycles or missing references so that it can fall back to "unknown" instead of crashing or looping forever.

What is the scope of your refactoring within that file?
I split the hasUnresolvedRef function into three separate functions:

• resolveRef (new) — handles matching and looking up a $ref, and checking whether it has already been seen (to detect cycles)
• collectChildSchemas (new) — gathers all the child schemas from anyOf, oneOf, allOf, properties, items, and additionalProperties
• hasUnresolvedRef (retained) — now just orchestrates the two functions above, along with the existing visited-based check for identity cycles

I didn't touch any other functions in the file, and the behavior of the code is unchanged, only its internal structure changed.

Which Qlty‑reported issue did you address?
"Function with high complexity," count = 18, in hasUnresolvedRef (line 43).

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
At a complexity score of 18, hasUnresolvedRef packed ref-resolution logic, cycle detection, and child-schema traversal all into one function with many branch points — nested ternaries, a triple || condition, optional chaining, and inline array-building with conditionals. This made it hard to reason about which condition caused an "unresolved" verdict, hard to unit-test any single piece in isolation, and risky to modify, since a small change to one concern (like how child schemas are gathered) could silently affect the unrelated ref-resolution logic sitting in the same function body.

What changes did you make to resolve the issue?
I split the function into three:

• resolveRef — handles $ref parsing, $defs lookup, and the cycle check
• collectChildSchemas — gathers the child schemas to recurse into
• hasUnresolvedRef — now orchestrates the two functions above, plus the existing visited-based identity guarding

The algorithm itself didn't change; I just distributed the same logic across named, single-purpose functions.

How do your changes improve maintainability? Did you consider alternatives?
Each function now has a low, independent complexity and can be read and tested on its own. This is actually what allowed me to identify a genuine coverage gap in the ref-propagation path that the original function's tests didn't exercise. I considered simply suppressing the complexity warning, but that wouldn't have addressed the underlying readability issue. I also considered a more substantial restructuring, such as a general-purpose schema-visitor, but concluded that extracting the two responsibilities already implicit in the original function was the smallest change that resolved the issue without altering the algorithm or its call sites.

3. Validation

How did you validate that the change is correct?
I added five test cases in test/signature.test.ts targeting the specific branches of the original hasUnresolvedRef logic: a $ref naming a missing $defs entry, a self-referencing cycle, a mutually recursive cycle across two $defs entries, a cycle reached only through anyOf/items rather than properties, and a resolved $ref whose target is itself unresolved (reached only through allOf). I then ran bun test to confirm that every line inside these new functions was actually executed. I also confirmed that the full existing test suite still passes with no regressions.

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

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.