From 51e16cee93b16aa0c8790b25e3d9971dd412ac62 Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Mon, 7 Sep 2026 10:29:57 +0200 Subject: [PATCH 1/8] feat(agent): add opt-in append-note concept_update_mode (#245) Adds concept_update_mode (default "rewrite", unchanged) with a new "append" mode: instead of sending an existing concept/entity page's full body to the LLM for a rewrite, generates a short 1-2 sentence note (the LLM never sees the existing page) and appends it deterministically under a "## Notes" heading, keyed by source document so re-ingesting an updated document replaces its own line instead of duplicating it. Removal cleanup and config docs updated accordingly. Resolves #245 --- config.yaml.example | 8 + openkb/agent/compiler.py | 188 ++++++++++++++++++++++-- openkb/agent/compiler_notes.py | 257 +++++++++++++++++++++++++++++++++ openkb/config.py | 23 +++ tests/test_compiler.py | 22 +++ tests/test_compiler_notes.py | 172 ++++++++++++++++++++++ tests/test_config.py | 30 ++++ 7 files changed, 685 insertions(+), 15 deletions(-) create mode 100644 openkb/agent/compiler_notes.py create mode 100644 tests/test_compiler_notes.py diff --git a/config.yaml.example b/config.yaml.example index 47d2fda2b..c19e4e7c6 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -27,6 +27,14 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # - dataset # - model +# Optional: how concept/entity pages absorb new documents on `openkb add`. +# rewrite default — send the existing page's full body to the LLM and +# let it rewrite the whole page to incorporate the new document. +# append cheaper for large/mature wikis — instead of a full rewrite, +# generate a short note about the new document and append it to +# the page under a "## Notes" heading (no full-page LLM rewrite). +# concept_update_mode: rewrite + # Optional: LLM / LiteLLM tuning. Keys are forwarded to LiteLLM; `timeout` and # `extra_headers` apply per request, the rest are set as litellm.. # litellm: diff --git a/openkb/agent/compiler.py b/openkb/agent/compiler.py index d0c9f878d..33463b3f1 100644 --- a/openkb/agent/compiler.py +++ b/openkb/agent/compiler.py @@ -30,6 +30,7 @@ import litellm from openkb import frontmatter +from openkb.agent import compiler_notes from openkb.config import ( DEFAULT_ENTITY_TYPES, get_extra_headers, @@ -1334,6 +1335,8 @@ def _remove_doc_from_pages( ``## Related Documents`` section. - Remove any standalone ``See also: [[summaries/{doc_name}]]`` lines (left by ``_add_related_link``). + - Remove this doc's ``## Notes`` line, if any (left by + ``concept_update_mode="append"``; a no-op for "rewrite"-mode pages). - If the ``sources:`` list becomes empty AND ``keep_empty`` is False, delete the page entirely. @@ -1390,6 +1393,17 @@ def _remove_doc_from_pages( flags=re.MULTILINE, ) + # Drop this doc's "## Notes" line (left by + # ``compiler_notes.append_concept_note``/``append_entity_note`` under + # ``concept_update_mode="append"``) — a no-op on "rewrite"-mode pages, + # which never contain this line shape. + new_text = re.sub( + rf"^- \*\*.*\(\[\[{re.escape(bare_source)}\]\]\)[ \t]*\n?", + "", + new_text, + flags=re.MULTILINE, + ) + if sources_empty and not keep_empty: path.unlink() deleted.append(path.stem) @@ -1603,6 +1617,7 @@ async def _compile_concepts( doc_type: str = "short", rewrite_summary: bool = False, entity_types: list[str] | None = None, + concept_update_mode: str = "rewrite", bundle=None, ) -> None: """Shared Steps 2-4: concepts plan → generate/update → index. @@ -1613,6 +1628,14 @@ async def _compile_concepts( written to disk. When ``rewrite_summary=True`` (short-doc path), the summary is rewritten by the LLM after concepts are finalized so its wikilinks reflect the actual concept pages on disk. + + ``concept_update_mode`` (see ``openkb.config.resolve_concept_update_mode``) + controls how EXISTING concept/entity pages absorb this document: the + default ``"rewrite"`` sends the full page back to the LLM for a rewrite; + ``"append"`` generates a short note instead (the LLM never sees the + existing page) and appends it via ``openkb.agent.compiler_notes`` — no + LLM call for the write itself. New pages are generated the same way in + both modes for "rewrite" (full content) vs. a note for "append". """ source_file = f"summaries/{doc_name}.md" @@ -1963,17 +1986,140 @@ async def _gen_entity_update(ent: dict) -> tuple[str, str, str, str]: _require_nonempty_content(content, name) return name, content, brief, etype_out + # --- "append" mode closures: a short note instead of a full-page rewrite. + # The LLM never sees the existing page (no existing_content read, no + # known_targets_msg turn — notes stay plain text, see compiler_notes.py). + # Return shapes are IDENTICAL to the four closures above (name, + # content-or-note, is_update-or-brief, brief-or-type), so every downstream + # step (gather, ghost-link stripping, index bookkeeping) is shared between + # modes — only the final disk write branches (see below). + async def _gen_note_create(concept: dict) -> tuple[str, str, bool, str]: + name = concept["name"] + title = concept.get("title", name) + async with semaphore: + raw = await _llm_call_page_async( + model, + [ + system_msg, + doc_msg, # cached (BP1) + summary_msg, # cached (BP2) + { + "role": "user", + "content": compiler_notes._CONCEPT_NOTE_CREATE_USER.format( + title=title, + doc_name=doc_name, + ), + }, + ], + f"concept-note: {name}", + response_format=_JSON_RESPONSE_FORMAT, + bundle=bundle, + ) + description, note = compiler_notes.note_fields(raw) + _require_nonempty_content(note, name) + return name, note, False, description + + async def _gen_note_update(concept: dict) -> tuple[str, str, bool, str]: + name = concept["name"] + title = concept.get("title", name) + async with semaphore: + raw = await _llm_call_page_async( + model, + [ + system_msg, + doc_msg, # cached (BP1) + summary_msg, # cached (BP2) + { + "role": "user", + "content": compiler_notes._CONCEPT_NOTE_UPDATE_USER.format( + title=title, + doc_name=doc_name, + ), + }, + ], + f"concept-note-update: {name}", + response_format=_JSON_RESPONSE_FORMAT, + bundle=bundle, + ) + _, note = compiler_notes.note_fields(raw) + _require_nonempty_content(note, name) + return name, note, True, "" + + async def _gen_entity_note_create(ent: dict) -> tuple[str, str, str, str]: + name = ent["name"] + title = ent.get("title", name) + etype = ent.get("type", "other") + async with semaphore: + raw = await _llm_call_page_async( + model, + [ + system_msg, + doc_msg, # cached (BP1) + summary_msg, # cached (BP2) + { + "role": "user", + "content": compiler_notes._ENTITY_NOTE_CREATE_USER.format( + title=title, + type=etype, + doc_name=doc_name, + ), + }, + ], + f"entity-note: {name}", + response_format=_JSON_RESPONSE_FORMAT, + bundle=bundle, + ) + description, note = compiler_notes.note_fields(raw) + _require_nonempty_content(note, name) + return name, note, description, etype + + async def _gen_entity_note_update(ent: dict) -> tuple[str, str, str, str]: + name = ent["name"] + title = ent.get("title", name) + etype = ent.get("type", "other") + async with semaphore: + raw = await _llm_call_page_async( + model, + [ + system_msg, + doc_msg, # cached (BP1) + summary_msg, # cached (BP2) + { + "role": "user", + "content": compiler_notes._ENTITY_NOTE_UPDATE_USER.format( + title=title, + type=etype, + doc_name=doc_name, + ), + }, + ], + f"entity-note-update: {name}", + response_format=_JSON_RESPONSE_FORMAT, + bundle=bundle, + ) + _, note = compiler_notes.note_fields(raw) + _require_nonempty_content(note, name) + return name, note, "", etype + tasks = [] - tasks.extend(_gen_create(c) for c in create_items) - tasks.extend(_gen_update(c) for c in update_items) + if concept_update_mode == "append": + tasks.extend(_gen_note_create(c) for c in create_items) + tasks.extend(_gen_note_update(c) for c in update_items) + else: + tasks.extend(_gen_create(c) for c in create_items) + tasks.extend(_gen_update(c) for c in update_items) # --- Step 3 (entities): build the entity task list up front so it can be # gathered concurrently with the concept tasks below. Entity coroutines # return 4-arity tuples (name, content, brief, type), so their results are # processed in their own loop rather than mixed with the concept tuples. entity_tasks = [] - entity_tasks.extend(_gen_entity_create(e) for e in entity_create) - entity_tasks.extend(_gen_entity_update(e) for e in entity_update) + if concept_update_mode == "append": + entity_tasks.extend(_gen_entity_note_create(e) for e in entity_create) + entity_tasks.extend(_gen_entity_note_update(e) for e in entity_update) + else: + entity_tasks.extend(_gen_entity_create(e) for e in entity_create) + entity_tasks.extend(_gen_entity_update(e) for e in entity_update) concept_names: list[str] = [] concept_briefs_map: dict[str, str] = {} @@ -2063,7 +2209,12 @@ async def _gen_entity_update(ent: dict) -> tuple[str, str, str, str]: ) safe = _sanitize_concept_name(name) is_update = (wiki_dir / "entities" / f"{safe}.md").exists() - _write_entity(wiki_dir, name, cleaned, source_file, is_update, brief=brief, type_=etype) + if concept_update_mode == "append": + compiler_notes.append_entity_note( + wiki_dir, name, cleaned, source_file, doc_name, description=brief, type_=etype + ) + else: + _write_entity(wiki_dir, name, cleaned, source_file, is_update, brief=brief, type_=etype) entity_names.append(safe) entity_meta[safe] = (etype, brief) @@ -2154,14 +2305,19 @@ async def _gen_entity_update(ent: dict) -> tuple[str, str, str, str]: # --- Write concept pages to disk --- for name, page_content, is_update, brief in pending_writes: - _write_concept( - wiki_dir, - name, - page_content, - source_file, - is_update, - brief=brief, - ) + if concept_update_mode == "append": + compiler_notes.append_concept_note( + wiki_dir, name, page_content, source_file, doc_name, description=brief + ) + else: + _write_concept( + wiki_dir, + name, + page_content, + source_file, + is_update, + brief=brief, + ) # --- Step 3b: Process related items (code only, no LLM) --- sanitized_related = [_sanitize_concept_name(s) for s in related_items] @@ -2215,7 +2371,7 @@ async def compile_short_doc( Step 1: Build base context A (schema + doc content), generate summary. Steps 2-4: Delegated to ``_compile_concepts``. """ - from openkb.config import resolve_effective_config + from openkb.config import resolve_concept_update_mode, resolve_effective_config config = resolve_effective_config(kb_dir)[0] language: str = config.get("language", "en") @@ -2280,6 +2436,7 @@ async def compile_short_doc( doc_type="short", rewrite_summary=True, entity_types=entity_types, + concept_update_mode=resolve_concept_update_mode(config), bundle=bundle, ) finally: @@ -2303,7 +2460,7 @@ async def compile_long_doc( The summary page is already written by the indexer. This function generates concept pages and updates the index. """ - from openkb.config import resolve_effective_config + from openkb.config import resolve_concept_update_mode, resolve_effective_config config = resolve_effective_config(kb_dir)[0] language: str = config.get("language", "en") @@ -2364,6 +2521,7 @@ async def compile_long_doc( doc_brief=doc_description, doc_type="pageindex", entity_types=entity_types, + concept_update_mode=resolve_concept_update_mode(config), bundle=bundle, ) finally: diff --git a/openkb/agent/compiler_notes.py b/openkb/agent/compiler_notes.py new file mode 100644 index 000000000..5e8f9a2b3 --- /dev/null +++ b/openkb/agent/compiler_notes.py @@ -0,0 +1,257 @@ +"""Append-only note ingest for OpenKB's wiki compiler (``concept_update_mode="append"``). + +Companion to ``openkb.agent.compiler``: instead of sending an existing concept/ +entity page's full body back to the LLM for a rewrite (``_gen_update``/ +``_gen_entity_update`` in ``compiler.py``), this module generates a short note +about the new document — the LLM never sees the existing page — and appends +it deterministically (no LLM call for the write itself) as a dated, +source-linked line under a ``## Notes`` heading in the same page. Reconciling +the accumulated notes back into curated prose is a separate, future concern — +out of scope here. +""" + +from __future__ import annotations + +import datetime +import json +import logging +import re +from pathlib import Path + +from openkb import frontmatter +from openkb.locks import atomic_write_text + +logger = logging.getLogger(__name__) + +_NOTES_HEADING = "## Notes" + +# --------------------------------------------------------------------------- +# Prompt templates — deliberately slim: no existing page content, no wikilink +# whitelist (notes stay plain text; see module docstring / SKILL.md for why). +# --------------------------------------------------------------------------- + +_CONCEPT_NOTE_CREATE_USER = """\ +This is a NEW concept page: {title} + +This concept was just identified in document "{doc_name}" (summarized above). + +Return a JSON object with two keys: +- "description": A single sentence (under 100 chars) defining this concept +- "note": 1-2 short sentences (or just a few keywords if that's enough) \ +capturing what THIS document says about {title} — it will be appended to a \ +running list of notes, not written as prose. Do NOT use [[wikilinks]]. + +Return ONLY valid JSON, no fences. +""" + +_CONCEPT_NOTE_UPDATE_USER = """\ +Concept page: {title} + +Document "{doc_name}" (summarized above) mentions this concept. + +Return a JSON object with one key: +- "note": 1-2 short sentences (or just a few keywords if that's enough) \ +capturing what THIS document adds about {title} — it will be appended to a \ +running list of notes, not merged into the existing page (which you do not \ +see). Do NOT use [[wikilinks]]. + +Return ONLY valid JSON, no fences. +""" + +_ENTITY_NOTE_CREATE_USER = """\ +This is a NEW entity page: {title} (type: {type}) + +This entity was just identified in document "{doc_name}" (summarized above). + +Return a JSON object with two keys: +- "description": A single sentence (under 100 chars) identifying this entity +- "note": 1-2 short sentences (or just a few keywords if that's enough) \ +capturing what THIS document says about {title} — it will be appended to a \ +running list of notes, not written as prose. Do NOT use [[wikilinks]]. + +Return ONLY valid JSON, no fences. +""" + +_ENTITY_NOTE_UPDATE_USER = """\ +Entity page: {title} (type: {type}) + +Document "{doc_name}" (summarized above) mentions this entity. + +Return a JSON object with one key: +- "note": 1-2 short sentences (or just a few keywords if that's enough) \ +capturing what THIS document adds about {title} — it will be appended to a \ +running list of notes, not merged into the existing page (which you do not \ +see). Do NOT use [[wikilinks]]. + +Return ONLY valid JSON, no fences. +""" + + +def note_fields(raw: str) -> tuple[str, str]: + """Map a note LLM response to ``(description, note)``. + + Mirrors ``compiler._page_fields`` for the smaller note shape: not-JSON + responses fall back to using the raw text as the note itself (a model + that ignores the JSON instruction still produces a usable short note). + """ + from openkb.agent import compiler as _compiler # local: avoid import cycle + + try: + obj = _compiler._parse_page_json(raw) + except (json.JSONDecodeError, ValueError): + return "", raw.strip() + if obj is None: + return "", "" + return obj.get("description", ""), (obj.get("note") or "").strip() + + +def _upsert_note_line(body: str, doc_name: str, note: str, heading: str = _NOTES_HEADING) -> str: + """Insert/replace the note line for ``doc_name`` right after ``heading``. + + Keyed by the ``[[summaries/{doc_name}]]`` source marker (not the note + text), so re-ingesting an updated version of the same document replaces + its own line instead of accumulating duplicates. New/replaced lines land + directly after the heading — newest first, matching the ``sources:`` + frontmatter convention. + """ + marker = f"[[summaries/{doc_name}]]" + date = datetime.date.today().isoformat() + line = f"- **{date}** {note} ({marker})" + + lines = body.split("\n") + line_re = re.compile(rf"^- \*\*.*\({re.escape(marker)}\)\s*$") + lines = [ln for ln in lines if not line_re.match(ln)] + + heading_idx = next((i for i, ln in enumerate(lines) if ln.strip() == heading), None) + if heading_idx is None: + while lines and lines[-1].strip() == "": + lines.pop() + if lines: + lines.append("") + lines.append(heading) + lines.append("") + lines.append(line) + else: + insert_at = heading_idx + 1 + if insert_at < len(lines) and lines[insert_at].strip() == "": + insert_at += 1 + lines.insert(insert_at, line) + + return "\n".join(lines) + + +def _build_frontmatter(fm_lines: list[str]) -> str: + """Build a fresh frontmatter block (delimiters + trailing blank line).""" + return "---\n" + "\n".join(fm_lines) + "\n---\n\n" + + +def append_concept_note( + wiki_dir: Path, + name: str, + note: str, + source_file: str, + doc_name: str, + description: str = "", +) -> None: + """Append a short note about ``doc_name`` to a concept page (no LLM write). + + Creates the page (with ``description`` in its frontmatter, if given) when + it doesn't exist yet; on an existing page, only ``sources:`` is updated + and a note line is upserted — ``description`` is never touched once set. + """ + from openkb.agent import compiler as _compiler # local: avoid import cycle + + concepts_dir = wiki_dir / "concepts" + concepts_dir.mkdir(parents=True, exist_ok=True) + safe_name = _compiler._sanitize_concept_name(name) + path = (concepts_dir / f"{safe_name}.md").resolve() + if not path.is_relative_to(concepts_dir.resolve()): + logger.warning("Concept name escapes concepts dir: %s", name) + return + + if path.exists(): + existing = path.read_text(encoding="utf-8") + if source_file not in existing: + existing = _compiler._prepend_source_to_frontmatter(existing, source_file) + parts = frontmatter.split(existing) + if parts is not None: + fm_block, body = parts + new_body = _upsert_note_line(body.lstrip("\n"), doc_name, note) + atomic_write_text(path, fm_block + "\n" + new_body) + else: + # Malformed/absent frontmatter: rebuild rather than write a bare + # body (mirrors compiler._write_concept's recovery path). + fm_block = _build_frontmatter( + [ + frontmatter.kv_line("type", "Concept"), + frontmatter.list_line("sources", [source_file]), + ] + ) + new_body = _upsert_note_line(existing, doc_name, note) + atomic_write_text(path, fm_block + new_body) + return + + fm_lines = [ + frontmatter.kv_line("type", "Concept"), + frontmatter.list_line("sources", [source_file]), + ] + if description: + fm_lines.append(frontmatter.kv_line("description", description)) + body = _upsert_note_line(f"{_NOTES_HEADING}\n\n", doc_name, note) + atomic_write_text(path, _build_frontmatter(fm_lines) + body) + + +def append_entity_note( + wiki_dir: Path, + name: str, + note: str, + source_file: str, + doc_name: str, + description: str = "", + type_: str = "other", +) -> None: + """Append a short note about ``doc_name`` to an entity page (no LLM write). + + Mirrors :func:`append_concept_note`; ``type_`` is only used to seed a new + page's frontmatter (no re-classification on update, unlike the rewrite + path's ``_gen_entity_update``) — a deliberate scope simplification for the + append mode. + """ + from openkb.agent import compiler as _compiler # local: avoid import cycle + + entities_dir = wiki_dir / "entities" + entities_dir.mkdir(parents=True, exist_ok=True) + safe_name = _compiler._sanitize_concept_name(name) + path = (entities_dir / f"{safe_name}.md").resolve() + if not path.is_relative_to(entities_dir.resolve()): + logger.warning("Entity name escapes entities dir: %s", name) + return + + if path.exists(): + existing = path.read_text(encoding="utf-8") + if source_file not in existing: + existing = _compiler._prepend_source_to_frontmatter(existing, source_file) + parts = frontmatter.split(existing) + if parts is not None: + fm_block, body = parts + new_body = _upsert_note_line(body.lstrip("\n"), doc_name, note) + atomic_write_text(path, fm_block + "\n" + new_body) + else: + fm_block = _build_frontmatter( + [ + frontmatter.list_line("sources", [source_file]), + frontmatter.kv_line("type", (type_ or "other").title()), + ] + ) + new_body = _upsert_note_line(existing, doc_name, note) + atomic_write_text(path, fm_block + new_body) + return + + fm_lines = [ + frontmatter.list_line("sources", [source_file]), + frontmatter.kv_line("type", (type_ or "other").title()), + ] + if description: + fm_lines.append(frontmatter.kv_line("description", description)) + body = _upsert_note_line(f"{_NOTES_HEADING}\n\n", doc_name, note) + atomic_write_text(path, _build_frontmatter(fm_lines) + body) diff --git a/openkb/config.py b/openkb/config.py index 95ca8691f..46173c840 100644 --- a/openkb/config.py +++ b/openkb/config.py @@ -36,8 +36,14 @@ # global/KB list overrides it wholesale; resolve_entity_types cleans the # effective value on read. "entity_types": list(DEFAULT_ENTITY_TYPES), + # How concept/entity pages absorb new documents on `openkb add` — see + # resolve_concept_update_mode(). KB config.yaml only (like `debug`), not + # in GLOBAL_SCALAR_KEYS. + "concept_update_mode": "rewrite", } +VALID_CONCEPT_UPDATE_MODES: tuple[str, ...] = ("rewrite", "append") + GLOBAL_CONFIG_DIR = Path.home() / ".config" / "openkb" GLOBAL_CONFIG_PATH = GLOBAL_CONFIG_DIR / "global.yaml" GLOBAL_CONFIG_LOCK_PATH = GLOBAL_CONFIG_DIR / "global.lock" @@ -140,6 +146,23 @@ def resolve_entity_types(config: dict, *, warn: bool = True) -> list[str]: return cleaned +def resolve_concept_update_mode(config: dict) -> str: + """Resolve ``concept_update_mode:`` — ``"rewrite"`` (default, full-page + LLM rewrite on update) or ``"append"`` (short note instead, see + ``openkb.agent.compiler_notes``). Invalid values degrade to ``"rewrite"`` + with a warning. + """ + value = config.get("concept_update_mode", "rewrite") + if value not in VALID_CONCEPT_UPDATE_MODES: + logger.warning( + "config: 'concept_update_mode' must be one of %s, got %r — using 'rewrite'.", + VALID_CONCEPT_UPDATE_MODES, + value, + ) + return "rewrite" + return value + + def resolve_extra_headers(config: dict) -> dict[str, str]: """Resolve the optional ``extra_headers:`` config key into a str→str dict. diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 95a57cc4c..beabf65d9 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -2252,6 +2252,28 @@ def test_strips_standalone_see_also_line(self, tmp_path): assert "See also" not in shared assert "summaries/other" in shared + def test_strips_append_mode_note_line_keeps_other_notes(self, tmp_path): + # A "concept_update_mode=append" page (compiler_notes.append_entity_note) + # keeps its notes under "## Notes", one dated line per source doc. + # Removing one doc must strip only ITS line, not the whole section. + ent = tmp_path / "entities" + ent.mkdir() + (ent / "shared.md").write_text( + "---\ntype: organization\nsources: [summaries/doc.md, summaries/other.md]\n---\n\n" + "## Notes\n\n" + "- **2026-09-07** Mentioned in doc. ([[summaries/doc]])\n" + "- **2026-09-01** Mentioned in other. ([[summaries/other]])\n", + encoding="utf-8", + ) + result = remove_doc_from_entity_pages(tmp_path, "doc") + assert result == {"modified": ["shared"], "deleted": []} + shared = (ent / "shared.md").read_text(encoding="utf-8") + assert "summaries/doc" not in shared + assert "Mentioned in doc." not in shared + assert "## Notes" in shared + assert "Mentioned in other." in shared + assert "summaries/other" in shared + class TestCompileEntitiesEndToEnd: @pytest.mark.asyncio diff --git a/tests/test_compiler_notes.py b/tests/test_compiler_notes.py new file mode 100644 index 000000000..23d25803f --- /dev/null +++ b/tests/test_compiler_notes.py @@ -0,0 +1,172 @@ +"""Tests for openkb.agent.compiler_notes (concept_update_mode="append").""" + +from __future__ import annotations + +from openkb.agent.compiler_notes import ( + _NOTES_HEADING, + _upsert_note_line, + append_concept_note, + append_entity_note, + note_fields, +) + + +class TestNoteFields: + def test_parses_description_and_note(self): + raw = '{"description": "A greeting", "note": "Says hello."}' + assert note_fields(raw) == ("A greeting", "Says hello.") + + def test_update_shape_has_no_description(self): + raw = '{"note": "Adds a detail."}' + assert note_fields(raw) == ("", "Adds a detail.") + + def test_non_json_falls_back_to_raw_text_as_note(self): + assert note_fields(" Just a plain note. ") == ("", "Just a plain note.") + + def test_malformed_shape_returns_empty(self): + # A JSON array of scalars is valid JSON but not a usable object. + assert note_fields("[1, 2, 3]") == ("", "") + + +class TestUpsertNoteLine: + def test_seeds_heading_and_inserts_first_note(self): + body = _upsert_note_line(f"{_NOTES_HEADING}\n\n", "jira-1", "First note.") + expected = f"{_NOTES_HEADING}\n\n- **{_today()}** First note. ([[summaries/jira-1]])" + assert body.rstrip("\n") == expected + + def test_second_doc_inserted_above_first(self): + body = _upsert_note_line(f"{_NOTES_HEADING}\n\n", "jira-1", "First note.") + body = _upsert_note_line(body, "jira-2", "Second note.") + lines = body.split("\n") + note_lines = [ln for ln in lines if ln.startswith("- **")] + assert len(note_lines) == 2 + assert "jira-2" in note_lines[0] # newest first + assert "jira-1" in note_lines[1] + + def test_reingesting_same_doc_replaces_not_duplicates(self): + body = _upsert_note_line(f"{_NOTES_HEADING}\n\n", "jira-1", "Old text.") + body = _upsert_note_line(body, "jira-2", "Unrelated.") + body = _upsert_note_line(body, "jira-1", "Updated text.") + note_lines = [ln for ln in body.split("\n") if ln.startswith("- **")] + assert len(note_lines) == 2 + assert any("Updated text." in ln for ln in note_lines) + assert not any("Old text." in ln for ln in note_lines) + + def test_creates_missing_heading(self): + body = _upsert_note_line("Some unrelated body.", "jira-1", "A note.") + assert body.split("\n") == [ + "Some unrelated body.", + "", + _NOTES_HEADING, + "", + f"- **{_today()}** A note. ([[summaries/jira-1]])", + ] + + +class TestAppendConceptNote: + def test_creates_new_page_with_description(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + append_concept_note( + wiki, + "approval-workflows", + "Customer reports a timeout.", + "summaries/jira-1.md", + "jira-1", + description="How approvals are routed.", + ) + path = wiki / "concepts" / "approval-workflows.md" + text = path.read_text(encoding="utf-8") + assert 'type: "Concept"' in text + assert 'sources: ["summaries/jira-1.md"]' in text + assert 'description: "How approvals are routed."' in text + assert _NOTES_HEADING in text + assert "Customer reports a timeout." in text + assert "[[summaries/jira-1]]" in text + + def test_creates_new_page_without_description(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + append_concept_note(wiki, "approval-workflows", "A note.", "summaries/jira-1.md", "jira-1") + text = (wiki / "concepts" / "approval-workflows.md").read_text(encoding="utf-8") + assert "description:" not in text + + def test_update_appends_source_and_note_keeps_description(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + append_concept_note( + wiki, + "approval-workflows", + "First ticket note.", + "summaries/jira-1.md", + "jira-1", + description="Original description.", + ) + append_concept_note( + wiki, + "approval-workflows", + "Second ticket note.", + "summaries/jira-2.md", + "jira-2", + description="Ignored — page already exists.", + ) + text = (wiki / "concepts" / "approval-workflows.md").read_text(encoding="utf-8") + assert '"summaries/jira-1.md"' in text + assert '"summaries/jira-2.md"' in text + assert "First ticket note." in text + assert "Second ticket note." in text + # description is frozen at first creation, never overwritten. + assert 'description: "Original description."' in text + assert "Ignored" not in text + + def test_reingest_same_doc_replaces_note_not_source_duplicate(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + append_concept_note( + wiki, "approval-workflows", "Old note.", "summaries/jira-1.md", "jira-1" + ) + append_concept_note( + wiki, "approval-workflows", "Updated note.", "summaries/jira-1.md", "jira-1" + ) + text = (wiki / "concepts" / "approval-workflows.md").read_text(encoding="utf-8") + assert text.count("summaries/jira-1.md") == 1 # sources: list stays deduped + assert "Updated note." in text + assert "Old note." not in text + + +class TestAppendEntityNote: + def test_creates_new_page_with_capitalized_type(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + append_entity_note( + wiki, + "acme-corp", + "Mentioned as the customer.", + "summaries/jira-1.md", + "jira-1", + description="A customer organization.", + type_="organization", + ) + text = (wiki / "entities" / "acme-corp.md").read_text(encoding="utf-8") + assert 'type: "Organization"' in text + assert 'description: "A customer organization."' in text + assert "Mentioned as the customer." in text + + def test_update_keeps_original_type_regardless_of_new_value(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + append_entity_note( + wiki, "acme-corp", "First note.", "summaries/jira-1.md", "jira-1", type_="organization" + ) + append_entity_note( + wiki, "acme-corp", "Second note.", "summaries/jira-2.md", "jira-2", type_="person" + ) + text = (wiki / "entities" / "acme-corp.md").read_text(encoding="utf-8") + assert 'type: "Organization"' in text + assert "Person" not in text + + +def _today() -> str: + import datetime + + return datetime.date.today().isoformat() diff --git a/tests/test_config.py b/tests/test_config.py index 65572d6b9..8254d8f68 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -12,6 +12,7 @@ kb_root_dir, load_config, registered_kbs, + resolve_concept_update_mode, resolve_concurrency, resolve_effective_config, resolve_extra_headers, @@ -145,6 +146,35 @@ def test_default_config_values(): assert DEFAULT_CONFIG["pageindex_threshold"] == 20 +# --- concept_update_mode ------------------------------------------------------- + + +def test_concept_update_mode_default_in_config(): + assert DEFAULT_CONFIG["concept_update_mode"] == "rewrite" + + +def test_concept_update_mode_not_in_global_scalar_keys(): + # KB config.yaml only (like `debug`/`insert_mode`) — not workbench/global- + # editable, so it must never leak into the global.yaml layering. + assert "concept_update_mode" not in GLOBAL_SCALAR_KEYS + + +def test_resolve_concept_update_mode_absent_is_default(): + assert resolve_concept_update_mode({}) == "rewrite" + + +def test_resolve_concept_update_mode_valid_values(): + assert resolve_concept_update_mode({"concept_update_mode": "rewrite"}) == "rewrite" + assert resolve_concept_update_mode({"concept_update_mode": "append"}) == "append" + + +def test_resolve_concept_update_mode_rejects_invalid(caplog): + with caplog.at_level(logging.WARNING, logger="openkb.config"): + result = resolve_concept_update_mode({"concept_update_mode": "bogus"}) + assert result == "rewrite" + assert "concept_update_mode" in caplog.text + + def test_concurrency_not_in_default_config(): # Like the other optional tuning knobs (timeout, extra_headers, # parallel_tool_calls), concurrency stays out of DEFAULT_CONFIG — From 9081134dae6f0570f4ad76769ca9159b2ee21b48 Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Mon, 7 Sep 2026 11:28:54 +0200 Subject: [PATCH 2/8] feat(agent): add `openkb consolidate` to fold pending notes into prose (#245) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new `openkb consolidate [PAGE_NAME] [--all] [--min-notes N] [--dry-run] [--yes]` command (mirrors `recompile`'s CLI shape) and a new `openkb/agent/consolidator.py` module. For concept/entity pages accumulating notes under `concept_update_mode: append`, this folds the pending "## Notes" section into curated prose with a single LLM call per page — no new source document, no concept/entity classification, since the page is already fixed. - Contradictions between notes (or notes vs. existing prose) are described directly in the rewritten text rather than silently resolved. - The full wikilink whitelist is sent here (once per page per run, not per ticket) so consolidated prose can properly cross-link to other pages. - The "## Notes" section is replaced entirely; `sources:`/`type:` are preserved, only `description:` may be refreshed. A later `append_*_note` call re-creates a fresh "## Notes" section, so the next consolidation run only ever sees what changed since the last one — no extra tracking state needed. - Manual, opt-in only (no automatic trigger during ingest). --- openkb/agent/consolidator.py | 226 ++++++++++++++++++++++++++++++++++ openkb/cli.py | 128 +++++++++++++++++++ tests/test_consolidate_cli.py | 107 ++++++++++++++++ tests/test_consolidator.py | 166 +++++++++++++++++++++++++ 4 files changed, 627 insertions(+) create mode 100644 openkb/agent/consolidator.py create mode 100644 tests/test_consolidate_cli.py create mode 100644 tests/test_consolidator.py diff --git a/openkb/agent/consolidator.py b/openkb/agent/consolidator.py new file mode 100644 index 000000000..00b448c0e --- /dev/null +++ b/openkb/agent/consolidator.py @@ -0,0 +1,226 @@ +"""Consolidate a concept/entity page's accumulated "## Notes" into prose. + +Companion to ``openkb.agent.compiler_notes`` (``concept_update_mode="append"``): +once a page has accumulated one or more notes, ``openkb consolidate`` folds +them into the page's existing prose with a single LLM call — no new source +document, no concept/entity classification step, since the page is already +fixed. Contradictions between notes (or between notes and existing prose) are +described directly in the rewritten text rather than silently resolved. The +"## Notes" section is replaced entirely: after consolidation the page reads +like an ordinary ``concept_update_mode="rewrite"`` page — new notes appended +later (see ``compiler_notes``) start a fresh "## Notes" section, so the next +consolidation run only ever sees what changed since the last one. +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +from openkb import frontmatter +from openkb.agent.compiler_notes import _NOTES_HEADING +from openkb.lint import list_existing_wiki_targets, strip_ghost_wikilinks +from openkb.locks import atomic_write_text +from openkb.schema import get_agents_md + +logger = logging.getLogger(__name__) + +_CONSOLIDATE_CONCEPT_USER = """\ +Consolidate the concept page: {title} + +Existing prose on this page (may be empty if this is the first consolidation): +{existing_content} + +Accumulated notes to fold in, newest first (each tied to a source document): +{notes_content} + +Rewrite the ENTIRE page as a single, coherent Markdown page that: +- Preserves every distinct fact from both the existing prose and the notes. +- If notes conflict with each other or with the existing prose, describe the \ +conflict directly in the text (which source said what, and when) instead of \ +silently picking one side. +- Uses [[wikilinks]] to related concepts/entities, per the whitelist message \ +above. +- Does NOT include a "## Notes" section or any raw note lines — fold their \ +content into the prose instead. + +Return a JSON object with two keys: +- "description": A single sentence (under 100 chars) defining this concept +- "content": The rewritten full concept page in Markdown + +Return ONLY valid JSON, no fences. +""" + +_CONSOLIDATE_ENTITY_USER = """\ +Consolidate the entity page: {title} (type: {type}) + +Existing prose on this page (may be empty if this is the first consolidation): +{existing_content} + +Accumulated notes to fold in, newest first (each tied to a source document): +{notes_content} + +Rewrite the ENTIRE page as a single, coherent Markdown page that: +- Preserves every distinct fact from both the existing prose and the notes. +- If notes conflict with each other or with the existing prose, describe the \ +conflict directly in the text (which source said what, and when) instead of \ +silently picking one side. +- Uses [[wikilinks]] to related concepts/entities, per the whitelist message \ +above. +- Does NOT include a "## Notes" section or any raw note lines — fold their \ +content into the prose instead. + +Return a JSON object with two keys: +- "description": A single sentence (under 100 chars) identifying this entity +- "content": The rewritten full entity page in Markdown + +Return ONLY valid JSON, no fences. +""" + + +def _title_from_slug(slug: str) -> str: + return slug.replace("-", " ").title() + + +def _split_notes(body: str) -> tuple[str, str]: + """Split a page body into ``(existing_prose, notes_content)``. + + ``notes_content`` is empty when there is no ``## Notes`` heading (nothing + pending) — callers treat that as "skip, no notes to consolidate". + """ + lines = body.split("\n") + idx = next((i for i, ln in enumerate(lines) if ln.strip() == _NOTES_HEADING), None) + if idx is None: + return body.strip(), "" + existing = "\n".join(lines[:idx]).strip() + notes = "\n".join(lines[idx + 1 :]).strip() + return existing, notes + + +def count_pending_notes(text: str) -> int: + """Count ``## Notes`` bullet lines in a page's raw text (0 if none).""" + _, notes = _split_notes(text) + if not notes: + return 0 + return sum(1 for ln in notes.split("\n") if ln.lstrip().startswith("- **")) + + +def find_consolidation_candidates(wiki_dir: Path, min_notes: int = 1) -> list[tuple[str, str, int]]: + """Return ``(page_dir, slug, note_count)`` for pages with pending notes. + + ``page_dir`` is ``"concepts"`` or ``"entities"``. Only pages with at least + ``min_notes`` pending note lines are included. + """ + candidates: list[tuple[str, str, int]] = [] + for page_dir in ("concepts", "entities"): + dir_path = wiki_dir / page_dir + if not dir_path.is_dir(): + continue + for path in sorted(dir_path.glob("*.md")): + count = count_pending_notes(path.read_text(encoding="utf-8")) + if count >= min_notes: + candidates.append((page_dir, path.stem, count)) + return candidates + + +def resolve_page(wiki_dir: Path, name: str) -> list[tuple[str, str]]: + """Resolve ``name`` to ``[(page_dir, slug)]`` matches (exact slug first). + + Returns an empty list when nothing matches, or more than one entry when + ``name`` is an ambiguous substring across concepts/entities — callers + decide how to report each case. + """ + for page_dir in ("concepts", "entities"): + if (wiki_dir / page_dir / f"{name}.md").exists(): + return [(page_dir, name)] + + matches: list[tuple[str, str]] = [] + for page_dir in ("concepts", "entities"): + dir_path = wiki_dir / page_dir + if not dir_path.is_dir(): + continue + for path in sorted(dir_path.glob("*.md")): + if name.lower() in path.stem.lower(): + matches.append((page_dir, path.stem)) + return matches + + +def consolidate_page( + wiki_dir: Path, page_dir: str, slug: str, model: str, language: str = "en" +) -> bool: + """Fold ``page_dir/slug``'s pending notes into curated prose. + + Returns ``False`` (no-op, no LLM call) when the page has no ``## Notes`` + section. Raises on LLM/parse failure — the CLI command treats a raised + exception for one page as a per-page failure, not a whole-batch abort + (mirrors ``recompile``). + """ + from openkb.agent import compiler as _compiler + + path = wiki_dir / page_dir / f"{slug}.md" + text = path.read_text(encoding="utf-8") + parts = frontmatter.split(text) + if parts is None: + logger.warning("Skipping %s/%s: malformed or missing frontmatter.", page_dir, slug) + return False + fm_block, body = parts + existing_content, notes_content = _split_notes(body.lstrip("\n")) + if not notes_content: + return False + + fm = frontmatter.parse(text) + title = _title_from_slug(slug) + known_targets = list_existing_wiki_targets(wiki_dir) + known_targets_str = _compiler._format_known_targets(known_targets) + + system_msg = { + "role": "system", + "content": _compiler._SYSTEM_TEMPLATE.format( + schema_md=get_agents_md(wiki_dir), + language=language, + ), + } + known_targets_msg = { + "role": "user", + "content": _compiler._KNOWN_TARGETS_USER.format(known_targets=known_targets_str), + } + if page_dir == "entities": + etype = fm.get("type", "other") + user_content = _CONSOLIDATE_ENTITY_USER.format( + title=title, + type=etype, + existing_content=existing_content or "(none — first consolidation for this page)", + notes_content=notes_content, + ) + else: + user_content = _CONSOLIDATE_CONCEPT_USER.format( + title=title, + existing_content=existing_content or "(none — first consolidation for this page)", + notes_content=notes_content, + ) + + raw = _compiler._llm_call( + model, + [system_msg, known_targets_msg, {"role": "user", "content": user_content}], + f"consolidate: {page_dir}/{slug}", + response_format=_compiler._JSON_RESPONSE_FORMAT, + ) + description, content, _obj = _compiler._page_fields(raw) + _compiler._require_nonempty_content(content, slug) + + clean_parts = frontmatter.split(content) + clean = clean_parts[1].lstrip("\n") if clean_parts is not None else content + cleaned, ghosts = strip_ghost_wikilinks(clean, known_targets) + if ghosts: + logger.info( + "stripped %d ghost wikilink(s) from consolidated %s/%s: %s", + len(ghosts), + page_dir, + slug, + ghosts[:5], + ) + + if description: + fm_block = frontmatter.set_line(fm_block, "description", description) + atomic_write_text(path, fm_block + "\n" + cleaned) + return True diff --git a/openkb/cli.py b/openkb/cli.py index c9e543181..58032a49e 100644 --- a/openkb/cli.py +++ b/openkb/cli.py @@ -2055,6 +2055,134 @@ def _classify(meta: dict) -> str: append_log(wiki_dir, "recompile", f"recompiled {recompiled}, skipped {skipped}") +@cli.command() +@click.argument("page_name", required=False) +@click.option( + "--all", "all_pages", is_flag=True, default=False, help="Consolidate every pending page." +) +@click.option( + "--min-notes", + type=int, + default=1, + show_default=True, + help="With --all, only consider pages with at least this many pending notes.", +) +@click.option( + "--dry-run", + is_flag=True, + default=False, + help="List the pages that would be consolidated; no LLM calls, no writes.", +) +@click.option( + "--yes", "-y", is_flag=True, default=False, help="Skip the --all confirmation prompt." +) +@click.pass_context +@_with_kb_lock(exclusive=True) +def consolidate(ctx, page_name, all_pages, min_notes, dry_run, yes): + """Fold a concept/entity page's accumulated "## Notes" into curated prose. + + Only relevant under ``concept_update_mode: append`` (see ``openkb add``): + each ingested document appends a short dated note to the concept/entity + pages it touches instead of rewriting them in full. This command folds + those notes into the page's prose with a single LLM call per page — no + new source document, no concept/entity classification, since the page is + already fixed. Contradictions between notes (or between notes and + existing prose) are described directly in the rewritten text rather than + silently resolved. + + PAGE_NAME resolves like ``openkb remove`` — exact slug first, else a + unique substring match across ``wiki/concepts/`` and ``wiki/entities/``. + ``--all`` consolidates every page with at least ``--min-notes`` pending + notes. Exactly one of PAGE_NAME or ``--all`` is required. + + Side effect: this replaces the page's "## Notes" section with prose — + manual edits inside that section are overwritten. Existing prose above + it, ``sources:``, and ``type:`` are preserved; only ``description:`` may + be refreshed. + """ + from openkb.agent import consolidator + + if bool(page_name) == bool(all_pages): + click.echo("Specify exactly one of PAGE_NAME or --all.") + return + + kb_dir = _find_kb_dir(ctx.obj.get("kb_dir_override")) + if kb_dir is None: + click.echo("No knowledge base found. Run `openkb init` first.") + return + wiki_dir = kb_dir / "wiki" + + if page_name: + matches = consolidator.resolve_page(wiki_dir, page_name) + if not matches: + click.echo(f"No concept/entity page matching '{page_name}' found.") + return + if len(matches) > 1: + click.echo(f"'{page_name}' matches multiple pages:") + for page_dir, slug in matches: + click.echo(f" - {page_dir}/{slug}") + return + page_dir, slug = matches[0] + targets = [ + ( + page_dir, + slug, + consolidator.count_pending_notes( + (wiki_dir / page_dir / f"{slug}.md").read_text(encoding="utf-8") + ), + ) + ] + else: + targets = consolidator.find_consolidation_candidates(wiki_dir, min_notes=min_notes) + if not targets: + click.echo("No pages with pending notes found.") + return + + if dry_run: + click.echo(f"Would consolidate {len(targets)} page(s):") + for page_dir, slug, count in targets: + click.echo(f" - {page_dir}/{slug} ({count} note(s))") + click.echo("(dry-run — nothing modified)") + return + + if all_pages and not yes and len(targets) > 1: + click.echo( + f"This will consolidate {len(targets)} page(s), replacing each " + 'page\'s "## Notes" section with rewritten prose.' + ) + if not click.confirm("Proceed?", default=False): + click.echo("Aborted.") + return + + _setup_llm_key(kb_dir) + config = resolve_effective_config(kb_dir)[0] + model: str = config.get("model", DEFAULT_CONFIG["model"]) + language: str = config.get("language", "en") + + consolidated = 0 + skipped = 0 + total = len(targets) + for i, (page_dir, slug, count) in enumerate(targets, 1): + click.echo(f"[{i}/{total}] Consolidating {page_dir}/{slug} ({count} note(s))...") + start = time.time() + try: + ok = consolidator.consolidate_page(wiki_dir, page_dir, slug, model, language=language) + except Exception as exc: + click.echo(f" [ERROR] Consolidation failed: {exc}") + logging.getLogger(__name__).debug("Consolidate traceback:", exc_info=True) + skipped += 1 + continue + if ok: + click.echo(f" [OK] {page_dir}/{slug} ({time.time() - start:.1f}s)") + consolidated += 1 + else: + click.echo(f" [SKIP] {page_dir}/{slug} (no pending notes).") + skipped += 1 + + click.echo(f"\nDone: consolidated {consolidated}, skipped {skipped}.") + append_log(wiki_dir, "consolidate", f"consolidated {consolidated}, skipped {skipped}") + + async def iter_recompile( kb_dir: Path, doc_name: str | None = None, diff --git a/tests/test_consolidate_cli.py b/tests/test_consolidate_cli.py new file mode 100644 index 000000000..b44741014 --- /dev/null +++ b/tests/test_consolidate_cli.py @@ -0,0 +1,107 @@ +"""Tests for the `openkb consolidate` CLI command.""" + +from __future__ import annotations + +from unittest.mock import patch + +from click.testing import CliRunner + +from openkb.cli import cli + + +def _invoke(kb_dir, args): + return CliRunner().invoke(cli, ["--kb-dir", str(kb_dir), *args]) + + +def _seed_page_with_notes(kb_dir, page_dir="concepts", slug="approval-workflows"): + d = kb_dir / "wiki" / page_dir + d.mkdir(parents=True, exist_ok=True) + (d / f"{slug}.md").write_text( + '---\nsources: ["summaries/a.md"]\n---\n\nExisting prose.\n\n## Notes\n\n' + "- **2026-01-01** A note. ([[summaries/a]])\n", + encoding="utf-8", + ) + (kb_dir / "wiki" / "log.md").write_text("# Log\n\n", encoding="utf-8") + + +class TestConsolidateArgValidation: + def test_requires_exactly_one_of_name_or_all(self, kb_dir): + _seed_page_with_notes(kb_dir) + result = _invoke(kb_dir, ["consolidate"]) + assert result.exit_code == 0 + assert "exactly one" in result.output.lower() + + def test_unknown_page_name(self, kb_dir): + _seed_page_with_notes(kb_dir) + result = _invoke(kb_dir, ["consolidate", "nonexistent"]) + assert "No concept/entity page matching" in result.output + + +class TestConsolidateDryRun: + def test_dry_run_lists_candidates_no_calls_no_writes(self, kb_dir): + _seed_page_with_notes(kb_dir) + path = kb_dir / "wiki" / "concepts" / "approval-workflows.md" + before = path.read_text(encoding="utf-8") + with patch("openkb.agent.consolidator.consolidate_page") as mock_consolidate: + result = _invoke(kb_dir, ["consolidate", "--all", "--dry-run"]) + + assert result.exit_code == 0, result.output + mock_consolidate.assert_not_called() + assert "approval-workflows" in result.output + assert "1 note" in result.output + assert path.read_text(encoding="utf-8") == before + + def test_min_notes_filters_dry_run_candidates(self, kb_dir): + _seed_page_with_notes(kb_dir) + with patch("openkb.agent.consolidator.consolidate_page") as mock_consolidate: + result = _invoke(kb_dir, ["consolidate", "--all", "--min-notes", "5", "--dry-run"]) + + assert result.exit_code == 0, result.output + mock_consolidate.assert_not_called() + assert "No pages with pending notes found." in result.output + + +class TestConsolidateExecution: + def test_single_page_by_name_dispatches_consolidate_page(self, kb_dir): + _seed_page_with_notes(kb_dir) + with patch("openkb.agent.consolidator.consolidate_page", return_value=True) as mock_c: + result = _invoke(kb_dir, ["consolidate", "approval-workflows"]) + + assert result.exit_code == 0, result.output + mock_c.assert_called_once() + args = mock_c.call_args.args + assert args[1] == "concepts" + assert args[2] == "approval-workflows" + assert "Done: consolidated 1, skipped 0." in result.output + log_text = (kb_dir / "wiki" / "log.md").read_text(encoding="utf-8") + assert "consolidate" in log_text + + def test_all_with_yes_skips_confirmation(self, kb_dir): + _seed_page_with_notes(kb_dir) + with patch("openkb.agent.consolidator.consolidate_page", return_value=True) as mock_c: + result = _invoke(kb_dir, ["consolidate", "--all", "--yes"]) + + assert result.exit_code == 0, result.output + mock_c.assert_called_once() + assert "Done: consolidated 1, skipped 0." in result.output + + def test_skip_result_counts_as_skipped(self, kb_dir): + _seed_page_with_notes(kb_dir) + with patch("openkb.agent.consolidator.consolidate_page", return_value=False): + result = _invoke(kb_dir, ["consolidate", "approval-workflows"]) + + assert result.exit_code == 0, result.output + assert "Done: consolidated 0, skipped 1." in result.output + + def test_exception_in_one_page_reported_as_error_not_fatal(self, kb_dir): + _seed_page_with_notes(kb_dir, slug="approval-workflows") + _seed_page_with_notes(kb_dir, slug="second-page") + with patch( + "openkb.agent.consolidator.consolidate_page", + side_effect=[ValueError("boom"), True], + ): + result = _invoke(kb_dir, ["consolidate", "--all", "--yes"]) + + assert result.exit_code == 0, result.output + assert "[ERROR] Consolidation failed: boom" in result.output + assert "Done: consolidated 1, skipped 1." in result.output diff --git a/tests/test_consolidator.py b/tests/test_consolidator.py new file mode 100644 index 000000000..fdc0fa0ac --- /dev/null +++ b/tests/test_consolidator.py @@ -0,0 +1,166 @@ +"""Tests for openkb.agent.consolidator.""" + +from __future__ import annotations + +import json +from unittest.mock import MagicMock, patch + +from openkb.agent.consolidator import ( + _split_notes, + consolidate_page, + count_pending_notes, + find_consolidation_candidates, + resolve_page, +) + + +def _mock_completion(response: str): + def side_effect(*args, **kwargs): + mock_resp = MagicMock() + mock_resp.choices = [MagicMock()] + mock_resp.choices[0].message.content = response + mock_resp.usage = MagicMock(prompt_tokens=100, completion_tokens=50) + mock_resp.usage.prompt_tokens_details = None + return mock_resp + + return side_effect + + +def _write_page(wiki, page_dir: str, slug: str, body: str): + d = wiki / page_dir + d.mkdir(parents=True, exist_ok=True) + path = d / f"{slug}.md" + path.write_text(body, encoding="utf-8") + return path + + +class TestSplitNotesAndCount: + def test_no_notes_heading(self): + existing, notes = _split_notes("# Attention\n\nSome prose.") + assert existing == "# Attention\n\nSome prose." + assert notes == "" + + def test_splits_prose_and_notes(self): + body = ( + "# Attention\n\nSome prose.\n\n## Notes\n\n- **2026-01-01** A note. ([[summaries/x]])" + ) + existing, notes = _split_notes(body) + assert existing == "# Attention\n\nSome prose." + assert "A note." in notes + + def test_count_pending_notes_zero_without_heading(self): + assert count_pending_notes("# Attention\n\nSome prose.") == 0 + + def test_count_pending_notes_counts_bullets(self): + text = ( + "---\nsources: [a]\n---\n\n## Notes\n\n" + "- **2026-01-02** Second. ([[summaries/b]])\n" + "- **2026-01-01** First. ([[summaries/a]])\n" + ) + assert count_pending_notes(text) == 2 + + +class TestFindConsolidationCandidates: + def test_finds_pages_with_enough_notes(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page( + wiki, + "concepts", + "approval-workflows", + '---\nsources: ["a"]\n---\n\n## Notes\n\n- **2026-01-01** Note. ([[summaries/a]])\n', + ) + _write_page(wiki, "concepts", "no-notes", '---\nsources: ["a"]\n---\n\n# Prose only.\n') + candidates = find_consolidation_candidates(wiki, min_notes=1) + assert candidates == [("concepts", "approval-workflows", 1)] + + def test_min_notes_filters_out_thin_pages(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page( + wiki, + "entities", + "acme-corp", + '---\nsources: ["a"]\n---\n\n## Notes\n\n- **2026-01-01** Note. ([[summaries/a]])\n', + ) + assert find_consolidation_candidates(wiki, min_notes=2) == [] + + +class TestResolvePage: + def test_exact_slug_match(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page(wiki, "concepts", "approval-workflows", "---\nsources: []\n---\n\nBody.") + assert resolve_page(wiki, "approval-workflows") == [("concepts", "approval-workflows")] + + def test_no_match(self, tmp_path): + wiki = tmp_path / "wiki" + wiki.mkdir() + assert resolve_page(wiki, "nonexistent") == [] + + def test_ambiguous_substring_match(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page(wiki, "concepts", "approval-workflows", "---\nsources: []\n---\n\nBody.") + _write_page(wiki, "entities", "approval-bot", "---\nsources: []\n---\n\nBody.") + matches = resolve_page(wiki, "approval") + assert set(matches) == {("concepts", "approval-workflows"), ("entities", "approval-bot")} + + +class TestConsolidatePage: + def test_returns_false_without_notes_section(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page(wiki, "concepts", "approval-workflows", '---\nsources: ["a"]\n---\n\nProse.\n') + with patch("openkb.agent.compiler.litellm") as mock_litellm: + mock_litellm.completion = MagicMock(side_effect=AssertionError("should not be called")) + result = consolidate_page(wiki, "concepts", "approval-workflows", "gpt-4o-mini") + assert result is False + + def test_consolidates_and_replaces_notes_section(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page( + wiki, + "concepts", + "approval-workflows", + '---\nsources: ["summaries/a.md", "summaries/b.md"]\ntype: "Concept"\n' + 'description: "Old description"\n---\n\n' + "Existing prose.\n\n## Notes\n\n" + "- **2026-01-02** Second ticket note. ([[summaries/b]])\n" + "- **2026-01-01** First ticket note. ([[summaries/a]])\n", + ) + response = json.dumps( + { + "description": "How approvals are routed and escalated.", + "content": "# Approval Workflows\n\nConsolidated prose covering both tickets.", + } + ) + with patch("openkb.agent.compiler.litellm") as mock_litellm: + mock_litellm.completion = MagicMock(side_effect=_mock_completion(response)) + result = consolidate_page(wiki, "concepts", "approval-workflows", "gpt-4o-mini") + + assert result is True + text = (wiki / "concepts" / "approval-workflows.md").read_text(encoding="utf-8") + assert "## Notes" not in text + assert "Consolidated prose covering both tickets." in text + assert 'description: "How approvals are routed and escalated."' in text + # sources: untouched by consolidation. + assert '"summaries/a.md"' in text + assert '"summaries/b.md"' in text + + def test_strips_ghost_wikilinks_from_consolidated_content(self, tmp_path): + wiki = tmp_path / "wiki" + _write_page( + wiki, + "concepts", + "approval-workflows", + '---\nsources: ["summaries/a.md"]\n---\n\n## Notes\n\n' + "- **2026-01-01** Note. ([[summaries/a]])\n", + ) + response = json.dumps( + { + "description": "Desc.", + "content": "Mentions [[concepts/nonexistent-page]] which doesn't exist.", + } + ) + with patch("openkb.agent.compiler.litellm") as mock_litellm: + mock_litellm.completion = MagicMock(side_effect=_mock_completion(response)) + consolidate_page(wiki, "concepts", "approval-workflows", "gpt-4o-mini") + + text = (wiki / "concepts" / "approval-workflows.md").read_text(encoding="utf-8") + assert "[[concepts/nonexistent-page]]" not in text From a397e61f833f1f72c895551299e67c45a33c763b Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Mon, 7 Sep 2026 11:43:46 +0200 Subject: [PATCH 3/8] test(compiler): cover concept_update_mode=append through the full async LLM pipeline --- tests/test_compiler.py | 58 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/test_compiler.py b/tests/test_compiler.py index beabf65d9..dc9579530 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -1220,6 +1220,64 @@ async def test_full_pipeline(self, tmp_path): assert "[[summaries/test-doc]]" in index_text assert "[[concepts/transformer]]" in index_text + @pytest.mark.asyncio + async def test_append_mode_full_pipeline_writes_note_not_full_page(self, tmp_path): + """concept_update_mode="append" end-to-end, through the SAME mocked + acompletion path as the "rewrite" pipeline above — proves the note + closures work under this branch's _llm_call_page_async. + """ + wiki = tmp_path / "wiki" + (wiki / "sources").mkdir(parents=True) + (wiki / "summaries").mkdir(parents=True) + (wiki / "concepts").mkdir(parents=True) + (wiki / "index.md").write_text( + "# Index\n\n## Documents\n\n## Concepts\n\n## Explorations\n", + encoding="utf-8", + ) + source_path = wiki / "sources" / "test-doc.md" + source_path.write_text("# Test Doc\n\nA support ticket about approvals.", encoding="utf-8") + (tmp_path / ".openkb").mkdir() + (tmp_path / ".openkb" / "config.yaml").write_text( + "concept_update_mode: append\n", encoding="utf-8" + ) + + summary_response = json.dumps( + {"description": "A ticket about approvals", "content": "# Summary\n\nApproval ticket."} + ) + concepts_plan_response = json.dumps( + { + "create": [{"name": "approval-workflows", "title": "Approval Workflows"}], + "update": [], + "related": [], + } + ) + summary_rewrite_response = ( + "# Summary\n\nApproval ticket about [[concepts/approval-workflows]]." + ) + note_response = json.dumps( + { + "description": "How approvals are routed.", + "note": "This ticket reports a timeout during approval.", + } + ) + + with patch("openkb.agent.compiler.litellm") as mock_litellm: + mock_litellm.completion = MagicMock( + side_effect=_mock_completion( + [summary_response, concepts_plan_response, summary_rewrite_response] + ) + ) + mock_litellm.acompletion = AsyncMock(side_effect=_mock_acompletion([note_response])) + await compile_short_doc("test-doc", source_path, tmp_path, "gpt-4o-mini") + + concept_path = wiki / "concepts" / "approval-workflows.md" + assert concept_path.exists() + text = concept_path.read_text(encoding="utf-8") + assert "## Notes" in text + assert "This ticket reports a timeout during approval." in text + assert 'description: "How approvals are routed."' in text + assert 'sources: ["summaries/test-doc.md"]' in text + @pytest.mark.asyncio async def test_handles_bad_json(self, tmp_path): wiki = tmp_path / "wiki" From 53e39768b2346f3f4f6364b6c8692dad6a5eacad Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Mon, 7 Sep 2026 16:30:29 +0200 Subject: [PATCH 4/8] feat(agent): curb concept/entity sprawl via strict types, name-length gate, and a pending-topics buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: the plan step creates a dedicated wiki page for almost anything proposed on its first mention, including a document's own ticket/case identifier and overly specific one-off names. entity_types only labeled the "type" field and never gated whether an entity got created at all. Changes: - config: opt-in `strict_entity_types` (default false) drops an entity whose type doesn't match entity_types instead of coercing it to "other". - compiler: hard cap of 3 words on brand-new concept/entity names (_count_words), applied to "create" items only. - compiler: __DOC_TOKEN_GUIDANCE__ prompt substitution — a concrete, per-document "propose at most N brand-new items" suggestion computed from the document's real token count (reused from the existing summary/overview call's usage, no extra API call). Purely textual guidance, never enforced in code. - new openkb/pending.py (PendingTopicsStore): a brand-new concept/entity now collects up to 2 short notes across documents before a real page is created on the 3rd mention. Works in both concept_update_mode values — "append" mode promotion reuses the existing note-append writers with no extra LLM call; "rewrite" mode does one enriched create call using the buffered notes as context, then merges all contributing sources. - pending topics are surfaced to the plan call as quasi-existing briefs (for dedup) but never added to the wikilink whitelist. Tests: strict_entity_types resolver, max-words filter (concepts+entities), _doc_token_guidance, PendingTopicsStore unit tests, and two end-to-end lifecycle tests (rewrite/append) proving 3 mentions across documents are needed before a page exists. Updated existing create-path tests for the new buffer-then-promote semantics — 0 regressions against the pre-existing Windows-environment baseline (18 known failures, unchanged). Resolves #247 --- config.yaml.example | 7 + examples/configuration/README.md | 8 + openkb/agent/compiler.py | 486 ++++++++++++++++++++++++++++--- openkb/config.py | 23 ++ openkb/pending.py | 112 +++++++ tests/test_compiler.py | 419 ++++++++++++++++++++++++-- tests/test_config.py | 30 ++ tests/test_file_size.py | 1 + tests/test_pending.py | 117 ++++++++ 9 files changed, 1135 insertions(+), 68 deletions(-) create mode 100644 openkb/pending.py create mode 100644 tests/test_pending.py diff --git a/config.yaml.example b/config.yaml.example index c19e4e7c6..6408aeaec 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -27,6 +27,13 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # - dataset # - model +# Optional: reject an entity whose LLM-returned type doesn't match +# entity_types (above) instead of coercing it to "other". Off by default +# (backward compatible); turn on for a narrow, domain-specific entity_types +# list where an "other"-typed entity usually signals a bad/too-specific +# candidate you'd rather drop than keep. +# strict_entity_types: false + # Optional: how concept/entity pages absorb new documents on `openkb add`. # rewrite default — send the existing page's full body to the LLM and # let it rewrite the whole page to incorporate the new document. diff --git a/examples/configuration/README.md b/examples/configuration/README.md index 25e5b3a85..a337299ea 100644 --- a/examples/configuration/README.md +++ b/examples/configuration/README.md @@ -95,6 +95,13 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # - dataset # - model +# Optional: reject an entity whose LLM-returned type doesn't match +# entity_types (above) instead of coercing it to "other". Off by default +# (backward compatible); turn on for a narrow, domain-specific entity_types +# list where an "other"-typed entity usually signals a bad/too-specific +# candidate you'd rather drop than keep. +# strict_entity_types: false + # Optional: LLM / LiteLLM tuning. Keys are forwarded to LiteLLM; `timeout` and # `extra_headers` apply per request, the rest are set as litellm.. # litellm: @@ -114,6 +121,7 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex | `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. | | `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). | | `entity_types` | 7 defaults | Custom vocabulary for entity pages. `other` is always kept. | +| `strict_entity_types` | `false` | When `true`, drops an entity whose type doesn't match `entity_types` instead of coercing it to `other`. | | `litellm:` | – | A pass-through block for LiteLLM. See below. | ### The `litellm:` block diff --git a/openkb/agent/compiler.py b/openkb/agent/compiler.py index 33463b3f1..c2cf9ea15 100644 --- a/openkb/agent/compiler.py +++ b/openkb/agent/compiler.py @@ -39,6 +39,7 @@ ) from openkb.lint import list_existing_wiki_targets, strip_ghost_wikilinks from openkb.locks import atomic_write_text +from openkb.pending import MAX_NOTES_BEFORE_PROMOTION, PendingTopicsStore from openkb.schema import INDEX_SEED, get_agents_md logger = logging.getLogger(__name__) @@ -87,6 +88,14 @@ _ENTITY_TYPE_LIST = DEFAULT_ENTITY_TYPES _ENTITY_TYPES = frozenset(_ENTITY_TYPE_LIST) +# Hard cap on words in a brand-new concept/entity name (see _count_words) and +# the token-density constant used to compute the soft per-document "how many +# brand-new items" guidance substituted into __DOC_TOKEN_GUIDANCE__. Both are +# intentionally NOT config keys (see issue #247) — the only new config-driven +# knob in this feature is strict_entity_types. +_MAX_NAME_WORDS = 3 +_TOKENS_PER_NEW_ITEM = 1000 + _CONCEPTS_PLAN_USER = """\ Based on the summary above, decide how to update the wiki's CONCEPT pages and @@ -116,11 +125,20 @@ {{"name": "anthropic", "title": "Anthropic", "type": "organization"}} Rules: -- For the first few documents, create 2-3 foundational concepts at most. +- Most of your proposals should be "update" or "related" against the + existing pages listed above — reuse and cross-link what's already there + rather than fragmenting knowledge into new pages. Only propose "create" for + a topic that clearly doesn't fit anything existing yet. +- __DOC_TOKEN_GUIDANCE__ +- Concept and entity names must be short and general — at most 3 words. + Never use a unique identifier, hash, or ticket/case number as a name, and + avoid overly specific combinations (e.g. a person's name plus their role in + this one case, or a technology plus one specific incident path). If a + candidate doesn't reduce to a short, general, reusable phrase, do not + propose it. - Create an ENTITY page only when the entity is (a) central to this document or (b) likely to recur across sources. Do NOT page proper nouns mentioned - only in passing. Roughly 5-15 entities per document is typical; fewer for - sparse documents. + only in passing. - Prefer "update" over "create" for any concept or entity already listed above. - Do NOT create a concept/entity that overlaps an existing one — use "update". - Do NOT create concepts that are just the document topic itself. @@ -186,6 +204,7 @@ Write the entity page for: {title} (type: {type}) This entity relates to the document "{doc_name}" summarized above. +{update_instruction} Return a JSON object with three keys: - "description": A single sentence (under 100 chars) identifying this entity @@ -224,6 +243,11 @@ # ``.openkb/config.yaml`` override the default enum everywhere at once. The # token is a plain string (not a ``{}`` placeholder) so it does not collide with # the ``{{ }}`` JSON braces these templates feed to ``str.format``. +# +# ``__DOC_TOKEN_GUIDANCE__`` (in ``_CONCEPTS_PLAN_USER``) is substituted the +# same way, with a sentence computed from the current document's real token +# count (see ``_doc_token_guidance``) — a soft, non-enforced suggestion for +# how many brand-new concepts+entities to propose, never a filter. _SUMMARY_REWRITE_USER = """\ Task: Rewrite the summary you wrote above into a final version that is \ @@ -405,9 +429,17 @@ def _llm_call( raise_on_truncation: bool = False, *, bundle=None, + capture_usage: dict | None = None, **kwargs, ) -> str: - """Single LLM call with animated progress and debug logging.""" + """Single LLM call with animated progress and debug logging. + + ``capture_usage``, when given a dict, is populated with + ``{"prompt_tokens": ..., "completion_tokens": ...}`` from the response + before returning — lets a caller read the real token count of a call + (e.g. the summary call) without changing this function's string return + type for every other call site. + """ messages = _prepare_messages(model, messages) extra_headers = bundle.extra_headers if bundle is not None else get_extra_headers() if extra_headers: @@ -434,6 +466,9 @@ def _llm_call( logger.debug( "LLM response [%s]:\n%s", step_name, content[:500] + ("..." if len(content) > 500 else "") ) + if capture_usage is not None: + capture_usage["prompt_tokens"] = getattr(response.usage, "prompt_tokens", None) + capture_usage["completion_tokens"] = getattr(response.usage, "completion_tokens", None) if raise_on_truncation and truncated: raise TruncatedResponseError( f"LLM [{step_name}] hit the length limit; skipping to avoid a truncated page" @@ -594,8 +629,54 @@ def _page_fields(raw: str) -> tuple[str, str, dict | None]: return obj.get("description", ""), (obj.get("content") or ""), obj -def _filter_concept_items(items: list, label: str) -> list[dict]: - """Keep only dicts that carry a non-empty ``name``; warn about anything else.""" +_WORD_SPLIT_RE = re.compile(r"[-_\s]+") + + +def _count_words(name: str) -> int: + """Count words in a candidate name, splitting on ``-``/``_``/whitespace. + + Used to gate brand-new concept/entity names to a handful of words — a + lightweight, deterministic proxy for "too specific to be reusable + knowledge" (unique keys, hashes, ticket numbers, and multi-part + combinations all tend to produce long names). + """ + return len([w for w in _WORD_SPLIT_RE.split(name.strip()) if w]) + + +def _doc_token_guidance(doc_tokens: int | None) -> str: + """Build the ``__DOC_TOKEN_GUIDANCE__`` sentence for ``_CONCEPTS_PLAN_USER``. + + Purely a soft, textual suggestion for the LLM — never enforced/filtered + in code (see issue #247: "create" volume is only ever nudged via the + prompt; reuse/update/promotion of existing or pending topics is never + capped). ``suggested_cap`` uses ``_TOKENS_PER_NEW_ITEM`` as a single + internal constant for both the flat floor (short documents) and the + divisor (longer documents). Falls back to a generic sentence without + numbers when ``doc_tokens`` couldn't be determined (e.g. the usage object + was missing for a non-standard provider). + """ + if not doc_tokens or doc_tokens <= 0: + return ( + "As a rough guideline, propose only a handful of brand-new " + "concepts and entities combined for this document — reuse/update " + "existing pages for everything else." + ) + suggested_cap = 3 if doc_tokens < _TOKENS_PER_NEW_ITEM else doc_tokens // _TOKENS_PER_NEW_ITEM + return ( + f"This document is approximately {doc_tokens} tokens long. As a rough " + f"guideline, propose at most {suggested_cap} brand-new concepts and " + "entities combined for this document." + ) + + +def _filter_concept_items(items: list, label: str, *, max_words: int | None = None) -> list[dict]: + """Keep only dicts that carry a non-empty ``name``; warn about anything else. + + ``max_words``, when given, additionally drops names with more words (see + :func:`_count_words`) than that. Pass ``None`` (the default; used for + "update" items, which target an already-existing, already-vetted name) to + skip this check. + """ if not isinstance(items, list): logger.warning( "concepts plan: %s was %s, expected list — dropping", label, type(items).__name__ @@ -619,6 +700,17 @@ def _filter_concept_items(items: list, label: str) -> list[dict]: label, ", ".join(sorted(set(reasons))), ) + if max_words is not None: + too_long = [c for c in valid if _count_words(c["name"]) > max_words] + if too_long: + logger.info( + "concepts plan: dropped %d %s item(s) with names over %d words: %s", + len(too_long), + label, + max_words, + [c["name"] for c in too_long][:5], + ) + valid = [c for c in valid if _count_words(c["name"]) <= max_words] return valid @@ -648,7 +740,13 @@ def _filter_related_slugs(items: list) -> list[str]: return valid -def _filter_entity_items(items: object, valid_types: frozenset | None = None) -> list[dict]: +def _filter_entity_items( + items: object, + valid_types: frozenset | None = None, + *, + strict: bool = False, + max_words: int | None = None, +) -> list[dict]: """Validate entity create/update objects: require name+title, coerce type. Each kept item is normalized to ``{"name", "title", "type"}`` where @@ -656,32 +754,67 @@ def _filter_entity_items(items: object, valid_types: frozenset | None = None) -> and ``title`` falls back to ``name``. ``valid_types`` defaults to the module-level ``_ENTITY_TYPES`` so callers that don't thread a config-driven set keep today's behavior. + + ``strict`` (see ``config.resolve_strict_entity_types``), when ``True``, + drops an item whose type falls outside ``valid_types`` instead of coercing + it to ``"other"``. ``max_words`` (see :func:`_count_words`), when given, + drops names with more words than that. Both default to today's lenient + behavior (``False``/``None``); pass them only for "create" items — an + "update" targets an already-existing, already-vetted name/type. """ if valid_types is None: valid_types = _ENTITY_TYPES out: list[dict] = [] if not isinstance(items, list): return out + dropped_strict = 0 + dropped_words = 0 for it in items: if not isinstance(it, dict): continue name = it.get("name") if not isinstance(name, str) or not name.strip(): continue + if max_words is not None and _count_words(name) > max_words: + dropped_words += 1 + continue title = it.get("title") if isinstance(it.get("title"), str) else name etype = it.get("type") if not isinstance(etype, str) or etype not in valid_types: + if strict: + dropped_strict += 1 + continue etype = "other" out.append({"name": name, "title": title, "type": etype}) + if dropped_strict: + logger.info( + "concepts plan: dropped %d entity item(s) with type outside the configured " + "entity_types (strict_entity_types=true)", + dropped_strict, + ) + if dropped_words: + logger.info( + "concepts plan: dropped %d entity item(s) with names over %d words", + dropped_words, + max_words, + ) return out -def _parse_entities_plan(parsed: object, valid_types: frozenset | None = None) -> dict: +def _parse_entities_plan( + parsed: object, + valid_types: frozenset | None = None, + *, + strict: bool = False, + max_words: int | None = None, +) -> dict: """Extract the entities group from a plan dict, with graceful fallback. Returns ``{"create": [...], "update": [...], "related": [...]}``. A missing/malformed ``entities`` key yields empty lists, so older or - partial LLM responses never raise. + partial LLM responses never raise. ``strict``/``max_words`` (see + :func:`_filter_entity_items`) are applied to "create" only — an "update" + targets an already-existing, already-vetted name/type. """ empty = {"create": [], "update": [], "related": []} if not isinstance(parsed, dict): @@ -690,7 +823,9 @@ def _parse_entities_plan(parsed: object, valid_types: frozenset | None = None) - if not isinstance(group, dict): return empty return { - "create": _filter_entity_items(group.get("create", []), valid_types), + "create": _filter_entity_items( + group.get("create", []), valid_types, strict=strict, max_words=max_words + ), "update": _filter_entity_items(group.get("update", []), valid_types), "related": _filter_related_slugs(group.get("related", [])), } @@ -790,6 +925,21 @@ def _read_entity_briefs(wiki_dir: Path) -> str: return "\n".join(lines) or "(none yet)" +def _combine_briefs(existing_briefs: str, pending_store: "PendingTopicsStore", kind: str) -> str: + """Append pending-topic brief lines (see ``openkb.pending``) to the + existing-page briefs fed to the plan call, so the LLM treats a pending + topic like a quasi-existing page for dedup ("prefer update"/"related" + over proposing a near-duplicate create). Pending slugs are NOT added to + the wikilink whitelist elsewhere — no real page exists for them yet. + """ + pending_lines = pending_store.brief_lines(kind) + if not pending_lines: + return existing_briefs + if existing_briefs == "(none yet)": + return "\n".join(pending_lines) + return existing_briefs + "\n" + "\n".join(pending_lines) + + def _iter_h2_headings(lines: list[str]) -> list[tuple[int, str]]: """Return ``[(line_index, normalized_heading), ...]`` for every ATX H2. @@ -1618,6 +1768,8 @@ async def _compile_concepts( rewrite_summary: bool = False, entity_types: list[str] | None = None, concept_update_mode: str = "rewrite", + strict_entity_types: bool = False, + doc_tokens: int | None = None, bundle=None, ) -> None: """Shared Steps 2-4: concepts plan → generate/update → index. @@ -1647,8 +1799,11 @@ async def _compile_concepts( valid_types = frozenset(entity_types) # --- Step 2: Get concepts plan (A cached) --- - concept_briefs = _read_concept_briefs(wiki_dir) - entity_briefs = _read_entity_briefs(wiki_dir) + # Pending-topics buffer (issue #247): instantiated once here and reused + # below (repartition + task-building) — see PendingTopicsStore docstring. + pending_store = PendingTopicsStore(kb_dir / ".openkb" / "pending_topics.json") + concept_briefs = _combine_briefs(_read_concept_briefs(wiki_dir), pending_store, "concepts") + entity_briefs = _combine_briefs(_read_entity_briefs(wiki_dir), pending_store, "entities") # Second cache breakpoint: end of the assistant summary message. Covers # (system + doc + summary) for the plan call and every concept call. @@ -1665,7 +1820,9 @@ async def _compile_concepts( "content": _CONCEPTS_PLAN_USER.format( concept_briefs=concept_briefs, entity_briefs=entity_briefs, - ).replace("__ENTITY_TYPES__", types_str), + ) + .replace("__ENTITY_TYPES__", types_str) + .replace("__DOC_TOKEN_GUIDANCE__", _doc_token_guidance(doc_tokens)), }, ], "concepts-plan", @@ -1735,18 +1892,26 @@ def _write_v1_summary_stripped() -> None: return if isinstance(parsed, list): - plan = {"create": _filter_concept_items(parsed, "list"), "update": [], "related": []} + plan = { + "create": _filter_concept_items(parsed, "list", max_words=_MAX_NAME_WORDS), + "update": [], + "related": [], + } entities_plan = {"create": [], "update": [], "related": []} else: concepts_group = ( parsed.get("concepts") if isinstance(parsed.get("concepts"), dict) else parsed ) plan = { - "create": _filter_concept_items(concepts_group.get("create", []), "create"), + "create": _filter_concept_items( + concepts_group.get("create", []), "create", max_words=_MAX_NAME_WORDS + ), "update": _filter_concept_items(concepts_group.get("update", []), "update"), "related": _filter_related_slugs(concepts_group.get("related", [])), } - entities_plan = _parse_entities_plan(parsed, valid_types) + entities_plan = _parse_entities_plan( + parsed, valid_types, strict=strict_entity_types, max_words=_MAX_NAME_WORDS + ) create_items = plan["create"] update_items = plan["update"] @@ -1773,6 +1938,52 @@ def _write_v1_summary_stripped() -> None: if (wiki_dir / "entities" / f"{_sanitize_concept_name(s)}.md").exists() ] + # --- Ebene 3 (issue #247): route any candidate without a REAL page yet + # through the pending-topics buffer instead of an instant create. Routing + # is by actual disk state, not the LLM's create/update label — a pending + # topic is only visible to the LLM as brief text (see below), so it + # doesn't reliably know whether to call it "create" or "update". A + # candidate that already has a real page keeps today's exact behavior + # (normal update path); "create" itself never writes a page directly + # anymore — it always goes through the buffer first. + def _has_real_page(dirpath: Path, name: str) -> bool: + return (dirpath / f"{_sanitize_concept_name(name)}.md").exists() + + concepts_dir = wiki_dir / "concepts" + entities_dir = wiki_dir / "entities" + + concept_candidates = create_items + update_items + update_items = [c for c in concept_candidates if _has_real_page(concepts_dir, c["name"])] + pending_concept_candidates = [ + c for c in concept_candidates if not _has_real_page(concepts_dir, c["name"]) + ] + create_items = [] + + entity_candidates = entity_create + entity_update + entity_update = [e for e in entity_candidates if _has_real_page(entities_dir, e["name"])] + pending_entity_candidates = [ + e for e in entity_candidates if not _has_real_page(entities_dir, e["name"]) + ] + entity_create = [] + + # Only candidates whose buffer ALREADY holds MAX_NOTES_BEFORE_PROMOTION + # notes will promote to a real page this round (this document's mention + # is their 3rd) — those need to be in the wikilink whitelist below; a + # still-buffering candidate (1st/2nd mention) must NOT be, since no page + # will exist for it yet. + pending_concept_promote = [ + c + for c in pending_concept_candidates + if pending_store.note_count("concepts", _sanitize_concept_name(c["name"])) + >= MAX_NOTES_BEFORE_PROMOTION + ] + pending_entity_promote = [ + e + for e in pending_entity_candidates + if pending_store.note_count("entities", _sanitize_concept_name(e["name"])) + >= MAX_NOTES_BEFORE_PROMOTION + ] + # Distinguish "filters dropped everything" from "LLM emitted an empty plan". # Count entity items too, so a plan that emitted only entities — all of # which were dropped as malformed — still surfaces the warning. @@ -1789,12 +2000,12 @@ def _raw_group_count(group: object) -> int: else: original_total = _raw_group_count(concepts_group) + _raw_group_count(parsed.get("entities")) post_filter_total = ( - len(create_items) - + len(update_items) + len(update_items) + len(related_items) - + len(entity_create) + len(entity_update) + len(entity_related) + + len(pending_concept_candidates) + + len(pending_entity_candidates) ) if original_total > 0 and post_filter_total == 0: sys.stdout.write( @@ -1804,12 +2015,12 @@ def _raw_group_count(group: object) -> int: sys.stdout.flush() if ( - not create_items - and not update_items + not update_items and not related_items - and not entity_create and not entity_update and not entity_related + and not pending_concept_candidates + and not pending_entity_candidates ): if rewrite_summary: _write_v1_summary_stripped() @@ -1818,14 +2029,16 @@ def _raw_group_count(group: object) -> int: # Build the whitelist of valid wikilink targets the LLM may emit. It # combines what already exists on disk with what *this* round will - # produce (plan.create + plan.update + plan.related), plus the - # summary about to be written for this document. - planned_slugs = {_sanitize_concept_name(c["name"]) for c in create_items + update_items} | { - _sanitize_concept_name(s) for s in related_items - } - entity_planned = {_sanitize_concept_name(e["name"]) for e in entity_create + entity_update} | { - _sanitize_concept_name(s) for s in entity_related - } + # produce (plan.update + plan.related + any pending topic about to be + # promoted), plus the summary about to be written for this document. + # Still-buffering pending topics are deliberately excluded — no page + # will exist for them yet. + planned_slugs = { + _sanitize_concept_name(c["name"]) for c in update_items + pending_concept_promote + } | {_sanitize_concept_name(s) for s in related_items} + entity_planned = { + _sanitize_concept_name(e["name"]) for e in entity_update + pending_entity_promote + } | {_sanitize_concept_name(s) for s in entity_related} known_targets: set[str] = ( list_existing_wiki_targets(wiki_dir) | {f"concepts/{s}" for s in planned_slugs} @@ -1855,7 +2068,7 @@ def _raw_group_count(group: object) -> int: # --- Step 3: Generate/update concept pages concurrently (A cached) --- semaphore = asyncio.Semaphore(max_concurrency) - async def _gen_create(concept: dict) -> tuple[str, str, bool, str]: + async def _gen_create(concept: dict, extra_context: str = "") -> tuple[str, str, bool, str]: name = concept["name"] title = concept.get("title", name) async with semaphore: @@ -1871,7 +2084,7 @@ async def _gen_create(concept: dict) -> tuple[str, str, bool, str]: "content": _CONCEPT_PAGE_USER.format( title=title, doc_name=doc_name, - update_instruction="", + update_instruction=extra_context, ), }, ], @@ -1918,7 +2131,7 @@ async def _gen_update(concept: dict) -> tuple[str, str, bool, str]: _require_nonempty_content(content, name) return name, content, True, brief - async def _gen_entity_create(ent: dict) -> tuple[str, str, str, str]: + async def _gen_entity_create(ent: dict, extra_context: str = "") -> tuple[str, str, str, str]: name = ent["name"] title = ent.get("title", name) etype = ent.get("type", "other") @@ -1936,6 +2149,7 @@ async def _gen_entity_create(ent: dict) -> tuple[str, str, str, str]: title=title, type=etype, doc_name=doc_name, + update_instruction=extra_context, ).replace("__ENTITY_TYPES__", types_str), }, ], @@ -2101,7 +2315,147 @@ async def _gen_entity_note_update(ent: dict) -> tuple[str, str, str, str]: _require_nonempty_content(note, name) return name, note, "", etype + async def _gen_pending_concept(concept: dict) -> tuple[str, str] | None: + """Buffer a note for a concept with no real page yet, or promote it + to a real page on its 3rd mention (see openkb.pending). Returns + ``None`` for a buffer-only step (nothing written); returns + ``(safe_name, brief)`` for a promotion — the page is already written + directly (with all buffered sources) by this function, so the caller + only needs the slug/brief to register it for backlinking/index + update, same as a normal create. + """ + name = concept["name"] + title = concept.get("title", name) + slug = _sanitize_concept_name(name) + async with semaphore: + raw = await _llm_call_page_async( + model, + [ + system_msg, + doc_msg, # cached (BP1) + summary_msg, # cached (BP2) + { + "role": "user", + "content": compiler_notes._CONCEPT_NOTE_CREATE_USER.format( + title=title, doc_name=doc_name + ), + }, + ], + f"concept-note: {name}", + response_format=_JSON_RESPONSE_FORMAT, + bundle=bundle, + ) + brief, note = compiler_notes.note_fields(raw) + _require_nonempty_content(note, name) + new_count = pending_store.add_note("concepts", slug, title, doc_name, source_file, note) + if new_count <= MAX_NOTES_BEFORE_PROMOTION: + return # still buffering — no page yet + entry = pending_store.get("concepts", slug) + prior_notes = entry["notes"][:-1] if entry else [] + pending_store.remove("concepts", slug) + if concept_update_mode == "append": + for n in prior_notes: + compiler_notes.append_concept_note( + wiki_dir, name, n["note"], n["source_file"], n["doc_name"], description=brief + ) + compiler_notes.append_concept_note( + wiki_dir, name, note, source_file, doc_name, description=brief + ) + return + notes_ctx = "\n".join(f"- ({n['doc_name']}) {n['note']}" for n in prior_notes) + extra_context = f"Earlier notes about this topic from prior documents:\n{notes_ctx}" + _, content, _, brief2 = await _gen_create(concept, extra_context=extra_context) + cleaned, ghosts = strip_ghost_wikilinks(content, known_targets) + if ghosts: + logger.info( + "stripped %d ghost wikilink(s) from promoted concept %s: %s", + len(ghosts), + name, + ghosts[:5], + ) + _write_concept(wiki_dir, name, cleaned, source_file, False, brief=brief2) + path = (wiki_dir / "concepts" / f"{slug}.md").resolve() + existing = path.read_text(encoding="utf-8") + for n in prior_notes: + existing = _prepend_source_to_frontmatter(existing, n["source_file"]) + atomic_write_text(path, existing) + return slug, brief2 + + async def _gen_pending_entity(ent: dict) -> tuple[str, str, str] | None: + """Entity counterpart of :func:`_gen_pending_concept` — see there. + Returns ``(safe_name, brief, type)`` on promotion, else ``None``. + """ + name = ent["name"] + title = ent.get("title", name) + etype = ent.get("type", "other") + slug = _sanitize_concept_name(name) + async with semaphore: + raw = await _llm_call_page_async( + model, + [ + system_msg, + doc_msg, # cached (BP1) + summary_msg, # cached (BP2) + { + "role": "user", + "content": compiler_notes._ENTITY_NOTE_CREATE_USER.format( + title=title, type=etype, doc_name=doc_name + ), + }, + ], + f"entity-note: {name}", + response_format=_JSON_RESPONSE_FORMAT, + bundle=bundle, + ) + brief, note = compiler_notes.note_fields(raw) + _require_nonempty_content(note, name) + new_count = pending_store.add_note( + "entities", slug, title, doc_name, source_file, note, type_=etype + ) + if new_count <= MAX_NOTES_BEFORE_PROMOTION: + return # still buffering — no page yet + entry = pending_store.get("entities", slug) + prior_notes = entry["notes"][:-1] if entry else [] + pending_store.remove("entities", slug) + if concept_update_mode == "append": + for n in prior_notes: + compiler_notes.append_entity_note( + wiki_dir, + name, + n["note"], + n["source_file"], + n["doc_name"], + description=brief, + type_=etype, + ) + compiler_notes.append_entity_note( + wiki_dir, name, note, source_file, doc_name, description=brief, type_=etype + ) + return + notes_ctx = "\n".join(f"- ({n['doc_name']}) {n['note']}" for n in prior_notes) + extra_context = f"Earlier notes about this topic from prior documents:\n{notes_ctx}" + _, content, brief2, etype_out = await _gen_entity_create(ent, extra_context=extra_context) + cleaned, ghosts = strip_ghost_wikilinks(content, known_targets) + if ghosts: + logger.info( + "stripped %d ghost wikilink(s) from promoted entity %s: %s", + len(ghosts), + name, + ghosts[:5], + ) + _write_entity(wiki_dir, name, cleaned, source_file, False, brief=brief2, type_=etype_out) + path = (wiki_dir / "entities" / f"{slug}.md").resolve() + existing = path.read_text(encoding="utf-8") + for n in prior_notes: + existing = _prepend_source_to_frontmatter(existing, n["source_file"]) + atomic_write_text(path, existing) + return slug, brief2, etype_out + tasks = [] + # Pending-buffer tasks scheduled first (mirrors the old "create tasks come + # before update tasks" ordering: create_items is always empty now, so a + # brand-new topic's task is this one instead). + tasks.extend(_gen_pending_concept(c) for c in pending_concept_candidates) if concept_update_mode == "append": tasks.extend(_gen_note_create(c) for c in create_items) tasks.extend(_gen_note_update(c) for c in update_items) @@ -2114,6 +2468,7 @@ async def _gen_entity_note_update(ent: dict) -> tuple[str, str, str, str]: # return 4-arity tuples (name, content, brief, type), so their results are # processed in their own loop rather than mixed with the concept tuples. entity_tasks = [] + entity_tasks.extend(_gen_pending_entity(e) for e in pending_entity_candidates) if concept_update_mode == "append": entity_tasks.extend(_gen_entity_note_create(e) for e in entity_create) entity_tasks.extend(_gen_entity_note_update(e) for e in entity_update) @@ -2151,11 +2506,29 @@ async def _gen_entity_note_update(ent: dict) -> tuple[str, str, str, str]: if tasks: failure_types: list[str] = [] + self_handled = 0 for r in results: if isinstance(r, Exception): logger.warning("Concept generation failed: %s", r) failure_types.append(type(r).__name__) continue + if r is None: + # Pending-buffer step (openkb.pending): a still-buffering note + # or a promotion that already wrote its own page directly — + # neither is a failure, and neither has anything left to do + # in pending_writes below. + self_handled += 1 + continue + if len(r) == 2: + # Promoted via the pending buffer (openkb.pending): the page + # is already written directly — just register it for + # backlinking/whitelist/index like a normal create. + safe_name, brief = r + concept_names.append(safe_name) + if brief: + concept_briefs_map[safe_name] = brief + self_handled += 1 + continue name, page_content, is_update, brief = r pending_writes.append((name, page_content, is_update, brief)) safe_name = _sanitize_concept_name(name) @@ -2166,7 +2539,7 @@ async def _gen_entity_note_update(ent: dict) -> tuple[str, str, str, str]: # Include exception type names inline so the stdout line is # self-contained — per-failure WARNINGs go to stderr. written = len(pending_writes) - if written < total: + if written + self_handled < total: reason = ", ".join(sorted(set(failure_types))) if failure_types else "see log (stderr)" sys.stdout.write( f" [WARN] {total} concept(s) planned but only {written} written " @@ -2176,16 +2549,30 @@ async def _gen_entity_note_update(ent: dict) -> tuple[str, str, str, str]: if entity_tasks: entity_failure_types: list[str] = [] + entity_self_handled = 0 for r in entity_results: if isinstance(r, Exception): logger.warning("Entity generation failed: %s", r) entity_failure_types.append(type(r).__name__) continue + if r is None: + # Pending-buffer step (openkb.pending) — see the concept loop + # above for why this isn't a failure. + entity_self_handled += 1 + continue + if len(r) == 3: + # Promoted via the pending buffer (openkb.pending) — see the + # concept loop above. + safe_name, brief, etype = r + entity_names.append(safe_name) + entity_meta[safe_name] = (etype, brief) + entity_self_handled += 1 + continue name, page_content, brief, etype = r entity_pending.append((name, page_content, brief, etype)) ewritten = len(entity_pending) - if ewritten < etotal: + if ewritten + entity_self_handled < etotal: reason = ( ", ".join(sorted(set(entity_failure_types))) if entity_failure_types @@ -2371,7 +2758,11 @@ async def compile_short_doc( Step 1: Build base context A (schema + doc content), generate summary. Steps 2-4: Delegated to ``_compile_concepts``. """ - from openkb.config import resolve_concept_update_mode, resolve_effective_config + from openkb.config import ( + resolve_concept_update_mode, + resolve_effective_config, + resolve_strict_entity_types, + ) config = resolve_effective_config(kb_dir)[0] language: str = config.get("language", "en") @@ -2406,13 +2797,16 @@ async def compile_short_doc( # for the plan + concept-generation calls, then rewritten into a final # v2 (with a whitelist of known wikilink targets) inside # _compile_concepts before being written to disk. + summary_usage: dict = {} summary_raw = _llm_call( model, [system_msg, doc_msg], "summary", response_format=_JSON_RESPONSE_FORMAT, bundle=bundle, + capture_usage=summary_usage, ) + doc_tokens = summary_usage.get("prompt_tokens") try: summary_parsed = _parse_json(summary_raw) doc_brief = summary_parsed.get("description", "") @@ -2437,6 +2831,8 @@ async def compile_short_doc( rewrite_summary=True, entity_types=entity_types, concept_update_mode=resolve_concept_update_mode(config), + strict_entity_types=resolve_strict_entity_types(config), + doc_tokens=doc_tokens, bundle=bundle, ) finally: @@ -2460,7 +2856,11 @@ async def compile_long_doc( The summary page is already written by the indexer. This function generates concept pages and updates the index. """ - from openkb.config import resolve_concept_update_mode, resolve_effective_config + from openkb.config import ( + resolve_concept_update_mode, + resolve_effective_config, + resolve_strict_entity_types, + ) config = resolve_effective_config(kb_dir)[0] language: str = config.get("language", "en") @@ -2505,7 +2905,15 @@ async def compile_long_doc( } # --- Step 1: Generate overview --- - overview = _llm_call(model, [system_msg, doc_msg], "overview", bundle=bundle) + # doc_tokens here approximates the tokens of the PageIndex SUMMARY fed to + # this call, not the original long document (which is never sent whole + # to a single call) — an accepted approximation for the token-density + # guidance substituted into __DOC_TOKEN_GUIDANCE__. + overview_usage: dict = {} + overview = _llm_call( + model, [system_msg, doc_msg], "overview", bundle=bundle, capture_usage=overview_usage + ) + doc_tokens = overview_usage.get("prompt_tokens") # --- Steps 2-4: Concept plan → generate/update → index --- try: @@ -2522,6 +2930,8 @@ async def compile_long_doc( doc_type="pageindex", entity_types=entity_types, concept_update_mode=resolve_concept_update_mode(config), + strict_entity_types=resolve_strict_entity_types(config), + doc_tokens=doc_tokens, bundle=bundle, ) finally: diff --git a/openkb/config.py b/openkb/config.py index 46173c840..f4c3f854f 100644 --- a/openkb/config.py +++ b/openkb/config.py @@ -40,6 +40,10 @@ # resolve_concept_update_mode(). KB config.yaml only (like `debug`), not # in GLOBAL_SCALAR_KEYS. "concept_update_mode": "rewrite", + # Opt-in gate for `entity_types:` — see resolve_strict_entity_types(). KB + # config.yaml only, not in GLOBAL_SCALAR_KEYS (same treatment as + # concept_update_mode above). + "strict_entity_types": False, } VALID_CONCEPT_UPDATE_MODES: tuple[str, ...] = ("rewrite", "append") @@ -163,6 +167,25 @@ def resolve_concept_update_mode(config: dict) -> str: return value +def resolve_strict_entity_types(config: dict) -> bool: + """Resolve ``strict_entity_types:`` — ``False`` by default. + + When ``True``, an entity whose LLM-returned ``type`` doesn't match the + configured :func:`resolve_entity_types` vocabulary is dropped instead of + being coerced to ``"other"``. A non-bool value degrades to ``False`` with + a warning (matches :func:`resolve_concept_update_mode`'s degrade-on- + malformed-value behavior). + """ + value = config.get("strict_entity_types", False) + if not isinstance(value, bool): + logger.warning( + "config: 'strict_entity_types' must be a bool, got %r — using False.", + value, + ) + return False + return value + + def resolve_extra_headers(config: dict) -> dict[str, str]: """Resolve the optional ``extra_headers:`` config key into a str→str dict. diff --git a/openkb/pending.py b/openkb/pending.py new file mode 100644 index 000000000..80fe7b6e6 --- /dev/null +++ b/openkb/pending.py @@ -0,0 +1,112 @@ +"""Pending-topic buffer for the wiki compiler (see issue #247). + +Buffers up to :data:`MAX_NOTES_BEFORE_PROMOTION` short notes per brand-new +concept/entity candidate before a dedicated wiki page is created, so a topic +mentioned exactly once or twice (a document's own ticket/case identifier, a +person mentioned only in passing) doesn't immediately get its own low-value +page — while a topic that genuinely recurs still gets promoted to a real +page, with all buffered notes as its starting content/sources. Used by +``openkb.agent.compiler``; kept as its own module so the JSON-backed +persistence mirrors ``openkb.state.HashRegistry``'s load-on-init / +persist-on-mutation pattern without growing that module further. +""" + +from __future__ import annotations + +import datetime +import json +from pathlib import Path + +from openkb.locks import atomic_write_json + +#: Number of notes collected before the NEXT note promotes the topic to a +#: real page (i.e. the 3rd mention creates the page). Intentionally not a +#: config key (see issue #247) — internal, hardcoded constant. +MAX_NOTES_BEFORE_PROMOTION = 2 + + +class PendingTopicsStore: + """Persistent buffer of not-yet-paged concept/entity candidates. + + Persisted as ``kb_dir/.openkb/pending_topics.json`` (same directory + convention as ``state.HashRegistry``'s ``hashes.json``). Callers already + hold the KB's exclusive lock (``@_with_kb_lock`` wraps every `add`/ + `add-all` run in ``cli.py``), so no additional locking is needed here. + """ + + def __init__(self, path: Path) -> None: + self._path = path + if path.exists(): + with path.open("r", encoding="utf-8") as fh: + data = json.load(fh) + self._data: dict[str, dict[str, dict]] = data if isinstance(data, dict) else {} + else: + self._data = {} + self._data.setdefault("concepts", {}) + self._data.setdefault("entities", {}) + + def get(self, kind: str, slug: str) -> dict | None: + """Return the pending entry for ``slug`` in ``kind``, or None.""" + return self._data[kind].get(slug) + + def note_count(self, kind: str, slug: str) -> int: + """Return how many notes are already buffered for ``slug`` (0 if absent).""" + entry = self.get(kind, slug) + return len(entry["notes"]) if entry else 0 + + def add_note( + self, + kind: str, + slug: str, + title: str, + doc_name: str, + source_file: str, + note: str, + type_: str | None = None, + ) -> int: + """Append a note for ``slug``, creating the entry if it's missing. + + Returns the new total note count (callers promote once this reaches + ``MAX_NOTES_BEFORE_PROMOTION + 1``, i.e. the 3rd note). + """ + entry = self._data[kind].setdefault(slug, {"title": title, "notes": []}) + entry["title"] = title + if type_ is not None: + entry["type"] = type_ + entry["notes"].append( + { + "doc_name": doc_name, + "source_file": source_file, + "note": note, + "date": datetime.date.today().isoformat(), + } + ) + self._persist() + return len(entry["notes"]) + + def remove(self, kind: str, slug: str) -> None: + """Remove a pending entry (called right after promotion to a real page).""" + if slug in self._data[kind]: + del self._data[kind][slug] + self._persist() + + def brief_lines(self, kind: str) -> list[str]: + """Return ``- {slug} (pending, {n}/{total} mentions) — {last note}`` lines. + + Extends the plan call's existing-page briefs so the LLM treats + pending topics like quasi-existing pages for dedup ("prefer update"/ + "related" over proposing a near-duplicate create). Pending slugs + must NEVER be added to the wikilink whitelist (no real page exists + yet) — enforcing that is the caller's responsibility, not this + method's. + """ + total = MAX_NOTES_BEFORE_PROMOTION + 1 + lines: list[str] = [] + for slug, entry in self._data[kind].items(): + notes = entry.get("notes", []) + last = notes[-1]["note"] if notes else "" + lines.append(f"- {slug} (pending, {len(notes)}/{total} mentions) — {last}") + return lines + + def _persist(self) -> None: + atomic_write_json(self._path, self._data) diff --git a/tests/test_compiler.py b/tests/test_compiler.py index dc9579530..8d0f37732 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -15,6 +15,9 @@ _backlink_summary, _backlink_summary_entities, _compile_concepts, + _count_words, + _doc_token_guidance, + _filter_concept_items, _filter_entity_items, _parse_entities_plan, _parse_json, @@ -33,6 +36,7 @@ remove_doc_from_entity_pages, ) from openkb.config import resolve_entity_types +from openkb.pending import PendingTopicsStore from openkb.schema import AGENTS_MD @@ -195,6 +199,133 @@ def test_default_valid_types_backward_compat(self): assert out[0]["type"] == "organization" +class TestStrictEntityTypes: + """strict_entity_types=true (see openkb.config.resolve_strict_entity_types + / issue #247): a type outside the configured vocabulary drops the item + entirely instead of coercing it to "other".""" + + def test_strict_false_still_coerces_to_other(self): + valid = frozenset({"person", "dataset", "other"}) + items = [{"name": "x", "title": "X", "type": "organization"}] + out = _filter_entity_items(items, valid, strict=False) + assert out == [{"name": "x", "title": "X", "type": "other"}] + + def test_strict_true_drops_mismatched_type(self): + valid = frozenset({"person", "dataset", "other"}) + items = [{"name": "x", "title": "X", "type": "organization"}] + out = _filter_entity_items(items, valid, strict=True) + assert out == [] + + def test_strict_true_keeps_matching_type(self): + valid = frozenset({"person", "dataset", "other"}) + items = [{"name": "imagenet", "title": "ImageNet", "type": "dataset"}] + out = _filter_entity_items(items, valid, strict=True) + assert out == [{"name": "imagenet", "title": "ImageNet", "type": "dataset"}] + + +class TestMaxWordsFilter: + """Hard cap on brand-new concept/entity names to 3 words (see + compiler._count_words / issue #247) — a lightweight proxy for "too + specific to be reusable knowledge".""" + + def test_count_words_splits_on_hyphen_underscore_and_space(self): + assert _count_words("attention") == 1 + assert _count_words("flash-attention") == 2 + assert _count_words("andreas-mueller-alwart-ssmpa-2573") == 5 + assert _count_words("some_snake_case_name") == 4 + assert _count_words("a name with spaces") == 4 + + def test_concept_items_over_limit_are_dropped(self): + items = [ + {"name": "attention", "title": "Attention"}, + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas"}, + ] + out = _filter_concept_items(items, "create", max_words=3) + assert [c["name"] for c in out] == ["attention"] + + def test_concept_items_without_max_words_are_unaffected(self): + items = [{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas"}] + out = _filter_concept_items(items, "update") + assert len(out) == 1 + + def test_entity_items_over_limit_are_dropped(self): + items = [ + {"name": "nvidia", "title": "NVIDIA", "type": "organization"}, + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "person"}, + ] + out = _filter_entity_items(items, max_words=3) + assert [e["name"] for e in out] == ["nvidia"] + + def test_entity_items_without_max_words_are_unaffected(self): + items = [{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "other"}] + out = _filter_entity_items(items) + assert len(out) == 1 + + +class TestParseEntitiesPlanStrictAndMaxWords: + """strict/max_words are threaded through _parse_entities_plan for + "create" only — "update" targets an already-existing, already-vetted + name/type (see issue #247).""" + + def test_create_gets_max_words_and_strict(self): + valid = frozenset({"person", "other"}) + parsed = { + "entities": { + "create": [ + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "A", "type": "person"}, + {"name": "nvidia", "title": "NVIDIA", "type": "organization"}, + ], + "update": [], + "related": [], + } + } + out = _parse_entities_plan(parsed, valid, strict=True, max_words=3) + assert out["create"] == [] + + def test_update_is_never_word_or_strict_filtered(self): + valid = frozenset({"person", "other"}) + parsed = { + "entities": { + "create": [], + "update": [ + { + "name": "andreas-mueller-alwart-ssmpa-2573", + "title": "A", + "type": "organization", + } + ], + "related": [], + } + } + out = _parse_entities_plan(parsed, valid, strict=True, max_words=3) + assert len(out["update"]) == 1 + assert out["update"][0]["type"] == "other" # coerced, not strict-dropped + + +class TestDocTokenGuidance: + """Soft, textual __DOC_TOKEN_GUIDANCE__ substitution (see issue #247) — + never a code-enforced filter, only guidance text for the plan prompt.""" + + def test_none_falls_back_to_generic_text(self): + text = _doc_token_guidance(None) + assert "rough guideline" in text + assert "tokens long" not in text + + def test_zero_or_negative_falls_back_to_generic_text(self): + assert "tokens long" not in _doc_token_guidance(0) + assert "tokens long" not in _doc_token_guidance(-5) + + def test_short_document_uses_flat_floor(self): + text = _doc_token_guidance(500) + assert "approximately 500 tokens" in text + assert "at most 3" in text + + def test_long_document_uses_density_formula(self): + text = _doc_token_guidance(4500) + assert "approximately 4500 tokens" in text + assert "at most 4" in text # floor(4500 / 1000) + + class TestParseBriefContent: def test_dict_with_brief_and_content(self): text = json.dumps({"brief": "A short desc", "content": "# Full page\n\nDetails."}) @@ -1146,6 +1277,63 @@ async def side_effect(*args, **kwargs): return side_effect +def _seed_pending(kb_dir, kind: str, slug: str, title: str, n: int = 2, type_: str | None = None): + """Pre-seed the pending-topics buffer (see openkb.pending / issue #247) so + the NEXT mention of ``slug`` promotes it to a real page instead of just + buffering another note — lets create-path tests written before the + pending buffer keep asserting an immediate page write. + """ + store = PendingTopicsStore(kb_dir / ".openkb" / "pending_topics.json") + for i in range(n): + store.add_note( + kind, + slug, + title, + f"prior-doc-{i}", + f"summaries/prior-doc-{i}.md", + f"prior note {i}", + type_=type_, + ) + + +def _message_text(messages) -> str: + """Flatten a litellm ``messages`` list to a single string for substring checks.""" + parts = [] + for m in messages or []: + content = m.get("content") + if isinstance(content, list): + parts.append("".join(b.get("text", "") for b in content if isinstance(b, dict))) + else: + parts.append(content or "") + return "\n".join(parts) + + +def _routed_acompletion(rules: list[tuple[str, str]]): + """Async mock for litellm.acompletion routed by message content. + + ``rules`` is an ordered list of ``(marker_substring, response_json)``; + the first rule whose marker appears in the joined message text wins. + Robust against concurrent task scheduling order — unlike a purely + positional mock, this doesn't assume a fixed call order, which matters + now that a pending-buffer promotion issues an internal note-generation + call before its page-content call (see openkb.pending / issue #247). + """ + + async def side_effect(*args, **kwargs): + text = _message_text(kwargs.get("messages")) + for marker, response in rules: + if marker in text: + mock_resp = MagicMock() + mock_resp.choices = [MagicMock()] + mock_resp.choices[0].message.content = response + mock_resp.usage = MagicMock(prompt_tokens=100, completion_tokens=50) + mock_resp.usage.prompt_tokens_details = None + return mock_resp + raise AssertionError(f"no mock rule matched acompletion call: {text[:300]!r}") + + return side_effect + + class TestCompileShortDoc: @pytest.mark.asyncio async def test_full_pipeline(self, tmp_path): @@ -1179,12 +1367,14 @@ async def test_full_pipeline(self, tmp_path): ) # The rewrite step (third sync call) returns raw Markdown. summary_rewrite_response = "# Summary\n\nThis document discusses [[concepts/transformer]]." + note_response = json.dumps({"description": "NN architecture", "note": "seen in test-doc"}) concept_page_response = json.dumps( { "brief": "NN architecture using self-attention", "content": "# Transformer\n\nA neural network architecture.", } ) + _seed_pending(tmp_path, "concepts", "transformer", "Transformer") with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock( @@ -1197,7 +1387,12 @@ async def test_full_pipeline(self, tmp_path): ) ) mock_litellm.acompletion = AsyncMock( - side_effect=_mock_acompletion([concept_page_response]) + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", concept_page_response), + ] + ) ) await compile_short_doc("test-doc", source_path, tmp_path, "gpt-4o-mini") @@ -1213,7 +1408,7 @@ async def test_full_pipeline(self, tmp_path): # Verify concept written concept_path = wiki / "concepts" / "transformer.md" assert concept_path.exists() - assert 'sources: ["summaries/test-doc.md"]' in concept_path.read_text() + assert '"summaries/test-doc.md"' in concept_path.read_text() # Verify index updated index_text = (wiki / "index.md").read_text() @@ -1260,6 +1455,7 @@ async def test_append_mode_full_pipeline_writes_note_not_full_page(self, tmp_pat "note": "This ticket reports a timeout during approval.", } ) + _seed_pending(tmp_path, "concepts", "approval-workflows", "Approval Workflows") with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock( @@ -1276,7 +1472,7 @@ async def test_append_mode_full_pipeline_writes_note_not_full_page(self, tmp_pat assert "## Notes" in text assert "This ticket reports a timeout during approval." in text assert 'description: "How approvals are routed."' in text - assert 'sources: ["summaries/test-doc.md"]' in text + assert '"summaries/test-doc.md"' in text @pytest.mark.asyncio async def test_handles_bad_json(self, tmp_path): @@ -1348,7 +1544,9 @@ async def test_rewrite_empty_response_falls_back_to_v1(self, tmp_path): ) # Rewrite returns an empty string → must fall back to v1 rewrite_response = "" + note_response = json.dumps({"description": "N", "note": "seen in doc"}) concept_response = json.dumps({"brief": "C", "content": "# T\n\nBody."}) + _seed_pending(tmp_path, "concepts", "transformer", "Transformer") with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock( @@ -1360,7 +1558,14 @@ async def test_rewrite_empty_response_falls_back_to_v1(self, tmp_path): ] ) ) - mock_litellm.acompletion = AsyncMock(side_effect=_mock_acompletion([concept_response])) + mock_litellm.acompletion = AsyncMock( + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", concept_response), + ] + ) + ) await compile_short_doc("doc", source_path, tmp_path, "gpt-4o-mini") summary_path = wiki / "summaries" / "doc.md" @@ -1391,6 +1596,8 @@ async def test_rewrite_exception_falls_back_to_v1(self, tmp_path): } ) concept_response = json.dumps({"brief": "C", "content": "# T\n\nBody."}) + note_response = json.dumps({"description": "N", "note": "seen in doc"}) + _seed_pending(tmp_path, "concepts", "transformer", "Transformer") # Third sync call (rewrite) raises a simulated API error. sync_call_count = {"n": 0} @@ -1412,7 +1619,14 @@ def sync_side_effect(*args, **kwargs): with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock(side_effect=sync_side_effect) - mock_litellm.acompletion = AsyncMock(side_effect=_mock_acompletion([concept_response])) + mock_litellm.acompletion = AsyncMock( + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", concept_response), + ] + ) + ) # Must NOT raise out of compile_short_doc await compile_short_doc("doc", source_path, tmp_path, "gpt-4o-mini") @@ -1551,7 +1765,11 @@ async def test_short_doc_marks_doc_and_summary(self, tmp_path): ) # 3rd sync call is the summary-rewrite (raw Markdown, not JSON). summary_rewrite_response = "# Summary\n\nrewritten body" - concept_response = json.dumps({"brief": "C", "content": "page body"}) + # Carries both "content" (page-generation call) and "note" (the + # pending-buffer's internal note-generation call, see openkb.pending) + # keys so the SAME canned response works for either async call. + concept_response = json.dumps({"brief": "C", "content": "page body", "note": "page body"}) + _seed_pending(tmp_path, "concepts", "topic", "Topic") captured_sync_calls: list[list[dict]] = [] captured_async_calls: list[list[dict]] = [] @@ -1604,8 +1822,12 @@ async def async_side_effect(*args, **kwargs): ) # Step 3 (concept generation): BP1 + BP2 + new BP3 (known_targets msg). + # The pending-buffer promotion (see openkb.pending) issues an internal + # note-generation call BEFORE the page-content call — that note call + # has no known_targets message, so this checks the LAST async call + # (the actual page-content/promotion one), not the first. assert captured_async_calls, "expected at least one async concept call" - concept_call = captured_async_calls[0] + concept_call = captured_async_calls[-1] assert self._has_cache_breakpoint(concept_call[1]) assert self._has_cache_breakpoint(concept_call[2]) # New: BP3 is the known_targets user message at index 3, sitting @@ -1696,19 +1918,26 @@ async def test_full_pipeline(self, tmp_path): "related": [], } ) + note_response = json.dumps({"description": "Subfield of ML", "note": "seen in big-doc"}) concept_page_response = json.dumps( { "brief": "Subfield of ML using neural networks", "content": "# Deep Learning\n\nA subfield of ML.", } ) + _seed_pending(tmp_path, "concepts", "deep-learning", "Deep Learning") with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock( side_effect=_mock_completion([overview_response, concepts_list_response]) ) mock_litellm.acompletion = AsyncMock( - side_effect=_mock_acompletion([concept_page_response]) + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", concept_page_response), + ] + ) ) await compile_long_doc("big-doc", summary_path, "doc-123", tmp_path, "gpt-4o-mini") @@ -1762,6 +1991,9 @@ async def test_create_and_update_flow(self, tmp_path): "related": [], } ) + note_response = json.dumps( + {"description": "Efficient attention algorithm", "note": "seen in test-doc"} + ) create_page_response = json.dumps( { "brief": "Efficient attention algorithm", @@ -1774,30 +2006,23 @@ async def test_create_and_update_flow(self, tmp_path): "content": "# Attention\n\nUpdated content with new info.", } ) + _seed_pending(tmp_path, "concepts", "flash-attention", "Flash Attention") system_msg = {"role": "system", "content": "You are a wiki agent."} doc_msg = {"role": "user", "content": "Document about attention mechanisms."} summary = "Summary of the document." - call_order = {"n": 0} - - async def ordered_acompletion(*args, **kwargs): - idx = call_order["n"] - call_order["n"] += 1 - mock_resp = MagicMock() - mock_resp.choices = [MagicMock()] - # create tasks come first, then update tasks - if idx == 0: - mock_resp.choices[0].message.content = create_page_response - else: - mock_resp.choices[0].message.content = update_page_response - mock_resp.usage = MagicMock(prompt_tokens=100, completion_tokens=50) - mock_resp.usage.prompt_tokens_details = None - return mock_resp - with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock(side_effect=_mock_completion([plan_response])) - mock_litellm.acompletion = AsyncMock(side_effect=ordered_acompletion) + mock_litellm.acompletion = AsyncMock( + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", create_page_response), + ("Update the concept page for", update_page_response), + ] + ) + ) await _compile_concepts( wiki, tmp_path, @@ -1813,7 +2038,7 @@ async def ordered_acompletion(*args, **kwargs): fa_path = wiki / "concepts" / "flash-attention.md" assert fa_path.exists() fa_text = fa_path.read_text() - assert 'sources: ["summaries/test-doc.md"]' in fa_text + assert '"summaries/test-doc.md"' in fa_text assert "Flash Attention" in fa_text # Verify attention updated (is_update=True path in _write_concept) @@ -1847,10 +2072,19 @@ async def test_page_json_wrapped_in_single_array_is_recovered(self, tmp_path): plan_response = json.dumps( {"create": [{"name": "attention", "title": "Attention"}], "update": [], "related": []} ) + note_response = json.dumps({"description": "b", "note": "seen in test-doc"}) array_page = json.dumps([{"brief": "b", "content": "# Attention\n\nRecovered body."}]) + _seed_pending(tmp_path, "concepts", "attention", "Attention") with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock(side_effect=_mock_completion([plan_response])) - mock_litellm.acompletion = AsyncMock(side_effect=_mock_acompletion([array_page])) + mock_litellm.acompletion = AsyncMock( + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", array_page), + ] + ) + ) await _compile_concepts( wiki, tmp_path, @@ -2108,12 +2342,14 @@ async def test_fallback_list_format(self, tmp_path): {"name": "attention", "title": "Attention"}, ] ) + note_response = json.dumps({"description": "b", "note": "seen in test-doc"}) concept_page_response = json.dumps( { "brief": "A mechanism for focusing", "content": "# Attention\n\nA mechanism for focusing.", } ) + _seed_pending(tmp_path, "concepts", "attention", "Attention") system_msg = {"role": "system", "content": "You are a wiki agent."} doc_msg = {"role": "user", "content": "Document content."} @@ -2122,7 +2358,12 @@ async def test_fallback_list_format(self, tmp_path): with patch("openkb.agent.compiler.litellm") as mock_litellm: mock_litellm.completion = MagicMock(side_effect=_mock_completion([plan_response])) mock_litellm.acompletion = AsyncMock( - side_effect=_mock_acompletion([concept_page_response]) + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", concept_page_response), + ] + ) ) await _compile_concepts( wiki, @@ -2139,10 +2380,115 @@ async def test_fallback_list_format(self, tmp_path): att_path = wiki / "concepts" / "attention.md" assert att_path.exists() att_text = att_path.read_text() - assert 'sources: ["summaries/test-doc.md"]' in att_text + assert '"summaries/test-doc.md"' in att_text assert "Attention" in att_text +class TestPendingBufferLifecycle: + """End-to-end: a brand-new concept needs 3 mentions across documents + before a real page exists, in EITHER concept_update_mode (see + openkb.pending / issue #247).""" + + @staticmethod + def _setup_wiki(tmp_path): + wiki = tmp_path / "wiki" + (wiki / "concepts").mkdir(parents=True) + (wiki / "entities").mkdir(parents=True) + (wiki / "index.md").write_text("# Index\n\n## Documents\n\n## Concepts\n", encoding="utf-8") + return wiki + + @staticmethod + def _plan_response(): + return json.dumps( + { + "concepts": { + "create": [{"name": "flaky-timeout", "title": "Flaky Timeout"}], + "update": [], + "related": [], + }, + "entities": {"create": [], "update": [], "related": []}, + } + ) + + @pytest.mark.asyncio + async def test_three_mentions_promote_in_rewrite_mode(self, tmp_path): + wiki = self._setup_wiki(tmp_path) + note_response = json.dumps({"description": "A recurring timeout bug", "note": "seen"}) + page_response = json.dumps( + {"brief": "A recurring timeout bug", "content": "# Flaky Timeout\n\nDetails."} + ) + path = wiki / "concepts" / "flaky-timeout.md" + + for i in range(1, 4): + with patch("openkb.agent.compiler.litellm") as mock_litellm: + mock_litellm.completion = MagicMock( + side_effect=_mock_completion([self._plan_response()]) + ) + mock_litellm.acompletion = AsyncMock( + side_effect=_routed_acompletion( + [ + ("NEW concept page", note_response), + ("Write the concept page for", page_response), + ] + ) + ) + await _compile_concepts( + wiki, + tmp_path, + "gpt-4o-mini", + {"role": "system", "content": "s"}, + {"role": "user", "content": "d"}, + "summary", + f"doc-{i}", + 5, + ) + if i < 3: + assert not path.exists(), f"should not promote after mention {i}" + else: + assert path.exists(), "should promote on the 3rd mention" + + text = path.read_text(encoding="utf-8") + assert '"summaries/doc-1.md"' in text + assert '"summaries/doc-2.md"' in text + assert '"summaries/doc-3.md"' in text + + @pytest.mark.asyncio + async def test_three_mentions_promote_in_append_mode(self, tmp_path): + wiki = self._setup_wiki(tmp_path) + path = wiki / "concepts" / "flaky-timeout.md" + + for i in range(1, 4): + note_response = json.dumps( + {"description": "A recurring timeout bug", "note": f"seen in doc-{i}"} + ) + with patch("openkb.agent.compiler.litellm") as mock_litellm: + mock_litellm.completion = MagicMock( + side_effect=_mock_completion([self._plan_response()]) + ) + mock_litellm.acompletion = AsyncMock(side_effect=_mock_acompletion([note_response])) + await _compile_concepts( + wiki, + tmp_path, + "gpt-4o-mini", + {"role": "system", "content": "s"}, + {"role": "user", "content": "d"}, + "summary", + f"doc-{i}", + 5, + concept_update_mode="append", + ) + if i < 3: + assert not path.exists(), f"should not promote after mention {i}" + else: + assert path.exists(), "should promote on the 3rd mention" + + text = path.read_text(encoding="utf-8") + assert "seen in doc-1" in text + assert "seen in doc-2" in text + assert "seen in doc-3" in text + assert '"summaries/doc-1.md"' in text + + class TestBriefIntegration: @pytest.mark.asyncio async def test_short_doc_briefs_in_index_and_frontmatter(self, tmp_path): @@ -2343,7 +2689,10 @@ async def test_entity_and_concept_split(self, tmp_path, monkeypatch): ) # Mocked LLM: plan call returns one concept + one entity; each - # generation call returns a tiny page. + # generation call returns a tiny page. The pending buffer's own + # note-generation calls (see openkb.pending) are labeled + # "concept-note: .../entity-note: ..." and need a "note" field + # instead of "content". def fake_llm(model, messages, label, **kw): if label == "concepts-plan": return json.dumps( @@ -2362,6 +2711,8 @@ def fake_llm(model, messages, label, **kw): }, } ) + if label.startswith("concept-note:") or label.startswith("entity-note:"): + return json.dumps({"description": "b", "note": "seen in doc"}) return json.dumps({"description": "b", "type": "organization", "content": "# Page\n"}) async def fake_llm_async(model, messages, label, **kw): @@ -2369,6 +2720,8 @@ async def fake_llm_async(model, messages, label, **kw): monkeypatch.setattr("openkb.agent.compiler._llm_call", fake_llm) monkeypatch.setattr("openkb.agent.compiler._llm_call_async", fake_llm_async) + _seed_pending(tmp_path, "concepts", "ai-demand", "AI Demand") + _seed_pending(tmp_path, "entities", "nvidia", "NVIDIA", type_="organization") from openkb.agent.compiler import _compile_concepts @@ -2489,6 +2842,8 @@ def fake_llm(model, messages, label, **kw): ) if label == "summary-rewrite": return "# Doc\n\nSee [[concepts/real-concept]] and [[concepts/ghost-concept]].\n" + if label.startswith("concept-note:"): + return json.dumps({"description": "b", "note": "seen in doc"}) # concept generation body references the non-existent ghost concept return json.dumps( {"brief": "b", "content": "# Real\n\nLinks [[concepts/ghost-concept]].\n"} @@ -2499,6 +2854,7 @@ async def fake_llm_async(model, messages, label, **kw): monkeypatch.setattr("openkb.agent.compiler._llm_call", fake_llm) monkeypatch.setattr("openkb.agent.compiler._llm_call_async", fake_llm_async) + _seed_pending(tmp_path, "concepts", "real-concept", "Real") from openkb.agent.compiler import _compile_concepts @@ -2553,6 +2909,8 @@ def fake_llm(model, messages, label, **kw): }, } ) + if label.startswith("entity-note:"): + return json.dumps({"description": "b", "note": "seen in doc"}) return json.dumps({"description": "b", "type": "dataset", "content": "# Page\n"}) async def fake_llm_async(model, messages, label, **kw): @@ -2561,6 +2919,7 @@ async def fake_llm_async(model, messages, label, **kw): monkeypatch.setattr("openkb.agent.compiler._llm_call", fake_llm) monkeypatch.setattr("openkb.agent.compiler._llm_call_async", fake_llm_async) + _seed_pending(tmp_path, "entities", "imagenet", "ImageNet", type_="dataset") from openkb.agent.compiler import _compile_concepts diff --git a/tests/test_config.py b/tests/test_config.py index 8254d8f68..752cc1aaa 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -20,6 +20,7 @@ resolve_litellm_settings, resolve_model_settings, resolve_parallel_tool_calls, + resolve_strict_entity_types, resolve_timeout, save_config, save_global_config, @@ -183,6 +184,35 @@ def test_concurrency_not_in_default_config(): assert "concurrency" not in DEFAULT_CONFIG +# --- strict_entity_types ------------------------------------------------------- + + +def test_strict_entity_types_default_in_config(): + assert DEFAULT_CONFIG["strict_entity_types"] is False + + +def test_strict_entity_types_not_in_global_scalar_keys(): + # KB config.yaml only (like concept_update_mode/debug) — not workbench/ + # global-editable. + assert "strict_entity_types" not in GLOBAL_SCALAR_KEYS + + +def test_resolve_strict_entity_types_absent_is_default(): + assert resolve_strict_entity_types({}) is False + + +def test_resolve_strict_entity_types_valid_values(): + assert resolve_strict_entity_types({"strict_entity_types": True}) is True + assert resolve_strict_entity_types({"strict_entity_types": False}) is False + + +def test_resolve_strict_entity_types_rejects_non_bool(caplog): + with caplog.at_level(logging.WARNING, logger="openkb.config"): + result = resolve_strict_entity_types({"strict_entity_types": "yes"}) + assert result is False + assert "strict_entity_types" in caplog.text + + def test_load_concurrency_override(tmp_path): config_path = tmp_path / "config.yaml" config_path.write_text("concurrency: 12\n", encoding="utf-8") diff --git a/tests/test_file_size.py b/tests/test_file_size.py index d41e43c85..40eeb4f2a 100644 --- a/tests/test_file_size.py +++ b/tests/test_file_size.py @@ -24,6 +24,7 @@ "openkb/cli.py", # monolithic Click entry point; split into command groups "openkb/agent/compiler.py", # LLM wiki compiler; split into focused units "openkb/agent/chat.py", # chat loop; extract cohesive concerns + "openkb/config.py", # dense config resolvers; consider splitting by concern } diff --git a/tests/test_pending.py b/tests/test_pending.py new file mode 100644 index 000000000..db68a4510 --- /dev/null +++ b/tests/test_pending.py @@ -0,0 +1,117 @@ +"""Tests for openkb.pending.PendingTopicsStore (see issue #247).""" + +from __future__ import annotations + +from openkb.pending import MAX_NOTES_BEFORE_PROMOTION, PendingTopicsStore + + +def test_new_store_has_no_pending_entries(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + assert store.get("concepts", "attention") is None + assert store.note_count("concepts", "attention") == 0 + assert store.brief_lines("concepts") == [] + + +def test_add_note_creates_entry_and_returns_count(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + count = store.add_note( + "concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "first note" + ) + assert count == 1 + entry = store.get("concepts", "attention") + assert entry["title"] == "Attention" + assert entry["notes"] == [ + { + "doc_name": "doc-1", + "source_file": "summaries/doc-1.md", + "note": "first note", + "date": entry["notes"][0]["date"], + } + ] + + +def test_add_note_accumulates_in_order(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + count = store.add_note( + "concepts", "attention", "Attention", "doc-2", "summaries/doc-2.md", "note 2" + ) + assert count == 2 + notes = store.get("concepts", "attention")["notes"] + assert [n["note"] for n in notes] == ["note 1", "note 2"] + + +def test_promotion_threshold_is_third_note(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + for i in range(MAX_NOTES_BEFORE_PROMOTION): + store.add_note( + "concepts", "attention", "Attention", f"doc-{i}", f"summaries/doc-{i}.md", f"note {i}" + ) + # Not yet promote-eligible after MAX_NOTES_BEFORE_PROMOTION notes. + assert store.note_count("concepts", "attention") == MAX_NOTES_BEFORE_PROMOTION + count = store.add_note( + "concepts", "attention", "Attention", "doc-final", "summaries/doc-final.md", "final note" + ) + assert count == MAX_NOTES_BEFORE_PROMOTION + 1 + + +def test_remove_clears_entry(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + store.remove("concepts", "attention") + assert store.get("concepts", "attention") is None + # Removing an absent entry is a no-op, not an error. + store.remove("concepts", "attention") + + +def test_entity_notes_carry_type(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note( + "entities", + "nvidia", + "NVIDIA", + "doc-1", + "summaries/doc-1.md", + "seen in doc-1", + type_="organization", + ) + entry = store.get("entities", "nvidia") + assert entry["type"] == "organization" + + +def test_brief_lines_format(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + store.add_note("concepts", "attention", "Attention", "doc-2", "summaries/doc-2.md", "note 2") + lines = store.brief_lines("concepts") + assert lines == [f"- attention (pending, 2/{MAX_NOTES_BEFORE_PROMOTION + 1} mentions) — note 2"] + + +def test_concepts_and_entities_are_independent_namespaces(tmp_path): + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note("concepts", "shared-name", "C", "doc-1", "summaries/doc-1.md", "concept note") + store.add_note("entities", "shared-name", "E", "doc-1", "summaries/doc-1.md", "entity note") + assert store.note_count("concepts", "shared-name") == 1 + assert store.note_count("entities", "shared-name") == 1 + store.remove("concepts", "shared-name") + assert store.get("concepts", "shared-name") is None + assert store.get("entities", "shared-name") is not None + + +def test_persistence_across_instances(tmp_path): + path = tmp_path / "pending_topics.json" + store1 = PendingTopicsStore(path) + store1.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + + store2 = PendingTopicsStore(path) + assert store2.note_count("concepts", "attention") == 1 + entry = store2.get("concepts", "attention") + assert entry["notes"][0]["note"] == "note 1" + + +def test_creates_parent_directory(tmp_path): + path = tmp_path / ".openkb" / "pending_topics.json" + assert not path.parent.exists() + store = PendingTopicsStore(path) + store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + assert path.exists() From b33ce06e2837d805ea3327b59db02681da2220e5 Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Wed, 9 Sep 2026 08:37:24 +0200 Subject: [PATCH 5/8] 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). --- config.yaml.example | 5 ++- examples/configuration/README.md | 7 +++-- openkb/agent/compiler.py | 45 +++++++++++++++++---------- tests/test_compiler.py | 53 ++++++++++++++++++++++++++++++-- 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/config.yaml.example b/config.yaml.example index 6408aeaec..5c0c0fc10 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -31,7 +31,10 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # entity_types (above) instead of coercing it to "other". Off by default # (backward compatible); turn on for a narrow, domain-specific entity_types # list where an "other"-typed entity usually signals a bad/too-specific -# candidate you'd rather drop than keep. +# candidate you'd rather drop than keep. Also enables a name-length gate: +# a brand-new entity name longer than 3 words is dropped too (concepts +# always enforce this cap; entities only opt in together with this flag). +# Drops are logged as warnings, never silent. # strict_entity_types: false # Optional: how concept/entity pages absorb new documents on `openkb add`. diff --git a/examples/configuration/README.md b/examples/configuration/README.md index a337299ea..e70d8390c 100644 --- a/examples/configuration/README.md +++ b/examples/configuration/README.md @@ -99,7 +99,10 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # entity_types (above) instead of coercing it to "other". Off by default # (backward compatible); turn on for a narrow, domain-specific entity_types # list where an "other"-typed entity usually signals a bad/too-specific -# candidate you'd rather drop than keep. +# candidate you'd rather drop than keep. Also enables a name-length gate: +# a brand-new entity name longer than 3 words is dropped too (concepts +# always enforce this cap; entities only opt in together with this flag). +# Drops are logged as warnings, never silent. # strict_entity_types: false # Optional: LLM / LiteLLM tuning. Keys are forwarded to LiteLLM; `timeout` and @@ -121,7 +124,7 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex | `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. | | `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). | | `entity_types` | 7 defaults | Custom vocabulary for entity pages. `other` is always kept. | -| `strict_entity_types` | `false` | When `true`, drops an entity whose type doesn't match `entity_types` instead of coercing it to `other`. | +| `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. | | `litellm:` | – | A pass-through block for LiteLLM. See below. | ### The `litellm:` block diff --git a/openkb/agent/compiler.py b/openkb/agent/compiler.py index c2cf9ea15..d2d56d566 100644 --- a/openkb/agent/compiler.py +++ b/openkb/agent/compiler.py @@ -92,7 +92,11 @@ # the token-density constant used to compute the soft per-document "how many # brand-new items" guidance substituted into __DOC_TOKEN_GUIDANCE__. Both are # intentionally NOT config keys (see issue #247) — the only new config-driven -# knob in this feature is strict_entity_types. +# knob in this feature is strict_entity_types. Concepts always enforce this +# cap; entities only enforce it when strict_entity_types=true (see +# _filter_entity_items) — it opts into the same "reject, don't coerce" spirit +# as the type check, so leaving strict_entity_types off keeps entity name +# length unrestricted, matching pre-issue-#247 behavior exactly. _MAX_NAME_WORDS = 3 _TOKENS_PER_NEW_ITEM = 1000 @@ -757,46 +761,52 @@ def _filter_entity_items( ``strict`` (see ``config.resolve_strict_entity_types``), when ``True``, drops an item whose type falls outside ``valid_types`` instead of coercing - it to ``"other"``. ``max_words`` (see :func:`_count_words`), when given, - drops names with more words than that. Both default to today's lenient - behavior (``False``/``None``); pass them only for "create" items — an - "update" targets an already-existing, already-vetted name/type. + it to ``"other"``, and additionally enables the ``max_words`` (see + :func:`_count_words`) name-length gate — both checks are opt-in together, + so leaving ``strict_entity_types`` at its default keeps today's lenient + behavior (no type drop, no length drop) exactly. Pass ``max_words`` only + for "create" items — an "update" targets an already-existing, + already-vetted name/type. Drops are logged at warning level (visible + without ``-v``) with a sample of the affected names, never silently. """ if valid_types is None: valid_types = _ENTITY_TYPES out: list[dict] = [] if not isinstance(items, list): return out - dropped_strict = 0 - dropped_words = 0 + dropped_strict: list[str] = [] + dropped_words: list[str] = [] for it in items: if not isinstance(it, dict): continue name = it.get("name") if not isinstance(name, str) or not name.strip(): continue - if max_words is not None and _count_words(name) > max_words: - dropped_words += 1 + if strict and max_words is not None and _count_words(name) > max_words: + dropped_words.append(name) continue title = it.get("title") if isinstance(it.get("title"), str) else name etype = it.get("type") if not isinstance(etype, str) or etype not in valid_types: if strict: - dropped_strict += 1 + dropped_strict.append(name) continue etype = "other" out.append({"name": name, "title": title, "type": etype}) if dropped_strict: - logger.info( + logger.warning( "concepts plan: dropped %d entity item(s) with type outside the configured " - "entity_types (strict_entity_types=true)", - dropped_strict, + "entity_types (strict_entity_types=true): %s", + len(dropped_strict), + dropped_strict[:5], ) if dropped_words: - logger.info( - "concepts plan: dropped %d entity item(s) with names over %d words", - dropped_words, + logger.warning( + "concepts plan: dropped %d entity item(s) with names over %d words " + "(strict_entity_types=true): %s", + len(dropped_words), max_words, + dropped_words[:5], ) return out @@ -813,7 +823,8 @@ def _parse_entities_plan( Returns ``{"create": [...], "update": [...], "related": [...]}``. A missing/malformed ``entities`` key yields empty lists, so older or partial LLM responses never raise. ``strict``/``max_words`` (see - :func:`_filter_entity_items`) are applied to "create" only — an "update" + :func:`_filter_entity_items` — the name-length gate only applies when + ``strict`` is ``True``) are applied to "create" only — an "update" targets an already-existing, already-vetted name/type. """ empty = {"create": [], "update": [], "related": []} diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 8d0f37732..d122ae1f2 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -226,7 +226,9 @@ def test_strict_true_keeps_matching_type(self): class TestMaxWordsFilter: """Hard cap on brand-new concept/entity names to 3 words (see compiler._count_words / issue #247) — a lightweight proxy for "too - specific to be reusable knowledge".""" + specific to be reusable knowledge". Concepts always enforce this cap; + entities only enforce it when strict=True (opt-in together with + strict_entity_types, see issue #247 follow-up).""" def test_count_words_splits_on_hyphen_underscore_and_space(self): assert _count_words("attention") == 1 @@ -248,19 +250,48 @@ def test_concept_items_without_max_words_are_unaffected(self): out = _filter_concept_items(items, "update") assert len(out) == 1 - def test_entity_items_over_limit_are_dropped(self): + def test_entity_items_over_limit_are_dropped_when_strict(self): items = [ {"name": "nvidia", "title": "NVIDIA", "type": "organization"}, {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "person"}, ] - out = _filter_entity_items(items, max_words=3) + out = _filter_entity_items(items, max_words=3, strict=True) assert [e["name"] for e in out] == ["nvidia"] + def test_entity_items_over_limit_kept_when_not_strict(self): + # The word-length gate is opt-in together with strict_entity_types + # (see issue #247 follow-up) — passing max_words alone must not drop + # anything unless strict=True is also set. + items = [ + {"name": "nvidia", "title": "NVIDIA", "type": "organization"}, + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "person"}, + ] + out = _filter_entity_items(items, max_words=3, strict=False) + assert [e["name"] for e in out] == ["nvidia", "andreas-mueller-alwart-ssmpa-2573"] + def test_entity_items_without_max_words_are_unaffected(self): items = [{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "other"}] out = _filter_entity_items(items) assert len(out) == 1 + def test_dropped_entity_items_are_logged_at_warning_not_silently(self, caplog): + # Drops must be visible without -v/--verbose (root logger defaults to + # WARNING, see cli.py) — logging them at INFO would be effectively + # silent for a normal `openkb add` run. + import logging + + valid = frozenset({"person", "other"}) + items = [ + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "A", "type": "person"}, + {"name": "x", "title": "X", "type": "organization"}, + ] + with caplog.at_level(logging.WARNING, logger="openkb.agent.compiler"): + out = _filter_entity_items(items, valid, strict=True, max_words=3) + assert out == [] + messages = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING] + assert any("over 3 words" in m for m in messages) + assert any("type outside the configured" in m for m in messages) + class TestParseEntitiesPlanStrictAndMaxWords: """strict/max_words are threaded through _parse_entities_plan for @@ -301,6 +332,22 @@ def test_update_is_never_word_or_strict_filtered(self): assert len(out["update"]) == 1 assert out["update"][0]["type"] == "other" # coerced, not strict-dropped + def test_create_max_words_ignored_without_strict(self): + # The name-length gate is opt-in together with strict_entity_types — + # passing max_words without strict=True must not drop long names. + valid = frozenset({"person", "other"}) + parsed = { + "entities": { + "create": [ + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "A", "type": "person"}, + ], + "update": [], + "related": [], + } + } + out = _parse_entities_plan(parsed, valid, strict=False, max_words=3) + assert [e["name"] for e in out["create"]] == ["andreas-mueller-alwart-ssmpa-2573"] + class TestDocTokenGuidance: """Soft, textual __DOC_TOKEN_GUIDANCE__ substitution (see issue #247) — From c1562646ba43787f884f0e87b8dc12b5ac352b7f Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Wed, 9 Sep 2026 08:43:09 +0200 Subject: [PATCH 6/8] docs(config): note the entity name-length gate in strict_entity_types docstring --- openkb/config.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/openkb/config.py b/openkb/config.py index f4c3f854f..aad5cce21 100644 --- a/openkb/config.py +++ b/openkb/config.py @@ -172,9 +172,12 @@ def resolve_strict_entity_types(config: dict) -> bool: When ``True``, an entity whose LLM-returned ``type`` doesn't match the configured :func:`resolve_entity_types` vocabulary is dropped instead of - being coerced to ``"other"``. A non-bool value degrades to ``False`` with - a warning (matches :func:`resolve_concept_update_mode`'s degrade-on- - malformed-value behavior). + being coerced to ``"other"``, and a brand-new entity name longer than 3 + words is also dropped (see ``agent.compiler._filter_entity_items`` / + ``_MAX_NAME_WORDS``) — both gates are opt-in together. A non-bool value + degrades to ``False`` with a warning (matches + :func:`resolve_concept_update_mode`'s degrade-on-malformed-value + behavior). """ value = config.get("strict_entity_types", False) if not isinstance(value, bool): From 7bcca160ed8b11b4fa795678b89afe9e273ef1d7 Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Wed, 9 Sep 2026 09:26:27 +0200 Subject: [PATCH 7/8] fix(agent): rename strict_entity_types to strict_item_mode, gate concept name-length drop too - Renamed config key/resolver strict_entity_types/resolve_strict_entity_types to strict_item_mode/resolve_strict_item_mode: this single opt-in flag now controls BOTH the entity type-mismatch drop and the concept/entity name-length drop. - The concept name-length gate (previously unconditional) is now also gated behind strict_item_mode, matching the entity behavior from the prior commit -- leaving strict_item_mode at its default keeps concept names unrestricted in length, same as entities. - Concept name-length drops are now logged at WARNING (was INFO, invisible without -v) with a sample of the affected names, same treatment as entities. --- config.yaml.example | 17 +++++---- examples/configuration/README.md | 20 +++++----- openkb/agent/compiler.py | 63 +++++++++++++++++++------------- openkb/config.py | 32 +++++++++------- tests/test_compiler.py | 49 ++++++++++++++++++------- tests/test_config.py | 28 +++++++------- 6 files changed, 126 insertions(+), 83 deletions(-) diff --git a/config.yaml.example b/config.yaml.example index 5c0c0fc10..969249ab8 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -27,15 +27,16 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # - dataset # - model -# Optional: reject an entity whose LLM-returned type doesn't match -# entity_types (above) instead of coercing it to "other". Off by default -# (backward compatible); turn on for a narrow, domain-specific entity_types -# list where an "other"-typed entity usually signals a bad/too-specific -# candidate you'd rather drop than keep. Also enables a name-length gate: -# a brand-new entity name longer than 3 words is dropped too (concepts -# always enforce this cap; entities only opt in together with this flag). +# Optional: strict mode for brand-new concept/entity `create` items. Off by +# default (backward compatible). When true: +# - a brand-new concept or entity name longer than 3 words is dropped +# (a lightweight proxy for "too specific to be reusable knowledge"). +# - an entity whose LLM-returned type doesn't match entity_types (above) +# is dropped instead of coerced to "other" (entities only, concepts +# have no type) — useful for a narrow, domain-specific entity_types +# list where an "other"-typed entity usually signals a bad candidate. # Drops are logged as warnings, never silent. -# strict_entity_types: false +# strict_item_mode: false # Optional: how concept/entity pages absorb new documents on `openkb add`. # rewrite default — send the existing page's full body to the LLM and diff --git a/examples/configuration/README.md b/examples/configuration/README.md index e70d8390c..13dd951bc 100644 --- a/examples/configuration/README.md +++ b/examples/configuration/README.md @@ -95,15 +95,16 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex # - dataset # - model -# Optional: reject an entity whose LLM-returned type doesn't match -# entity_types (above) instead of coercing it to "other". Off by default -# (backward compatible); turn on for a narrow, domain-specific entity_types -# list where an "other"-typed entity usually signals a bad/too-specific -# candidate you'd rather drop than keep. Also enables a name-length gate: -# a brand-new entity name longer than 3 words is dropped too (concepts -# always enforce this cap; entities only opt in together with this flag). +# Optional: strict mode for brand-new concept/entity `create` items. Off by +# default (backward compatible). When true: +# - a brand-new concept or entity name longer than 3 words is dropped +# (a lightweight proxy for "too specific to be reusable knowledge"). +# - an entity whose LLM-returned type doesn't match entity_types (above) +# is dropped instead of coerced to "other" (entities only, concepts +# have no type) — useful for a narrow, domain-specific entity_types +# list where an "other"-typed entity usually signals a bad candidate. # Drops are logged as warnings, never silent. -# strict_entity_types: false +# strict_item_mode: false # Optional: LLM / LiteLLM tuning. Keys are forwarded to LiteLLM; `timeout` and # `extra_headers` apply per request, the rest are set as litellm.. @@ -124,7 +125,8 @@ pageindex_threshold: 20 # PDF pages threshold for PageIndex | `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. | | `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). | | `entity_types` | 7 defaults | Custom vocabulary for entity pages. `other` is always kept. | -| `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. | +| `strict_item_mode` | `false` | When `true`, drops a brand-new concept/entity name longer than 3 words, and drops an entity whose type doesn't match `entity_types` instead of coercing it to `other`. Drops are logged as warnings, never silent. | +| `debug` | `false` | Enable per-file debug logging to `logs/.log` (see below) without needing `-v` on every command. | | `litellm:` | – | A pass-through block for LiteLLM. See below. | ### The `litellm:` block diff --git a/openkb/agent/compiler.py b/openkb/agent/compiler.py index d2d56d566..c4ce1d3ae 100644 --- a/openkb/agent/compiler.py +++ b/openkb/agent/compiler.py @@ -92,11 +92,12 @@ # the token-density constant used to compute the soft per-document "how many # brand-new items" guidance substituted into __DOC_TOKEN_GUIDANCE__. Both are # intentionally NOT config keys (see issue #247) — the only new config-driven -# knob in this feature is strict_entity_types. Concepts always enforce this -# cap; entities only enforce it when strict_entity_types=true (see -# _filter_entity_items) — it opts into the same "reject, don't coerce" spirit -# as the type check, so leaving strict_entity_types off keeps entity name -# length unrestricted, matching pre-issue-#247 behavior exactly. +# knob in this feature is strict_item_mode. Both concepts and entities only +# enforce this cap when strict_item_mode=true (see _filter_concept_items / +# _filter_entity_items) — it opts into a "reject, don't keep" spirit (same +# as the entity type check), so leaving strict_item_mode off keeps concept +# and entity name length unrestricted, matching pre-issue-#247 behavior +# exactly. _MAX_NAME_WORDS = 3 _TOKENS_PER_NEW_ITEM = 1000 @@ -673,13 +674,19 @@ def _doc_token_guidance(doc_tokens: int | None) -> str: ) -def _filter_concept_items(items: list, label: str, *, max_words: int | None = None) -> list[dict]: +def _filter_concept_items( + items: list, label: str, *, strict: bool = False, max_words: int | None = None +) -> list[dict]: """Keep only dicts that carry a non-empty ``name``; warn about anything else. - ``max_words``, when given, additionally drops names with more words (see - :func:`_count_words`) than that. Pass ``None`` (the default; used for - "update" items, which target an already-existing, already-vetted name) to - skip this check. + ``strict`` (see ``config.resolve_strict_item_mode``), when ``True``, + enables the ``max_words`` (see :func:`_count_words`) name-length gate — + a name with more words than that is dropped. Leaving ``strict`` at its + default ``False`` keeps names unrestricted in length regardless of + ``max_words``. Pass ``max_words`` only for "create" items — an "update" + targets an already-existing, already-vetted name. Drops are logged at + warning level (visible without ``-v``) with a sample of the affected + names, never silently. """ if not isinstance(items, list): logger.warning( @@ -704,11 +711,12 @@ def _filter_concept_items(items: list, label: str, *, max_words: int | None = No label, ", ".join(sorted(set(reasons))), ) - if max_words is not None: + if strict and max_words is not None: too_long = [c for c in valid if _count_words(c["name"]) > max_words] if too_long: - logger.info( - "concepts plan: dropped %d %s item(s) with names over %d words: %s", + logger.warning( + "concepts plan: dropped %d %s item(s) with names over %d words " + "(strict_item_mode=true): %s", len(too_long), label, max_words, @@ -759,11 +767,11 @@ def _filter_entity_items( module-level ``_ENTITY_TYPES`` so callers that don't thread a config-driven set keep today's behavior. - ``strict`` (see ``config.resolve_strict_entity_types``), when ``True``, + ``strict`` (see ``config.resolve_strict_item_mode``), when ``True``, drops an item whose type falls outside ``valid_types`` instead of coercing it to ``"other"``, and additionally enables the ``max_words`` (see :func:`_count_words`) name-length gate — both checks are opt-in together, - so leaving ``strict_entity_types`` at its default keeps today's lenient + so leaving ``strict_item_mode`` at its default keeps today's lenient behavior (no type drop, no length drop) exactly. Pass ``max_words`` only for "create" items — an "update" targets an already-existing, already-vetted name/type. Drops are logged at warning level (visible @@ -796,14 +804,14 @@ def _filter_entity_items( if dropped_strict: logger.warning( "concepts plan: dropped %d entity item(s) with type outside the configured " - "entity_types (strict_entity_types=true): %s", + "entity_types (strict_item_mode=true): %s", len(dropped_strict), dropped_strict[:5], ) if dropped_words: logger.warning( "concepts plan: dropped %d entity item(s) with names over %d words " - "(strict_entity_types=true): %s", + "(strict_item_mode=true): %s", len(dropped_words), max_words, dropped_words[:5], @@ -1779,7 +1787,7 @@ async def _compile_concepts( rewrite_summary: bool = False, entity_types: list[str] | None = None, concept_update_mode: str = "rewrite", - strict_entity_types: bool = False, + strict_item_mode: bool = False, doc_tokens: int | None = None, bundle=None, ) -> None: @@ -1904,7 +1912,9 @@ def _write_v1_summary_stripped() -> None: if isinstance(parsed, list): plan = { - "create": _filter_concept_items(parsed, "list", max_words=_MAX_NAME_WORDS), + "create": _filter_concept_items( + parsed, "list", strict=strict_item_mode, max_words=_MAX_NAME_WORDS + ), "update": [], "related": [], } @@ -1915,13 +1925,16 @@ def _write_v1_summary_stripped() -> None: ) plan = { "create": _filter_concept_items( - concepts_group.get("create", []), "create", max_words=_MAX_NAME_WORDS + concepts_group.get("create", []), + "create", + strict=strict_item_mode, + max_words=_MAX_NAME_WORDS, ), "update": _filter_concept_items(concepts_group.get("update", []), "update"), "related": _filter_related_slugs(concepts_group.get("related", [])), } entities_plan = _parse_entities_plan( - parsed, valid_types, strict=strict_entity_types, max_words=_MAX_NAME_WORDS + parsed, valid_types, strict=strict_item_mode, max_words=_MAX_NAME_WORDS ) create_items = plan["create"] @@ -2772,7 +2785,7 @@ async def compile_short_doc( from openkb.config import ( resolve_concept_update_mode, resolve_effective_config, - resolve_strict_entity_types, + resolve_strict_item_mode, ) config = resolve_effective_config(kb_dir)[0] @@ -2842,7 +2855,7 @@ async def compile_short_doc( rewrite_summary=True, entity_types=entity_types, concept_update_mode=resolve_concept_update_mode(config), - strict_entity_types=resolve_strict_entity_types(config), + strict_item_mode=resolve_strict_item_mode(config), doc_tokens=doc_tokens, bundle=bundle, ) @@ -2870,7 +2883,7 @@ async def compile_long_doc( from openkb.config import ( resolve_concept_update_mode, resolve_effective_config, - resolve_strict_entity_types, + resolve_strict_item_mode, ) config = resolve_effective_config(kb_dir)[0] @@ -2941,7 +2954,7 @@ async def compile_long_doc( doc_type="pageindex", entity_types=entity_types, concept_update_mode=resolve_concept_update_mode(config), - strict_entity_types=resolve_strict_entity_types(config), + strict_item_mode=resolve_strict_item_mode(config), doc_tokens=doc_tokens, bundle=bundle, ) diff --git a/openkb/config.py b/openkb/config.py index aad5cce21..7112f37f6 100644 --- a/openkb/config.py +++ b/openkb/config.py @@ -40,10 +40,10 @@ # resolve_concept_update_mode(). KB config.yaml only (like `debug`), not # in GLOBAL_SCALAR_KEYS. "concept_update_mode": "rewrite", - # Opt-in gate for `entity_types:` — see resolve_strict_entity_types(). KB - # config.yaml only, not in GLOBAL_SCALAR_KEYS (same treatment as - # concept_update_mode above). - "strict_entity_types": False, + # Opt-in strict mode for brand-new concept/entity `create` items — see + # resolve_strict_item_mode(). KB config.yaml only, not in + # GLOBAL_SCALAR_KEYS (same treatment as concept_update_mode above). + "strict_item_mode": False, } VALID_CONCEPT_UPDATE_MODES: tuple[str, ...] = ("rewrite", "append") @@ -167,22 +167,28 @@ def resolve_concept_update_mode(config: dict) -> str: return value -def resolve_strict_entity_types(config: dict) -> bool: - """Resolve ``strict_entity_types:`` — ``False`` by default. +def resolve_strict_item_mode(config: dict) -> bool: + """Resolve ``strict_item_mode:`` — ``False`` by default. - When ``True``, an entity whose LLM-returned ``type`` doesn't match the - configured :func:`resolve_entity_types` vocabulary is dropped instead of - being coerced to ``"other"``, and a brand-new entity name longer than 3 - words is also dropped (see ``agent.compiler._filter_entity_items`` / - ``_MAX_NAME_WORDS``) — both gates are opt-in together. A non-bool value + When ``True``, a brand-new concept/entity ``create`` item is dropped + instead of kept whenever it looks like a bad/too-specific candidate: + + - a name longer than 3 words is dropped (see + ``agent.compiler._filter_concept_items`` / ``_filter_entity_items`` / + ``_MAX_NAME_WORDS``) — applies to both concepts and entities. + - an entity whose LLM-returned ``type`` doesn't match the configured + :func:`resolve_entity_types` vocabulary is dropped instead of being + coerced to ``"other"`` — entities only, concepts have no ``type``. + + All gates are opt-in together under this single flag. A non-bool value degrades to ``False`` with a warning (matches :func:`resolve_concept_update_mode`'s degrade-on-malformed-value behavior). """ - value = config.get("strict_entity_types", False) + value = config.get("strict_item_mode", False) if not isinstance(value, bool): logger.warning( - "config: 'strict_entity_types' must be a bool, got %r — using False.", + "config: 'strict_item_mode' must be a bool, got %r — using False.", value, ) return False diff --git a/tests/test_compiler.py b/tests/test_compiler.py index d122ae1f2..6985b9ace 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -199,10 +199,10 @@ def test_default_valid_types_backward_compat(self): assert out[0]["type"] == "organization" -class TestStrictEntityTypes: - """strict_entity_types=true (see openkb.config.resolve_strict_entity_types - / issue #247): a type outside the configured vocabulary drops the item - entirely instead of coercing it to "other".""" +class TestStrictItemMode: + """strict_item_mode=true (see openkb.config.resolve_strict_item_mode / + issue #247 follow-up): a type outside the configured vocabulary drops + the entity item entirely instead of coercing it to "other".""" def test_strict_false_still_coerces_to_other(self): valid = frozenset({"person", "dataset", "other"}) @@ -226,9 +226,9 @@ def test_strict_true_keeps_matching_type(self): class TestMaxWordsFilter: """Hard cap on brand-new concept/entity names to 3 words (see compiler._count_words / issue #247) — a lightweight proxy for "too - specific to be reusable knowledge". Concepts always enforce this cap; - entities only enforce it when strict=True (opt-in together with - strict_entity_types, see issue #247 follow-up).""" + specific to be reusable knowledge". Both concepts and entities only + enforce it when strict=True (opt-in together with strict_item_mode, + see issue #247 follow-up).""" def test_count_words_splits_on_hyphen_underscore_and_space(self): assert _count_words("attention") == 1 @@ -237,19 +237,40 @@ def test_count_words_splits_on_hyphen_underscore_and_space(self): assert _count_words("some_snake_case_name") == 4 assert _count_words("a name with spaces") == 4 - def test_concept_items_over_limit_are_dropped(self): + def test_concept_items_over_limit_are_dropped_when_strict(self): items = [ {"name": "attention", "title": "Attention"}, {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas"}, ] - out = _filter_concept_items(items, "create", max_words=3) + out = _filter_concept_items(items, "create", strict=True, max_words=3) assert [c["name"] for c in out] == ["attention"] + def test_concept_items_over_limit_kept_when_not_strict(self): + # The word-length gate is opt-in together with strict_item_mode — + # passing max_words alone must not drop anything unless strict=True + # is also set. + items = [ + {"name": "attention", "title": "Attention"}, + {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas"}, + ] + out = _filter_concept_items(items, "create", strict=False, max_words=3) + assert [c["name"] for c in out] == ["attention", "andreas-mueller-alwart-ssmpa-2573"] + def test_concept_items_without_max_words_are_unaffected(self): items = [{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas"}] - out = _filter_concept_items(items, "update") + out = _filter_concept_items(items, "update", strict=True) assert len(out) == 1 + def test_dropped_concept_items_are_logged_at_warning_not_silently(self, caplog): + import logging + + items = [{"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas"}] + with caplog.at_level(logging.WARNING, logger="openkb.agent.compiler"): + out = _filter_concept_items(items, "create", strict=True, max_words=3) + assert out == [] + messages = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING] + assert any("over 3 words" in m for m in messages) + def test_entity_items_over_limit_are_dropped_when_strict(self): items = [ {"name": "nvidia", "title": "NVIDIA", "type": "organization"}, @@ -259,9 +280,9 @@ def test_entity_items_over_limit_are_dropped_when_strict(self): assert [e["name"] for e in out] == ["nvidia"] def test_entity_items_over_limit_kept_when_not_strict(self): - # The word-length gate is opt-in together with strict_entity_types - # (see issue #247 follow-up) — passing max_words alone must not drop - # anything unless strict=True is also set. + # The word-length gate is opt-in together with strict_item_mode — + # passing max_words alone must not drop anything unless strict=True + # is also set. items = [ {"name": "nvidia", "title": "NVIDIA", "type": "organization"}, {"name": "andreas-mueller-alwart-ssmpa-2573", "title": "Andreas", "type": "person"}, @@ -333,7 +354,7 @@ def test_update_is_never_word_or_strict_filtered(self): assert out["update"][0]["type"] == "other" # coerced, not strict-dropped def test_create_max_words_ignored_without_strict(self): - # The name-length gate is opt-in together with strict_entity_types — + # The name-length gate is opt-in together with strict_item_mode — # passing max_words without strict=True must not drop long names. valid = frozenset({"person", "other"}) parsed = { diff --git a/tests/test_config.py b/tests/test_config.py index 752cc1aaa..fde21319a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -20,7 +20,7 @@ resolve_litellm_settings, resolve_model_settings, resolve_parallel_tool_calls, - resolve_strict_entity_types, + resolve_strict_item_mode, resolve_timeout, save_config, save_global_config, @@ -184,33 +184,33 @@ def test_concurrency_not_in_default_config(): assert "concurrency" not in DEFAULT_CONFIG -# --- strict_entity_types ------------------------------------------------------- +# --- strict_item_mode ------------------------------------------------------- -def test_strict_entity_types_default_in_config(): - assert DEFAULT_CONFIG["strict_entity_types"] is False +def test_strict_item_mode_default_in_config(): + assert DEFAULT_CONFIG["strict_item_mode"] is False -def test_strict_entity_types_not_in_global_scalar_keys(): +def test_strict_item_mode_not_in_global_scalar_keys(): # KB config.yaml only (like concept_update_mode/debug) — not workbench/ # global-editable. - assert "strict_entity_types" not in GLOBAL_SCALAR_KEYS + assert "strict_item_mode" not in GLOBAL_SCALAR_KEYS -def test_resolve_strict_entity_types_absent_is_default(): - assert resolve_strict_entity_types({}) is False +def test_resolve_strict_item_mode_absent_is_default(): + assert resolve_strict_item_mode({}) is False -def test_resolve_strict_entity_types_valid_values(): - assert resolve_strict_entity_types({"strict_entity_types": True}) is True - assert resolve_strict_entity_types({"strict_entity_types": False}) is False +def test_resolve_strict_item_mode_valid_values(): + assert resolve_strict_item_mode({"strict_item_mode": True}) is True + assert resolve_strict_item_mode({"strict_item_mode": False}) is False -def test_resolve_strict_entity_types_rejects_non_bool(caplog): +def test_resolve_strict_item_mode_rejects_non_bool(caplog): with caplog.at_level(logging.WARNING, logger="openkb.config"): - result = resolve_strict_entity_types({"strict_entity_types": "yes"}) + result = resolve_strict_item_mode({"strict_item_mode": "yes"}) assert result is False - assert "strict_entity_types" in caplog.text + assert "strict_item_mode" in caplog.text def test_load_concurrency_override(tmp_path): From 5c7a36123fcdb32229df83514a077fd7091a9ef7 Mon Sep 17 00:00:00 2001 From: Sebastian Braun Date: Thu, 10 Sep 2026 10:34:07 +0200 Subject: [PATCH 8/8] feat(agent): give pending topics a cumulative description instead of the raw last note (#247) PendingTopicsStore entries now carry a `description` (the LLM-generated one-sentence brief from the note-create call) instead of a `title` field. `brief_lines()` shows `- {slug} (pending, n/total mentions) - {description}` (slug is the sole identifier, matching how real concept/entity pages are listed today), replacing the previous unbounded raw last-note text - the main driver of pending-buffer bloat in the concepts-plan prompt. `_gen_pending_concept`/`_gen_pending_entity` now build an "earlier notes" context (via the new shared `_prior_notes_context` helper, factored out of the existing promotion-time logic) for the 2nd/3rd note too, so the description accumulates across all buffered notes for a topic instead of only reflecting the most recent one. The ~1800 legacy entries in the sf-case-wiki vault (which predate the `description` field) were migrated separately with a one-off external script (not part of this PR): their existing `title` value was reused as-is for `description` (no LLM re-run needed). Verified via regex (`"title"` -> 0 matches) and a structured count (1800/1800 entries have `description`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- openkb/agent/compiler.py | 36 ++++++++++++++---- openkb/agent/compiler_notes.py | 2 + openkb/pending.py | 25 ++++++++++--- tests/test_compiler.py | 6 ++- tests/test_pending.py | 67 ++++++++++++++++++++++++++-------- 5 files changed, 105 insertions(+), 31 deletions(-) diff --git a/openkb/agent/compiler.py b/openkb/agent/compiler.py index c4ce1d3ae..0801af73f 100644 --- a/openkb/agent/compiler.py +++ b/openkb/agent/compiler.py @@ -732,6 +732,19 @@ def _require_nonempty_content(content, name: str) -> None: raise ValueError(f"LLM returned empty content for concept {name!r}") +def _prior_notes_context(prior_notes: list[dict]) -> str: + """Format previously buffered pending notes as LLM context, or "" if none. + + Shared by the pending-note create prompt (each buffered mention updates + its ``description`` from all notes so far, see openkb.pending) and the + promotion-to-full-page prompt, so both cumulate the same way. + """ + if not prior_notes: + return "" + notes_ctx = "\n".join(f"- ({n['doc_name']}) {n['note']}" for n in prior_notes) + return f"Earlier notes about this topic from prior documents:\n{notes_ctx}" + + def _filter_related_slugs(items: list) -> list[str]: """Keep only non-empty string slugs; warn about anything else.""" if not isinstance(items, list): @@ -2246,6 +2259,7 @@ async def _gen_note_create(concept: dict) -> tuple[str, str, bool, str]: "content": compiler_notes._CONCEPT_NOTE_CREATE_USER.format( title=title, doc_name=doc_name, + extra_context="", ), }, ], @@ -2300,6 +2314,7 @@ async def _gen_entity_note_create(ent: dict) -> tuple[str, str, str, str]: title=title, type=etype, doc_name=doc_name, + extra_context="", ), }, ], @@ -2351,6 +2366,8 @@ async def _gen_pending_concept(concept: dict) -> tuple[str, str] | None: name = concept["name"] title = concept.get("title", name) slug = _sanitize_concept_name(name) + prior_entry = pending_store.get("concepts", slug) + note_extra_context = _prior_notes_context(prior_entry["notes"] if prior_entry else []) async with semaphore: raw = await _llm_call_page_async( model, @@ -2361,7 +2378,7 @@ async def _gen_pending_concept(concept: dict) -> tuple[str, str] | None: { "role": "user", "content": compiler_notes._CONCEPT_NOTE_CREATE_USER.format( - title=title, doc_name=doc_name + title=title, doc_name=doc_name, extra_context=note_extra_context ), }, ], @@ -2371,7 +2388,7 @@ async def _gen_pending_concept(concept: dict) -> tuple[str, str] | None: ) brief, note = compiler_notes.note_fields(raw) _require_nonempty_content(note, name) - new_count = pending_store.add_note("concepts", slug, title, doc_name, source_file, note) + new_count = pending_store.add_note("concepts", slug, brief, doc_name, source_file, note) if new_count <= MAX_NOTES_BEFORE_PROMOTION: return # still buffering — no page yet entry = pending_store.get("concepts", slug) @@ -2386,8 +2403,7 @@ async def _gen_pending_concept(concept: dict) -> tuple[str, str] | None: wiki_dir, name, note, source_file, doc_name, description=brief ) return - notes_ctx = "\n".join(f"- ({n['doc_name']}) {n['note']}" for n in prior_notes) - extra_context = f"Earlier notes about this topic from prior documents:\n{notes_ctx}" + extra_context = _prior_notes_context(prior_notes) _, content, _, brief2 = await _gen_create(concept, extra_context=extra_context) cleaned, ghosts = strip_ghost_wikilinks(content, known_targets) if ghosts: @@ -2413,6 +2429,8 @@ async def _gen_pending_entity(ent: dict) -> tuple[str, str, str] | None: title = ent.get("title", name) etype = ent.get("type", "other") slug = _sanitize_concept_name(name) + prior_entry = pending_store.get("entities", slug) + note_extra_context = _prior_notes_context(prior_entry["notes"] if prior_entry else []) async with semaphore: raw = await _llm_call_page_async( model, @@ -2423,7 +2441,10 @@ async def _gen_pending_entity(ent: dict) -> tuple[str, str, str] | None: { "role": "user", "content": compiler_notes._ENTITY_NOTE_CREATE_USER.format( - title=title, type=etype, doc_name=doc_name + title=title, + type=etype, + doc_name=doc_name, + extra_context=note_extra_context, ), }, ], @@ -2434,7 +2455,7 @@ async def _gen_pending_entity(ent: dict) -> tuple[str, str, str] | None: brief, note = compiler_notes.note_fields(raw) _require_nonempty_content(note, name) new_count = pending_store.add_note( - "entities", slug, title, doc_name, source_file, note, type_=etype + "entities", slug, brief, doc_name, source_file, note, type_=etype ) if new_count <= MAX_NOTES_BEFORE_PROMOTION: return # still buffering — no page yet @@ -2456,8 +2477,7 @@ async def _gen_pending_entity(ent: dict) -> tuple[str, str, str] | None: wiki_dir, name, note, source_file, doc_name, description=brief, type_=etype ) return - notes_ctx = "\n".join(f"- ({n['doc_name']}) {n['note']}" for n in prior_notes) - extra_context = f"Earlier notes about this topic from prior documents:\n{notes_ctx}" + extra_context = _prior_notes_context(prior_notes) _, content, brief2, etype_out = await _gen_entity_create(ent, extra_context=extra_context) cleaned, ghosts = strip_ghost_wikilinks(content, known_targets) if ghosts: diff --git a/openkb/agent/compiler_notes.py b/openkb/agent/compiler_notes.py index 5e8f9a2b3..7be715311 100644 --- a/openkb/agent/compiler_notes.py +++ b/openkb/agent/compiler_notes.py @@ -34,6 +34,7 @@ This is a NEW concept page: {title} This concept was just identified in document "{doc_name}" (summarized above). +{extra_context} Return a JSON object with two keys: - "description": A single sentence (under 100 chars) defining this concept @@ -62,6 +63,7 @@ This is a NEW entity page: {title} (type: {type}) This entity was just identified in document "{doc_name}" (summarized above). +{extra_context} Return a JSON object with two keys: - "description": A single sentence (under 100 chars) identifying this entity diff --git a/openkb/pending.py b/openkb/pending.py index 80fe7b6e6..f6557c740 100644 --- a/openkb/pending.py +++ b/openkb/pending.py @@ -58,7 +58,7 @@ def add_note( self, kind: str, slug: str, - title: str, + description: str, doc_name: str, source_file: str, note: str, @@ -66,11 +66,17 @@ def add_note( ) -> int: """Append a note for ``slug``, creating the entry if it's missing. + ``description`` is the LLM-generated one-sentence brief for this note + (same value a real page's frontmatter ``description`` would get) — + always overwritten with the latest call's value, not accumulated as a + list, so it reflects the most recent summary of the topic across all + buffered notes so far. + Returns the new total note count (callers promote once this reaches ``MAX_NOTES_BEFORE_PROMOTION + 1``, i.e. the 3rd note). """ - entry = self._data[kind].setdefault(slug, {"title": title, "notes": []}) - entry["title"] = title + entry = self._data[kind].setdefault(slug, {"description": description, "notes": []}) + entry["description"] = description if type_ is not None: entry["type"] = type_ entry["notes"].append( @@ -91,7 +97,7 @@ def remove(self, kind: str, slug: str) -> None: self._persist() def brief_lines(self, kind: str) -> list[str]: - """Return ``- {slug} (pending, {n}/{total} mentions) — {last note}`` lines. + """Return ``- {slug} (pending, {n}/{total} mentions) — {description}`` lines. Extends the plan call's existing-page briefs so the LLM treats pending topics like quasi-existing pages for dedup ("prefer update"/ @@ -99,13 +105,20 @@ def brief_lines(self, kind: str) -> list[str]: must NEVER be added to the wikilink whitelist (no real page exists yet) — enforcing that is the caller's responsibility, not this method's. + + ``entry["description"]`` is read directly (no fallback): every entry + is guaranteed to have one, either from ``add_note`` or from the + one-time migration of legacy entries that predate this field (see + issue #247 follow-up) — a missing field is a genuine data bug, so it + raises a plain ``KeyError`` rather than degrading silently. """ total = MAX_NOTES_BEFORE_PROMOTION + 1 lines: list[str] = [] for slug, entry in self._data[kind].items(): notes = entry.get("notes", []) - last = notes[-1]["note"] if notes else "" - lines.append(f"- {slug} (pending, {len(notes)}/{total} mentions) — {last}") + lines.append( + f"- {slug} (pending, {len(notes)}/{total} mentions) — {entry['description']}" + ) return lines def _persist(self) -> None: diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 6985b9ace..5eed96cdd 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -1345,7 +1345,9 @@ async def side_effect(*args, **kwargs): return side_effect -def _seed_pending(kb_dir, kind: str, slug: str, title: str, n: int = 2, type_: str | None = None): +def _seed_pending( + kb_dir, kind: str, slug: str, description: str, n: int = 2, type_: str | None = None +): """Pre-seed the pending-topics buffer (see openkb.pending / issue #247) so the NEXT mention of ``slug`` promotes it to a real page instead of just buffering another note — lets create-path tests written before the @@ -1356,7 +1358,7 @@ def _seed_pending(kb_dir, kind: str, slug: str, title: str, n: int = 2, type_: s store.add_note( kind, slug, - title, + description, f"prior-doc-{i}", f"summaries/prior-doc-{i}.md", f"prior note {i}", diff --git a/tests/test_pending.py b/tests/test_pending.py index db68a4510..e5cf4a83b 100644 --- a/tests/test_pending.py +++ b/tests/test_pending.py @@ -2,6 +2,8 @@ from __future__ import annotations +import pytest + from openkb.pending import MAX_NOTES_BEFORE_PROMOTION, PendingTopicsStore @@ -15,11 +17,16 @@ def test_new_store_has_no_pending_entries(tmp_path): def test_add_note_creates_entry_and_returns_count(tmp_path): store = PendingTopicsStore(tmp_path / "pending_topics.json") count = store.add_note( - "concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "first note" + "concepts", + "attention", + "A mechanism for weighting input relevance.", + "doc-1", + "summaries/doc-1.md", + "first note", ) assert count == 1 entry = store.get("concepts", "attention") - assert entry["title"] == "Attention" + assert entry["description"] == "A mechanism for weighting input relevance." assert entry["notes"] == [ { "doc_name": "doc-1", @@ -32,32 +39,46 @@ def test_add_note_creates_entry_and_returns_count(tmp_path): def test_add_note_accumulates_in_order(tmp_path): store = PendingTopicsStore(tmp_path / "pending_topics.json") - store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + store.add_note("concepts", "attention", "desc 1", "doc-1", "summaries/doc-1.md", "note 1") count = store.add_note( - "concepts", "attention", "Attention", "doc-2", "summaries/doc-2.md", "note 2" + "concepts", "attention", "desc 2", "doc-2", "summaries/doc-2.md", "note 2" ) assert count == 2 notes = store.get("concepts", "attention")["notes"] assert [n["note"] for n in notes] == ["note 1", "note 2"] +def test_add_note_overwrites_description_not_accumulates(tmp_path): + """The 2nd (or later) note's description replaces the previous one — it's + a single cumulative summary field, not a list of past descriptions.""" + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note( + "concepts", "attention", "desc from note 1", "doc-1", "summaries/doc-1.md", "note 1" + ) + store.add_note( + "concepts", "attention", "desc from note 2", "doc-2", "summaries/doc-2.md", "note 2" + ) + entry = store.get("concepts", "attention") + assert entry["description"] == "desc from note 2" + + def test_promotion_threshold_is_third_note(tmp_path): store = PendingTopicsStore(tmp_path / "pending_topics.json") for i in range(MAX_NOTES_BEFORE_PROMOTION): store.add_note( - "concepts", "attention", "Attention", f"doc-{i}", f"summaries/doc-{i}.md", f"note {i}" + "concepts", "attention", f"desc {i}", f"doc-{i}", f"summaries/doc-{i}.md", f"note {i}" ) # Not yet promote-eligible after MAX_NOTES_BEFORE_PROMOTION notes. assert store.note_count("concepts", "attention") == MAX_NOTES_BEFORE_PROMOTION count = store.add_note( - "concepts", "attention", "Attention", "doc-final", "summaries/doc-final.md", "final note" + "concepts", "attention", "final desc", "doc-final", "summaries/doc-final.md", "final note" ) assert count == MAX_NOTES_BEFORE_PROMOTION + 1 def test_remove_clears_entry(tmp_path): store = PendingTopicsStore(tmp_path / "pending_topics.json") - store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + store.add_note("concepts", "attention", "desc", "doc-1", "summaries/doc-1.md", "note 1") store.remove("concepts", "attention") assert store.get("concepts", "attention") is None # Removing an absent entry is a no-op, not an error. @@ -69,7 +90,7 @@ def test_entity_notes_carry_type(tmp_path): store.add_note( "entities", "nvidia", - "NVIDIA", + "A semiconductor and AI computing company.", "doc-1", "summaries/doc-1.md", "seen in doc-1", @@ -81,16 +102,32 @@ def test_entity_notes_carry_type(tmp_path): def test_brief_lines_format(tmp_path): store = PendingTopicsStore(tmp_path / "pending_topics.json") - store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") - store.add_note("concepts", "attention", "Attention", "doc-2", "summaries/doc-2.md", "note 2") + store.add_note("concepts", "attention", "desc 1", "doc-1", "summaries/doc-1.md", "note 1") + store.add_note("concepts", "attention", "desc 2", "doc-2", "summaries/doc-2.md", "note 2") lines = store.brief_lines("concepts") - assert lines == [f"- attention (pending, 2/{MAX_NOTES_BEFORE_PROMOTION + 1} mentions) — note 2"] + assert lines == [f"- attention (pending, 2/{MAX_NOTES_BEFORE_PROMOTION + 1} mentions) — desc 2"] + + +def test_brief_lines_requires_description(tmp_path): + """A legacy entry that predates the ``description`` field (see the + one-time migration for issue #247) is a genuine data bug by the time + ``brief_lines`` runs — it fails loudly with a plain ``KeyError`` rather + than degrading silently, no dedicated guard/exception class needed.""" + store = PendingTopicsStore(tmp_path / "pending_topics.json") + store.add_note("concepts", "attention", "desc", "doc-1", "summaries/doc-1.md", "note 1") + del store.get("concepts", "attention")["description"] + with pytest.raises(KeyError): + store.brief_lines("concepts") def test_concepts_and_entities_are_independent_namespaces(tmp_path): store = PendingTopicsStore(tmp_path / "pending_topics.json") - store.add_note("concepts", "shared-name", "C", "doc-1", "summaries/doc-1.md", "concept note") - store.add_note("entities", "shared-name", "E", "doc-1", "summaries/doc-1.md", "entity note") + store.add_note( + "concepts", "shared-name", "concept desc", "doc-1", "summaries/doc-1.md", "concept note" + ) + store.add_note( + "entities", "shared-name", "entity desc", "doc-1", "summaries/doc-1.md", "entity note" + ) assert store.note_count("concepts", "shared-name") == 1 assert store.note_count("entities", "shared-name") == 1 store.remove("concepts", "shared-name") @@ -101,7 +138,7 @@ def test_concepts_and_entities_are_independent_namespaces(tmp_path): def test_persistence_across_instances(tmp_path): path = tmp_path / "pending_topics.json" store1 = PendingTopicsStore(path) - store1.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + store1.add_note("concepts", "attention", "desc", "doc-1", "summaries/doc-1.md", "note 1") store2 = PendingTopicsStore(path) assert store2.note_count("concepts", "attention") == 1 @@ -113,5 +150,5 @@ def test_creates_parent_directory(tmp_path): path = tmp_path / ".openkb" / "pending_topics.json" assert not path.parent.exists() store = PendingTopicsStore(path) - store.add_note("concepts", "attention", "Attention", "doc-1", "summaries/doc-1.md", "note 1") + store.add_note("concepts", "attention", "desc", "doc-1", "summaries/doc-1.md", "note 1") assert path.exists()