feature/lefthook

[interim17]

Problem

Closes #414 and #251

Solution

Installed lefthook and added config file with pre-commit checks for format, lint, typecheck, and testing.

We can adjust how strict we want pre-commit checks to be....

Some people find that lefthook doesn't play nice with the VSCode git GUI unless the window has been launched from the terminal, due to PATH variables being unavaiable (command npx not found, etc) I launched mine from terminal, and it worked fine.

With lefthook you may prefer committing from the CLI since the print out from lefthook is visible that way (tests running, etc). Without that readout it sometimes seems to be slow or just hanging if you commit from the VS code GUI.

We can go deeper on how to configure it: pre-commit vs pre-push or other options.

Advantages of lefthook over husky are that it can be used in polyglot projects although in this case we are just using it in node context.

• New feature (non-breaking change which adds functionality)

[Copilot]

Pull request overview

This PR adds lefthook as a Git hooks manager to enforce code quality checks before commits. The implementation runs format, lint, typecheck, and test commands automatically during the pre-commit phase.

Changes:

• Added lefthook as a dev dependency and configured it to install via the prepare npm script
• Created lefthook.yml configuration file with parallel pre-commit jobs for formatting, linting, type checking, and testing

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File	Description
package.json	Added lefthook dependency and prepare script to install Git hooks
lefthook.yml	Configured pre-commit hooks to run code quality checks in parallel

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The glob pattern in the lint script has a space before the file extensions which may not match files correctly. It should be \"src/**/*.{tsx,ts}\" without spaces around the comma.

[interim17]

it is defined

[Copilot]

The lint script in package.json uses its own glob pattern and doesn't accept file arguments. This will cause the staged_files parameter to be ignored. Consider updating the lint script to accept file arguments: eslint --quiet --fix

Suggested change

	run: yarn lint {staged_files}
	run: yarn lint

[rugeli]

I ran a bunch of experiments (switching commands with npx, adding fail_text as a job field, tweaking parameters etc. Here's update:

• for today's work, I wasn't able to fully fix vscode GUI popup readability issue. I did get the popup to show real text instead of empty lines, but it still feels a bit misleading.
• good news: the vscode Show Command Output view can be cleaned up using execution_info, this removes the extra blocks and special characters. Now the outputs looks much better.

these are the changes in this file that seemed useful during testing (I didn't submit a sub-pr since the changes are small and not yet optimal):

output:
    - summary
    - failure
    - execution_info
pre-commit:
    ...keep unchanged...

let me know what you think. I can work on this more and open a sub-pr if I find a better fix!

[interim17]

Thanks for working on this! I don't feel precious about this branch and I'm also fine to walk away from this feature if it's taking a lot of time or energy, I don' think it's high priority.

Feel free to commit to this branch or open new branches.

[netlify]

✅ Deploy Preview for cell-catalog ready!

Name	Link
🔨 Latest commit	d27823a
🔍 Latest deploy log	https://app.netlify.com/projects/cell-catalog/deploys/69a8d821166f2d0008d7bbda
😎 Deploy Preview	https://deploy-preview-417--cell-catalog.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[rugeli]

This pr was already in good shape thanks to @interim17. I noticed some readability/formatting issues with the lefthook output when committing via vscode GUI, so I experimented a bit with the config. Since this feels like a useful feature, I think we should move forward and merge it.

updates:

• tweaked output settings in lefthook.yml
• updated typeCheck command

before vs after:

vscode GUI commit popup

Show Command Output

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The format job passes {staged_files} but yarn format is defined as prettier "src/**/*{.tsx, .ts}" --write, so this will still format all TS/TSX under src on every commit. Combined with stage_fixed: true, this can stage unrelated files that weren’t part of the commit. Consider changing the hook to run prettier against {staged_files} directly (or add a dedicated format:staged script that doesn’t include a hard-coded glob).

Suggested change

	run: yarn --silent format {staged_files}
	run: npx prettier --write {staged_files}

[interim17]

This is an interesting point, I could go either way, Ideally we are catching formatting errors before commits, so if there is anything erroneous to flag and catch in the repo, we might as well do so and add it to this commit?

[Copilot]

Same issue as format: yarn lint is defined as eslint "src/**/*{.tsx, .ts}" --quiet --fix, so {staged_files} won’t limit the run to staged files. With --fix + stage_fixed: true, this can modify and stage a large set of unrelated files. Consider updating the hook to run eslint only on {staged_files} (or add a lint:staged script without the hard-coded glob).

Suggested change

	run: yarn --silent lint {staged_files}
	run: npx eslint --quiet --fix {staged_files}

[Copilot]

parallel: true allows format (prettier --write) and lint (eslint --fix) to run concurrently against the same files. Since both jobs can rewrite the same staged files and both use stage_fixed: true, this can lead to nondeterministic results or file write races. Consider running these jobs sequentially (set parallel: false) or avoid having two concurrent fixers touching the same files.

[interim17]

Without having looked into it this seems like a good thing to implement. I noticed some duplicate warnings and wonder if the parallel execution was leading to that.

[interim17]

@rugeli Thanks for working on this! The output is so much better!

But now that it's better, I'm noticing a couple things. I made a bad commit with linting and formatting problems to test it, but instead of flagging those, the warning on the alert was about not having a license field in package.json.

That's a valid thing to flag but it seems like it's printing the first console warning that comes up not the first console warning that would actually constitute a failure vis a vis our config.

[interim17]

@meganrm I'm curious your take on this.

At this point this hook does a great job running our CI checks pre commit and prints nice readable output in the terminal, if you commit from the terminal with git commit -m "..."

The output in vscode if you commit with the GUI is less than perfect, it opens an alert with a rarely useful message and the chance to click "Show Command Output" to see the more detailed message.

I don't mind committing with the CLI, or trusting that VS code users will follow the prompt to look at the output, but it's a point of friction. I do think these pre-commit checks are a really good idea and would like to land on something we can all live with. What do you think?

[interim17]

@meganrm @rugeli made a couple small changes and will merge after a pause, but just to lyk I told the formatting scripts to include json since we were getting churn on package.json

and I added scripts format:files and lint:files to package.json because lefthook's {staged_files} args were being superseded by the globs in format and lint scripts.

if we don't catch that {staged_files} flag its really slow which is a pain if it runs every commit