Skip to content

Commit b33ce06

Browse files
author
Sebastian Braun
committed
fix(agent): gate entity name-length drop behind strict_entity_types, warn on drops
- Entity create-item name-length gate (_MAX_NAME_WORDS) now only applies when strict_entity_types=true, matching the type-mismatch drop's opt-in behavior. Previously it applied unconditionally regardless of the flag. - Dropped entity items (both type-mismatch and name-length) are now logged at WARNING (visible without -v) with a sample of affected names, instead of INFO (effectively silent for normal runs). Concepts are unaffected (no strict toggle exists for them; the cap still always applies there).
1 parent 53e3976 commit b33ce06

4 files changed

Lines changed: 87 additions & 23 deletions

File tree

‎config.yaml.example‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex
3131
# entity_types (above) instead of coercing it to "other". Off by default
3232
# (backward compatible); turn on for a narrow, domain-specific entity_types
3333
# list where an "other"-typed entity usually signals a bad/too-specific
34-
# candidate you'd rather drop than keep.
34+
# candidate you'd rather drop than keep. Also enables a name-length gate:
35+
# a brand-new entity name longer than 3 words is dropped too (concepts
36+
# always enforce this cap; entities only opt in together with this flag).
37+
# Drops are logged as warnings, never silent.
3538
# strict_entity_types: false
3639

3740
# Optional: how concept/entity pages absorb new documents on `openkb add`.

‎examples/configuration/README.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex
9999
# entity_types (above) instead of coercing it to "other". Off by default
100100
# (backward compatible); turn on for a narrow, domain-specific entity_types
101101
# list where an "other"-typed entity usually signals a bad/too-specific
102-
# candidate you'd rather drop than keep.
102+
# candidate you'd rather drop than keep. Also enables a name-length gate:
103+
# a brand-new entity name longer than 3 words is dropped too (concepts
104+
# always enforce this cap; entities only opt in together with this flag).
105+
# Drops are logged as warnings, never silent.
103106
# strict_entity_types: false
104107

105108
# Optional: LLM / LiteLLM tuning. Keys are forwarded to LiteLLM; `timeout` and
@@ -121,7 +124,7 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex
121124
| `concurrency` | `null` | Caps concurrent LLM calls OpenKB makes during ingest — both PageIndex's indexing of a long document and OpenKB's own concept/entity compilation. The two never run at once for the same document, so one setting covers both. Lower it if you hit provider rate limits or "too many open files" on large PDFs. `null` lets each stage apply its own default. |
122125
| `parallel_tool_calls` | unset | Whether the LLM agents (query, chat, lint, skill) may call tools in parallel. Unset keeps OpenKB's per-agent defaults; `true`/`false` force allow/sequential for every agent; `null` omits the setting (provider default). **Amazon Bedrock needs `null`** (see below). |
123126
| `entity_types` | 7 defaults | Custom vocabulary for entity pages. `other` is always kept. |
124-
| `strict_entity_types` | `false` | When `true`, drops an entity whose type doesn't match `entity_types` instead of coercing it to `other`. |
127+
| `strict_entity_types` | `false` | When `true`, drops an entity whose type doesn't match `entity_types` instead of coercing it to `other`, and also drops a brand-new entity name longer than 3 words (concepts always enforce the name-length cap; entities only opt in together with this flag). Drops are logged as warnings, never silent. |
125128
| `litellm:` | – | A pass-through block for LiteLLM. See below. |
126129

127130
### The `litellm:` block

‎openkb/agent/compiler.py‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,11 @@
9292
# the token-density constant used to compute the soft per-document "how many
9393
# brand-new items" guidance substituted into __DOC_TOKEN_GUIDANCE__. Both are
9494
# intentionally NOT config keys (see issue #247) — the only new config-driven
95-
# knob in this feature is strict_entity_types.
95+
# knob in this feature is strict_entity_types. Concepts always enforce this
96+
# cap; entities only enforce it when strict_entity_types=true (see
97+
# _filter_entity_items) — it opts into the same "reject, don't coerce" spirit
98+
# as the type check, so leaving strict_entity_types off keeps entity name
99+
# length unrestricted, matching pre-issue-#247 behavior exactly.
96100
_MAX_NAME_WORDS = 3
97101
_TOKENS_PER_NEW_ITEM = 1000
98102

