fix(compiler): guard concept/entity generation against malformed & truncated LLM output#161
Merged
Merged
Conversation
…uncated LLM output Two silent-data-loss bugs in _compile_concepts, both from trusting the per-page LLM response shape: - #158: a response returned as a JSON array (e.g. [{...}] or a multi-item list) reached .get() on a list and raised AttributeError, which the local except (JSONDecodeError, ValueError) did not catch — the page was dropped and, because the doc index records it as written, never retried. New _parse_page_json unwraps a single-element [{...}] array (recovering the common case) and returns None for other wrong shapes so the page is skipped cleanly instead of writing the raw JSON as its body. - #148: a response that hit finish_reason=length was repaired by json_repair and written anyway, overwriting an existing concept page with truncated content while still reporting [OK]. _warn_if_truncated now reports whether it truncated, and the four page-generation calls pass raise_on_truncation=True so a truncated response skips the write (existing page preserved). Other callers (plan, summary, overview) keep the warn-only behavior. Closes #158. Closes #148. Claude-Session: https://claude.ai/code/session_01UtbmJxjtw6FtP8fUXUKVtg
…r entity/prose paths Addresses code-review findings on the #158/#148 fix, no behavior change: - Collapse the four near-identical parse/fallback blocks in the concept and entity generation closures into a shared _page_fields() helper, so a future edge case is fixed in one place instead of four copies that can drift. - Add _llm_call_page_async() which hard-codes raise_on_truncation=True, so a page-generating call site can't silently forget the truncation guard and regress #148. - Tests: a _page_fields shape unit test (object / single-element-array unwrap / wrong-shape skip / non-JSON prose fallback) and an entity-path truncation-skip integration test. Claude-Session: https://claude.ai/code/session_01UtbmJxjtw6FtP8fUXUKVtg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes two silent-data-loss bugs in
_compile_concepts, both from trusting the per-page LLM response shape.#158 —
'list' object has no attribute 'get'When a model returns the page as a JSON array (e.g.
[{...}], or a multi-item list) instead of a bare object,parsed.get(...)hit alistand raisedAttributeError— which the localexcept (JSONDecodeError, ValueError)does not catch, so the page was dropped. Because the doc index records the concept as written, a re-run never retries it → permanent silent loss (the reporter saw 45–65% of concepts vanish on a weaker gateway model).New
_parse_page_jsonunwraps a single-element[{...}]array (recovers the common case) and returnsNonefor other wrong shapes so the page is skipped cleanly rather than crashing or writing the raw JSON text as the body. Applied to all four generation paths (concept/entity × create/update).#148 — truncated pages overwrite good ones
A response that hit
finish_reason=lengthwas repaired byjson_repairand written anyway — on the update path this overwrote an existing complete concept page with a cut-off body while still printing[OK]._warn_if_truncatednow reports whether it truncated, and the four page-generation calls passraise_on_truncation=Trueso a truncated response skips the write (existing page preserved) instead of clobbering it. Plan/summary/overview callers keep the previous warn-only behavior.Tests
tests/test_compiler.py: a_parse_page_jsonshape unit test, single-element-array recovery, truncated-update-preserves-existing, and truncated-create-skips-partial. All 4 fail onmainand pass here.Gate:
ruff check✓ ·ruff format --check✓ ·mypy openkb✓ ·pytest923 passed.Follow-up (not in this PR): now that these sites are shape-safe, the
union-attrsuppression in[[tool.mypy.overrides]]foropenkb.agent.compilercan eventually be narrowed — a few plan-path sites still need guards first. Tracked in tech-debt.Closes #158
Closes #148
https://claude.ai/code/session_01UtbmJxjtw6FtP8fUXUKVtg