nschneir
diff --git a/‎docs/cli.md‎
Lines changed: 2 additions & 0 deletions b/‎docs/cli.md‎
Lines changed: 2 additions & 0 deletions
diff --git a/‎src/c64lib/audio.py‎
Lines changed: 27 additions & 12 deletions b/‎src/c64lib/audio.py‎
Lines changed: 27 additions & 12 deletions
diff --git a/‎src/c64lib/charset.py‎
Lines changed: 42 additions & 0 deletions b/‎src/c64lib/charset.py‎
Lines changed: 42 additions & 0 deletions
@@ -414,6 +414,8 @@ List all checkpoints with hit counts.
 JSON: `{"breakpoints": [{"id", "address", "end", "op", "enabled", "hits",
 "has_condition"}, ...]}`. `op` is the **string** `exec`, `load` or `store` —
 several joined by `|` in that order (`load|store`), never the raw bitmask.
+A checkpoint with none of the three bits set reports the empty string `""`
+rather than a placeholder word; VICE has never been seen to produce one.
 `c64_break_list` reports the same string.
 
 ### `c64 break remove` (alias: `c64 break rm`)
 
@@ -1336,10 +1336,18 @@ def sid_report(log_path, outdir, wav_path=None, ref_path=None, *,
     `peak_hz` adds a `"peak"` key measuring the recording's loudest
     frequency, and needs a `wav_path` — a dominant partial is a property of
     the recording, not of the register log. Off by default because it is one
-    rFFT over the whole file. Refusing the combination is the caller's, and
-    both front ends do refuse it, each naming its own flags; with no WAV
-    here there is nothing to measure, so no `"peak"` is attached.
+    rFFT over the whole file. Without a WAV it raises `ValueError` rather
+    than returning a report with no `"peak"` in it: `peak_hz=True` promises
+    the key, and the CLI renders it unconditionally. Both front ends refuse
+    the combination first in their own flag names, so this is a backstop for
+    a library caller and no user ever reads it.
     """
+    if peak_hz and wav_path is None:
+        # Before anything is parsed or written, like both front ends' own
+        # refusals: a call that cannot produce what it asked for costs nothing
+        # and leaves no half-written `outdir` behind.
+        raise ValueError("peak_hz needs a wav_path: a dominant partial is a "
+                         "property of the recording, not of the register log")
     import yaml
 
     # Imported here, not at module scope: this pulls in numpy and Pillow, and
@@ -1362,7 +1370,7 @@ def sid_report(log_path, outdir, wav_path=None, ref_path=None, *,
         # Neither an OSError nor a ValueError, so it would reach a front end
         # as a traceback; a hand-written score is exactly where a typo lands.
         raise AudioError(f"{ref_path} is not readable YAML ({e})") from e
-    metrics = spectrogram = None
+    metrics = spectrogram = peak = None
     if wav_path is not None:
         try:
             metrics = sid_analysis.wav_metrics(wav_path)
@@ -1372,6 +1380,16 @@ def sid_report(log_path, outdir, wav_path=None, ref_path=None, *,
             # Not an OSError and not a ValueError, so it would reach a front
             # end as a traceback: a truncated or non-RIFF file is a report.
             raise AudioError(f"{wav_path} is not a readable WAV ({e})") from e
+        if peak_hz:
+            # Measured HERE, inside the only block that knows there is a WAV,
+            # rather than beside the key it becomes: the guard at the top of
+            # this function makes "peak_hz was asked for" and "there is a
+            # recording to measure" one condition, and this is where a reader
+            # (and a type checker) can see that they are. Read through the
+            # module imported at the top of this function, for the startup
+            # reason stated there — the same reason both front ends used to
+            # import `dominant_partial_hz` by hand at their call sites.
+            peak = sid_analysis.dominant_partial_hz(wav_path)
     # After the metrics, not before: one of the anomaly checks reads a note the
     # log calls sounding against the levels the recording actually reached, so
     # the WAV has to have been measured first. Without one it is skipped and
@@ -1406,14 +1424,11 @@ def sid_report(log_path, outdir, wav_path=None, ref_path=None, *,
                     if metrics is not None else None),
         **timing,
     }
-    # `and wav_path is not None` re-states what both front ends already
-    # refused, so a library caller that skipped the refusal gets a report
-    # without a peak rather than a crash inside the FFT.
-    if peak_hz and wav_path is not None:
-        # Through the module already imported at the top of this function, for
-        # the startup reason stated there — which is the same reason both front
-        # ends used to import `dominant_partial_hz` by hand at the call site.
-        out["peak"] = sid_analysis.dominant_partial_hz(wav_path)
+    # No second `wav_path is not None` here: the guard at the top of this
+    # function is the whole rule, so `"peak"` is in the payload exactly when
+    # it was asked for, which is what lets a front end index it unconditionally.
+    if peak_hz:
+        out["peak"] = peak
     return out
 
 
 
@@ -15,6 +15,8 @@
 
 from __future__ import annotations
 
+import re
+from pathlib import Path
 from typing import NamedTuple
 
 _MC_LEGEND = {".": 0b00, "1": 0b01, "2": 0b10, "3": 0b11}
@@ -136,6 +138,46 @@ def close(lineno: int) -> None:
     return glyphs
 
 
+def parse_charset_file(path: str | Path,
+                       multicolor: bool = True) -> list[Glyph]:
+    """Read an authored sheet from disk and split it into `Glyph` blocks.
+
+    The charset twin of `sprites.encode_sheet_file`: the read, and the naming
+    of the file in whatever it raises, happen once — `c64 charset encode` and
+    the c64_charset_encode tool had one each, worded identically, and only the
+    rendering of a failure is the front ends' own half.
+
+    `read_text` on a .prg or a .png raises UnicodeDecodeError — which IS a
+    ValueError and is NOT an OSError, exactly the trap the CLI twin once
+    leaked a traceback through — and whose own message is a byte offset and a
+    codec: true, and no help in saying which of the paths was the wrong one.
+    The emptiness check needs no line here; `parse_charset` already raises
+    "no glyphs found" for a sheet that holds none.
+    """
+    try:
+        text = Path(path).read_text()
+    except (OSError, ValueError) as e:
+        raise ValueError(f"cannot read charset sheet {path}: {e}") from None
+    return parse_charset(text, multicolor=multicolor)
+
+
+def check_label(label: str, flag: str) -> None:
+    """Reject a block label ca65 could not assemble, naming the caller's flag.
+
+    Beside `format_glyphs` because that is what emits the label — as `NAME:`
+    and `NAME_end:`, so anything the assembler will not take as an identifier
+    produces a file that cannot be included rather than an error here. `flag`
+    is the only thing either front end passes: the CLI says `--label`, the
+    tool says `label`, and each caller is told about the one it used. It has
+    no default on purpose — a default is one front end's spelling silently
+    lent to the other, which is the drift this helper exists to prevent.
+    """
+    if not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", label):
+        raise CharsetError(
+            f"{flag} {label!r} is not an assembler identifier (letters, digits "
+            f"and underscore, not starting with a digit)")
+
+
 def encode_row(row: str, multicolor: bool = True) -> int:
     """Pack one art row into one charset byte (MSB = leftmost pixel)."""
     legend, bits = (_MC_LEGEND, 2) if multicolor else (_HIRES_LEGEND, 1)