chore: add react-doctor and knip dev tooling#815
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds infrastructure for two new development tools: Knip (dependency analysis) and react-doctor (React code health checking). It introduces configuration files for both tools, updates package.json with corresponding scripts and dependencies, and documents the new workflows in the agent policy guidelines. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
cc1ad73 to
056f4c1
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 056f4c1. Configure here.
| // This import is intentionally satisfied transitively through bitsocial-react-hooks. | ||
| "electron/start-plebbit-rpc.js": ["unlisted"], | ||
| // Knip falsely infers v8 coverage for Vitest config even though this repo uses Istanbul. | ||
| "vitest.config.ts": ["unlisted"] |
There was a problem hiding this comment.
Dead config reference to nonexistent vitest.config.ts
Low Severity
The ignoreIssues entry for vitest.config.ts references a file that does not exist in this repository. This project has no vitest.config.ts — vitest runs via defaults and vite.config.js. This appears to be a leftover from the source config ("ported from 5chan") that was not removed when adapting paths for seedit. The comment about Istanbul vs. v8 coverage is also inapplicable here, making the entry misleading dead configuration.
Reviewed by Cursor Bugbot for commit 056f4c1. Configure here.
Add react-doctor 0.0.31 and knip 6.1.0 as dev dependencies with matching scripts (doctor, doctor:score, doctor:verbose, knip, knip:full). Port a knip config from 5chan with seedit-shaped paths. Update AGENTS.md task router and verification rules so that React UI logic changes run yarn doctor and dependency changes run yarn knip.
react-doctor.config.json takes precedence and already contains the `diff: false` value plus ignore patterns for test files. Keeping the shadow block in package.json was dead config that would silently ignore any future edits a maintainer made there.
056f4c1 to
5b12a8b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
AGENTS.md (2)
31-32: Two adjacent rows on dependency changes — make their split obvious.Line 31 fires on
package.jsonchanged (→corepack yarn install) and Line 32 fires on "Dependencies or import graph changed" (→yarn knip). They overlap but aren't identical (import-graph changes can happen withoutpackage.jsonedits). Worth a small wording tweak so a contributor reading top-to-bottom doesn't think the second row supersedes the first or vice-versa.📝 Suggested tweak
-| `package.json` changed | Run `corepack yarn install` to keep `yarn.lock` in sync | -| Dependencies or import graph changed | Run `yarn knip` as an advisory manifest/import audit | +| `package.json` changed | Run `corepack yarn install` to keep `yarn.lock` in sync | +| Dependencies added/removed, or new direct imports introduced | Run `yarn knip` as an advisory manifest/import audit (in addition to the lockfile sync above when `package.json` changed) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 31 - 32, Reword the two adjacent table rows so their triggers are clearly distinct: change the first row labeled "`package.json` changed" to emphasize it runs when package.json is edited (e.g., "`package.json` changed — run `corepack yarn install` to update yarn.lock`"), and change the second row labeled "Dependencies or import graph changed" to clarify it covers import-graph or dependency-tree changes even without package.json edits (e.g., "`Import graph or dependency graph changed (no package.json edit) — run `yarn knip` for an advisory audit`"), keeping the associated commands `corepack yarn install` and `yarn knip` attached to their respective rows.
118-119: Clarify thatyarn doctoris a tree-wide signal, not a diff-scoped one.
react-doctor.config.jsonsets"diff": false, soyarn doctoralways scans the whole repo and reports the full backlog (the PR description mentions 97/100 with 62 advisory warnings baseline). Contributors who land here from the Task Router and read "prioritizeerrorthenwarning" may try to triage the entire backlog on every UI change.Suggest tightening the wording so the policy is clearly: only act on findings that are introduced or made worse by the current diff, not the existing baseline. Otherwise this rule will either be silently ignored or cause large unrelated changes per PR.
📝 Suggested wording tweak
- After React UI logic changes, run: `yarn doctor`. - Treat React Doctor output as actionable guidance; prioritize `error` then `warning`. + After React UI logic changes, run: `yarn doctor`. + React Doctor scans the whole tree (not just the diff). Treat output as actionable guidance for findings introduced or made worse by the change; prioritize `error` then `warning`. Do not block on pre-existing baseline warnings unrelated to the diff.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 118 - 119, The current guidance implies `yarn doctor` is diff-scoped; clarify that `react-doctor.config.json` sets "diff": false so `yarn doctor` scans the whole repo and reports the full baseline; update the AGENTS.md text (the two lines about running `yarn doctor` and prioritizing `error` then `warning`) to explicitly state that contributors should only act on issues that are introduced or made worse by the current diff (not existing baseline advisories), and reference `react-doctor.config.json` and the "diff": false setting so reviewers know why the tool reports repo-wide findings.knip.jsonc (1)
16-40:ignoreDependencieslooks well-justified; one nit on grouping.Each entry has a comment explaining why Knip can't trace it (Vite/Electron/Capacitor entrypoints, polyfills injected by
vite-plugin-node-polyfills, workbox viavite-plugin-pwa, commitizen viaconfig.commitizen.path, etc.), which matches the repo wiring I can see. Two small notes:
babel-plugin-react-compileris referenced by name string insidevite.config.js(plugins: [['babel-plugin-react-compiler', …]]), so Knip not tracing it is expected — worth adding a one-word note in the comment (currently grouped under "build/config/html entrypoints").react-grabis in the same "build/config/html" bucket but I don't see it invite.config.js; if it's wired throughindex.htmlor a runtime injection, a more specific comment will save the next maintainer the lookup.Not blocking — just maintenance hygiene for a file that future contributors will diff frequently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@knip.jsonc` around lines 16 - 40, Update the comments in knip.jsonc to clarify why specific packages are ignored: add a one-word note like "(vite.config plugin)" next to "babel-plugin-react-compiler" to indicate it's referenced in vite.config.js (plugins: [['babel-plugin-react-compiler', …]]), and replace the generic "build/config/html entrypoints" comment for "react-grab" with a more specific note such as "(index.html or runtime-injected)" or the exact location if you can find it; edit the comment block immediately above the listed entries to include these brief clarifications so future maintainers can quickly see the wiring for babel-plugin-react-compiler and react-grab.package.json (1)
86-87:yarn knipandyarn knip:fullcover different scopes — make sure the policy reflects that.
yarn knipruns with--production --include dependencies,unlisted,binaries, so it intentionally narrows to a subset of issue types and to non-dev code. It will not flag:
- unused exports / unused exported types
- unused files
- duplicate exports
- anything reachable only from dev/test entrypoints
yarn knip:fullis broader but pairs with--no-exit-code, so CI/agents can't gate on it.That's a reasonable split, but the AGENTS.md guidance ("Run
yarn knipas an advisory manifest/import audit" / "treat findings as advisory") may give a false sense of coverage — a contributor adding an unused export will see a cleanyarn knipand assume Knip is happy. Consider either widening the defaultknipscript's--includeset, or noting in AGENTS.md thatyarn knip:fullis the broader sweep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 86 - 87, The current "knip" npm script uses a narrowed scope ("knip" script) while "knip:full" is broader but non-failing; update either the package.json "knip" script or AGENTS.md to avoid false confidence: either widen the "knip" script's flags to include unused exports/files/duplicate exports (make "knip" run with the broader --include set currently used by "knip:full") so CI gates on the broader sweep, or explicitly document in AGENTS.md that "knip" only checks production dependencies and omits unused exports/files/duplicate exports and that contributors should run "yarn knip:full" (noting it uses --no-exit-code) when adding/removing exports or files; reference the script names "knip" and "knip:full" and the AGENTS.md guidance when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@AGENTS.md`:
- Around line 31-32: Reword the two adjacent table rows so their triggers are
clearly distinct: change the first row labeled "`package.json` changed" to
emphasize it runs when package.json is edited (e.g., "`package.json` changed —
run `corepack yarn install` to update yarn.lock`"), and change the second row
labeled "Dependencies or import graph changed" to clarify it covers import-graph
or dependency-tree changes even without package.json edits (e.g., "`Import graph
or dependency graph changed (no package.json edit) — run `yarn knip` for an
advisory audit`"), keeping the associated commands `corepack yarn install` and
`yarn knip` attached to their respective rows.
- Around line 118-119: The current guidance implies `yarn doctor` is
diff-scoped; clarify that `react-doctor.config.json` sets "diff": false so `yarn
doctor` scans the whole repo and reports the full baseline; update the AGENTS.md
text (the two lines about running `yarn doctor` and prioritizing `error` then
`warning`) to explicitly state that contributors should only act on issues that
are introduced or made worse by the current diff (not existing baseline
advisories), and reference `react-doctor.config.json` and the "diff": false
setting so reviewers know why the tool reports repo-wide findings.
In `@knip.jsonc`:
- Around line 16-40: Update the comments in knip.jsonc to clarify why specific
packages are ignored: add a one-word note like "(vite.config plugin)" next to
"babel-plugin-react-compiler" to indicate it's referenced in vite.config.js
(plugins: [['babel-plugin-react-compiler', …]]), and replace the generic
"build/config/html entrypoints" comment for "react-grab" with a more specific
note such as "(index.html or runtime-injected)" or the exact location if you can
find it; edit the comment block immediately above the listed entries to include
these brief clarifications so future maintainers can quickly see the wiring for
babel-plugin-react-compiler and react-grab.
In `@package.json`:
- Around line 86-87: The current "knip" npm script uses a narrowed scope ("knip"
script) while "knip:full" is broader but non-failing; update either the
package.json "knip" script or AGENTS.md to avoid false confidence: either widen
the "knip" script's flags to include unused exports/files/duplicate exports
(make "knip" run with the broader --include set currently used by "knip:full")
so CI gates on the broader sweep, or explicitly document in AGENTS.md that
"knip" only checks production dependencies and omits unused
exports/files/duplicate exports and that contributors should run "yarn
knip:full" (noting it uses --no-exit-code) when adding/removing exports or
files; reference the script names "knip" and "knip:full" and the AGENTS.md
guidance when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2fe16b28-eeb7-4572-a7dd-87999b666779
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
AGENTS.mdknip.jsoncpackage.jsonreact-doctor.config.json


Summary
`yarn knip` runs clean (0 findings, exit 0).
`yarn doctor` scores 97/100 with 62 advisory warnings — left for follow-up clean-up.
Test plan
Note
Low Risk
Low risk: this only adds dev-only tooling/scripts and agent workflow guidance, with no runtime or app-logic changes; the main risk is CI/local workflow friction from new checks or config mismatches.
Overview
Adds new dev tooling for code health audits. Introduces
knipandreact-doctoras pinneddevDependencies, along with newyarn knip*andyarn doctor*scripts.Adds tool configuration and updates contributor workflow docs. Adds
knip.jsoncandreact-doctor.config.jsonto tune findings/ignores, updatesAGENTS.mdto requireyarn doctorafter React UI logic changes and to recommendyarn knipwhen dependencies/imports change, and refreshes the common command list (with lockfile updates).Reviewed by Cursor Bugbot for commit 5b12a8b. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit