Skip to content

Make nodes report errors instead of panicking or failing silently - #4837

Open
timon-schelling wants to merge 8 commits into
masterfrom
add-node-errors
Open

timon-schelling wants to merge 8 commits into
masterfrom
add-node-errors

Conversation

@timon-schelling

Copy link
Copy Markdown
Member

No description provided.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

26 issues found across 65 files

Confidence score: 2/5

  • transform.rs only rejects an exactly zero determinant, so NaN or subnormal values can still reach inverse() and panic. Reject non-finite reciprocals before inversion.
  • render_node.rs still uses expect for render failures, so a large export or another vello_scene failure can panic instead of returning the function’s Result. Propagate those errors.
  • text/src/lib.rs can panic on negative decimal_places and abort on allocation failure for a large repeat count. Clamp the decimal-place value before casting and use fallible reservation.
  • shapes.rs still allows side counts far below its limit to request allocations large enough to OOM. Lower the limit to a practical bound and return TooManySides before allocating.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/nodes/transform/src/transform_nodes.rs">

<violation number="1" location="node-graph/nodes/transform/src/transform_nodes.rs:165">
P3: The error message is inaccurate for the non-finite and subnormal cases this guard also rejects. A NaN or infinite transform is reported as collapsing to a line or point, which sends users looking for a degenerate transform that isn't there. Reword the message to cover all cases the check rejects.</violation>

<violation number="2" location="node-graph/nodes/transform/src/transform_nodes.rs:172">
P2: This determinant-only guard can still return a non-finite inverse when an inverse component overflows, propagating invalid transform values downstream. Reject the inverse unless the computed `DAffine2` is finite.</violation>
</file>

<file name="node-graph/libraries/vector-types/src/vector/algorithms/shapes.rs">

<violation number="1" location="node-graph/libraries/vector-types/src/vector/algorithms/shapes.rs:20">
P2: This limit is too high to prevent the failure it targets. Requests far below it, such as 10^12 sides, still try to allocate tens of GB and abort or get OOM-killed instead of returning `PolygonError::TooManySides`. Use a much lower practical cap, or check allocation with `try_reserve_exact` before building the path.</violation>

<violation number="2" location="node-graph/libraries/vector-types/src/vector/algorithms/shapes.rs:266">
P2: `spiral_bezpath` still silently returns an empty path for NaN and negative-infinite `turns`, despite exposing `TurnsNotFinite`. Reject every non-finite value with `if !turns.is_finite()` before calculating `theta_end`.</violation>
</file>

<file name="node-graph/nodes/text/src/json.rs">

<violation number="1" location="node-graph/nodes/text/src/json.rs:51">
P3: An invalid path is silently accepted when the JSON input is empty. Validate the path before the empty-input early return so both query nodes report `InvalidPath` consistently.</violation>
</file>

<file name="node-graph/libraries/vector-types/src/vector/algorithms/bezpath_algorithms.rs">

<violation number="1" location="node-graph/libraries/vector-types/src/vector/algorithms/bezpath_algorithms.rs:408">
P2: Returning an empty `Vec` here drops the scattered points without any error, which is the silent failure this PR is meant to remove. An infinite bounding box is a real input problem, so surface it through the caller as an error, for example by returning a `Result` from `poisson_disk_points` and mapping it to a node error.</violation>
</file>

<file name="node-graph/nodes/text/src/path_builder.rs">

<violation number="1" location="node-graph/nodes/text/src/path_builder.rs:107">
P2: `render_glyph_run` now returns `Result`, but the `ReadFontsRef::from_index(...).unwrap()` a few lines above still panics on invalid font data. Propagate it with `?` as `TextToVectorError::UnreadableFont`, which fits this PR's goal of removing panics.</violation>
</file>

<file name="node-graph/nodes/vector/src/voronoi.rs">

