Files
music_library/backlog/completed/ml-187 - Restructure-pre-commit-hooks-to-run-only-necessary-checks-depending-on-change.md
T
Claudio Ortolina c537ea0d21 Backlog cleanup
2026-05-20 13:15:11 +01:00

19 KiB

id, title, status, assignee, created_date, updated_date, labels, dependencies, modified_files
id title status assignee created_date updated_date labels dependencies modified_files
ML-187 Restructure pre-commit hooks to run only necessary checks depending on change Done
pi
2026-05-15 08:41 2026-05-18 15:09
scripts/dev/precommit
docs/project-conventions.md
docs/available-tasks.md
AGENTS.md

Description

Currently pre-commit hooks run the entire verification suite irrespectively of the change. This is inefficient and tedious. Some examples of annoyances.

  1. Changes to /backlog items only need the pretty linter.
  2. Changes to documentation only need the pretty linter.
  3. Changes to the presto application only need to run the (upcoming) presto test suite.
  4. Possibly some other patterns, need to verify the complete repo structure.

Acceptance Criteria

  • #1 Backlog-only changes trigger only prettier on backlog files
  • #2 Docs-only changes trigger only prettier on docs files
  • #3 Elixir changes (without mix.exs/mix.lock) trigger credo, sobelow, gettext, format, and test, but NOT deps.unlock
  • #4 mix.exs or mix.lock changes trigger deps.unlock in addition to other Elixir checks
  • #5 Shell script changes trigger only shellcheck
  • #6 Presto changes trigger only pytest
  • #7 Dockerfile changes trigger validate-docker-image
  • #8 Empty STAGED exits early without errors
  • #9 Combined changes from multiple categories trigger all relevant checks
  • #10 Documentation updated: project-conventions.md has Pre-commit Hooks section, available-tasks.md has updated description

Implementation Plan

Objective

Restructure scripts/dev/precommit (the pre-commit hook driver) to inspect staged files and run only the checks relevant to the change, instead of always running the full verification suite (credo, sobelow, tests, shellcheck, prettier, etc.).

Problem→Solution Mapping

  • Problem: Staging a markdown file in docs/ or backlog/ triggers mix test (~minutes), mix credo, shellcheck, and all other checks — none relevant to markdown.
  • Problem: Staging presto/main.py triggers the full Elixir test suite, but only the presto test suite (pytest) is relevant.
  • Problem: Staging Dockerfile doesn't trigger dev:validate-docker-image — that check runs only in CI, too late.
  • Solution: Categorize staged files by path pattern and gate each check behind grep -qE on the staged file list. No new dependencies.

Approach: Pattern-based conditional execution in the precommit script

Chosen approach: Modify scripts/dev/precommit to read staged file paths (already exported as $STAGED by .git/hooks/pre-commit) and use grep -qE pattern matching to determine which check categories to run.

Alternatives considered and rejected:

  • Lefthook / pre-commit framework: Adds a Node.js dependency, requires config file. Overkill. The project already has a working bash-based pre-commit script; we're just adding conditions.
  • Separate mise tasks per category: Would require chaining multiple mise invocations, adds indirection without benefit. The single script approach keeps debugging simple (bash -x scripts/dev/precommit).
  • Makefile-based: Unnecessary abstraction layer. Bash with set -e is sufficient.
  • Git hook as dispatcher to multiple scripts: The hook already delegates to mise run dev:precommit; keeping all logic in one script is simpler than splitting across hook + multiple scripts.

File Categories and Their Checks

Category grep -E pattern Checks to run
elixir ^(lib/|test/|config/|mix\.exs|mix\.lock|priv/repo/migrations/|priv/gettext/|\.credo\.exs|\.formatter\.exs|\.sobelow-conf) mix format --check-formatted
mix credo --strict
mix sobelow --compact --exit
mix gettext.extract --check-up-to-date
mix deps.unlock --unused
mix test
shell ^(scripts/|\.shellcheckrc) fd . 'scripts/' --exclude '*.hurl' -t file --exec shellcheck --color
assets ^(assets/|\.pi/extensions/.*\.(ts|js|json)$) prettier --check 'assets/css/**/*.css' 'assets/js/**/*.js' '.pi/extensions/**/*.{ts,js,json}' '!.pi/extensions/**/node_modules/**'
docs ^(docs/|README\.md|AGENTS\.md) prettier --check 'docs/**/*.md' 'docs/**/*.livemd' 'README.md' 'AGENTS.md'
backlog ^backlog/ prettier --check 'backlog/archive/**/*.md' 'backlog/completed/**/*.md' 'backlog/tasks/**/*.md' 'backlog/docs/**/*.md'
presto ^presto/ cd presto && mise run test
docker ^(Dockerfile|\.dockerignore|compose\.yaml) mise run dev:validate-docker-image (only if Dockerfile in staged files)

mix deps.unlock --unused sub-gate: Although listed under the elixir category, this check has an additional sub-gate — it runs only when mix.exs or mix.lock is in the staged files, not on every Elixir change. This is because an unused dependency can only be introduced by changing the dependency specification, not by changing application code.

If no files match any category (e.g., git commit --allow-empty), the script exits early with a message.

Implementation Steps

Step 1: Restructure scripts/dev/precommit

Read staged file list from $STAGED (already exported by .git/hooks/pre-commit). Replace the linear execution with conditionally-guarded blocks using the structure:

if echo "$STAGED" | grep -qE '<category-pattern>'; then
  debug_msg "Running <check description>..."
  <check command>
fi

Each guard tests whether any staged file matches its category pattern. Each check block runs independently — set -e ensures fail-fast behaviour (unchanged from current).

Key design decisions:

  • Prettier blocks are split so each glob group runs only when its category has changes (backlog→backlog prettier, docs→docs prettier, assets→assets/pi prettier). This avoids running prettier on docs/ when only backlog/ changed, and vice versa.
  • The assets prettier command excludes node_modules inside .pi/extensions/ via the '!.pi/extensions/**/node_modules/**' negation pattern (prettier supports .gitignore-style negation).
  • The docs prettier command now includes AGENTS.md alongside README.md and docs globs.
  • mix test runs as mix test (same as current) — not partitioned. The pre-commit is for fast feedback; partitioning is a CI concern.
  • mix deps.unlock --unused has a sub-gate on mix.exs or mix.lock changes specifically (not all Elixir changes). See the footnote in the category table.
  • mix gettext.extract --check-up-to-date gates on Elixir changes (it needs compiled code, so any Elixir change could affect gettext extraction).

Update the #MISE description comment at the top of the script to: "Run checks before a commit (conditional on staged file types)".

File: scripts/dev/precommit

Verification:

  • Stage only a backlog/tasks/*.md file: git add backlog/tasks/some-task.md && mise run dev:precommit. Confirm only prettier on backlog runs. Confirm mix test does NOT run.
  • Stage only a presto/main.py file: confirm only presto pytest runs.
  • Stage only a lib/music_library/records.ex file: confirm all Elixir checks run. Confirm mix deps.unlock --unused does NOT run (sub-gate: mix.exs/mix.lock not staged).
  • Stage mix.exs only: confirm mix deps.unlock --unused does run. Confirm other Elixir checks run. Confirm mix test runs (any Elixir change triggers tests).
  • Stage files from all categories simultaneously: confirm all checks run.
  • git commit --allow-empty -m "test": confirm script exits early without errors.

Step 2: Add presto test check

Add a guard for ^presto/ that runs cd presto && mise run test. This uses the presto mise task defined in presto/mise.toml (pytest tests/ -v).

Verification: Stage presto/main.py, run precommit, confirm pytest executes.

Step 3: Add Docker image validation

Add a guard for Dockerfile changes that runs mise run dev:validate-docker-image. This check already exists in CI lint; adding it to pre-commit catches builder image problems before push.

Verification: Stage Dockerfile, run precommit, confirm validate-docker-image executes.

Step 4: Manual integration test

Run through each category in isolation and combined:

# Elixir-only (without mix.exs/mix.lock — deps.unlock should NOT run)
git add lib/music_library/records.ex && mise run dev:precommit

# Elixir dependency change (deps.unlock SHOULD run)
git add mix.exs && mise run dev:precommit

# Docs-only
git add docs/architecture.md && mise run dev:precommit

# AGENTS.md (docs category, must trigger docs prettier including AGENTS.md)
git add AGENTS.md && mise run dev:precommit

# Backlog-only
git add backlog/tasks/some-task.md && mise run dev:precommit

# Presto-only
git add presto/main.py && mise run dev:precommit

# Shell-only
git add scripts/dev/precommit && mise run dev:precommit

# Docker-only
git add Dockerfile && mise run dev:precommit

# All categories
git add lib/ test/ scripts/ docs/ backlog/ presto/ Dockerfile && mise run dev:precommit

Verification: For each case, observe which checks run (via debug_msg output) and confirm only expected checks execute.

Step 5: Update documentation

  • docs/project-conventions.md: Add a "Pre-commit Hooks" subsection under "Workflow" documenting the conditional check behavior and the file-to-check mapping table. Copy the category table from this plan (including the deps.unlock sub-gate footnote). Add a note that CI always runs the full suite unconditionally.
  • docs/available-tasks.md: Update the dev:precommit description from "Run checks before a commit" to "Run checks before a commit (conditional on staged file types)".

Architecture Impact Analysis

Touchpoint Impact
.git/hooks/pre-commit No change — already exports $STAGED and delegates to mise run dev:precommit
scripts/dev/precommit Primary change — restructured from linear to conditional execution
scripts/_helpers.sh No change needed
mise.toml No change — dev:precommit task definition unchanged (runs scripts/dev/precommit)
presto/mise.toml No change — test task already exists
CI workflows No change — CI runs full suite unconditionally (correct for CI)
Supervision tree No impact
PubSub topics No impact
Schemas / contexts No impact
External APIs No impact
Routes / UI components No impact

Performance Profile

Scenario Before After Improvement
Backlog/doc change only ~minutes (full suite) ~2 seconds (prettier) ~100x
Presto change only ~minutes (full suite) ~10 seconds (pytest) ~10x
Shell script change only ~minutes (full suite) ~1 second (shellcheck) ~100x
Dockerfile change only ~minutes (full suite) ~3 seconds (validate) ~50x
Elixir change ~minutes (full suite) ~minutes (full Elixir) No regression
All categories ~minutes (full suite) ~minutes (full suite) No regression

Runtime complexity: O(1) — grep on the staged file list is a single pass. Each check block is independently gated.

Database query patterns: Unchanged. Tests still run the same queries when they execute.

Memory footprint: Unchanged — bash script, no persistent state.

Production Changes

None. This is a local development tooling change. The pre-commit hook only runs on developer machines, never in production or CI.

Documentation Updates

  1. docs/project-conventions.md: Add a "Pre-commit Hooks" subsection under "Workflow" with:

    • Description of conditional check behavior
    • File-to-check mapping table (same as above, including deps.unlock sub-gate footnote)
    • Note that CI always runs the full suite
  2. docs/available-tasks.md: Update dev:precommit description from "Run checks before a commit" to "Run checks before a commit (conditional on staged file types)"

Implementation Notes

Original Draft Notes

Current state: pre-commit hooks run the entire verification suite irrespective of the change.

Annoyances identified:

  1. Changes to /backlog items only need the pretty linter.
  2. Changes to documentation only need the pretty linter.
  3. Changes to the presto application only need to run the presto test suite.
  4. Possibly some other patterns, need to verify the complete repo structure.

Analysis Summary

Current pre-commit flow

  1. .git/hooks/pre-commit gathers staged files via git diff-index --cached --name-only HEAD, exports $STAGED and $MISE_PRE_COMMIT=1, then runs mise run dev:precommit
  2. scripts/dev/precommit runs all checks linearly: shellcheck → credo → sobelow → gettext → format → prettier → deps.unlock → tests
  3. No inspection of $STAGED — all checks run always

File categories identified in repo

  • Elixir: lib/, test/, config/, priv/repo/migrations/, priv/gettext/, mix.exs, .credo.exs, .formatter.exs, .sobelow-conf
  • Shell: scripts/, .shellcheckrc
  • Assets/JS/CSS: assets/, .pi/extensions/
  • Documentation: docs/, README.md, AGENTS.md
  • Backlog: backlog/
  • Presto (MicroPython): presto/
  • Docker/Infra: Dockerfile, .dockerignore, compose.yaml
  • Config/misc: mise.toml, .gitignore (fall through to Elixir checks since these affect the build)

Presto test suite

  • Lives in presto/tests/ (Python, pytest)
  • Runnable via cd presto && mise run test (uses monorepo config root)
  • Already exists and is functional

Docker validation

  • mise run dev:validate-docker-image exists and is used in CI
  • Not currently in pre-commit — adding it closes a CI→local gap

Implementation complete

Step 1: Restructured scripts/dev/precommit with conditional guards for 7 categories: shell, elixir, deps (sub-gate), assets, docs, backlog, presto, docker.

Step 2: Added presto check via (cd presto && mise run test) gated on ^presto/.

Step 3: Added Docker image validation via mise run dev:validate-docker-image gated on docker pattern.

Step 4: Verified pattern matching for all categories and edge cases (empty STAGED, mix.exs sub-gate, combined categories). Ran real integration test: backlog-only triggered only prettier; Elixir triggered credo → sobelow chain.

Step 5: Updated docs/project-conventions.md with Pre-commit Hooks subsection (category table + sub-gate footnote). Updated docs/available-tasks.md dev:precommit description.

Commit

Committed as 974ef45 - ML-187: restructure pre-commit checks to run conditionally. Pre-commit hook verified: ran only shellcheck + docs prettier for the staged shell/docs files (no Elixir checks, no tests).

Also fixed pre-existing prettier issue in AGENTS.md.

Final Summary

Summary

Restructured scripts/dev/precommit to run only relevant checks based on staged file types, replacing the linear "run everything" approach.

Changes

  • scripts/dev/precommit: Replaced linear execution with 7 conditionally-gated check categories (shell, elixir, deps, assets, docs, backlog, presto, docker). Each gated behind grep -qE on $STAGED. Added deps.unlock sub-gate (only when mix.exs or mix.lock staged). Added presto and docker checks that were missing entirely. Added early exit for empty STAGED. Updated MISE description.

  • docs/project-conventions.md: Added "Pre-commit Hooks" subsection under Workflow with file-to-check mapping table and deps.unlock sub-gate footnote.

  • docs/available-tasks.md: Updated dev:precommit description to note conditional behavior.

Verification

Pattern matching verified for all 7 categories in isolation and combined. Real integration test: backlog-only triggered only prettier; Elixir triggered credo→sobelow chain (skipped deps.unlock when no mix.exs/mix.lock staged). Empty STAGED exits early.