ESMCI / ESMCI/cime

get_all_hist_files builds an unmatchable regex, silently skipping components in test history comparison

Open
#5,053 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
174
Forks
225
Avg merge
3d 10h
Merged PRs (30d)
18

Description

Found and filed by claude.

## Summary

`ArchiveBase.get_all_hist_files()` constructs a matching regex that can never
match any real file, depending on the text of the `` entry
in `config_archive.xml`. When it misfires, the affected component is silently
dropped from CIME test history comparison — no error, no warning, the test just
compares nothing for that component and passes.

## Where

`CIME/XML/archive_base.py`, in `get_all_hist_files()`:

```python
# Strip any trailing $ if suffix is present and add it back after the suffix
for ext in extensions:
if ext.endswith("$") and has_suffix:
ext = ext[:-1]
string = model + r"\d?_?(\d{4})?(_d\d{2})?\." + ext
if has_suffix:
if not suffix in string: # <-- substring test on the whole regex
string += r"\." + suffix + "$"
if not string.endswith("$"):
string += "$"
```

`_component_compare_copy` reaches this via `copy_histfiles(case, "base", match_suffix="nc")`,
so `suffix="nc"` on the comparison path.

## Mechanism

The `if not suffix in string` guard is a substring test against the entire
partially-built regex, and the append assumes history files are named exactly
`..`. Both assumptions break:

1. **A bare extension gets `\.nc$` glued directly onto it.** For a component
whose files carry additional dot-separated segments after the extension
(stream name, date, qualifiers), the result matches nothing.

2. **Any extension whose text happens to contain the substring `nc`
suppresses the append entirely** — including incidental matches like
`increment`, which then yields a regex ending in `\.increment$` that will
never match a `.nc` file either.

## Reproduction

Standalone, no CIME install needed — this is the loop body verbatim:

```python
import re

def build(model, ext, suffix):
has_suffix = bool(suffix)
if ext.endswith("$") and has_suffix:
ext = ext[:-1]
string = model + r"\d?_?(\d{4})?(_d\d{2})?\." + ext
if has_suffix:
if not suffix in string:
string += r"\." + suffix + "$"
if not string.endswith("$"):
string += "$"
return string

files = [
"CASE.mpaso.hist.am.fmeDerivedFields.0001-01.remapped.nc",
"CASE.mpaso.hist.0001-01-01_00000.nc",
]

for ext in ["hist", r"hist\..*\.nc$"]:
rx = build("mpaso", ext, "nc")
print(f"ext={ext!r}\n regex = {rx}")
for f in files:
print(f" {'MATCH ' if re.compile(rx).search(f) else 'NO MATCH'} {f}")
```

Output:

```
ext='hist'
regex = mpaso\d?_?(\d{4})?(_d\d{2})?\.hist\.nc$
NO MATCH CASE.mpaso.hist.am.fmeDerivedFields.0001-01.remapped.nc
NO MATCH CASE.mpaso.hist.0001-01-01_00000.nc

ext='hist\..*\.nc$'
regex = mpaso\d?_?(\d{4})?(_d\d{2})?\.hist\..*\.nc$
MATCH CASE.mpaso.hist.am.fmeDerivedFields.0001-01.remapped.nc
MATCH CASE.mpaso.hist.0001-01-01_00000.nc
```

And the substring heuristic misfiring on an unrelated word:

```
ext='increment' -> foo\d?_?(\d{4})?(_d\d{2})?\.increment$ # no \.nc$ appended
ext='hist' -> foo\d?_?(\d{4})?(_d\d{2})?\.hist\.nc$ # appended
```

## Impact

The failure is silent. `get_all_hist_files` returns an empty list, so
`copy_histfiles` copies nothing and `compare_histfiles` compares nothing for
that component. An ERS/ERR/other comparison test still reports PASS while
providing no coverage of that component whatsoever. There is no log line saying
"no history files matched" at default verbosity — only the
`logger.debug("Regex is {}")` line, which is off unless debugging.

This is easy to hit because `hist`
is a natural thing to write, and it works correctly on the *archiving* path
(where `suffix` is empty and no `\.nc$` is appended). Only the comparison path,
which passes `match_suffix="nc"`, is broken — so archiving looks fine while
comparison quietly does nothing.

Found in E3SM: `cime_config/config_archive.xml` used a bare `hist` extension for
the `mpaso` and `mpassi` components. Combined with `exclude_testing="true"` on
those same entries, MPAS history files were never compared in tests. Removing
`exclude_testing` alone did not help, because the regex still matched nothing;
the extension had to be rewritten as `hist\..*\.nc$` to work around this.

## Suggested fix

The intent appears to be "ensure the regex is anchored and ends at the file
suffix." A couple of options:

- Replace the `not suffix in string` substring test with a check on whether the
*extension* already accounts for the suffix (e.g. `ext.rstrip("$").endswith(suffix)`),
rather than scanning the whole regex for the letters.
- Rather than concatenating `\.$`, append something that tolerates
intervening dot-separated segments, e.g. `(\..*)?\.$`, so
`....nc` matches as well as `..nc`.

Either way, a warning when a configured `hist_file_extension` matches zero files
in a test comparison would turn this class of problem from silent to obvious.

## Workaround

Write the extension so it already ends in `$` *and* contains the suffix text —
e.g. `hist\..*\.nc$` — which produces the same regex on both the archiving and
comparison paths.

Contributor guide

Open the contributing guide

Research direction

Start in CIME/XML/archive_base.py at get_all_hist_files(), then trace how _component_compare_copy reaches it through copy_histfiles and compare_histfiles. Run the standalone regex reproduction from the issue and inspect the configured hist_file_extension values. Done means comparison matching handles bare and multi-segment extensions without silently returning no files.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.