fix(entity-types): address xhigh review — DEFAULT_CONFIG parity, empty-list inherit, silent reads, set-diff

- config: define DEFAULT_ENTITY_TYPES before DEFAULT_CONFIG and add
  `entity_types` to DEFAULT_CONFIG so the key layers like every other
  GLOBAL_SCALAR_KEY and always appears in the effective config (#1).
- config: resolve_effective_config treats an empty entity_types list the
  same as null → inherit, so a KB that cleared its override doesn't pin an
  empty vocabulary (#2).
- config: resolve_entity_types(config, *, warn=True); config-read paths pass
  warn=False so a plain GET doesn't spam coercion warnings (#4).
- api_config: both read paths call resolve_entity_types(..., warn=False).
- KbSettingsSheet: inherited badge shows the effective list, not
  `globalValue ?? effective`; drop the now-unused globalValue prop (#3).
- Settings: global entity_types diff is order-insensitive — the vocabulary
  is a set, so re-adding a removed type is not a change (#6).

Backend pytest 1240 passed; ruff/format/mypy clean; frontend build green.

Claude-Session: https://claude.ai/code/session_01XMxbhmAkxxVV8CFWCZDBaG

Author: KylinMountain

frontend/src/components/KbSettingsSheet.tsx

@@ -278,7 +278,6 @@ function KbConfigSection({ kb }: { kb: string }) {
       <EntityTypesRow
         source={config.sources.entity_types}
         effective={config.entity_types}
-        globalValue={config.global_values.entity_types}
         busy={busy}
         onSet={setEntityTypesOverride}
         onRevert={() => setEntityTypesOverride(null)}
@@ -415,13 +414,12 @@ function OverrideRow({
  *  chips list (EntityTypesEditor) and every add/remove persists immediately —
  *  the chips always show the server-cleaned effective list. */
 function EntityTypesRow({
-  source, effective, globalValue, busy, onSet, onRevert,
+  source, effective, busy, onSet, onRevert,
 }: {
   source: ConfigSource
-  /** Cleaned effective list (always includes "other"). */
+  /** Cleaned effective list (always includes "other") — shown as-is in the
+   *  inherited badge, so it matches the vocabulary the compiler actually uses. */
   effective: string[]
-  /** RAW global-layer list; null when global.yaml is silent. */
-  globalValue: string[] | null
   busy: boolean
   onSet: (value: string[]) => void
   onRevert: () => void
@@ -432,7 +430,7 @@ function EntityTypesRow({
 
   const inheritedBadge =
     source === 'global'
-      ? t('kbSettings:inheritGlobal', { value: (globalValue ?? effective).join(', ') })
+      ? t('kbSettings:inheritGlobal', { value: effective.join(', ') })
       : t('kbSettings:inheritDefault', { value: effective.join(', ') })
 
   return (

frontend/src/pages/Settings.tsx

@@ -109,11 +109,12 @@ export default function Settings() {
     if (threshold.trim() !== '' && Number.isInteger(n) && n >= 1 && n !== config.pageindex_threshold) {
       cfg.pageindex_threshold = n
     }
-    // Order-sensitive compare against the last-fetched cleaned list. The
-    // editor only appends/removes, so equal content ⇒ equal order; a reorder
-    // never happens client-side.
-    const baseTypes = config.entity_types
-    if (entityTypes.length !== baseTypes.length || entityTypes.some((v, i) => v !== baseTypes[i])) {
+    // Order-INSENSITIVE compare: the vocabulary is a set (the compiler treats
+    // it as an allowlist), and removing-then-re-adding a type appends it at the
+    // end — so a same-set reorder must NOT dirty the form or persist a no-op.
+    const a = [...entityTypes].sort()
+    const b = [...config.entity_types].sort()
+    if (a.length !== b.length || a.some((v, i) => v !== b[i])) {
       cfg.entity_types = entityTypes
     }
     if (Object.keys(cfg).length > 0) patch.config = cfg

openkb/api_config.py

@@ -153,7 +153,7 @@ def read_kb_config(kb_dir: Path) -> KbConfigResponse:
         pageindex_threshold=effective["pageindex_threshold"],
         # Cleaned effective list (what the compiler will use), not the raw stored
         # value — resolve_entity_types defaults + dedupes + ensures "other".
-        entity_types=resolve_entity_types(effective),
+        entity_types=resolve_entity_types(effective, warn=False),
         openai_api_base=bundle.base_url,
         has_api_key=bundle.api_key is not None,
         sources=sources,
@@ -268,7 +268,7 @@ def read_global_config() -> GlobalConfigResponse:
         language=gc.get("language", DEFAULT_CONFIG["language"]),
         pageindex_threshold=gc.get("pageindex_threshold", DEFAULT_CONFIG["pageindex_threshold"]),
         # Effective global vocabulary (cleaned; defaults to DEFAULT_ENTITY_TYPES).
-        entity_types=resolve_entity_types(gc),
+        entity_types=resolve_entity_types(gc, warn=False),
         # Report the EFFECTIVE root (env > global.yaml kb_root > default) via the
         # module object so a test-monkeypatched GLOBAL_CONFIG_DIR is honored.
         kb_root=str(_config_module.kb_root_dir()),

openkb/config.py

@@ -15,13 +15,7 @@
 
 logger = logging.getLogger(__name__)
 
-DEFAULT_CONFIG: dict[str, Any] = {
-    "model": "gpt-5.4",
-    "language": "en",
-    "pageindex_threshold": 20,
-}
-
-# Default entity-type vocabulary. Overridable per-KB via the optional
+# Default entity-type vocabulary. Overridable globally / per-KB via the optional
 # ``entity_types:`` config key (see ``resolve_entity_types``).
 DEFAULT_ENTITY_TYPES: tuple[str, ...] = (
     "person",
@@ -33,6 +27,17 @@
     "other",
 )
 
+DEFAULT_CONFIG: dict[str, Any] = {
+    "model": "gpt-5.4",
+    "language": "en",
+    "pageindex_threshold": 20,
+    # A GLOBAL_SCALAR_KEY like the three above, so the merged `effective` dict
+    # always carries it (the layering/`sources` logic is type-agnostic). A
+    # global/KB list overrides it wholesale; resolve_entity_types cleans the
+    # effective value on read.
+    "entity_types": list(DEFAULT_ENTITY_TYPES),
+}
+
 GLOBAL_CONFIG_DIR = Path.home() / ".config" / "openkb"
 GLOBAL_CONFIG_PATH = GLOBAL_CONFIG_DIR / "global.yaml"
 GLOBAL_CONFIG_LOCK_PATH = GLOBAL_CONFIG_DIR / "global.lock"
@@ -88,7 +93,7 @@ def _load_global_config_unlocked() -> dict[str, Any]:
     return {}
 
 
-def resolve_entity_types(config: dict) -> list[str]:
+def resolve_entity_types(config: dict, *, warn: bool = True) -> list[str]:
     """Resolve the effective entity-type list from a loaded config dict.
 
     If ``config["entity_types"]`` is a non-empty list, each string item is
@@ -98,18 +103,23 @@ def resolve_entity_types(config: dict) -> list[str]:
     de-duped (order preserving) and ``"other"`` is always appended when missing
     (it is the coercion fallback). Otherwise — key absent, not a list, empty,
     or fully malformed — :data:`DEFAULT_ENTITY_TYPES` is returned, so behavior
-    is byte-identical to the default. A warning is logged only when
-    ``entity_types`` was present-but-malformed.
+    is byte-identical to the default.
+
+    ``warn`` logs a warning when ``entity_types`` is present-but-malformed. It is
+    ``True`` for the compile path (a real config problem the author should see)
+    and passed ``False`` by the config-READ endpoints, which run on every GET and
+    must not spam the log for a malformed value.
     """
     raw = config.get("entity_types")
     if raw is None:
         return list(DEFAULT_ENTITY_TYPES)
     if not isinstance(raw, list):
-        logger.warning(
-            "config: 'entity_types' must be a list of strings, got %s — "
-            "falling back to the default entity types.",
-            type(raw).__name__,
-        )
+        if warn:
+            logger.warning(
+                "config: 'entity_types' must be a list of strings, got %s — "
+                "falling back to the default entity types.",
+                type(raw).__name__,
+            )
         return list(DEFAULT_ENTITY_TYPES)
     cleaned: list[str] = []
     for x in raw:
@@ -119,10 +129,11 @@ def resolve_entity_types(config: dict) -> list[str]:
         if s and s not in cleaned:
             cleaned.append(s)
     if not cleaned:
-        logger.warning(
-            "config: 'entity_types' was present but yielded no usable values — "
-            "falling back to the default entity types.",
-        )
+        if warn:
+            logger.warning(
+                "config: 'entity_types' was present but yielded no usable values — "
+                "falling back to the default entity types.",
+            )
         return list(DEFAULT_ENTITY_TYPES)
     if "other" not in cleaned:
         cleaned.append("other")
@@ -533,10 +544,12 @@ def resolve_effective_config(kb_dir: Path) -> tuple[dict[str, Any], dict[str, st
         if not isinstance(kb_config, dict):
             kb_config = {}
         for key, value in kb_config.items():
-            # A scalar explicitly nulled in config.yaml means "inherit": don't
-            # let it clobber the global/default layer. The gate is on
-            # GLOBAL_SCALAR_KEYS so non-scalar nulls (parallel_tool_calls) stay.
-            if key in GLOBAL_SCALAR_KEYS and value is None:
+            # A GLOBAL_SCALAR_KEYS value explicitly nulled — or an empty
+            # entity_types list, since an empty vocabulary is meaningless —
+            # means "inherit": don't clobber the global/default layer, and keep
+            # `sources` reporting the layer that actually supplied the value. The
+            # gate is on GLOBAL_SCALAR_KEYS so non-scalar nulls (parallel_tool_calls) stay.
+            if key in GLOBAL_SCALAR_KEYS and (value is None or value == []):
                 continue
             effective[key] = value
             if key in GLOBAL_SCALAR_KEYS: