createEventsTable argument change silently rebinds old calls #336

Note on provenance: This issue was found by Claude Opus 5 while reviewing #325. The reproductions and output shown below were produced by building and running the code, but I have only briefly reviewed the analysis myself and I am not an expert in this area. Please review it critically.

Impact

NWBFile::createEventsTable gained a new bool createDurationColumn parameter in the middle of its argument list, and durationResolution changed from float to std::optional<float>. Existing positional calls still compile. They just mean something different now.

Because every parameter involved is convertible to the one that took its place, the compiler accepts the old call and binds the arguments one slot out. You get a table with the wrong resolution and a missing column, with no error and only an easily missed warning.

Scope is limited: createEventsTable was added during this same unreleased 0.4.0 cycle, so no released version had the old signature. The exposure is code written against main in the meantime, plus anyone following an older example.

What happens

The old six argument call, unchanged:

nwbfile->createEventsTable("events", "desc", "src", 0.001f, 0.5f, true);

Compiles, with one warning:

warning: implicit conversion from 'float' to 'bool' changes value from 0.5 to true
        [-Wliteral-conversion]

and produces:

duration column exists:     yes
duration resolution:        1.000000     (was 0.5)
annotation column exists:   no           (was yes)

0.5f binds to the new bool createDurationColumn as true, the literal true binds to std::optional<float> durationResolution as 1.0f, and createAnnotationColumn falls back to its default of false.

createEventsTable(name, description,
                  const std::string& sourceDescription = "",
                  float timestampResolution = 0.001f,
                  float durationResolution = -1.0f,
                  const bool createAnnotationColumn = false,
                  const SizeType rowChunkSize = 100)

createEventsTable(name, description,
                  const std::optional<std::string>& sourceDescription = ...,
                  std::optional<float> timestampResolution = ...,
                  bool createDurationColumn = false,          // new, mid list
                  std::optional<float> durationResolution = ...,
                  bool createAnnotationColumn = false,
                  SizeType rowChunkSize = 100)

Suggested direction

Moving createDurationColumn to the end of the list would make old calls keep their old meaning, at some cost to how the signature reads. If the current ordering is preferred, the change is worth an explicit CHANGELOG entry under the existing **[BREAKING]** convention, since there is currently no mention of createDurationColumn, durationResolution or the duration column anywhere in that file.

The same mid list insertion applies to the public EventsTable::createDefaultDataSpecs, which is what forced the false, insertions in the example tests.

Notes

All in tree callers were updated in the same PR, so nothing in the repo still uses the old form and CI cannot catch this.