-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add biome linter configuration, CI workflow, and code style improvements #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…nts. - Add AGENTS.md for better compatibility with AI helpers. - Improve codestyle with biome configuration. Biome linter checks will be run on every PR request. - Add suggested MCP services and extensions to install.
WalkthroughAdds Biome and VS Code tooling, a GitHub Actions lint workflow, docs and changelog updates, many stylistic edits, an MCP config entry, EOF newlines in migration meta, and a behavioral change: users.create now throws 422 if INSERT ... RETURNING returns no row. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as Users API
participant Service as UsersService
participant DB as Database
Client->>API: POST /users (body)
API->>Service: UsersService.create(body)
Service->>DB: INSERT ... RETURNING *
alt DB returns user
DB-->>Service: user
Service-->>API: user
API-->>Client: 201 Created (user)
else DB returns no row
DB-->>Service: null
Service-->>API: throw 422 { code, message, details }
API-->>Client: 422 Unprocessable Entity (error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.vscode/settings.json (1)
14-31: Consider removing redundant language-specific formatter settings.Since
editor.defaultFormatteris already set to"biomejs.biome"at the global level (Line 7), the language-specific overrides (Lines 14-31) are redundant and can be safely removed to simplify the configuration.Apply this diff to simplify the settings:
"editor.codeActionsOnSave": { "source.fixAll.biome": "explicit", "source.organizeImports.biome": "explicit" - }, - - "[javascript]": { - "editor.defaultFormatter": "biomejs.biome" - }, - "[javascriptreact]": { - "editor.defaultFormatter": "biomejs.biome" - }, - "[typescript]": { - "editor.defaultFormatter": "biomejs.biome" - }, - "[typescriptreact]": { - "editor.defaultFormatter": "biomejs.biome" - }, - "[json]": { - "editor.defaultFormatter": "biomejs.biome" - }, - "[jsonc]": { - "editor.defaultFormatter": "biomejs.biome" } }src/tests/users.test.ts (3)
1-32: MockingUsersServiceis fine; consider adding typing and (optional) restorationThe
mockUsersService+Object.assign(UsersService, mockUsersService)pattern will redirect calls from the router to the mocks as intended, and thebeforeEachmockClearkeeps call counts per test clean. To catch future signature drift and make this safer, you could:
- Type the mock, for example with
satisfies Pick<typeof UsersService, 'create' | 'get'>, so argument/return types stay aligned.- Optionally store the original
UsersServicemethods and restore them in anafterAllif other tests in the suite ever rely on the real implementation.
40-83: POST /users tests exercise both success and validation paths wellThe happy‑path test validates status, key response fields, and that
UsersService.createis invoked with the exact payload; the validation test asserts a 422 and thatcreateis not called, which nicely proves that validation happens before hitting the service. If you ever need tighter guarantees, you could also assert on the error response body shape in the 422 case.
86-101: GET /users test aligns with the typed response contractCasting
response.json()toUsersModel.getResponseand then assertingusers/totalplus basic shape checks gives a good contract test for the route. Verifying thatmockUsersService.getwas called ensures the handler is correctly delegating to the service. If desired, you could further assert on the contents ofresponseData.users[0]to lock in the projection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
.cursor/mcp.json(1 hunks).github/workflows/lint.yml(1 hunks).vscode/extensions.json(1 hunks).vscode/settings.json(1 hunks)AGENTS.md(1 hunks)CHANGELOG.md(3 hunks)biome.json(1 hunks)drizzle.config.ts(1 hunks)package.json(2 hunks)src/common/config.ts(1 hunks)src/common/logger.ts(1 hunks)src/db/index.ts(1 hunks)src/db/migrations/meta/20251111132328_snapshot.json(1 hunks)src/db/migrations/meta/_journal.json(1 hunks)src/db/schema/users.ts(1 hunks)src/main.ts(1 hunks)src/modules/users/index.ts(3 hunks)src/modules/users/model.ts(1 hunks)src/modules/users/service.ts(1 hunks)src/tests/users.test.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/tests/users.test.ts (2)
src/modules/users/index.ts (1)
users(6-45)src/modules/users/model.ts (2)
getResponse(31-34)getResponse(35-35)
src/modules/users/service.ts (3)
src/modules/users/model.ts (10)
createRequest(6-10)createRequest(11-11)createResponse(13-13)createResponse(14-14)createError(16-16)createError(17-17)getQuery(20-28)getQuery(29-29)getResponse(31-34)getResponse(35-35)src/db/schema/users.ts (1)
users(10-19)src/modules/users/index.ts (1)
users(6-45)
src/db/schema/users.ts (1)
src/modules/users/index.ts (1)
users(6-45)
src/modules/users/model.ts (1)
src/db/schema/users.ts (2)
userCreate(37-37)userSelect(38-38)
src/main.ts (2)
src/common/logger.ts (1)
log(4-19)src/modules/users/index.ts (1)
users(6-45)
src/modules/users/index.ts (2)
src/common/logger.ts (1)
log(4-19)src/modules/users/model.ts (5)
usersModelPlugin(41-48)createResponse(13-13)createResponse(14-14)getResponse(31-34)getResponse(35-35)
🪛 markdownlint-cli2 (0.18.1)
AGENTS.md
39-39: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
40-40: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
41-41: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
42-42: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (22)
src/db/migrations/meta/20251111132328_snapshot.json (1)
70-70: ✓ Formatting change aligns with Biome standards.Adding a trailing newline to the file follows linting best practices and is consistent with the Biome configuration being introduced in this PR.
src/db/migrations/meta/_journal.json (1)
13-13: ✓ Formatting change aligns with Biome standards.Adding a trailing newline to the file follows linting best practices and is consistent with the Biome configuration being introduced in this PR.
.vscode/extensions.json (1)
1-7: VSCode extension recommendations align with the new toolingBiome + conventional commits recommendations match the linting and workflow setup; config is valid JSON and non-invasive.
tsconfig.json (1)
29-34: tsconfig change is purely stylisticRemoving the trailing comma after
"baseUrl": "."keeps the config valid and does not alter TypeScript compiler behavior.Please re-run your usual
tsc/build step once to confirm there are no accidental config diffs in downstream tooling.src/db/index.ts (1)
1-11: Drizzle/Postgres pool configuration looks validThe
drizzle-orm/node-postgresusage with aconnectionobject (includingconnectionStringandcasing: 'snake_case') matches documented patterns for drizzle’s Postgres driver. (orm.drizzle.team)
Settingmax: 10relies on pg’s standard pool configuration and is a reasonable default for many small services. (node-postgres.com) The trailing comma added here is purely stylistic and does not affect behavior.After merging, hit a real database (or run existing DB integration tests) to verify connection pooling behaves as expected with your workload and max connection limits.
.cursor/mcp.json (1)
1-8: MCP configuration is well-formed and self-containedThe
context7MCP server definition vianpx -y @upstash/context7-mcpis valid JSON and scoped to tooling, so it won’t affect runtime behavior of the app.The first time you use this in Cursor, confirm that
npxcan fetch the package in your environment and that you’re comfortable with the transitive dependencies it pulls.drizzle.config.ts (1)
1-14: Drizzle Kit configuration matches recommended structure
defineConfigwithdialect: 'postgresql', schema/out paths, anddbCredentials: { url: config.DATABASE_URL }aligns with Drizzle’s documented Postgres setup; added trailing commas are stylistic only. (orm.drizzle.team)When running
drizzle-kit(e.g.,db:generate/db:migrate), make sureDATABASE_URLis set in the environment Bun sees so migrations continue to work as before..github/workflows/lint.yml (1)
1-27: Biome lint workflow is wired correctlyThe workflow correctly installs dependencies with Bun and runs the
lintscript, so Biome will gate pushes/PRs tomainanddevelopas intended. The push + pull_request triggers are fine; if you ever want to avoid duplicate runs on PR branches, you can narrow this to justpull_request.Let this run on a test PR from a feature branch into
developto confirmbun install --frozen-lockfileandbun run lintsucceed in CI with your current lockfile.src/common/config.ts (1)
1-15: Explicit NODE_ENV and LOG_LEVEL validation is clear and sensibleRefactoring NODE_ENV and LOG_LEVEL into multi-line
strconfigs with explicitchoicesanddefaultvalues improves readability without changing semantics; the remaining fields are untouched.Double-check that all environment names and log levels you actually use in each deployment (e.g., any custom “local”/“preview” envs) are covered by the allowed
choicesto avoid surprises at startup.package.json (1)
4-31: Biome scripts and dependency bumps are consistent with the new toolingThe new
lint,lint:fix, andformatscripts are correctly wired to the Biome CLI and match the GitHub Actions workflow. The added@biomejs/biomedevDependency and minor bumps toelysiaanddrizzle-kitare coherent with the rest of the PR.After installing these changes, run
bun run lintand your existing test/smoke suite to confirm the Biome CLI is picked up correctly and that the framework/Drizzle upgrades don’t introduce regressions.src/common/logger.ts (1)
6-18: LGTM! Improved non-production logging.The change from
NODE_ENV === 'development'toNODE_ENV !== 'production'enables pretty-printing in all non-production environments (test, staging, etc.), which improves observability and debugging capabilities across the development lifecycle.biome.json (1)
1-58: LGTM! Well-configured Biome setup.The Biome configuration is comprehensive and follows best practices:
- VCS integration with
.gitignoresupport- Sensible formatter defaults (80 char line width, 2-space indent, LF endings)
- Recommended linter rules with
noStaticOnlyClassappropriately disabled for the abstract service pattern used in this codebase- JavaScript preferences enforcing single quotes, semicolons, and trailing commas for consistency
- Import organization enabled
CHANGELOG.md (1)
10-16: LGTM! Changelog properly documents the changes.The unreleased section appropriately documents the addition of Biome for linting/formatting and the dependency updates, following the Keep a Changelog format.
src/modules/users/model.ts (1)
1-48: LGTM! Consistent formatting improvements.All changes in this file are formatting/style adjustments that align with the new Biome configuration (single quotes, trailing commas, semicolons, multi-line object formatting). No functional changes to types or runtime behavior.
AGENTS.md (1)
1-67: Excellent project documentation!This comprehensive guide provides clear instructions for AI assistants and contributors, covering all essential aspects: tech stack, setup commands, database operations, code conventions, testing requirements, and PR workflow. The documentation aligns perfectly with the Biome-based tooling introduced in this PR.
src/modules/users/service.ts (2)
8-23: LGTM! Improved error handling with defensive programming.The refactored
createmethod removes the abstraction layer (handleDbOperation) in favor of direct database operations with explicit error handling. The addition of the check for an empty insert result (Lines 16-20) is a good defensive programming practice that ensures the service fails gracefully with a proper 422 status if the insert operation doesn't return a user.The use of
satisfiesfor type-safe error messages is also a nice touch.
25-45: LGTM! Cleaner query structure.The
getmethod maintains its functionality while adopting a more explicit field selection pattern that improves readability and makes it clear which fields are being returned.src/db/schema/users.ts (1)
1-39: LGTM! Consistent formatting improvements.All changes in this file are formatting/style adjustments that align with the new Biome configuration:
- Single quotes for imports
- Multi-line object formatting for better readability
- Semicolons for statement termination
No functional changes to the schema definition, field types, constraints, or validations.
src/modules/users/index.ts (2)
11-27: POST /users handler and route metadata are consistent with the modelThe handler returning
Promise<UsersModel.createResponse>and delegating toUsersService.create(body)matches thecreateResponsetype, and the route metadata (body: 'users.createRequest',response: { 200: 'users.createResponse', 422: 'users.createError' },detail) lines up with the users model plugin. This gives you a clear contract for both runtime validation and documentation.
30-45: GET /users route correctly mirrors the typed response and query modelTyping the handler as
Promise<UsersModel.getResponse>, returning the result ofUsersService.get(query), and wiringquery: 'users.getQuery'plus the 200/422 response mappings keeps the handler, model, and docs in sync. The added description is clear and matches the actual behavior.src/main.ts (2)
1-42: App wiring for CORS, logging, OpenAPI, and users route looks coherentRegistering
cors, then the logger vialog.into(...), then the OpenAPI plugin, and finally theusersmodule, with a centralizedonErrorthat logsctx.error, gives a clean bootstrap path. The OpenAPI configuration usingconfig.SERVER_HOSTNAME/SERVER_PORTand explicit info/servers metadata is straightforward and should keep docs aligned with the running instance.
44-56: Signal handlers provide clear shutdown behaviorThe
SIGINT/SIGTERMhandlers now log a concise shutdown message, then stop the app, close the DB client, and exit with code 0. This keeps shutdown behavior explicit and should make local and production termination more observable.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
AGENTS.md (1)
1-67: Document provides comprehensive project guidelines; appears to resolve past formatting issues.The file effectively documents the project structure, conventions, and workflows for AI helpers and contributors. The markdown structure now appears properly formatted—the nested list items (lines 39–42) use correct 2-space indentation, and the duplicate "Architecture" entry mentioned in past reviews is no longer present.
However, one gap relative to PR objectives:
The file documents Biome as the linter/formatter (line 36) and references linting/formatting commands, but provides no details about MCP services and extensions mentioned in the PR description ("Add suggested MCP services and extensions to install"). Consider adding a new section (e.g., under "Setup & Commands" or as a standalone "Recommended Tools") that lists recommended MCP services and VS Code extensions for this project.Additionally, the CI workflow (GitHub Actions) is mentioned indirectly in the linting instructions but not explicitly documented. You may want to add a brief note about automated checks running on PR requests to align with the PR objective: "Biome linter checks will be run on every PR request."
If you'd like, I can help generate the missing sections. Otherwise, the documentation is well-structured and clear for contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Added local MCP server configuration | ||
| - Added CI lint workflow | ||
| - Added Biome-based formatting/linting | ||
| - Added VS Code recommendations/settings included |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify the redundant phrasing on line 15.
The phrase "Added VS Code recommendations/settings included" is awkwardly phrased with the redundant "included" at the end. Simplify to match the style of other entries.
Apply this diff to clarify:
- Added VS Code recommendations/settings included
+ Added VS Code recommendations/settings📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Added VS Code recommendations/settings included | |
| - Added VS Code recommendations/settings |
🤖 Prompt for AI Agents
In CHANGELOG.md around line 15, the entry "Added VS Code
recommendations/settings included" uses redundant wording; replace it with a
concise phrase matching other entries such as "Added VS Code
recommendations/settings" (remove the trailing "included") so the line reads
clearly and consistently with the changelog style.
…e improvements.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.