refactor(schema): drop the declarations nothing has ever read - #562
Open
EtienneLescot wants to merge 2 commits into
Open
refactor(schema): drop the declarations nothing has ever read#562EtienneLescot wants to merge 2 commits into
EtienneLescot wants to merge 2 commits into
Conversation
`addAssetInputSchema.autoTranscribe` was declared with a `true` default and consumed by nobody: one occurrence in the whole tree, its own declaration. That is worse than clutter. It reads like the switch that decides whether an imported asset gets transcribed, so the next person to work on transcription finds it, believes the decision already has a home, and wires their logic to a flag no code path consults. It surfaced during the design of #560 for exactly that reason. The field goes; the schema keeps its shape otherwise. Nothing type-checks against it — `document-service.ts` declares its own `AddAssetInput` interface and uses that one, so the inferred type this schema exports was not in play either.
`addAssetInputSchema` and `chatInputSchema` had no reference anywhere outside their own declaration and their own `z.infer` line — no parse site, no import, no namespace import of the module. They named an IPC contract that no boundary actually validates against. `addAssetInputSchema` was the worse of the two: the `AddAssetInput` it exported is shadowed by a same-named interface declared in `document-service.ts`, and that interface is the one every caller uses. Two types, one name, and the one that looked canonical was inert. `createProjectInputSchema` STAYS. It reads as dead by the same grep — no use outside `schema/index.ts` — but its inferred `CreateProjectInput` types `createEmptyDocument`, which has 50 call sites. Same file, one line down; the reference is easy to miss and expensive to remove.
EtienneLescot
changed the title
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change removes Changes
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized cleanup removes unused schema declarations without changing runtime behavior or validated application contracts. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three removals in
schema/index.ts, all of them declarations nothing ever read.addAssetInputSchema.autoTranscribe— declared with atruedefault, one occurrence in the whole tree: its own line. Worse than clutter, because it reads like the switch that decides whether an imported asset gets transcribed. The next person working on transcription finds it, believes the decision already has a home, and wires their logic to a flag no code path consults. It surfaced while designing #560, which needs somewhere to express "transcribe only what can carry speech" — and this looked like it.addAssetInputSchemaitself, and theAddAssetInputit exported. No parse site, no import, no namespace import of the module. It named an IPC contract that no boundary validates against. It was also actively confusing:document-service.tsdeclares a same-namedAddAssetInputinterface (line 38) and that is the one every caller uses. Two types, one name, and the one that looked canonical was inert.chatInputSchemaand itsChatInput— same story, no shadowing twist.What is NOT removed, and why it looked removable
createProjectInputSchemastays. It reads as dead by exactly the same grep — zero references outsideschema/index.ts— but its inferredCreateProjectInputtypescreateEmptyDocument, which has 50 call sites. The reference is one line below the type export, inside the same file, and is easy to miss.Worth stating rather than quietly keeping: "no references outside its own module" is not a liveness test for a type that a function in that module consumes.
Related issue
Refs #560 — found while pinning down where automatic transcription should be decided.
Type of change
Release impact
Desktop impact
Screenshots / video
None — no user-visible change.
Testing
npm run test— 2236 passed, 4 skipped, 0 failed (187 files)npx tsc --noEmitandnpx tsc -p tsconfig.test.json --noEmit— cleannpm run lint(Biome) — clean (15 pre-existing warnings in untouched files)No test asserted on any of the removed declarations, so none needed changing. The typecheck is the real proof here: had any of the three been reachable, both
tscpasses would say so.🤖 Generated with Claude Code
Summary by CodeRabbit