Feature/563 Add pre-commit hook to auto fix lint issues before committing - #782
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds lefthook as a dev dependency, wires it into ChangesLefthook Pre-commit Hook Implementation
Docker Context Ignore List
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
docs/tickets/563/review.md (1)
32-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd language specifiers to fenced code blocks.
Lines 32 and 72 show quoted file content in fenced code blocks without language tags. Add
textormarkdownspecifiers for consistent rendering.Also applies to: 72-74
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f8d07295-8ec9-4591-9f10-64692f5a5ee2
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
.gitignoreCLAUDE.mddocs/tickets/563/plan.mddocs/tickets/563/review.mddocs/tickets/563/tasks.mddocs/tickets/563/ticket.mdlefthook.ymlpackage.json
| # Git Hooks (installed automatically via postinstall) | ||
| # Pre-commit hook runs Biome --write on all staged files and re-stages fixes. | ||
| # To skip the hook for a single commit: LEFTHOOK=0 git commit ... | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update documentation to match actual hook scope.
Line 19 states "all staged files", but the hook targets only TypeScript files (**/*.{ts,tsx} per review.md). Update to "TypeScript/TSX files" for accuracy. Also consider clarifying that Biome handles lint/format only, not type errors.
| Add to root `package.json` devDependencies: | ||
|
|
||
| ```json | ||
| "lefthook": "1.11.14" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Plan specifies incorrect lefthook version.
The plan documents version 1.11.14, but package.json installs 2.1.9. Update the plan to match the actual dependency version to avoid confusion.
| ## Implementation Tasks | ||
|
|
||
| - [x] Add `lefthook` dev dependency to root `package.json` (pinned version) | ||
| - [x] Update `postinstall` script in root `package.json` to run `lefthook install` (CI-guarded) after `yarn db:generate` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove stale --reset-hooks-path reference.
The implementation removed this flag (per review.md), but tasks.md still records it. Update to match the actual node -e "process.env.CI || ..." guard.
| pre-commit: | ||
| parallel: false | ||
| commands: | ||
| biome-check: | ||
| run: yarn biome check --write {staged_files} | ||
| stage_fixed: true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if glob is present in lefthook.yml
cat -n lefthook.ymlRepository: hmcts/cath-service
Length of output: 332
🌐 Web query:
Lefthook command glob staged_files default behaviour no glob file filtering documentation
💡 Result:
In Lefthook, the default behavior for file filtering when no glob is explicitly defined is to process all relevant files based on the hook type (e.g., all staged files for pre-commit hooks) [1][2][3]. When you define a glob or exclude filter, Lefthook applies it to the file list before executing your command [3][4]. The specific mechanics are as follows: 1. Automatic Filtering: If you use a file template in your run command (such as {staged_files}) or provide a custom files command, Lefthook uses your specified glob and exclude patterns to filter that list [1][5]. 2. Implicit Filtering without Templates: If you specify a glob or exclude filter but do not include a file template (like {staged_files}) in your run command, Lefthook will still automatically check the relevant file set—{staged_files} for pre-commit or {push_files} for pre-push [1][4]. 3. Command Skipping: In this scenario, if the filtering process results in no files remaining (e.g., no staged files match your glob pattern), Lefthook will automatically skip the execution of that command entirely [1][6][4]. By default, Lefthook uses the gobwas/glob library for pattern matching [1][7]. A key detail to note is that the `` pattern in this library matches one or more directories deep by default; to match files at the top level and nested directories simultaneously, you must either provide separate patterns or enable glob_matcher: doublestar in your configuration [1][7].
Citations:
- 1: https://lefthook.dev/configuration/glob/
- 2: https://lefthook.dev/configuration/run/
- 3: https://lefthook.dev/examples/filters/
- 4: https://lefthook.dev/configuration/exclude/
- 5: https://github.com/evilmartians/lefthook/blob/v1.7.15/docs/configuration.md
- 6: Skip command in pre-commit if no files were changed evilmartians/lefthook#228
- 7: https://github.com/evilmartians/lefthook/blob/master/docs/configuration/glob.md
Add the glob filter back — without it, this hook will run biome check --write against every staged file, not just TS/TSX files.
🎭 Playwright E2E Test Results84 tests 52 ✅ 6m 25s ⏱️ Results for commit f710058. ♻️ This comment has been updated with latest results. |
The postinstall references `node scripts/install-hooks.js` but the file was never committed because `*.js` in .gitignore was catching it. Added `!scripts/install-hooks.js` exception to .gitignore alongside the existing `!e2e-tests/run-with-credentials.js` exception for hand-written CommonJS utility scripts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
| commands: | ||
| biome-check: | ||
| run: yarn biome check --write --no-errors-on-unmatched {staged_files} | ||
| stage_fixed: true |
There was a problem hiding this comment.
Can you ask AI to compare this hook with existing CaTH? https://github.com/hmcts/pip-frontend/blob/master/lefthook.yml.
|
Preview Deployment Successful 🚀Your preview environment is ready:
The environment will be automatically cleaned up when this PR is closed. |
10 STATUS + IMPL changes (closed issue + merged closing PR → verified): REQ-0078 (#301): implemented → verified (PR #458) REQ-0105 (#428): in_progress → verified (PR #749) REQ-0106 (#429): approved → verified (PR #761) REQ-0107 (#431): implemented → verified (PR #701) REQ-0108 (#434): approved → verified (PR #772) REQ-0109 (#436): implemented → verified (PR #727) REQ-0112 (#467): implemented → verified (PR #670) REQ-0124 (#563): approved → verified (PR #782) REQ-0135 (#569): in_progress → verified (PR #748) REQ-0137 (#729): approved → verified (PR #766) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Jira link
#563
Change description
Add pre-commit hook to auto fix lint issues before committing
Testing done
Security Vulnerability Assessment
CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?
Checklist
Summary by CodeRabbit
.dockerignoreto reduce build context bloat.