@@ -757,46 +761,52 @@ def _filter_entity_items(
757761
758762
``strict`` (see ``config.resolve_strict_entity_types``), when ``True``,
759763
drops an item whose type falls outside ``valid_types`` instead of coercing
760-
it to ``"other"``. ``max_words`` (see :func:`_count_words`), when given,
761-
drops names with more words than that. Both default to today's lenient
762-
behavior (``False``/``None``); pass them only for "create" items — an
763-
"update" targets an already-existing, already-vetted name/type.
764+
it to ``"other"``, and additionally enables the ``max_words`` (see
765+
:func:`_count_words`) name-length gate — both checks are opt-in together,
766+
so leaving ``strict_entity_types`` at its default keeps today's lenient
767+
behavior (no type drop, no length drop) exactly. Pass ``max_words`` only
768+
for "create" items — an "update" targets an already-existing,
769+
already-vetted name/type. Drops are logged at warning level (visible
770+
without ``-v``) with a sample of the affected names, never silently.
764771
"""
765772
if valid_types is None:
766773
valid_types = _ENTITY_TYPES
767774
out: list[dict] = []
768775
if not isinstance(items, list):
769776
return out
770-
dropped_strict = 0
771-
dropped_words = 0
777+
dropped_strict: list[str] = []
778+
dropped_words: list[str] = []
772779
for it in items:
773780
if not isinstance(it, dict):
774781
continue
775782
name = it.get("name")
776783
if not isinstance(name, str) or not name.strip():
777784
continue
778-
if max_words is not None and _count_words(name) > max_words:
779-
dropped_words += 1
785+
if strict and max_words is not None and _count_words(name) > max_words:
786+
dropped_words.append(name)
780787
continue
781788
title = it.get("title") if isinstance(it.get("title"), str) else name
782789
etype = it.get("type")
783790
if not isinstance(etype, str) or etype not in valid_types:
784791
if strict:
785-
dropped_strict += 1
792+
dropped_strict.append(name)
786793
continue
787794
etype = "other"
788795
out.append({"name": name, "title": title, "type": etype})
789796
if dropped_strict:
790-
logger.info(
797+
logger.warning(
791798
"concepts plan: dropped %d entity item(s) with type outside the configured "
792-
"entity_types (strict_entity_types=true)",
793-
dropped_strict,
799+
"entity_types (strict_entity_types=true): %s",
800+
len(dropped_strict),
801+
dropped_strict[:5],
794802
)
795803
if dropped_words:
796-
logger.info(
797-
"concepts plan: dropped %d entity item(s) with names over %d words",
798-
dropped_words,
804+
logger.warning(
805+
"concepts plan: dropped %d entity item(s) with names over %d words "
806+
"(strict_entity_types=true): %s",
807+
len(dropped_words),
799808
max_words,
809+
dropped_words[:5],
800810
)
801811
return out
802812

@@ -813,7 +823,8 @@ def _parse_entities_plan(
813823
Returns ``{"create": [...], "update": [...], "related": [...]}``. A
814824
missing/malformed ``entities`` key yields empty lists, so older or
815825
partial LLM responses never raise. ``strict``/``max_words`` (see
816-
:func:`_filter_entity_items`) are applied to "create" only — an "update"
826+
:func:`_filter_entity_items` — the name-length gate only applies when
827+
``strict`` is ``True``) are applied to "create" only — an "update"
817828
targets an already-existing, already-vetted name/type.
818829
"""
819830
empty = {"create": [], "update": [], "related": []}

‎tests/test_compiler.py‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,9 @@ def test_strict_true_keeps_matching_type(self):
226226
class TestMaxWordsFilter:
227227
"""Hard cap on brand-new concept/entity names to 3 words (see
228228
compiler._count_words / issue #247) — a lightweight proxy for "too
229-
specific to be reusable knowledge"."""
229+
specific to be reusable knowledge". Concepts always enforce this cap;
230+
entities only enforce it when strict=True (opt-in together with
231+
strict_entity_types, see issue #247 follow-up)."""
230232

231233
def test_count_words_splits_on_hyphen_underscore_and_space(self):
232234
assert _count_words("attention") == 1
@@ -248,19 +250,48 @@ def test_concept_items_without_max_words_are_unaffected(self):
248250
out = _filter_concept_items(items, "update")
249251
assert len(out) == 1
250252

251-
def test_entity_items_over_limit_are_dropped(self):
253+
def test_entity_items_over_limit_are_dropped_when_strict(self):
252254
items = [
253255
{"name": "nvidia", "title": "NVIDIA", "type": "organization"},
254256
{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "person"},
255257
]
256-
out = _filter_entity_items(items, max_words=3)
258+
out = _filter_entity_items(items, max_words=3, strict=True)
257259
assert [e["name"] for e in out] == ["nvidia"]
258260

261+
def test_entity_items_over_limit_kept_when_not_strict(self):
262+
# The word-length gate is opt-in together with strict_entity_types
263+
# (see issue #247 follow-up) — passing max_words alone must not drop
264+
# anything unless strict=True is also set.
265+
items = [
266+
{"name": "nvidia", "title": "NVIDIA", "type": "organization"},
267+
{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "person"},
268+
]
269+
out = _filter_entity_items(items, max_words=3, strict=False)
270+
assert [e["name"] for e in out] == ["nvidia", "andreas-mueller-alwart-ssmpa-2573"]
271+
259272
def test_entity_items_without_max_words_are_unaffected(self):
260273
items = [{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "other"}]
261274
out = _filter_entity_items(items)
262275
assert len(out) == 1
263276

277+
def test_dropped_entity_items_are_logged_at_warning_not_silently(self, caplog):
278+
# Drops must be visible without -v/--verbose (root logger defaults to
279+
# WARNING, see cli.py) — logging them at INFO would be effectively
280+
# silent for a normal `openkb add` run.
281+
import logging
282+
283+
valid = frozenset({"person", "other"})
284+
items = [
285+
{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "A", "type": "person"},
286+
{"name": "x", "title": "X", "type": "organization"},
287+
]
288+
with caplog.at_level(logging.WARNING, logger="openkb.agent.compiler"):
289+
out = _filter_entity_items(items, valid, strict=True, max_words=3)
290+
assert out == []
291+
messages = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING]
292+
assert any("over 3 words" in m for m in messages)
293+
assert any("type outside the configured" in m for m in messages)
294+
264295

265296
class TestParseEntitiesPlanStrictAndMaxWords:
266297
"""strict/max_words are threaded through _parse_entities_plan for
@@ -301,6 +332,22 @@ def test_update_is_never_word_or_strict_filtered(self):
301332
assert len(out["update"]) == 1
302333
assert out["update"][0]["type"] == "other" # coerced, not strict-dropped
303334

335+
def test_create_max_words_ignored_without_strict(self):
336+
# The name-length gate is opt-in together with strict_entity_types —
337+
# passing max_words without strict=True must not drop long names.
338+
valid = frozenset({"person", "other"})
339+
parsed = {
340+
"entities": {
341+
"create": [
342+
{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "A", "type": "person"},
343+
],
344+
"update": [],
345+
"related": [],
346+
}
347+
}
348+
out = _parse_entities_plan(parsed, valid, strict=False, max_words=3)
349+
assert [e["name"] for e in out["create"]] == ["andreas-mueller-alwart-ssmpa-2573"]
350+
304351

305352
class TestDocTokenGuidance:
306353
"""Soft, textual __DOC_TOKEN_GUIDANCE__ substitution (see issue #247) —

0 commit comments

Comments
 (0)