Skip to content

[Sad Catapella B3][Validation] ConversionSettings accepts noninteger numeric inputs that fail downstream #80

Closed

Description

@guijacquemet

Baseline: d7a538e56d41ac1093ba79adcad397c21b3dfd32. Classification: confirmed input-validation inconsistency; late error mode not runtime-tested.

Evidence: ConversionSettings.__post_init__() lines 122–139 validates int(self.chunk_size), int(self.compression_level) and int(self.pyramid_levels) but stores the unconverted supplied value. For instance, ConversionSettings(chunk_size=16.5) passes the 16–8192 check, yet _open() lines 451–462 uses the floating chunk side directly in chunks, which requires integral dimensions. Fractional pyramid levels similarly pass truncating checks, and the compressor receives the original level. Existing tests check only out-of-range integer values.

Impact: invalid user configuration is accepted, then either silently truncated for comparisons or rejected late while creating outputs rather than before any I/O. Contradicts the no-silent-coercion design rule.

Fix: reject noninteger types and bools explicitly for chunk_size, compression_level, pyramid_levels, before opening outputs. ConversionSettings.from_dict currently coerces with int() (lines 166–175): enforce the same strict policy there instead of converting 16.5 to 16. Add noninteger float, string, bool, NaN, infinity and normal integer tests and ensure failures occur before staging. No Numba opportunity.

Activity

  1. guijacquemet commented on Nov 2, 2026

    @guijacquemet
    ContributorAuthor

    Obsolete in v0.4.0: convert_tiffs, zarr_export and OME-Zarr copy reuse were removed in PP-2 (#66, commit 3c2802d; plan question 46). Named-axis copies of images with axis errors are now the OME-TIFF convert step (PP-11, #124, merged as 8db1f4a). Closing as not planned (owner-approved triage, 2026-11-02).

    PP-11 validates its settings strictly before any I/O (memory_limit an int above 0, not a bool; missing_as_singleton a bool).


    Generated by Claude Code

  2. guijacquemet commented on Nov 2, 2026

    @guijacquemet
    ContributorAuthor

    Reopening: an independent review of commit 8db1f4a found the closure premature. The Python API (convert_experiment) validates its settings strictly, but the CLI path does not: cli_run.parse_size returns int(float(number) * unit), so --memory-limit 1.5 silently becomes 1 byte and fractional bytes are truncated, and convert_command runs its own preflight before convert_experiment can refuse a zero. Fix: the parsed size must be an exact positive whole number of bytes (refuse otherwise), validated before any I/O. Tracked in PR PP-11c.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions