feat(uik): publish installed and free memory - #65
Merged
Conversation
Petrus asked for load and memory from every machine. The MacBook already reported memory; the Mini published load only, because the shared host-vitals path had no memory fields at all. Units follow what the fleet already publishes — GB as 1e9 bytes, so a 128 GiB MacBook reads 137.4 — rather than introducing a second meaning for a field name that is already in use. No used-percentage, deliberately. On macOS `freemem()` counts only genuinely free pages and excludes cache the OS hands back on demand, so a percentage derived from it reports 95% used on a machine under no memory pressure. Free and total are facts; the percentage would be an alarm the numbers do not support. Tests cover both, verified by planting a derived percentage and a dropped zero and confirming they fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b291d30026
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+84
to
+85
| const total = sources.totalMemBytes?.(); | ||
| const free = sources.freeMemBytes?.(); |
There was a problem hiding this comment.
When either injected memory accessor throws, the outer catch discards both metrics: a failing totalMemBytes prevents freeMemBytes from running, while a failing freeMemBytes abandons the already-read total before out is returned. This violates the field-by-field best-effort contract for hosts where only one memory source is readable; catch each accessor independently so the surviving measurement is still published.
Useful? React with 👍 / 👎.
Petrus: "can you add this type of info for the macs as well!" — the Pi's card
carries what the box is, how much memory it has and how it is attached, while
a Mac appeared in the fleet as a name and nothing else.
Adds `hw` ("Apple M4 (Mac16,10)", or the Pi's own device-tree name), plus
`network` ("wifi"/"ethernet") and `lan_ip`, and puts the whole vitals set on
the desktop device card rather than only the agent record.
No SSID. On current macOS that needs location permission, and a field that
works on one machine and silently fails on another is worse than no field —
the medium answers the question that matters anyway (the Pi is deliberately
never on the house wifi).
Commands run through argv, never a shell string, with a 2s timeout, and any
failure leaves the field absent rather than guessed.
The hardware read is cached per sources object, not module-globally. The
first version cached in a module variable, which the tests caught: once the
real machine had populated it, injected sources were ignored and every later
caller got the first answer regardless of what it asked for.
35 tests pass, including a wired Mac reporting ethernet, a Mac on the wifi
device reporting wifi, and both fields omitted when the commands fail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review from @grok: the Mac card and the Pi card were drifting into two vocabularies for the same ideas. `kind` is the cheap half of that — the Pi already publishes it, so Macs now use the same field instead of a synonym. The rest of the gap is deliberate, not an oversight: - `model` on the Pi is the LLM it serves. The Mini serves none right now, and a hardware string does not belong in a field the fleet reads as "the model loaded here". `hw` is a separate fact and keeps its own name. - `reach_url` on the Pi points at its dashboard. The Mini has no equivalent user-facing service, so publishing one would be inventing a destination for the sake of symmetry. `lan_ip` says where the box is without implying something is served there. - memory already converged on the numeric fields in both publishers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Petrus: "Saanko load ja memory lukemat kaikista koneista uik?"
The MacBook already published memory; the Mini published load only, because the shared host-vitals path had no memory fields. This adds
mem_total_gbandmem_free_gb, so every publisher on that path reports memory.The Mini now produces:
{"machine":"mac-mini","load_1m":2.79,"cpu_count":10,"load_pct":28,"mem_total_gb":25.8,"mem_free_gb":1.1}Units match the fleet, not my preference. The MacBook reports 137.4 for a 128 GiB machine, so GB here is 1e9 bytes. Publishing GiB would have given the same field name two meanings across machines — the
load1/load_1msplit again.No used-percentage, on purpose. On macOS
freemem()counts only genuinely free pages and excludes cache the OS returns on demand, so a percentage derived from it reads as 95% used on a machine under no pressure. Free and total are facts; the percentage would be an alarm the numbers don't support. If we want a real pressure figure on macOS it needs a different source, as a separate change.31 tests pass, verified by planting a derived percentage and a dropped zero and confirming both fail.
Still outstanding on the fleet, not in this PR: the Pi publishes
memoryas free text ("16GB total, model held in me…") rather than numeric fields, so it can't be rendered as a measurement. That's the Pi publisher, @grok's lane.Co-Authored-By: Claude Opus 5 noreply@anthropic.com