fix(health): gate memory severity on RSS floor#160
Conversation
Previously heapUsed/heapTotal ratio alone decided memory severity, so a small steady-state Node process whose V8 heap naturally fills its 46 MB allocation could report memory_critical_97% while the RSS sat at 120 MB and the host had plenty of free RAM. Require a second signal before degrading overall health: heap ratio above threshold AND RSS above memoryRssFloorBytes (default 512 MB). When heap is tight but RSS is below the floor, record a non-alerting memory_heap_tight_NN% note so the info is still visible in the snapshot without inflating status to degraded/critical. New tests cover the reporter's live-deployment example plus the critical / degraded / healthy branches around the RSS floor.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded a configurable RSS floor to memory health evaluation (default 512 MiB). Critical/warn memory alerts now require both heap ratio thresholds and RSS at/above the floor; high heap with low RSS emits a new heap-only Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/health-thresholds.test.ts (1)
1-79: LGTM — coverage for the new RSS-floor branching looks good.The four cases cleanly exercise: heap-tight under the floor, critical above the floor, warn above a caller-supplied floor, and both directions of caller-supplied
memoryRssFloorBytes. Thesnap()helper is a nice minimal fixture.Optional: consider adding an assertion on the exact
memory_heap_tight_...suffix (e.g._rss120mb) in the issue-#158 case to lock in the message format that operators/dashboards will key off of.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/health-thresholds.test.ts` around lines 1 - 79, Add a stricter assertion in the "stays healthy when heap fills a tiny steady-state process (issue `#158`)" test: after calling evaluateHealth(s) (using the snap helper) assert that alerts includes the exact memory_heap_tight_... token you expect (e.g., the suffix "_rss120mb") rather than only checking startsWith; update the test that references evaluateHealth and the alerts array to verify the precise alert string so downstream dashboards/operators can rely on the exact message format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/health/thresholds.ts`:
- Around line 68-76: The current else-if only emits memory_heap_tight when
memPercent > cfg.memoryCriticalPercent, leaving no alert when memPercent is in
the warn band but rssAboveFloor is false; update the condition for the
memory_heap_tight branch in src/health/thresholds.ts (the block referencing
memPercent, cfg.memoryWarnPercent, cfg.memoryCriticalPercent, rssAboveFloor,
alerts, memMb, critical and degraded) to fire when memPercent >
cfg.memoryWarnPercent && !rssAboveFloor (i.e., cover warn and critical heap
ratios when RSS is below the floor) so a memory_heap_tight... alert is pushed
without changing degraded/critical flags.
---
Nitpick comments:
In `@test/health-thresholds.test.ts`:
- Around line 1-79: Add a stricter assertion in the "stays healthy when heap
fills a tiny steady-state process (issue `#158`)" test: after calling
evaluateHealth(s) (using the snap helper) assert that alerts includes the exact
memory_heap_tight_... token you expect (e.g., the suffix "_rss120mb") rather
than only checking startsWith; update the test that references evaluateHealth
and the alerts array to verify the precise alert string so downstream
dashboards/operators can rely on the exact message format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0eddd642-dbfe-400e-a7f9-029a706e6086
📒 Files selected for processing (2)
src/health/thresholds.tstest/health-thresholds.test.ts
CodeRabbit flagged that the 'visibility without inflating severity' goal was only met at critical-level heap. When heap sat in the warn band (80-95%) with RSS below the floor, no alert was produced at all, so operators got zero signal that heap pressure was building. Widen the heap_tight branch to cover the full memoryWarnPercent+ band. New test: 85%% heap / 50 MB RSS stays healthy but records memory_heap_tight_*.
Bump version + ship CHANGELOG covering everything that merged since v0.8.13: - #118 security advisory drafts for v0.8.2 CVEs - #132 semantic eviction routing + batched retention audit - #157 iii console docs + vendored screenshots in README - #160 (#158) health gated on RSS floor - #161 (#159) standalone MCP proxies to the running server - #162 (#125) mem::forget audit coverage + policy doc - #163 (#62) @agentmemory/fs-watcher filesystem connector - #164 Next.js website (website/ root, ship to Vercel) Version bumps (8 files): - package.json / package-lock.json (top + packages['']) - plugin/.claude-plugin/plugin.json - packages/mcp/package.json (self + ~0.9.0 dep pin) - src/version.ts (union extended, assigned 0.9.0) - src/types.ts (ExportData.version union) - src/functions/export-import.ts (supportedVersions set) - test/export-import.test.ts (export assertion) Tests: 777 passing. Build clean.
Fixes #158.
Problem
evaluateHealthdecided memory severity fromheapUsed / heapTotalalone. Node's V8 heap naturally fills its current allocation, so a small steady-state process with ~45 MB heap, ~46 MB heapTotal, ~120 MB RSS reportedmemory_critical_97%while the service was healthy and the host had plenty of RAM.Fix
Require two signals to degrade memory status: high heap ratio AND RSS above
memoryRssFloorBytes(default 512 MB, configurable). When heap ratio is high but RSS is still small, record a non-alertingmemory_heap_tight_NN%_rssMMmbnote so the info is captured without inflating status.Tests
test/health-thresholds.test.ts— four cases:healthy, carriesmemory_heap_tight_*.critical.degraded.memoryRssFloorBytes.All 758 existing tests still pass.
Summary by CodeRabbit
New Features
Tests