<violation number="1" location="node-graph/nodes/vector/src/voronoi.rs:55">
P3: The doc comments for `relax_sites`, `voronoi_cells`, and `delaunay_triangles` don't mention the new `Err(NonFinitePoints)` return. `relax_sites` still says it returns the sites unchanged when no diagram can be formed, which is no longer true for non-finite input with nonzero `iterations`. Add a note on the error case to each doc.</violation>
</file>

<file name="node-graph/nodes/raster/src/std_nodes.rs">

<violation number="1" location="node-graph/nodes/raster/src/std_nodes.rs:165">
P2: This degenerate-transform guard returns the image unchanged with `Ok`, so the failure is silent. That contradicts the PR's goal of reporting errors. Consider returning `Err((image, error))` with a suitable error, or documenting why the pass-through is the intended behavior.</violation>
</file>

<file name="node-graph/nodes/gstd/src/render_node.rs">

<violation number="1" location="node-graph/nodes/gstd/src/render_node.rs:107">
P2: The `expect` calls on `vello_scene` and `render_vello_scene` still panic, even though this function now returns `Result`. `vello_scene` fails for any render larger than `u16::MAX` pixels per side, so a large export can crash the app. Propagate these as `Interrupt` errors the same way the executor lookup above does.

(Based on your team's feedback about avoiding panics in application code.)</violation>
</file>

<file name="node-graph/libraries/raster-types/src/image.rs">

<violation number="1" location="node-graph/libraries/raster-types/src/image.rs:216">
P2: `encoded_size` drops the error here, so an image that is too large is reported as unreadable. The ingest handler then shows "This file could not be read as an image." Consider returning `Result<(u32, u32), ImageError>` and passing `TooLarge` through to the ingest message.</violation>
</file>

<file name="node-graph/libraries/wgpu-executor/src/texture_conversion.rs">

<violation number="1" location="node-graph/libraries/wgpu-executor/src/texture_conversion.rs:15">
P2: For a zero-width or zero-height image with the other dimension nonzero, this writes only one pixel into a texture the cache sized as 1xN or Nx1. The remaining texels keep whatever a reused cached texture held. Write transparent over the texture's actual dimensions, or request a 1x1 texture explicitly in this branch.</violation>
</file>

<file name="node-graph/libraries/vector-types/src/vector/algorithms/offset_bezpath.rs">

<violation number="1" location="node-graph/libraries/vector-types/src/vector/algorithms/offset_bezpath.rs:60">
P2: NaN coordinates bypass this guard, because `coordinate.abs() >= MAX_CLIPPED_COORDINATE` is false for NaN. Reject non-finite coordinates explicitly, for example by testing `!(coordinate.abs() < MAX_CLIPPED_COORDINATE)`.</violation>
</file>

<file name="node-graph/nodes/gstd/src/platform_application_io.rs">

<violation number="1" location="node-graph/nodes/gstd/src/platform_application_io.rs:69">
P2: `discard_result` requests buffer the entire 4xx/5xx response before the discard branch runs. A server can stream a large error body, causing unbounded memory use and blocking the request; cap the captured error body and skip body collection for discarded requests.</violation>

<violation number="2" location="node-graph/nodes/gstd/src/platform_application_io.rs:118">
P2: With `discard_result` set, a failed or non-2xx request now returns an error that stops graph execution. The doc comment on `discard_result` says the request should not block graph execution. Either document this new failure behavior, or skip the status and transport error check when the result is discarded.</violation>
</file>

<file name="node-graph/nodes/text/src/lib.rs">

<violation number="1" location="node-graph/nodes/text/src/lib.rs:402">
P2: A negative `decimal_places` with fixed decimals reaches `"0".repeat(extra_zeros)` with a near-`usize::MAX` count, which panics. Clamp the value before casting, for example `let requested_places = usize::try_from(decimal_places).unwrap_or(0);`, so the padding stays bounded.</violation>

<violation number="2" location="node-graph/nodes/text/src/lib.rs:591">
P2: `String::with_capacity` aborts on allocation failure, so a large but in-range repeat count still crashes the process instead of returning `TextTooLongError::Repeat`. Reserve with `try_reserve_exact` and map the failure to the same error.</violation>
</file>

<file name="node-graph/nodes/vector/src/generator_nodes.rs">

<violation number="1" location="node-graph/nodes/vector/src/generator_nodes.rs:439">
P2: This guard misses near-cancelling angles, allowing the following division to overflow and generate non-finite grid coordinates. Reject a non-finite or sufficiently small tangent sum (and resulting spacing) before constructing the lattice.</violation>
</file>

<file name="node-graph/nodes/vector/src/vector_nodes.rs">

<violation number="1" location="node-graph/nodes/vector/src/vector_nodes.rs:1200">
P2: The new point-bounds fallback can produce a zero-width or zero-height `source_size`, so box warp divides by zero and writes NaN/infinite coordinates for degenerate point-only inputs. Handle each degenerate axis before normalizing, or retain a non-degenerate fallback bounds.</violation>
</file>

<file name="node-graph/nodes/text/src/regex.rs">

<violation number="1" location="node-graph/nodes/text/src/regex.rs:52">
P2: `refuse_keep_out_in_lookaround` rejects every subroutine call inside a lookaround whenever `\K` appears anywhere in the pattern, even when the called group contains no `\K`. Resolve the called group before classifying the call so valid patterns are not reported as errors.</violation>
</file>

<file name="node-graph/nodes/path-bool/src/lib.rs">

<violation number="1" location="node-graph/nodes/path-bool/src/lib.rs:116">
P2: `wrapping_add` can turn a valid winding of ±65,536 into zero, which `is_inside` treats as outside and corrupts boolean results. Use a wider winding accumulator such as `i32` or `i64` rather than wrapping `i16` values.</violation>

<violation number="2" location="node-graph/nodes/path-bool/src/lib.rs:187">
P2: Topology failures are still reported as success with an empty vector. Add a topology error variant to `BooleanOperationError` and return the failure instead of `Ok(list)` so the node reports the error.</violation>
</file>

<file name="node-graph/nodes/gstd/src/render_cache.rs">

<violation number="1" location="node-graph/nodes/gstd/src/render_cache.rs:405">
P2: This branch collapses a partial executor result into a final render, so an incomplete scope evaluation can be reported as `Poll::Final`. Preserve the executor’s partial state when constructing the cache result.</violation>

<violation number="2" location="node-graph/nodes/gstd/src/render_cache.rs:406">
P2: This branch drops the executor’s `NodeError` while retaining its fallback value, so `render_output_cache` can hide an executor failure. Preserve and merge `boxed.1` into the result’s fallback error.</violation>
</file>

<file name="editor/src/messages/portfolio/document/overlays/utility_types_native.rs">

<violation number="1" location="editor/src/messages/portfolio/document/overlays/utility_types_native.rs:1160">
P2: This discards every `TextToVectorError` and silently renders no overlay text. The bounding box still drives layout, so labels can disappear while their background remains; log the error or propagate it instead of converting it to an empty list.</violation>
</file>

<file name="node-graph/libraries/core-types/src/transform.rs">

<violation number="1" location="node-graph/libraries/core-types/src/transform.rs:192">
P1: This guard only handles an exactly zero determinant. NaN or subnormal determinants still reach `inverse()`, which can panic with `debug-glam-assert`; reject non-finite reciprocals as the renderer does.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

};

pub fn viewport_bounds_in_local_space(&self) -> AxisAlignedBbox {
if self.transform.matrix2.determinant() == 0. {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This guard only handles an exactly zero determinant. NaN or subnormal determinants still reach inverse(), which can panic with debug-glam-assert; reject non-finite reciprocals as the renderer does.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/core-types/src/transform.rs, line 192:

<comment>This guard only handles an exactly zero determinant. NaN or subnormal determinants still reach `inverse()`, which can panic with `debug-glam-assert`; reject non-finite reciprocals as the renderer does.</comment>

<file context>
@@ -189,6 +189,10 @@ impl Footprint {
 	};
 
 	pub fn viewport_bounds_in_local_space(&self) -> AxisAlignedBbox {
+		if self.transform.matrix2.determinant() == 0. {
+			return AxisAlignedBbox::ZERO;
+		}
</file context>
Suggested change
if self.transform.matrix2.determinant() == 0. {
if !self.transform.matrix2.determinant().recip().is_finite() {

const SPIRAL_CIRCLE_GROWTH: f64 = 1e-6;

/// The most anchors a polygon or star can have before allocating them overflows the address space.
const MAX_POLYGON_ANCHORS: u64 = (isize::MAX as usize / size_of::<Anchor>()) as u64;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This limit is too high to prevent the failure it targets. Requests far below it, such as 10^12 sides, still try to allocate tens of GB and abort or get OOM-killed instead of returning PolygonError::TooManySides. Use a much lower practical cap, or check allocation with try_reserve_exact before building the path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/vector-types/src/vector/algorithms/shapes.rs, line 20:

<comment>This limit is too high to prevent the failure it targets. Requests far below it, such as 10^12 sides, still try to allocate tens of GB and abort or get OOM-killed instead of returning `PolygonError::TooManySides`. Use a much lower practical cap, or check allocation with `try_reserve_exact` before building the path.</comment>

<file context>
@@ -13,6 +13,19 @@ use std::f64::consts::TAU;
+const SPIRAL_CIRCLE_GROWTH: f64 = 1e-6;
+
+/// The most anchors a polygon or star can have before allocating them overflows the address space.
+const MAX_POLYGON_ANCHORS: u64 = (isize::MAX as usize / size_of::<Anchor>()) as u64;
+
+/// The failure of [`regular_polygon_bezpath`] and [`star_polygon_bezpath`].
</file context>

let width = this_bbox.width();
let height = this_bbox.height();

if width.is_infinite() || height.is_infinite() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Returning an empty Vec here drops the scattered points without any error, which is the silent failure this PR is meant to remove. An infinite bounding box is a real input problem, so surface it through the caller as an error, for example by returning a Result from poisson_disk_points and mapping it to a node error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/vector-types/src/vector/algorithms/bezpath_algorithms.rs, line 408:

<comment>Returning an empty `Vec` here drops the scattered points without any error, which is the silent failure this PR is meant to remove. An infinite bounding box is a real input problem, so surface it through the caller as an error, for example by returning a `Result` from `poisson_disk_points` and mapping it to a node error.</comment>

<file context>
@@ -385,6 +405,10 @@ pub fn poisson_disk_points(bezpath_index: usize, bezpaths: &[(BezPath, Rect)], s
 	let width = this_bbox.width();
 	let height = this_bbox.height();
 
+	if width.is_infinite() || height.is_infinite() {
+		return Vec::new();
+	}
</file context>

}

pub fn render_glyph_run(&mut self, glyph_run: &GlyphRun<'_, ()>, letter_tilt: f64, per_glyph_items: bool, x_offset: f32, space_extra: f32) {
pub fn render_glyph_run(&mut self, glyph_run: &GlyphRun<'_, ()>, letter_tilt: f64, per_glyph_items: bool, x_offset: f32, space_extra: f32) -> Result<(), TextToVectorError> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: render_glyph_run now returns Result, but the ReadFontsRef::from_index(...).unwrap() a few lines above still panics on invalid font data. Propagate it with ? as TextToVectorError::UnreadableFont, which fits this PR's goal of removing panics.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/nodes/text/src/path_builder.rs, line 107:

<comment>`render_glyph_run` now returns `Result`, but the `ReadFontsRef::from_index(...).unwrap()` a few lines above still panics on invalid font data. Propagate it with `?` as `TextToVectorError::UnreadableFont`, which fits this PR's goal of removing panics.</comment>

<file context>
@@ -100,10 +101,10 @@ impl PathBuilder {
 	}
 
-	pub fn render_glyph_run(&mut self, glyph_run: &GlyphRun<'_, ()>, letter_tilt: f64, per_glyph_items: bool, x_offset: f32, space_extra: f32) {
+	pub fn render_glyph_run(&mut self, glyph_run: &GlyphRun<'_, ()>, letter_tilt: f64, per_glyph_items: bool, x_offset: f32, space_extra: f32) -> Result<(), TextToVectorError> {
 		let mut run_x = glyph_run.offset() + x_offset;
 		let run_y = glyph_run.baseline();
</file context>

return empty_image(bounds, Color::TRANSPARENT);
return empty_image(bounds, Color::TRANSPARENT).map_err(|error| (image, error));
}
if image_transform.matrix2.determinant() == 0. {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This degenerate-transform guard returns the image unchanged with Ok, so the failure is silent. That contradicts the PR's goal of reporting errors. Consider returning Err((image, error)) with a suitable error, or documenting why the pass-through is the intended behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/nodes/raster/src/std_nodes.rs, line 165:

<comment>This degenerate-transform guard returns the image unchanged with `Ok`, so the failure is silent. That contradicts the PR's goal of reporting errors. Consider returning `Err((image, error))` with a suitable error, or documenting why the pass-through is the intended behavior.</comment>

<file context>
@@ -132,20 +148,22 @@ fn mask(
-		return empty_image(bounds, Color::TRANSPARENT);
+		return empty_image(bounds, Color::TRANSPARENT).map_err(|error| (image, error));
+	}
+	if image_transform.matrix2.determinant() == 0. {
+		return Ok(image);
 	}
</file context>

let executor = executor.into_element().expect("GPU executor not available");
let executor = match executor.eval(&Context::default()) {
Poll::Final(executor) | Poll::Partial(executor) => executor.into_element(),
Poll::Fallback(boxed) => boxed.0.into_element(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This branch drops the executor’s NodeError while retaining its fallback value, so render_output_cache can hide an executor failure. Preserve and merge boxed.1 into the result’s fallback error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/nodes/gstd/src/render_cache.rs, line 406:

<comment>This branch drops the executor’s `NodeError` while retaining its fallback value, so `render_output_cache` can hide an executor failure. Preserve and merge `boxed.1` into the result’s fallback error.</comment>

<file context>
@@ -401,7 +401,12 @@ fn render_output_cache(
-	let executor = executor.into_element().expect("GPU executor not available");
+	let executor = match executor.eval(&Context::default()) {
+		Poll::Final(executor) | Poll::Partial(executor) => executor.into_element(),
+		Poll::Fallback(boxed) => boxed.0.into_element(),
+		Poll::Pending => return Poll::Pending,
+		Poll::Error(error) => return Poll::Error(error),
</file context>

let (text_size, text_list) = TextContext::with_thread_local(|text_context| {
let text_size = text_context.bounding_box(text, &FALLBACK_FONT_RESOURCE, typesetting, false);
let text_list = text_context.to_path(text, &FALLBACK_FONT_RESOURCE, typesetting, false);
let text_list = text_context.to_path(text, &FALLBACK_FONT_RESOURCE, typesetting, false).unwrap_or_default();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This discards every TextToVectorError and silently renders no overlay text. The bounding box still drives layout, so labels can disappear while their background remains; log the error or propagate it instead of converting it to an empty list.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At editor/src/messages/portfolio/document/overlays/utility_types_native.rs, line 1160:

<comment>This discards every `TextToVectorError` and silently renders no overlay text. The bounding box still drives layout, so labels can disappear while their background remains; log the error or propagate it instead of converting it to an empty list.</comment>

<file context>
@@ -1157,7 +1157,7 @@ impl OverlayContextInternal {
 		let (text_size, text_list) = TextContext::with_thread_local(|text_context| {
 			let text_size = text_context.bounding_box(text, &FALLBACK_FONT_RESOURCE, typesetting, false);
-			let text_list = text_context.to_path(text, &FALLBACK_FONT_RESOURCE, typesetting, false);
+			let text_list = text_context.to_path(text, &FALLBACK_FONT_RESOURCE, typesetting, false).unwrap_or_default();
 			(text_size, text_list)
 		});
</file context>


#[derive(Debug, thiserror::Error)]
pub enum InvertTransformError {
#[error("The transform can't be inverted because it collapses to a line or point")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The error message is inaccurate for the non-finite and subnormal cases this guard also rejects. A NaN or infinite transform is reported as collapsing to a line or point, which sends users looking for a degenerate transform that isn't there. Reword the message to cover all cases the check rejects.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/nodes/transform/src/transform_nodes.rs, line 165:

<comment>The error message is inaccurate for the non-finite and subnormal cases this guard also rejects. A NaN or infinite transform is reported as collapsing to a line or point, which sends users looking for a degenerate transform that isn't there. Reword the message to cover all cases the check rejects.</comment>

<file context>
@@ -160,14 +160,23 @@ fn extract_transform<T: Send>(_: impl Ctx, #[implementations(Graphic, Vector, Ra
 
+#[derive(Debug, thiserror::Error)]
+pub enum InvertTransformError {
+	#[error("The transform can't be inverted because it collapses to a line or point")]
+	Singular,
+}
</file context>
Suggested change
#[error("The transform can't be inverted because it collapses to a line or point")]
#[error("The transform can't be inverted because its determinant is zero, subnormal, or non-finite")]


let cleaned = strip_trailing_commas(json.element());
let Ok(value) = serde_json::from_str::<serde_json::Value>(&cleaned) else { return json };
if cleaned.trim().is_empty() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: An invalid path is silently accepted when the JSON input is empty. Validate the path before the empty-input early return so both query nodes report InvalidPath consistently.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/nodes/text/src/json.rs, line 51:

<comment>An invalid path is silently accepted when the JSON input is empty. Validate the path before the empty-input early return so both query nodes report `InvalidPath` consistently.</comment>

<file context>
@@ -33,17 +42,20 @@ fn format_json(
 
 	let cleaned = strip_trailing_commas(json.element());
-	let Ok(value) = serde_json::from_str::<serde_json::Value>(&cleaned) else { return json };
+	if cleaned.trim().is_empty() {
+		return Ok(json);
+	}
</file context>

/// unbounded perimeter cells would drag those sites around (inward for the convex hull, or outward into the corners of a
/// fixed bounding box), distorting the shape over successive iterations.
pub fn relax_sites(sites: &[DVec2], iterations: f64) -> Vec<DVec2> {
pub fn relax_sites(sites: &[DVec2], iterations: f64) -> Result<Vec<DVec2>, PointDiagramError> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The doc comments for relax_sites, voronoi_cells, and delaunay_triangles don't mention the new Err(NonFinitePoints) return. relax_sites still says it returns the sites unchanged when no diagram can be formed, which is no longer true for non-finite input with nonzero iterations. Add a note on the error case to each doc.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/nodes/vector/src/voronoi.rs, line 55:

<comment>The doc comments for `relax_sites`, `voronoi_cells`, and `delaunay_triangles` don't mention the new `Err(NonFinitePoints)` return. `relax_sites` still says it returns the sites unchanged when no diagram can be formed, which is no longer true for non-finite input with nonzero `iterations`. Add a note on the error case to each doc.</comment>

<file context>
@@ -37,32 +52,32 @@ pub fn voronoi_cells(sites: &[DVec2]) -> Vec<Vec<DVec2>> {
 /// unbounded perimeter cells would drag those sites around (inward for the convex hull, or outward into the corners of a
 /// fixed bounding box), distorting the shape over successive iterations.
-pub fn relax_sites(sites: &[DVec2], iterations: f64) -> Vec<DVec2> {
+pub fn relax_sites(sites: &[DVec2], iterations: f64) -> Result<Vec<DVec2>, PointDiagramError> {
 	const MAX_STEPS_FOR_SAFETY: f64 = 1000.;
 	let iterations = iterations.clamp(0., MAX_STEPS_FOR_SAFETY);
</file context>

This branch was successfully deployed

1 active deployment
graphite-dev (Preview) — 34719b07 Deployed Oct 27, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant