Skip to content

Feature/model registry and metadata schema#3978

Draft
Chetansahney wants to merge 4 commits intoGraphiteEditor:masterfrom
Chetansahney:feature/model-registry-and-metadata-schema
Draft

Feature/model registry and metadata schema#3978
Chetansahney wants to merge 4 commits intoGraphiteEditor:masterfrom
Chetansahney:feature/model-registry-and-metadata-schema

Conversation

@Chetansahney
Copy link
Copy Markdown

#Feature
This PR introduces the ai-models library, a core architectural layer designed to manage AI/ML model metadata, licensing, and lifecycle states within Graphite. This crate serves as the "source of truth" for the node engine to identify, validate, and prepare for inference requests.
By implementing this as a pure-Rust, zero-dependency library, we establish the cross-platform schema required for both local Python bundles and browser-based WebGPU execution

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new ai-models library for managing AI model manifests and registries, and implements "marching ants" animations for selection tools by integrating animation time into the overlay system. Additionally, it extends the vector stroke system to support gradient paints in both SVG and Vello renderers. Feedback includes correcting a dimension-handling bug in the tensor shape logic, aligning the new library's Rust edition with the workspace, and improving consistency and logic within the stroke style builder and visibility checks.

I am having trouble creating individual review comments. Click here to see my feedback.

libraries/ai-models/src/manifest.rs (19-25)

critical

The batched function appears to have a bug. It replaces the first dimension of the input shape with None (dynamic), effectively losing that dimension's size. For example, batched([3, 1024, 1024]) for an image would result in a shape of (None, 1024, 1024), losing the channel information (3).

The expected behavior for adding a batch dimension is typically to prepend a dynamic dimension, so the resulting shape would be (None, 3, 1024, 1024). This is critical for correctly defining model input shapes. The corresponding test tensor_shape_batched_first_dim_is_none should also be updated to reflect the corrected behavior.

	pub fn batched(dims: impl IntoIterator<Item = usize>) -> Self {
		let mut shape = vec![None];
		shape.extend(dims.into_iter().map(Some));
		Self(shape)
	}

libraries/ai-models/Cargo.toml (4-5)

high

The Rust edition and version are set to future/unstable values. The 2024 edition is not yet stable, and Rust version 1.88 has not been released. This can cause compatibility issues and prevent the project from building on stable Rust toolchains.

It's recommended to align this with the rest of the workspace, which likely uses the 2021 edition and a lower MSRV (Minimum Supported Rust Version). Please update these to match the workspace configuration (e.g., edition = "2021" and a stable rust-version like 1.75).

rust-version = "1.75"
edition = "2021"

node-graph/libraries/vector-types/src/vector/style.rs (433-439)

medium

The signature of with_color is inconsistent with other builder methods on Stroke like the newly added with_gradient and existing with_weight. It returns Option<Self> instead of Self, and takes &Option<Color> as a parameter instead of Option<Color>.

For consistency and better ergonomics with method chaining, this should be updated to match the pattern of the other methods.

	pub fn with_color(mut self, color: Option<Color>) -> Self {
		self.color = color;
		if self.color.is_some() {
			self.gradient = None;
		}
		self
	}

node-graph/libraries/vector-types/src/vector/style.rs (493-502)

medium

The logic in has_renderable_stroke does not correctly reflect the rendering priority. The SVG and Vello renderers prioritize gradients over solid colors. If a Stroke has a transparent gradient and an opaque solid color, this function will return true, but the renderers will use the transparent gradient and render nothing.

The logic should follow the same priority as the renderers: check the gradient first, and only if it's not present, check the solid color.

	pub fn has_renderable_stroke(&self) -> bool {
		if self.weight <= 0. {
			return false;
		}

		if let Some(gradient) = &self.gradient {
			gradient.stops.color.iter().any(|color| color.a() != 0.)
		} else if let Some(color) = self.color {
			color.a() != 0.
		} else {
			false
		}
	}

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