Stop mutating argparse.Namespace inside cli._normalize_and_validate

[rfrenchseti]

Background

src/picmaker/cli.py::_normalize_and_validate (lines 357-504) mutates its input argparse.Namespace in place: options.scale, options.bands, options.pointer, options.valid, options.limits, options.percentiles, etc. are all rebound during normalization. Calling the function twice on the same Namespace therefore produces different output, because the second call's pre-normalization checks see the post-normalization values from the first call.

The function's name advertises a transformation, not a side effect. The current name is _normalize_and_validate, but the implementation is closer to _normalize_in_place_and_build_option_dict.

See CODEBASE_CRITIQUE.md §2 (Best practices alignment, "Finding (Medium) — _normalize_and_validate mutates the input argparse.Namespace in place").

Why it matters

• Today, cli.main always parses a fresh Namespace per --versions line, so the bug doesn't fire. But there is no contract anywhere that says "don't reuse the Namespace" — a future contributor (or an internal refactor that wants to call _normalize_and_validate for both single-image and --versions paths from the same parse) would silently see different results from the second call.
• Tests are not affected today because tests/test_cli_unit.py always builds a fresh Namespace via _parse(...) per test.

Suggested approach

Refactor _normalize_and_validate to work on local copies:

def _normalize_and_validate(
    options: argparse.Namespace, replace: str, proceed: bool,
) -> dict[str, Any]:
    """..."""
    # Read every value from `options` once into local variables.
    hst = options.hst
    band = options.band
    bands = options.bands
    scale = options.scale
    wscale = options.wscale
    hscale = options.hscale
    # ... etc.

    # Pre-normalization checks (use local variables, not options.*).
    if hst:
        if band is not None or bands is not None:
            raise ValueError(...)
    # ... etc.

    # Normalization (mutate locals only, never options.*).
    if scale is None:
        scale = 100.0
    if wscale is None:
        wscale = scale
    if hscale is None:
        hscale = scale
    scale = (wscale, hscale)
    # ... etc.

    option_dict = {'scale': scale, 'bands': bands, ...}
    PicmakerOptions(**option_dict).validate()
    return option_dict

Once that's done, returning a PicmakerOptions instead of a dict[str, Any] is a small follow-up (see also #12 which tracks the RORO migration of images_to_pics).

Acceptance criteria

• _normalize_and_validate does not mutate any attribute of the input argparse.Namespace.
• An assertion-style test pins this: parser.parse_args([...]) → take the namespace, call _normalize_and_validate twice with the same replace/proceed, assert the two returned dicts are equal.
• Existing tests in test_cli_unit.py pass unchanged.
• Consider following up with a PicmakerOptions return type (companion to #12).

Related

• CODEBASE_CRITIQUE.md §2 — original finding.
• #12 — the RORO migration of images_to_pics is the natural place to switch the return type from dict[str, Any] to PicmakerOptions.