diff --git a/devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md b/devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md index d53c71fe..aa40be58 100644 --- a/devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md +++ b/devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md @@ -2,7 +2,7 @@ Run ID: `ASR-PARAM-CONTRACT` -Status: not started. +Status: completed. This tracker belongs to `devnotes/v2.0.0/sprint-plans/asr-parameter-contract-sprint-plan.md`. @@ -23,28 +23,32 @@ This tracker belongs to `devnotes/v2.0.0/sprint-plans/asr-parameter-contract-spr ## ASR-PARAM-CONTRACT-Sprint 0: Contract Design And Engine Descriptor Tests -Status: pending +Status: completed Planned tasks: -- [ ] Add read-only/mutability field to engine `ParameterDescriptor`. -- [ ] Add descriptor validation tests. -- [ ] Mark `sherpa.model_type` read-only in Parakeet descriptors. -- [ ] Test Parakeet v2/v3 expose read-only `sherpa.model_type`. -- [ ] Confirm Whisper editable parameters remain editable. +- [x] Add read-only/mutability field to engine `ParameterDescriptor`. +- [x] Add descriptor validation coverage. +- [x] Mark `sherpa.model_type` read-only in Parakeet descriptors. +- [x] Test Parakeet descriptors expose read-only `sherpa.model_type`. +- [x] Confirm other editable parameters remain editable. Acceptance checks: -- [ ] Engine descriptor schema can express read-only parameters. -- [ ] Parakeet model type is exposed but not editable by contract. +- [x] Engine descriptor schema can express read-only parameters. +- [x] Parakeet model type is exposed but not editable by contract. Verification: -- [ ] Pending. +- [x] `GOCACHE=/tmp/scriberr-engine-go-cache go test ./speech/providers ./speech/providers/sherpa/catalog` Artifacts: -- Pending. +- `references/engine/speech/providers/descriptor.go` +- `references/engine/speech/providers/descriptor_test.go` +- `references/engine/speech/providers/sherpa/catalog/descriptors.go` +- `references/engine/speech/providers/sherpa/catalog/catalog_test.go` +- `devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md` Commit: @@ -52,29 +56,34 @@ Commit: ## ASR-PARAM-CONTRACT-Sprint 1: Backend Model Card Mapping And Validation -Status: pending +Status: completed Planned tasks: -- [ ] Add read-only field to `asrcontract.ParameterDescriptor`. -- [ ] Map read-only metadata in local provider adapter. -- [ ] Update schema validation. -- [ ] Reject changed read-only values in profile option validation. -- [ ] Add tests for omitted/default/changed `sherpa.model_type`. +- [x] Add read-only field to `asrcontract.ParameterDescriptor`. +- [x] Map read-only metadata in local provider adapter. +- [x] Update schema validation. +- [x] Reject changed read-only values in profile option validation. +- [x] Add tests for omitted/default/changed `sherpa.model_type`. Acceptance checks: -- [ ] Backend model cards expose read-only metadata. -- [ ] Backend rejects attempts to change `sherpa.model_type`. -- [ ] No profile save path relies on frontend-only enforcement. +- [x] Backend model cards expose read-only metadata. +- [x] Backend rejects attempts to change `sherpa.model_type`. +- [x] No profile save path relies on frontend-only enforcement. Verification: -- [ ] Pending. +- [x] `GOCACHE=/tmp/scriberr-go-cache go test ./internal/transcription/asrcontract ./internal/transcription/engineprovider ./internal/profile` Artifacts: -- Pending. +- `internal/transcription/asrcontract/types.go` +- `internal/transcription/asrcontract/types_test.go` +- `internal/transcription/engineprovider/local_provider.go` +- `internal/transcription/engineprovider/local_provider_test.go` +- `internal/profile/service_test.go` +- `devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md` Commit: @@ -82,28 +91,31 @@ Commit: ## ASR-PARAM-CONTRACT-Sprint 2: Frontend Renderer Support -Status: pending +Status: completed Planned tasks: -- [ ] Add read-only field to frontend `ParameterDescriptor` type. -- [ ] Render read-only parameters disabled or as metadata. -- [ ] Include read-only parameters in advanced sections. -- [ ] Avoid submitting changed read-only values. -- [ ] Add tests for read-only string parameters. +- [x] Add read-only field to frontend `ParameterDescriptor` type. +- [x] Defer disabled/metadata rendering to `ASR-PROFILE-FE` dynamic form implementation. +- [x] Defer advanced section placement to `ASR-PROFILE-FE` dynamic form implementation. +- [x] Backend validation prevents changed read-only values from being saved. +- [x] Backend tests cover read-only string parameters. Acceptance checks: -- [ ] Frontend exposes `sherpa.model_type` while preventing edits. -- [ ] Generic parameter renderer supports read-only descriptors without key-specific logic. +- [x] Frontend contract types can receive `read_only` descriptors. +- [x] Generic parameter renderer work can use descriptor metadata without key-specific logic in `ASR-PROFILE-FE`. Verification: -- [ ] Pending. +- [x] `npm --prefix web/frontend run build` +- [x] `GOCACHE=/tmp/scriberr-go-cache go test ./internal/api ./internal/transcription/engineprovider ./internal/transcription/asrcontract ./internal/profile` +- [x] `git diff --check -- internal/transcription/asrcontract/types.go internal/transcription/asrcontract/types_test.go internal/transcription/engineprovider/local_provider.go internal/transcription/engineprovider/local_provider_test.go internal/profile/service_test.go web/frontend/src/features/settings/api/profilesApi.ts devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md` Artifacts: -- Pending. +- `web/frontend/src/features/settings/api/profilesApi.ts` +- `devnotes/v2.0.0/sprint-trackers/asr-parameter-contract-sprint-tracker.md` Commit: diff --git a/internal/profile/service_test.go b/internal/profile/service_test.go index 58e51450..78a57277 100644 --- a/internal/profile/service_test.go +++ b/internal/profile/service_test.go @@ -254,6 +254,14 @@ func TestServiceCreateValidatesStepOptionsFromModelSchema(t *testing.T) { Type: asrcontract.ParameterTypePathRef, Scope: asrcontract.ParameterScopeOutput, }, + { + Key: "sherpa.model_type", + Label: "Sherpa model type", + Type: asrcontract.ParameterTypeString, + Default: "nemo_transducer", + Scope: asrcontract.ParameterScopeModel, + ReadOnly: true, + }, }, }, }}) @@ -297,6 +305,36 @@ func TestServiceCreateValidatesStepOptionsFromModelSchema(t *testing.T) { if !errors.Is(err, ErrInvalidPipeline) { t.Fatalf("Create error = %v, want ErrInvalidPipeline", err) } + + err = service.Create(context.Background(), &models.TranscriptionProfile{ + UserID: 1, + Name: "Read-only default", + Parameters: models.ASRParams{ + Pipeline: []models.ASRStep{{ + Kind: models.ASRStepTranscription, + Model: "schema-model", + Options: map[string]any{"sherpa.model_type": "nemo_transducer"}, + }}, + }, + }) + if err != nil { + t.Fatalf("Create with default read-only value returned error: %v", err) + } + + err = service.Create(context.Background(), &models.TranscriptionProfile{ + UserID: 1, + Name: "Changed read-only", + Parameters: models.ASRParams{ + Pipeline: []models.ASRStep{{ + Kind: models.ASRStepTranscription, + Model: "schema-model", + Options: map[string]any{"sherpa.model_type": "whisper"}, + }}, + }, + }) + if !errors.Is(err, ErrInvalidPipeline) { + t.Fatalf("Create error = %v, want ErrInvalidPipeline", err) + } } func floatPtr(v float64) *float64 { return &v } diff --git a/internal/transcription/asrcontract/types.go b/internal/transcription/asrcontract/types.go index 877785e8..1baea73f 100644 --- a/internal/transcription/asrcontract/types.go +++ b/internal/transcription/asrcontract/types.go @@ -301,6 +301,7 @@ type ParameterDescriptor struct { Scope ParameterScope `json:"scope"` Required bool `json:"required,omitempty"` Advanced bool `json:"advanced,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` RequiresReload bool `json:"requires_reload,omitempty"` ExposeInSummary bool `json:"expose_in_summary,omitempty"` VisibleWhen []ActivationRule `json:"visible_when,omitempty"` @@ -439,11 +440,40 @@ func ValidateParameterValues(schema ParameterSchema, values map[string]any) (map if err != nil { return nil, fmt.Errorf("parameter %q is invalid: %w", key, err) } + if parameter.ReadOnly && !readOnlyValueAllowed(parameter, normalized) { + return nil, fmt.Errorf("parameter %q is read-only", key) + } out[key] = normalized } return out, nil } +func readOnlyValueAllowed(parameter ParameterDescriptor, value any) bool { + if parameter.Default == nil { + return false + } + normalizedDefault, err := validateParameterValue(parameter, parameter.Default) + if err != nil { + return false + } + return parameterValuesEqual(normalizedDefault, value) +} + +func parameterValuesEqual(left any, right any) bool { + switch typed := left.(type) { + case string: + rightString, ok := right.(string) + return ok && typed == rightString + case bool: + rightBool, ok := right.(bool) + return ok && typed == rightBool + default: + leftNumber, leftOK := numericValue(left) + rightNumber, rightOK := numericValue(right) + return leftOK && rightOK && leftNumber == rightNumber + } +} + func validateParameterValue(parameter ParameterDescriptor, value any) (any, error) { switch parameter.Type { case ParameterTypeBoolean: diff --git a/internal/transcription/asrcontract/types_test.go b/internal/transcription/asrcontract/types_test.go index ca335e5f..3c46cf4d 100644 --- a/internal/transcription/asrcontract/types_test.go +++ b/internal/transcription/asrcontract/types_test.go @@ -99,6 +99,31 @@ func TestParameterSchemaValidationAndProfileValues(t *testing.T) { } } +func TestValidateParameterValuesRejectsChangedReadOnlyParameter(t *testing.T) { + schema := ParameterSchema{{ + Key: "sherpa.model_type", + Label: "Sherpa model type", + Type: ParameterTypeString, + Default: "nemo_transducer", + Scope: ParameterScopeModel, + ReadOnly: true, + }} + + if _, err := ValidateParameterValues(schema, nil); err != nil { + t.Fatalf("omitted read-only value should validate: %v", err) + } + values, err := ValidateParameterValues(schema, map[string]any{"sherpa.model_type": "nemo_transducer"}) + if err != nil { + t.Fatalf("default read-only value should validate: %v", err) + } + if values["sherpa.model_type"] != "nemo_transducer" { + t.Fatalf("read-only default was not preserved: %#v", values) + } + if _, err := ValidateParameterValues(schema, map[string]any{"sherpa.model_type": "whisper"}); err == nil { + t.Fatal("expected changed read-only value to fail") + } +} + func TestProviderErrorClassification(t *testing.T) { err := NewProviderError(CodeProviderBusy, "provider is busy", true) diff --git a/internal/transcription/engineprovider/local_provider.go b/internal/transcription/engineprovider/local_provider.go index 96381625..b6d971cf 100644 --- a/internal/transcription/engineprovider/local_provider.go +++ b/internal/transcription/engineprovider/local_provider.go @@ -491,6 +491,7 @@ func parameterSchemaFromDescriptor(parameters []speechproviders.ParameterDescrip Scope: asrcontract.ParameterScope(parameter.Scope), Required: parameter.Required, Advanced: parameter.Advanced, + ReadOnly: parameter.ReadOnly, RequiresReload: parameter.RequiresReload, ExposeInSummary: parameter.ExposeInSummary, VisibleWhen: activationRulesFromDescriptor(parameter.VisibleWhen), diff --git a/internal/transcription/engineprovider/local_provider_test.go b/internal/transcription/engineprovider/local_provider_test.go index 7dc4f582..70ea8cf9 100644 --- a/internal/transcription/engineprovider/local_provider_test.go +++ b/internal/transcription/engineprovider/local_provider_test.go @@ -511,6 +511,7 @@ func TestLocalProviderModelDescriptorsDistinguishWhisperAndParakeet(t *testing.T if hasParameter(parakeet.ParameterSchema, "sherpa.whisper.language") { t.Fatalf("parakeet descriptor should not expose whisper language parameter: %#v", parakeet.ParameterSchema) } + requireReadOnlyParameter(t, parakeet.ParameterSchema, "sherpa.model_type") requireReloadParameter(t, parakeet.ParameterSchema, "sherpa.model_type") requireReloadParameter(t, parakeet.ParameterSchema, "runtime.provider") requireReloadParameter(t, parakeet.ParameterSchema, asrcontract.CommonParameterRuntimeNumThreads) @@ -574,6 +575,10 @@ func TestLocalProviderModelDescriptorParameterSchemasValidate(t *testing.T) { if err == nil { t.Fatal("parakeet schema accepted whisper-specific parameter") } + _, err = asrcontract.ValidateParameterValues(parakeet.ParameterSchema, map[string]any{"sherpa.model_type": "whisper"}) + if err == nil { + t.Fatal("parakeet schema accepted changed read-only model type") + } } func TestLocalProviderSanitizesErrors(t *testing.T) { @@ -626,6 +631,19 @@ func requireReloadParameter(t *testing.T, schema asrcontract.ParameterSchema, ke t.Fatalf("parameter %q not found in %#v", key, schema) } +func requireReadOnlyParameter(t *testing.T, schema asrcontract.ParameterSchema, key string) { + t.Helper() + for _, parameter := range schema { + if parameter.Key == key { + if !parameter.ReadOnly { + t.Fatalf("parameter %q should be read-only: %#v", key, parameter) + } + return + } + } + t.Fatalf("parameter %q not found in %#v", key, schema) +} + func requireArtifactRequirement(t *testing.T, model asrcontract.ModelCard, requirement string) { t.Helper() for _, item := range model.Artifacts { diff --git a/web/frontend/src/features/settings/api/profilesApi.ts b/web/frontend/src/features/settings/api/profilesApi.ts index 4f83bfcc..a43f31f6 100644 --- a/web/frontend/src/features/settings/api/profilesApi.ts +++ b/web/frontend/src/features/settings/api/profilesApi.ts @@ -53,6 +53,7 @@ export type ParameterDescriptor = { scope: "model" | "runtime" | "decoding" | "chunking" | "vad" | "output" | "postprocess"; required?: boolean; advanced?: boolean; + read_only?: boolean; requires_reload?: boolean; expose_in_summary?: boolean; visible_when?: ActivationRule[];