From e354cfca0fa71a99e7a48b9d7ef24458bc9b9d73 Mon Sep 17 00:00:00 2001 From: sjquant Date: Mon, 20 Jul 2026 23:34:06 +0900 Subject: [PATCH 1/5] fix: align auto-layout diagnostics with rendered bounds --- quickthumb/_diagnostics.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/quickthumb/_diagnostics.py b/quickthumb/_diagnostics.py index f78e332..1d5e714 100644 --- a/quickthumb/_diagnostics.py +++ b/quickthumb/_diagnostics.py @@ -539,6 +539,10 @@ def _render_layer_alpha_channel(self, measured: LayerMeasurement) -> Image.Image box = require_bbox(measured) image = Image.new("RGBA", (self._ctx.width, self._ctx.height), (0, 0, 0, 0)) layer = measured.raw_layer + if isinstance(layer, GroupLayer): + origin = measured.metadata.get("position") + if isinstance(origin, tuple): + layer = layer.model_copy(update={"position": origin, "align": None}) if not isinstance(layer, (GroupLayer, TextLayer, ImageLayer, SvgLayer, ShapeLayer)): return Image.new("L", (box.width, box.height), 0) self._canvas._render_layer(image, layer) From 038b4d1a97153748ca643c5799338fd04f573d34 Mon Sep 17 00:00:00 2001 From: sjquant Date: Mon, 20 Jul 2026 23:58:50 +0900 Subject: [PATCH 2/5] feat: harden diagnose and lint workflows --- docs/api/canvas.md | 5 +- docs/api/deck.md | 6 + docs/concepts.md | 2 +- docs/diagnostics.md | 19 ++- docs/faq.md | 2 +- quickthumb/_diagnostics.py | 22 ++- quickthumb/canvas.py | 16 +- quickthumb/cli.py | 294 +++++++++++++++++++++++++++---------- quickthumb/deck.py | 26 +++- tests/test_cli.py | 124 ++++++++++++++++ tests/test_diagnostics.py | 6 +- 11 files changed, 434 insertions(+), 88 deletions(-) diff --git a/docs/api/canvas.md b/docs/api/canvas.md index b85178f..c268904 100644 --- a/docs/api/canvas.md +++ b/docs/api/canvas.md @@ -64,7 +64,10 @@ for finding in canvas.diagnose(): print(finding.severity, finding.code, finding.message) ``` -Finding codes: `off-canvas`, `tiny-text`, `text-overflow`, `low-contrast`. See [Diagnostics & CLI](../diagnostics.md) for details and the `quickthumb lint` equivalent. +Finding codes: `off-canvas`, `tiny-text`, `text-overflow`, `text-clipped`, `missing-glyph`, +`low-contrast`, `layer-overlap`, `layer-hidden`, and `edge-crowding`. See +[Diagnostics & CLI](../diagnostics.md) for details and the `quickthumb lint` (or +`quickthumb diagnose`) equivalent. ## Export methods diff --git a/docs/api/deck.md b/docs/api/deck.md index 28a93b6..e55e99b 100644 --- a/docs/api/deck.md +++ b/docs/api/deck.md @@ -183,6 +183,12 @@ for finding in deck.diagnose(): | `message` | `str` | Human-readable description | | `slide_index` | `int \| None` | Originating slide, or `None` for deck-wide findings | | `layer_index` | `int \| None` | Originating layer within the slide, when applicable | +| `layer_id` | `str \| None` | Stable originating layer id, including group-child ids | +| `layer_name` | `str \| None` | Originating layer name when present | +| `bbox` | `dict \| None` | Measured canvas-space bounding box | +| `related_layers` | `list[str]` | Layer ids involved in the finding | +| `measured` | `dict` | Rule-specific measured values | +| `suggestion` | `str \| None` | Repair hint when available | A `mixed-slide-size` warning is added when slides do not all share the same dimensions. The PDF path sizes each page to its slide, but PPTX export uses the first slide's size for the whole deck, so larger later slides are clipped by PowerPoint — keep slides a uniform size when targeting `.pptx`. diff --git a/docs/concepts.md b/docs/concepts.md index a0e62d6..5f67577 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -284,7 +284,7 @@ Tokens are resolved at parse time; unknown tokens raise `ValidationError`. See [ ## Diagnostics -`canvas.diagnose()` checks a composition for common problems — layers outside the canvas, illegibly small text, unwrappable words, and low text contrast — without writing a file. The CLI equivalent is `quickthumb lint spec.json`. See [Diagnostics & CLI](diagnostics.md). +`canvas.diagnose()` checks a composition for common problems — layers outside the canvas, illegibly small text, unwrappable or clipped text, missing glyphs, hidden or overlapping layers, safe-area crowding, and low text contrast — without writing a file. The CLI equivalents are `quickthumb lint spec.json` and `quickthumb diagnose spec.json`. See [Diagnostics & CLI](diagnostics.md). ## JSON round-trip diff --git a/docs/diagnostics.md b/docs/diagnostics.md index d6d0116..df5c77f 100644 --- a/docs/diagnostics.md +++ b/docs/diagnostics.md @@ -23,7 +23,7 @@ Each `Diagnostic` has stable human-readable fields and optional structured field | Field | Type | Description | | --- | --- | --- | -| `code` | `str` | One of `"off-canvas"`, `"tiny-text"`, `"text-overflow"`, `"text-clipped"`, `"missing-glyph"`, `"low-contrast"`, `"layer-overlap"`, `"layer-hidden"`, `"edge-crowding"`, `"near-alignment"` | +| `code` | `str` | One of `"off-canvas"`, `"tiny-text"`, `"text-overflow"`, `"text-clipped"`, `"missing-glyph"`, `"low-contrast"`, `"layer-overlap"`, `"layer-hidden"`, `"edge-crowding"`, or `"near-alignment"` | | `severity` | `str` | `"warning"` or `"error"` | | `layer_index` | `int` | Index of the offending layer in `canvas.layers` | | `message` | `str` | Human-readable explanation with the measured values | @@ -72,6 +72,7 @@ Checks a JSON spec for the same findings as `diagnose()`: ```bash quickthumb lint spec.json quickthumb lint spec.json --format json +quickthumb diagnose spec.json --format json ``` ```text @@ -160,6 +161,22 @@ Exit codes make it easy to gate CI or agent pipelines: | `2` | Rendering failure | | `3` | Findings reported | +`diagnose` is an alias for `lint`. Both commands accept the same automation options: + +```bash +quickthumb lint spec.json --fail-on error +quickthumb lint spec.json --ignore edge-crowding --ignore missing-glyph +``` + +`--fail-on warning` (the default) exits 3 for any remaining finding. `--fail-on error` +allows warnings while still failing on errors, and `--fail-on never` always exits 0 after +printing the findings. `--ignore` removes matching diagnostic codes from both the output and +the exit-status calculation. Invalid specs with `--format json` emit an `error` object rather +than a traceback. + +Deck JSON is accepted as well as Canvas JSON. Deck findings include `slide_index` and retain +the originating layer id, bounding box, measurements, related layers, and suggestion. + ### `quickthumb render` Renders a JSON spec to an image file: diff --git a/docs/faq.md b/docs/faq.md index 8eeb540..692a0f1 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -68,7 +68,7 @@ A child's `align` is also ignored; use the group's `item_align` for cross-axis p ### How do I check a composition for problems before rendering? -Call `canvas.diagnose()` (or run `quickthumb lint spec.json`). It reports off-canvas layers, tiny text, unwrappable words, and low text contrast. See [Diagnostics & CLI](diagnostics.md). +Call `canvas.diagnose()` (or run `quickthumb lint spec.json` / `quickthumb diagnose spec.json`). It reports off-canvas layers, tiny or clipped text, unwrappable words, missing glyphs, hidden or overlapping layers, safe-area crowding, and low text contrast. See [Diagnostics & CLI](diagnostics.md). --- diff --git a/quickthumb/_diagnostics.py b/quickthumb/_diagnostics.py index 1d5e714..3e9bd2a 100644 --- a/quickthumb/_diagnostics.py +++ b/quickthumb/_diagnostics.py @@ -1,3 +1,4 @@ +import warnings from collections.abc import Iterable from itertools import islice from math import ceil @@ -91,9 +92,19 @@ def diagnose(self) -> list[Diagnostic]: """Check layers for layout and legibility issues without producing an output file. Returns structured findings (off-canvas, tiny-text, text-overflow, - low-contrast, layer-overlap, near-alignment) that an agent or human can - act on before rendering. + text-clipped, missing-glyph, low-contrast, layer-overlap, layer-hidden, + edge-crowding, near-alignment) that an agent or human can act on before + rendering. """ + with warnings.catch_warnings(): + warnings.filterwarnings( + "ignore", + message=r"Word .* exceeds max_width and cannot be wrapped\.", + category=UserWarning, + ) + return self._diagnose_impl() + + def _diagnose_impl(self) -> list[Diagnostic]: self._alpha_cache.clear() self._canvas._validate_image_paths() self._ctx.begin_render_pass() @@ -123,10 +134,11 @@ def diagnose(self) -> list[Diagnostic]: diagnostics.extend(self._diagnose_hidden_layers(measurements)) edge_ignored_layer_ids = set() for finding in diagnostics: - if finding.code == "off-canvas" and finding.layer_id is not None: + if ( + finding.code in {"off-canvas", "layer-hidden"} + and finding.layer_id is not None + ): edge_ignored_layer_ids.add(finding.layer_id) - elif finding.code == "layer-hidden": - edge_ignored_layer_ids.update(finding.related_layers) diagnostics.extend( self._diagnose_edge_crowding(measurements, ignored_layer_ids=edge_ignored_layer_ids) ) diff --git a/quickthumb/canvas.py b/quickthumb/canvas.py index 622bf92..38da3e7 100644 --- a/quickthumb/canvas.py +++ b/quickthumb/canvas.py @@ -6,6 +6,7 @@ from typing import Any, Literal, cast from PIL import Image, ImageDraw, ImageFont +from pydantic import ValidationError as PydanticValidationError from typing_extensions import Self from quickthumb._base import FileFormat, RenderContext, aspect_ratio_dimensions, is_url @@ -259,8 +260,8 @@ def layers(self, value: list[RenderableLayer]): def diagnose(self) -> list[Diagnostic]: """Check layers for layout and legibility issues without producing an output file. - Returns structured findings (off-canvas, tiny-text, text-overflow, low-contrast) - that an agent or human can act on before rendering. + Returns structured findings for layout, legibility, visibility, and safe-area + checks that an agent or human can act on before rendering. """ return self._diagnostics.diagnose() @@ -927,6 +928,17 @@ def _omit_unset_composition_fields(cls, value): @classmethod def from_json(cls, data: str) -> Self: + try: + return cls._from_json(data) + except PydanticValidationError as error: + messages = [] + for detail in error.errors(): + field = " -> ".join(map(str, detail["loc"])) + messages.append(f"Field '{field}': {detail['msg']}") + raise ValidationError(" | ".join(messages), original_error=error) from error + + @classmethod + def _from_json(cls, data: str) -> Self: import json as _json from pydantic import TypeAdapter diff --git a/quickthumb/cli.py b/quickthumb/cli.py index 142cbe9..2b9826f 100644 --- a/quickthumb/cli.py +++ b/quickthumb/cli.py @@ -3,7 +3,7 @@ import json import re from pathlib import Path -from typing import Annotated +from typing import Annotated, Protocol, TypeAlias import typer from PIL import Image @@ -18,10 +18,37 @@ create_diff_image, ) from quickthumb.canvas import _VAR_RE, Canvas, _is_theme_reference +from quickthumb.deck import Deck, DeckDiagnostic from quickthumb.errors import RenderingError, ValidationError from quickthumb.schema import canvas_json_schema _VALID_FORMATS = {"PNG", "JPEG", "WEBP"} +_DIAGNOSTIC_CODES = { + "off-canvas", + "tiny-text", + "text-overflow", + "text-clipped", + "missing-glyph", + "low-contrast", + "layer-overlap", + "layer-hidden", + "edge-crowding", + "mixed-slide-size", +} +_FAIL_ON_VALUES = {"warning", "error", "never"} +Diagnosable: TypeAlias = Canvas | Deck + + +class _DiagnosticLike(Protocol): + code: str + severity: str + layer_index: int | None + layer_id: str | None + message: str + suggestion: str | None + + def model_dump(self, *, mode: str, exclude_none: bool) -> dict: ... + app = typer.Typer(help="quickthumb — programmatic thumbnail generation") @@ -40,32 +67,156 @@ def _parse_var_options(var: list[str] | None) -> dict[str, str]: for item in var or []: key, sep, value = item.partition("=") if not sep: - typer.echo(f"Invalid --var '{item}': expected KEY=VALUE format.", err=True) - raise typer.Exit(1) + raise ValidationError(f"Invalid --var '{item}': expected KEY=VALUE format.") variables[key] = value return variables -def _load_canvas(spec: Path, variables: dict[str, str]) -> Canvas: - """Read, substitute, and parse a spec file; any input error exits with code 1.""" - try: - text = spec.read_text() - except (FileNotFoundError, PermissionError) as e: - typer.echo(str(e), err=True) - raise typer.Exit(1) from e +def _load_canvas(spec: Path, variables: dict[str, str]) -> Diagnosable: + """Read, substitute, and parse a Canvas or Deck JSON source.""" + text = spec.read_text() if variables: - try: - text = _substitute_vars(text, variables) - except ValidationError as e: - typer.echo(str(e), err=True) - raise typer.Exit(1) from e + text = _substitute_vars(text, variables) + + raw = json.loads(text) + if isinstance(raw, dict) and "slides" in raw: + return Deck.from_json(text) + return Canvas.from_json(text) + + +def _echo_input_error( + error: Exception, output_format: str = "text", code: str = "invalid-spec" +) -> None: + if output_format == "json": + typer.echo(json.dumps({"error": {"code": code, "message": str(error)}})) + else: + typer.echo(str(error), err=True) + + +def _validate_diagnostic_options( + fail_on: str, ignored_codes: list[str] | None, output_format: str +) -> tuple[str, set[str]]: + normalized_fail_on = fail_on.lower() + if normalized_fail_on not in _FAIL_ON_VALUES: + _echo_input_error( + ValidationError( + f"Invalid --fail-on '{fail_on}'. Must be one of: warning, error, never" + ), + output_format, + code="invalid-options", + ) + raise typer.Exit(1) from None + + ignored = {code.lower() for code in ignored_codes or []} + unknown = sorted(ignored - _DIAGNOSTIC_CODES) + if unknown: + _echo_input_error( + ValidationError(f"Unknown diagnostic code(s): {', '.join(unknown)}"), + output_format, + code="invalid-options", + ) + raise typer.Exit(1) from None + return normalized_fail_on, ignored + + +def _diagnostic_payload(finding: _DiagnosticLike) -> dict: + return finding.model_dump(mode="json", exclude_none=True) + + +def _filter_diagnostics( + findings: list[_DiagnosticLike], ignored_codes: set[str] +) -> list[_DiagnosticLike]: + return [finding for finding in findings if finding.code not in ignored_codes] + + +def _should_fail(findings: list[_DiagnosticLike], fail_on: str) -> bool: + if fail_on == "never": + return False + return any( + finding.severity == "error" + or (fail_on == "warning" and finding.severity == "warning") + for finding in findings + ) + + +def _format_finding(finding: _DiagnosticLike) -> str: + location = "deck" if finding.layer_index is None else f"layer {finding.layer_index}" + slide_index = finding.slide_index if isinstance(finding, DeckDiagnostic) else None + if slide_index is not None: + location = f"slide {slide_index}, {location}" + layer_id = finding.layer_id + if layer_id: + location = f"{location} ({layer_id})" + message = f"[{finding.severity}] {location}: {finding.code} — {finding.message}" + suggestion = finding.suggestion + if suggestion and suggestion not in finding.message: + message += f" Suggestion: {suggestion}." + return message + + +def _run_lint( + spec: Path, + output_format: str, + var: list[str] | None, + fail_on: str, + ignored_codes: list[str] | None, +) -> None: + lint_format = output_format.lower() + if lint_format not in ("text", "json"): + typer.echo( + f"Invalid lint format '{output_format}'. Must be one of: text, json", + err=True, + ) + raise typer.Exit(1) + normalized_fail_on, ignored = _validate_diagnostic_options( + fail_on, ignored_codes, lint_format + ) try: - return Canvas.from_json(text) - except (json.JSONDecodeError, ValidationError) as e: - typer.echo(str(e), err=True) - raise typer.Exit(1) from e + source = _load_canvas(spec, _parse_var_options(var)) + except (OSError, UnicodeError, json.JSONDecodeError, ValidationError) as error: + _echo_input_error(error, lint_format) + raise typer.Exit(1) from None + + try: + diagnostics = _filter_diagnostics(source.diagnose(), ignored) + except FileNotFoundError as error: + _echo_input_error(FileNotFoundError(f"Referenced file not found: {error}"), lint_format) + raise typer.Exit(1) from None + except (RenderingError, OSError) as error: + if lint_format == "json": + typer.echo(json.dumps({"error": {"code": "rendering-failure", "message": str(error)}})) + else: + typer.echo(str(error), err=True) + raise typer.Exit(2) from None + + if lint_format == "json": + error_count = sum(1 for finding in diagnostics if finding.severity == "error") + warning_count = sum( + 1 for finding in diagnostics if finding.severity == "warning" + ) + typer.echo( + json.dumps( + { + "summary": { + "diagnostic_count": len(diagnostics), + "error_count": error_count, + "warning_count": warning_count, + }, + "diagnostics": [_diagnostic_payload(finding) for finding in diagnostics], + }, + indent=2, + ) + ) + elif not diagnostics: + typer.echo("No issues found.") + else: + for finding in diagnostics: + typer.echo(_format_finding(finding)) + + if _should_fail(diagnostics, normalized_fail_on): + raise typer.Exit(3) @app.callback() @@ -129,10 +280,26 @@ def render( """Render a JSON spec file to an image, to SVG/PPTX/PDF/HTML, or to an animated GIF/MP4/WebM that plays the spec's layer animations, by extension.""" _validate_render_options(fmt, quality) - canvas = _load_canvas(spec, _parse_var_options(var)) + try: + source = _load_canvas(spec, _parse_var_options(var)) + except (OSError, UnicodeError, json.JSONDecodeError, ValidationError) as error: + typer.echo(str(error), err=True) + raise typer.Exit(1) from None try: - canvas.render( + if isinstance(source, Deck): + if debug: + typer.echo("--debug is only supported for Canvas specs.", err=True) + raise typer.Exit(1) + written = source.render( + str(output), + format=fmt.upper() if fmt else None, # type: ignore[arg-type] + quality=quality, + ) + for path in written: + typer.echo(path) + return + source.render( str(output), format=fmt.upper() if fmt else None, # type: ignore[arg-type] quality=quality, @@ -231,7 +398,8 @@ def _save_diff_image(expected: Image.Image, actual: Image.Image, output: Path) - diff_image.save(output) -@app.command() +@app.command("diagnose") +@app.command("lint") def lint( spec: Annotated[Path, typer.Argument(help="Path to a JSON spec file")], output_format: Annotated[ @@ -242,62 +410,20 @@ def lint( list[str] | None, typer.Option("--var", help="Variable substitution as KEY=VALUE"), ] = None, + fail_on: Annotated[ + str, + typer.Option("--fail-on", help="Fail on warning, error, or never"), + ] = "warning", + ignore: Annotated[ + list[str] | None, + typer.Option("--ignore", help="Ignore a diagnostic code (repeatable)"), + ] = None, ) -> None: """Check a JSON spec for layout and legibility issues without rendering a file. Exit codes: 0 no issues, 1 invalid spec, 2 rendering failure, 3 issues found. """ - lint_format = output_format.lower() - if lint_format not in ("text", "json"): - typer.echo( - f"Invalid lint format '{output_format}'. Must be one of: text, json", - err=True, - ) - raise typer.Exit(1) - - canvas = _load_canvas(spec, _parse_var_options(var)) - - try: - diagnostics = canvas.diagnose() - except FileNotFoundError as e: - typer.echo(f"Referenced file not found: {e}", err=True) - raise typer.Exit(1) from e - except (RenderingError, OSError) as e: - typer.echo(str(e), err=True) - raise typer.Exit(2) from e - - if lint_format == "json": - error_count = sum(1 for finding in diagnostics if finding.severity == "error") - warning_count = sum(1 for finding in diagnostics if finding.severity == "warning") - typer.echo( - json.dumps( - { - "summary": { - "diagnostic_count": len(diagnostics), - "error_count": error_count, - "warning_count": warning_count, - }, - "diagnostics": [ - finding.model_dump(mode="json", exclude_none=True) - for finding in diagnostics - ], - }, - indent=2, - ) - ) - if diagnostics: - raise typer.Exit(3) - return - - if not diagnostics: - typer.echo("No issues found.") - return - - for finding in diagnostics: - typer.echo( - f"[{finding.severity}] layer {finding.layer_index}: {finding.code} — {finding.message}" - ) - raise typer.Exit(3) + _run_lint(spec, output_format, var, fail_on, ignore) def _substitute_vars(text: str, variables: dict[str, str]) -> str: @@ -351,8 +477,8 @@ def serve( """Serve HTML slides with live reload and a ?presenter view.""" from quickthumb._serve import serve_slides - variables = _parse_var_options(var) try: + variables = _parse_var_options(var) serve_slides( source=source, host=host, @@ -406,15 +532,33 @@ def watch( raise typer.Exit(1) from None _validate_render_options(fmt, quality) - variables = _parse_var_options(var) + try: + variables = _parse_var_options(var) + except ValidationError as error: + typer.echo(str(error), err=True) + raise typer.Exit(1) from None def _render_once() -> None: try: canvas = _load_canvas(spec, variables) except typer.Exit: return # error already echoed; keep watching for the next change + except (OSError, UnicodeError, json.JSONDecodeError, ValidationError) as error: + typer.echo(str(error), err=True) + return try: + if isinstance(canvas, Deck): + if debug: + typer.echo("--debug is only supported for Canvas specs.", err=True) + return + for path in canvas.render( + str(output), + format=fmt.upper() if fmt else None, # type: ignore[arg-type] + quality=quality, + ): + typer.echo(path) + return canvas.render( str(output), format=fmt.upper() if fmt else None, # type: ignore[arg-type] diff --git a/quickthumb/deck.py b/quickthumb/deck.py index 38d25b2..649a750 100644 --- a/quickthumb/deck.py +++ b/quickthumb/deck.py @@ -12,7 +12,7 @@ import math import os import tempfile -from dataclasses import dataclass +from dataclasses import asdict, dataclass from typing import TYPE_CHECKING, cast from typing_extensions import Self @@ -50,6 +50,23 @@ class DeckDiagnostic: message: str slide_index: int | None = None layer_index: int | None = None + layer_id: str | None = None + layer_name: str | None = None + bbox: dict[str, int] | None = None + related_layers: list[str] | None = None + measured: dict | None = None + suggestion: str | None = None + + def model_dump(self, *, mode: str = "python", exclude_none: bool = False) -> dict: + """Return a serialization-compatible payload like a Pydantic diagnostic.""" + payload = asdict(self) + if mode == "json": + import json + + payload = json.loads(json.dumps(payload)) + if exclude_none: + payload = {key: value for key, value in payload.items() if value is not None} + return payload class Deck: @@ -620,6 +637,7 @@ def diagnose(self) -> list[DeckDiagnostic]: for slide_index, canvas in enumerate(self._slides): for finding in canvas.diagnose(): + payload = finding.model_dump(mode="json", exclude_none=True) findings.append( DeckDiagnostic( code=finding.code, @@ -627,6 +645,12 @@ def diagnose(self) -> list[DeckDiagnostic]: message=finding.message, slide_index=slide_index, layer_index=finding.layer_index, + layer_id=payload.get("layer_id"), + layer_name=payload.get("layer_name"), + bbox=payload.get("bbox"), + related_layers=payload.get("related_layers"), + measured=payload.get("measured"), + suggestion=payload.get("suggestion"), ) ) return findings diff --git a/tests/test_cli.py b/tests/test_cli.py index d618231..3f4b2cd 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1418,6 +1418,130 @@ def test_should_exit_1_for_invalid_spec(self): # then assert result.exit_code == 1 + def test_should_emit_json_error_without_traceback_for_invalid_layer(self): + """lint --format json emits a parseable error for an invalid layer discriminator""" + from quickthumb.cli import app + + # given: a spec with an unknown layer type + spec_path = self._write_spec( + {"width": 100, "height": 100, "layers": [{"type": "unknown"}]} + ) + + # when + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: expected input failures are structured and do not expose a traceback + assert result.exit_code == 1 + assert "Traceback" not in result.output + payload = json.loads(result.output) + assert payload["error"]["code"] == "invalid-spec" + assert "unknown" in payload["error"]["message"] + + def test_should_support_deck_specs_and_preserve_slide_diagnostic_fields(self): + """lint accepts deck JSON and includes slide and layer diagnostic context""" + from quickthumb.cli import app + + # given: a deck whose first slide has an off-canvas layer + spec_path = self._write_spec( + { + "slides": [ + { + "width": 100, + "height": 100, + "layers": [ + { + "type": "shape", + "shape": "rectangle", + "position": [300, 300], + "width": 50, + "height": 50, + "color": "#FF0000", + } + ], + } + ] + } + ) + + # when + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: the finding carries both slide and original Canvas context + assert result.exit_code == 3 + finding = json.loads(result.output)["diagnostics"][0] + assert finding["slide_index"] == 0 + assert finding["layer_index"] == 0 + assert finding["layer_id"] == "layer:0" + assert finding["bbox"] == {"x": 300, "y": 300, "width": 50, "height": 50} + assert finding["suggestion"] == "move layer to x=50, y=50 to fit within the canvas" + + def test_should_support_diagnose_alias_and_fail_on_filters(self): + """diagnose aliases lint and fail-on controls warning-only findings""" + from quickthumb.cli import app + + # given: a shape that produces only an edge-crowding warning + spec_path = self._write_spec( + { + "width": 100, + "height": 100, + "layers": [ + { + "type": "shape", + "shape": "rectangle", + "position": [0, 10], + "width": 20, + "height": 20, + "color": "#FF0000", + } + ], + } + ) + + # when: warnings are ignored for exit status and then filtered entirely + try: + warning_result = CliRunner().invoke( + app, ["diagnose", spec_path, "--fail-on", "error"] + ) + ignored_result = CliRunner().invoke( + app, + ["diagnose", spec_path, "--ignore", "edge-crowding", "--format", "json"], + ) + finally: + os.unlink(spec_path) + + # then + assert warning_result.exit_code == 0 + assert "edge-crowding" in warning_result.output + assert ignored_result.exit_code == 0 + assert json.loads(ignored_result.output)["diagnostics"] == [] + + def test_should_emit_json_error_for_unknown_diagnostic_filter(self, spec_file): + """lint --format json reports unknown diagnostic filters as structured errors""" + from quickthumb.cli import app + + # given: a valid canvas and a misspelled diagnostic code + + # when + result = CliRunner().invoke( + app, + ["lint", spec_file, "--format", "json", "--ignore", "not-a-rule"], + ) + + # then + assert result.exit_code == 1 + assert json.loads(result.output) == { + "error": { + "code": "invalid-options", + "message": "Unknown diagnostic code(s): not-a-rule", + } + } + def test_should_exit_1_when_lint_variable_substitution_leaves_placeholder(self): """lint exits 1 when variable substitution leaves unresolved placeholders""" from quickthumb.cli import app diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index 6e401c5..947434b 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -1764,7 +1764,11 @@ def test_should_warn_when_large_shape_fully_covers_small_shape(self): diagnostics = canvas.diagnose() # then - assert [finding.code for finding in diagnostics] == ["layer-overlap", "layer-hidden"] + assert [finding.code for finding in diagnostics] == [ + "layer-overlap", + "layer-hidden", + "edge-crowding", + ] assert "visible_overlap_pct=0% of upper, 100% of lower" in diagnostics[0].message def test_should_remeasure_alpha_masks_after_canvas_layers_change(self): From 8cbf781352556b66a884adec350090a48ebf841e Mon Sep 17 00:00:00 2001 From: sjquant Date: Tue, 21 Jul 2026 00:27:53 +0900 Subject: [PATCH 3/5] fix: validate discriminated JSON documents --- docs/api/deck.md | 2 +- docs/concepts.md | 1 + docs/cookbook/ai-workflow.md | 2 +- docs/cookbook/shorts-cover.md | 1 + docs/diagnostics.md | 8 +- docs/getting-started.md | 1 + docs/json-schema.md | 14 ++- examples/launch_announcement.json | 1 + examples/shorts_cover_agent.json | 1 + quickthumb/_document.py | 25 ++++ quickthumb/_serve.py | 4 +- quickthumb/canvas.py | 7 ++ quickthumb/cli.py | 29 +++-- quickthumb/deck.py | 19 ++- quickthumb/models.py | 2 + quickthumb/schema.py | 5 +- tests/test_background_layers.py | 1 + tests/test_canvas.py | 3 + tests/test_cli.py | 189 ++++++++++++++++++++++++++++-- tests/test_deck.py | 1 + tests/test_diagnostics.py | 16 ++- tests/test_image_layers.py | 3 + tests/test_serve.py | 5 +- 23 files changed, 305 insertions(+), 35 deletions(-) create mode 100644 quickthumb/_document.py diff --git a/docs/api/deck.md b/docs/api/deck.md index e55e99b..888ed35 100644 --- a/docs/api/deck.md +++ b/docs/api/deck.md @@ -196,7 +196,7 @@ A `mixed-slide-size` warning is added when slides do not all share the same dime ### `.to_json()` / `Deck.from_json(json_str)` -Round-trips the deck through JSON, reusing each canvas's serialization. The shape is `{"width": ..., "height": ..., "theme": {...}, "slides": [, ...]}`, where `width`/`height` (the default slide size) and `theme` are emitted only when set. Per-slide `transition`, `audio`, `duration`, and `notes` fields are preserved alongside the canvas fields. A top-level `theme` is shared with every slide so slides can use `$theme.*` tokens, exactly like `Canvas.from_json`. +Round-trips the deck through JSON, reusing each canvas's serialization. The shape is `{"kind": "deck", "width": ..., "height": ..., "theme": {...}, "slides": [, ...]}`, where `width`/`height` (the default slide size) and `theme` are emitted only when set. Per-slide `transition`, `audio`, `duration`, and `notes` fields are preserved alongside the canvas fields. A top-level `theme` is shared with every slide so slides can use `$theme.*` tokens, exactly like `Canvas.from_json`. ```python spec = deck.to_json() diff --git a/docs/concepts.md b/docs/concepts.md index 5f67577..da56abf 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -271,6 +271,7 @@ JSON specs can define brand tokens once in a top-level `theme` block and referen ```json { + "kind": "canvas", "width": 1280, "height": 720, "theme": { "colors": { "primary": "#B8FF00" }, "sizes": { "title": 96 } }, diff --git a/docs/cookbook/ai-workflow.md b/docs/cookbook/ai-workflow.md index 88ad80d..b2e4ce9 100644 --- a/docs/cookbook/ai-workflow.md +++ b/docs/cookbook/ai-workflow.md @@ -32,7 +32,7 @@ quickthumb is designed to be a reliable target for LLM-generated image specs. Th Generate a quickthumb JSON config for a 1280×720 YouTube thumbnail. Schema rules: -- Top-level fields: "width", "height", "layers" +- Top-level fields: "kind": "canvas", "width", "height", "layers" - Every layer must have a "type": "background" | "text" | "image" | "shape" | "outline" - Every effect must have a "type": "stroke" | "shadow" | "glow" | "filter" | "background" - Positions are [x, y] JSON arrays — values can be integers (px) or percentage strings ("50%") diff --git a/docs/cookbook/shorts-cover.md b/docs/cookbook/shorts-cover.md index 33c2740..1e3b91b 100644 --- a/docs/cookbook/shorts-cover.md +++ b/docs/cookbook/shorts-cover.md @@ -23,6 +23,7 @@ That's it. The entire composition lives in the JSON file. ```json { + "kind": "canvas", "width": 1080, "height": 1920, "layers": [ diff --git a/docs/diagnostics.md b/docs/diagnostics.md index df5c77f..a46ef22 100644 --- a/docs/diagnostics.md +++ b/docs/diagnostics.md @@ -174,7 +174,8 @@ printing the findings. `--ignore` removes matching diagnostic codes from both th the exit-status calculation. Invalid specs with `--format json` emit an `error` object rather than a traceback. -Deck JSON is accepted as well as Canvas JSON. Deck findings include `slide_index` and retain +JSON inputs must declare a top-level `kind` discriminator (`canvas` or `deck`). Deck findings +include `slide_index` and retain the originating layer id, bounding box, measurements, related layers, and suggestion. ### `quickthumb render` @@ -218,8 +219,9 @@ quickthumb serve deck.json --port 4040 --no-open ``` A Python source should define one module-level `Deck` or `Canvas` as `deck`, -`slides`, or `canvas`. A JSON source may be a canvas spec or a deck object with a -top-level `slides` list. Use `--host` and `--port` to change the listening +`slides`, or `canvas`. A JSON source must declare `kind: "canvas"` or `kind: "deck"`; +Canvas documents contain `layers`, while Deck documents contain a top-level `slides` list. +Use `--host` and `--port` to change the listening address, `--no-open` to suppress the initial browser window, and repeat `--var KEY=VALUE` for JSON template substitutions. diff --git a/docs/getting-started.md b/docs/getting-started.md index 9aa2342..2294209 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -191,6 +191,7 @@ from quickthumb import Canvas config = """ { + "kind": "canvas", "width": 1280, "height": 720, "layers": [ diff --git a/docs/json-schema.md b/docs/json-schema.md index 80b0f99..34c40af 100644 --- a/docs/json-schema.md +++ b/docs/json-schema.md @@ -26,10 +26,11 @@ canvas = Canvas.from_json(json_str) ## JSON structure -A quickthumb JSON document has three required top-level fields, plus an optional `theme` block: +A Canvas JSON document has four required top-level fields, plus an optional `theme` block: ```json { + "kind": "canvas", "width": 1280, "height": 720, "theme": { ... }, @@ -37,7 +38,10 @@ A quickthumb JSON document has three required top-level fields, plus an optional } ``` -Every layer object requires a `"type"` discriminator field. Layers render in array order — first item is backmost. +The top-level `"kind"` discriminator is required by the CLI and must be `"canvas"` or `"deck"`. +Canvas documents carry `layers`; Deck documents carry `slides` and may also define a shared +default size, theme, and transitions. Every layer object requires its own `"type"` +discriminator field. Layers render in array order — first item is backmost. ## Published schema @@ -53,6 +57,9 @@ quickthumb schema --output quickthumb.schema.json and constrained generation. `Canvas.from_json()` remains the source of truth for what quickthumb can load. + The CLI and `serve` command require the top-level `kind` discriminator. The + direct `Canvas.from_json()` API continues to accept legacy Canvas strings without it. + Authoring conveniences such as `$theme.*` are resolved by quickthumb before model validation, so generic JSON Schema validators may reject unresolved authoring specs that quickthumb itself can load. @@ -75,6 +82,7 @@ Define brand tokens once in a top-level `theme` block and reference them anywher ```json { + "kind": "canvas", "width": 1280, "height": 720, "theme": { @@ -521,7 +529,7 @@ quickthumb JSON is well-suited for LLM generation because the schema is flat, ev Generate a quickthumb JSON config for a 1280×720 YouTube thumbnail. Rules: -- Top-level fields: "width", "height", "layers" (optional "theme" for shared tokens) +- Top-level fields: "kind": "canvas", "width", "height", "layers" (optional "theme" for shared tokens) - Every layer must have a "type" field: "background", "text", "image", "shape", "svg", "group", or "outline" - Every effect must have a "type" field: "stroke", "shadow", "glow", "filter", "background", or "grain" - Positions are [x, y] arrays — values can be integers (px) or percentage strings like "50%" diff --git a/examples/launch_announcement.json b/examples/launch_announcement.json index 1575ac2..b46360c 100644 --- a/examples/launch_announcement.json +++ b/examples/launch_announcement.json @@ -1,4 +1,5 @@ { + "kind": "canvas", "width": 1280, "height": 720, "theme": { diff --git a/examples/shorts_cover_agent.json b/examples/shorts_cover_agent.json index c90ca4d..b477ef0 100644 --- a/examples/shorts_cover_agent.json +++ b/examples/shorts_cover_agent.json @@ -1,4 +1,5 @@ { + "kind": "canvas", "width": 1080, "height": 1920, "layers": [ diff --git a/quickthumb/_document.py b/quickthumb/_document.py new file mode 100644 index 0000000..9781602 --- /dev/null +++ b/quickthumb/_document.py @@ -0,0 +1,25 @@ +"""Shared validation for top-level JSON document discriminators.""" + +from __future__ import annotations + +from typing import Literal, cast + +from quickthumb.errors import ValidationError + +DocumentKind = Literal["canvas", "deck"] + + +def require_document_kind(raw: object) -> DocumentKind: + """Validate and return the top-level kind of a JSON document.""" + if not isinstance(raw, dict): + raise ValidationError("JSON document must be an object with a 'kind' field.") + + document = cast(dict[str, object], raw) + kind = document.get("kind") + if kind not in ("canvas", "deck"): + raise ValidationError("JSON document 'kind' must be either 'canvas' or 'deck'.") + if kind == "canvas" and "slides" in raw: + raise ValidationError("Canvas JSON must not contain a top-level 'slides' field.") + if kind == "deck" and "layers" in raw: + raise ValidationError("Deck JSON must not contain a top-level 'layers' field.") + return cast(DocumentKind, kind) diff --git a/quickthumb/_serve.py b/quickthumb/_serve.py index b020ef4..b265319 100644 --- a/quickthumb/_serve.py +++ b/quickthumb/_serve.py @@ -20,6 +20,7 @@ from jinja2 import Environment, Template +from quickthumb._document import require_document_kind from quickthumb.canvas import _VAR_RE, Canvas, _is_theme_reference from quickthumb.deck import Deck from quickthumb.errors import ValidationError @@ -288,7 +289,8 @@ def _render_json(self) -> str: if self.variables: text = _substitute_variables(text, self.variables) payload = json.loads(text) - if isinstance(payload, dict) and "slides" in payload: + kind = require_document_kind(payload) + if kind == "deck": return Deck.from_json(text).to_html() return Canvas.from_json(text).to_html() diff --git a/quickthumb/canvas.py b/quickthumb/canvas.py index 38da3e7..eaac380 100644 --- a/quickthumb/canvas.py +++ b/quickthumb/canvas.py @@ -902,6 +902,7 @@ def to_json(self) -> str: layers_json.append(self._omit_unset_composition_fields(layer_json)) payload: dict[str, Any] = { + "kind": "canvas", "width": self.width, "height": self.height, "layers": layers_json, @@ -950,6 +951,12 @@ def _from_json(cls, data: str) -> Self: CanvasModel.model_validate_json(data) # raises ValidationError with good message raw = cast(dict[str, Any], raw) + kind = raw.pop("kind", None) + if kind is not None and kind != "canvas": + raise ValidationError("Canvas JSON 'kind' must be 'canvas'.") + if "slides" in raw: + raise ValidationError("Canvas JSON must not contain a top-level 'slides' field.") + theme = raw.pop("theme", {}) if not isinstance(theme, dict): raise ValidationError("'theme' must be a JSON object of token groups") diff --git a/quickthumb/cli.py b/quickthumb/cli.py index 2b9826f..0ffebef 100644 --- a/quickthumb/cli.py +++ b/quickthumb/cli.py @@ -2,8 +2,9 @@ import json import re +from collections.abc import Iterable from pathlib import Path -from typing import Annotated, Protocol, TypeAlias +from typing import Annotated, Protocol, TypeAlias, cast import typer from PIL import Image @@ -17,6 +18,7 @@ compare_images, create_diff_image, ) +from quickthumb._document import require_document_kind from quickthumb.canvas import _VAR_RE, Canvas, _is_theme_reference from quickthumb.deck import Deck, DeckDiagnostic from quickthumb.errors import RenderingError, ValidationError @@ -80,7 +82,8 @@ def _load_canvas(spec: Path, variables: dict[str, str]) -> Diagnosable: text = _substitute_vars(text, variables) raw = json.loads(text) - if isinstance(raw, dict) and "slides" in raw: + kind = require_document_kind(raw) + if kind == "deck": return Deck.from_json(text) return Canvas.from_json(text) @@ -125,7 +128,7 @@ def _diagnostic_payload(finding: _DiagnosticLike) -> dict: def _filter_diagnostics( - findings: list[_DiagnosticLike], ignored_codes: set[str] + findings: Iterable[_DiagnosticLike], ignored_codes: set[str] ) -> list[_DiagnosticLike]: return [finding for finding in findings if finding.code not in ignored_codes] @@ -134,8 +137,7 @@ def _should_fail(findings: list[_DiagnosticLike], fail_on: str) -> bool: if fail_on == "never": return False return any( - finding.severity == "error" - or (fail_on == "warning" and finding.severity == "warning") + finding.severity == "error" or (fail_on == "warning" and finding.severity == "warning") for finding in findings ) @@ -169,9 +171,7 @@ def _run_lint( err=True, ) raise typer.Exit(1) - normalized_fail_on, ignored = _validate_diagnostic_options( - fail_on, ignored_codes, lint_format - ) + normalized_fail_on, ignored = _validate_diagnostic_options(fail_on, ignored_codes, lint_format) try: source = _load_canvas(spec, _parse_var_options(var)) @@ -180,7 +180,9 @@ def _run_lint( raise typer.Exit(1) from None try: - diagnostics = _filter_diagnostics(source.diagnose(), ignored) + diagnostics = _filter_diagnostics( + cast(Iterable[_DiagnosticLike], source.diagnose()), ignored + ) except FileNotFoundError as error: _echo_input_error(FileNotFoundError(f"Referenced file not found: {error}"), lint_format) raise typer.Exit(1) from None @@ -193,9 +195,7 @@ def _run_lint( if lint_format == "json": error_count = sum(1 for finding in diagnostics if finding.severity == "error") - warning_count = sum( - 1 for finding in diagnostics if finding.severity == "warning" - ) + warning_count = sum(1 for finding in diagnostics if finding.severity == "warning") typer.echo( json.dumps( { @@ -305,6 +305,9 @@ def render( quality=quality, debug=debug, ) + except ValidationError as e: + typer.echo(str(e), err=True) + raise typer.Exit(1) from e except (RenderingError, OSError) as e: typer.echo(str(e), err=True) raise typer.Exit(2) from e @@ -566,7 +569,7 @@ def _render_once() -> None: debug=debug, ) typer.echo(str(output)) - except (RenderingError, OSError) as e: + except (ValidationError, RenderingError, OSError) as e: typer.echo(str(e), err=True) typer.echo(f"Watching {spec} … (Ctrl+C to stop)") diff --git a/quickthumb/deck.py b/quickthumb/deck.py index 649a750..9e588dd 100644 --- a/quickthumb/deck.py +++ b/quickthumb/deck.py @@ -85,6 +85,9 @@ def __init__( theme: dict | None = None, transition: Transition | dict | str | None = None, ): + for name, value in (("width", width), ("height", height)): + if value is not None and (isinstance(value, bool) or not isinstance(value, int)): + raise ValidationError(f"{name} must be an integer") if (width is None) != (height is None): raise ValidationError("Provide both width and height, or neither.") if width is not None and width <= 0: @@ -659,6 +662,7 @@ def to_json(self) -> str: import json payload: dict = {} + payload["kind"] = "deck" if self._width is not None: payload["width"] = self._width payload["height"] = self._height @@ -697,6 +701,11 @@ def from_json(cls, data: str) -> Self: raw = json.loads(data) if not isinstance(raw, dict) or "slides" not in raw: raise ValidationError("Deck JSON must be an object with a 'slides' list.") + kind = raw.get("kind") + if kind is not None and kind != "deck": + raise ValidationError("Deck JSON 'kind' must be 'deck'.") + if "layers" in raw: + raise ValidationError("Deck JSON must not contain a top-level 'layers' field.") slides_raw = raw["slides"] if not isinstance(slides_raw, list): raise ValidationError("Deck 'slides' must be a list of canvas specs.") @@ -709,6 +718,9 @@ def from_json(cls, data: str) -> Self: if transition is not None and not isinstance(transition, dict): raise ValidationError("Deck 'transition' must be a JSON object.") + for name, value in (("width", raw.get("width")), ("height", raw.get("height"))): + if value is not None and (isinstance(value, bool) or not isinstance(value, int)): + raise ValidationError(f"Deck '{name}' must be an integer.") deck = cls( width=raw.get("width"), height=raw.get("height"), @@ -729,6 +741,9 @@ def from_json(cls, data: str) -> Self: notes = slide.get("notes") if notes is not None and not isinstance(notes, str): raise ValidationError("Deck slide 'notes' must be a string.") + slide_theme = slide.get("theme", {}) + if not isinstance(slide_theme, dict): + raise ValidationError("Deck slide 'theme' must be an object of token groups.") slide = { key: value for key, value in slide.items() @@ -736,8 +751,8 @@ def from_json(cls, data: str) -> Self: } # Share the deck-level theme so $theme.* tokens resolve; a slide's # own theme block takes precedence. - if theme: - slide = {**slide, "theme": {**theme, **slide.get("theme", {})}} + if theme or "theme" in slide: + slide = {**slide, "theme": {**theme, **slide_theme}} if ( deck._width is not None and "width" not in slide diff --git a/quickthumb/models.py b/quickthumb/models.py index 0ca2c62..c5b4a23 100644 --- a/quickthumb/models.py +++ b/quickthumb/models.py @@ -1178,6 +1178,7 @@ class CanvasInspection(quickthumbModel): class CanvasModel(quickthumbModel): + kind: Literal["canvas"] = "canvas" width: PositiveInt | None = None height: PositiveInt | None = None platform: str | None = None @@ -1185,6 +1186,7 @@ class CanvasModel(quickthumbModel): class CanvasSpecModel(quickthumbModel): + kind: Literal["canvas"] = "canvas" width: PositiveInt | None = None height: PositiveInt | None = None platform: str | None = None diff --git a/quickthumb/schema.py b/quickthumb/schema.py index 2231b23..37c4afe 100644 --- a/quickthumb/schema.py +++ b/quickthumb/schema.py @@ -26,9 +26,12 @@ def canvas_json_schema() -> dict[str, Any]: schema["$id"] = QUICKTHUMB_SCHEMA_ID schema["title"] = "quickthumb Canvas JSON Spec" schema["description"] = ( - "A quickthumb canvas spec accepted by Canvas.from_json() and the quickthumb CLI." + "A quickthumb canvas spec accepted by the quickthumb CLI. Canvas.from_json() also " + "accepts legacy documents without the top-level kind." ) + schema["properties"]["kind"] = {"const": "canvas", "type": "string"} schema["properties"]["platform"] = platform_schema + schema["required"] = ["kind", *schema.get("required", [])] schema["anyOf"] = [ { "properties": { diff --git a/tests/test_background_layers.py b/tests/test_background_layers.py index 6490370..31fa11f 100644 --- a/tests/test_background_layers.py +++ b/tests/test_background_layers.py @@ -272,6 +272,7 @@ def test_should_serialize_background_layer_to_json(self): # tuple colors appear as hex strings, not arrays. assert json.loads(canvas.to_json()) == snapshot( { + "kind": "canvas", "width": 1920, "height": 1080, "layers": [ diff --git a/tests/test_canvas.py b/tests/test_canvas.py index d82e37c..1a98253 100644 --- a/tests/test_canvas.py +++ b/tests/test_canvas.py @@ -103,6 +103,7 @@ def test_should_serialize_multiple_layers_in_order(self): # Then: All layers should be serialized in correct order assert canvas_dict == snapshot( { + "kind": "canvas", "width": 1920, "height": 1080, "layers": [ @@ -338,6 +339,7 @@ def draw_dot(image: Image.Image, *, color: str = "red", size: int = 10) -> None: json_str = canvas.to_json() assert json.loads(json_str) == snapshot( { + "kind": "canvas", "width": 100, "height": 100, "layers": [ @@ -469,6 +471,7 @@ def test_should_substitute_var_placeholders(self, placeholder, key): # Then: The layer has the substituted color value in its JSON representation assert json.loads(canvas.to_json()) == snapshot( { + "kind": "canvas", "width": 100, "height": 100, "layers": [ diff --git a/tests/test_cli.py b/tests/test_cli.py index 3f4b2cd..d3da7b1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -10,6 +10,7 @@ SIMPLE_SPEC = json.dumps( { + "kind": "canvas", "width": 100, "height": 100, "layers": [ @@ -69,7 +70,16 @@ def test_should_include_canvas_theme_platform_and_layer_contracts(self): # then: the schema describes canvas fields, theme tokens, and layer types assert result.exit_code == 0 payload = json.loads(result.output) - assert set(payload["properties"]) == {"height", "layers", "platform", "theme", "width"} + assert set(payload["properties"]) == { + "height", + "kind", + "layers", + "platform", + "theme", + "width", + } + assert payload["properties"]["kind"] == {"const": "canvas", "type": "string"} + assert "kind" in payload["required"] assert payload["anyOf"][0]["properties"] == { "width": {"exclusiveMinimum": 0, "type": "integer"}, "height": {"exclusiveMinimum": 0, "type": "integer"}, @@ -354,6 +364,7 @@ def test_should_exit_2_on_rendering_error(self): f.write( json.dumps( { + "kind": "canvas", "width": 100, "height": 100, "layers": [ @@ -377,6 +388,39 @@ def test_should_exit_2_on_rendering_error(self): finally: os.unlink(bad_spec) + def test_should_report_deck_render_validation_errors_without_traceback(self): + """render maps invalid Deck audio metadata to a clean input error.""" + from quickthumb.cli import app + + # given: a Deck JSON document referring to an audio file that does not exist + with tempfile.NamedTemporaryFile(suffix=".json", mode="w", delete=False) as spec_file: + json.dump( + { + "kind": "deck", + "slides": [ + { + "width": 100, + "height": 100, + "layers": [], + "audio": "missing-narration.wav", + } + ], + }, + spec_file, + ) + spec_path = spec_file.name + + # when: rendering the Deck to the narrated MP4 path + try: + result = CliRunner().invoke(app, ["render", spec_path, "-o", "deck.mp4"]) + finally: + os.unlink(spec_path) + + # then: the CLI returns an input error without leaking a traceback + assert result.exit_code == 1 + assert "Audio file not found" in result.output + assert "Traceback" not in result.output + def test_should_substitute_var_placeholders(self): """Test that --var KEY=VALUE substitutes $KEY placeholders in the spec before parsing""" # Given: A spec template using $bg_color and a --var flag @@ -384,6 +428,7 @@ def test_should_substitute_var_placeholders(self): template_spec = json.dumps( { + "kind": "canvas", "width": 100, "height": 100, "layers": [{"type": "background", "color": "$bg_color"}], @@ -447,6 +492,7 @@ def test_should_exit_1_on_unresolved_placeholder(self): template_spec = json.dumps( { + "kind": "canvas", "width": 100, "height": 100, "layers": [{"type": "background", "color": "$missing_var"}], @@ -519,6 +565,7 @@ def test_should_substitute_braced_var_placeholders(self): template_spec = json.dumps( { + "kind": "canvas", "width": 100, "height": 100, "layers": [{"type": "background", "color": "${bg}"}], @@ -546,6 +593,7 @@ def test_should_render_spec_with_theme_tokens(self): themed_spec = json.dumps( { + "kind": "canvas", "width": 100, "height": 100, "theme": {"colors": {"bg": "#FF0000"}}, @@ -731,11 +779,40 @@ def fake_watch(spec): assert result.exit_code == 0 assert "cannot write output" in result.output + def test_should_keep_watching_after_deck_render_validation_error(self, spec_file, monkeypatch): + """watch reports Deck validation errors and continues to the next change.""" + import sys + import types + + import quickthumb.cli as cli + from quickthumb import Canvas, Deck + + # given: a Deck whose narration path is invalid and a watcher interrupted after startup + deck = Deck(100, 100).slide(Canvas(), audio="missing-narration.wav") + + def fake_load_canvas(spec, variables): + return deck + + def fake_watch(spec): + raise KeyboardInterrupt + yield + + monkeypatch.setattr(cli, "_load_canvas", fake_load_canvas) + monkeypatch.setitem(sys.modules, "watchfiles", types.SimpleNamespace(watch=fake_watch)) + + # when: the user starts watching a Deck MP4 render + result = CliRunner().invoke(cli.app, ["watch", spec_file, "-o", "deck.mp4"]) + + # then: validation output is reported and the watcher exits normally + assert result.exit_code == 0 + assert "Audio file not found" in result.output + class TestCLILint: """Test suite for the quickthumb lint subcommand""" def _write_spec(self, spec: dict) -> str: + spec = {"kind": "deck" if "slides" in spec else "canvas", **spec} with tempfile.NamedTemporaryFile(suffix=".json", mode="w", delete=False) as f: f.write(json.dumps(spec)) return f.name @@ -1423,9 +1500,7 @@ def test_should_emit_json_error_without_traceback_for_invalid_layer(self): from quickthumb.cli import app # given: a spec with an unknown layer type - spec_path = self._write_spec( - {"width": 100, "height": 100, "layers": [{"type": "unknown"}]} - ) + spec_path = self._write_spec({"width": 100, "height": 100, "layers": [{"type": "unknown"}]}) # when try: @@ -1440,6 +1515,108 @@ def test_should_emit_json_error_without_traceback_for_invalid_layer(self): assert payload["error"]["code"] == "invalid-spec" assert "unknown" in payload["error"]["message"] + def test_should_reject_an_ambiguous_canvas_document(self): + """lint rejects a Canvas document that also claims to contain Deck slides.""" + from quickthumb.cli import app + + # given: a document explicitly marked as Canvas but carrying a top-level slides field + spec_path = self._write_spec( + { + "kind": "canvas", + "width": 100, + "height": 100, + "layers": [], + "slides": [], + } + ) + + # when: linting the ambiguous JSON document + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: the structured invalid-spec contract is preserved + assert result.exit_code == 1 + payload = json.loads(result.output) + assert payload["error"]["code"] == "invalid-spec" + assert "slides" in payload["error"]["message"] + + def test_should_require_a_top_level_document_kind(self): + """lint rejects JSON documents without an explicit Canvas or Deck discriminator.""" + from quickthumb.cli import app + + # given: a legacy shape-only JSON document + with tempfile.NamedTemporaryFile(suffix=".json", mode="w", delete=False) as spec_file: + json.dump({"width": 100, "height": 100, "layers": []}, spec_file) + spec_path = spec_file.name + + # when: linting the document through the CLI boundary + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: the missing discriminator is a structured invalid-spec error + assert result.exit_code == 1 + payload = json.loads(result.output) + assert payload["error"]["code"] == "invalid-spec" + assert "kind" in payload["error"]["message"] + + @pytest.mark.parametrize("width", ["100", 100.5, True]) + def test_should_reject_non_integer_deck_dimensions(self, width): + """lint rejects Deck dimensions that are not positive JSON integers.""" + from quickthumb.cli import app + + # given: a Deck with one malformed root dimension + spec_path = self._write_spec({"kind": "deck", "width": width, "height": 100, "slides": []}) + + # when: linting the malformed Deck + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: invalid dimensions produce structured JSON and no traceback + assert result.exit_code == 1 + assert "Traceback" not in result.output + payload = json.loads(result.output) + assert payload["error"]["code"] == "invalid-spec" + assert "integer" in payload["error"]["message"] + + def test_should_reject_non_mapping_deck_slide_theme(self): + """lint rejects a non-object slide theme instead of leaking a merge TypeError.""" + from quickthumb.cli import app + + # given: a Deck with a shared theme and an invalid per-slide theme value + spec_path = self._write_spec( + { + "kind": "deck", + "theme": {"brand": "#B8FF00"}, + "slides": [ + { + "width": 100, + "height": 100, + "theme": "not-an-object", + "layers": [], + } + ], + } + ) + + # when: linting the malformed slide theme + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: the public invalid-spec response is structured and traceback-free + assert result.exit_code == 1 + assert "Traceback" not in result.output + payload = json.loads(result.output) + assert payload["error"]["code"] == "invalid-spec" + assert "theme" in payload["error"]["message"] + def test_should_support_deck_specs_and_preserve_slide_diagnostic_fields(self): """lint accepts deck JSON and includes slide and layer diagnostic context""" from quickthumb.cli import app @@ -1505,9 +1682,7 @@ def test_should_support_diagnose_alias_and_fail_on_filters(self): # when: warnings are ignored for exit status and then filtered entirely try: - warning_result = CliRunner().invoke( - app, ["diagnose", spec_path, "--fail-on", "error"] - ) + warning_result = CliRunner().invoke(app, ["diagnose", spec_path, "--fail-on", "error"]) ignored_result = CliRunner().invoke( app, ["diagnose", spec_path, "--ignore", "edge-crowding", "--format", "json"], diff --git a/tests/test_deck.py b/tests/test_deck.py index 320ea0e..88f6e1d 100644 --- a/tests/test_deck.py +++ b/tests/test_deck.py @@ -388,6 +388,7 @@ def test_should_round_trip_per_slide_audio_metadata(self): restored = Deck.from_json(deck.to_json()) # then + assert payload["kind"] == "deck" assert payload["slides"][0]["audio"] == { "path": "voice.wav", "volume": 1.0, diff --git a/tests/test_diagnostics.py b/tests/test_diagnostics.py index 947434b..ac6edda 100644 --- a/tests/test_diagnostics.py +++ b/tests/test_diagnostics.py @@ -2633,8 +2633,20 @@ def test_should_normalize_platform_aliases_to_canonical_names(self): ) == ( "youtube-thumbnail", "instagram-reels", - {"width": 1280, "height": 720, "layers": [], "platform": "youtube-thumbnail"}, - {"width": 1080, "height": 1920, "layers": [], "platform": "instagram-reels"}, + { + "kind": "canvas", + "width": 1280, + "height": 720, + "layers": [], + "platform": "youtube-thumbnail", + }, + { + "kind": "canvas", + "width": 1080, + "height": 1920, + "layers": [], + "platform": "instagram-reels", + }, ) diff --git a/tests/test_image_layers.py b/tests/test_image_layers.py index 5f00725..baa6959 100644 --- a/tests/test_image_layers.py +++ b/tests/test_image_layers.py @@ -740,6 +740,7 @@ def test_should_serialize_percentage_position_to_json(self): # Then assert data == snapshot( { + "kind": "canvas", "width": 1920, "height": 1080, "layers": [ @@ -781,6 +782,7 @@ def test_should_serialize_image_with_effects_to_json(self): assert data == snapshot( { + "kind": "canvas", "width": 400, "height": 300, "layers": [ @@ -896,6 +898,7 @@ def test_should_serialize_image_with_filter_effect_to_json(self): assert data == snapshot( { + "kind": "canvas", "width": 400, "height": 300, "layers": [ diff --git a/tests/test_serve.py b/tests/test_serve.py index 551882f..3aa0d3b 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -88,6 +88,7 @@ def test_should_render_variable_substituted_json_decks(self, tmp_path: Path): source_path.write_text( json.dumps( { + "kind": "deck", "width": 320, "height": 180, "slides": [ @@ -117,6 +118,7 @@ def test_should_preserve_theme_references_while_substituting_json_variables( source_path.write_text( json.dumps( { + "kind": "deck", "width": 320, "height": 180, "theme": {"colors": {"background": "$ACCENT"}}, @@ -139,7 +141,8 @@ def test_should_render_canvas_json_and_static_html_sources(self, tmp_path: Path) # given: one canvas JSON document and one standalone HTML document canvas_path = tmp_path / "canvas.json" canvas_path.write_text( - json.dumps({"width": 80, "height": 40, "layers": []}), encoding="utf-8" + json.dumps({"kind": "canvas", "width": 80, "height": 40, "layers": []}), + encoding="utf-8", ) html_path = tmp_path / "slides.html" html_path.write_text("Existing slides", encoding="utf-8") From 65a780457ee00c99e43ec270b63c81d5f3ed4f1a Mon Sep 17 00:00:00 2001 From: sjquant Date: Wed, 22 Jul 2026 00:14:07 +0900 Subject: [PATCH 4/5] fix: harden JSON envelope validation --- docs/json-schema.md | 14 +++++++--- quickthumb/__init__.py | 3 ++- quickthumb/_document.py | 12 +++++++++ quickthumb/_serve.py | 9 ++----- quickthumb/_validation.py | 20 ++++++++++++++ quickthumb/canvas.py | 16 +++++------ quickthumb/cli.py | 22 ++++++++------- quickthumb/deck.py | 19 ++++--------- quickthumb/schema.py | 57 +++++++++++++++++++++++++++++++++++++++ tests/test_canvas.py | 9 +++++++ tests/test_cli.py | 57 +++++++++++++++++++++++++++++++++++++++ 11 files changed, 195 insertions(+), 43 deletions(-) create mode 100644 quickthumb/_validation.py diff --git a/docs/json-schema.md b/docs/json-schema.md index 34c40af..485ae34 100644 --- a/docs/json-schema.md +++ b/docs/json-schema.md @@ -50,21 +50,27 @@ Emit the current JSON Schema from the same Pydantic models used by `Canvas.from_ ```bash quickthumb schema > quickthumb.schema.json quickthumb schema --output quickthumb.schema.json +quickthumb schema --document --output quickthumb.document-schema.json ``` +The same schemas are available from Python as `canvas_json_schema()` and +`document_json_schema()`. + !!! note "Schema scope" - `quickthumb schema` describes concrete quickthumb specs for external tooling - and constrained generation. `Canvas.from_json()` remains the source of truth + `quickthumb schema` describes concrete Canvas specs for external tooling and + constrained generation. Use `quickthumb schema --document` for the combined + Canvas/Deck discriminator schema. `Canvas.from_json()` remains the source of truth for what quickthumb can load. The CLI and `serve` command require the top-level `kind` discriminator. The - direct `Canvas.from_json()` API continues to accept legacy Canvas strings without it. + direct `Canvas.from_json()` and `Deck.from_json()` APIs continue to accept + legacy strings without it; newly serialized documents always include `kind`. Authoring conveniences such as `$theme.*` are resolved by quickthumb before model validation, so generic JSON Schema validators may reject unresolved authoring specs that quickthumb itself can load. -The command writes deterministic JSON only, so it can be checked into a repo, piped into an editor, or passed directly to a constrained-generation API. The schema includes the current canvas fields, built-in layer discriminators, effects, animations, supported platform presets, and the optional top-level `theme` block. +The command writes deterministic JSON only, so it can be checked into a repo, piped into an editor, or passed directly to a constrained-generation API. The Canvas schema includes the current canvas fields, built-in layer discriminators, effects, animations, supported platform presets, and the optional top-level `theme` block. The `--document` schema adds the Deck root and maps both document kinds through `kind`. For constrained generation, prefer concrete resolved values in typed fields: diff --git a/quickthumb/__init__.py b/quickthumb/__init__.py index a4fc251..34de980 100644 --- a/quickthumb/__init__.py +++ b/quickthumb/__init__.py @@ -60,7 +60,7 @@ Wheel, Wipe, ) -from quickthumb.schema import canvas_json_schema +from quickthumb.schema import canvas_json_schema, document_json_schema from quickthumb.transitions import Transition __all__ = [ @@ -126,5 +126,6 @@ "VideoOptions", "Transition", "canvas_json_schema", + "document_json_schema", "transitions", ] diff --git a/quickthumb/_document.py b/quickthumb/_document.py index 9781602..a1220d7 100644 --- a/quickthumb/_document.py +++ b/quickthumb/_document.py @@ -2,8 +2,11 @@ from __future__ import annotations +import json from typing import Literal, cast +from quickthumb.canvas import Canvas +from quickthumb.deck import Deck from quickthumb.errors import ValidationError DocumentKind = Literal["canvas", "deck"] @@ -23,3 +26,12 @@ def require_document_kind(raw: object) -> DocumentKind: if kind == "deck" and "layers" in raw: raise ValidationError("Deck JSON must not contain a top-level 'layers' field.") return cast(DocumentKind, kind) + + +def load_document(text: str) -> Canvas | Deck: + """Parse a discriminated Canvas or Deck JSON document.""" + raw = json.loads(text) + kind = require_document_kind(raw) + if kind == "deck": + return Deck.from_json(text) + return Canvas.from_json(text) diff --git a/quickthumb/_serve.py b/quickthumb/_serve.py index b265319..dd6569f 100644 --- a/quickthumb/_serve.py +++ b/quickthumb/_serve.py @@ -2,7 +2,6 @@ from __future__ import annotations -import json import re import runpy import sys @@ -20,7 +19,7 @@ from jinja2 import Environment, Template -from quickthumb._document import require_document_kind +from quickthumb._document import load_document from quickthumb.canvas import _VAR_RE, Canvas, _is_theme_reference from quickthumb.deck import Deck from quickthumb.errors import ValidationError @@ -288,11 +287,7 @@ def _render_json(self) -> str: text = self.path.read_text(encoding="utf-8") if self.variables: text = _substitute_variables(text, self.variables) - payload = json.loads(text) - kind = require_document_kind(payload) - if kind == "deck": - return Deck.from_json(text).to_html() - return Canvas.from_json(text).to_html() + return load_document(text).to_html() def version(self) -> str: """Return a cheap change token used by the browser reload poller.""" diff --git a/quickthumb/_validation.py b/quickthumb/_validation.py new file mode 100644 index 0000000..df5c039 --- /dev/null +++ b/quickthumb/_validation.py @@ -0,0 +1,20 @@ +"""Shared validation helpers for public document models.""" + +from __future__ import annotations + +from typing import Any + +from quickthumb.errors import ValidationError + + +def validate_dimensions(width: Any, height: Any) -> None: + """Validate an optional width/height pair used by Canvas and Deck.""" + for name, value in (("width", width), ("height", height)): + if value is not None and (isinstance(value, bool) or not isinstance(value, int)): + raise ValidationError(f"{name} must be an integer") + if (width is None) != (height is None): + raise ValidationError("Provide both width and height, or neither.") + if width is not None and width <= 0: + raise ValidationError("width must be > 0") + if height is not None and height <= 0: + raise ValidationError("height must be > 0") diff --git a/quickthumb/canvas.py b/quickthumb/canvas.py index eaac380..f3d9669 100644 --- a/quickthumb/canvas.py +++ b/quickthumb/canvas.py @@ -20,6 +20,7 @@ from quickthumb._measurements import BBox, LayerMeasurement, measure_layers from quickthumb._shapes import ShapeEngine from quickthumb._text import TextEngine +from quickthumb._validation import validate_dimensions from quickthumb.errors import RenderingError, ValidationError from quickthumb.models import ( Align, @@ -148,6 +149,7 @@ def __init__( layers: list[RenderableLayer] | None = None, platform: str | None = None, ): + validate_dimensions(width, height) if platform is not None: try: platform = PLATFORM_SAFE_MARGIN_PRESETS[platform].name @@ -156,13 +158,6 @@ def __init__( raise ValidationError( f"Unsupported platform preset '{platform}'. Supported: {supported}" ) from None - if (width is None) != (height is None): - raise ValidationError("Provide both width and height, or neither.") - if width is not None and width <= 0: - raise ValidationError("width must be > 0") - if height is not None and height <= 0: - raise ValidationError("height must be > 0") - # An unsized canvas defers its dimensions until a Deck injects them. Layer # builders never need a size (coordinates resolve at render time), so the # placeholder ctx stays valid; render/diagnose/serialize guard on _has_size. @@ -960,10 +955,15 @@ def _from_json(cls, data: str) -> Self: theme = raw.pop("theme", {}) if not isinstance(theme, dict): raise ValidationError("'theme' must be a JSON object of token groups") + unknown = sorted(set(raw) - {"width", "height", "platform", "layers"}) + if unknown: + raise ValidationError(f"Canvas JSON contains unknown field(s): {', '.join(unknown)}") + if "layers" not in raw: + raise ValidationError("Canvas JSON must contain a 'layers' list.") raw = _resolve_theme_tokens(raw, theme) raw = cast(dict[str, Any], raw) - layers_raw = raw.get("layers", []) + layers_raw = raw["layers"] if not isinstance(layers_raw, list): CanvasModel.model_validate_json(data) # raises ValidationError with good message diff --git a/quickthumb/cli.py b/quickthumb/cli.py index 0ffebef..790f02c 100644 --- a/quickthumb/cli.py +++ b/quickthumb/cli.py @@ -18,11 +18,11 @@ compare_images, create_diff_image, ) -from quickthumb._document import require_document_kind +from quickthumb._document import load_document from quickthumb.canvas import _VAR_RE, Canvas, _is_theme_reference from quickthumb.deck import Deck, DeckDiagnostic from quickthumb.errors import RenderingError, ValidationError -from quickthumb.schema import canvas_json_schema +from quickthumb.schema import canvas_json_schema, document_json_schema _VALID_FORMATS = {"PNG", "JPEG", "WEBP"} _DIAGNOSTIC_CODES = { @@ -81,11 +81,7 @@ def _load_canvas(spec: Path, variables: dict[str, str]) -> Diagnosable: if variables: text = _substitute_vars(text, variables) - raw = json.loads(text) - kind = require_document_kind(raw) - if kind == "deck": - return Deck.from_json(text) - return Canvas.from_json(text) + return load_document(text) def _echo_input_error( @@ -234,9 +230,17 @@ def schema( Path | None, typer.Option("-o", "--output", help="Write schema JSON to a file instead of stdout"), ] = None, + document: Annotated[ + bool, + typer.Option( + "--document", + help="Emit the combined Canvas/Deck discriminator schema", + ), + ] = False, ) -> None: - """Emit the JSON Schema for quickthumb canvas specs.""" - payload = json.dumps(canvas_json_schema(), indent=2, sort_keys=True) + "\n" + """Emit the JSON Schema for quickthumb Canvas or Deck specs.""" + schema_payload = document_json_schema() if document else canvas_json_schema() + payload = json.dumps(schema_payload, indent=2, sort_keys=True) + "\n" if output is None: typer.echo(payload, nl=False) return diff --git a/quickthumb/deck.py b/quickthumb/deck.py index 9e588dd..f91b36e 100644 --- a/quickthumb/deck.py +++ b/quickthumb/deck.py @@ -18,6 +18,7 @@ from typing_extensions import Self from quickthumb._base import FileFormat, aspect_ratio_dimensions +from quickthumb._validation import validate_dimensions from quickthumb.canvas import Canvas from quickthumb.errors import RenderingError, ValidationError from quickthumb.models import ( @@ -85,15 +86,7 @@ def __init__( theme: dict | None = None, transition: Transition | dict | str | None = None, ): - for name, value in (("width", width), ("height", height)): - if value is not None and (isinstance(value, bool) or not isinstance(value, int)): - raise ValidationError(f"{name} must be an integer") - if (width is None) != (height is None): - raise ValidationError("Provide both width and height, or neither.") - if width is not None and width <= 0: - raise ValidationError("width must be > 0") - if height is not None and height <= 0: - raise ValidationError("height must be > 0") + validate_dimensions(width, height) if theme is not None and not isinstance(theme, dict): raise ValidationError("theme must be a dict of token groups.") @@ -704,8 +697,9 @@ def from_json(cls, data: str) -> Self: kind = raw.get("kind") if kind is not None and kind != "deck": raise ValidationError("Deck JSON 'kind' must be 'deck'.") - if "layers" in raw: - raise ValidationError("Deck JSON must not contain a top-level 'layers' field.") + unknown = sorted(set(raw) - {"kind", "width", "height", "theme", "transition", "slides"}) + if unknown: + raise ValidationError(f"Deck JSON contains unknown field(s): {', '.join(unknown)}") slides_raw = raw["slides"] if not isinstance(slides_raw, list): raise ValidationError("Deck 'slides' must be a list of canvas specs.") @@ -718,9 +712,6 @@ def from_json(cls, data: str) -> Self: if transition is not None and not isinstance(transition, dict): raise ValidationError("Deck 'transition' must be a JSON object.") - for name, value in (("width", raw.get("width")), ("height", raw.get("height"))): - if value is not None and (isinstance(value, bool) or not isinstance(value, int)): - raise ValidationError(f"Deck '{name}' must be an integer.") deck = cls( width=raw.get("width"), height=raw.get("height"), diff --git a/quickthumb/schema.py b/quickthumb/schema.py index 37c4afe..6fe418d 100644 --- a/quickthumb/schema.py +++ b/quickthumb/schema.py @@ -32,6 +32,7 @@ def canvas_json_schema() -> dict[str, Any]: schema["properties"]["kind"] = {"const": "canvas", "type": "string"} schema["properties"]["platform"] = platform_schema schema["required"] = ["kind", *schema.get("required", [])] + schema["additionalProperties"] = False schema["anyOf"] = [ { "properties": { @@ -47,3 +48,59 @@ def canvas_json_schema() -> dict[str, Any]: }, ] return schema + + +def document_json_schema() -> dict[str, Any]: + """Return a discriminated JSON Schema for Canvas and Deck documents.""" + canvas = canvas_json_schema() + canvas_defs = canvas.pop("$defs", {}) + canvas_document = {key: value for key, value in canvas.items() if key not in {"$schema", "$id"}} + deck_document = { + "type": "object", + "title": "quickthumb Deck JSON Spec", + "properties": { + "kind": {"const": "deck", "type": "string"}, + "width": {"exclusiveMinimum": 0, "type": "integer"}, + "height": {"exclusiveMinimum": 0, "type": "integer"}, + "theme": {"type": "object"}, + "transition": {"type": "object"}, + "slides": { + "type": "array", + "items": {"$ref": "#/$defs/CanvasDocument"}, + }, + }, + "required": ["kind", "slides"], + "additionalProperties": False, + "allOf": [ + { + "if": {"required": ["width"]}, + "then": {"required": ["height"]}, + }, + { + "if": {"required": ["height"]}, + "then": {"required": ["width"]}, + }, + ], + } + return { + "$schema": JSON_SCHEMA_DRAFT, + "$id": f"{QUICKTHUMB_SCHEMA_ID.rsplit('/', 1)[0]}/document-schema.json", + "title": "quickthumb JSON Document Spec", + "description": "A discriminated quickthumb Canvas or Deck JSON document.", + "oneOf": [ + {"$ref": "#/$defs/CanvasDocument"}, + {"$ref": "#/$defs/DeckDocument"}, + ], + "discriminator": { + "propertyName": "kind", + "mapping": { + "canvas": "#/$defs/CanvasDocument", + "deck": "#/$defs/DeckDocument", + }, + }, + "$defs": { + **canvas_defs, + "CanvasDocument": canvas_document, + "DeckDocument": deck_document, + }, + } diff --git a/tests/test_canvas.py b/tests/test_canvas.py index 1a98253..46f3e4f 100644 --- a/tests/test_canvas.py +++ b/tests/test_canvas.py @@ -223,6 +223,15 @@ def test_should_recreate_canvas_from_json(self): ), "no_such_fn_xyz", ), + ('{"kind": "canvas", "width": 100, "height": 100}', "layers"), + ( + '{"kind": "canvas", "width": 100, "height": 100, "layerz": []}', + "unknown field", + ), + ( + '{"kind": "canvas", "width": true, "height": 100, "layers": []}', + "integer", + ), ], ) def test_should_raise_error_for_invalid_json(self, json_str, match): diff --git a/tests/test_cli.py b/tests/test_cli.py index d3da7b1..485e461 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -124,6 +124,30 @@ def test_should_include_canvas_theme_platform_and_layer_contracts(self): "text", } + def test_should_emit_a_discriminated_canvas_and_deck_schema(self): + """schema --document describes both top-level JSON document kinds.""" + from quickthumb.cli import app + + # given: the quickthumb schema command + + # when: the user requests the combined document schema + result = CliRunner().invoke(app, ["schema", "--document"]) + + # then: the published schema maps both discriminated document roots + assert result.exit_code == 0 + payload = json.loads(result.output) + assert payload["title"] == "quickthumb JSON Document Spec" + assert payload["discriminator"] == { + "propertyName": "kind", + "mapping": { + "canvas": "#/$defs/CanvasDocument", + "deck": "#/$defs/DeckDocument", + }, + } + assert payload["$defs"]["DeckDocument"]["properties"]["slides"]["items"] == { + "$ref": "#/$defs/CanvasDocument" + } + def test_should_reflect_model_validation_for_common_fields(self): """schema preserves generated constraints from the public Pydantic models""" # given: the quickthumb CLI application @@ -1617,6 +1641,39 @@ def test_should_reject_non_mapping_deck_slide_theme(self): assert payload["error"]["code"] == "invalid-spec" assert "theme" in payload["error"]["message"] + @pytest.mark.parametrize( + ("spec", "message"), + [ + ({"kind": "canvas", "width": 100, "height": 100}, "layers"), + ( + {"kind": "canvas", "width": 100, "height": 100, "layerz": []}, + "unknown field", + ), + ( + {"kind": "canvas", "width": True, "height": 100, "layers": []}, + "integer", + ), + ], + ) + def test_should_reject_malformed_canvas_envelopes(self, spec, message): + """lint rejects missing, misspelled, and boolean Canvas envelope fields.""" + from quickthumb.cli import app + + # given: a Canvas document with one malformed top-level field + spec_path = self._write_spec(spec) + + # when: linting the malformed Canvas document + try: + result = CliRunner().invoke(app, ["lint", spec_path, "--format", "json"]) + finally: + os.unlink(spec_path) + + # then: the CLI emits a structured invalid-spec response + assert result.exit_code == 1 + payload = json.loads(result.output) + assert payload["error"]["code"] == "invalid-spec" + assert message in payload["error"]["message"] + def test_should_support_deck_specs_and_preserve_slide_diagnostic_fields(self): """lint accepts deck JSON and includes slide and layer diagnostic context""" from quickthumb.cli import app From b10da1b92c3b275c7a2bd1fd746004865c0617f1 Mon Sep 17 00:00:00 2001 From: sjquant Date: Wed, 22 Jul 2026 00:29:40 +0900 Subject: [PATCH 5/5] fix: satisfy quality formatting check --- quickthumb/_diagnostics.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/quickthumb/_diagnostics.py b/quickthumb/_diagnostics.py index 3e9bd2a..7b169ce 100644 --- a/quickthumb/_diagnostics.py +++ b/quickthumb/_diagnostics.py @@ -134,10 +134,7 @@ def _diagnose_impl(self) -> list[Diagnostic]: diagnostics.extend(self._diagnose_hidden_layers(measurements)) edge_ignored_layer_ids = set() for finding in diagnostics: - if ( - finding.code in {"off-canvas", "layer-hidden"} - and finding.layer_id is not None - ): + if finding.code in {"off-canvas", "layer-hidden"} and finding.layer_id is not None: edge_ignored_layer_ids.add(finding.layer_id) diagnostics.extend( self._diagnose_edge_crowding(measurements, ignored_layer_ids=edge_ignored_layer_ids)