[ggmarshall]

Systematic bug-fix and error-handling sweep across the package, following the same review recently applied to legend-dataflow.

Crash bugs

• par-geds-hit-aoe crashed with NameError on every run without --override-files; same undefined-in-branch class in hit/ecal.py (hit_dict/in_results_dict without --in-hit-dict) and dsp/eopt.py (optimiser plot objects when run_eopt: false).
• hit/qc.py crashed for any channel with <4000 clean events (rng.choice(..., replace=False)).
• hit/ecal.py guess-offset lookup subscripted a string default (.get(key, "fallback")["parameters"]) — the default is a fallback key into hit_dict.
• build-tier-hit-single-channel passed an empty {} to build_hit and discarded the computed config (silently useless output); it now passes hit_config=hit_cfg and extracts ["pars"] like the multi-channel path.
• --table-map (build-tier-dsp/hit) and --qbb-grid-path (par-geds-dsp-eopt) are now required=True — the rules always pass them and omitting them crashed with TypeError.
• dataflow -v with no subcommand raised AttributeError; it now prints help and exits 1.
• cfgtools.get_channel_config evaluated the __default__ lookup eagerly, raising KeyError even when the channel was present.
• filedb.py ran scan_files twice, with the second call outside the error-context wrapper.

Wrong results

• hit/ecal.py get_median/get_err had inverted conditions — they returned NaN for well-populated time bins and values for near-empty ones, effectively blanking pulser-stability monitoring.
• alias_table tested struct membership by substring, so adding an alias whose name is a substring of an existing field (e.g. raw next to raw_blind) silently skipped the datatype registration.
• as_ro on Path inputs returned only the rewritten basename, dropping all directory components.
• hit/lq.py results are now stored per timestamp, mirroring hit/aoe.py.

Logging: crashes now reach log files as ERROR records

build_log's excepthook wrote raw tracebacks via traceback.print_exception to a handler stream discovered from the named logger — which has no handlers of its own in the dictConfig path (the FileHandler sits on root), so uncaught tracebacks fell back to stdout and never reached the per-job log file; even when they did, they carried no ERROR token for downstream log scanning. The excepthook now emits log.error("uncaught exception", exc_info=...) (formatted, severity-tagged, propagates to the root FileHandler), delegates KeyboardInterrupt to the default hook, and the fallback logger name is honoured in the dictConfig branch.

New shared helpers (legenddataflowscripts.utils)

expand_filelist, check_pulser_mask, require_config_keys, get_rule_config, and the fixed get_channel_config (now with an optional name= for error context). Adopted across the hit/dsp scripts: filelist arguments fail with named errors on empty/missing input, every mask[threshold_mask] is preceded by a length check that names the table and suggests mismatched pulser/input filelists, and the run_*/calibration config contracts are validated up front naming the channel and config file. These helpers are also intended to replace the duplicated copies in legend-dataflow (companion change there; its pin will need a bump to the next release).

Tests (18 → 49)

• Console-script smoke test: imports every [project.scripts] entry point and runs --help (17 scripts) — catches broken imports, entry-point typos, and argparse regressions.
• Unit tests for all new error paths: helper behaviors, the get_channel_config eager-default regression, as_ro Path regression, dataflow no-subcommand, alias_table membership on a synthetic HDF5 file, and the build_log excepthook (asserts the ERROR record + traceback lands in the log file).

Noted, not changed

• eval() of config strings in aoe/lq/ecal/dplms/eopt — replacing these needs a whitelist built from the names production configs actually use; follow-up.
• CLI contract note: aoe/lq/ecal previously treated files[0] as a filelist unconditionally; with expand_filelist a single argument is only expanded when it has a .filelist suffix (the Snakemake rules always pass .filelist paths).

Verification

• pytest: 49 passed (warnings-as-errors)
• ruff check / ruff format --check: clean
• dataflow -v: prints help, exits 1, no traceback

---

