Skip to content

Commit 56c836d

Browse files
committed
fix(compiler): close two crash paths from #71 review
Code review on PR #75 surfaced two function-aborting bugs that the original defenses did not cover. Both shift the same silent-loss class one step upstream — `_compile_concepts` raises before any concept task runs, the v1 summary is never written on the short-doc path, and the new `[WARN] planned vs written` line never fires. - `_filter_concept_items` also requires a non-empty string `name`. Without this, dicts that omit the `name` key (JSON mode constrains syntax, not schema) reach the `planned_slugs` set comprehension at line 1014 and raise `KeyError: 'name'`. - New `_filter_related_slugs` mirrors the same guard for the `related` list, dropping non-strings. The previous code passed `parsed.get("related", [])` straight into `_sanitize_concept_name`, which calls `unicodedata.normalize("NFKC", name)` and raises `TypeError` on any non-`str` entry. Verified end-to-end against the original screenwriter EPUB on deepseek-v4-flash (no regression) and via direct unit-style calls that feed every mishape into both helpers and observe the expected drops + WARN messages.
1 parent ac22a64 commit 56c836d

1 file changed

Lines changed: 39 additions & 9 deletions

File tree

openkb/agent/compiler.py

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -306,23 +306,53 @@ def _parse_json(text: str) -> list | dict:
306306

307307

308308
def _filter_concept_items(items: list, label: str) -> list[dict]:
309-
"""Keep only dict items; warn about anything else.
309+
"""Keep only dicts that carry a non-empty ``name``; warn about anything else.
310310
311311
The concepts-plan prompt asks for ``[{"name": ..., "title": ...}, ...]``
312-
but LLMs occasionally emit nested lists or bare strings. Letting those
313-
through crashes ``_gen_create`` at ``concept.get("title")`` and silently
314-
loses every concept in the batch (issue #71).
312+
but LLMs occasionally emit nested lists, bare strings, or dicts that
313+
forgot ``name``. JSON mode constrains syntax, not schema, so all of
314+
these still slip through ``_parse_json``. Without this guard a
315+
name-less dict crashes the ``planned_slugs`` set comprehension
316+
(``c["name"]`` → KeyError) and aborts the whole concepts step.
315317
"""
316318
if not isinstance(items, list):
317319
logger.warning("concepts plan: %s was %s, expected list — dropping",
318320
label, type(items).__name__)
319321
return []
320-
valid = [c for c in items if isinstance(c, dict)]
322+
valid = [c for c in items if isinstance(c, dict) and isinstance(c.get("name"), str) and c["name"].strip()]
321323
if len(valid) < len(items):
322-
bad_types = sorted({type(c).__name__ for c in items if not isinstance(c, dict)})
324+
reasons: list[str] = []
325+
for c in items:
326+
if not isinstance(c, dict):
327+
reasons.append(type(c).__name__)
328+
elif not isinstance(c.get("name"), str) or not c["name"].strip():
329+
reasons.append("dict-missing-name")
323330
logger.warning(
324-
"concepts plan: dropped %d malformed %s item(s) (types: %s)",
325-
len(items) - len(valid), label, ", ".join(bad_types),
331+
"concepts plan: dropped %d malformed %s item(s) (reasons: %s)",
332+
len(items) - len(valid), label, ", ".join(sorted(set(reasons))),
333+
)
334+
return valid
335+
336+
337+
def _filter_related_slugs(items: list) -> list[str]:
338+
"""Keep only non-empty string slugs; warn about anything else.
339+
340+
``related`` is documented in the prompt as "array of slug strings",
341+
but the same shape drift that motivates ``_filter_concept_items``
342+
applies here. Non-strings reaching ``_sanitize_concept_name`` raise
343+
TypeError inside ``unicodedata.normalize`` and crash the whole
344+
``_compile_concepts`` call.
345+
"""
346+
if not isinstance(items, list):
347+
logger.warning("concepts plan: related was %s, expected list — dropping",
348+
type(items).__name__)
349+
return []
350+
valid = [s for s in items if isinstance(s, str) and s.strip()]
351+
if len(valid) < len(items):
352+
bad_types = sorted({type(s).__name__ for s in items if not (isinstance(s, str) and s.strip())})
353+
logger.warning(
354+
"concepts plan: dropped %d malformed related item(s) (types: %s)",
355+
len(items) - len(valid), ", ".join(bad_types),
326356
)
327357
return valid
328358

@@ -993,7 +1023,7 @@ def _write_v1_summary_stripped() -> None:
9931023
plan = {
9941024
"create": _filter_concept_items(parsed.get("create", []), "create"),
9951025
"update": _filter_concept_items(parsed.get("update", []), "update"),
996-
"related": parsed.get("related", []),
1026+
"related": _filter_related_slugs(parsed.get("related", [])),
9971027
}
9981028

9991029
create_items = plan["create"]

0 commit comments

Comments
 (0)