Skip to content

fix(csv): accept decoded time drum units in round-trip#14

Open
GruppoVeneta wants to merge 9 commits into
ssweber:mainfrom
GruppoVeneta:fix/time-drum-csv-units
Open

fix(csv): accept decoded time drum units in round-trip#14
GruppoVeneta wants to merge 9 commits into
ssweber:mainfrom
GruppoVeneta:fix/time-drum-csv-units

Conversation

@GruppoVeneta

Copy link
Copy Markdown

Summary

Allow CSV parsing and writer round-trip validation to accept every time-drum unit that the binary decoder can already emit (Tms, Ts, Tm, Th, and Td).

Previously, parse_af_call() validated units against _TIME_FC, which only contains the units whose binary function codes are currently implemented. As a result, exporting a CLICK program containing a time drum in seconds, minutes, or hours failed during CSV round-trip validation with:

CSV round-trip validation failed: emitted rows do not reparse:
Unsupported time drum unit: 'Ts'

The CSV parser does not build a binary blob, so it now validates against _DRUM_UNIT_TO_INDEX. Binary encoding remains conservative: _compute_func_codes() still rejects units without known function codes.

Tests

  • Added parser coverage for all five known time-drum units.
  • Preserved rejection of unknown units.
  • Added writer round-trip coverage for Ts, Tm, and Th.
  • Focused CSV suite: 146 passed.
  • Full upstream suite: 695 passed.
  • Ruff and git diff --check: passed.

The issue was reproduced from a real CLICK program export using ClickNick 0.19.3 / laddercodec 0.1.8.

@ssweber

ssweber commented Jul 12, 2026

Copy link
Copy Markdown
Owner

Thanks! I’ll take a look at this in the next day or so and probably merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants