chore(knip): ignore submodule + remove orphaned dead code (#110) - #115
Merged
Conversation
knip had no config, so it scanned everything and drowned ~30 bot-own
flags under the ZenPlugins submodule noise plus dozens of false-positive
"unused files" (scripts, bank-sync.ts, tests, miniapp) that it could not
reach without declared entry points.
Config decisions:
- entry: bank-sync.ts (2nd PM2 process), ecosystem.config.cjs, scripts/**,
src/**/*.test.ts, and the two module public-API barrels (dev-pipeline,
analytics/ta) that are the intended import surface.
- project: exclude src/services/bank/ZenPlugins/** exactly like biome.jsonc
("!..."); the bot mirrors the types it needs locally.
- ignore: zenmoney-types.ts (deliberate local mirror of ZenMoney submodule
types, intentionally complete) and src/test-utils/** (shared test helpers
imported ad-hoc by tests, per the no-real-network test rule).
- ignoreDependencies: pino-pretty (used at runtime as a pino transport
target string in logger.ts, invisible to static analysis).
- ignoreExportsUsedInFile: an export used only inside its own file is a
redundant modifier / intentional test seam, not dead code; don't churn
stable shared modules to strip a keyword.
- ignoreBinaries: git subcommands mis-detected from Bun.$ shell templates
in dev-pipeline/git-ops.ts.
Refs #110
Investigation: GroupSpreadsheet was introduced with the monthly-budget-tabs feature (6352a74) as the row-shape type for the group_spreadsheets table. GroupSpreadsheetRepository never adopted it — it reads narrower inline shapes (SELECT spreadsheet_id ..., {year, spreadsheet_id}) — so the interface has no importer. It is not orphaned-by-migration (no callsite ever existed: `git log -S "GroupSpreadsheet("` is empty) but it documents a real table and mirrors every other row type in this file. Kept (not deleted) per the conservative dead-code rule; the @public JSDoc tag exempts it from the knip gate. Wiring it into the repo's return types would change stable working code and is out of scope. Refs #110
jsqr was the original QR decoder (added in a447be0, "Implement photo processing feature"). The QR scanner was migrated to the `qr` package in 25d16da ("feat(qr-scanner): integrate new QR decoding library"), which removed the jsqr import. Evidence it is now dead: git log -S "jsqr" -> a447be0 (add), 25d16da (remove import) grep -rI "jsqr|jsQR" src libs scripts miniapp index.ts bank-sync.ts -> none No static import, no dynamic import/require anywhere; qr-scanner.ts uses `qr/decode.js`. Safe to drop. Refs #110
AgentEvent was the event-union of the original agent streaming design (added in 0792822, consumed by agent.ts). Commit 5e57a49 migrated the agent to the AgentStreamWriter push-interface (TelegramStreamWriter), removing the `import type { ..., AgentEvent, ... } from './types'` line from agent.ts. Nothing has referenced it since — classic orphaned-by-migration. Evidence the last consumer was removed: git log -S "AgentEvent" -- src/services/ai/agent.ts -> 0792822 (introduce/consume), 5e57a49 (remove import) git show 5e57a49 -- src/services/ai/agent.ts -> "-import type { AgentContext, AgentEvent, ToolCallResult } from './types'" grep -rn AgentEvent src -> only its own definition ToolResult (referenced by the removed union) stays — still used by ToolCallResult and elsewhere. Full test suite green. Refs #110
|
🔁 PR closed. Stage bot switched back to |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62234efc08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "bank-sync.ts", | ||
| "ecosystem.config.cjs", | ||
| "scripts/**/*.ts", | ||
| "src/**/*.test.ts", |
There was a problem hiding this comment.
When the ZenPlugins submodule is checked out, this broad test entry still matches files such as src/services/bank/ZenPlugins/src/plugins/tbc-ge/__tests__/api/*.test.ts; Knip analyzes entry roots independently of project, so the project exclusion below does not fully ignore the submodule. In local environments that initialize the submodule, bunx knip can still traverse third-party tests and report their imports/exports; add a !src/services/bank/ZenPlugins/** entry negation or narrow this glob.
Useful? React with 👍 / 👎.
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.
Why
Resolve knip ticket #110. knip was flagging ~35 real items drowned under 279 false-positive ZenPlugins-submodule "unused files" (no knip config existed). Every flag was investigated via
git log -S+ introducing-commit history before acting — per the repo's "never delete dead code without investigation" rule.What
knip.jsoncadded: excludes theZenPlugins/submodule (mirrors biome), declares entry points (scripts/, libs/, barrels),ignoreExportsUsedInFile, and allowlists documented below.AgentEventtype — itsagent.tsconsumer was removed in5e57a49when it moved toAgentStreamWriter.jsqrdependency — replaced byqrin25d16da; zero source refs.GroupSpreadsheet(@public row type),zenmoney-typesmirror (deliberate local copy per CLAUDE.md),pino-pretty(runtime pino transport, invisible to static analysis), and exports used only within their own file (createBot,CATEGORY_EMOJIS,aiDebugLogger, etc.) viaignoreExportsUsedInFile.Results
knip exit 0 (clean), type-check + biome lint clean, full suite 3617/3617. No behavior change beyond the two orphaned removals (tests green confirm nothing referenced them).
Closes #110
🤖 Generated with Claude Code
https://claude.ai/code/session_01Rq7c53bgYsq6uZ9hmdhXu9