Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR prepares the CI pipeline by updating test flags and CLI invocations, refreshes dependencies and lint configurations, and enhances code robustness through refined type imports, stricter generics, improved runtime guards, and removal of a redundant export. Class diagram for updated StateReducer typeclassDiagram
class StateReducer {
<<type>>
[Key in FnKeys<A extends object>]: (prevState: S, patch: Patches<A>[Key]) => S
}
Class diagram for VsCodeApi interface (unchanged, but import style updated)classDiagram
class VsCodeApi {
postMessage(message: unknown): void
getState(): unknown
setState(state: unknown): void
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughConfiguration updates to CI, ESLint, and permissions; dependency tweaks; minor client type/import adjustments; a stricter generic constraint in a public type alias; a refined type guard in host utilities; and removal of a commented export. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10–15 minutes Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Summary of Changes
Hello @hbmartin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on a significant cleanup and modernization of the project's development environment. It primarily involves updating various core dependencies to their latest versions and removing the Storybook component library infrastructure. Additionally, the changes include minor code refinements for improved type safety and setting up necessary permissions for future continuous integration processes.
Highlights
- Dependency Updates: Key dependencies, including
@vitejs/plugin-reactand@rolldown/pluginutils, have been updated to their latest versions, along with a minor update to@babel/core. - Storybook Removal: Storybook and its associated dependencies, configuration files, and ESLint plugin have been completely removed from the project, streamlining the development setup.
- CI Preparation: The Claude AI assistant's local settings have been updated to allow
npm run buildandnpm run lintcommands, indicating preparation for continuous integration workflows. - Codebase Refinements: Minor type safety improvements were made, such as adding an
extends objectconstraint to a generic type and refining a type guard function to explicitly exclude arrays.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The CI step for @arethetypeswrong/cli still uses continue-on-error—consider failing the build on type mismatches to catch issues early.
- Removing
.storybookandstorybook-staticfrom the ESLint ignore list may surface unwanted lint errors—double-check that change is intentional. - The new
--passWithNoTestsflag can mask missing test failures—consider reverting or using a stricter approach so actual test failures break the build.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CI step for @arethetypeswrong/cli still uses continue-on-error—consider failing the build on type mismatches to catch issues early.
- Removing `.storybook` and `storybook-static` from the ESLint ignore list may surface unwanted lint errors—double-check that change is intentional.
- The new `--passWithNoTests` flag can mask missing test failures—consider reverting or using a stricter approach so actual test failures break the build.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 3fa8b2b in 1 minute and 51 seconds. Click for details.
- Reviewed
121lines of code in8files - Skipped
1files when reviewing. - Skipped posting
10draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .claude/settings.local.json:4
- Draft comment:
Removed wildcard in build command; confirm this limitation is intentional. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. .github/workflows/ci.yml:37
- Draft comment:
Updated npm test command with '--passWithNoTests'; verify tests aren’t inadvertently skipped. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to verify that tests aren't inadvertently skipped, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
3. .github/workflows/ci.yml:51
- Draft comment:
Switched CLI command from 'are-the-types-wrong' to '@arethetypeswrong/cli'; ensure migration compatibility. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure migration compatibility, which is against the rules. It doesn't provide a specific suggestion or ask for a specific test to be written. It is more of a general cautionary note.
4. eslint.config.js:22
- Draft comment:
Removed Storybook paths from globalIgnores; double-check if Storybook files now require linting. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to double-check if Storybook files now require linting after removing them from globalIgnores. This violates the rule against asking the author to double-check things.
5. package.json:67
- Draft comment:
Updated '@vitejs/plugin-react' version to ^5.0.3; verify no breaking API changes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify that there are no breaking API changes, which is against the rules. It is related to dependency changes, which should not be commented on unless it's a specific suggestion or issue.
6. package.json:80
- Draft comment:
Removed 'eslint-plugin-storybook'; ensure removal doesn't affect Storybook linting if used. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the removal of 'eslint-plugin-storybook' doesn't affect Storybook linting. This is a request for confirmation and testing, which violates the rules.
7. src/lib/client.ts:1
- Draft comment:
Cleaned up commented export; consider removing legacy code if unused. - Reason this comment was not posted:
Comment looked like it was already resolved.
8. src/lib/client/WebviewProvider.tsx:16
- Draft comment:
Switched to 'import type' syntax for VsCodeApi; good use of type-only imports. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it simply acknowledges the use of 'import type' syntax without providing any actionable feedback or suggestions for improvement. It doesn't align with the rules for useful comments.
9. src/lib/client/types.ts:57
- Draft comment:
Added 'A extends object' constraint in StateReducer for improved type safety. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it simply states what was changed without providing any actionable feedback or suggestions. It does not ask for confirmation or suggest improvements.
10. src/lib/host/utils.ts:10
- Draft comment:
Added '!Array.isArray(message)' check and removed redundant undefined check; improves message type validation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, describing what changes were made without providing any actionable feedback or suggestions. It does not align with the rules for useful comments.
Workflow ID: wflow_iumTMZHWJRgeRRlj
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
There was a problem hiding this comment.
Code Review
This pull request delivers a solid set of updates, focusing on dependency management, CI configuration, and code quality. The changes are well-aligned with the description and contribute positively to the project's health.
Key improvements include:
- Dependency Updates: Updating
@vitejs/plugin-reactand removing the unusedeslint-plugin-storybookare good maintenance steps. - Enhanced Type Safety: The constraint
A extends objectadded toStateReducerinsrc/lib/client/types.tsis an excellent change that improves type safety and prevents potential misuse with primitive types. - Robust Type Guards: The
isMyActionMessagetype guard insrc/lib/host/utils.tshas been made more robust by removing a redundant check and correctly handling array inputs with the!Array.isArray(message)condition.
The code changes are of high quality and I have no specific comments on the diff itself.
For future consideration, I noticed that a similar type guard, isMyPatchMessage in src/lib/client/useVscodeState.ts, was not updated in this PR. To maintain consistency and robustness, it would be beneficial to apply the same improvements to it in a follow-up task.
Overall, this is a great contribution. Well done!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/lib/host/utils.ts (1)
10-10: Dropsymbolacceptance forkey(not cloneable over postMessage).Symbols don’t survive structured clone; accepting them is unreachable and can mask mistakes. Prefer string‑only keys.
- (typeof message.key === 'string' || typeof message.key === 'symbol') && + typeof message.key === 'string' &&.github/workflows/ci.yml (1)
37-37: Make type/package validation blocking on main.Leaving publint/ATTW as non‑blocking is fine for PRs, but consider failing on main to prevent shipping broken types.
- - name: Validate package.json - run: | - npx publint - npx @arethetypeswrong/cli --pack . - continue-on-error: true + - name: Validate package.json (PRs - non-blocking) + if: ${{ github.event_name == 'pull_request' }} + run: | + npx publint + npx @arethetypeswrong/cli --pack . + continue-on-error: true + + - name: Validate package.json (main - blocking) + if: ${{ github.ref == 'refs/heads/main' && github.event_name == 'push' }} + run: | + npx publint + npx @arethetypeswrong/cli --pack . + continue-on-error: falseAlso applies to: 51-51
src/lib/client/types.ts (1)
57-59: Constraint change may be breaking; consider narrowing to string‑keyed records.A now requires
object. Confirm no downstreams relied on primitives/unknown. Optionally preferRecord<string, unknown>to align with string method keys.-export type StateReducer<S, A extends object> = { +export type StateReducer<S, A extends Record<string, unknown>> = { [Key in FnKeys<A>]: (prevState: S, patch: Patches<A>[Key]) => S; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.claude/settings.local.json(1 hunks).github/workflows/ci.yml(2 hunks)eslint.config.js(0 hunks)package.json(1 hunks)src/lib/client.ts(0 hunks)src/lib/client/WebviewProvider.tsx(1 hunks)src/lib/client/types.ts(1 hunks)src/lib/host/utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- eslint.config.js
- src/lib/client.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/client/types.ts (1)
src/lib/client.ts (1)
StateReducer(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
.claude/settings.local.json (1)
4-5: Principle of least privilege: good tighten.Replacing the wildcard with explicit build/lint is a solid move.
src/lib/host/utils.ts (1)
10-10: Stronger type guard.Adding the explicit not‑an‑array check closes a gap and makes the narrowing safer.
src/lib/client/WebviewProvider.tsx (1)
16-16: Type‑only import is correct.Eliminates a runtime import for a types‑only symbol; ideal for webview bundling.
package.json (1)
67-67: Vite compatibility OK — verify React 19 HMR@vitejs/plugin-react@5.0.3 declares Vite peer support (includes ^7.x) and the repo’s vite is 7.1.5 — Vite/plugin pairing looks fine; the plugin does not declare a React peerDep but there are reported HMR/useEffect issues with React 19. Run a dev + HMR smoke test (save/edit/useEffect) on React 19 before merging. (github.com)
Summary by Sourcery
Update CI workflow to allow no-test passes and switch to the scoped types CLI, bump plugin dependencies, clean up lint configuration, and refine TypeScript code and type guards.
Enhancements:
CI:
Summary by CodeRabbit