Skip to content

fix(compiler): cap concept/entity brief lists in the plan prompt - #268

Closed
rodrigedilson-ia wants to merge 1 commit into
VectifyAI:mainfrom
rodrigedilson-ia:fix/cap-plan-brief-lists
Closed

rodrigedilson-ia wants to merge 1 commit into
VectifyAI:mainfrom
rodrigedilson-ia:fix/cap-plan-brief-lists

Conversation

@rodrigedilson-ia

Copy link
Copy Markdown

Problem

_read_concept_briefs and _read_entity_briefs read every page in the KB, and the result is rebuilt into the concepts-plan prompt for each compiled document. Their size is therefore O(KB), with no ceiling — so a sufficiently large KB pushes that call past the model's context window.

The failure mode is unkind: it lands as a hard error partway through a recompile, not as degraded output. And because it scales with the KB rather than with the document, every retry is guaranteed to fail the same way.

We hit this on a KB of ~260 documents: seven consecutive recompile runs exited non-zero, all of them at the plan step, with prompts measured between 200k and 356k tokens against a 200k limit.

Change

A per-list character budget, briefs_budget_chars, default 120_000 (roughly 30k tokens each) — a KB has to grow well past a few hundred pages before anything is trimmed. Set it to 0 to restore the previous uncapped behavior.

Two details worth review attention:

Ranking is by source count, not alphabetical. That count is already the cross-document recurrence signal the plan call uses for create-vs-update, so it is also the most defensible thing to keep when the list has to be cut. Ties break alphabetically — prompt caching depends on byte-identical prefixes, and an unstable order would silently cost cache hits.

The trim is announced inside the list. A shortened list that reads as complete is worse than a long one here: the planner uses these briefs to decide create-vs-update, so a silently dropped concept comes back as a duplicate page for something the KB already has. The trailing marker states how many were omitted and tells the model to prefer update when unsure.

In the degenerate case where the budget cannot fit even one line, the output says the list was omitted rather than returning an empty one — an empty list reads as an empty KB, which would push the planner to recreate everything.

Compatibility

Small KBs produce byte-identical output to before (covered by test_under_budget_is_unchanged), so existing prompt caches are not invalidated.

Verification

ruff check .          # clean
ruff format --check   # clean
mypy openkb           # Success: no issues found in 56 source files
pytest                # 1089 passed

New tests in tests/test_compiler.py: TestBriefsBudget (under-budget identity, salience ranking, announced truncation, 0 disables, degenerate budget, stable ordering, entities) and TestResolveBriefsBudget (config fallback on a typo — a bad value should not abort a compile).

The four tests/test_api*.py modules were not run locally: they need the optional fastapi extra, and they are untouched by this change.

`_read_concept_briefs` and `_read_entity_briefs` read every page in the KB and
the result is rebuilt into the concepts-plan prompt for each compiled document.
Their size is therefore O(KB), with no ceiling — so a large KB eventually
pushes that call past the model's context window.

The failure mode is unkind: it lands as a hard error partway through a
recompile, not as degraded output. We hit it on a KB of ~260 documents, where
seven consecutive `recompile` runs exited non-zero, all of them at the plan
step, with prompts measured between 200k and 356k tokens against a 200k limit.
Because it scales with the KB, every retry was guaranteed to fail the same way.

This adds a per-list character budget (`briefs_budget_chars`, default 120_000 —
roughly 30k tokens each, so a KB has to grow well past a few hundred pages
before anything is trimmed). Set it to 0 to restore the previous uncapped
behavior.

Two details worth calling out:

- **Ranking is by source count, not alphabetical.** That count is already the
  cross-document recurrence signal the plan call uses for create-vs-update, so
  it is also the right thing to keep when the list has to be cut. Ties break
  alphabetically, because prompt caching depends on byte-identical prefixes and
  an unstable order would silently cost cache hits.

- **The trim is announced in the list.** A shortened list that reads as
  complete is worse than a long one: the planner uses these briefs to decide
  create-vs-update, so a silently dropped concept comes back as a duplicate
  page for something the KB already has. The trailing marker states how many
  were omitted and tells the model to prefer `update` when unsure. In the
  degenerate case where the budget cannot fit even one line, the output says
  the list was omitted rather than returning an empty one — an empty list reads
  as an empty KB, which would push the planner to recreate everything.

Small KBs are byte-identical to before (covered by a test), so prompt caches
are not invalidated for existing users.
@rodrigedilson-ia

Copy link
Copy Markdown
Author

Closing this — we're handling it on our side instead of upstreaming, so I don't want to leave a PR open that I won't follow through on. Apologies for the noise.

Leaving the finding here in case it's useful to anyone: _read_concept_briefs and _read_entity_briefs read every page in the KB, and the result goes into the concepts-plan prompt for each compiled document — so prompt size is O(KB). On a ~260-document KB we saw seven consecutive recompiles fail at the plan step, with prompts between 200k and 356k tokens against a 200k limit. Because it scales with the KB rather than with the document, retries fail identically.

The branch stays on my fork if anyone wants to pick it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant