fix(modeladmin): show and persist map fields like entity_actions in the editor

richiejp · richiejp · commit 3435c90ab0a5 · 2026-06-07T10:02:01.000+01:00
Two coupled fixes so detector entity_actions (and other map fields: roles,
engine_args) can be viewed and edited in the model config UI:

- Editor: flattenConfig stopped recursing past registered map leaves, so a
  populated entity_actions block now renders with its rows instead of being
  flattened into invisible dotted scalar paths. The load effect is split into
  a fetch and a derive step so a late metadata arrival re-flattens without a
  second fetch, and useConfigMetadata returns stable empty slices.

- PatchConfig: replace the mergo deep-merge (which unions maps and never
  deletes keys) with a metadata-driven patchMerge that overwrites map-typed
  leaves wholesale. Removing an entity action in the editor and saving now
  drops it from the YAML; emptying the map removes the field entirely.

Assisted-by: claude-code:claude-opus-4-8 [Claude Code]
diff --git a/core/http/react-ui/e2e/model-config.spec.js b/core/http/react-ui/e2e/model-config.spec.js
@@ -14,6 +14,7 @@ const MOCK_METADATA = {
     { path: 'parameters.top_p', yaml_key: 'top_p', go_type: '*float64', ui_type: 'float', section: 'parameters', label: 'Top P', description: 'Nucleus sampling threshold', component: 'slider', min: 0, max: 1, step: 0.05, order: 10 },
     { path: 'pii_detection.builtins', yaml_key: 'builtins', go_type: '[]string', ui_type: '[]string', section: 'general', label: 'Built-in Secret Patterns', description: 'Built-in credential patterns', component: 'pii-builtins-select', options: [{ value: 'anthropic_api_key', label: 'anthropic_api_key — Anthropic API key' }, { value: 'github_token', label: 'github_token — GitHub token' }], order: 213 },
     { path: 'pii_detection.patterns', yaml_key: 'patterns', go_type: '[]config.PIIPattern', ui_type: 'object', section: 'general', label: 'Custom Secret Patterns', description: 'Operator-defined restricted-regex patterns', component: 'pii-pattern-list', order: 214 },
+    { path: 'pii_detection.entity_actions', yaml_key: 'entity_actions', go_type: 'map[string]string', ui_type: 'map', section: 'general', label: 'Detector Entity Actions', description: 'Per-entity-group action policy', component: 'entity-action-list', order: 212 },
   ],
 }
 
@@ -287,4 +288,45 @@ test.describe('Model Editor - Interactive Tab', () => {
     await expect(page.locator('input[placeholder^="match,"]')).toBeVisible()
   })
 
+  // Regression: a map-typed field (entity_actions) present in the loaded YAML
+  // must render WITH its values. flattenConfig used to recurse into the map,
+  // scattering it across pii_detection.entity_actions.<GROUP> paths that match
+  // no registered field, so the editor showed neither the field nor the
+  // per-entity policy (e.g. SSN -> block) the operator had configured.
+  test('entity_actions map field present in YAML renders with its values', async ({ page }) => {
+    // Override the edit endpoint for this test: YAML that carries a populated
+    // entity_actions map alongside a scalar sibling (default_action).
+    await page.route('**/api/models/edit/ner-model', (route) => {
+      route.fulfill({
+        contentType: 'application/json',
+        body: JSON.stringify({
+          name: 'ner-model',
+          config: [
+            'name: ner-model',
+            'backend: llama-cpp',
+            'pii_detection:',
+            '    default_action: mask',
+            '    entity_actions:',
+            '        SSN: block',
+            '        EMAIL: mask',
+            '',
+          ].join('\n'),
+        }),
+      })
+    })
+
+    await page.goto('/app/model-editor/ner-model')
+
+    // The entity-action-list editor is rendered (field label visible)…
+    await expect(page.getByText('Detector Entity Actions').first()).toBeVisible()
+    // …and bound to the existing map: one row per configured group, in order.
+    const groupInputs = page.locator('input[aria-label="Entity group"]')
+    await expect(groupInputs).toHaveCount(2)
+    await expect(groupInputs.nth(0)).toHaveValue('SSN')
+    await expect(groupInputs.nth(1)).toHaveValue('EMAIL')
+    // The action select shows the bound action label (block), proving the map
+    // values bound, not just an empty editor.
+    await expect(page.getByText(/block —/i).first()).toBeVisible()
+  })
+
 })
diff --git a/core/http/react-ui/src/hooks/useConfigMetadata.js b/core/http/react-ui/src/hooks/useConfigMetadata.js
@@ -1,6 +1,11 @@
 import { useState, useEffect } from 'react'
 import { modelsApi } from '../utils/api'
 
+// Stable empty references so consumers that memoize on `sections`/`fields`
+// (e.g. ModelEditor's leafPaths) don't see a new array every render while
+// the metadata request is still in flight — which would thrash their effects.
+const EMPTY = []
+
 export function useConfigMetadata() {
   const [metadata, setMetadata] = useState(null)
   const [loading, setLoading] = useState(true)
@@ -14,8 +19,8 @@ export function useConfigMetadata() {
   }, [])
 
   return {
-    sections: metadata?.sections || [],
-    fields: metadata?.fields || [],
+    sections: metadata?.sections || EMPTY,
+    fields: metadata?.fields || EMPTY,
     loading,
     error,
   }
diff --git a/core/http/react-ui/src/pages/ModelEditor.jsx b/core/http/react-ui/src/pages/ModelEditor.jsx
@@ -30,13 +30,23 @@ const SECTION_COLORS = {
   mitm: 'var(--color-warning)', pii: 'var(--color-error)', other: 'var(--color-text-muted)',
 }
 
