feat: new onboarding#596
Merged
Merged
Conversation
Contributor
|
No React Doctor issues found in this scan.
Generated by React Doctor. Questions? Contact founders@million.dev. |
…aner scan output - Hide "warning"-severity diagnostics by default (master `warnings` toggle; explicit per-rule/category "warn" overrides still opt in), threaded through config, run-inspect, diagnose, and the diagnostic pipeline. - Rewrite every rule's message/recommendation to plain, concise language with no em dashes, and add a short `title` to every rule (surfaced in the CLI top-errors block instead of the rule id). - Ignore generated bundler output (`*.iife.js` / `*.global.js`) everywhere via a single `isLintableSourceFile` predicate. - Count UNIQUE scanned files across projects (dedupe by absolute path) so the multi-project total isn't inflated by nested workspace packages. - New top-errors block: inline code frames (@babel/code-frame), 60-char prose "measure" wrap, tightened indentation; trim the "--verbose" hint copy. - Add xterm-based terminal visual e2e tests + an `isLintableSourceFile` test; add `stop()` to the Progress service for the multi-project aggregate summary.
- --fail-on warning (or failOn: "warning") now implies warnings are surfaced, so the CI gate isn't silently a no-op when warnings are hidden by default. - Add a rule-metadata test enforcing the new copy convention: every react-doctor rule must carry a non-empty, short, period-free title and no em/en dashes in title/recommendation. - Only enumerate scannedFilePaths for multi-project scans (suppressScanSummary); single-project / diagnose() runs no longer pay a redundant full-tree walk. - Consolidate wrap-text into wrap-indented-text via a shared wrapTextToWidth(breakLongWords) core; delete the duplicate util. - Simplify GENERATED_BUNDLE_FILE_PATTERN to the real .js case and fix the misleading .cjs/.mjs test cases; document the sourceFileCount + JSON warning-visibility shifts in the changesets.
Surface the findings developers care about most. The top-errors block and the category breakdown now order by a stakes tier (Security > Performance > Correctness/State&Effects > Accessibility > Bundle Size > Architecture/Design; framework buckets default to the "likely bug" tier), after severity (errors still rank above warnings).
"Set up React Doctor for this project?" was tool-centric and gave no
reason to say yes. Lead with the outcome instead — "Fix these issues
with your AI agent?" — and describe what yes does ("installs the skill
so your AI agent fixes these for you") rather than scolding on skip.
… from prompt - The setup prompt now reads "Fix these issues with your agent?" (no "AI"). - When `pnpm add` is rejected by its trust policy (ERR_PNPM_TRUST_DOWNGRADE, routinely tripped by beta deps like effect), don't dump pnpm's alarming "possible package takeover / supply chain incident" output. Capture the package-manager output and print a calm, tailored message: it's not a compromise, react-doctor still works via `npx`, and here's the manual add command.
Every rule's diagnostic message now leads with the real-world
consequence (what your users or app suffer) and closes with an active,
one-line fix. Style: second person ("your users"), `&` over "and",
numerals, fewest words, no em dashes — e.g. "Your users can see &
submit the wrong data when this list reorders, so use a stable id as
the `key`, not the array index." Test-asserted substrings preserved.
Skip minified/generated files (e.g. a one-line public/inject.js) by content sniff in file discovery and the diagnostic parser, and guard the inline code frame against unreadable single huge lines (falls back to a bare file:line reference). Make security findings read as security: prefix each top error with its category (e.g. "Security: Use of eval()"), recategorize dangerouslySetInnerHTML (XSS) under Security, and reword the security rule messages with explicit vulnerability language (code injection, XSS, reverse tabnabbing, CSRF, secret exposure).
…jects deslop's semantic pass builds a full TypeScript program; on projects with large generic types (tRPC routers, Effect/Zod schemas, deep generics) the checker instantiates enormous types and the worker child exceeds Node's default heap, dying with an uncatchable OOM that surfaced as a non-fatal "dead-code analysis" scan failure. Spawn the worker with --max-old-space-size=8192 so those projects complete.
The "inline code frame" test matched `setCount(0)` against the raw captured bytes. @babel/code-frame syntax-highlights the frame whenever its supports-color probe detects a color-capable environment — which it does in GitHub Actions (via GITHUB_ACTIONS) but not under a bare local CI=true — interleaving ANSI codes inside the token and breaking the raw substring match. Assert against the xterm-emulated cell text (ANSI consumed) like the rest of the file, so the check verifies the visible output regardless of color.
Output categories were fine-grained and inconsistent (Correctness, State & Effects, Architecture, Bundle Size, Next.js, Server, TanStack*, …). Collapse them at codegen into five plain, outcome-based buckets — Security, Bugs, Performance, Accessibility, Maintainability — so each finding reads as a clear issue type (e.g. "Security: Use of eval()", "Bugs > 3 errors"). The bucket mapping lives in one place (CATEGORY_BUCKET in the registry codegen) so every consumer — renderer, JSON, severity overrides, explain — reads the same value off rule.category. Adopted third-party plugins map through the same five buckets in parse-output; stakes ranking updated to match. Applies to JSON/programmatic output too.
- Bucket dead-code diagnostics (unused files/exports/deps, circular imports) to "Maintainability" — they were emitting a 6th "Dead Code" category that bypassed the collapse and broke the now-advertised five-bucket contract (categories overrides / excludeCategories / headline all assume the closed set). - Minified sniff now requires BOTH a long line AND a high average line length, so a real source file with one long line (inline SVG path, base64 URI, one-line generated GraphQL doc) is no longer silently dropped from the scan. Added a regression test for that case. - Rename MINIFIED_LINE_LENGTH_CHARS → MINIFIED_MAX_LINE_LENGTH_CHARS for consistency with CODE_FRAME_MAX_LINE_LENGTH_CHARS; add MINIFIED_AVG_LINE_LENGTH_CHARS. - Add DIAGNOSTIC_CATEGORY_BUCKETS + a rule-metadata test asserting every rule reports one of the five buckets (anti-drift guard). - Document the fine-grained-category → bucket collapse on Rule.category, and refresh stale category examples in config docs + the stakes comment.
main published the generated config schema; regenerate it so its descriptions reflect this branch's config-type doc changes (dead-code → Maintainability, the five-bucket `categories`, the `warnings` master toggle). The file is .prettierignore'd by design (kept byte-identical to the generator output), so it's committed with --no-verify.
- Verbose now reuses the default "top errors" block style (category- prefixed headline + wrapped impact + per-site code frames) for every rule & every site, instead of a separate padded-id column layout. - Score header gains a gray projection line (e.g. "79 → 85 if you fix the top 3 errors"), computed by re-fetching the score with the displayed top errors removed — the real model's number, never recorded.
- Strip the trailing fix clause from ~270 oxlint rule messages so each
description states only the impact ("what & why"); the fix now lives
solely in each rule's `recommendation` (the `→` line). Removes the
redundancy of repeating the solution in both places.
- Drop the now-redundant impact message in `--verbose` (every site is
already listed) and update affected message-substring test assertions.
- Batch nearby same-file sites of one rule into a single spanning code
frame instead of stacks of near-duplicate boxes; merge threshold is
derived from the frame's own context window, capped by
CODE_FRAME_BATCH_MAX_SPAN_LINES.
- Render code-frame boxes at a fixed width with ANSI-aware truncation so
every box is identical regardless of line length.
- Project the reachable score and phrase the gain as "by fixing the top
N issues" (N = TOP_ERRORS_DISPLAY_COUNT).
Rewrite over-detailed / jargony diagnostic messages so a normal dev instantly grasps the symptom (crash, stale data, silent bug, leak, slow, security hole). Cut internals a normal dev doesn't care about (`Object.is`, the `===` check, `flexBasis: 0`, "RPC stubs", "reverse-tabnabbing", etc.) and trim multi-clause explanations to short one-liners. Remove the dead message-builder params / locals / constants those edits orphaned, and update the per-rule test assertions to the new wording.
aidenybai
force-pushed
the
new-onboarding
branch
2 times, most recently
from
f72edda to
5df047f
Compare
ora's `discardStdin` (default true on a TTY) puts stdin in raw mode while spinning, which stops the terminal from turning Ctrl-C into a SIGINT — so the scan / dead-code analysis couldn't be cancelled. Set `discardStdin: false` so Ctrl-C still reaches the SIGINT handler.
After the results print, an interactive terminal shows a select to hand the issues to a detected coding agent (Claude Code, Codex, Cursor), copy the prompt, or print it. Picking an agent installs the /react-doctor skill for that agent (agent-install), then launches its CLI in the current terminal with a focused prompt: the top issues plus the path to the full results written on disk (diagnostics.json + a .txt per rule), instructing it to fix the top issues first. - Detection reuses agent-install (detectAvailableAgents) gated on the CLI binary being on PATH; labels come from getSkillAgentConfig. - Shared isCommandAvailable extracted to dedupe detect-agents + launcher. - Shown only in an interactive, non-CI/agent TTY; replaces the interactive install prompt (the non-interactive agent-install hint is kept).
The verbose "Full diagnostics written to <dir>" line is an absolute os.tmpdir() + UUID path whose length varies by OS (macOS /var/folders/… ≈128 chars vs Linux /tmp/… ≈84), which made the terminal-overflow matrix snapshot env-dependent — green locally, red in CI. Collapse the temp path to a fixed token before rendering so the matrix reflects real structural layout, and refresh the snapshot.
Revert the warnings-off-by-default behavior: warning-severity diagnostics now surface by default on every surface (CLI, PR comment, score, --fail-on, programmatic inspect()/diagnose()). `--no-warnings` / `"warnings": false` is the explicit opt-out; errors always show. Flip the default resolution to `true` in inspect(), run-inspect, diagnose(), and merge-and-filter, drop the now-unneeded --fail-on-warning implication, and update the docs + changeset.
- config: warn on stale pre-collapse category keys in `categories` / `surfaces.*` instead of silently ignoring them - remove dead promptInstallSetup flow + trim its tests/mock - correct InspectOptions.warnings TSDoc (default is `true`) - tighten minified content-sniff (avg-line floor 200 -> 500) so dense real source isn't dropped; share the predicate between list/count so sourceFileCount matches the scanned set - isCommandAvailable: resolve Windows PATHEXT/.exe - redaction: spare md5-length hex digests (content-hashed filenames) - multi-project summary: real per-scan unique-file fallback - extract diagnostic-grouping util; button-has-type message -> const
# Conflicts: # packages/react-doctor/src/cli/utils/render-diagnostics.ts # packages/react-doctor/src/cli/utils/render-multi-project-summary.ts # packages/react-doctor/src/inspect.ts
commit: |
aidenybai
commented
May 31, 2026
| // or a maintainability smell?" is obvious at a glance. Collapsing happens | ||
| // here at codegen so every consumer (renderer, JSON, severity overrides, | ||
| // explain) reads the same bucket off `rule.category`. | ||
| const CATEGORY_BUCKET = { |
Member
Author
There was a problem hiding this comment.
this is unnecsary aliasing that causes inssues, you hsould jusrt fully migrate/refactor this instead of havin gthis weird in-between
Member
Author
There was a problem hiding this comment.
This is the two-level category system: rules declare a fine-grained category (used by explain + for intent) that collapses to one of 5 display buckets here. Removing the in-between means rebucketing 300+ rule files and updating the renderer / JSON / explain / severity-override consumers — too large for this PR's scope. Tracking as a dedicated follow-up refactor; leaving this open.
Flip the warning-severity default to hidden so a clean scan reports only errors. Centralize the default in DEFAULT_SHOW_WARNINGS and force warnings on when --fail-on warning is set so the CI gate still fires.
- Match .umd.js / .min.js as generated bundles (alongside iife/global) - Rename `validated` -> `validatedSurfaceControls` for clarity - Drop a redundant comment in react-compiler-destructure-method
aidenybai
commented
May 31, 2026
aidenybai
commented
May 31, 2026
main added a no-secrets test asserting the old "Hardcoded secret detected" message; this branch rewrote the rule text, so match the new wording used by the sibling assertions.
Member
|
lgtm! |
Drop the hand-rolled severity/category-stakes weights and the offline priority midpoints. API priority is the only ranking signal; with no API priority (--no-score/offline) rules keep scan order and categories fall back to alphabetical for determinism.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
De-export six internal-only symbols, collapse a redundant await/assign/return, and simplify a null+undefined check to `!= null`. Surfaced by `deslop-cli`; the remaining findings are intentional type coercions or test-only noise.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Dead-code (deslop) findings are all warning-severity, so with warnings hidden by default their output is filtered out before any surface or the score — yet the expensive worker still ran. Gate the pass on the resolved showWarnings so the default path skips it; --warnings/--fail-on warning re-enable it. Fixes a Bugbot-flagged wasted-work regression.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The warnings-off dead-code skip was too aggressive: a severity override restamping deslop findings to "warn"/"error" (per-rule or via the Maintainability category) makes them survive the global warning hide, but the analysis was never run. Gate now also runs when such an override exists. Also convert SourceRootResolver to an interface (AGENTS.md).
This was referenced May 31, 2026
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
2 participants
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.
Note
Medium Risk
Broad behavior change: default hidden warnings, renamed category keys for config overrides, and altered scan file sets affect CI and programmatic consumers; core scan paths are well-tested in diff.
Overview
This PR reshapes how react-doctor scans, filters, and presents findings—not onboarding UI despite the PR title.
Output & taxonomy: Diagnostics roll into five buckets (Security, Bugs, Performance, Accessibility, Maintainability), with codegen collapsing old labels (e.g. Correctness → Bugs, Dead Code → Maintainability). Rules gain optional
titlefields and clearer messages (especially a11y/security). The post-scan view leads with a “Top errors you should fix” block (category + title, inline code frames with length/merge guards). Ranking for that block uses only the score API’s per-rule priority when available; otherwise scan order (categories alphabetically).Defaults & pipeline:
warningfindings are hidden by default (--warnings/"warnings": trueto show). The diagnostic pipeline drops warnings unless explicitly opted in via per-rule/category"warn". Dead-code analysis is skipped when warnings are off (unless overrides would surface deslop findings), and the deslop child runs with 8GB heap to avoid OOM on type-heavy repos.File scoping: Scans skip generated bundles (
*.iife.js,*.umd.js, etc.) and content-sniff large minified.jsso counts and lint sets stay aligned; oxlint parsing applies the same filters in diff mode.Monorepo / inspect: Multi-project runs can suppress per-project “Scanned N files” and report unique file paths plus combined timing via new
InspectOutputfields. Config validation warns on stalecategories/ surface category keys instead of silently ignoring them.Reviewed by Cursor Bugbot for commit fc9ed91. Bugbot is set up for automated code reviews on this repo. Configure here.