| name | add-code-quality-hook |
| description | Recipe for wiring a new linter into Opik's unified ๐ Code Quality pipeline (pre-commit + CI). Use when adding a pre-commit-driven linter/formatter to the repo โ enumerates every file that must change (`.pre-commit-config.yaml`, `scripts/precommit-hook-descriptions.tsv`, `scripts/precommit-detect-hooks.py`, `CONTRIBUTING.md`), the non-obvious gotchas (mandatory explicit `files:`, the blank-description trap, `TOOLCHAIN_BY_ID`/`TYPED_IDS`), the retroactive fix-vs-suppress policy, and the pass/fail verification loop. Worked examples: actionlint (live) and hadolint (OPIK-6673). |
Add a Code Quality Hook
Opik runs all linters through one pipeline: pre-commit locally, and the ๐ Code Quality workflow (.github/workflows/code_quality.yml) in CI. CI does not run pre-commit wholesale โ it derives, per PR, the set of hooks that actually have work using scripts/precommit-detect-hooks.py, then runs one CI job per matched linter. That design makes adding a linter a small, fixed set of edits โ but each edit is load-bearing, and skipping one produces a silent gap (the hook runs nowhere, or renders blank in the summary, or provisions the wrong runtime) rather than a loud failure. This skill is the checklist.
The whole recipe is a generalization of two real PRs: actionlint (live in .pre-commit-config.yaml today โ grep it as you read) and hadolint (OPIK-6673, PR #7352 โ the first Docker-image hook, toolchain: none). Read the actionlint hook alongside this doc; it is the canonical, verifiable example.
The wiring files
Each linter touches these four files. Do all four.
1. .pre-commit-config.yaml โ add the hook
Add the upstream hook (repo / rev / id). Pin rev to a tag or SHA โ never a floating ref.
An explicit files: regex is mandatory, not optional. This is the single most common miss. The CI matrix detector (precommit-detect-hooks.py) routes work to hooks by path regex, not by pre-commit's types:. Most upstream hooks (actionlint, hadolint) ship a types:-only match with no files:. If you copy them verbatim, the detector cannot route any file to your hook and CI silently never runs it โ pre-commit locally still works, so the gap hides until something slips through. The detector guards against this: it raises loudly if a hook has types:/types_or: without files: (see precommit-detect-hooks.py lines ~115). So a missing files: fails the detect step rather than regressing silently โ but you still must write the regex.
Write a files: regex that captures exactly the paths the linter should gate. Example (actionlint โ workflows only):
- repo: https://github.com/rhysd/actionlint
rev: v1.7.12
hooks:
- id: actionlint
name: โ๏ธ actionlint โ github workflows
files: ^\.github/workflows/.+\.(yml|yaml)$
The name: is what the reader and the CI summary see โ give it a clear, emoji-prefixed display name matching the house style of the other hooks. The description keyword you add in step 2 is matched against this name, so pick a name containing a stable, distinctive substring (e.g. actionlint, hadolint).
2. scripts/precommit-hook-descriptions.tsv โ add the description row
This TSV is the single source of truth for the per-hook descriptions shown in the Code Quality timing/skipped tables (the CI summary comment). Format: <keyword>\t<description>. The keyword is matched as a substring of the hook display name (name: from step 1).
Miss this and the hook renders with a blank description in the CI summary. Add a row:
hadolint Lint Dockerfiles
Order matters โ most-specific first. Matching is first-substring-wins top-to-bottom, so a more specific keyword must precede any that it contains (the file already does this: ruff-format before ruff). If your keyword is a substring of an existing one, place it above that line.
3. scripts/precommit-detect-hooks.py โ toolchain and content-type maps
Two maps in this file may need an entry. Most new hooks need neither (default is toolchain: none, no content-type narrowing) โ but decide deliberately:
-
TOOLCHAIN_BY_ID โ add your hook id here only if the leg's CI job must provision a heavy runtime: java (shells out to mvn), node-fe / node-ts (shells out to npm). Pre-commit's own hooks self-provision in isolated envs and need nothing; a language: golang hook (actionlint) self-builds; a Docker-image hook (hadolint) runs the image โ all of these are none. code_quality.yml branches its setup steps on matrix.leg.toolchain, so a wrong value means a job either wastes minutes provisioning an unused runtime or lacks the runtime it needs.
-
TYPED_IDS โ add your hook id here only if it carries an upstream types: that narrows its files: match to a content suffix, so the detector doesn't emit a leg that would no-op at runtime. Value is the tuple of suffixes the hook actually acts on (e.g. (".py", ".pyi")). Symptom of a missing entry: the CI timing comment reports fewer ran rows than emitted legs ("detect over-emitted a leg"). If your files: regex is already suffix-precise (like actionlint's \.(yml|yaml)$), you don't need TYPED_IDS.
Rule of thumb by hook type:
| Hook type | TOOLCHAIN_BY_ID | TYPED_IDS |
|---|
| Docker-image linter (hadolint) | none (omit) | usually omit โ make files: suffix-precise |
language: golang/self-built (actionlint) | none (omit) | omit if files: is suffix-precise |
| Python tool (ruff, mypy) | none (omit) | add suffixes if files: is a broad dir regex |
| Shells out to mvn | java | as needed |
| Shells out to npm (FE/TS) | node-fe / node-ts | as needed |
4. CONTRIBUTING.md โ "how to run locally" note
Add a short section alongside the existing GitHub Actions workflows (actionlint) note: what the linter checks, that it runs in the unified ๐ Code Quality workflow and locally via pre-commit, and that make hooks enables it. If the hook needs a local dependency (a Docker daemon for hadolint, etc.), say so; if pre-commit self-provisions it (actionlint builds from source), say that instead.
Retroactive step โ fix the existing violations
Adding a linter to a repo that has never run it will surface pre-existing violations. The gate must be green on day one. Policy, in order of preference:
- Fix the violation in place โ this is the default. Most findings are real and worth fixing.
- Suppress inline, with a reason, next to the code โ only when a fix would be genuinely undesirable. The canonical case: exact-pinning rolling-channel OS packages (e.g.
apt-get install foo=1.2.3), which rots as mirrors move โ the honest answer is an inline # hadolint ignore=DL3008 with a one-line why. Inline suppression is scoped to that one line and visible in review.
- Never a global config-file ignore. A repo-wide ignore (a
.hadolint.yaml ignored: list, an eslint config-level disable) fails open on every future file โ it silently exempts code no one has reviewed. Inline-with-reason keeps the gate strict on everything new.
See the OPIK-6673 discussion for a worked case where some rules genuinely couldn't be honestly fixed and inline-with-reason was the right call.
Verification loop
Before opening the PR, confirm the wiring end-to-end โ don't trust that the four edits compose:
-
Detect emits a leg for a target file. Run the detector against a file the hook should gate and confirm your hook id appears in legs with the right toolchain:
python3 scripts/precommit-detect-hooks.py .pre-commit-config.yaml path/to/target.file
If it lands in skipped instead, your files: regex doesn't match. If it errors about types: without files:, add the files: regex (step 1).
-
Hook passes clean. Run it on the current tree and confirm green (this is also the retroactive check):
pre-commit run <hook-id> --all-files
-
Hook fails on a new violation. Introduce a deliberate violation in a target file and confirm the hook catches it, then revert. A hook that can't fail isn't gating anything.
-
Description resolves. Confirm the summary won't render blank:
echo "<your hook display name>" | python3 scripts/precommit-hook-desc.py
Expect <name>\t<description> โ an empty second column means the keyword row (step 2) is missing or mis-ordered.
Checklist