-function flattenConfig(obj, prefix = '') {
+// flattenConfig turns a parsed YAML config into a flat { 'a.b.c': value }
+// map keyed by the same dotted paths the field registry uses. leafPaths is
+// the set of registered schema leaf paths: recursion STOPS at any of them so
+// a map-typed field (e.g. pii_detection.entity_actions, a {GROUP: action}
+// object) is stored whole at its own path. Without this guard a map's value
+// was scattered into `pii_detection.entity_actions.SSN` etc. — paths that
+// match no registered field — so the editor rendered neither the field nor
+// its values, hiding per-entity policy like SSN→block from the operator.
+function flattenConfig(obj, leafPaths, prefix = '') {
   const result = {}
   if (!obj || typeof obj !== 'object') return result
   for (const [key, val] of Object.entries(obj)) {
     const path = prefix ? `${prefix}.${key}` : key
-    if (val !== null && typeof val === 'object' && !Array.isArray(val)) {
-      Object.assign(result, flattenConfig(val, path))
+    if (leafPaths && leafPaths.has(path)) {
+      result[path] = val
+    } else if (val !== null && typeof val === 'object' && !Array.isArray(val)) {
+      Object.assign(result, flattenConfig(val, leafPaths, path))
     } else {
       result[path] = val
     }
@@ -80,6 +90,16 @@ export default function ModelEditor() {
   const { addToast } = useOutletContext()
   const { sections, fields, loading: metaLoading, error: metaError } = useConfigMetadata()
 
+  // Registered schema leaf paths. flattenConfig stops recursing at these so
+  // map-typed fields (e.g. pii_detection.entity_actions) bind as a whole
+  // object to their registered editor instead of vanishing into sub-paths.
+  const leafPaths = useMemo(() => new Set(fields.map(f => f.path)), [fields])
+
+  // The parsed (not-yet-flattened) config loaded from the server. Flattening
+  // is deferred to a separate effect keyed on leafPaths so the schema metadata
+  // can arrive after the config without a fetch race re-clobbering values.
+  const [loadedConfig, setLoadedConfig] = useState(null)
+
   const isCreateMode = !name
   const [selectedTemplate, setSelectedTemplate] = useState(null)
 
@@ -121,34 +141,39 @@ export default function ModelEditor() {
     }
   }, [isCreateMode, searchParams, handleSelectTemplate])
 
-  // Load raw YAML config (edit mode only)
+  // Load raw YAML config (edit mode only). This only fetches + parses; the
+  // flatten-into-form-values step is the separate effect below so it can
+  // re-run when the schema metadata (leafPaths) resolves without re-fetching.
   useEffect(() => {
     if (!name) return
     modelsApi.getEditConfig(name)
       .then(data => {
         const raw = data?.config || ''
         setYamlText(raw)
         setSavedYamlText(raw)
-
-        // Parse YAML to get only the fields actually present in the file
         try {
-          const parsed = YAML.parse(raw)
-          const flat = flattenConfig(parsed || {})
-          const active = new Set(Object.keys(flat))
-          setValues(flat)
-          setInitialValues(structuredClone(flat))
-          setActiveFieldPaths(active)
+          setLoadedConfig(YAML.parse(raw) || {})
         } catch {
-          // If YAML parsing fails, start with empty state
-          setValues({})
-          setInitialValues({})
-          setActiveFieldPaths(new Set())
+          setLoadedConfig({})
         }
       })
       .catch(err => addToast(`Failed to load config: ${err.message}`, 'error'))
       .finally(() => setConfigLoading(false))
   }, [name, addToast])
 
+  // Flatten the loaded config into form values. Keyed on leafPaths so a late
+  // schema-metadata resolution re-flattens (keeping map fields whole) WITHOUT
+  // re-fetching — avoiding a two-fetch race that could clobber values. Only
+  // fires on (re)load: loadedConfig changes per model, leafPaths is stable
+  // once metadata is in, so this never stomps in-progress edits.
+  useEffect(() => {
+    if (loadedConfig === null) return
+    const flat = flattenConfig(loadedConfig, leafPaths)
+    setValues(flat)
+    setInitialValues(structuredClone(flat))
+    setActiveFieldPaths(new Set(Object.keys(flat)))
+  }, [loadedConfig, leafPaths])
+
   // Build field lookup
   const fieldsByPath = useMemo(() => {
     const map = {}
@@ -323,7 +348,7 @@ export default function ModelEditor() {
         try {
           const parsed = YAML.parse(yamlText)
           parsedName = parsed?.name ?? null
-          const flat = flattenConfig(parsed || {})
+          const flat = flattenConfig(parsed || {}, leafPaths)
           setValues(flat)
           setInitialValues(structuredClone(flat))
           setActiveFieldPaths(new Set(Object.keys(flat)))
diff --git a/core/services/modeladmin/config.go b/core/services/modeladmin/config.go
@@ -6,12 +6,13 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 
-	"dario.cat/mergo"
 	"gopkg.in/yaml.v3"
 
 	"github.com/mudler/LocalAI/core/config"
+	"github.com/mudler/LocalAI/core/config/meta"
 	"github.com/mudler/LocalAI/core/gallery"
 	"github.com/mudler/LocalAI/pkg/model"
 	"github.com/mudler/LocalAI/pkg/utils"
@@ -114,9 +115,7 @@ func (s *ConfigService) PatchConfig(_ context.Context, name string, patch map[st
 	if existingMap == nil {
 		existingMap = map[string]any{}
 	}
-	if err := mergo.Merge(&existingMap, patch, mergo.WithOverride); err != nil {
-		return nil, fmt.Errorf("merge configs: %w", err)
-	}
+	patchMerge(existingMap, patch, mapLeafFieldPaths(), "")
 	yamlData, err := yaml.Marshal(existingMap)
 	if err != nil {
 		return nil, fmt.Errorf("marshal merged YAML: %w", err)
@@ -142,6 +141,55 @@ func (s *ConfigService) PatchConfig(_ context.Context, name string, patch map[st
 	return &updated, nil
 }
 
+// mapLeafFieldPaths returns the set of dotted config paths whose schema type is
+// a map that the editor edits as one complete value (e.g.
+// pii_detection.entity_actions, roles, engine_args). A PATCH must REPLACE these
+// wholesale rather than union them: the deep-merge only adds and overrides
+// keys, so a map entry the admin deleted in the editor would otherwise silently
+// survive. Derived from the config schema so it stays correct as map fields are
+// added. (UIType comes from reflection, independent of any registry override.)
+func mapLeafFieldPaths() map[string]struct{} {
+	md := meta.BuildConfigMetadata(reflect.TypeFor[config.ModelConfig]())
+	out := make(map[string]struct{})
+	for _, f := range md.Fields {
+		if f.UIType == "map" {
+			out[f.Path] = struct{}{}
+		}
+	}
+	return out
+}
+
+// patchMerge deep-merges src into dst with the same shape as the previous
+// mergo.WithOverride behaviour — scalars and slices replace; nested
+// struct-maps (e.g. pii_detection, parameters) recurse so unknown sibling keys
+// the editor doesn't model survive — EXCEPT that any path in mapLeaves is
+// replaced wholesale, and removed when the patch sets it empty, so deletions
+// inside a map field persist to disk.
+func patchMerge(dst, src map[string]any, mapLeaves map[string]struct{}, prefix string) {
+	for k, sv := range src {
+		path := k
+		if prefix != "" {
+			path = prefix + "." + k
+		}
+		if _, isLeaf := mapLeaves[path]; isLeaf {
+			if m, ok := sv.(map[string]any); ok && len(m) == 0 {
+				delete(dst, k) // emptied map field -> drop it from the YAML
+			} else {
+				dst[k] = sv
+			}
+			continue
+		}
+		// Recurse into struct-like nesting so dst-only sibling keys survive.
+		if sm, ok := sv.(map[string]any); ok {
+			if dm, ok2 := dst[k].(map[string]any); ok2 {
+				patchMerge(dm, sm, mapLeaves, path)
+				continue
+			}
+		}
+		dst[k] = sv
+	}
+}
+
 // EditYAML replaces the YAML for an installed model, with optional rename
 // support. ml may be nil; when set, EditYAML calls ml.ShutdownModel(oldName)
 // after a successful write so the next inference picks up the new config.
diff --git a/core/services/modeladmin/config_test.go b/core/services/modeladmin/config_test.go
@@ -107,6 +107,64 @@ var _ = Describe("ConfigService", func() {
 			_, err := svc.PatchConfig(ctx, "qwen", map[string]any{})
 			Expect(err).To(MatchError(ErrEmptyBody))
 		})
+
+		It("replaces a map field wholesale so deleted entries do not survive", func() {
+			// A detector model with a populated entity_actions map. The editor
+			// removes SSN and re-sends the remaining map; a naive deep-merge
+			// would re-add SSN (it only adds/overrides keys, never deletes).
+			writeModelYAML(svc, dir, "ner", map[string]any{
+				"backend":        "llama-cpp",
+				"known_usecases": []any{"token_classify"},
+				"pii_detection": map[string]any{
+					"default_action": "mask",
+					"entity_actions": map[string]any{"SSN": "block", "EMAIL": "mask"},
+				},
+			})
+
+			_, err := svc.PatchConfig(ctx, "ner", map[string]any{
+				"pii_detection": map[string]any{
+					"default_action": "mask",
+					"entity_actions": map[string]any{"EMAIL": "mask"},
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+
+			raw, err := os.ReadFile(filepath.Join(dir, "ner.yaml"))
+			Expect(err).ToNot(HaveOccurred())
+			var got map[string]any
+			Expect(yaml.Unmarshal(raw, &got)).To(Succeed())
+			pii := got["pii_detection"].(map[string]any)
+			ea := pii["entity_actions"].(map[string]any)
+			Expect(ea).To(HaveKeyWithValue("EMAIL", "mask"))
+			Expect(ea).NotTo(HaveKey("SSN"), "deleted map entry must not survive the patch")
+			// The scalar sibling in the same nested block is still preserved.
+			Expect(pii).To(HaveKeyWithValue("default_action", "mask"))
+		})
+
+		It("drops a map field entirely when the patch empties it", func() {
+			writeModelYAML(svc, dir, "ner", map[string]any{
+				"backend":        "llama-cpp",
+				"known_usecases": []any{"token_classify"},
+				"pii_detection": map[string]any{
+					"default_action": "mask",
+					"entity_actions": map[string]any{"SSN": "block"},
+				},
+			})
+
+			_, err := svc.PatchConfig(ctx, "ner", map[string]any{
+				"pii_detection": map[string]any{
+					"entity_actions": map[string]any{},
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+
+			raw, err := os.ReadFile(filepath.Join(dir, "ner.yaml"))
+			Expect(err).ToNot(HaveOccurred())
+			var got map[string]any
+			Expect(yaml.Unmarshal(raw, &got)).To(Succeed())
+			pii := got["pii_detection"].(map[string]any)
+			Expect(pii).NotTo(HaveKey("entity_actions"))
+		})
 	})
 
 	Describe("EditYAML", func() {
