Skip to content

feat(proxy): lazy memory embedder warmup + /debug/rss endpoint (memory-arch-v2) - #623

Open
neogenix wants to merge 3 commits into
headroomlabs-ai:mainfrom
neogenix:feat/memory-architecture-v2
Open

feat(proxy): lazy memory embedder warmup + /debug/rss endpoint (memory-arch-v2)#623
neogenix wants to merge 3 commits into
headroomlabs-ai:mainfrom
neogenix:feat/memory-architecture-v2

Conversation

@neogenix

@neogenix neogenix commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Description

Adds opt-in lazy loading for the memory embedder and a loopback-only RSS diagnostic endpoint. This reconciles the original memory-architecture-v2 work with the current cross-platform memory tracker and avoids forcing garbage collection from a request handler.

Closes #

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Code refactoring (no functional changes)

Changes Made

  • Add HEADROOM_SKIP_MEMORY_WARMUP as an explicit opt-in; eager warmup remains the default.
  • Add loopback-only GET /debug/rss with current RSS, best-effort cross-platform peak RSS, GC/object statistics, model/cache counters, and warmup status.
  • Use the existing MemoryTracker for current process and component statistics.
  • Keep Windows supported when the POSIX resource module is unavailable.
  • Avoid invoking gc.collect() from the endpoint.
  • Document the new environment variable.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom)
  • New tests added for new functionality
  • Manual testing performed

Test Output

$ uv run --extra dev pytest -q tests/test_memory_handler_concurrent_init.py tests/test_memory_handler_native_ops.py tests/test_memory_arch_v2.py -x
49 passed, 1 warning in 32.69s

$ uv run --extra dev pytest -q tests/test_memory_arch_v2.py tests/test_memory_usage_integration.py -x
15 passed, 1 skipped in 3.56s

$ uv run ruff format --check headroom/proxy/server.py tests/test_memory_arch_v2.py
2 files already formatted

$ uv run ruff check headroom/proxy/server.py tests/test_memory_arch_v2.py
All checks passed!

Focused mypy found no new error in this change after the diagnostic snapshot annotations were corrected; four pre-existing server.py errors remain outside this PR.

Real Behavior Proof

  • Environment: macOS, Python 3.12.13, FastAPI TestClient
  • Exact command / steps: request /debug/rss from loopback and a non-loopback peer; start proxies with the skip flag set to each supported truthy value and with it unset.
  • Observed result: loopback receives the complete diagnostic schema; a non-loopback peer receives 404; truthy flags skip warmup; the default still warms exactly once.
  • Not tested: live Windows process metrics and a production memory backend under traffic.

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I did not edit CHANGELOG.md — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)

Screenshots (if applicable)

N/A — this changes proxy startup behavior and a JSON diagnostics endpoint; there is no graphical UI.

Additional Notes

peak_rss_mb is null on platforms without resource; rss_mb remains available through the current cross-platform tracker. hnsw_elements is currently null because current per-user vector indexes are not globally enumerable without reaching into backend-private state.

@neogenix
neogenix marked this pull request as ready for review June 5, 2026 04:01
@neogenix
neogenix force-pushed the feat/memory-architecture-v2 branch from e6a1745 to dc1915d Compare June 9, 2026 17:11
@JerrettDavis

Copy link
Copy Markdown
Collaborator

Thanks. The skip-warmup option and /debug/rss endpoint are useful, but this is not ready in its current state.

Please rebase/update against current main and fix the failing lint check. Also make sure the /debug/rss tests still pass after the Host-header loopback guard work: TestClient fixtures for guarded debug endpoints should pin both the loopback client address and a loopback base_url, otherwise the new Host-header gate will correctly return 404.

Once the resolved branch is green, this should be worth a focused review.

@neogenix
neogenix force-pushed the feat/memory-architecture-v2 branch from dc1915d to c6574f0 Compare June 10, 2026 21:05
@github-actions github-actions Bot added the status: ci failing Required or reported CI checks are failing label Jun 10, 2026
@JerrettDavis

Copy link
Copy Markdown
Collaborator

Thanks for the follow-up. The latest branch is still dirty and lint is failing, so it needs another refresh before final review.

Please rebase/update against current main, fix the lint failure, and rerun CI. The debug endpoint and lazy warmup pieces are worth reviewing once the branch is mergeable and green.

