Ask before reopening a bundle that carries no cuts

A bundle without a source block cannot be round-tripped, but its PDF can
still be cut from scratch. Offer that instead of refusing: confirm, run
detection, and line the markers up by position when the slice counts
match exactly.

docs/bundle-format.md

@@ -228,6 +228,14 @@ The archived document is the source of truth for reopening: the slice images are
 output, and a consumer that reopens a bundle re-renders them rather than
 importing them.
 
+`source` is optional, so a bundle without one is still valid — it is simply not
+reopenable, and a consumer should say so rather than pretend otherwise. What it
+can still recover from such a bundle is the title block, and, if it cuts the
+document again and happens to find exactly as many slices, the markers: the
+slices array is in reading order, so it lines up with any other list in reading
+order. One slice more or fewer and it does not, which is why that is a fallback
+and not the design.
+
 ## Engraving
 
 Most slices are photographs of print: an image and nothing more. A slice that

docs/guide.md

@@ -117,6 +117,15 @@ It refuses to overwrite a PDF or project file that is already there, since the
 obvious place to unpack is exactly where someone's unfinished work lives — pass
 `--pdf elsewhere.pdf` or `--force` if you mean it.
 
+A bundle from a producer that does not record its cuts can still be opened, but
+it is a fresh session rather than a round trip: it asks first, then cuts the PDF
+from scratch with detection. The title block always comes back. Markers and
+re-engraved systems land only if detection happens to find exactly as many
+slices as the bundle has — the slices are in reading order on both sides, so
+they can be lined up, but one system found or missed would shift every marker
+onto the wrong slice, so in that case they are left off entirely and it says so.
+`--detect` answers the question in advance, for scripts.
+
 ## Re-engraving a slice (optional, needs LilyPond)
 
 When a system is beyond rescue — a bad scan, a wrong transposition, a passage

noteman_slicer/bundle.py

@@ -203,48 +203,20 @@ def write(project: Project, source: Source, path: Path) -> Path:
     return path
 
 
-def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[Project, Path]:
-    """Unpack a bundle back into an editable project. Returns it and its PDF.
+class NoCuts(ValueError):
+    """The bundle carries no `source` block, so its cuts cannot be restored."""
 
-    The archived PDF is written out beside the bundle and becomes the project's
-    source again, because the PDF is what the pipeline renders from — the slice
-    images in the zip are output, and are discarded rather than re-imported.
 
-    The cuts, skew, levels and content rectangles come from the manifest's
-    `source` block, so this is a real round trip rather than a re-detection
-    that happens to land nearby. A bundle written without one cannot give them
-    back, and that is refused rather than guessed at.
-    """
-    from .project import Cut, Marker, Page, Project, Replacement, Voice, default_path, hash_file
-
-    path = Path(path)
-    with zipfile.ZipFile(path) as zf:
-        names = set(zf.namelist())
-        if "song.json" not in names:
-            raise ValueError(f"{path.name} is not a bundle: no song.json")
-        manifest = json.loads(zf.read("song.json"))
-        if manifest.get("v") != FORMAT_VERSION:
-            raise ValueError(f"unsupported bundle version {manifest.get('v')!r}")
-        geometry = manifest.get("source")
-        if not geometry:
-            raise ValueError(
-                f"{path.name} carries no cuts — it was written by a producer that "
-                "does not record them, so the score would have to be cut again"
-            )
-        pdf_name = geometry.get("file", "original.pdf")
-        if pdf_name not in names:
-            raise ValueError(f"{path.name} names {pdf_name} but does not contain it")
-        pdf_bytes = zf.read(pdf_name)
-
-    pages_json = geometry["pages"]
-    slices = manifest["slices"]
+def _pages_from(geometry: dict, slices: list[dict]):
+    """Rebuild the pages from a manifest's source geometry."""
+    from .project import Cut, Page
 
     # Every slot on a page exists; the ones no slice claims were discarded.
     # That is the one thing the bundle states by omission rather than directly,
     # since shipping a discarded slice's image would defeat discarding it.
-    claimed = {(s["page"], s["slot"]): i for i, s in enumerate(slices)}
+    claimed = {(s["page"], s["slot"]) for s in slices}
     pages = []
-    for i, page in enumerate(pages_json):
+    for i, page in enumerate(geometry["pages"]):
         cuts = [Cut([tuple(p) for p in cut]) for cut in page["cuts"]]
         count = len(cuts) + 1
         pages.append(
@@ -259,24 +231,51 @@ def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[
                 levels=tuple(page["levels"]) if page.get("levels") else None,
             )
         )
+    return pages
 
-    position_of = {index: position for position, index in claimed.items()}
-    for entry in slices:
-        page, slot = entry["page"], entry["slot"]
-        pages[page].bars[slot] = entry.get("bar")
-        pages[page].markers[slot] = [
+
+def _pages_from_detection(pdf: Path):
+    """Cut the PDF again from scratch, for a bundle that recorded no geometry."""
+    from .detect import detect_page
+    from .pdf import open_source, page_raster
+    from .project import Project
+
+    source = open_source(pdf)
+    detections, heights = [], []
+    try:
+        for i in range(len(source)):
+            gray = page_raster(source, i)
+            detections.append(detect_page(gray))
+            heights.append(gray.shape[0])
+    finally:
+        source.close()
+    return Project.from_detection(pdf, detections, heights)
+
+
+def _restore(project, slices: list[dict], positions: list[tuple[int, int]]) -> None:
+    """Put each slice's bar number, markers and engraving back on its slot."""
+    from .project import Marker, Replacement, Voice
+
+    for entry, (page, slot) in zip(slices, positions):
+        project.pages[page].bars[slot] = entry.get("bar")
+        project.pages[page].markers[slot] = [
             Marker(
                 type=marker["type"],
                 label=marker.get("label"),
                 # Back from an array index to the (page, slot) the editor works
                 # in — the inverse of what export does.
-                destination=position_of.get(marker.get("destination")),
+                destination=(
+                    positions[marker["destination"]]
+                    if marker.get("destination") is not None
+                    and marker["destination"] < len(positions)
+                    else None
+                ),
             )
             for marker in entry.get("markers", [])
         ]
         engraving = entry.get("engraving")
         if engraving and engraving.get("lang") == "lilypond":
-            pages[page].replacements[slot] = Replacement(
+            project.pages[page].replacements[slot] = Replacement(
                 voices=[
                     Voice(
                         clef=v.get("clef", "treble"),
@@ -287,6 +286,63 @@ def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[
                 ],
                 print_time=engraving.get("print_time", False),
             )
+    for entry in slices:
+        # Key and time are per slice in the bundle and per song here; the first
+        # engraving that states them is as good a song default as exists.
+        engraving = entry.get("engraving") or {}
+        if engraving.get("key"):
+            project.key = engraving["key"]
+            project.time = engraving.get("time", project.time)
+            break
+
+
+def has_cuts(path: Path) -> bool:
+    """Whether this bundle records the geometry its slices were cut with.
+
+    Worth asking before unpacking, since the answer decides whether reopening
+    is a round trip or a fresh session with the same PDF.
+    """
+    with zipfile.ZipFile(Path(path)) as zf:
+        if "song.json" not in zf.namelist():
+            return False
+        return bool(json.loads(zf.read("song.json")).get("source"))
+
+
+def read(
+    path: Path, into: Path | None = None, *, force: bool = False, detect: bool = False
+) -> tuple[Project, Path]:
+    """Unpack a bundle back into an editable project. Returns it and its PDF.
+
+    The archived PDF is written out beside the bundle and becomes the project's
+    source again, because the PDF is what the pipeline renders from — the slice
+    images in the zip are output, and are discarded rather than re-imported.
+
+    The cuts, skew, levels and content rectangles come from the manifest's
+    `source` block, so this is a real round trip rather than a re-detection
+    that happens to land nearby. A bundle written without one raises `NoCuts`;
+    `detect` says to cut the PDF from scratch instead, which is a different
+    thing and worth a caller asking about first.
+    """
+    from .project import Project, default_path, hash_file
+
+    path = Path(path)
+    with zipfile.ZipFile(path) as zf:
+        names = set(zf.namelist())
+        if "song.json" not in names:
+            raise ValueError(f"{path.name} is not a bundle: no song.json")
+        manifest = json.loads(zf.read("song.json"))
+        if manifest.get("v") != FORMAT_VERSION:
+            raise ValueError(f"unsupported bundle version {manifest.get('v')!r}")
+        geometry = manifest.get("source")
+        if not geometry and not detect:
+            raise NoCuts(
+                f"{path.name} carries no cuts — it was written by a producer that "
+                "does not record them"
+            )
+        pdf_name = (geometry or {}).get("file", "original.pdf")
+        if pdf_name not in names:
+            raise ValueError(f"{path.name} names {pdf_name} but does not contain it")
+        pdf_bytes = zf.read(pdf_name)
 
     # Unpacking writes two files. Refuse to land on either if it is already
     # there: the obvious place to open a bundle is next to the score it came
@@ -300,25 +356,32 @@ def read(path: Path, into: Path | None = None, *, force: bool = False) -> tuple[
         )
     target.write_bytes(pdf_bytes)
 
-    metadata = {
-        field: str(manifest[field]) for field in METADATA_FIELDS if manifest.get(field) is not None
-    }
+    slices = manifest["slices"]
+    if geometry:
+        pages_json = geometry["pages"]
         first = pages_json[0] if pages_json else {}
         project = Project(
             source=target,
             source_hash=hash_file(target),
-        pages=pages,
+            pages=_pages_from(geometry, slices),
             content_rect=tuple(first.get("content", (0.0, 0.0, 1.0, 1.0))),
             levels=tuple(first.get("levels", (0, 255))),
-        metadata=metadata,
             path=default_path(target),
         )
-    # Key and time are per slice in the bundle and per song here; the first
-    # engraving that states them is as good a song default as exists.
-    for entry in slices:
-        engraving = entry.get("engraving") or {}
-        if engraving.get("key"):
-            project.key = engraving["key"]
-            project.time = engraving.get("time", project.time)
-            break
+        positions = [(s["page"], s["slot"]) for s in slices]
+    else:
+        project = _pages_from_detection(target)
+        project.path = default_path(target)
+        # Detection's slices are in reading order and so are the bundle's, so
+        # they can be lined up — but only if there are exactly as many. One
+        # system found or missed shifts every marker onto the wrong slice,
+        # which is worse than not placing them at all.
+        positions = project.kept_slices()
+        if len(positions) != len(slices):
+            positions = []
+
+    project.metadata = {
+        field: str(manifest[field]) for field in METADATA_FIELDS if manifest.get(field) is not None
+    }
+    _restore(project, slices[: len(positions)], positions)
     return project, target

noteman_slicer/cli.py

@@ -102,20 +102,50 @@ def _export(args: argparse.Namespace) -> int:
     return 0
 
 
+def _confirm(question: str) -> bool:
+    """Ask before doing something the caller did not ask for. No tty, no."""
+    if not sys.stdin.isatty():
+        print(f"{question} (not a terminal — pass --detect to say yes)")
+        return False
+    return input(f"{question} [y/N] ").strip().lower() in ("y", "yes")
+
+
 def _open(args: argparse.Namespace) -> int:
     from . import bundle
     from .editor import launch
 
+    zip_path, where = Path(args.zip), Path(args.pdf) if args.pdf else None
     try:
-        project, pdf = bundle.read(
-            Path(args.zip), Path(args.pdf) if args.pdf else None, force=args.force
-        )
+        cuts = bundle.has_cuts(zip_path)
+    except (OSError, ValueError) as error:
+        print(f"cannot open: {error}")
+        return 1
+
+    if not cuts and not args.detect:
+        print(f"{zip_path.name} carries no cuts — it was written by a producer that")
+        print("does not record them. Its PDF can be cut again from scratch, but that")
+        print("is a fresh session: the cuts will be detection's, and the markers land")
+        print("only if detection happens to find the same number of slices.")
+        if not _confirm("Open it that way?"):
+            return 1
+
+    try:
+        project, pdf = bundle.read(zip_path, where, force=args.force, detect=not cuts)
     except (ValueError, KeyError) as error:
         print(f"cannot open: {error}")
         return 1
 
     saved = project.save()
-    print(f"{pdf.name}: {len(project.pages)} pages, {len(project.kept_slices())} slices")
+    kept = project.kept_slices()
+    print(f"{pdf.name}: {len(project.pages)} pages, {len(kept)} slices")
+    if not cuts:
+        marked = sum(len(m) for page in project.pages for m in page.markers)
+        print("  cut from scratch by detection — check every cut before exporting")
+        print(
+            f"  {marked} markers placed by position"
+            if marked
+            else "  markers not placed: detection found a different number of slices"
+        )
     print(f"  project written to {saved.name}")
     if args.no_edit:
         return 0
@@ -182,6 +212,11 @@ def main(argv: list[str] | None = None) -> int:
     opn.add_argument(
         "--force", action="store_true", help="overwrite an existing PDF or project file"
     )
+    opn.add_argument(
+        "--detect",
+        action="store_true",
+        help="for a bundle with no cuts: cut its PDF from scratch, without asking",
+    )
     opn.set_defaults(func=_open)
 
     ed = sub.add_parser("edit", help="open the editor")

tests/test_markers.py

@@ -165,6 +165,38 @@ def main() -> int:
     assert back.bars[second] == 7
     assert back.replacements[second].voices[0].lyrics == "la la"
 
+    # A bundle from a producer that records no cuts: refused by default, and
+    # cut from scratch by detection when the caller says so. The slices are in
+    # reading order either way, so markers can be lined up by position — but
+    # only when detection finds exactly as many.
+    plain = tmp / "plain.zip"
+    with zipfile.ZipFile(out) as src, zipfile.ZipFile(plain, "w") as dst:
+        for name in src.namelist():
+            data = src.read(name)
+            if name == "song.json":
+                manifest = json.loads(data)
+                manifest.pop("source")
+                for entry in manifest["slices"]:
+                    entry.pop("page", None)
+                    entry.pop("slot", None)
+                data = json.dumps(manifest).encode()
+            dst.writestr(name, data)
+    assert bundle.has_cuts(out) and not bundle.has_cuts(plain)
+    try:
+        bundle.read(plain, tmp / "nocuts.pdf")
+    except bundle.NoCuts as error:
+        assert "no cuts" in str(error), error
+    else:
+        raise AssertionError("a bundle without cuts should not open silently")
+
+    cut_again, again_pdf = bundle.read(plain, tmp / "nocuts.pdf", detect=True)
+    assert again_pdf.exists()
+    assert cut_again.metadata["title"] == "Test song", "the title block still comes back"
+    assert len(cut_again.kept_slices()) == len(reloaded.kept_slices())
+    placed = [m for page in cut_again.pages for slot in page.markers for m in slot]
+    assert len(placed) == 3, placed
+    assert placed[0].label == "A"
+
     # Unpacking never lands on files that are already there.
     try:
         bundle.read(out, tmp / "reopened.pdf")
@@ -174,7 +206,17 @@ def main() -> int:
         raise AssertionError("reopening over an existing PDF should be refused")
 
     source.close()
-    for f in (pdf, out, saved, unpacked, default_path(unpacked), default_path(pdf)):
+    for f in (
+        pdf,
+        out,
+        plain,
+        saved,
+        unpacked,
+        again_pdf,
+        default_path(unpacked),
+        default_path(again_pdf),
+        default_path(pdf),
+    ):
         f.unlink(missing_ok=True)
     tmp.rmdir()
     print("ok")