chore(commits): introduce conventional commits by g-braeunlich · Pull Request #221 · IDEALLab/EngiBench

g-braeunlich · 2026-02-12T16:11:03Z

Description

In this PR, a new commit-msg hook is introduced which also will be checked in CI.
This should enforce conventional commits.

The convention is also mentioned in [CONTRIBUTING.md] and [README.md].

Bonus: a new AGENTS.md is included (to be read by AI agents).

Type of change

Please delete options that are not relevant.

Documentation only change (no code changed)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

I have run the pre-commit checks with pre-commit run --all-files
I have run ruff check . and ruff format
I have run mypy .
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Reviewer Checklist:

The content of this PR brings value to the community. It is not too specific to a particular use case.
The tests and checks pass (linting, formatting, type checking). For a new problem, double check the github actions workflow to ensure the problem is being tested.
The documentation is updated.
The code is understandable and commented. No large code blocks are left unexplained, no huge file. Can I read and understand the code easily?
There is no merge conflict.
The changes are not breaking the existing results (datasets, training curves, etc.). If they do, is there a good reason for it? And is the associated problem version bumped?
For a new problem, has the dataset been generated with our slurm script so we can re-generate it if needed? (This also ensures that the problem is running on the HPC.)
For bugfixes, it is a robust fix and not a hacky workaround.

g-braeunlich · 2026-02-13T08:44:15Z

@SoheylM Also added you as a reviewer as I now also added an AGENTS.md file.

g-braeunlich · 2026-02-13T08:48:13Z

One point of discussion:

I added a pre-commit for commit message linting which works out of the box, but is a bit slow when commiting as it uses a docker container behind the scenes. A smoother workflow could be achieved by switching to a "system" pre-commit, but this would require one manual step more (downloading the gommitlint binary).
What do you think?

SoheylM

Nice addition - conventional commits are a good fit for this project (and others). A few issues to address, one of which, if I am not mistken, is a CI bug that will break on pushes to main.

SoheylM · 2026-02-18T08:04:10Z

.github/workflows/pre-commit.yaml

      - uses: actions/setup-python@v5
      - uses: pre-commit/action@v3.0.1
+
+  conventional-commits:


My cursor agent says this:
Bug: this job inherits the top-level on: push trigger, but uses github.event.pull_request.base.sha/head.sha which are undefined on push events. The git fetch origin main:main will also fail when already on main. Needs a guard:

Suggested change

conventional-commits:

conventional-commits:

if: github.event_name == 'pull_request'

runs-on: ubuntu-latest

Good catch. Addressed in the latest revision: the conventional-commits job now has if: github.event_name == pull_request, so pull_request fields are only evaluated in PR context.

SoheylM · 2026-02-18T08:06:58Z

.github/workflows/pre-commit.yaml

+      - name: Fetch main branch ref (not available in PR context)
+        run: git fetch origin main:main
+
+      - uses: itiquette/gommitlint-action@v0.8.11


This uses v0.8.11. Isn't there a conflict with the local pre-commit hook that uses the Docker image at v0.9.1? In that case, this could cause a commit to pass locally but fail in CI (or vice versa). Worth aligning if this is the case.

Aligned now. CI installs conventional-commit-lint pinned to 0.9.0, matching the local pre-commit hook version and avoiding local versus CI drift.

SoheylM · 2026-02-18T08:07:55Z

.pre-commit-config.yaml

+          See: https://codeberg.org/Itiquette/gommitlint
+        entry:
+          codeberg.org/itiquette/gommitlint:v0.9.1 validate --message-file
+        language: docker_image


Do I understand correctly that this means contributors need a running Docker daemon to commit? That would then be a significant new requirement that isn't mentioned anywhere in the docs. Worth noting in README or CONTRIBUTING, or documenting how to skip the hook if Docker isn't available.

This concern is resolved in the current version. The Docker-based gommitlint hook was removed, and commit-message linting now uses conventional-commit-lint as a Python commit-msg hook.

