From 15fbddd7ebcbf9445f03d1efee369122df5b19b0 Mon Sep 17 00:00:00 2001 From: Matt Beanland Date: Fri, 8 May 2026 20:46:14 +0930 Subject: [PATCH] v1 validation phase 1 --- artefact/CHANGELOG.md | 14 ++ artefact/lib/artefact.ex | 320 ++++++++++++++++++++++++++--- artefact/lib/artefact/uuid.ex | 8 + artefact/mix.exs | 2 +- artefact/test/artefact_test.exs | 298 +++++++++++++++++++++++++++ artefact_kino/CHANGELOG.md | 6 + artefact_kino/lib/artefact_kino.ex | 6 + artefact_kino/mix.exs | 6 +- 8 files changed, 630 insertions(+), 30 deletions(-) diff --git a/artefact/CHANGELOG.md b/artefact/CHANGELOG.md index 4bec341..906121b 100644 --- a/artefact/CHANGELOG.md +++ b/artefact/CHANGELOG.md @@ -5,6 +5,20 @@ SPDX-License-Identifier: MIT # Changelog +## 0.1.5 — 2026-05-05 + +- `Artefact.is_artefact?/1`, `Artefact.is_valid?/1`, `Artefact.validate/1`, `Artefact.validate!/1` — public validation API. Closes [#26], [#27] +- `Artefact.UUID.valid?/1` — UUIDv7 format predicate; used internally by validation and exposed for callers +- All public ops (`new/1`, `compose/3`, `combine/3`, `harmonise/4`, `graft/3`) now validate their input artefacts and validate the produced artefact before returning — corruption fails at the call site rather than several steps downstream. Closes [#30] (empty/invalid uuid rejected at op input), [#24] (non-list `:labels` rejected at op input) +- `Artefact.graft/3` enforces the no-new-islands rule — every new node in `args` must reach a bind-only key via `args.relationships`; raises `ArgumentError` listing orphan keys otherwise. Closes [#29] +- Validation rule-set: artefact uuid is UUIDv7; node uuid is UUIDv7; node `:labels` is a list of strings; node `:properties` is a map; relationship `:type` is a non-empty string; relationship `:from_id`/`:to_id` reference an extant node; node uuids, node ids and relationship ids are unique within the graph + +[#24]: https://github.com/diffo-dev/artefactory/issues/24 +[#26]: https://github.com/diffo-dev/artefactory/issues/26 +[#27]: https://github.com/diffo-dev/artefactory/issues/27 +[#29]: https://github.com/diffo-dev/artefactory/issues/29 +[#30]: https://github.com/diffo-dev/artefactory/issues/30 + ## 0.1.4 — 2026-05-05 - `Artefact.graft/3` — pipeline-friendly convenience for extending an artefact with new nodes and relationships declared inline (same shape as `Artefact.new`); every node in args MUST carry `:uuid` (no auto-find — uuid is the binding); nodes whose uuid lives in left bind to it (labels unioned, properties merged left-wins), nodes with new uuids are added; opts honour `:title` and `:description`; raises `ArgumentError` for missing uuid, duplicate keys, or relationship referencing an unknown key; records `:grafted` provenance source diff --git a/artefact/lib/artefact.ex b/artefact/lib/artefact.ex index c30e5a8..5af5be9 100644 --- a/artefact/lib/artefact.ex +++ b/artefact/lib/artefact.ex @@ -23,7 +23,24 @@ defmodule Artefact do (same shape as `new`'s inline form, but every node MUST carry `:uuid`) into an existing artefact. - Every operation records its lineage in the result's `metadata.provenance`. + Every operation records its lineage in the result's `metadata.provenance`, + validates its inputs, and validates the produced artefact before returning + — so corruption fails at the call site rather than five steps downstream. + + ## Validation + + * `is_artefact?/1` — true when the value is an `%Artefact{}` struct. + * `is_valid?/1` — true when the artefact passes every structural rule. + * `validate/1` — returns `:ok` or `{:error, reasons}` (a list of strings). + * `validate!/1` — returns `:ok` or raises `ArgumentError` with the + collected reasons. + + An artefact is *valid* when its uuid is a UUIDv7, every node has a + UUIDv7 uuid, every node's labels is a list of strings, every node's + properties is a map, every relationship's `from_id` and `to_id` + reference an extant node, every relationship type is a non-empty + string, and node uuids, node ids and relationship ids are unique + within the graph. ## Exporting @@ -55,6 +72,145 @@ defmodule Artefact do metadata: map() } + # ===================================================================== + # Validation API + # ===================================================================== + + @doc "Returns `true` when `value` is an `%Artefact{}` struct." + def is_artefact?(%__MODULE__{}), do: true + def is_artefact?(_), do: false + + @doc "Returns `true` when `value` is a valid artefact (see module docs)." + def is_valid?(value) do + case validate(value) do + :ok -> true + {:error, _} -> false + end + end + + @doc """ + Validate an artefact. Returns `:ok` or `{:error, reasons}` where reasons + is a list of human-readable strings describing each rule violation. + """ + def validate(%__MODULE__{} = a) do + errors = + [] + |> check(Artefact.UUID.valid?(a.uuid), "uuid is not a valid UUIDv7") + |> check_string_or_nil(a.title, :title) + |> check_string_or_nil(a.description, :description) + |> check_string_or_nil(a.base_label, :base_label) + |> check_graph(a.graph) + + case errors do + [] -> :ok + _ -> {:error, Enum.reverse(errors)} + end + end + + def validate(_), do: {:error, ["not an %Artefact{} struct"]} + + @doc """ + Validate an artefact. Returns `:ok` or raises `ArgumentError` with the + collected reasons. + """ + def validate!(value) do + case validate(value) do + :ok -> + :ok + + {:error, reasons} -> + raise ArgumentError, "invalid artefact: " <> Enum.join(reasons, "; ") + end + end + + defp check(errors, true, _msg), do: errors + defp check(errors, false, msg), do: [msg | errors] + + defp check_string_or_nil(errors, nil, _field), do: errors + defp check_string_or_nil(errors, value, _field) when is_binary(value), do: errors + defp check_string_or_nil(errors, _value, field), do: ["#{field} is not a string or nil" | errors] + + defp check_graph(errors, %Artefact.Graph{nodes: nodes, relationships: rels}) + when is_list(nodes) and is_list(rels) do + errors + |> check_nodes(nodes) + |> check_relationships(rels, nodes) + end + + defp check_graph(errors, _), do: ["graph is not %Artefact.Graph{} with list nodes/relationships" | errors] + + defp check_nodes(errors, nodes) do + errors = + nodes + |> Enum.with_index() + |> Enum.reduce(errors, fn {n, i}, acc -> check_node(acc, n, i) end) + + errors + |> check_unique(Enum.map(nodes, &node_uuid/1), "node uuid") + |> check_unique(Enum.map(nodes, &node_id/1), "node id") + end + + defp node_uuid(%Artefact.Node{uuid: u}), do: u + defp node_uuid(_), do: nil + defp node_id(%Artefact.Node{id: id}), do: id + defp node_id(_), do: nil + + defp check_node(errors, %Artefact.Node{} = n, idx) do + p = "node[#{idx}]" + + errors + |> check(is_binary(n.id) and n.id != "", "#{p} id is not a non-empty string") + |> check(Artefact.UUID.valid?(n.uuid), "#{p} uuid is not a valid UUIDv7") + |> check(is_list(n.labels) and Enum.all?(n.labels, &is_binary/1), + "#{p} labels is not a list of strings") + |> check(is_map(n.properties), "#{p} properties is not a map") + end + + defp check_node(errors, _, idx), do: ["node[#{idx}] is not %Artefact.Node{}" | errors] + + defp check_relationships(errors, rels, nodes) do + node_ids = MapSet.new(nodes, fn + %Artefact.Node{id: id} -> id + _ -> nil + end) + + errors = + rels + |> Enum.with_index() + |> Enum.reduce(errors, fn {r, i}, acc -> check_relationship(acc, r, i, node_ids) end) + + check_unique(errors, Enum.map(rels, fn + %Artefact.Relationship{id: id} -> id + _ -> nil + end), "relationship id") + end + + defp check_relationship(errors, %Artefact.Relationship{} = r, idx, node_ids) do + p = "relationship[#{idx}]" + + errors + |> check(is_binary(r.id) and r.id != "", "#{p} id is not a non-empty string") + |> check(is_binary(r.type) and r.type != "", "#{p} type is not a non-empty string") + |> check(MapSet.member?(node_ids, r.from_id), "#{p} from_id #{inspect(r.from_id)} not in graph") + |> check(MapSet.member?(node_ids, r.to_id), "#{p} to_id #{inspect(r.to_id)} not in graph") + |> check(is_map(r.properties), "#{p} properties is not a map") + end + + defp check_relationship(errors, _, idx, _), do: ["relationship[#{idx}] is not %Artefact.Relationship{}" | errors] + + defp check_unique(errors, list, label) do + duplicates = (list -- Enum.uniq(list)) |> Enum.uniq() |> Enum.reject(&is_nil/1) + + case duplicates do + [] -> errors + dupes -> ["duplicate #{label}s: #{inspect(dupes)}" | errors] + end + end + + # ===================================================================== + # Construction & Operations + # ===================================================================== + @doc """ Create a new Artefact. Defaults `base_label` and `title` to the short name of the calling module. Override with `title:` or `base_label:` in attrs. @@ -71,17 +227,30 @@ defmodule Artefact do default_base_label = caller_name && String.replace(caller_name, ~r/[^A-Za-z0-9]/, "") quote do - attrs = unquote(attrs) - metadata = %{provenance: %{source: :struct, module: unquote(caller)}} - title = Keyword.get(attrs, :title, unquote(caller_name)) - base_label = Keyword.get(attrs, :base_label, unquote(default_base_label)) + Artefact.do_new( + unquote(attrs), + unquote(caller), + unquote(caller_name), + unquote(default_base_label) + ) + end + end + + @doc false + def do_new(attrs, caller, caller_name, default_base_label) do + metadata = %{provenance: %{source: :struct, module: caller}} + title = Keyword.get(attrs, :title, caller_name) + base_label = Keyword.get(attrs, :base_label, default_base_label) - Artefact.build([ + result = + build([ {:title, title}, {:base_label, base_label}, {:metadata, metadata} | Keyword.drop(attrs, [:title, :base_label, :metadata]) ]) - end + + validate!(result) + result end @doc """ @@ -104,6 +273,9 @@ defmodule Artefact do @doc false def do_compose(%__MODULE__{} = a1, %__MODULE__{} = a2, opts, caller) do + validate!(a1) + validate!(a2) + base_label = Keyword.get(opts, :base_label, portmanteau(a1.base_label, a2.base_label)) title = Keyword.get(opts, :title, base_label) graph = merge_graphs(a1.graph, a2.graph) @@ -127,7 +299,11 @@ defmodule Artefact do } } - build([{:title, title}, {:base_label, base_label}, {:graph, graph}, {:metadata, metadata}]) + result = + build([{:title, title}, {:base_label, base_label}, {:graph, graph}, {:metadata, metadata}]) + + validate!(result) + result end @doc """ @@ -161,13 +337,20 @@ defmodule Artefact do @doc false def do_combine(%__MODULE__{} = heart, %__MODULE__{} = other, opts, caller) do + validate!(heart) + validate!(other) + {:ok, bindings} = Artefact.Binding.find(heart, other) - result = do_harmonise(heart, other, bindings, opts, caller) + harmonised = do_harmonise(heart, other, bindings, opts, caller) - case Keyword.fetch(opts, :description) do - {:ok, description} -> %{result | description: description} - :error -> result - end + result = + case Keyword.fetch(opts, :description) do + {:ok, description} -> %{harmonised | description: description} + :error -> harmonised + end + + validate!(result) + result end @doc """ @@ -231,10 +414,12 @@ defmodule Artefact do @doc false def do_graft(%__MODULE__{} = left, args, opts, caller) do + validate!(left) + node_specs = Keyword.get(args, :nodes, []) rel_specs = Keyword.get(args, :relationships, []) - validate_graft_node_uuids!(node_specs) + validate_graft_node_specs!(node_specs) validate_graft_unique_keys!(node_specs) left_by_uuid = Map.new(left.graph.nodes, &{&1.uuid, &1}) @@ -272,6 +457,7 @@ defmodule Artefact do key_map = Map.merge(bind_key_map, new_key_map) validate_graft_rel_keys!(rel_specs, key_map) + validate_graft_no_new_islands!(rel_specs, bind_key_map, new_key_map) bind_updates = Map.new(bind_specs, fn {_key, node_opts} -> @@ -329,24 +515,99 @@ defmodule Artefact do } } - build([ - {:title, title}, - {:description, description}, - {:base_label, left.base_label}, - {:graph, graph}, - {:metadata, metadata} - ]) + result = + build([ + {:title, title}, + {:description, description}, + {:base_label, left.base_label}, + {:graph, graph}, + {:metadata, metadata} + ]) + + validate!(result) + result end - defp validate_graft_node_uuids!(node_specs) do + defp validate_graft_node_specs!(node_specs) do Enum.each(node_specs, fn {key, node_opts} -> - case Keyword.fetch(node_opts, :uuid) do - {:ok, _} -> :ok - :error -> raise ArgumentError, "graft: node #{inspect(key)} is missing required :uuid" + uuid = + case Keyword.fetch(node_opts, :uuid) do + {:ok, u} -> u + :error -> raise ArgumentError, "graft: node #{inspect(key)} is missing required :uuid" + end + + unless Artefact.UUID.valid?(uuid) do + raise ArgumentError, + "graft: node #{inspect(key)} :uuid #{inspect(uuid)} is not a valid UUIDv7" + end + + case Keyword.fetch(node_opts, :labels) do + :error -> + :ok + + {:ok, labels} -> + unless is_list(labels) and Enum.all?(labels, &is_binary/1) do + raise ArgumentError, + "graft: node #{inspect(key)} :labels #{inspect(labels)} is not a list of strings" + end + end + + case Keyword.fetch(node_opts, :properties) do + :error -> + :ok + + {:ok, properties} -> + unless is_map(properties) do + raise ArgumentError, + "graft: node #{inspect(key)} :properties #{inspect(properties)} is not a map" + end end end) end + defp validate_graft_no_new_islands!(rel_specs, bind_key_map, new_key_map) do + bind_keys = Map.keys(bind_key_map) + new_keys = Map.keys(new_key_map) + + if new_keys == [] do + :ok + else + adjacency = + Enum.reduce(rel_specs, %{}, fn spec, acc -> + f = Keyword.fetch!(spec, :from) + t = Keyword.fetch!(spec, :to) + + acc + |> Map.update(f, MapSet.new([t]), &MapSet.put(&1, t)) + |> Map.update(t, MapSet.new([f]), &MapSet.put(&1, f)) + end) + + anchored = reach(adjacency, MapSet.new(bind_keys)) + islands = MapSet.difference(MapSet.new(new_keys), anchored) + + if MapSet.size(islands) > 0 do + raise ArgumentError, + "graft: args introduces disconnected islands — these new node keys are not reachable from any bind-only key via args.relationships: " <> + inspect(Enum.sort(MapSet.to_list(islands))) + end + end + end + + defp reach(adjacency, seeds) do + Enum.reduce(seeds, seeds, fn seed, visited -> + reach_from(adjacency, seed, visited) + end) + end + + defp reach_from(adjacency, node, visited) do + visited = MapSet.put(visited, node) + neighbours = Map.get(adjacency, node, MapSet.new()) + + Enum.reduce(neighbours, visited, fn n, acc -> + if MapSet.member?(acc, n), do: acc, else: reach_from(adjacency, n, acc) + end) + end + defp validate_graft_unique_keys!(node_specs) do keys = Enum.map(node_specs, fn {k, _} -> k end) dupes = keys -- Enum.uniq(keys) @@ -399,6 +660,9 @@ defmodule Artefact do @doc false def do_harmonise(%__MODULE__{} = a1, %__MODULE__{} = a2, bindings, opts, caller) do + validate!(a1) + validate!(a2) + if a1.uuid == a2.uuid do raise ArgumentError, "cannot harmonise an artefact with itself (uuid: #{a1.uuid})" end @@ -487,7 +751,11 @@ defmodule Artefact do } } - build([{:title, title}, {:base_label, base_label}, {:graph, graph}, {:metadata, metadata}]) + result = + build([{:title, title}, {:base_label, base_label}, {:graph, graph}, {:metadata, metadata}]) + + validate!(result) + result end @doc false diff --git a/artefact/lib/artefact/uuid.ex b/artefact/lib/artefact/uuid.ex index f700247..878620b 100644 --- a/artefact/lib/artefact/uuid.ex +++ b/artefact/lib/artefact/uuid.ex @@ -5,6 +5,14 @@ defmodule Artefact.UUID do @moduledoc false import Bitwise + # 8-4-4-4-12 hex with hyphens, version digit "7" at offset 14, variant in + # {8,9,a,b} at offset 19. Anchored ^...$. + @v7_regex ~r/^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/ + + @doc "Returns true when `value` is a valid UUIDv7 string (lowercase hex)." + def valid?(value) when is_binary(value), do: Regex.match?(@v7_regex, value) + def valid?(_), do: false + @doc "Generate a UUIDv7 string. Time-ordered; lower value = earlier creation." def generate_v7 do # 48-bit millisecond timestamp diff --git a/artefact/mix.exs b/artefact/mix.exs index a3ed5e3..5d27d20 100644 --- a/artefact/mix.exs +++ b/artefact/mix.exs @@ -5,7 +5,7 @@ defmodule Artefact.MixProject do @moduledoc false use Mix.Project - @version "0.1.4" + @version "0.1.5" @github_url "https://github.com/diffo-dev/artefactory" def project do diff --git a/artefact/test/artefact_test.exs b/artefact/test/artefact_test.exs index c426387..695683b 100644 --- a/artefact/test/artefact_test.exs +++ b/artefact/test/artefact_test.exs @@ -1590,4 +1590,302 @@ defmodule ArtefactTest do end end end + + describe "Artefact.UUID.valid?/1" do + test "true for a freshly generated UUIDv7" do + assert Artefact.UUID.valid?(Artefact.UUID.generate_v7()) + end + + test "true for canonical UUIDv7 strings" do + assert Artefact.UUID.valid?("019ddb71-c70b-7b3e-83b1-58f4d0be2852") + assert Artefact.UUID.valid?("019d0000-0000-7000-8000-000000000000") + end + + test "false for empty string" do + refute Artefact.UUID.valid?("") + end + + test "false for non-binary input" do + refute Artefact.UUID.valid?(nil) + refute Artefact.UUID.valid?(:atom) + refute Artefact.UUID.valid?(123) + end + + test "false for wrong format" do + refute Artefact.UUID.valid?("019ddb71c70b7b3e83b158f4d0be2852") + refute Artefact.UUID.valid?("019ddb71-c70b-7b3e-83b1") + refute Artefact.UUID.valid?("019DDB71-C70B-7B3E-83B1-58F4D0BE2852") + end + + test "false for wrong version digit (must be 7)" do + refute Artefact.UUID.valid?("019ddb71-c70b-4b3e-83b1-58f4d0be2852") + end + + test "false for wrong variant digit (must be 8/9/a/b)" do + refute Artefact.UUID.valid?("019ddb71-c70b-7b3e-c3b1-58f4d0be2852") + end + end + + describe "Artefact.is_artefact?/1" do + test "true for an %Artefact{} struct" do + assert Artefact.is_artefact?(Artefact.new()) + end + + test "false for non-artefact values" do + refute Artefact.is_artefact?(nil) + refute Artefact.is_artefact?(%{}) + refute Artefact.is_artefact?(%Artefact.Node{}) + refute Artefact.is_artefact?("artefact") + end + end + + describe "Artefact.validate/1 + is_valid?/1" do + @good_uuid_a "019d0000-0000-7000-8000-0000000000a1" + @good_uuid_b "019d0000-0000-7000-8000-0000000000a2" + + defp valid_artefact_with(nodes, rels) do + %Artefact{ + id: Artefact.UUID.generate_v7(), + uuid: Artefact.UUID.generate_v7(), + title: nil, + base_label: nil, + style: nil, + metadata: %{}, + graph: %Artefact.Graph{nodes: nodes, relationships: rels} + } + end + + test "fresh Artefact.new is valid" do + assert Artefact.is_valid?(Artefact.new()) + assert Artefact.validate(Artefact.new()) == :ok + end + + test "non-artefact returns error" do + assert {:error, ["not an %Artefact{} struct"]} = Artefact.validate(%{}) + end + + test "rejects empty uuid on the artefact itself" do + a = %{Artefact.new() | uuid: ""} + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "uuid is not a valid UUIDv7")) + end + + test "rejects non-list labels on a node" do + n = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: "Engine", properties: %{}} + a = valid_artefact_with([n], []) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "labels is not a list of strings")) + end + + test "rejects list-of-non-strings labels on a node" do + n = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: [:Engine], properties: %{}} + a = valid_artefact_with([n], []) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "labels is not a list of strings")) + end + + test "rejects non-map properties on a node" do + n = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: [], properties: []} + a = valid_artefact_with([n], []) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "properties is not a map")) + end + + test "rejects relationship with from_id not in graph" do + n = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: [], properties: %{}} + + r = %Artefact.Relationship{ + id: "r0", + type: "X", + from_id: "ghost", + to_id: "n0", + properties: %{} + } + + a = valid_artefact_with([n], [r]) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ ~s(from_id "ghost" not in graph))) + end + + test "rejects relationship with empty type" do + n0 = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: [], properties: %{}} + n1 = %Artefact.Node{id: "n1", uuid: @good_uuid_b, labels: [], properties: %{}} + + r = %Artefact.Relationship{ + id: "r0", + type: "", + from_id: "n0", + to_id: "n1", + properties: %{} + } + + a = valid_artefact_with([n0, n1], [r]) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "type is not a non-empty string")) + end + + test "rejects duplicate node uuids" do + n0 = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: [], properties: %{}} + n1 = %Artefact.Node{id: "n1", uuid: @good_uuid_a, labels: [], properties: %{}} + a = valid_artefact_with([n0, n1], []) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "duplicate node uuids")) + end + + test "rejects duplicate node ids" do + n0 = %Artefact.Node{id: "n0", uuid: @good_uuid_a, labels: [], properties: %{}} + n1 = %Artefact.Node{id: "n0", uuid: @good_uuid_b, labels: [], properties: %{}} + a = valid_artefact_with([n0, n1], []) + assert {:error, reasons} = Artefact.validate(a) + assert Enum.any?(reasons, &(&1 =~ "duplicate node ids")) + end + + test "is_valid? is the boolean shortcut" do + n = %Artefact.Node{id: "n0", uuid: "", labels: [], properties: %{}} + a = valid_artefact_with([n], []) + refute Artefact.is_valid?(a) + assert Artefact.is_valid?(Artefact.new()) + end + end + + describe "Artefact.validate!/1" do + test ":ok for a valid artefact" do + assert Artefact.validate!(Artefact.new()) == :ok + end + + test "raises ArgumentError with reasons for invalid artefact" do + a = %{Artefact.new() | uuid: ""} + + assert_raise ArgumentError, + ~r/invalid artefact:.*uuid is not a valid UUIDv7/, + fn -> Artefact.validate!(a) end + end + end + + describe "Artefact.graft/3 — input rejection (validation)" do + alias Artefact.Test.Fixtures.OurShells + + test "raises when a node :uuid is empty string" do + left = OurShells.our_shells() + + assert_raise ArgumentError, ~r/:uuid "" is not a valid UUIDv7/, fn -> + Artefact.graft(left, + nodes: [bad: [labels: ["Knowing"], uuid: ""]], + relationships: [] + ) + end + end + + test "raises when a node :uuid is malformed" do + left = OurShells.our_shells() + + assert_raise ArgumentError, ~r/is not a valid UUIDv7/, fn -> + Artefact.graft(left, + nodes: [bad: [labels: ["X"], uuid: "not-a-uuid"]], + relationships: [] + ) + end + end + + test "raises when a node :labels is not a list" do + left = OurShells.our_shells() + + assert_raise ArgumentError, ~r/:labels "Engine" is not a list of strings/, fn -> + Artefact.graft(left, + nodes: [bad: [labels: "Engine", uuid: "019d0000-0000-7000-8000-0000000000d1"]], + relationships: [] + ) + end + end + + test "raises when a node :properties is not a map" do + left = OurShells.our_shells() + + assert_raise ArgumentError, ~r/:properties.* is not a map/, fn -> + Artefact.graft(left, + nodes: [bad: [properties: [], uuid: "019d0000-0000-7000-8000-0000000000d2"]], + relationships: [] + ) + end + end + end + + describe "Artefact.graft/3 — no new islands (#29)" do + alias Artefact.Test.Fixtures.OurShells + + test "raises when a single new node has no relationship to a bind-only node" do + left = OurShells.our_shells() + + assert_raise ArgumentError, ~r/disconnected islands/, fn -> + Artefact.graft(left, + nodes: [ + {:me, [uuid: OurShells.me_uuid()]}, + {:floating, [labels: ["X"], uuid: "019d0000-0000-7000-8000-0000000000e1"]} + ], + relationships: [] + ) + end + end + + test "raises when new nodes form a chain disconnected from any bind-only node" do + left = OurShells.our_shells() + + assert_raise ArgumentError, ~r/disconnected islands/, fn -> + Artefact.graft(left, + nodes: [ + {:me, [uuid: OurShells.me_uuid()]}, + {:b, [labels: ["X"], uuid: "019d0000-0000-7000-8000-0000000000e2"]}, + {:c, [labels: ["X"], uuid: "019d0000-0000-7000-8000-0000000000e3"]} + ], + relationships: [[from: :b, type: "X", to: :c]] + ) + end + end + + test "passes when a new node connects directly to a bind-only node" do + left = OurShells.our_shells() + + result = + Artefact.graft(left, + nodes: [ + {:me, [uuid: OurShells.me_uuid()]}, + {:b, [labels: ["X"], uuid: "019d0000-0000-7000-8000-0000000000e4"]} + ], + relationships: [[from: :me, type: "REACHES", to: :b]] + ) + + assert Artefact.is_valid?(result) + end + + test "passes when a chain of new nodes is anchored via the first to a bind-only" do + left = OurShells.our_shells() + + result = + Artefact.graft(left, + nodes: [ + {:me, [uuid: OurShells.me_uuid()]}, + {:b, [labels: ["X"], uuid: "019d0000-0000-7000-8000-0000000000e5"]}, + {:c, [labels: ["X"], uuid: "019d0000-0000-7000-8000-0000000000e6"]} + ], + relationships: [ + [from: :me, type: "REACHES", to: :b], + [from: :b, type: "NEXT", to: :c] + ] + ) + + assert Artefact.is_valid?(result) + end + + test "passes when args has only bind-only nodes (no new nodes)" do + left = OurShells.our_shells() + + result = + Artefact.graft(left, + nodes: [{:me, [uuid: OurShells.me_uuid()]}], + relationships: [] + ) + + assert Artefact.is_valid?(result) + end + end end diff --git a/artefact_kino/CHANGELOG.md b/artefact_kino/CHANGELOG.md index fc2038c..0feae67 100644 --- a/artefact_kino/CHANGELOG.md +++ b/artefact_kino/CHANGELOG.md @@ -5,6 +5,12 @@ SPDX-License-Identifier: MIT # Changelog +## 0.1.5 — 2026-05-05 + +- `ArtefactKino.new/1,2` now calls `Artefact.validate!/1` on its input — a hand-built `%Artefact{}` with malformed fields (non-list labels, missing uuid, dangling relationship endpoint, etc.) raises `ArgumentError` with structured reasons instead of a cryptic render-time error. Closes [#28]. Bumps `artefact` requirement to `~> 0.1.5` for the new validation API. + +[#28]: https://github.com/diffo-dev/artefactory/issues/28 + ## 0.1.4 — 2026-05-05 - Inspector panel collapsible (matching the Export panel); both default collapsed to give the graph more room on bigger artefacts; selecting a node or relationship in the graph auto-expands the Inspector. Bumps `artefact` requirement to `~> 0.1.4` for convenience. diff --git a/artefact_kino/lib/artefact_kino.ex b/artefact_kino/lib/artefact_kino.ex index 0075e27..c4bb2e6 100644 --- a/artefact_kino/lib/artefact_kino.ex +++ b/artefact_kino/lib/artefact_kino.ex @@ -21,10 +21,16 @@ defmodule ArtefactKino do @doc """ Render an `%Artefact{}` as a three-panel Kino widget. + Validates the artefact via `Artefact.validate!/1` first so a + hand-built struct with malformed fields (non-list labels, missing + uuid, dangling relationship endpoint, etc.) fails with a clear + `ArgumentError` instead of a cryptic render-time error. + Options: - `default:` — `:create` (default) or `:merge` """ def new(%Artefact{} = artefact, opts \\ []) do + Artefact.validate!(artefact) default = Keyword.get(opts, :default, :create) Kino.JS.new(__MODULE__, build_data(artefact, default)) end diff --git a/artefact_kino/mix.exs b/artefact_kino/mix.exs index ee0af6a..0bd9e4f 100644 --- a/artefact_kino/mix.exs +++ b/artefact_kino/mix.exs @@ -5,7 +5,7 @@ defmodule ArtefactKino.MixProject do @moduledoc false use Mix.Project - @version "0.1.4" + @version "0.1.5" @github_url "https://github.com/diffo-dev/artefactory" def project do @@ -42,13 +42,13 @@ defmodule ArtefactKino.MixProject do defp artefact_dep do cond do System.get_env("HEX_PUBLISH") == "1" -> - {:artefact, "~> 0.1.4"} + {:artefact, "~> 0.1.5"} File.exists?(Path.join(__DIR__, "../artefact/mix.exs")) -> {:artefact, path: "../artefact"} true -> - {:artefact, "~> 0.1.4"} + {:artefact, "~> 0.1.5"} end end