mirror of
https://github.com/rishikanthc/Scriberr.git
synced 2026-06-29 07:15:54 +00:00
Enforce read-only ASR parameters
This commit is contained in:
@@ -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:
|
||||
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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[];
|
||||
|
||||
Reference in New Issue
Block a user