FIX: NEXRAD Level 2 LDM stride drops MSG_31 records (#376)

[aladinor]

Summary

Closes #376.

NEXRADRecordFile.init_record hard-codes a 120-message stride between LDM-compressed records. NEXRAD ICD 2620010J §7.3.4 mandates a variable count: each LDM Compressed Record contains "120 radial messages (type 31) plus 0 or more RDA Status messages (type 2)". When MSG_2 is interleaved alongside the 120 MSG_31s, the mod-120 budget exhausts on 118 MSG_31 + 2 MSG_2, and xradar jumps to the next LDM, dropping the trailing MSG_31s.

-    def get_ldm(recnum):
-        if recnum < 134:
-            return 0
-        mod = ((recnum - 134) // 120) + 1   # bug: hard-codes 120 msgs/LDM
-        return mod
-    ...
-    rnum = (recnum - 134) % 120              # bug: same assumption
-    start = self.record_size + self.filepos if rnum else 0

Reproducer

s3://unidata-nexrad-level2/2023/06/29/KILX/KILX20230629_154426_V06. LDM 49 of this file contains 122 messages (120 MSG_31 + 2 MSG_2). xradar reports sweep_10 = 358; the file actually has 360 (verified by direct bz2.decompress byte-walk and by danielway/nexrad).

	Total	sweep_10
xradar 0.12.0 (today)	6838	358
danielway/nexrad 1.0.0-rc.7	6840	360
Direct bz2.decompress byte-walk	6840	360
xradar with this PR	6840	360

Implementation

Replace the synthetic mod-120 stride with a byte-walk that detects end-of-LDM by buffer length, not message count.

• _load_ldm(idx) — decompress + lazy-cache LDM idx (extracted from the previous inline block, no behavior change).
• _resolve_compressed_data_position(recnum) — return (ldm_idx, byte_offset) via:
  • cache hit (non-sequential jumps, e.g. get_data per-moment replay),
  • first compressed call (cold or post-metadata) → LDM 1 byte 0,
  • sequential advance from self.filepos + self.record_size, crossing into the next LDM when the current buffer can no longer hold a message header.
• init_record — three-branch dispatch (metadata / uncompressed-data / compressed-data) with a shared trailer.

Test plan

• All 104 tests in tests/io/test_nexrad_level2.py pass
• Smoke test on KILX: sweep_10 = 360, total = 6840
• DataTree end-to-end: sweep_10.azimuth = 360, max azimuth gap 1.044°
• Extended test_bz2_compressed_buffer_path_real to pin the cold-entry → sequential-walk transition (+2 lines)
• black --check and ruff check clean

Note on regression test

None of the existing fixtures (KATX, KLIX, KLBB, KLBB.gz) trigger this bug — only metadata LDMs in those files have >120 messages, which goes through the (correct) metadata path. A targeted regression test against KILX20230629_154426_V06 is tracked as a follow-up once the file lands in open-radar-data.

Spec citations

• ICD 2620010J §7.3.4 (LDM Compressed Record): the per-LDM payload is variable — 120 radials + 0 or more RDA Status messages.
• ICD 2620010J §7.3.6.1.1: 2432-byte fixed framing for non-MSG_31/29 messages — already handled by get_end(). The within-LDM walk was always correct; only the cross-LDM stride was wrong.

[codecov]

Codecov Report

❌ Patch coverage is 96.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.20%. Comparing base (d723857) to head (d97f9b3).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
xradar/io/backends/nexrad_level2.py	96.66%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #377      +/-   ##
==========================================
+ Coverage   94.18%   94.20%   +0.02%     
==========================================
  Files          29       29              
  Lines        6416     6440      +24     
==========================================
+ Hits         6043     6067      +24     
  Misses        373      373              

Flag	Coverage Δ
notebooktests	0.00% <0.00%> (ø)
unittests	94.20% <96.66%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

[kmuehlbauer]

@aladinor So for proper testing, we would need to have to import this specific file into open-radar-data?

[aladinor]

@kmuehlbauer Yes — to pin the actual symptom of #376 against the file that exposed it (KILX20230629_154426_V06, where LDM 49 has 120 MSG_31 + 2 MSG_2 = 122 messages and sweep_10 should be 360 rays), we need that fixture in open-radar-data.

What this PR currently has:

• 6 unit tests covering all the new code paths in init_record / _load_ldm / _resolve_compressed_data_position (sequential walk, cache hit, first compressed call, EOF, invariant violations) — patch coverage 100% on the changed lines per Codecov
• One contract pin in test_bz2_compressed_buffer_path_real that the cold-entry at recnum=134 transitions cleanly into a sequential walk

What's missing (waiting for fixture):

• A test that loads the actual KILX file and asserts len(msg_31_header[10]) == 360 and total = 6840

I'll open a PR against openradar/open-radar-data to add the file (10 MB) and then send a follow-up here that adds the targeted regression assertion. None of the existing fixtures (KATX / KLIX / KLBB) trigger this bug — only metadata LDMs in those files have >120 messages, which goes through the (correct) metadata path. So the open-radar-data upload is the only realistic route to a true regression test.

[kmuehlbauer]

@aladinor open-radar-data v0.8.0 release is out. Please add the necessary changes. Thanks!

[aladinor]

@kmuehlbauer KILX20230629_154426_V06 just landed in open-radar-data 0.8.0 — added the targeted regression test in 21fa5fb:

• New nexradlevel2_kilx_ldm_stride_file session fixture in tests/conftest.py
• New test_kilx_over_120_ldm_decodes_all_msg31 asserting per-sweep ray counts [720]*6 + [360]*7, total = 6840, and sweep_10 == 360
• Bumped open-radar-data >= 0.8.0 in requirements_dev.txt, environment.yml, ci/unittests.yml, and ci/notebooktests.yml

LDM 49 of that file contains 120 MSG_31 + 2 MSG_2 = 122 messages, which is exactly the configuration that triggered the mod-120 stride bug. The test fails on the unfixed code (sweep_10 == 358, total = 6838) and passes with the fix.

[kmuehlbauer]

@aladinor We still need to wait for conda forge. I'll add the new version now

[aladinor]

@kmuehlbauer, let me know if you have additional comments

[kmuehlbauer]

@aladinor There is quite some code shuffle here ;-) I'll have another look next week.

One thing, this nexrad notebook doesn't work when the fix is applied: https://openradar-docs--377.org.readthedocs.build/projects/xradar/en/377/notebooks/nexrad_read_chunks.html.