fix(archive): anchor component name when matching history files - #5021
Draft
chengzhuzhang wants to merge 1 commit into
Draft
fix(archive): anchor component name when matching history files#5021chengzhuzhang wants to merge 1 commit into
chengzhuzhang wants to merge 1 commit into
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5021 +/- ##
==========================================
+ Coverage 28.48% 28.65% +0.16%
==========================================
Files 262 266 +4
Lines 38490 39024 +534
Branches 8146 8278 +132
==========================================
+ Hits 10964 11181 +217
- Misses 26268 26568 +300
- Partials 1258 1275 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ArchiveBase.get_all_hist_files()builds its match regex by concatenating the component name directly onto the extension pattern:and then uses
re.search, so the component name matches anywhere in the filename — including inside another component's name. The concrete case is E3SM's atmosphere components:eamis a substring ofscream, so theeamspec matches EAMxx history files:casename.scream.h...containseam(inscr-eam) followed by., andeam's extensionh\d*.*\.nc$then matches the rest.In a real case only one atmosphere spec is loaded, so this does not corrupt production archiving. Where it does bite is anywhere several specs are in play at once —
test_st_archive, which loads everycomp_archive_specin the model — so a component's spec can silently "cover" for another component's broken or missing pattern, and the test passes when it should not. That is exactly what happened in E3SM: a wronghist_file_extensionforscreamwent unnoticed becauseeam's pattern matched the files instead (E3SM-Project/E3SM#6190, E3SM-Project/E3SM#8589).Fix
Anchor the component name at a filename boundary — start of name, or preceded by
.,-, or_:The leading
^alternative keeps the existingf.startswith(model)case working, where a file is named after the component rather than the case. The added group is non-capturing, so the existing(\d{4})?/(_d\d{2})?group numbering is unchanged (nothing reads those groups here, but it keeps the diff behavior-neutral).Every separator that appears before a component name in the naming conventions I could find is covered:
casename.eam.h0…,casename.eam_0002.e…,casename.clm2_0002.h0…,ga0xnw.mom6.frc…,casename.mpaso.hist.am…, and the long test-name form…C.fake_testing_only_20160816_164150-20160816_164240.cpl.h.nc.Tests
Two new tests in
CIME/tests/test_unit_xml_archive_base.py, written as pytest functions perAGENTS.md:test_component_name_is_not_matched_as_substring— with both aneamand ascreamspec loaded, each component returns only its own history file. Fails on master, passes with this change.test_component_name_at_start_of_filename— covers the^alternative, i.e. a file named after the component instead of the case.Verified locally:
pytest CIME/tests/test_unit_xml_archive_base.py CIME/tests/test_unit_case_st_archive.py— 14 passed.pytest CIME/tests/ -k unit— 486 passed, 15 skipped, 3 failed; the same 3 (test_unit_doctest::test_lib_docsand twotest_unit_system_tests::test_generate_baseline*) fail identically on unmodified master in this environment, so they are pre-existing and unrelated.black --checkclean on both files.Note
_get_extension()in the same module builds its regex the same way and has the same substring exposure. I left it alone to keep this change small, since it is only reached with a single component's model name at a time; happy to extend it here if you would prefer both fixed together.Analysis and patch produced by Claude (Claude Code).