Skip to content

fix(retrieve): only count preference_full_count on actual full render#3177

Open
nankingjing wants to merge 2 commits into
volcengine:mainfrom
nankingjing:fix/preference-full-count-budget-check
Open

fix(retrieve): only count preference_full_count on actual full render#3177
nankingjing wants to merge 2 commits into
volcengine:mainfrom
nankingjing:fix/preference-full-count-budget-check

Conversation

@nankingjing

Copy link
Copy Markdown
Contributor

Bug

In openviking/retrieve/type_quota_recall.py::search_type_quota_recall, the preference full-render cap counter (preference_full_count) was incremented before the budget check:

if memory_type == "preferences":
    can_try_full = preference_full_count < PREFERENCE_FULL_LIMIT
    preference_full_count += 1   # BUG: incremented unconditionally

So the first PREFERENCE_FULL_LIMIT preference memories that failed the per-type character budget (e.g. oversized content) wasted the cap. Subsequent preferences that would have fit were denied full rendering even though their full fragments fit within both the per-type and global budgets.

Fix

Move the increment inside the successful-full branch so it only counts when a preference actually renders as "full":

if memory_type == "preferences":
    can_try_full = preference_full_count < PREFERENCE_FULL_LIMIT
# ...budget check...
if (
    can_try_full
    and used_by_type.get(memory_type, 0) + full_chars
    <= budgets.get(memory_type, max_chars)
    and total_chars + full_chars <= max_chars
):
    mode = "full"
    entry_content = content
    fragment = full
    used_by_type[memory_type] = used_by_type.get(memory_type, 0) + full_chars
    total_chars += full_chars
    if memory_type == "preferences":
        preference_full_count += 1   # Only count on actual success

Regression test

tests/server/test_type_quota_preference_counter.py covers two scenarios:

  1. Counter no longer wasted on failure — first PREFERENCE_FULL_LIMIT entries have oversized content (5KB) that fails the budget; subsequent entries have small content. With the fix, the small entries render as "full" because the cap wasn't wasted on the oversized ones.
  2. Cap still enforced on success — when budget allows all entries to render full, exactly PREFERENCE_FULL_LIMIT render as "full"; later entries stay at summary/uri. Pins the invariant that the cap still works when content does fit.

The preference_full_count counter was incremented before the budget check,
so the first PREFERENCE_FULL_LIMIT preference memories that *failed* the
budget check (e.g. oversized content) wasted the cap. Subsequent
preferences that would have fit were denied full rendering even though
budget was still available.

Move the increment inside the successful-full branch so it only counts
when a preference actually renders as 'full'. Add a regression test
verifying that preferences after the limit can still render full when
the preceding ones overflowed the budget.
@nankingjing

Copy link
Copy Markdown
Contributor Author
Self-reviewed: diff is correct, minimal, and well-tested. No issues found.

@nankingjing

nankingjing commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Updated the regression test for current main after #3175 introduced parallel recall and content-hash dedupe. The test now uses unique memory content and asserts by URI identity while preserving the original invariant: only successful full renders consume the preference limit. Focused test: pytest tests/server/test_type_quota_preference_counter.py -v (2 passed).

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

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

1 participant