Add a pre-commit-hook for decay file validation

[jpwgnr]

Summary

Add a first-party decaylanguage-validate command and pre-commit hook for validating EvtGen .dec files with DecFileParser.

The validator reports stable diagnostic codes so downstream experiments can choose their own policy, for example ignoring LHCb-style unmatched CDecay source warnings with --ignore=DLW004.

Changes

• Add decaylanguage.dec.validate with a CLI entry point exposed as decaylanguage-validate.
• Add .pre-commit-hooks.yaml so external repositories can use id: decaylanguage-validate.
• Add the validator to this repository's pre-commit config for bundled/test .dec files, excluding intentional negative fixtures.
• Add selectable diagnostics:
  • DLP001 parse errors
  • DLW001 duplicate Decay blocks
  • DLW002 missing CopyDecay source
  • DLW003 duplicate Decay/CDecay
  • DLW004 missing CDecay source
  • DLW005 self-conjugate CDecay
  • DLW999 unclassified parser warnings
• Add compact pre-commit-style output with source pointers for parse errors, diagnostic summaries, and --max-diagnostics.
• Allow validating individual files or directories recursively.
• Document CLI usage, pre-commit configuration, ignore policy, and expected failure output.
• Add focused validator tests.

Testing

uv run pytest tests/dec/test_validate.py -q
uv run ruff check src/decaylanguage/dec/validate.py tests/dec/test_validate.py
uv run ruff format --check src/decaylanguage/dec/validate.py tests/dec/test_validate.py
uv run --with mypy mypy src/decaylanguage/dec/validate.py
uv run pre-commit validate-config .pre-commit-config.yaml
uv run pre-commit validate-manifest .pre-commit-hooks.yaml
uv run pre-commit run decaylanguage-validate --all-files

Also tested on the local LHCb DecFiles checkout:

uv run decaylanguage-validate --ignore=DLW004 /Users/jpherdi/Documents/CERN_Software/DecFiles

This leaves DLP001, DLW001, and DLW003 diagnostics after ignoring the expected LHCb-style DLW004 findings.

[jpwgnr]

Hi @eduardo-rodrigues,

as a follow up to our discussion yesterday at the Decfiles repo, here a codex-based draft version of the pre-commit-hook.

Before marking this as ready, I will test this for sure a bit more in depth.

But at least it already goes along the lines, I would like it to be eventually.
If you find the time, I would be happy if you could give it some review at a conceptual basis.

Details we can probably discuss once we align on a conceptual vision of the tool.
Thank you!

[eduardo-rodrigues]

Hello. I am overwhelmed these days. It is a good problem to have ... ;-)

I will try and get back to you this week. I am also adding @henryiii to the review for all his expertise.

Thank you very much!

[henryiii]

Here's a quick AI review from Opus 4.8, you can feed this back into codex (rubber duck style) if you wish. Thanks for letting me know what model you are using, so I know which model to review with. :)

From my fairly quick look at the moment, this looks a lot like flake8, which is a good thing. Idea seems good too.

(Needs a rebase, too, of course)

🤖 AI text below 🤖

I've completed a thorough review of PR #598. Here's my assessment.

Review: PR #598 — Add decaylanguage-validate command and pre-commit hook

Verdict: Solid, well-documented draft. The code is clean (mypy --strict clean, ruff clean, 9 tests pass). Architecture is sound — all five DLW* warnings are genuinely emitted during parse() (verified: _find_parsed_decays, _add_decays_to_be_copied, _add_charge_conjugate_decays all run in parse()), and they're correctly captured inside warnings.catch_warnings(record=True) so they don't trip the suite's filterwarnings = error. Main items below are about finishing the draft and reducing a fragile coupling.

Should fix before un-drafting

1. PR description ↔ code mismatch (the core "unfinished" item). The description says "Add the validator to this repository's pre-commit config for bundled/test .dec files." But the final commit (06fbce5 ci: remove local decay validator hook) removed it, and .pre-commit-config.yaml now has no validate entry. So the new hook never runs against decaylanguage's own bundled files (DECAY_LHCB.DEC, DECAY_BELLE2.DEC, the negative fixtures) — it's dogfooded nowhere in CI. Either re-wire it (with the appropriate --ignore/exclude for intentional negative fixtures) or update the description to match.

Robustness / maintainability

2. Fragile string-coupling with dec.py, partially untested. DLW001–DLW005 re-encode the exact warning strings from dec.py as regexes, in two places each (the fullmatch classifier and the _compact_warning_message extractor), with no shared source of truth. If anyone rewords a warning, the code silently degrades to DLW999 — no test would catch it. And tests only cover DLW001, DLW003, DLP001; the DLW002/DLW004/DLW005 classification + compaction paths are entirely untested. Minimum: add a test exercising each rule. Better (larger): have dec.py emit a custom Warning subclass carrying the code, so validate.py reads an attribute instead of pattern-matching prose.

3. Latent iterator-exhaustion bug in validate_files. ignore is materialized (ignored = tuple(ignore)) but additional_decay_models is not, yet it's splatted per-file inside the loop (load_additional_decay_models(*additional_decay_models)). A generator argument would be exhausted after the first file. Harmless from the CLI (argparse gives a list), but inconsistent — tuple(additional_decay_models) once at the top fixes it.

Minor / nits

• AUTHORS.rst — Jan Wagner isn't added (project convention in CLAUDE.md / CONTRIBUTING).
• Nonexistent path → reported as DLP001 parse-error (a FileNotFoundError surfaces through the broad except Exception), rather than a clearer "file not found."
• files: '\.(dec|DEC)$' in the hook only matches all-lower/all-upper, whereas the CLI dir-walk is case-insensitive (.suffix.lower()). Mixed-case .Dec would be skipped by the hook but caught by the CLI. Trivial.

Nice touches worth keeping

Correct grammar-load ordering for --additional-decay-model (called before parse()), os.path.relpath guarded against the Windows cross-drive ValueError, NO_COLOR/tty-aware coloring, proper exit codes via SystemExit(main()), and --ignore codes validated against known codes/prefixes.

Want me to post these as inline comments on the PR, or draft a review comment summarizing them?

[eduardo-rodrigues]

Hello @jpwgnr, @henryiii provided a great review hence I will wait for you to act on it before looking further (at details). Is that OK with you? Thank you and enjoy the weekend, both.

[jpwgnr]

Yes, that is fine. I am on holidays for one and a half week, so not sure when I have the time to act on it. But once I am back, I will do it as one of the first things.

[jpwgnr]

And thanks for the review by the way. It all sounds reasonable.

[eduardo-rodrigues]

Super! My all means do enjoy your hols! No rush. Thank you.

[jpwgnr]

Thanks @henryiii for the detailed review, and @eduardo-rodrigues for waiting for me to address it. The review was very helpful. I have now rebased the branch and worked through all of the points.

For context, I am using GPT-5.6 Codex for the implementation and review iterations and this overview here:

The changes since the review are:

• The validator is now dogfooded in this repository's pre-commit configuration. It checks the bundled and test .dec files, ignores the warning family for this repository's policy, and excludes the fixtures that are intentionally invalid or require custom decay models.
• The fragile warning-text classification has been removed. dec.py now emits dedicated DecFileWarning subclasses carrying the diagnostic codes, and the validator maps those categories directly to DLW001–DLW005. The warning text is only used to produce a compact human-readable message; changing it can no longer silently change the diagnostic code to DLW999.
• Tests now cover the classification and compact output for every warning category (DLW001 through DLW005), in addition to parse errors and the CLI behavior.
• additional_decay_models is materialized once in validate_files, so generators work consistently across multiple files.
• I added myself to AUTHORS.rst.
• Missing or non-regular input paths now produce an explicit DLP001 diagnostic saying that the file does not exist or is not a regular file.
• The hook file matcher is now case-insensitive, matching the recursive CLI behavior for .dec, .DEC, and mixed-case variants.

There were a few further fixes after a deeper review:

• Exceptions with empty or whitespace-only messages no longer crash diagnostic formatting.
• Parse-error carets are aligned with the actual source column, including lines containing tabs.
• --show-ok reports the number of expanded files rather than the number of input paths, so directory input is counted correctly.
• The local dogfooding hook installs the current project with additional_dependencies: [.]. This ensures its decaylanguage-validate console script exists in pre-commit's isolated Python environment and fixes the pre-commit.ci failure.

I verified the result with the focused validator tests, the complete test suite, all configured pre-commit hooks, mypy, Ruff, codespell, git diff --check, and fresh source-distribution and wheel builds. I also recreated the validator hook from a completely fresh pre-commit cache; it installs the local package and passes against all selected decay files.

The PR should now be ready for another review. Thanks again!

[jpwgnr]

Small correction to my previous update: my first attempt at fixing the repository-local hook for pre-commit.ci was not sufficient.

additional_dependencies: [.] appeared to work in my initial local test, but that test was run through uv run pre-commit, so the decaylanguage-validate executable was inherited from the surrounding project environment. In pre-commit.ci's genuinely isolated environment, the executable was still unavailable. I reproduced that behavior locally with a fresh cache before making the follow-up change.

The setup now separates the two use cases explicitly:

• The published hook in .pre-commit-hooks.yaml remains a normal language: python hook with entry: decaylanguage-validate. Downstream repositories install this repository as the hook package, so pre-commit creates the console script in the hook environment as intended.
• The repository-local dogfooding hook uses uv run decaylanguage-validate with language: system. It therefore runs against the current checkout in this repository's GitHub CI, where uv is installed before the pre-commit/prek step.
• The local hook is listed in pre-commit.ci's documented ci.skip setting because hosted pre-commit.ci does not perform this repository-specific setup. The validator is still exercised in the repository's regular CI rather than being silently dropped from CI coverage.

I verified the local hook from a fresh pre-commit cache, and the new PR head (85ad882) now also passes pre-commit.ci.

[jpwgnr]

One final follow-up: I added a downstream-style pre-commit try-repo CI test for the published hook, plus regression coverage for reusable additional-model generators and the DLW999 fallback.

I also corrected the documentation: the feature is now listed under Unreleased rather than v1.0.0, DLP001 covers unreadable as well as invalid files, --additional-decay-model is documented, and the example carets match the current output.

The focused validator suite now has 23 passing tests, and all relevant pre-commit checks pass.

[jpwgnr]

Local speed comparison

Benchmark on macOS arm64 using 9,415 tracked .dec files from LHCb/DecFiles, with the same hook revision and --ignore=DLW004 policy. Versions were pre-commit 4.6.0 and prek 0.4.8.

Runner	Cold cache	Warm cache, mean of 3
pre-commit	8.27 s	3.99 s
prek	5.51 s	3.24 s

prek was about 33% faster from a cold cache (preparing hook repo, building the pre-commit env, etc.) and 19% faster with warm caches. Warm prek runs were 3.20–3.26 s, versus 3.96–4.05 s for pre-commit. Both runners found the same substantive diagnostics; only batching and output order differed.

The validator's parsing work dominates warm runs, so prek provides a useful but not dramatic steady-state improvement. No hook changes are required to use it.

[eduardo-rodrigues]

Hi. Many thanks for the grand update. I will get to it very soon ...

Starting with the bottom comment - the timing seems reasonable to me, I mean with the warm cache. On my laptop, with ProcessPoolExecutor, I typically parse all 9800-ish LHCb dec files in about 35 seconds.

[codecov]

Codecov Report

❌ Patch coverage is 99.54338% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.60%. Comparing base (60b832a) to head (5fd0904).

Files with missing lines	Patch %	Lines
src/decaylanguage/dec/dec.py	94.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #598      +/-   ##
==========================================
+ Coverage   96.68%   97.60%   +0.91%     
==========================================
  Files           5        6       +1     
  Lines         996     1210     +214     
==========================================
+ Hits          963     1181     +218     
+ Misses         33       29       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[eduardo-rodrigues]

At this point, when the pre-commit hook hasn't yet been presented, giving the example output seems a wee out of place. It would be better to exemplify the sort of outputs given by the decaylanguage-validate script. If the same - have not yet checked - then it's just a matter of changing the wording.

[eduardo-rodrigues]

This part would deserve a subsection on its own, similarly to what you have presently in CONTRIBUTING.rst. This way people looking at the package can immediately realise the "good-old functionality" and the fact that validation can now be done trivially in repositories via the hook.

[eduardo-rodrigues]

Similar comment to above - is it really specific to the pre-commit output or rather the output of the script introduced above?

[eduardo-rodrigues]

Ditto - mixing the validation script itself with its usage via the hooks. This is the quickstart, the one page ead before anything, hence introduce the script and maybe link to the section with more details, including the availability of the pre-commit hook.

[eduardo-rodrigues]

The set(...) constructor does not seem necessary here since _RULES_BY_CODE is defined as a set via ´{...}´. You could then get rid of the temporary variable valid_codes and simplify a tiny bit the code.

[eduardo-rodrigues]

If I understand correctly, the "massaging" here is to be able to ignore say all DLW with a simple ignore=DLW? Otherwise, why would you define the prefixes and have valid_codes | prefixes below? Maybe I'm just confused. In any case, if I'm correct, then this regex-ish possibility needs to be documented (apologies if it is already, I did not check yet in the remainder of the PR) and it would make sense to add a comment in this function to be clear of intentions to the code reader.

(There may be other places where a little comment line would help the code reader.)

[eduardo-rodrigues]

Suggest renaming enabled to use_color.

[eduardo-rodrigues]

Why isn't "black"/std colour text returned?

[eduardo-rodrigues]

This test should cover the line above to which you added "pragma: no cover"?

[eduardo-rodrigues]

As hinted at above, I think this is more for the documentation above. A CONTRIBUTING file is for contributors, and the hook is for customers. This would be on the edge if people were developing dec files to add, but then they would run the script to validate, I would imagine.

WDYT?

[eduardo-rodrigues]

Hello again, @jpwgnr. I did a full pass, now. This is truly super 👍. Thank you so much!

I left a few little suggestions, many trivial to resolve. Others are up for discussion and we could also move them to a follow-up contribution if you welcome a quick release with this to then start making tests in LHCb with the pre-commit hook. Just let me know and we adapt.

Again, thank you!

[jpwgnr]

Thanks a lot for your super detailed feedback, I saw it and I hope I find the time to come back to you tomorrow :).

[eduardo-rodrigues]

Thanks a lot for your super detailed feedback, I saw it and I hope I find the time to come back to you tomorrow :).

Top. My privelege - I had to try and be thorough as this is really great.

I see you've been busy on the LHCb Mattermost, no worries ;-).