Custom model validation

internal/e2e/daemon/ai_models_test.go

@@ -117,7 +117,7 @@ func TestAIModelDetails(t *testing.T) {
 	t.Run("should return full details for a valid custom model ID", func(t *testing.T) {
 		_, err := custommodel.Store(customModelDir.Join("my-model"), custommodel.ModelDescriptor{
 			ID:          "custom-classification-model-eim",
-			Name:        "this the name of the model",
+			Name:        "this is the name of the model",
 			Description: "this is the description of the model",
 			Bricks: []custommodel.BrickConfig{
 				{ID: "arduino:audio_classification"},
@@ -133,12 +133,12 @@ func TestAIModelDetails(t *testing.T) {
 		got := response.JSON200
 		require.Equal(t, &client.AIModelItem{
 			Id:          f.Ptr("custom-classification-model-eim"),
-			Name:        f.Ptr("this the name of the model"),
+			Name:        f.Ptr("this is the name of the model"),
 			IsBuiltin:   f.Ptr(false),
 			Runner:      f.Ptr(""),
 			Description: f.Ptr("this is the description of the model"),
 			BrickIds:    &[]string{"arduino:audio_classification"},
-			DiskUsage:   f.Ptr(222),
+			DiskUsage:   f.Ptr(225),
 		}, got, "The returned model details should match the expected values")
 
 		// TODO test metadata and model configuration contents and runner
@@ -174,6 +174,7 @@ func TestAIModelDetails(t *testing.T) {
 }
 
 func TestAIModelDelete(t *testing.T) {
+
 	customModelDir, err := paths.MkTempDir("", "custom-models")
 	require.NoError(t, err)
 
@@ -230,6 +231,7 @@ func TestAIModelDelete(t *testing.T) {
 
 		_, err := custommodel.Store(customModelDir.Join("my-custom-model"), custommodel.ModelDescriptor{
 			ID:     modelId,
+			Name:   "this the name of the model",
 			Runner: "brick",
 			Bricks: []custommodel.BrickConfig{
 				{ID: "arduino:audio_classification"},

internal/orchestrator/check.go

@@ -46,36 +46,47 @@ func checkBricks(a app.AppDescriptor, index *bricksindex.BricksIndex, modelIndex
 		indexBrick, found := index.FindBrickByID(appBrick.ID)
 		if !found {
 			allErrors = errors.Join(allErrors, fmt.Errorf("brick %q not found", appBrick.ID))
-			continue // Skip further validation for this brick since it doesn't exist
+			continue
 		}
 
+		var model *modelsindex.AIModel
 		if len(appBrick.Model) != 0 {
-			_, modelFound := modelIndex.GetModelByID(appBrick.Model)
+			m, modelFound := modelIndex.GetModelByID(appBrick.Model)
 			if !modelFound {
 				allErrors = errors.Join(allErrors, fmt.Errorf("model %q for brick %q not found", appBrick.Model, appBrick.ID))
+			} else {
+				model = m
 			}
 		}
 
 		for appBrickVariableName := range appBrick.Variables {
-			_, exist := indexBrick.GetVariable(appBrickVariableName)
-			if !exist {
-				// TODO: we should return warnings
+			if _, exist := indexBrick.GetVariable(appBrickVariableName); !exist {
 				slog.Warn("[skip] variable does not exist into the brick definition", "variable", appBrickVariableName, "brick", indexBrick.ID)
 			}
 		}
 
-		// Check that all required brick variables are provided by app
+		// Check that all required brick variables are provided by app or by the model
 		for _, indexBrickVariable := range indexBrick.Variables {
-			if indexBrickVariable.IsRequired() {
-				if _, exist := appBrick.Variables[indexBrickVariable.Name]; !exist {
-					allErrors = errors.Join(allErrors, fmt.Errorf("variable %q is required by brick %q", indexBrickVariable.Name, indexBrick.ID))
+			if !indexBrickVariable.IsRequired() {
+				continue
+			}
+			_, providedByApp := appBrick.Variables[indexBrickVariable.Name]
+			providedByModel := false
+			if model != nil {
+				for _, brickConfig := range model.Bricks {
+					if brickConfig.ID == appBrick.ID {
+						_, providedByModel = brickConfig.ModelConfiguration[indexBrickVariable.Name]
+						break
+					}
 				}
 			}
+			if !providedByApp && !providedByModel {
+				allErrors = errors.Join(allErrors, fmt.Errorf("variable %q is required by brick %q", indexBrickVariable.Name, indexBrick.ID))
+			}
 		}
 	}
 	return allErrors
 }
-
 func checkRequiredDevices(bricksIndex *bricksindex.BricksIndex, appBricks []app.Brick, availableDevices peripherals.AvailableDevices) error {
 	requiredDeviceClasses := make(map[peripherals.DeviceClass]bool)
 

internal/orchestrator/check_test.go

@@ -438,3 +438,85 @@
 		})
 	}
 }
+
+func TestCheckBricksModelProvidesRequiredVariable(t *testing.T) {
+	bricksIndex := &bricksindex.BricksIndex{
+		Bricks: []bricksindex.Brick{
+			{
+				ID:   "arduino:ai-brick-with-required-var",
+				Name: "AI brick with required variable",
+				Variables: []bricksindex.BrickVariable{
+					{
+						Name:         "A_REQUIRED_VAR",
+						Description:  "a-required-variable-by-the-brick",
+						DefaultValue: "",
+					},
+				},
+			},
+		},
+	}
+
+	modelIndex := &modelsindex.ModelsIndex{
+		InternalModels: []modelsindex.AIModel{
+			{
+				ID: "my-model",
+				Bricks: []modelsindex.BrickConfig{
+					{
+						ID: "arduino:ai-brick-with-required-var",
+						ModelConfiguration: map[string]string{
+							"A_REQUIRED_VAR": "/models/ootb/ei/my-model.eim",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	testCases := []struct {
+		name          string
+		yamlContent   string
+		expectedError error
+	}{
+		{
+			name: "valid if required variable is provided by model",
+			yamlContent: `
+name: App with model providing required variable
+bricks:
+  - arduino:ai-brick-with-required-var:
+      model: my-model
+`,
+			expectedError: nil,
+		},
+		{
+			name: "invalid if required variable is not provided by app nor by model",
+			yamlContent: `
+name: App missing required variable
+bricks:
+  - arduino:ai-brick-with-required-var:
+`,
+			expectedError: errors.New("variable \"A_REQUIRED_VAR\" is required by brick \"arduino:ai-brick-with-required-var\""),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tempDir := t.TempDir()
+			err := paths.New(tempDir).MkdirAll()
+			require.NoError(t, err)
+			appYaml := paths.New(tempDir, "app.yaml")
+			err = os.WriteFile(appYaml.String(), []byte(tc.yamlContent), 0600)
+			require.NoError(t, err)
+
+			appDescriptor, err := app.ParseDescriptorFile(appYaml)
+			require.NoError(t, err)
+
+			err = checkBricks(appDescriptor, bricksIndex, modelIndex)
+			if tc.expectedError == nil {
+				assert.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				assert.Equal(t, tc.expectedError.Error(), err.Error())
+			}
+		})
+	}
+}

internal/orchestrator/modelsindex/custommodel/custommodel.go

@@ -110,10 +110,11 @@ func (a *AiModel) GetDescriptorPath() *paths.Path {
 }
 
 func (a *AiModel) writeDescriptorFile() error {
-	if !a.ModelDescriptor.IsValid() {
-		// TODO: provide more details about the invalidity
-		return errors.New("invalid model descriptor")
+	err := a.ModelDescriptor.Validate()
+	if err != nil {
+		return fmt.Errorf("invalid model descriptor: %w", err)
 	}
+
 	descriptorPath := a.GetDescriptorPath()
 	if descriptorPath == nil {
 		return errors.New("model descriptor file path is not set")

internal/orchestrator/modelsindex/custommodel/custommodel_test.go

@@ -90,7 +90,6 @@ func TestLoad(t *testing.T) {
 }
 
 func TestStore(t *testing.T) {
-
 	t.Run("it writes descriptor and model file when reader provided", func(t *testing.T) {
 		tempDir := t.TempDir()
 		modelDir := paths.New(tempDir).Join("test-model-with-blob")
@@ -126,8 +125,9 @@ func TestStore(t *testing.T) {
 		modelDir := paths.New(tempDir).Join("test-model-no-filename")
 
 		descr := ModelDescriptor{
-			ID:   "test-id",
-			Name: "test",
+			ID:     "test-id",
+			Name:   "test",
+			Runner: "brick",
 		}
 
 		blobReader := io.NopCloser(bytes.NewReader([]byte("content")))
@@ -142,8 +142,9 @@ func TestStore(t *testing.T) {
 		modelDir := paths.New(tempDir).Join("test-model-large")
 
 		descr := ModelDescriptor{
-			ID:   "test-large",
-			Name: "large model",
+			ID:     "test-large",
+			Name:   "large model",
+			Runner: "brick",
 		}
 
 		// Create 1MB blob

internal/orchestrator/modelsindex/custommodel/parser.go

@@ -19,9 +19,12 @@ package custommodel
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/arduino/go-paths-helper"
 	"github.com/goccy/go-yaml"
+
+	"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
 )
 
 type ModelDescriptor struct {
@@ -52,10 +55,64 @@ func ParseModelDescriptorFile(file *paths.Path) (ModelDescriptor, error) {
 	return descriptor, nil
 }
 
-func (a *ModelDescriptor) IsValid() bool {
-	/*  TODO: check
-	1) brick list are present into the brick-list
-	2) metadata are coherent with the source
-	*/
-	return true
+func (a *ModelDescriptor) Validate() error {
+	if a.ID == "" {
+		return fmt.Errorf("invalid model descriptor: id is empty")
+	}
+	if a.Name == "" {
+		return fmt.Errorf("invalid model descriptor: name is empty")
+	}
+	source, ok := a.Metadata["source"]
+	if !ok {
+		return nil // source is optional
+	}
+
+	switch source {
+	case "edgeimpulse":
+		return validateEdgeImpulseMetadata(a.Metadata)
+	default:
+		return fmt.Errorf("invalid model descriptor: unsupported source '%s'", source)
+	}
+}
+
+func (a *ModelDescriptor) CheckEdgeImpulseBricks(bricksIndex *bricksindex.BricksIndex) error {
+	for _, brickConfig := range a.Bricks {
+		brick, ok := bricksIndex.FindBrickByID(brickConfig.ID)
+		if !ok {
+			return fmt.Errorf("invalid model descriptor: brick with ID '%s' not found", brickConfig.ID)
+		}
+
+		for _, variable := range brick.Variables {
+			if strings.HasPrefix(variable.Name, "EI_") && strings.HasSuffix(variable.Name, "_MODEL") {
+				if val, ok := brickConfig.ModelConfiguration[variable.Name]; !ok || val == "" {
+					return fmt.Errorf("invalid model descriptor: missing model configuration for variable '%s' in brick '%s'", variable.Name, brickConfig.ID)
+				}
+			}
+		}
+	}
+	return nil
+}
+
+func validateEdgeImpulseMetadata(metadata map[string]string) error {
+	requiredFields := []string{
+		"ei-project-id",
+		"ei-impulse-id",
+		"ei-impulse-name",
+		"ei-deployment-version",
+	}
+	for _, field := range requiredFields {
+		if val, ok := metadata[field]; !ok || val == "" {
+			return fmt.Errorf("invalid Edge Impulse metadata: missing required field '%s'", field)
+		}
+	}
+
+	if metadata["ei-model-type"] != "float32" {
+		return fmt.Errorf("invalid Edge Impulse metadata: unsupported model type")
+	}
+
+	if metadata["ei-engine"] != "tflite" {
+		return fmt.Errorf("invalid Edge Impulse metadata: unsupported engine")
+	}
+
+	return nil
 }