@github-actions github-actions Bot added status: has conflicts Pull request has merge conflicts with the base branch and removed status: has conflicts Pull request has merge conflicts with the base branch labels Jun 11, 2026
@JerrettDavis JerrettDavis added status: has conflicts Pull request has merge conflicts with the base branch status: needs author action Pull request body or readiness checklist still needs author updates labels Jul 9, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not merge-ready in its current state. The branch is DIRTY against current main, the checks are from June 10, and the latest CI already had a failing lint job on this head. Since this PR changes headroom/proxy/server.py and adds a debug endpoint / warmup behavior, it needs a clean current-base CI run before the behavior can be reviewed confidently.\n\nPlease merge or rebase current headroomlabs/main, remove the manual changelog edit, fix the lint failure, and rerun the focused memory-arch tests plus the normal proxy CI.

@JerrettDavis

Copy link
Copy Markdown
Collaborator

Conflict audit against current main: Option F is superseded by the loopback-only /debug/memory endpoint and MemoryTracker, which already reports process RSS plus registered cache/index/logger component usage. Option D is not safely mergeable from the historical server hunk because startup now tracks memory backend/embedder loading/loaded/null/error lifecycle explicitly and supports multiple backend initialization paths.

A current port should add a validated lazy-warmup setting to that lifecycle, ensure the status endpoint distinguishes intentionally deferred from unavailable/failed, and test first-request initialization/concurrency. Restoring the old 140-line server change or dropping the debug half would either regress current observability or alter stated scope. Deferred for that focused port.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

PR governance

This PR does not yet satisfy the required template fields:

  • Missing required section Runtime Rollout Safety.

Please update the PR body, or move the PR back to draft while it is still in progress.

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs author action Pull request body or readiness checklist still needs author updates status: has conflicts Pull request has merge conflicts with the base branch status: ci failing Required or reported CI checks are failing status: ready for review Pull request body is complete and the author marked it ready for human review labels Aug 6, 2026
@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 30.00000% with 28 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
headroom/proxy/server.py 30.00% 27 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review after current-main reconciliation: conflicts are resolved, the manual changelog edit is removed, formatting/lint pass, and the focused memory/startup suites pass. The port also preserves default eager warmup while making peak RSS cross-platform and avoiding forced GC from the debug endpoint.

@kiyo-e

kiyo-e commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

A related unbounded-retention path was reproduced while auditing current main; I filed the implementation bug separately as #2874 so this diagnostics PR does not absorb unrelated cache-semantics work.

/debug/rss currently reports compression_cache_sessions, but the reproduced growth occurs inside each session cache: CompressionCache._stable_hashes and _first_seen grow independently of the bounded _cache. In a focused probe with max_entries=100 and 20,000 unique tool-result hashes, _cache remained at 0 entries while both side tables reached 20,000 entries.

Would it make sense for this endpoint to expose bounded aggregate diagnostics such as:

  • total and maximum _stable_hashes cardinality across retained session caches;
  • total and maximum _first_seen cardinality across retained session caches;
  • inbound_requests_by_path key count;
  • request-log entry count and, if inexpensive, approximate retained body bytes?

Aggregate/max counts should be enough; no session identifiers or content hashes need to be exposed. This would help distinguish worker-count/session-count growth from unbounded per-session bookkeeping, and Python-heap growth from native/allocator RSS high-water behavior.

@JerrettDavis JerrettDavis added status: ci failing Required or reported CI checks are failing status: needs author action Pull request body or readiness checklist still needs author updates labels Aug 12, 2026
@JerrettDavis JerrettDavis removed the status: ci failing Required or reported CI checks are failing label Aug 12, 2026
@JerrettDavis JerrettDavis added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs author action Pull request body or readiness checklist still needs author updates labels Aug 12, 2026
@github-actions github-actions Bot added status: needs author action Pull request body or readiness checklist still needs author updates and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Aug 21, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed and maintained the latest source against current upstream main. The conflict resolution keeps /debug/rss loopback-only and preserves main's added same-origin protection on /cache/clear. I also corrected the optional POSIX resource-module typing exposed by the Windows pre-commit gate without changing runtime behavior. Exact merged-tree verification: 136 memory/debug/loopback tests passed, targeted Ruff passed, focused mypy passed, and the full merge pre-commit hooks passed. The branch is now mergeable; remote CI is running. No blocking code finding.

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

Labels

status: needs author action Pull request body or readiness checklist still needs author updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants