From 972c4025d8d08df57bde9f86f6318557e9bf8f26 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 5 Mar 2026 17:49:05 +0100 Subject: [PATCH 01/11] add isValidImplementation --- internal/e2e/daemon/ai_models_test.go | 8 ++- internal/orchestrator/models.go | 2 +- .../modelsindex/custommodel/custommodel.go | 9 +-- .../custommodel/custommodel_test.go | 12 ++-- .../modelsindex/custommodel/parser.go | 61 +++++++++++++++++-- 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/internal/e2e/daemon/ai_models_test.go b/internal/e2e/daemon/ai_models_test.go index ed668f77f..5eef2ca25 100644 --- a/internal/e2e/daemon/ai_models_test.go +++ b/internal/e2e/daemon/ai_models_test.go @@ -34,6 +34,7 @@ import ( "github.com/arduino/arduino-app-cli/internal/api/models" "github.com/arduino/arduino-app-cli/internal/e2e" "github.com/arduino/arduino-app-cli/internal/e2e/client" + "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" "github.com/arduino/arduino-app-cli/internal/orchestrator/modelsindex/custommodel" ) @@ -68,6 +69,7 @@ func TestAIModelList(t *testing.T) { } func TestAIModelDetails(t *testing.T) { + bricksIndex := &bricksindex.BricksIndex{} customModelDir, err := paths.MkTempDir("", "custom-models") require.NoError(t, err) @@ -122,7 +124,7 @@ func TestAIModelDetails(t *testing.T) { Bricks: []custommodel.BrickConfig{ {ID: "arduino:audio_classification"}, }, - }, io.NopCloser(bytes.NewReader([]byte("some random data to create a non empty model file"))), "model.eim") + }, io.NopCloser(bytes.NewReader([]byte("some random data to create a non empty model file"))), "model.eim", bricksIndex) require.NoError(t, err) // We have to add an empty editor because there is a bug that make the function panic if we pass nil @@ -174,6 +176,8 @@ func TestAIModelDetails(t *testing.T) { } func TestAIModelDelete(t *testing.T) { + bricksIndex := &bricksindex.BricksIndex{} + customModelDir, err := paths.MkTempDir("", "custom-models") require.NoError(t, err) @@ -234,7 +238,7 @@ func TestAIModelDelete(t *testing.T) { Bricks: []custommodel.BrickConfig{ {ID: "arduino:audio_classification"}, }, - }, io.NopCloser(bytes.NewReader(nil)), "model.eim") + }, io.NopCloser(bytes.NewReader(nil)), "model.eim", bricksIndex) require.NoError(t, err, "failed to store the model in the custom model directory") /* Create an app */ diff --git a/internal/orchestrator/models.go b/internal/orchestrator/models.go index f6de90820..6220d8134 100644 --- a/internal/orchestrator/models.go +++ b/internal/orchestrator/models.go @@ -347,7 +347,7 @@ func InstallEIModel(ctx context.Context, bricksIndex *bricksindex.BricksIndex, m Bricks: bricks, } - aimodel, err := custommodel.Store(edgeModelsDir, customModelDescriptor, modelRC, "model.eim") + aimodel, err := custommodel.Store(edgeModelsDir, customModelDescriptor, modelRC, "model.eim", bricksIndex) if err != nil { if errors.Is(err, syscall.ENOSPC) { return AIModelItem{}, ErrInsufficientStorage diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel.go b/internal/orchestrator/modelsindex/custommodel/custommodel.go index af0ce8a1d..32b77cb10 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel.go @@ -28,6 +28,7 @@ import ( "github.com/goccy/go-yaml" "github.com/arduino/arduino-app-cli/internal/fatomic" + "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" ) type AiModel struct { @@ -67,7 +68,7 @@ func Load(path *paths.Path) (AiModel, error) { return m, nil } -func Store(dir *paths.Path, descr ModelDescriptor, modelFileReader io.ReadCloser, modelFilename string) (AiModel, error) { +func Store(dir *paths.Path, descr ModelDescriptor, modelFileReader io.ReadCloser, modelFilename string, bricksIndex *bricksindex.BricksIndex) (AiModel, error) { if modelFileReader == nil { return AiModel{}, fmt.Errorf("model file reader cannot be nil") } @@ -98,7 +99,7 @@ func Store(dir *paths.Path, descr ModelDescriptor, modelFileReader io.ReadCloser ModelDescriptor: descr, } - err = m.writeDescriptorFile() + err = m.writeDescriptorFile(bricksIndex) if err != nil { return AiModel{}, fmt.Errorf("failed to write model: %w", err) } @@ -109,8 +110,8 @@ func (a *AiModel) GetDescriptorPath() *paths.Path { return a.FullPath.Join("model.yaml") } -func (a *AiModel) writeDescriptorFile() error { - if !a.ModelDescriptor.IsValid() { +func (a *AiModel) writeDescriptorFile(bricksIndex *bricksindex.BricksIndex) error { + if !a.ModelDescriptor.IsValid(bricksIndex) { // TODO: provide more details about the invalidity return errors.New("invalid model descriptor") } diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go index b0b5b987a..5a99cb407 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go @@ -30,6 +30,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.bug.st/f" + + "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" ) func TestLoad(t *testing.T) { @@ -90,7 +92,7 @@ func TestLoad(t *testing.T) { } func TestStore(t *testing.T) { - + bricksIndex := &bricksindex.BricksIndex{} 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") @@ -104,7 +106,7 @@ func TestStore(t *testing.T) { blobContent := []byte("this is model blob content") blobReader := io.NopCloser(bytes.NewReader(blobContent)) - m, err := Store(modelDir, descr, blobReader, "model.blob") + m, err := Store(modelDir, descr, blobReader, "model.blob", bricksIndex) require.NoError(t, err) assert.Equal(t, descr, m.ModelDescriptor) @@ -132,7 +134,7 @@ func TestStore(t *testing.T) { blobReader := io.NopCloser(bytes.NewReader([]byte("content"))) - _, err := Store(modelDir, descr, blobReader, "") + _, err := Store(modelDir, descr, blobReader, "", bricksIndex) require.Error(t, err) assert.Contains(t, err.Error(), "model filename must be provided") }) @@ -153,7 +155,7 @@ func TestStore(t *testing.T) { } blobReader := io.NopCloser(bytes.NewReader(largeBlob)) - _, err := Store(modelDir, descr, blobReader, "large-model.blob") + _, err := Store(modelDir, descr, blobReader, "large-model.blob", bricksIndex) require.NoError(t, err) blobPath := modelDir.Join("large-model.blob") @@ -186,7 +188,7 @@ func TestStore(t *testing.T) { require.NoError(t, err) defer resp.Body.Close() - _, err = Store(modelDir, descr, resp.Body, "model-http.blob") + _, err = Store(modelDir, descr, resp.Body, "model-http.blob", bricksIndex) require.NoError(t, err) blobPath := modelDir.Join("model-http.blob") diff --git a/internal/orchestrator/modelsindex/custommodel/parser.go b/internal/orchestrator/modelsindex/custommodel/parser.go index 04e693e7f..335b39c36 100644 --- a/internal/orchestrator/modelsindex/custommodel/parser.go +++ b/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,58 @@ 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 - */ +func (a *ModelDescriptor) IsValid(bricksIndex *bricksindex.BricksIndex) bool { + if a.ID == "" || a.Name == "" || a.Runner != "brick" { + return false + } + for _, brickConfig := range a.Bricks { + brick, ok := bricksIndex.FindBrickByID(brickConfig.ID) + if !ok { + return false + } + + 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 false + } + } + } + } + + source, ok := a.Metadata["source"] + if !ok { + return true // source is optional + } + + switch source { + case "edgeimpulse": + return isValidEdgeImpulseMetadata(a.Metadata) + default: + return true + } +} + +func isValidEdgeImpulseMetadata(metadata map[string]string) bool { + 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 false + } + } + + if metadata["ei-model-type"] != "float32" { + return false + } + + if metadata["ei-engine"] != "tflite" { + return false + } + return true } From c957abf0ed5a3362cd51db951a13b8d3f37214eb Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Mon, 9 Mar 2026 15:59:01 +0100 Subject: [PATCH 02/11] add model validation part 2 --- .../modelsindex/custommodel/custommodel.go | 11 +++-- .../modelsindex/custommodel/parser.go | 46 ++++++++++--------- 2 files changed, 33 insertions(+), 24 deletions(-) diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel.go b/internal/orchestrator/modelsindex/custommodel/custommodel.go index 32b77cb10..496d59351 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel.go @@ -111,10 +111,15 @@ func (a *AiModel) GetDescriptorPath() *paths.Path { } func (a *AiModel) writeDescriptorFile(bricksIndex *bricksindex.BricksIndex) error { - if !a.ModelDescriptor.IsValid(bricksIndex) { - // TODO: provide more details about the invalidity - return errors.New("invalid model descriptor") + err := a.ModelDescriptor.Validate(bricksIndex) + if err != nil { + return fmt.Errorf("invalid model descriptor: %w", err) + } + err = a.ModelDescriptor.CheckBricks(bricksIndex) + 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") diff --git a/internal/orchestrator/modelsindex/custommodel/parser.go b/internal/orchestrator/modelsindex/custommodel/parser.go index 335b39c36..2837c2eab 100644 --- a/internal/orchestrator/modelsindex/custommodel/parser.go +++ b/internal/orchestrator/modelsindex/custommodel/parser.go @@ -55,39 +55,43 @@ func ParseModelDescriptorFile(file *paths.Path) (ModelDescriptor, error) { return descriptor, nil } -func (a *ModelDescriptor) IsValid(bricksIndex *bricksindex.BricksIndex) bool { +func (a *ModelDescriptor) Validate(bricksIndex *bricksindex.BricksIndex) error { if a.ID == "" || a.Name == "" || a.Runner != "brick" { - return false + return fmt.Errorf("invalid model descriptor: id or name is empty, or runner is not 'brick'") } + + 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) CheckBricks(bricksIndex *bricksindex.BricksIndex) error { for _, brickConfig := range a.Bricks { brick, ok := bricksIndex.FindBrickByID(brickConfig.ID) if !ok { - return false + 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 false + return fmt.Errorf("invalid model descriptor: missing model configuration for variable '%s' in brick '%s'", variable.Name, brickConfig.ID) } } } } - - source, ok := a.Metadata["source"] - if !ok { - return true // source is optional - } - - switch source { - case "edgeimpulse": - return isValidEdgeImpulseMetadata(a.Metadata) - default: - return true - } + return nil } -func isValidEdgeImpulseMetadata(metadata map[string]string) bool { +func validateEdgeImpulseMetadata(metadata map[string]string) error { requiredFields := []string{ "ei-project-id", "ei-impulse-id", @@ -96,17 +100,17 @@ func isValidEdgeImpulseMetadata(metadata map[string]string) bool { } for _, field := range requiredFields { if val, ok := metadata[field]; !ok || val == "" { - return false + return fmt.Errorf("invalid Edge Impulse metadata: missing required field '%s'", field) } } if metadata["ei-model-type"] != "float32" { - return false + return fmt.Errorf("invalid Edge Impulse metadata: unsupported model type") } if metadata["ei-engine"] != "tflite" { - return false + return fmt.Errorf("invalid Edge Impulse metadata: unsupported engine") } - return true + return nil } From e1f6e8e2290e32e52d236a83c6e7fc47be71ea88 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 12 Mar 2026 17:02:54 +0100 Subject: [PATCH 03/11] remove variable brick checks --- internal/e2e/daemon/ai_models_test.go | 7 ++----- internal/orchestrator/models.go | 2 +- .../modelsindex/custommodel/custommodel.go | 13 ++++--------- .../modelsindex/custommodel/custommodel_test.go | 11 ++++------- .../orchestrator/modelsindex/custommodel/parser.go | 4 ++-- 5 files changed, 13 insertions(+), 24 deletions(-) diff --git a/internal/e2e/daemon/ai_models_test.go b/internal/e2e/daemon/ai_models_test.go index 5eef2ca25..df56f658c 100644 --- a/internal/e2e/daemon/ai_models_test.go +++ b/internal/e2e/daemon/ai_models_test.go @@ -34,7 +34,6 @@ import ( "github.com/arduino/arduino-app-cli/internal/api/models" "github.com/arduino/arduino-app-cli/internal/e2e" "github.com/arduino/arduino-app-cli/internal/e2e/client" - "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" "github.com/arduino/arduino-app-cli/internal/orchestrator/modelsindex/custommodel" ) @@ -69,7 +68,6 @@ func TestAIModelList(t *testing.T) { } func TestAIModelDetails(t *testing.T) { - bricksIndex := &bricksindex.BricksIndex{} customModelDir, err := paths.MkTempDir("", "custom-models") require.NoError(t, err) @@ -124,7 +122,7 @@ func TestAIModelDetails(t *testing.T) { Bricks: []custommodel.BrickConfig{ {ID: "arduino:audio_classification"}, }, - }, io.NopCloser(bytes.NewReader([]byte("some random data to create a non empty model file"))), "model.eim", bricksIndex) + }, io.NopCloser(bytes.NewReader([]byte("some random data to create a non empty model file"))), "model.eim") require.NoError(t, err) // We have to add an empty editor because there is a bug that make the function panic if we pass nil @@ -176,7 +174,6 @@ func TestAIModelDetails(t *testing.T) { } func TestAIModelDelete(t *testing.T) { - bricksIndex := &bricksindex.BricksIndex{} customModelDir, err := paths.MkTempDir("", "custom-models") require.NoError(t, err) @@ -238,7 +235,7 @@ func TestAIModelDelete(t *testing.T) { Bricks: []custommodel.BrickConfig{ {ID: "arduino:audio_classification"}, }, - }, io.NopCloser(bytes.NewReader(nil)), "model.eim", bricksIndex) + }, io.NopCloser(bytes.NewReader(nil)), "model.eim") require.NoError(t, err, "failed to store the model in the custom model directory") /* Create an app */ diff --git a/internal/orchestrator/models.go b/internal/orchestrator/models.go index 6220d8134..f6de90820 100644 --- a/internal/orchestrator/models.go +++ b/internal/orchestrator/models.go @@ -347,7 +347,7 @@ func InstallEIModel(ctx context.Context, bricksIndex *bricksindex.BricksIndex, m Bricks: bricks, } - aimodel, err := custommodel.Store(edgeModelsDir, customModelDescriptor, modelRC, "model.eim", bricksIndex) + aimodel, err := custommodel.Store(edgeModelsDir, customModelDescriptor, modelRC, "model.eim") if err != nil { if errors.Is(err, syscall.ENOSPC) { return AIModelItem{}, ErrInsufficientStorage diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel.go b/internal/orchestrator/modelsindex/custommodel/custommodel.go index 496d59351..6562176ba 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel.go @@ -28,7 +28,6 @@ import ( "github.com/goccy/go-yaml" "github.com/arduino/arduino-app-cli/internal/fatomic" - "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" ) type AiModel struct { @@ -68,7 +67,7 @@ func Load(path *paths.Path) (AiModel, error) { return m, nil } -func Store(dir *paths.Path, descr ModelDescriptor, modelFileReader io.ReadCloser, modelFilename string, bricksIndex *bricksindex.BricksIndex) (AiModel, error) { +func Store(dir *paths.Path, descr ModelDescriptor, modelFileReader io.ReadCloser, modelFilename string) (AiModel, error) { if modelFileReader == nil { return AiModel{}, fmt.Errorf("model file reader cannot be nil") } @@ -99,7 +98,7 @@ func Store(dir *paths.Path, descr ModelDescriptor, modelFileReader io.ReadCloser ModelDescriptor: descr, } - err = m.writeDescriptorFile(bricksIndex) + err = m.writeDescriptorFile() if err != nil { return AiModel{}, fmt.Errorf("failed to write model: %w", err) } @@ -110,12 +109,8 @@ func (a *AiModel) GetDescriptorPath() *paths.Path { return a.FullPath.Join("model.yaml") } -func (a *AiModel) writeDescriptorFile(bricksIndex *bricksindex.BricksIndex) error { - err := a.ModelDescriptor.Validate(bricksIndex) - if err != nil { - return fmt.Errorf("invalid model descriptor: %w", err) - } - err = a.ModelDescriptor.CheckBricks(bricksIndex) +func (a *AiModel) writeDescriptorFile() error { + err := a.ModelDescriptor.Validate() if err != nil { return fmt.Errorf("invalid model descriptor: %w", err) } diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go index 5a99cb407..3d2718c7e 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go @@ -30,8 +30,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.bug.st/f" - - "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" ) func TestLoad(t *testing.T) { @@ -92,7 +90,6 @@ func TestLoad(t *testing.T) { } func TestStore(t *testing.T) { - bricksIndex := &bricksindex.BricksIndex{} 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") @@ -106,7 +103,7 @@ func TestStore(t *testing.T) { blobContent := []byte("this is model blob content") blobReader := io.NopCloser(bytes.NewReader(blobContent)) - m, err := Store(modelDir, descr, blobReader, "model.blob", bricksIndex) + m, err := Store(modelDir, descr, blobReader, "model.blob") require.NoError(t, err) assert.Equal(t, descr, m.ModelDescriptor) @@ -134,7 +131,7 @@ func TestStore(t *testing.T) { blobReader := io.NopCloser(bytes.NewReader([]byte("content"))) - _, err := Store(modelDir, descr, blobReader, "", bricksIndex) + _, err := Store(modelDir, descr, blobReader, "") require.Error(t, err) assert.Contains(t, err.Error(), "model filename must be provided") }) @@ -155,7 +152,7 @@ func TestStore(t *testing.T) { } blobReader := io.NopCloser(bytes.NewReader(largeBlob)) - _, err := Store(modelDir, descr, blobReader, "large-model.blob", bricksIndex) + _, err := Store(modelDir, descr, blobReader, "large-model.blob") require.NoError(t, err) blobPath := modelDir.Join("large-model.blob") @@ -188,7 +185,7 @@ func TestStore(t *testing.T) { require.NoError(t, err) defer resp.Body.Close() - _, err = Store(modelDir, descr, resp.Body, "model-http.blob", bricksIndex) + _, err = Store(modelDir, descr, resp.Body, "model-http.blob") require.NoError(t, err) blobPath := modelDir.Join("model-http.blob") diff --git a/internal/orchestrator/modelsindex/custommodel/parser.go b/internal/orchestrator/modelsindex/custommodel/parser.go index 2837c2eab..ecfeae9ac 100644 --- a/internal/orchestrator/modelsindex/custommodel/parser.go +++ b/internal/orchestrator/modelsindex/custommodel/parser.go @@ -55,7 +55,7 @@ func ParseModelDescriptorFile(file *paths.Path) (ModelDescriptor, error) { return descriptor, nil } -func (a *ModelDescriptor) Validate(bricksIndex *bricksindex.BricksIndex) error { +func (a *ModelDescriptor) Validate() error { if a.ID == "" || a.Name == "" || a.Runner != "brick" { return fmt.Errorf("invalid model descriptor: id or name is empty, or runner is not 'brick'") } @@ -73,7 +73,7 @@ func (a *ModelDescriptor) Validate(bricksIndex *bricksindex.BricksIndex) error { } } -func (a *ModelDescriptor) CheckBricks(bricksIndex *bricksindex.BricksIndex) error { +func (a *ModelDescriptor) CheckEdgeImpulseBricks(bricksIndex *bricksindex.BricksIndex) error { for _, brickConfig := range a.Bricks { brick, ok := bricksIndex.FindBrickByID(brickConfig.ID) if !ok { From 52e0ab7718b432035ce83e2b3294d4e4d328be4f Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 12 Mar 2026 17:25:53 +0100 Subject: [PATCH 04/11] add brick-model variable check --- internal/orchestrator/check.go | 31 +++++++---- internal/orchestrator/check_test.go | 82 +++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 10 deletions(-) diff --git a/internal/orchestrator/check.go b/internal/orchestrator/check.go index 421a62c6c..4839c84b0 100644 --- a/internal/orchestrator/check.go +++ b/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) diff --git a/internal/orchestrator/check_test.go b/internal/orchestrator/check_test.go index 1d86b6f5e..f2f66485b 100644 --- a/internal/orchestrator/check_test.go +++ b/internal/orchestrator/check_test.go @@ -438,3 +438,85 @@ func TestCheckRequiredDevice(t *testing.T) { }) } } + +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: "EI_MODEL_PATH", + Description: "Path to the model file", + DefaultValue: "", // required + }, + }, + }, + }, + } + + modelIndex := &modelsindex.ModelsIndex{ + InternalModels: []modelsindex.AIModel{ + { + ID: "my-model", + Bricks: []modelsindex.BrickConfig{ + { + ID: "arduino:ai-brick-with-required-var", + ModelConfiguration: map[string]string{ + "EI_MODEL_PATH": "/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 \"EI_MODEL_PATH\" 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()) + } + }) + } +} From 6e7eba1ae4bb47666f2d485210c3210b3b59b218 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Wed, 18 Mar 2026 16:42:44 +0100 Subject: [PATCH 05/11] make better error checking --- .../orchestrator/modelsindex/custommodel/parser.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/orchestrator/modelsindex/custommodel/parser.go b/internal/orchestrator/modelsindex/custommodel/parser.go index ecfeae9ac..0d3593948 100644 --- a/internal/orchestrator/modelsindex/custommodel/parser.go +++ b/internal/orchestrator/modelsindex/custommodel/parser.go @@ -56,10 +56,15 @@ func ParseModelDescriptorFile(file *paths.Path) (ModelDescriptor, error) { } func (a *ModelDescriptor) Validate() error { - if a.ID == "" || a.Name == "" || a.Runner != "brick" { - return fmt.Errorf("invalid model descriptor: id or name is empty, or runner is not 'brick'") + if a.ID == "" { + return fmt.Errorf("invalid model descriptor: id is empty") + } + if a.Name == "" { + return fmt.Errorf("invalid model descriptor: name is empty") + } + if a.Runner != "brick" { + return fmt.Errorf("invalid model descriptor: runner must be 'brick', got %q", a.Runner) } - source, ok := a.Metadata["source"] if !ok { return nil // source is optional From 8ba6ce881e33c5ed4a97acccc0b649cbc146b1c5 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 19 Mar 2026 12:13:57 +0100 Subject: [PATCH 06/11] fix e2e tests --- internal/e2e/daemon/ai_models_test.go | 4 +++- .../modelsindex/custommodel/custommodel_test.go | 16 ++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/internal/e2e/daemon/ai_models_test.go b/internal/e2e/daemon/ai_models_test.go index df56f658c..a26881fdc 100644 --- a/internal/e2e/daemon/ai_models_test.go +++ b/internal/e2e/daemon/ai_models_test.go @@ -118,6 +118,7 @@ func TestAIModelDetails(t *testing.T) { _, err := custommodel.Store(customModelDir.Join("my-model"), custommodel.ModelDescriptor{ ID: "custom-classification-model-eim", Name: "this the name of the model", + Runner: "brick", Description: "this is the description of the model", Bricks: []custommodel.BrickConfig{ {ID: "arduino:audio_classification"}, @@ -135,7 +136,7 @@ func TestAIModelDetails(t *testing.T) { Id: f.Ptr("custom-classification-model-eim"), Name: f.Ptr("this the name of the model"), IsBuiltin: f.Ptr(false), - Runner: f.Ptr(""), + Runner: f.Ptr("brick"), Description: f.Ptr("this is the description of the model"), BrickIds: &[]string{"arduino:audio_classification"}, DiskUsage: f.Ptr(222), @@ -231,6 +232,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"}, diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go index 3d2718c7e..d6b86e2d0 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go @@ -97,6 +97,7 @@ func TestStore(t *testing.T) { descr := ModelDescriptor{ ID: "test-id-blob", Name: "test model with blob", + Runner: "brick", Description: "test description", } @@ -125,8 +126,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"))) @@ -141,8 +143,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 @@ -169,8 +172,9 @@ func TestStore(t *testing.T) { modelDir := paths.New(tempDir).Join("test-model-http") descr := ModelDescriptor{ - ID: "test-http", - Name: "http model", + ID: "test-http", + Name: "http model", + Runner: "brick", } payload := []byte("http served model content") From ad4cba6e8cb68619a4600eb454778ffd24917f47 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 19 Mar 2026 12:55:09 +0100 Subject: [PATCH 07/11] update test --- .../orchestrator/modelsindex/custommodel/custommodel_test.go | 1 + .../modelsindex/custommodel/testdata/my-model/model.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go index d6b86e2d0..c2b8df143 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go @@ -69,6 +69,7 @@ func TestLoad(t *testing.T) { ID: "my-model-id", Name: "my custom model name", Description: "my description", + Runner: "brick", Bricks: []BrickConfig{ { ID: "arduino:a-brick-id", diff --git a/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml b/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml index bbcf9ece9..53a4467d3 100644 --- a/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml +++ b/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml @@ -1,6 +1,7 @@ id: "my-model-id" name: "my custom model name" description: "my description" +runner: "brick" bricks: - id: "arduino:a-brick-id" model_configuration: From 8641287dfbbc9a54594b823524e2f52507ba1632 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 19 Mar 2026 14:47:51 +0100 Subject: [PATCH 08/11] make loadCustomModels returning runner field --- internal/e2e/daemon/ai_models_test.go | 2 +- internal/orchestrator/modelsindex/models_index.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/e2e/daemon/ai_models_test.go b/internal/e2e/daemon/ai_models_test.go index a26881fdc..41fceaac3 100644 --- a/internal/e2e/daemon/ai_models_test.go +++ b/internal/e2e/daemon/ai_models_test.go @@ -139,7 +139,7 @@ func TestAIModelDetails(t *testing.T) { Runner: f.Ptr("brick"), 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 diff --git a/internal/orchestrator/modelsindex/models_index.go b/internal/orchestrator/modelsindex/models_index.go index eed3eb01c..323e81a39 100644 --- a/internal/orchestrator/modelsindex/models_index.go +++ b/internal/orchestrator/modelsindex/models_index.go @@ -180,6 +180,7 @@ func loadCustomModels(dir *paths.Path) ([]AIModel, error) { ID: m.ModelDescriptor.ID, Name: m.ModelDescriptor.Name, ModuleDescription: m.ModelDescriptor.Description, + Runner: m.ModelDescriptor.Runner, Bricks: f.Map(m.ModelDescriptor.Bricks, func(b custommodel.BrickConfig) BrickConfig { return BrickConfig(b) }), From f80d8705f3cf4d05dae1f3662ff787bea54bc434 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Thu, 2 Apr 2026 16:23:15 +0200 Subject: [PATCH 09/11] code review fix --- internal/orchestrator/check_test.go | 10 +++++----- .../modelsindex/custommodel/custommodel_test.go | 7 ++----- .../orchestrator/modelsindex/custommodel/parser.go | 3 --- .../custommodel/testdata/my-model/model.yaml | 1 - internal/orchestrator/modelsindex/models_index.go | 1 - 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/internal/orchestrator/check_test.go b/internal/orchestrator/check_test.go index f2f66485b..aab92f0d6 100644 --- a/internal/orchestrator/check_test.go +++ b/internal/orchestrator/check_test.go @@ -447,9 +447,9 @@ func TestCheckBricksModelProvidesRequiredVariable(t *testing.T) { Name: "AI brick with required variable", Variables: []bricksindex.BrickVariable{ { - Name: "EI_MODEL_PATH", - Description: "Path to the model file", - DefaultValue: "", // required + Name: "A_REQUIRED_VAR", + Description: "a-required-variable-by-the-brick", + DefaultValue: "", }, }, }, @@ -464,7 +464,7 @@ func TestCheckBricksModelProvidesRequiredVariable(t *testing.T) { { ID: "arduino:ai-brick-with-required-var", ModelConfiguration: map[string]string{ - "EI_MODEL_PATH": "/models/ootb/ei/my-model.eim", + "A_REQUIRED_VAR": "/models/ootb/ei/my-model.eim", }, }, }, @@ -494,7 +494,7 @@ name: App missing required variable bricks: - arduino:ai-brick-with-required-var: `, - expectedError: errors.New("variable \"EI_MODEL_PATH\" is required by brick \"arduino:ai-brick-with-required-var\""), + expectedError: errors.New("variable \"A_REQUIRED_VAR\" is required by brick \"arduino:ai-brick-with-required-var\""), }, } diff --git a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go index c2b8df143..2e6cb5c51 100644 --- a/internal/orchestrator/modelsindex/custommodel/custommodel_test.go +++ b/internal/orchestrator/modelsindex/custommodel/custommodel_test.go @@ -69,7 +69,6 @@ func TestLoad(t *testing.T) { ID: "my-model-id", Name: "my custom model name", Description: "my description", - Runner: "brick", Bricks: []BrickConfig{ { ID: "arduino:a-brick-id", @@ -98,7 +97,6 @@ func TestStore(t *testing.T) { descr := ModelDescriptor{ ID: "test-id-blob", Name: "test model with blob", - Runner: "brick", Description: "test description", } @@ -173,9 +171,8 @@ func TestStore(t *testing.T) { modelDir := paths.New(tempDir).Join("test-model-http") descr := ModelDescriptor{ - ID: "test-http", - Name: "http model", - Runner: "brick", + ID: "test-http", + Name: "http model", } payload := []byte("http served model content") diff --git a/internal/orchestrator/modelsindex/custommodel/parser.go b/internal/orchestrator/modelsindex/custommodel/parser.go index 0d3593948..26d12a3cf 100644 --- a/internal/orchestrator/modelsindex/custommodel/parser.go +++ b/internal/orchestrator/modelsindex/custommodel/parser.go @@ -62,9 +62,6 @@ func (a *ModelDescriptor) Validate() error { if a.Name == "" { return fmt.Errorf("invalid model descriptor: name is empty") } - if a.Runner != "brick" { - return fmt.Errorf("invalid model descriptor: runner must be 'brick', got %q", a.Runner) - } source, ok := a.Metadata["source"] if !ok { return nil // source is optional diff --git a/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml b/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml index 53a4467d3..bbcf9ece9 100644 --- a/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml +++ b/internal/orchestrator/modelsindex/custommodel/testdata/my-model/model.yaml @@ -1,7 +1,6 @@ id: "my-model-id" name: "my custom model name" description: "my description" -runner: "brick" bricks: - id: "arduino:a-brick-id" model_configuration: diff --git a/internal/orchestrator/modelsindex/models_index.go b/internal/orchestrator/modelsindex/models_index.go index 323e81a39..eed3eb01c 100644 --- a/internal/orchestrator/modelsindex/models_index.go +++ b/internal/orchestrator/modelsindex/models_index.go @@ -180,7 +180,6 @@ func loadCustomModels(dir *paths.Path) ([]AIModel, error) { ID: m.ModelDescriptor.ID, Name: m.ModelDescriptor.Name, ModuleDescription: m.ModelDescriptor.Description, - Runner: m.ModelDescriptor.Runner, Bricks: f.Map(m.ModelDescriptor.Bricks, func(b custommodel.BrickConfig) BrickConfig { return BrickConfig(b) }), From e0d07d6e50c0d4ff9de71d260f5eb4064427a5d0 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Fri, 3 Apr 2026 15:25:01 +0200 Subject: [PATCH 10/11] fix test --- internal/e2e/daemon/ai_models_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/e2e/daemon/ai_models_test.go b/internal/e2e/daemon/ai_models_test.go index 41fceaac3..03c3fb545 100644 --- a/internal/e2e/daemon/ai_models_test.go +++ b/internal/e2e/daemon/ai_models_test.go @@ -117,8 +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", - Runner: "brick", + Name: "this is the name of the model", Description: "this is the description of the model", Bricks: []custommodel.BrickConfig{ {ID: "arduino:audio_classification"}, @@ -134,9 +133,9 @@ 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("brick"), + Runner: f.Ptr(""), Description: f.Ptr("this is the description of the model"), BrickIds: &[]string{"arduino:audio_classification"}, DiskUsage: f.Ptr(225), From 9917c517a556c5229f2bd7b1a615127a21e137a2 Mon Sep 17 00:00:00 2001 From: mirkoCrobu Date: Fri, 3 Apr 2026 16:59:29 +0200 Subject: [PATCH 11/11] fix test --- internal/api/docs/openapi.yaml | 4 ++-- internal/e2e/client/client.gen.go | 6 +++--- internal/orchestrator/check_test.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/api/docs/openapi.yaml b/internal/api/docs/openapi.yaml index ec019c347..437ccd0a6 100644 --- a/internal/api/docs/openapi.yaml +++ b/internal/api/docs/openapi.yaml @@ -1505,8 +1505,6 @@ components: type: object AppLocalBrickCreateRequest: properties: - description: - type: string name: type: string type: object @@ -1623,6 +1621,8 @@ components: type: string name: type: string + readme: + type: string require_model: type: boolean status: diff --git a/internal/e2e/client/client.gen.go b/internal/e2e/client/client.gen.go index 199ef5698..79a35d7d6 100644 --- a/internal/e2e/client/client.gen.go +++ b/internal/e2e/client/client.gen.go @@ -170,8 +170,7 @@ type AppListResponse struct { // AppLocalBrickCreateRequest defines model for AppLocalBrickCreateRequest. type AppLocalBrickCreateRequest struct { - Description *string `json:"description,omitempty"` - Name *string `json:"name,omitempty"` + Name *string `json:"name,omitempty"` } // AppLocalBrickCreateResponse defines model for AppLocalBrickCreateResponse. @@ -235,6 +234,7 @@ type BrickInstance struct { Id *string `json:"id,omitempty"` Model *string `json:"model,omitempty"` Name *string `json:"name,omitempty"` + Readme *string `json:"readme,omitempty"` RequireModel *bool `json:"require_model,omitempty"` Status *string `json:"status,omitempty"` @@ -1681,7 +1681,7 @@ func NewCreateAppLocalBrickRequestWithBody(server string, appID string, contentT var pathParam0 string - pathParam0, err = runtime.StyleParamWithLocation("simple", false, "appID", runtime.ParamLocationPath, appID) + pathParam0, err = runtime.StyleParamWithOptions("simple", false, "appID", appID, runtime.StyleParamOptions{ParamLocation: runtime.ParamLocationPath, Type: "string", Format: ""}) if err != nil { return nil, err } diff --git a/internal/orchestrator/check_test.go b/internal/orchestrator/check_test.go index aab92f0d6..a05039256 100644 --- a/internal/orchestrator/check_test.go +++ b/internal/orchestrator/check_test.go @@ -441,7 +441,7 @@ func TestCheckRequiredDevice(t *testing.T) { func TestCheckBricksModelProvidesRequiredVariable(t *testing.T) { bricksIndex := &bricksindex.BricksIndex{ - Bricks: []bricksindex.Brick{ + BuiltInBricks: []bricksindex.Brick{ { ID: "arduino:ai-brick-with-required-var", Name: "AI brick with required variable",