Skip to content

Fix LRU eviction silent failure allowing unbounded memory growth (#449)#567

Merged
itomek merged 3 commits intomainfrom
fix/lru-eviction-449
Mar 19, 2026
Merged

Fix LRU eviction silent failure allowing unbounded memory growth (#449)#567
itomek merged 3 commits intomainfrom
fix/lru-eviction-449

Conversation

@itomek
Copy link
Collaborator

@itomek itomek commented Mar 18, 2026

Summary

  • Pre-flight rejection: _has_indexing_capacity() (new read-only method) checks limits before indexing starts. If at limit with eviction disabled, returns success=False with memory_limit_reached=True and a descriptive error — no silent growth.
  • Logging: _evict_lru_document() and _check_memory_limits() now emit structured self.log warnings/errors on all failure paths (disabled, no files, eviction failed, limit exceeded).
  • _check_memory_limits() returns bool: callers can now detect post-index limit violations; sets memory_limit_warning=True in stats.
  • Cache-load bug fix: pre-existing bug where cache-loaded files were never added to file_access_times/file_index_times, making them invisible to LRU eviction.
  • Stats enrichment: max_indexed_files and max_total_chunks always present in index_document() stats dict.
  • CLI flags: --max-indexed-files and --max-total-chunks wired through ChatAgentConfigRAGConfig for runtime configuration and UI testing.
  • 6 new unit tests in TestMemoryLimits covering all acceptance criteria.

Test plan

  • uv run python -m pytest tests/test_rag.py -xvs — 28/28 pass
  • python util/lint.py --all --fix — passes
  • UI test (requires --no-lru-eviction flag, not yet implemented): gaia chat --ui --max-indexed-files 2 --no-lru-eviction → upload 3 docs → 3rd rejected with "Memory limit reached" error
  • UI test (eviction path): gaia chat --ui --max-indexed-files 2 → upload 3 docs → 3rd succeeds, 1st evicted

Open items

  • --no-lru-eviction CLI flag not yet implemented — needed to fully validate the rejection path through the UI (see issue comments for full test instructions)

Closes #449

🤖 Generated with Claude Code

@itomek itomek self-assigned this Mar 18, 2026
@github-actions github-actions bot added agents Agent system changes rag RAG system changes cli CLI changes tests Test changes performance Performance-critical changes labels Mar 18, 2026
Base automatically changed from kalin/chat-ui to main March 18, 2026 17:43
@itomek itomek marked this pull request as ready for review March 18, 2026 18:28
@itomek itomek requested a review from kovtcharov-amd as a code owner March 18, 2026 18:28
@itomek itomek force-pushed the fix/lru-eviction-449 branch from 49f417f to 6cb61aa Compare March 19, 2026 17:54
@github-actions github-actions bot added the documentation Documentation changes label Mar 19, 2026
@itomek itomek marked this pull request as draft March 19, 2026 17:55
@itomek itomek force-pushed the fix/lru-eviction-449 branch from 6cb61aa to 34fbfa1 Compare March 19, 2026 18:02
@itomek itomek marked this pull request as ready for review March 19, 2026 18:05
@itomek itomek added this pull request to the merge queue Mar 19, 2026
Merged via the queue into main with commit 8a6452f Mar 19, 2026
49 checks passed
@itomek itomek deleted the fix/lru-eviction-449 branch March 19, 2026 18:05
itomek added a commit that referenced this pull request Mar 19, 2026
Forward --max-indexed-files to UI server via GAIA_MAX_INDEXED_FILES env
var. Add DB-level capacity check with LRU eviction in upload_by_path()
since per-upload RAGSDK instances can't track cross-upload state. Add
O_BINARY flag on Windows for safe_open_document() and RAGSDK._safe_open()
to prevent binary/text mode issues with fd-based file reads.
itomek and others added 3 commits March 19, 2026 18:01
- Add pre-flight capacity check (_has_indexing_capacity) that rejects
  indexing when memory limits are exceeded and eviction is disabled,
  returning success=False with memory_limit_reached=True and a
  descriptive error message
- Change _check_memory_limits() to return bool (True if limits met)
- Add structured logging to _evict_lru_document and _check_memory_limits
  for eviction attempts, failures, and limit violations
- Fix pre-existing bug: cache-load path never set file_access_times or
  file_index_times, making cached files invisible to LRU eviction
- Add max_indexed_files and max_total_chunks to index_document stats dict
- Add > 1 guard to files eviction loop (parity with chunks loop)
- Add 6 unit tests covering all acceptance criteria

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add --max-indexed-files and --max-total-chunks flags to `gaia chat`
so memory limits can be set at runtime for UI testing and validation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use monotonic counter for LRU ordering instead of time.time() (Windows resolution issue)
- Remove redundant default fallbacks in cli.py kwargs.get()
- Document --max-indexed-files and --max-total-chunks in CLI reference
- Rename misleading test and add proper eviction-failure test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent system changes cli CLI changes documentation Documentation changes performance Performance-critical changes rag RAG system changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix LRU eviction silent failure allowing unbounded memory growth

2 participants