Skip to content

ci: Add pre-commit hooks for code quality - #12

Open
platzhersh wants to merge 2 commits into
mainfrom
claude/add-pre-commit-hooks-Ebhs4
Open

ci: Add pre-commit hooks for code quality#12
platzhersh wants to merge 2 commits into
mainfrom
claude/add-pre-commit-hooks-Ebhs4

Conversation

@platzhersh

@platzhersh platzhersh commented Jan 10, 2026

Copy link
Copy Markdown
Owner

Implement ADR-0003 to enforce code quality checks before commits:

  • Add .pre-commit-config.yaml with ruff and mypy hooks
  • Add pre-commit>=3.5.0 to dev dependencies
  • Add pre-commit job to CI workflow
  • Update README with pre-commit setup instructions
  • Mark ADR-0003 as Accepted

Hooks run ruff linting/formatting and mypy type checking automatically.

Summary by CodeRabbit

  • New Features

    • Pre-commit hooks now enforce code formatting, linting, and type checking before commits and are run in CI prior to linting.
  • Documentation

    • Added installation and usage instructions for pre-commit hooks.
    • Updated ADR to reflect that the pre-commit strategy is accepted and implemented.
  • Chores

    • Added pre-commit to development dependencies.

✏️ Tip: You can customize this high-level summary in your review settings.

Implement ADR-0003 to enforce code quality checks before commits:
- Add .pre-commit-config.yaml with ruff and mypy hooks
- Add pre-commit>=3.5.0 to dev dependencies
- Add pre-commit job to CI workflow
- Update README with pre-commit setup instructions
- Mark ADR-0003 as Accepted

Hooks run ruff linting/formatting and mypy type checking automatically.
@coderabbitai

coderabbitai Bot commented Jan 10, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds pre-commit infrastructure: config file with ruff and mypy hooks, CI job running pre-commit on GitHub Actions, README setup instructions, ADR updated to Accepted, and pre-commit added to dev dependencies.

Changes

Cohort / File(s) Summary
CI Configuration
.github/workflows/ci.yml
Added a new pre-commit job to CI that checks out code, sets up Python 3.12, installs pre-commit, and runs hooks across all files; runs before existing lint job.
Pre-commit Configuration
.pre-commit-config.yaml
New pre-commit config adding ruff-pre-commit (v0.8.6) with ruff/ruff-format hooks and mirrors-mypy (v1.14.1) mypy hook with extra dependencies and file filter for src/openehr_sdk/.
Project Metadata
pyproject.toml
Added pre-commit>=3.5.0 to development dependencies.
Documentation & ADR
README.md, docs/adr/0003-pre-commit-hooks-for-code-quality.md
README: added installation and usage instructions for pre-commit. ADR: status changed to Accepted and updated to reflect implemented configuration, versions, and CI integration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble at code, then hop in delight,
Ruff straightens my whiskers, mypy keeps sight,
Hooks snap like twigs on a moonlit night,
Pre-commit hums — the repo feels right. 🐇✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding pre-commit hooks to enforce code quality checks as part of the CI workflow.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch claude/add-pre-commit-hooks-Ebhs4

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 9-26: The workflow defines a "pre-commit" job that runs hooks but
other jobs (e.g., jobs named "lint", "type-check", "test") are not dependent on
it, so they may run in parallel and bypass the pre-commit gating; update each
downstream job that should be gated (for example the "lint", "type-check", and
"test" jobs referenced in the same file) to include needs: pre-commit so they
wait for the pre-commit job to finish and fail fast if pre-commit fails.

In @docs/adr/0003-pre-commit-hooks-for-code-quality.md:
- Around line 5-8: The ADR's Status header is now "Accepted" but the Decision,
Notes, and "Proposed Configuration" sections still read like tentative text;
update those sections to reflect the implemented choices and current tool
versions, remove the "Status will be updated…" note, and ensure the
Decision/Notes/Proposed Configuration headings and any version strings
accurately describe the final configuration implemented.
🧹 Nitpick comments (1)
.pre-commit-config.yaml (1)

6-20: Pin mypy hook additional_dependencies with exact versions

Version ranges (>=) in additional_dependencies can drift independently and cause unexpected CI breaks. Pin each dependency with == for reproducible environments:

additional_dependencies: [pydantic==2.0, httpx==0.25, types-defusedxml==0.7]

(Note: ruff v0.8.6 and mypy v1.14.1 are outdated; consider updating to latest stable versions if compatible with your project.)

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a4d0aa and 560fa9c.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • .pre-commit-config.yaml
  • README.md
  • docs/adr/0003-pre-commit-hooks-for-code-quality.md
  • pyproject.toml
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: platzhersh/oehrpy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T11:06:41.671Z
Learning: Run ruff linter and formatter as part of development workflow for code quality enforcement
📚 Learning: 2026-01-09T11:06:41.671Z
Learnt from: CR
Repo: platzhersh/oehrpy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T11:06:41.671Z
Learning: Run ruff linter and formatter as part of development workflow for code quality enforcement

Applied to files:

  • README.md
  • .pre-commit-config.yaml
📚 Learning: 2026-01-09T11:06:41.671Z
Learnt from: CR
Repo: platzhersh/oehrpy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T11:06:41.671Z
Learning: Applies to src/openehr_sdk/serialization/flat.py : Implement FLAT Format support with FlatPath, FlatContext, and FlatBuilder components for hierarchical composition flattening

Applied to files:

  • README.md
📚 Learning: 2026-01-09T11:06:41.671Z
Learnt from: CR
Repo: platzhersh/oehrpy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T11:06:41.671Z
Learning: Applies to src/openehr_sdk/rm/*.py : Generated code should allow N801, N817, E402, SIM105 linting rule violations in src/openehr_sdk/rm/*.py per pyproject.toml configuration

Applied to files:

  • README.md
🔇 Additional comments (3)
pyproject.toml (1)

35-43: Good add: pre-commit in dev extras; align CI install with this constraint
Nice to have pre-commit>=3.5.0 here; I’d mirror this in CI (currently pip install pre-commit installs whatever is latest).

README.md (2)

261-281: Docs update is clear and matches the intended workflow
Setup + manual run + --no-verify escape hatch are all documented well.


329-336: Good: ADR-0003 is now linked from README
Makes the “why” discoverable for contributors.

Comment thread .github/workflows/ci.yml
Comment on lines 9 to +26
jobs:
pre-commit:
name: Pre-commit Hooks
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.12"

- name: Install pre-commit
run: pip install pre-commit

- name: Run pre-commit hooks
run: pre-commit run --all-files

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

pre-commit doesn’t actually run “before lint” without needs:
As written, GitHub Actions can run jobs in parallel; if you want pre-commit gating, add needs: pre-commit (at least on lint/type-check/test).

Proposed patch
 jobs:
   pre-commit:
     name: Pre-commit Hooks
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4

       - name: Set up Python
         uses: actions/setup-python@v5
         with:
           python-version: "3.12"

+      - name: Upgrade pip
+        run: python -m pip install --upgrade pip
+
       - name: Install pre-commit
-        run: pip install pre-commit
+        run: pip install "pre-commit>=3.5.0"
+
+      - name: Cache pre-commit
+        uses: actions/cache@v4
+        with:
+          path: ~/.cache/pre-commit
+          key: pre-commit-${{ runner.os }}-${{ hashFiles('.pre-commit-config.yaml') }}

       - name: Run pre-commit hooks
-        run: pre-commit run --all-files
+        run: pre-commit run --all-files --show-diff-on-failure

   lint:
     name: Lint
     runs-on: ubuntu-latest
+    needs: pre-commit
     steps:
       - uses: actions/checkout@v4
@@
   type-check:
     name: Type Check
     runs-on: ubuntu-latest
+    needs: pre-commit
     steps:
       - uses: actions/checkout@v4
@@
   test:
     name: Unit Tests
     runs-on: ubuntu-latest
+    needs: pre-commit
     steps:
       - uses: actions/checkout@v4

Also applies to: 27-48, 49-67

🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 9 - 26, The workflow defines a
"pre-commit" job that runs hooks but other jobs (e.g., jobs named "lint",
"type-check", "test") are not dependent on it, so they may run in parallel and
bypass the pre-commit gating; update each downstream job that should be gated
(for example the "lint", "type-check", and "test" jobs referenced in the same
file) to include needs: pre-commit so they wait for the pre-commit job to finish
and fail fast if pre-commit fails.

Comment thread docs/adr/0003-pre-commit-hooks-for-code-quality.md
- Change "Proposed Configuration" to "Configuration" with actual versions
- Update ruff to v0.8.6 and mypy to v1.14.1
- Change tentative language to past tense
- Remove "Implementation Timeline" and outdated "Notes" sections
- Add types-defusedxml to mypy dependencies

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @docs/adr/0003-pre-commit-hooks-for-code-quality.md:
- Line 29: Replace the bare URL in the sentence referencing the pre-commit
framework with markdown link syntax: change "the `pre-commit` framework
(https://pre-commit.com/)" to use a linked label like "`pre-commit` framework
[pre-commit](https://pre-commit.com/)" (or inline
`[pre-commit](https://pre-commit.com/)`) so the URL is not bare and MD034 is
satisfied.
- Line 38: Update the pinned pre-commit hook versions: change the Ruff rev
string currently shown as "rev: v0.8.6" to "rev: v0.14.11" and update the pinned
mypy version (the mypy hook entry referenced around line 45) to "1.19.1"; locate
the pre-commit config entries (the Ruff rev line and the mypy hook/version
field) and replace the old version strings with these new versions.
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 560fa9c and 1725fab.

📒 Files selected for processing (1)
  • docs/adr/0003-pre-commit-hooks-for-code-quality.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T11:06:41.671Z
Learnt from: CR
Repo: platzhersh/oehrpy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T11:06:41.671Z
Learning: Run ruff linter and formatter as part of development workflow for code quality enforcement

Applied to files:

  • docs/adr/0003-pre-commit-hooks-for-code-quality.md
🪛 LanguageTool
docs/adr/0003-pre-commit-hooks-for-code-quality.md

[uncategorized] ~71-~71: The official name of this software platform is spelled with a capital “H”.
Context: ...th mypy on SDK code 3. Added CI job in .github/workflows/ci.yml to run hooks on all f...

(GITHUB)

🪛 markdownlint-cli2 (0.18.1)
docs/adr/0003-pre-commit-hooks-for-code-quality.md

29-29: Bare URL used

(MD034, no-bare-urls)

🔇 Additional comments (1)
docs/adr/0003-pre-commit-hooks-for-code-quality.md (1)

31-51: Comprehensive and well-structured implementation documentation.

The ADR effectively documents the decision, concrete configuration with pinned versions, and clear setup instructions. The configuration appropriately targets SDK code with files: ^src/openehr_sdk/, uses auto-fix flags, and includes necessary type stubs (types-defusedxml). The CI integration ensures enforcement across all branches.

Also applies to: 53-83

## Decision

We will investigate and implement pre-commit hooks using the `pre-commit` framework (https://pre-commit.com/) to automatically run code quality checks before allowing commits.
We implemented pre-commit hooks using the `pre-commit` framework (https://pre-commit.com/) to automatically run code quality checks before allowing commits.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use markdown link syntax instead of bare URL.

Line 29 violates MD034 (no-bare-urls). Convert the inline URL to proper markdown link format for consistency with markdown standards.

✏️ Proposed fix
-We implemented pre-commit hooks using the `pre-commit` framework (https://pre-commit.com/) to automatically run code quality checks before allowing commits.
+We implemented pre-commit hooks using the `pre-commit` framework ([pre-commit.com](https://pre-commit.com/)) to automatically run code quality checks before allowing commits.
📝 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.

Suggested change
We implemented pre-commit hooks using the `pre-commit` framework (https://pre-commit.com/) to automatically run code quality checks before allowing commits.
We implemented pre-commit hooks using the `pre-commit` framework ([pre-commit.com](https://pre-commit.com/)) to automatically run code quality checks before allowing commits.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

29-29: Bare URL used

(MD034, no-bare-urls)

🤖 Prompt for AI Agents
In @docs/adr/0003-pre-commit-hooks-for-code-quality.md at line 29, Replace the
bare URL in the sentence referencing the pre-commit framework with markdown link
syntax: change "the `pre-commit` framework (https://pre-commit.com/)" to use a
linked label like "`pre-commit` framework [pre-commit](https://pre-commit.com/)"
(or inline `[pre-commit](https://pre-commit.com/)`) so the URL is not bare and
MD034 is satisfied.

repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.9 # Use latest stable version
rev: v0.8.6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

latest version of ruff python linter 2026

💡 Result:

The latest Ruff release is version 0.14.11 (published Jan 8, 2026). [1][2]

Sources:
[1] PyPI ruff release listing (ruff‑0.14.11 upload Jan 8, 2026).
[2] Ruff GitHub releases.


🌐 Web query:

latest version of mypy python type checker 2026

💡 Result:

The latest mypy release is version 1.19.1 (uploaded to PyPI on December 15, 2025). [1][2]

Sources:

  • PyPI package page showing mypy 1.19.1 (upload date Dec 15, 2025). [1]
  • Official mypy release notes / blog announcing the 1.19 series. [2]

Update Ruff and mypy to current versions.

Ruff v0.8.6 and mypy v1.14.1 are significantly outdated. As of January 2026, the latest versions are Ruff 0.14.11 (released January 8, 2026) and mypy 1.19.1 (released December 15, 2025). Update the pinned versions in the pre-commit configuration at lines 38 and 45.

🤖 Prompt for AI Agents
In @docs/adr/0003-pre-commit-hooks-for-code-quality.md at line 38, Update the
pinned pre-commit hook versions: change the Ruff rev string currently shown as
"rev: v0.8.6" to "rev: v0.14.11" and update the pinned mypy version (the mypy
hook entry referenced around line 45) to "1.19.1"; locate the pre-commit config
entries (the Ruff rev line and the mypy hook/version field) and replace the old
version strings with these new versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants