fix(health+cli): heap_tight goes to notes, demo probe checks agentmemory ready#176
Merged
fix(health+cli): heap_tight goes to notes, demo probe checks agentmemory ready#176
Conversation
## Health: heap_tight note vs alert CHANGELOG v0.9.0 (#160) said the sub-floor heap_tight marker is a "non-alerting note attached to the snapshot — visibility without the false positive", but the implementation still pushed it into the same alerts[] array as real critical/degraded conditions. Viewer rendered it under a red "Alerts (1)" card on fresh installs (~108 MB RSS is below the 512 MB floor). - thresholds.ts now returns { status, alerts, notes }, with heap_tight routed to notes[]. - HealthSnapshot gains optional notes: string[]. - Viewer: new "Notes (N)" card in neutral color next to Alerts. Heap gauge only reddens when RSS is above the 512 MB floor — below the floor, heap percent is informational. - Test suite updated to assert heap_tight lives in notes, not alerts. ## Demo readiness probe `npx @agentmemory/agentmemory demo` 404'd on /agentmemory/session/start when iii-engine was up but the agentmemory worker hadn't registered (e.g., right after a shutdown that left iii lingering). isEngineRunning probed `/` and accepted any response, including iii's own 404. Split the probe: isEngineRunning still probes root (used by main/start flow to detect whether to spawn iii), and a new isAgentmemoryReady probes /agentmemory/livez with res.ok. runDemo now uses the stricter probe with a specific error explaining the distinction.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes introduce a "notes" field throughout the health monitoring pipeline, classifying non-critical memory pressure conditions as informational notes rather than alerts, gated by RSS thresholds. Additionally, the CLI now probes Changes
Sequence DiagramsequenceDiagram
participant Evaluator as Health Evaluator
participant Monitor as Health Monitor
participant KV as KV Store
participant Viewer as Dashboard Viewer
Note over Evaluator: evaluateHealth()
Evaluator->>Evaluator: Check memory conditions<br/>Classify heap pressure<br/>as notes (not alerts)
Evaluator-->>Monitor: return { status, alerts, notes }
Monitor->>Monitor: snapshot.notes =<br/>evaluated.notes
Monitor->>KV: Write snapshot with notes<br/>to KV.health/latest
KV-->>Monitor: Persisted
Viewer->>KV: Read latest health snapshot
KV-->>Viewer: Return snapshot with notes
Viewer->>Viewer: Render alerts card<br/>Render notes card<br/>Apply RSS-gated<br/>heap color logic
Viewer-->>Viewer: Display dashboard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/viewer/index.html (1)
1077-1082:⚠️ Potential issue | 🟡 MinorCompare the raw RSS bytes for the floor check.
Line 1081 uses the rounded display value, so an actual RSS like 511.6 MiB rounds to
512and can still turn the heap gauge red/yellow despite being below the floor. Compare the raw byte value to matchevaluateHealth.🐛 Proposed fix
- var rss = Math.round((snap.memory.rss || 0) / 1024 / 1024); + var rssBytes = snap.memory.rss || 0; + var rss = Math.round(rssBytes / 1024 / 1024); var heapPct = heapTotal > 0 ? Math.round((heapUsed / heapTotal) * 100) : 0; - var rssAboveFloor = rss >= 512; + var rssAboveFloor = rssBytes >= 512 * 1024 * 1024; var heapColor = (heapPct > 80 && rssAboveFloor) ? 'var(--red)' : (heapPct > 60 && rssAboveFloor) ? 'var(--yellow)' : 'var(--green)';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/viewer/index.html` around lines 1077 - 1082, The health-color logic is using the rounded MiB value for rssAboveFloor which can misclassify values (e.g., 511.6 MiB rounds to 512); update the check to compare the raw byte value from snap.memory.rss (or a dedicated rssBytes variable) against 512 * 1024 * 1024 so rssAboveFloor is computed from raw bytes (keep heapPct calculation based on rounded MiB as-is and ensure you still reference snap.memory.rss and heapPct in the condition that computes heapColor to match evaluateHealth).
🧹 Nitpick comments (1)
src/cli.ts (1)
84-94: LGTM — readiness probe matches existing pattern.
isAgentmemoryReady()mirrors the probe insrc/mcp/rest-proxy.tsand correctly gates the demo on the/agentmemory/livezendpoint registered byapi::liveness.One optional cleanup:
runImportJsonlat Line 831-843 inlines essentially the same probe (plus a detail string for the error message). Consider havingisAgentmemoryReady()optionally return a reason, or extract a small shared helper, so both callers stay in sync if the probe path/timeout ever changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli.ts` around lines 84 - 94, The readiness probe in isAgentmemoryReady() is duplicated in runImportJsonl; refactor so both callers share the same logic by either (A) changing isAgentmemoryReady() to return a tuple or object with {ready: boolean, reason?: string} (so runImportJsonl can include the detail string) or (B) extract a small helper like probeAgentMemoryLive(url, timeout) that returns {ok, reason} and have both isAgentmemoryReady() and runImportJsonl call that helper; update references to isAgentmemoryReady and runImportJsonl accordingly so the probe path and timeout are defined in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/viewer/index.html`:
- Around line 1077-1082: The health-color logic is using the rounded MiB value
for rssAboveFloor which can misclassify values (e.g., 511.6 MiB rounds to 512);
update the check to compare the raw byte value from snap.memory.rss (or a
dedicated rssBytes variable) against 512 * 1024 * 1024 so rssAboveFloor is
computed from raw bytes (keep heapPct calculation based on rounded MiB as-is and
ensure you still reference snap.memory.rss and heapPct in the condition that
computes heapColor to match evaluateHealth).
---
Nitpick comments:
In `@src/cli.ts`:
- Around line 84-94: The readiness probe in isAgentmemoryReady() is duplicated
in runImportJsonl; refactor so both callers share the same logic by either (A)
changing isAgentmemoryReady() to return a tuple or object with {ready: boolean,
reason?: string} (so runImportJsonl can include the detail string) or (B)
extract a small helper like probeAgentMemoryLive(url, timeout) that returns {ok,
reason} and have both isAgentmemoryReady() and runImportJsonl call that helper;
update references to isAgentmemoryReady and runImportJsonl accordingly so the
probe path and timeout are defined in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e51bc653-fce6-4115-a6b7-36b9cbc4281c
📒 Files selected for processing (6)
src/cli.tssrc/health/monitor.tssrc/health/thresholds.tssrc/types.tssrc/viewer/index.htmltest/health-thresholds.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two bugs hit in a single v0.9.1 testing session on a fresh install:
Dashboard showed red
Alerts (1): memory_heap_tight_90%_rss108mbon a just-started process. v0.9.0's fix(health): gate memory severity on RSS floor #160 intended the sub-floor heap-tight marker to be a quiet note, but the code still pushed it into the samealerts[]array as real critical conditions, so the viewer rendered it under the red Alerts card and the heap gauge was red at 23 MB total.npx @agentmemory/agentmemory demofailed withPOST /agentmemory/session/start → 404when iii-engine was up but the agentmemory worker hadn't registered.isEngineRunningprobed/and treated any response as up, including iii's own 404s for unregistered paths.Changes
Health: heap_tight is a note, not an alert
src/health/thresholds.ts→evaluateHealthnow returns{ status, alerts, notes }. heap_tight goes tonotes;alertsis reserved for critical/degraded conditions.src/health/monitor.ts→ persistsnoteson the snapshot.src/types.ts→HealthSnapshot.notes?: string[].src/viewer/index.html→ new "Notes (N)" card in neutral color below Alerts. Heap gauge only reddens when RSS is above the 512 MB floor — below the floor, heap ratio is informational.test/health-thresholds.test.ts→ updated to assert heap_tight lives in notes.CLI: demo probe checks agentmemory, not just any listener
isAgentmemoryReady()that probes/agentmemory/livezwithres.ok. Keeps the originalisEngineRunning()(root probe) for the main start flow, which needs to detect iii-without-agentmemory to decide whether to spawn iii.runDemouses the stricter probe with a specific error: "agentmemory worker not reachable on port N (livez probe failed). Something may be on the port but it isn't serving /agentmemory/*."Follow-up
Viewer WS "RECONNECTING..." badge — split into #177.