Derive series-3 record_type from the binary, not the filename extension #41

Open
opened 2026-09-24 15:09:59 -04:00 by serversdown · 0 comments
Owner

Current behaviour

record_type (Waveform / Histogram / Manual / Event / Combo) is derived entirely from the filename extension's last character — never from file content:

  • minimateplus/event_file_io.py:786 — derive_record_type_from_filename()
  • minimateplus/event_file_io.py:984 — ev.record_type = derive_record_type_from_filename(path.name)
  • sfm/waveform_store.py:346 — re-derives it, overriding the above

The override at waveform_store.py:346 exists because read_blastware_file() is handed a tmp file with a .bw suffix, so the derivation silently falls back to "Waveform". Its own comment says it plainly:

Without this override every BW-imported event lands in the DB with record_type="Waveform" regardless of the actual type.

That is the failure mode this design invites: a filename that doesn't carry the suffix produces a silently wrong record type, not an error.

The binary does carry it

File offset 40 encodes the record type. Verified across every series-3 file in the prod store — 11,605 files, zero exceptions:

byte at offset 40 type files
0x46 Waveform 1,390
0x0A Histogram 10,215

Offset 40 sits at STRT + 18, inside the seven bytes blastware_file.py currently documents as "[zeros] — 7 bytes padding". They are not all padding.

0x46 is a nice corroboration: it is the same value already documented as the wire-protocol waveform record type (SUB 0x0A WAVEHDR length 0x46 = real event, and data[35] = 0x46 in the 5A STRT record — see CLAUDE.md).

Proposed change

  1. Extract the record-type byte in the STRT parse and expose it in strt_fields.
  2. Have read_blastware_file() set ev.record_type from that byte, falling back to the filename derivation when the byte is unrecognised.
  3. Drop the waveform_store.py:346 override once (1) and (2) land — it exists purely to paper over the tmp-suffix problem.
  4. Update the STRT layout comment in blastware_file.py (byte 18 is not padding).

Keep derive_record_type_from_filename() as the fallback — old S338 firmware and any file whose byte 40 is unexpected should still resolve sensibly rather than throw.

Why it matters

Nothing is broken today: in the current ingest path the Blastware filename is authoritative and correct, so this is robustness, not a live bug — hence Priority/Low.

It becomes more pressing if the series-4 direct-download path lands. There the unit is queried over the wire and we invent the filename, so "read the type off the name we just made up" is circular. Series-4 has no equivalent content discriminator at all (a .IDFH and a .IDFW are byte-identical for their first 64 bytes), so its type has to be carried out of the protocol — SUB 0x0A returns length 0x1E for a histogram and 0x00 for a waveform. Series-3 not needing that crutch would be a good precedent.

Verification

The change should be verifiable against the whole prod store: re-derive record_type from byte 40 for all 11,605 series-3 files and confirm it agrees with the filename-derived value in every case. Any disagreement is worth investigating before shipping, not after.

Not in scope

  • The M / E / C type codes have no representatives in the prod store, so their byte-40 values are unknown. The filename fallback covers them.
  • Series-4 record type (tracked with the direct-download work).
## Current behaviour `record_type` (Waveform / Histogram / Manual / Event / Combo) is derived **entirely from the filename extension's last character** — never from file content: - `minimateplus/event_file_io.py:786` — `derive_record_type_from_filename()` - `minimateplus/event_file_io.py:984` — `ev.record_type = derive_record_type_from_filename(path.name)` - `sfm/waveform_store.py:346` — re-derives it, overriding the above The override at `waveform_store.py:346` exists because `read_blastware_file()` is handed a **tmp file with a `.bw` suffix**, so the derivation silently falls back to `"Waveform"`. Its own comment says it plainly: > *Without this override every BW-imported event lands in the DB with `record_type="Waveform"` regardless of the actual type.* That is the failure mode this design invites: a filename that doesn't carry the suffix produces a **silently wrong** record type, not an error. ## The binary does carry it **File offset 40 encodes the record type.** Verified across every series-3 file in the prod store — **11,605 files, zero exceptions**: | byte at offset 40 | type | files | |---|---|---| | `0x46` | Waveform | 1,390 | | `0x0A` | Histogram | 10,215 | Offset 40 sits at **STRT + 18**, inside the seven bytes `blastware_file.py` currently documents as *"[zeros] — 7 bytes padding"*. They are not all padding. `0x46` is a nice corroboration: it is the **same value already documented as the wire-protocol waveform record type** (`SUB 0x0A` WAVEHDR length `0x46` = real event, and `data[35] = 0x46` in the 5A STRT record — see `CLAUDE.md`). ## Proposed change 1. Extract the record-type byte in the STRT parse and expose it in `strt_fields`. 2. Have `read_blastware_file()` set `ev.record_type` from that byte, falling back to the filename derivation when the byte is unrecognised. 3. Drop the `waveform_store.py:346` override once (1) and (2) land — it exists purely to paper over the tmp-suffix problem. 4. Update the STRT layout comment in `blastware_file.py` (byte 18 is not padding). Keep `derive_record_type_from_filename()` as the fallback — old S338 firmware and any file whose byte 40 is unexpected should still resolve sensibly rather than throw. ## Why it matters Nothing is broken today: in the current ingest path the Blastware filename is authoritative and correct, so this is **robustness, not a live bug** — hence Priority/Low. It becomes more pressing if the **series-4 direct-download** path lands. There the unit is queried over the wire and *we* invent the filename, so "read the type off the name we just made up" is circular. Series-4 has no equivalent content discriminator at all (a `.IDFH` and a `.IDFW` are byte-identical for their first 64 bytes), so its type has to be carried out of the protocol — `SUB 0x0A` returns length `0x1E` for a histogram and `0x00` for a waveform. Series-3 not needing that crutch would be a good precedent. ## Verification The change should be verifiable against the whole prod store: re-derive `record_type` from byte 40 for all 11,605 series-3 files and confirm it agrees with the filename-derived value in every case. Any disagreement is worth investigating before shipping, not after. ## Not in scope - The `M` / `E` / `C` type codes have no representatives in the prod store, so their byte-40 values are **unknown**. The filename fallback covers them. - Series-4 record type (tracked with the direct-download work).
serversdown added the Kind/Enhancement
Priority
Low
4
labels 2026-09-24 15:10:00 -04:00
Sign in to join this conversation.