refactor(core): remove dead branches and codify a no-dead-code rule - #354
Conversation
The coverage push surfaced arms that can never run: parseParallel's undefined case (its one call site always slices a string), the job-level bootstrap fallback behind an optional resolver that is never absent (the parameter is now required), and caseList's wildcard arm that memberTest answers before ever calling it (the parameter type now excludes it). pathForField's doc claimed a lowercase field name reduces to the provider default when the match is deliberately case-sensitive to protect camelCase word boundaries — the doc now says what the code does. The distinction the new rule draws: narrowing the type system forces is not dead code — the no-as/no-non-null style demands it — but a branch still unreachable once types are as tight as the call sites allow gets deleted, never covered for the gate's sake. Recorded as coding guideline 10 in AGENTS.md, mirrored in CONTRIBUTING.md and the PR checklist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk
|
🤖 Zuke AI review 🔎 security review —
|
|
🤖 Zuke AI review 🔎 generic review —
|
| Finding | Reason |
|---|---|
| Removed bootstrap fallback now relies entirely on resolver presence | The change removes the optional parameter and the fallback only because jobBootstrap is now only called from the resolver-present path, and the existing comment plus the new required pins: CiPinResolver type make the missing-resolver failure path unreachable in the shown code. |
The patch is mostly documentation and type-tightening; the only code change worth flagging is a narrowly scoped API assumption that removes a fallback path and could surface as a hard failure if the surrounding invariant ever regresses.
A workflow re-run reuses the original event payload, so the lint kept judging the pre-edit description. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk
|
🤖 Zuke AI review 🔎 security review —
|
|
🤖 Zuke AI review 🔎 generic review —
|
| Finding | Reason |
|---|---|
| Removed default bootstrap fallback can now surface an undefined action if resolver returns nothing | jobBootstrap now requires a CiPinResolver and the only remaining pins(ZUKE_ACTION) call is guarded by the comment’s stated resolver-present branch, so there is no reachable path in the shown code where pins is absent or the action becomes undefined. |
The patch is mostly documentation and type-tightening, with one small API change that appears internally consistent but slightly reduces fallback robustness.
2 participants
What & why
The coverage push (#352) surfaced branches that can never execute. This PR removes the genuinely dead ones and writes the policy down so the distinction stays sharp.
Dead code removed — by making the impossible state unrepresentable, not by deleting guards:
cli.tsparseParallelnow takesstring: its one call site always passes the sliced text after the parallel flag's equals sign, so theundefinedarm could never run.ci.tsjobBootstrapnow requires itsCiPinResolver: it is only called from the resolver-present branch ofwithPins, so both the optional call and theDEFAULT_ZUKE_ACTIONfallback were unreachable there. The pipeline-level fallback, which is reachable, stays.ci_schedule.tscaseListnow excludes the wildcard from its parameter type:memberTestanswers the wildcard before ever building a membership list, so that arm was dead.ci.tspathForFieldJSDoc claimed a lowercase field name reduces to the provider default — the match is deliberately case-sensitive so camelCase word boundaries are never split, and the doc now says what the code does.Deliberately NOT removed: runtime-unreachable narrowing that the type system forces — a nullish-coalescing fallback after a map lookup, an instanceof-Error check inside a catch, an undefined check after an array find. Deleting those would require the banned non-null assertion or an
ascast; they are the cost of guideline 1, not dead code.The rule, codified in three places: coding guideline 10 in
AGENTS.md— the full statement, including that an uncoverable branch is the signal to delete the branch, not to feed the coverage gate — mirrored as coding standard 6 inCONTRIBUTING.md, and a new checklist line in the PR template.Related issues
Follow-up to #352 — acts on the dead-code observations from its coverage review.
Checklist
type(scope): summary).deno task cipasses locally (lint, fmt, type-check, tests, spell).AGENTS.md,CONTRIBUTING.md, PR template, the correctedpathForFieldJSDoc)../zuke apiDocs— not applicable, all changed functions are module-internal.any, noascasts or!non-null assertions insrc/(narrow with type guards instead).🤖 Generated with Claude Code
https://claude.ai/code/session_019As9A3ugixLQiAvcat1ZKk