diff --git a/framework/configstore/clientconfig.go b/framework/configstore/clientconfig.go index c2c90508a4..6388c73aa6 100644 --- a/framework/configstore/clientconfig.go +++ b/framework/configstore/clientconfig.go @@ -1294,12 +1294,22 @@ func GenerateComplexityAnalyzerConfigHashes(config *ComplexityAnalyzerConfig) (C return ComplexityAnalyzerConfigHashes{}, fmt.Errorf("failed to hash complex keywords: %w", err) } - return ComplexityAnalyzerConfigHashes{ + hashes := ComplexityAnalyzerConfigHashes{ TierBoundaries: tierHash, SimpleKeywords: simpleHash, MediumKeywords: mediumHash, ComplexKeywords: complexHash, - }, nil + } + + if normalized.Semantic != nil { + settingsHash, err := hashComplexityValue(normalized.Semantic) + if err != nil { + return ComplexityAnalyzerConfigHashes{}, fmt.Errorf("failed to hash semantic settings: %w", err) + } + hashes.SemanticSettings = settingsHash + } + + return hashes, nil } // GenerateLegacyComplexityMediumKeywordsHash returns the Medium section hash diff --git a/framework/configstore/complexityconfig.go b/framework/configstore/complexityconfig.go index e9b0dfb7ca..c84ab53ed6 100644 --- a/framework/configstore/complexityconfig.go +++ b/framework/configstore/complexityconfig.go @@ -5,6 +5,9 @@ import ( "fmt" "sort" "strings" + "time" + + "github.com/maximhq/bifrost/core/schemas" ) // ComplexityTierBoundaries defines score thresholds for complexity tier classification. @@ -29,7 +32,9 @@ func (b *ComplexityTierBoundaries) Validate() error { return nil } -// ComplexityEditableKeywordConfig contains the user-editable keyword lists. +// ComplexityEditableKeywordConfig contains the user-editable per-tier lists. +// The same lists feed both classifiers: the lexical matcher treats entries as +// keywords, the semantic classifier embeds them as exemplars. type ComplexityEditableKeywordConfig struct { SimpleKeywords []string `json:"simple_keywords"` MediumKeywords []string `json:"medium_keywords"` @@ -102,6 +107,166 @@ func hasAnyComplexityField(fields map[string]json.RawMessage, names ...string) b return false } +// Fallback behaviors when semantic classification is unavailable (executor not +// wired, warmup incomplete) or exceeds its timeout. +const ( + ComplexitySemanticFallbackLexical = "lexical" + ComplexitySemanticFallbackNone = "none" +) + +// Vector store selection modes for exemplar embeddings. "embedded" (the +// default) uses the built-in chromem store; "auto" opts into the configured +// external store when present, falling back to embedded otherwise; +// "external" makes a missing external store a startup error. +const ( + ComplexitySemanticVectorStoreAuto = "auto" + ComplexitySemanticVectorStoreEmbedded = "embedded" + ComplexitySemanticVectorStoreExternal = "external" +) + +// DefaultComplexitySemanticTimeout bounds per-request embedding generation. +const DefaultComplexitySemanticTimeout = 100 * time.Millisecond + +// ComplexitySemanticConfig configures the embedding-based complexity +// classifier. A non-nil value enables semantic classification. The classifier +// embeds the analyzer's shared per-tier keyword lists as its exemplars; there +// is no separate exemplar storage. +type ComplexitySemanticConfig struct { + Provider schemas.ModelProvider `json:"provider"` + EmbeddingModel string `json:"embedding_model"` + Dimension int `json:"dimension"` + Timeout time.Duration `json:"timeout,omitempty"` + Fallback string `json:"fallback,omitempty"` + CountTowardBudgets bool `json:"count_toward_budgets,omitempty"` + VectorStore string `json:"vector_store,omitempty"` +} + +// UnmarshalJSON accepts Timeout as a duration string ("100ms") or a JSON number +// (milliseconds). It rejects unknown fields so unshipped semantic-only settings +// cannot be silently accepted through config.json or the management API. +func (c *ComplexitySemanticConfig) UnmarshalJSON(data []byte) error { + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return err + } + allowed := map[string]struct{}{ + "provider": {}, + "embedding_model": {}, + "dimension": {}, + "timeout": {}, + "fallback": {}, + "count_toward_budgets": {}, + "vector_store": {}, + } + for field := range fields { + if _, ok := allowed[field]; !ok { + return fmt.Errorf("unknown semantic complexity field %q", field) + } + } + + // alias suppresses ComplexitySemanticConfig's UnmarshalJSON to avoid + // infinite recursion. The outer Timeout (json.RawMessage) shadows + // alias.Timeout because the json package picks the shallower field. + type alias ComplexitySemanticConfig + aux := &struct { + Timeout json.RawMessage `json:"timeout,omitempty"` + *alias + }{alias: (*alias)(c)} + if err := json.Unmarshal(data, aux); err != nil { + return err + } + + if len(aux.Timeout) == 0 || string(aux.Timeout) == "null" { + return nil + } + + var s string + if err := json.Unmarshal(aux.Timeout, &s); err == nil { + d, err := time.ParseDuration(s) + if err != nil { + return fmt.Errorf("failed to parse semantic timeout duration string %q: %w", s, err) + } + c.Timeout = d + } else { + var ms float64 + if err := json.Unmarshal(aux.Timeout, &ms); err != nil { + return fmt.Errorf("unsupported semantic timeout value: %s", string(aux.Timeout)) + } + c.Timeout = time.Duration(ms * float64(time.Millisecond)) + } + if c.Timeout < 0 { + return fmt.Errorf("semantic timeout must be non-negative, got %v", c.Timeout) + } + return nil +} + +// MarshalJSON writes Timeout as a duration string so persisted configs decode +// back to the same value (the default int encoding is nanoseconds, which the +// millisecond-number decode path would misread). +func (c ComplexitySemanticConfig) MarshalJSON() ([]byte, error) { + type alias ComplexitySemanticConfig + var timeout string + if c.Timeout != 0 { + timeout = c.Timeout.String() + } + return json.Marshal(struct { + Timeout string `json:"timeout,omitempty"` + alias + }{ + Timeout: timeout, + alias: alias(c), + }) +} + +// normalized returns a canonical deep copy with defaults applied. +func (c *ComplexitySemanticConfig) normalized() *ComplexitySemanticConfig { + if c == nil { + return nil + } + out := &ComplexitySemanticConfig{ + Provider: schemas.ModelProvider(strings.ToLower(strings.TrimSpace(string(c.Provider)))), + EmbeddingModel: strings.TrimSpace(c.EmbeddingModel), + Dimension: c.Dimension, + Timeout: c.Timeout, + Fallback: strings.ToLower(strings.TrimSpace(c.Fallback)), + CountTowardBudgets: c.CountTowardBudgets, + VectorStore: strings.ToLower(strings.TrimSpace(c.VectorStore)), + } + if out.Timeout == 0 { + out.Timeout = DefaultComplexitySemanticTimeout + } + if out.Fallback == "" { + out.Fallback = ComplexitySemanticFallbackLexical + } + if out.VectorStore == "" { + out.VectorStore = ComplexitySemanticVectorStoreEmbedded + } + return out +} + +// Validate checks a normalized semantic config. +func (c *ComplexitySemanticConfig) Validate() error { + if c == nil { + return nil + } + if strings.TrimSpace(string(c.Provider)) == "" { + return fmt.Errorf("semantic config requires a provider") + } + if strings.TrimSpace(c.EmbeddingModel) == "" { + return fmt.Errorf("semantic config requires an embedding_model") + } + if c.Timeout <= 0 { + return fmt.Errorf("semantic timeout must be positive, got %v", c.Timeout) + } + switch c.VectorStore { + case ComplexitySemanticVectorStoreAuto, ComplexitySemanticVectorStoreEmbedded, ComplexitySemanticVectorStoreExternal: + default: + return fmt.Errorf("semantic vector_store must be %q, %q, or %q, got %q", + ComplexitySemanticVectorStoreAuto, ComplexitySemanticVectorStoreEmbedded, ComplexitySemanticVectorStoreExternal, c.VectorStore) + } + return nil +} + // ComplexityAnalyzerConfigHashes tracks the config.json hash for each editable // analyzer section. It is persisted with the config row, but not exposed through // API responses or config.json. @@ -110,6 +275,10 @@ type ComplexityAnalyzerConfigHashes struct { SimpleKeywords string `json:"simple_keywords,omitempty"` MediumKeywords string `json:"medium_keywords,omitempty"` ComplexKeywords string `json:"complex_keywords,omitempty"` + // SemanticSettings covers the semantic block (provider, model, timeout, + // budgets flag, vector store). The semantic classifier's + // exemplars are the shared keyword lists, tracked by the sections above. + SemanticSettings string `json:"semantic_settings,omitempty"` } type legacyComplexityAnalyzerConfigHashes struct { @@ -162,31 +331,33 @@ func (h *ComplexityAnalyzerConfigHashes) UnmarshalJSON(data []byte) error { // Empty reports whether no file-backed section hashes are present. func (h ComplexityAnalyzerConfigHashes) Empty() bool { - return h.TierBoundaries == "" && - h.SimpleKeywords == "" && - h.MediumKeywords == "" && - h.ComplexKeywords == "" + return h == ComplexityAnalyzerConfigHashes{} } // Equal reports whether all section hashes match. func (h ComplexityAnalyzerConfigHashes) Equal(other ComplexityAnalyzerConfigHashes) bool { - return h.TierBoundaries == other.TierBoundaries && - h.SimpleKeywords == other.SimpleKeywords && - h.MediumKeywords == other.MediumKeywords && - h.ComplexKeywords == other.ComplexKeywords + return h == other } // ComplexityAnalyzerConfig is the persisted runtime configuration for the complexity analyzer. type ComplexityAnalyzerConfig struct { TierBoundaries ComplexityTierBoundaries `json:"tier_boundaries"` Keywords ComplexityEditableKeywordConfig `json:"keywords"` + Semantic *ComplexitySemanticConfig `json:"semantic,omitempty"` ConfigHashes ComplexityAnalyzerConfigHashes `json:"-"` + // EmbeddingFingerprint records the (model, dimension, keyword lists) the + // stored exemplar embeddings were computed from. Warmup compares it against + // the current config to decide whether to re-embed. Persisted with the + // config row, not exposed through API responses or config.json. + EmbeddingFingerprint string `json:"-"` } type complexityAnalyzerConfigRecord struct { - TierBoundaries ComplexityTierBoundaries `json:"tier_boundaries"` - Keywords ComplexityEditableKeywordConfig `json:"keywords"` - ConfigHashes ComplexityAnalyzerConfigHashes `json:"_config_hashes,omitempty"` + TierBoundaries ComplexityTierBoundaries `json:"tier_boundaries"` + Keywords ComplexityEditableKeywordConfig `json:"keywords"` + Semantic *ComplexitySemanticConfig `json:"semantic,omitempty"` + ConfigHashes ComplexityAnalyzerConfigHashes `json:"_config_hashes,omitempty"` + EmbeddingFingerprint string `json:"_embedding_fingerprint,omitempty"` } // Validate checks that the config is internally consistent. @@ -211,6 +382,9 @@ func (c *ComplexityAnalyzerConfig) Validate() error { if len(missing) > 0 { return fmt.Errorf("keyword lists must be non-empty: %s", strings.Join(missing, ", ")) } + if err := c.Semantic.Validate(); err != nil { + return err + } return nil } @@ -226,7 +400,9 @@ func (c *ComplexityAnalyzerConfig) Normalized() ComplexityAnalyzerConfig { MediumKeywords: normalizeComplexityKeywordList(c.Keywords.MediumKeywords), ComplexKeywords: normalizeComplexityKeywordList(c.Keywords.ComplexKeywords), }, - ConfigHashes: c.ConfigHashes, + Semantic: c.Semantic.normalized(), + ConfigHashes: c.ConfigHashes, + EmbeddingFingerprint: c.EmbeddingFingerprint, } } @@ -263,7 +439,9 @@ func MergeComplexityAnalyzerConfig(base, file *ComplexityAnalyzerConfig) (*Compl MediumKeywords: mergeComplexityKeywordLists(normalizedBase.Keywords.MediumKeywords, normalizedFile.Keywords.MediumKeywords), ComplexKeywords: mergeComplexityKeywordLists(normalizedBase.Keywords.ComplexKeywords, normalizedFile.Keywords.ComplexKeywords), }, - ConfigHashes: normalizedFile.ConfigHashes, + Semantic: mergeComplexitySemanticConfig(normalizedBase.Semantic, normalizedFile.Semantic), + ConfigHashes: normalizedFile.ConfigHashes, + EmbeddingFingerprint: normalizedBase.EmbeddingFingerprint, } if err := merged.Validate(); err != nil { return nil, err @@ -271,6 +449,15 @@ func MergeComplexityAnalyzerConfig(base, file *ComplexityAnalyzerConfig) (*Compl return &merged, nil } +// mergeComplexitySemanticConfig overlays the file semantic settings. A nil +// file section keeps the base untouched. +func mergeComplexitySemanticConfig(base, file *ComplexitySemanticConfig) *ComplexitySemanticConfig { + if file == nil { + return base.normalized() + } + return file.normalized() +} + // MergeComplexityAnalyzerConfigByHashes overlays only file-backed sections whose // config.json hash changed. Keyword sections are additive; tier boundaries replace. func MergeComplexityAnalyzerConfigByHashes(base, file *ComplexityAnalyzerConfig) (*ComplexityAnalyzerConfig, error) { @@ -307,6 +494,15 @@ func MergeComplexityAnalyzerConfigByHashes(base, file *ComplexityAnalyzerConfig) merged.Keywords.ComplexKeywords = mergeComplexityKeywordLists(merged.Keywords.ComplexKeywords, normalizedFile.Keywords.ComplexKeywords) merged.ConfigHashes.ComplexKeywords = normalizedFile.ConfigHashes.ComplexKeywords } + // A config.json without a semantic section leaves DB semantic state (and its + // section hash) untouched: the section is optional, so absence means "no + // opinion", not removal. + if normalizedFile.Semantic != nil { + if merged.Semantic == nil || merged.ConfigHashes.SemanticSettings != normalizedFile.ConfigHashes.SemanticSettings { + merged.Semantic = normalizedFile.Semantic.normalized() + merged.ConfigHashes.SemanticSettings = normalizedFile.ConfigHashes.SemanticSettings + } + } if err := merged.Validate(); err != nil { return nil, err } @@ -325,9 +521,11 @@ func DecodeComplexityAnalyzerConfig(data []byte) (*ComplexityAnalyzerConfig, err } cfg := ComplexityAnalyzerConfig{ - TierBoundaries: record.TierBoundaries, - Keywords: record.Keywords, - ConfigHashes: record.ConfigHashes, + TierBoundaries: record.TierBoundaries, + Keywords: record.Keywords, + Semantic: record.Semantic, + ConfigHashes: record.ConfigHashes, + EmbeddingFingerprint: record.EmbeddingFingerprint, } normalized := cfg.Normalized() if err := normalized.Validate(); err != nil { @@ -338,9 +536,11 @@ func DecodeComplexityAnalyzerConfig(data []byte) (*ComplexityAnalyzerConfig, err func encodeComplexityAnalyzerConfig(config ComplexityAnalyzerConfig) ([]byte, error) { record := complexityAnalyzerConfigRecord{ - TierBoundaries: config.TierBoundaries, - Keywords: config.Keywords, - ConfigHashes: config.ConfigHashes, + TierBoundaries: config.TierBoundaries, + Keywords: config.Keywords, + Semantic: config.Semantic, + ConfigHashes: config.ConfigHashes, + EmbeddingFingerprint: config.EmbeddingFingerprint, } data, err := json.Marshal(record) if err != nil { diff --git a/framework/configstore/complexityconfig_test.go b/framework/configstore/complexityconfig_test.go new file mode 100644 index 0000000000..0d95b1f2c0 --- /dev/null +++ b/framework/configstore/complexityconfig_test.go @@ -0,0 +1,317 @@ +package configstore + +import ( + "context" + "encoding/json" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" +) + +func testSemanticConfig() *ComplexitySemanticConfig { + return &ComplexitySemanticConfig{ + Provider: "openai", + EmbeddingModel: "text-embedding-3-small", + } +} + +func testSemanticAnalyzerConfig() *ComplexityAnalyzerConfig { + cfg := testComplexityAnalyzerConfig() + cfg.Semantic = testSemanticConfig() + return cfg +} + +func TestComplexitySemanticConfigTimeoutDecoding(t *testing.T) { + tests := []struct { + name string + payload string + want time.Duration + wantErr bool + }{ + {name: "duration string", payload: `{"timeout":"250ms"}`, want: 250 * time.Millisecond}, + {name: "number is milliseconds", payload: `{"timeout":250}`, want: 250 * time.Millisecond}, + {name: "absent keeps zero", payload: `{}`, want: 0}, + {name: "null keeps zero", payload: `{"timeout":null}`, want: 0}, + {name: "negative number rejected", payload: `{"timeout":-5}`, wantErr: true}, + {name: "bad string rejected", payload: `{"timeout":"soon"}`, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var cfg ComplexitySemanticConfig + err := json.Unmarshal([]byte(tt.payload), &cfg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, cfg.Timeout) + }) + } +} + +func TestComplexitySemanticConfigTimeoutMarshalRoundTrip(t *testing.T) { + cfg := testSemanticConfig() + cfg.Timeout = 250 * time.Millisecond + + data, err := json.Marshal(cfg) + require.NoError(t, err) + assert.Contains(t, string(data), `"timeout":"250ms"`) + + var decoded ComplexitySemanticConfig + require.NoError(t, json.Unmarshal(data, &decoded)) + assert.Equal(t, cfg.Timeout, decoded.Timeout) +} + +func TestComplexitySemanticConfigNormalizedDefaults(t *testing.T) { + normalized := testSemanticConfig().normalized() + + assert.Equal(t, DefaultComplexitySemanticTimeout, normalized.Timeout) + assert.Equal(t, ComplexitySemanticFallbackLexical, normalized.Fallback) + assert.Equal(t, ComplexitySemanticVectorStoreEmbedded, normalized.VectorStore) + require.NoError(t, normalized.Validate()) +} + +func TestComplexitySemanticConfigValidation(t *testing.T) { + tests := []struct { + name string + mutate func(*ComplexitySemanticConfig) + }{ + {name: "missing provider", mutate: func(c *ComplexitySemanticConfig) { c.Provider = "" }}, + {name: "missing embedding model", mutate: func(c *ComplexitySemanticConfig) { c.EmbeddingModel = " " }}, + {name: "unknown vector store", mutate: func(c *ComplexitySemanticConfig) { c.VectorStore = "pgvector" }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := testSemanticConfig() + tt.mutate(cfg) + require.Error(t, cfg.normalized().Validate()) + }) + } +} + +func TestDecodeComplexityAnalyzerConfigSemanticRoundTrip(t *testing.T) { + cfg := testSemanticAnalyzerConfig() + cfg.ConfigHashes = ComplexityAnalyzerConfigHashes{ + TierBoundaries: "tier-hash", + SimpleKeywords: "simple-hash", + MediumKeywords: "medium-hash", + ComplexKeywords: "complex-hash", + SemanticSettings: "settings-hash", + } + cfg.EmbeddingFingerprint = "fingerprint-1" + + raw, err := encodeComplexityAnalyzerConfig(cfg.Normalized()) + require.NoError(t, err) + assert.Contains(t, string(raw), `"_embedding_fingerprint":"fingerprint-1"`) + + decoded, err := DecodeComplexityAnalyzerConfig(raw) + require.NoError(t, err) + require.NotNil(t, decoded.Semantic) + assert.Equal(t, cfg.Normalized().Semantic, decoded.Semantic) + assert.Equal(t, cfg.ConfigHashes, decoded.ConfigHashes) + assert.Equal(t, "fingerprint-1", decoded.EmbeddingFingerprint) +} + +func TestDecodeComplexityAnalyzerConfigWithoutSemantic(t *testing.T) { + raw, err := encodeComplexityAnalyzerConfig(testComplexityAnalyzerConfig().Normalized()) + require.NoError(t, err) + + decoded, err := DecodeComplexityAnalyzerConfig(raw) + require.NoError(t, err) + assert.Nil(t, decoded.Semantic) + assert.Empty(t, decoded.EmbeddingFingerprint) +} + +func TestGenerateComplexityAnalyzerConfigHashesSemantic(t *testing.T) { + base := testSemanticAnalyzerConfig() + baseHashes, err := GenerateComplexityAnalyzerConfigHashes(base) + require.NoError(t, err) + require.NotEmpty(t, baseHashes.SemanticSettings) + + // Keyword edits must not move the semantic settings hash: the shared lists + // are tracked by the keyword section hashes. + keywordEdit := testSemanticAnalyzerConfig() + keywordEdit.Keywords.SimpleKeywords = append(keywordEdit.Keywords.SimpleKeywords, "weather") + keywordHashes, err := GenerateComplexityAnalyzerConfigHashes(keywordEdit) + require.NoError(t, err) + assert.Equal(t, baseHashes.SemanticSettings, keywordHashes.SemanticSettings) + assert.NotEqual(t, baseHashes.SimpleKeywords, keywordHashes.SimpleKeywords) + + // Semantic scalar edits must not move the keyword hashes. + scalarEdit := testSemanticAnalyzerConfig() + scalarEdit.Semantic.EmbeddingModel = "text-embedding-3-large" + scalarHashes, err := GenerateComplexityAnalyzerConfigHashes(scalarEdit) + require.NoError(t, err) + assert.NotEqual(t, baseHashes.SemanticSettings, scalarHashes.SemanticSettings) + assert.Equal(t, baseHashes.SimpleKeywords, scalarHashes.SimpleKeywords) + + // No semantic section means no semantic hash. + plainHashes, err := GenerateComplexityAnalyzerConfigHashes(testComplexityAnalyzerConfig()) + require.NoError(t, err) + assert.Empty(t, plainHashes.SemanticSettings) +} + +func TestMergeComplexityAnalyzerConfigByHashesSemantic(t *testing.T) { + fileConfig := func() *ComplexityAnalyzerConfig { + cfg := testSemanticAnalyzerConfig() + hashes, err := GenerateComplexityAnalyzerConfigHashes(cfg) + require.NoError(t, err) + cfg.ConfigHashes = hashes + return cfg + } + + t.Run("file adds semantic to base without one", func(t *testing.T) { + base := testComplexityAnalyzerConfig() + file := fileConfig() + + merged, err := MergeComplexityAnalyzerConfigByHashes(base, file) + require.NoError(t, err) + require.NotNil(t, merged.Semantic) + assert.Equal(t, file.Normalized().Semantic, merged.Semantic) + assert.Equal(t, file.ConfigHashes.SemanticSettings, merged.ConfigHashes.SemanticSettings) + }) + + t.Run("unchanged hash preserves DB edits", func(t *testing.T) { + file := fileConfig() + base := fileConfig() + // Simulate a UI edit persisted after the last file sync. + base.Semantic.EmbeddingModel = "runtime-model" + + merged, err := MergeComplexityAnalyzerConfigByHashes(base, file) + require.NoError(t, err) + assert.Equal(t, "runtime-model", merged.Semantic.EmbeddingModel) + }) + + t.Run("settings change replaces the semantic block", func(t *testing.T) { + base := fileConfig() + + file := fileConfig() + file.Semantic.EmbeddingModel = "text-embedding-3-large" + fileHashes, err := GenerateComplexityAnalyzerConfigHashes(file) + require.NoError(t, err) + file.ConfigHashes = fileHashes + + merged, err := MergeComplexityAnalyzerConfigByHashes(base, file) + require.NoError(t, err) + assert.Equal(t, "text-embedding-3-large", merged.Semantic.EmbeddingModel) + assert.Equal(t, ComplexitySemanticFallbackLexical, merged.Semantic.Fallback) + assert.Equal(t, fileHashes.SemanticSettings, merged.ConfigHashes.SemanticSettings) + }) + + t.Run("file without semantic preserves DB semantic", func(t *testing.T) { + base := fileConfig() + base.EmbeddingFingerprint = "fingerprint-1" + + file := testComplexityAnalyzerConfig() + fileHashes, err := GenerateComplexityAnalyzerConfigHashes(file) + require.NoError(t, err) + file.ConfigHashes = fileHashes + + merged, err := MergeComplexityAnalyzerConfigByHashes(base, file) + require.NoError(t, err) + require.NotNil(t, merged.Semantic) + assert.Equal(t, base.Normalized().Semantic, merged.Semantic) + assert.Equal(t, base.ConfigHashes.SemanticSettings, merged.ConfigHashes.SemanticSettings) + assert.Equal(t, "fingerprint-1", merged.EmbeddingFingerprint) + }) +} + +func TestRDBConfigStore_ComplexityAnalyzerConfigSemanticPersistence(t *testing.T) { + store := setupRDBTestStore(t) + ctx := context.Background() + + cfg := testSemanticAnalyzerConfig() + cfg.EmbeddingFingerprint = "fingerprint-1" + require.NoError(t, store.UpdateComplexityAnalyzerConfig(ctx, cfg)) + + got, err := store.GetComplexityAnalyzerConfig(ctx) + require.NoError(t, err) + require.NotNil(t, got.Semantic) + assert.Equal(t, cfg.Normalized().Semantic, got.Semantic) + assert.Equal(t, "fingerprint-1", got.EmbeddingFingerprint) + + // A UI-style write without a fingerprint must not wipe the stored one. + update := testSemanticAnalyzerConfig() + update.Semantic.EmbeddingModel = "text-embedding-3-large" + require.NoError(t, store.UpdateComplexityAnalyzerConfig(ctx, update)) + + got, err = store.GetComplexityAnalyzerConfig(ctx) + require.NoError(t, err) + assert.Equal(t, "text-embedding-3-large", got.Semantic.EmbeddingModel) + assert.Equal(t, "fingerprint-1", got.EmbeddingFingerprint) +} + +// A writer that carries ConfigHashes/EmbeddingFingerprint over from the stored row must not +// clobber a concurrent writer that is setting fresh ones. The carry-over read and the save +// have to be one atomic unit; if they are not, the carrying writer can read the pre-update +// values, sleep through the other writer's save, and then persist the stale copy. +func TestRDBConfigStore_UpdateComplexityAnalyzerConfigConcurrentCarryOver(t *testing.T) { + store := setupRDBTestStore(t) + ctx := context.Background() + + // :memory: SQLite gives every pooled connection its own database, so pin the pool to one + // connection. Transactions still hold it for their whole span, which is what serializes + // the two writers below. + sqlDB, err := store.DB().DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(1) + + seed := testSemanticAnalyzerConfig() + seed.EmbeddingFingerprint = "fingerprint-old" + seedHashes, err := GenerateComplexityAnalyzerConfigHashes(seed) + require.NoError(t, err) + seed.ConfigHashes = seedHashes + require.NoError(t, store.UpdateComplexityAnalyzerConfig(ctx, seed)) + + // Widen the window between the carry-over read and the save so an unserialized update + // would reliably lose the race. Only armed for the concurrent phase below. + var armed atomic.Bool + require.NoError(t, store.DB().Callback().Query().After("gorm:query"). + Register("test:delay_governance_config_read", func(db *gorm.DB) { + if armed.Load() && db.Statement.Table == "governance_config" { + time.Sleep(50 * time.Millisecond) + } + })) + t.Cleanup(func() { + _ = store.DB().Callback().Query().Remove("test:delay_governance_config_read") + }) + + // Writer A supplies both fields, so it never reads. + writerA := testSemanticAnalyzerConfig() + writerA.Semantic.EmbeddingModel = "text-embedding-3-large" + writerA.EmbeddingFingerprint = "fingerprint-new" + hashesA, err := GenerateComplexityAnalyzerConfigHashes(writerA) + require.NoError(t, err) + writerA.ConfigHashes = hashesA + + // Writer B is a UI-style update: it omits both fields and carries them over. + writerB := testSemanticAnalyzerConfig() + writerB.Keywords.SimpleKeywords = []string{"hi"} + + armed.Store(true) + var wg sync.WaitGroup + errs := make([]error, 2) + for i, cfg := range []*ComplexityAnalyzerConfig{writerA, writerB} { + wg.Add(1) + go func() { + defer wg.Done() + errs[i] = store.UpdateComplexityAnalyzerConfig(ctx, cfg) + }() + } + wg.Wait() + armed.Store(false) + require.NoError(t, errs[0]) + require.NoError(t, errs[1]) + + // Whichever order the two writers land in, writer A's values must survive: it either + // wrote last, or writer B read them under the same lock and carried them forward. + got, err := store.GetComplexityAnalyzerConfig(ctx) + require.NoError(t, err) + assert.Equal(t, "fingerprint-new", got.EmbeddingFingerprint) + assert.Equal(t, hashesA, got.ConfigHashes) +} diff --git a/framework/configstore/rdb.go b/framework/configstore/rdb.go index 3f878848ac..285d4ca666 100644 --- a/framework/configstore/rdb.go +++ b/framework/configstore/rdb.go @@ -6055,29 +6055,45 @@ func (s *RDBConfigStore) UpdateComplexityAnalyzerConfig(ctx context.Context, con return err } - txDB := s.DB() if len(tx) > 0 && tx[0] != nil { - txDB = tx[0] + return s.updateComplexityAnalyzerConfigWithTx(ctx, &normalized, tx[0]) } + // Standalone calls own the transaction so the carry-over read below and the save + // that follows it cannot interleave with a concurrent update. + return s.DB().WithContext(ctx).Transaction(func(txDB *gorm.DB) error { + return s.updateComplexityAnalyzerConfigWithTx(ctx, &normalized, txDB) + }) +} - if normalized.ConfigHashes.Empty() { - existing, err := s.getComplexityAnalyzerConfigWithDB(ctx, txDB) +// updateComplexityAnalyzerConfigWithTx carries over ConfigHashes and EmbeddingFingerprint +// from the stored config when the incoming one omits them, then persists the result. The +// existing row is read FOR UPDATE (a plain read on SQLite, whose writer serialization +// already prevents the interleave) so a concurrent updater cannot save a stale copy of +// either field between this read and the save. txDB must be a transaction. +func (s *RDBConfigStore) updateComplexityAnalyzerConfigWithTx(ctx context.Context, normalized *ComplexityAnalyzerConfig, txDB *gorm.DB) error { + if normalized.ConfigHashes.Empty() || normalized.EmbeddingFingerprint == "" { + existing, err := s.getComplexityAnalyzerConfigWithDB(ctx, dbForUpdate(txDB)) if err != nil { return err } if existing != nil { - normalized.ConfigHashes = existing.ConfigHashes + if normalized.ConfigHashes.Empty() { + normalized.ConfigHashes = existing.ConfigHashes + } + if normalized.EmbeddingFingerprint == "" { + normalized.EmbeddingFingerprint = existing.EmbeddingFingerprint + } } } - raw, err := encodeComplexityAnalyzerConfig(normalized) + raw, err := encodeComplexityAnalyzerConfig(*normalized) if err != nil { return err } return s.UpdateConfig(ctx, &tables.TableGovernanceConfig{ Key: tables.ConfigComplexityAnalyzerConfigKey, Value: string(raw), - }, tx...) + }, txDB) } // GetAuthConfig retrieves the auth configuration from the database. diff --git a/plugins/governance/complexity/config.go b/plugins/governance/complexity/config.go index fe45e1532b..9487048ddf 100644 --- a/plugins/governance/complexity/config.go +++ b/plugins/governance/complexity/config.go @@ -48,6 +48,10 @@ type TierBoundaries = configstore.ComplexityTierBoundaries // EditableKeywordConfig is the user-facing subset of analyzer keyword lists. type EditableKeywordConfig = configstore.ComplexityEditableKeywordConfig +// SemanticConfig is the embedding-based classifier configuration. Its +// exemplars are the shared per-tier keyword lists in EditableKeywordConfig. +type SemanticConfig = configstore.ComplexitySemanticConfig + // AnalyzerConfig is the runtime configuration for the complexity analyzer. type AnalyzerConfig = configstore.ComplexityAnalyzerConfig diff --git a/transports/bifrost-http/lib/config.go b/transports/bifrost-http/lib/config.go index 1dd4233170..efc7ef8621 100644 --- a/transports/bifrost-http/lib/config.go +++ b/transports/bifrost-http/lib/config.go @@ -1015,6 +1015,11 @@ func LoadConfig(ctx context.Context, configDirPath string) (*Config, error) { loadWebhooksConfig(ctx, config, &configData) // 8. Governance config loadGovernanceConfig(ctx, config, &configData) + // 8a. Semantic complexity classification demands an external vector store + // only when one is actually configured (vector store init ran in step 2). + if err := validateComplexitySemanticVectorStore(config, &configData); err != nil { + return nil, err + } // 9. Auth config loadAuthConfig(ctx, config, &configData) // 10. Plugins @@ -3429,6 +3434,57 @@ func complexityAnalyzerConfigFromFile(configData *ConfigData) (*configstore.Comp return fileConfig, fileHashes, true } +// validateComplexitySemanticVectorStore fails startup when config.json requires +// an external vector store for semantic complexity classification but none is +// configured, instead of silently classifying with the fallback forever. +// +// The same setting reached from the database only warns. It is editable through +// the governance API, so failing boot on it would strand the operator with no +// running server to reach the UI that clears it — and the vector store itself is +// built from config.json alone, so nothing in the database can satisfy the +// requirement either. Booting degraded is safe: the semantic classifier fails to +// resolve a store, reports "failed" through the analyzer status endpoint, and +// every request resolves through the configured fallback. +func validateComplexitySemanticVectorStore(config *Config, configData *ConfigData) error { + if config == nil || config.GovernanceConfig == nil || config.GovernanceConfig.ComplexityAnalyzerConfig == nil { + return nil + } + semantic := config.GovernanceConfig.ComplexityAnalyzerConfig.Semantic + if semantic == nil || semantic.VectorStore != configstore.ComplexitySemanticVectorStoreExternal { + return nil + } + if config.VectorStore != nil { + return nil + } + if !fileRequestsExternalComplexityVectorStore(configData) { + logger.Error("stored governance complexity analyzer config sets semantic.vector_store to %q but no vector store is configured; semantic complexity classification will report \"failed\" and fall back to %q. Add a vector_store section to config.json, or set semantic.vector_store to %q or %q", + configstore.ComplexitySemanticVectorStoreExternal, + semantic.Fallback, + configstore.ComplexitySemanticVectorStoreAuto, + configstore.ComplexitySemanticVectorStoreEmbedded) + return nil + } + return fmt.Errorf("governance.complexity_analyzer_config.semantic.vector_store is %q but no vector store is configured; add a vector_store section to config.json or set it to %q or %q", + configstore.ComplexitySemanticVectorStoreExternal, + configstore.ComplexitySemanticVectorStoreAuto, + configstore.ComplexitySemanticVectorStoreEmbedded) +} + +// fileRequestsExternalComplexityVectorStore reports whether config.json itself +// asks for the external store. It reads the raw file value rather than the +// merged result so the source stays unambiguous even when the file's semantic +// section failed validation and was dropped in favor of the stored one. +func fileRequestsExternalComplexityVectorStore(configData *ConfigData) bool { + if configData == nil || configData.Governance == nil || configData.Governance.ComplexityAnalyzerConfig == nil { + return false + } + semantic := configData.Governance.ComplexityAnalyzerConfig.Semantic + if semantic == nil { + return false + } + return strings.ToLower(strings.TrimSpace(semantic.VectorStore)) == configstore.ComplexitySemanticVectorStoreExternal +} + // mergeComplexityAnalyzerConfigFromFile uses defaults as the first split-mode // base so config.json seeds do not erase built-in keyword coverage. func mergeComplexityAnalyzerConfigFromFile(current, fileConfig *configstore.ComplexityAnalyzerConfig) (*configstore.ComplexityAnalyzerConfig, error) { diff --git a/transports/bifrost-http/lib/config_test.go b/transports/bifrost-http/lib/config_test.go index 96c03f762a..4d83c52970 100644 --- a/transports/bifrost-http/lib/config_test.go +++ b/transports/bifrost-http/lib/config_test.go @@ -21160,3 +21160,152 @@ func TestResolveSetupToken_TrimsSurroundingWhitespace(t *testing.T) { configData := &ConfigData{SetupToken: schemas.NewSecretVar(" my-token ")} assert.Equal(t, "my-token", resolveSetupToken(configData)) } + +// ============================================================================= +// SEMANTIC COMPLEXITY VECTOR STORE VALIDATION +// ============================================================================= +// +// | Test Name | What It Tests | +// |------------------------------------------------------------------|----------------------------------------------------| +// | TestValidateComplexitySemanticVectorStore_FileExternalFailsBoot | config.json "external" + no store → startup error | +// | TestValidateComplexitySemanticVectorStore_FileExternalIsTrimmed | " External " in config.json still fails boot | +// | TestValidateComplexitySemanticVectorStore_StoredExternalWarnsOnly | DB-only "external" boots, logs, stays recoverable | +// | TestValidateComplexitySemanticVectorStore_ConfiguredStorePasses | "external" + configured store → no error, no log | +// | TestValidateComplexitySemanticVectorStore_NonExternalModesPass | embedded/auto never block boot | + +// recordingLogger captures Error lines so the degraded-boot path can be asserted +// to be loud rather than silently permissive. +type recordingLogger struct { + testLogger + errors []string +} + +func (l *recordingLogger) Error(msg string, args ...any) { + l.errors = append(l.errors, fmt.Sprintf(msg, args...)) +} + +// complexityConfigWithVectorStore builds a valid semantic block pinned to the +// given vector store mode; the other fields are irrelevant to this validation +// but are set so the config would survive normalization. +func complexityConfigWithVectorStore(mode string) *configstore.ComplexityAnalyzerConfig { + return &configstore.ComplexityAnalyzerConfig{ + Semantic: &configstore.ComplexitySemanticConfig{ + Provider: schemas.OpenAI, + EmbeddingModel: "text-embedding-3-small", + Dimension: 1536, + Fallback: configstore.ComplexitySemanticFallbackLexical, + VectorStore: mode, + }, + } +} + +// newTestChromemStore returns a real in-process chromem store, matching what the +// semantic classifier creates for its embedded mode. +func newTestChromemStore(t *testing.T) vectorstore.VectorStore { + t.Helper() + store, err := vectorstore.NewVectorStore(context.Background(), &vectorstore.Config{ + Enabled: true, + Type: vectorstore.VectorStoreTypeChromem, + Config: vectorstore.ChromemConfig{}, + }, &testLogger{}) + if err != nil { + t.Fatalf("failed to create chromem vector store: %v", err) + } + t.Cleanup(func() { store.Close(context.Background(), "") }) + return store +} + +func TestValidateComplexitySemanticVectorStore_FileExternalFailsBoot(t *testing.T) { + initTestLogger() + config := &Config{ + GovernanceConfig: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(configstore.ComplexitySemanticVectorStoreExternal), + }, + } + configData := &ConfigData{ + Governance: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(configstore.ComplexitySemanticVectorStoreExternal), + }, + } + + err := validateComplexitySemanticVectorStore(config, configData) + assert.Error(t, err, "config.json asking for an external store that does not exist must fail boot") + assert.Contains(t, err.Error(), "no vector store is configured") +} + +func TestValidateComplexitySemanticVectorStore_FileExternalIsTrimmed(t *testing.T) { + initTestLogger() + config := &Config{ + GovernanceConfig: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(configstore.ComplexitySemanticVectorStoreExternal), + }, + } + configData := &ConfigData{ + Governance: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(" External "), + }, + } + + err := validateComplexitySemanticVectorStore(config, configData) + assert.Error(t, err, "the raw file value is compared before normalization, so casing and padding must not hide it") +} + +func TestValidateComplexitySemanticVectorStore_StoredExternalWarnsOnly(t *testing.T) { + recorder := &recordingLogger{} + SetLogger(recorder) + t.Cleanup(initTestLogger) + + config := &Config{ + GovernanceConfig: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(configstore.ComplexitySemanticVectorStoreExternal), + }, + } + // config.json carries no governance section at all: the setting can only have + // come from the database, which is unreachable while the server refuses to boot. + configData := &ConfigData{} + + err := validateComplexitySemanticVectorStore(config, configData) + assert.NoError(t, err, "a stored-only external vector store must not strand the operator with an unbootable server") + assert.Len(t, recorder.errors, 1, "booting degraded must still be reported") + assert.Contains(t, recorder.errors[0], "no vector store is configured") +} + +func TestValidateComplexitySemanticVectorStore_ConfiguredStorePasses(t *testing.T) { + recorder := &recordingLogger{} + SetLogger(recorder) + t.Cleanup(initTestLogger) + + config := &Config{ + GovernanceConfig: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(configstore.ComplexitySemanticVectorStoreExternal), + }, + VectorStore: newTestChromemStore(t), + } + configData := &ConfigData{ + Governance: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(configstore.ComplexitySemanticVectorStoreExternal), + }, + } + + assert.NoError(t, validateComplexitySemanticVectorStore(config, configData)) + assert.Empty(t, recorder.errors, "a satisfied requirement must not log") +} + +func TestValidateComplexitySemanticVectorStore_NonExternalModesPass(t *testing.T) { + recorder := &recordingLogger{} + SetLogger(recorder) + t.Cleanup(initTestLogger) + + for _, mode := range []string{ + configstore.ComplexitySemanticVectorStoreEmbedded, + configstore.ComplexitySemanticVectorStoreAuto, + } { + config := &Config{ + GovernanceConfig: &configstore.GovernanceConfig{ + ComplexityAnalyzerConfig: complexityConfigWithVectorStore(mode), + }, + } + assert.NoError(t, validateComplexitySemanticVectorStore(config, &ConfigData{}), "mode %q must boot without a vector store", mode) + } + assert.Empty(t, recorder.errors) +} diff --git a/transports/config.schema.json b/transports/config.schema.json index 031d70c823..59c0354bb0 100644 --- a/transports/config.schema.json +++ b/transports/config.schema.json @@ -3695,11 +3695,59 @@ }, "keywords": { "$ref": "#/$defs/complexity_analyzer_keywords" + }, + "semantic": { + "$ref": "#/$defs/complexity_semantic_config" } }, "required": ["tier_boundaries", "keywords"], "additionalProperties": false }, + "complexity_semantic_config": { + "type": "object", + "description": "Embedding-based (semantic) complexity classification settings. Presence of this block enables the semantic classifier. The classifier embeds the analyzer's shared per-tier keyword lists as its exemplars.", + "properties": { + "provider": { + "type": "string", + "minLength": 1, + "description": "Provider used to generate embeddings for complexity classification" + }, + "embedding_model": { + "type": "string", + "minLength": 1, + "description": "Model used to generate embeddings for complexity classification" + }, + "timeout": { + "description": "Per-request embedding timeout (duration string like '100ms', or milliseconds as a number; default: 100ms). On timeout the fallback classifier is used.", + "oneOf": [ + { + "type": "string", + "pattern": "^[0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h)$" + }, + { + "type": "number", + "exclusiveMinimum": 0 + } + ] + }, + "fallback": { + "type": "string", + "enum": ["lexical", "none"], + "description": "Classifier used when semantic classification is unavailable or exceeds its timeout (default: lexical)" + }, + "count_toward_budgets": { + "type": "boolean", + "description": "Record classification embedding cost against governance budgets (record-only, never enforced; default: false)" + }, + "vector_store": { + "type": "string", + "enum": ["auto", "embedded", "external"], + "description": "Where exemplar embeddings are stored: 'embedded' uses the built-in chromem store, 'external' requires the configured vector_store section, 'auto' picks external when configured and embedded otherwise (default: embedded)" + } + }, + "required": ["provider", "embedding_model"], + "additionalProperties": false + }, "auth_config": { "type": "object", "description": "Authentication configuration. Deprecated: Use governance.auth_config instead.", diff --git a/transports/schema_test/config_schema_test.go b/transports/schema_test/config_schema_test.go index 98165cc7f3..e23828f6e6 100644 --- a/transports/schema_test/config_schema_test.go +++ b/transports/schema_test/config_schema_test.go @@ -480,6 +480,86 @@ func TestSchemaComplexityAnalyzerKeywordCompatibility(t *testing.T) { } } +func TestSchemaComplexitySemanticConfig(t *testing.T) { + compiled := compileSchema(t) + prefix := `{"governance":{"complexity_analyzer_config":{"tier_boundaries":{"simple_medium":0.2,"medium_complex":0.4},"keywords":{"simple_keywords":["hello"],"medium_keywords":["api"],"complex_keywords":["tradeoffs"]}` + suffix := `}}}` + + tests := []struct { + name string + semantic string + wantError bool + }{ + { + name: "no semantic block", + semantic: ``, + }, + { + name: "minimal semantic block", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536}`, + }, + { + name: "full semantic block", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536,"timeout":"100ms","fallback":"lexical","count_toward_budgets":true,"vector_store":"embedded"}`, + }, + { + name: "timeout as milliseconds number", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536,"timeout":250}`, + }, + { + name: "missing embedding_model", + semantic: `,"semantic":{"provider":"openai","dimension":1536}`, + wantError: true, + }, + { + name: "dimension below minimum", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1}`, + wantError: true, + }, + { + name: "custom provider name", + semantic: `,"semantic":{"provider":"my-custom-provider","embedding_model":"text-embedding-3-small","dimension":1536}`, + }, + { + name: "empty provider", + semantic: `,"semantic":{"provider":"","embedding_model":"text-embedding-3-small","dimension":1536}`, + wantError: true, + }, + { + name: "unknown fallback value", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536,"fallback":"llm"}`, + wantError: true, + }, + { + name: "unknown vector_store value", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536,"vector_store":"pgvector"}`, + wantError: true, + }, + { + name: "unknown semantic field", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536,"threshold":0.8}`, + wantError: true, + }, + { + name: "exemplars block no longer accepted", + semantic: `,"semantic":{"provider":"openai","embedding_model":"text-embedding-3-small","dimension":1536,"exemplars":{"simple_exemplars":["hi"]}}`, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateConfig(t, compiled, prefix+tt.semantic+suffix) + if tt.wantError && err == nil { + t.Fatal("expected validation error") + } + if !tt.wantError && err != nil { + t.Fatalf("expected config to validate, got: %v", err) + } + }) + } +} + func TestSchemaSCIMConfigValidation(t *testing.T) { compiled := compileSchema(t)