SoheylM · 2026-02-18T08:08:21Z

AGENTS.md

+## Setup of a local virtual environment
+
+- Run `python -m venv .venv` to create a virtual environment.
+- `. .venv/bin activate` to activate the environment.


Missing slash — this will error out.

Suggested change

- `. .venv/bin activate` to activate the environment.

- `. .venv/bin/activate` to activate the environment.

Fixed in the current AGENTS file: the activation command is now . .venv/bin/activate.

SoheylM · 2026-02-18T08:09:05Z

AGENTS.md

+  - `build`: Changes to build system or dependencies, i.e. [pyproject.toml](pyproject.toml)
+  - `ci`: Changes to CI configuration [.github/](.github)
+  - `chore`: Other changes that don't modify files in the directories [engibench](engibench), [docs](docs) or [tests](tests)
+- `optional scope`: Optional but recommended. Describes the changed component or module name (enclosed in square brackets).


I think conventional commit scopes use parentheses, not square brackets: feat(parser): add ability.

Suggested change

- `optional scope`: Optional but recommended. Describes the changed component or module name (enclosed in square brackets).

- `optional scope`: Optional but recommended. Describes the changed component or module name (enclosed in parentheses).

Updated as suggested. The scope description now uses parentheses, for example feat(parser): ...

SoheylM · 2026-02-18T08:09:40Z

README.md

 pip install -e ".[dev]"
 ```
+
+Our pre-commit config also include a hook to ensure compliance with [conventional commit](https://www.conventionalcommits.org)


Nit: subject-verb agreement.

Suggested change

Our pre-commit config also include a hook to ensure compliance with [conventional commit](https://www.conventionalcommits.org)

Our pre-commit config also includes a hook to ensure compliance with [conventional commit](https://www.conventionalcommits.org)

Fixed in the latest README update: the sentence now uses includes.

SoheylM · 2026-02-18T08:14:27Z

AGENTS.md

@@ -0,0 +1,73 @@
+# AGENTS instructions


Some suggestions to make this file more useful for AI agents:
Project structure - A short section would save significant exploration time (although that may need to be updated if we expect EngiBench structure to change). Something like:

Project structure

engibench/core.py — Problem base class, OptiStep, ObjectiveDirection

engibench/constraint.py — constraint system

engibench/problems/<name>/ — one subpackage per problem, each exports a single Problem subclass

engibench/utils/ — container runtimes, slurm, CLI helpers, problem discovery

tests/ — pytest suite; test_problem_implementations.py parametrizes over all builtin problems

docs/ — Sphinx (MyST), built with cd docs && make dirhtml

Implemented. AGENTS now includes a dedicated Project structure section with core modules, problem layout, tests, and docs build guidance.

g-braeunlich · 2026-02-26T11:58:48Z

@SoheylM @markfuge
In the meantime I should have addressed all above issues.
Regarding the choice of gommitlint, here is what happened:

I opened an issue to see how they react to my suggestion to ship python wheels -> rejected
I started to create my own repo which would build python wheels and provide a smooth integration with pre-commit -> there I realized that gommitlint does not even provide windows builds
I wrote my own linter: https://gitlab.ethz.ch/sis/tools/conventional-commit-lint
Result: using that one now -> no container needed, smooth integration with pre-commit, already tested with this MR

SoheylM · 2026-03-04T07:44:36Z

.github/workflows/pre-commit.yaml

+      - name: Lint all commit messages within the PR
+        run: |
+          python -m pip install --upgrade pip
+          pip install git+https://gitlab.ethz.ch/sis/tools/conventional-commit-lint.git


Could we pin this install to the same version as local pre-commit (rev: 0.9.0)? CI currently installs from Git HEAD, while local hooks are pinned, so upstream changes can create pass-local/fail-CI drift. Suggestion: pip install git+https://gitlab.ethz.ch/sis/tools/conventional-commit-lint.git@0.9.0.

Applied. CI now pins to git+https://gitlab.ethz.ch/sis/tools/conventional-commit-lint.git@0.9.0, matching the local pinned hook version.

SoheylM · 2026-03-04T07:45:31Z

AGENTS.md

+
+### Validation of the commit message
+
+- Use [gommitlint](https://codeberg.org/Itiquette/gommitlint) with the configuration of this repo, [.gommitlint.yml](.gommitlint.yml) as follows:


Same mismatch as CONTRIBUTING: this still instructs gommitlint validate ..., but the project now uses conventional-commit-lint. Please align this with the current toolchain so agent guidance remains accurate.

Aligned. AGENTS now documents conventional-commit-lint and no longer references gommitlint.

SoheylM · 2026-03-04T07:45:32Z

CONTRIBUTING.md

+### Commit Message Format
+
+We follow the rules of [conventional commit](https://www.conventionalcommits.org).
+This will be checked in our CI by [gommitlint](https://codeberg.org/Itiquette/gommitlint) using


This section says CI checks commit messages with gommitlint + .gommitlint.yml, but the workflow now runs conventional-commit-lint. Could we update this text to match the actual CI enforcement so contributors follow the right tool?

Aligned. CONTRIBUTING now points to conventional-commit-lint as the CI commit-message checker.

g-braeunlich · 2026-03-04T09:24:55Z

@SoheylM Thanks again for the careful review!

SoheylM · 2026-03-04T15:43:38Z

Final review pass on latest head 29df9f7:

All previously raised review points are now addressed in code/docs.
CI and required checks are passing, including conventional-commits.
I added per-thread follow-up replies with resolution context.

From my side, this is ready for merge.

g-braeunlich self-assigned this Feb 12, 2026

g-braeunlich force-pushed the conventional-commits branch 5 times, most recently from 2a265b7 to c568a41 Compare February 13, 2026 08:41

g-braeunlich requested review from SoheylM and markfuge February 13, 2026 08:43

SoheylM reviewed Feb 18, 2026

View reviewed changes

g-braeunlich force-pushed the conventional-commits branch 2 times, most recently from f650d60 to 52f2663 Compare February 26, 2026 11:45

SoheylM reviewed Mar 4, 2026

View reviewed changes

chore(commits): introduce conventional commits

29df9f7

g-braeunlich force-pushed the conventional-commits branch from 52f2663 to 29df9f7 Compare March 4, 2026 09:24

markfuge removed their request for review March 4, 2026 15:20

SoheylM merged commit 15b44c5 into main Mar 4, 2026
15 checks passed

SoheylM deleted the conventional-commits branch March 4, 2026 15:44

	- `. .venv/bin activate` to activate the environment.
	- `. .venv/bin/activate` to activate the environment.

	- `optional scope`: Optional but recommended. Describes the changed component or module name (enclosed in square brackets).
	- `optional scope`: Optional but recommended. Describes the changed component or module name (enclosed in parentheses).

	Our pre-commit config also include a hook to ensure compliance with [conventional commit](https://www.conventionalcommits.org)
	Our pre-commit config also includes a hook to ensure compliance with [conventional commit](https://www.conventionalcommits.org)


		### Validation of the commit message

		- Use [gommitlint](https://codeberg.org/Itiquette/gommitlint) with the configuration of this repo, [.gommitlint.yml](.gommitlint.yml) as follows:

Conversation

g-braeunlich commented Feb 12, 2026 • edited by SoheylM Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist:

Reviewer Checklist:

Uh oh!

g-braeunlich commented Feb 13, 2026

Uh oh!

g-braeunlich commented Feb 13, 2026

Uh oh!

SoheylM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Project structure

Uh oh!

Choose a reason for hiding this comment

Uh oh!

g-braeunlich commented Feb 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

g-braeunlich commented Mar 4, 2026

Uh oh!

SoheylM commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

g-braeunlich commented Feb 12, 2026 •

edited by SoheylM

Loading