Per AI_POLICY.md: this contribution was drafted with AI assistance (Claude Code) and reviewed by the submitter.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR performs a broad error-handling and correctness sweep across legenddataflowscripts, introducing shared helper utilities, hardening multiple CLI scripts against crash/logic edge cases, and significantly expanding test coverage to prevent regressions (including ensuring uncaught exceptions are logged as ERROR records into job log files).

Changes:

• Add shared utilities in legenddataflowscripts.utils (filelist expansion, config validation, rule config lookup, pulser-mask validation) and adopt them across hit/dsp scripts.
• Fix multiple crash paths / logic bugs (e.g., pulser mask length mismatches, as_ro Path handling, eager default lookup in get_channel_config, required CLI args).
• Add extensive tests (helper unit tests + console-script smoke tests + logging excepthook test) and adjust lint tooling config.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_utils_helpers.py	New unit tests covering new helpers (expand_filelist, require_config_keys, get_rule_config, pulser-mask checks, logging excepthook, alias membership).
tests/test_util.py	Adds regression test for as_ro preserving Path directory components.
tests/test_execenv.py	Adds tests for dataflow -v with no subcommand and missing execenv system selection.
tests/test_console_scripts.py	New smoke test importing all [project.scripts] entry points and invoking --help.
src/legenddataflowscripts/workflow/utils.py	Fixes as_ro to rewrite full Path string instead of only basename.
src/legenddataflowscripts/workflow/filedb.py	Improves ignore-keys parsing, avoids duplicate scanning, and simplifies ignore handling in timestamp validation.
src/legenddataflowscripts/workflow/execenv.py	Makes dataflow robust with no subcommand and adds _select_execenv with clearer errors.
src/legenddataflowscripts/utils/pulser_removal.py	Adds missing-key error for pulser masks and introduces check_pulser_mask.
src/legenddataflowscripts/utils/log.py	Updates excepthook to log uncaught exceptions as ERROR records (and delegate KeyboardInterrupt).
src/legenddataflowscripts/utils/files.py	New expand_filelist helper for .filelist expansion + validation.
src/legenddataflowscripts/utils/cfgtools.py	Fixes eager-default get_channel_config behavior and adds config-validation and rule-config helpers.
src/legenddataflowscripts/utils/alias_table.py	Fixes alias struct membership detection by parsing struct fields (avoids substring false positives).
src/legenddataflowscripts/utils/init.py	Exposes newly added shared helpers via legenddataflowscripts.utils.
src/legenddataflowscripts/tier/hit.py	Hardens CLI args (--table-map required, --pars-file default) and fixes single-channel config passing.
src/legenddataflowscripts/tier/dsp.py	Makes --table-map required and simplifies table-map logic.
src/legenddataflowscripts/par/geds/hit/qc.py	Adds helper usage (filelists, pulser-mask validation), fixes sampling crash for <4000 events, and tightens fft config handling.
src/legenddataflowscripts/par/geds/hit/lq.py	Adds upfront config validation and stores results per timestamp consistently; adopts helper utilities.
src/legenddataflowscripts/par/geds/hit/ecal.py	Fixes median/err threshold logic, adopts helpers, and hardens missing-dict initialization and guess-offset handling.
src/legenddataflowscripts/par/geds/hit/aoe.py	Adds upfront config validation, helper usage, and fixes undefined override-dict path.
src/legenddataflowscripts/par/geds/dsp/pz.py	Adds upfront config validation, helper usage, and clearer pulser-file requirements.
src/legenddataflowscripts/par/geds/dsp/nopt.py	Adds config validation and simplifies output dict writing.
src/legenddataflowscripts/par/geds/dsp/evtsel.py	Adds config validation, helper usage, and clearer pulser-file requirements.
src/legenddataflowscripts/par/geds/dsp/eopt.py	Makes --qbb-grid-path required, validates config, and avoids referencing optimiser plots when optimisation didn’t run.
src/legenddataflowscripts/par/geds/dsp/dplms.py	Adds upfront config validation for run_dplms.
pyproject.toml	Updates pylint target Python version to 3.11.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gipert]

i think this kind of "inputs validation/sanitisation" stuff is what we need and we should have more

[ggmarshall]

agreed, I'd merge this and then expand more