telemetry: publish the machine's own critical temperature - #105
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ThinkOffApp
pushed a commit
that referenced
this pull request
Sep 17, 2026
codexmb, reviewing #105: `c > hottest` dropped a valid critical point exactly when the temperature reached or passed it. A box reading 105 C against a 100 C critical published NO limit, and a reader falling back to a default would have painted a thermally emergency machine calmer than it actually was. My reasoning was that a critical point below the current reading must be a broken table. It is the opposite. The Linux thermal API defines crossing the critical trip as a protection event the kernel acts on, so that reading is the real state of a box in trouble - the one case the whole per-machine colour scheme exists to show. Plausibility is now judged on the value alone, TEMP_MIN_C..TEMP_MAX_C, with no reference to the reading. Unparseable, empty and hotter-than-any-real-die are still dropped; at-the-trip and over-the-trip are kept. New test covers 100/100 and 105/100. Verified against the known failure: restoring `c > hottest` fails that test and only that test. 33 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![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: d3ce64e954
ℹ️ 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".
| const c = Number(raw) / 1000; | ||
| // A critical point below the current reading, or outside plausible die | ||
| // temperatures, is a broken table rather than an emergency. | ||
| if (raw !== '' && Number.isFinite(c) && c > hottest && c <= TEMP_MAX_C) { |
There was a problem hiding this comment.
When the sensor reaches exactly its critical trip value, both sysfs files can contain the same integer millidegree value, so the strict c > hottest check omits temp_limit_c. Equality does not indicate a broken trip table and is precisely the critical state the dashboard needs to display; accept c === hottest and reject only limits below the reading.
Useful? React with 👍 / 👎.
petrus wants temperature coloured green / amber / fuchsia by how close a box
is to ITS limit, not to a guessed scale. He is right that the scale has to be
per-machine: 49 C is idle on a box that trips at 104 and alarming on one that
trips at 60, and a 0-100 C bar would render a healthy 72 C box as "72% of
something" that does not exist.
Linux declares the number itself. Every ACPI zone on the DGX Sparks carries a
critical trip point of 104.8 C, so a reader can colour 49.7 C as 47% of limit
instead of inventing a ceiling.
- New source readThermalCriticalMilli(zone) reads the zone's trip table and
returns the 'critical' trip only. A zone also advertises passive and hot
trips; those are throttle hints, not the number a reading is judged against.
- readLinuxTempC now returns {tempC, limitC} and the limit comes from THE SAME
zone as the reading. Pairing the hottest zone's temperature with a different
zone's critical point would describe neither, and a test fails if it does.
- A critical point below the current reading, or above the plausible-die
ceiling, is treated as a broken table and dropped. Publishing it would paint
a cool box fuchsia.
- Unreadable trip table costs the limit, never the temperature.
A host that declares no critical point simply omits temp_limit_c, and the
reader falls back to its own default: the same omit-never-zero contract the
rest of this module keeps. Macs omit it, as they already omit temp_c, because
that sensor is behind root.
32 tests pass. Verified against a known failure: pointing the limit read at
the wrong zone makes the same-zone test fail, and only that one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codexmb, reviewing #105: `c > hottest` dropped a valid critical point exactly when the temperature reached or passed it. A box reading 105 C against a 100 C critical published NO limit, and a reader falling back to a default would have painted a thermally emergency machine calmer than it actually was. My reasoning was that a critical point below the current reading must be a broken table. It is the opposite. The Linux thermal API defines crossing the critical trip as a protection event the kernel acts on, so that reading is the real state of a box in trouble - the one case the whole per-machine colour scheme exists to show. Plausibility is now judged on the value alone, TEMP_MIN_C..TEMP_MAX_C, with no reference to the reading. Unparseable, empty and hotter-than-any-real-die are still dropped; at-the-trip and over-the-trip are kept. New test covers 100/100 and 105/100. Verified against the known failure: restoring `c > hottest` fails that test and only that test. 33 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThinkOffApp
force-pushed
the
feat/host-temp-limit
branch
from
September 17, 2026 10:51
98634c3 to
52cd078
Compare
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
2 participants
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, 17 Sep: colour temperature green / amber / fuchsia by how close a box is to its limit, finding machine-specific limits and falling back to averages.
The per-machine part matters. 49 C is idle on a box that trips at 104 and alarming on one that trips at 60, and a 0–100 °C bar renders a perfectly healthy 72 °C machine as "72% of something" that does not exist.
Linux already declares the number. Every ACPI zone on both DGX Sparks carries a critical trip point of 104.8 °C, so a dashboard can say 47% of limit instead of inventing a ceiling.
Changes
readThermalCriticalMilli(zone)reads the zone's trip table and returns thecriticaltrip only. Zones also advertisepassiveandhot; those are throttle hints, not the number a reading should be judged against.readLinuxTempCnow returns{tempC, limitC}, and the limit comes from the same zone as the reading. Pairing the hottest zone's temperature with another zone's critical point produces a headroom figure describing neither.A host that declares no critical point simply omits
temp_limit_c, keeping this module's omit-never-zero contract, and the reader falls back to its own default. Macs omit it for the same reason they already omittemp_c: the sensor is behind root.Live result
Deployed to both Sparks; read back from
GET /intent/petrus:(
bosgame-m5still runs an older copy of the package; it picks the field up on its next update.)Tests
32 pass. Four new cases: same-zone pairing, no declared limit, unreadable trip table, and nonsense limits (below the reading, above the die ceiling, unparseable, empty).
Verified against a known failure rather than assumed: pointing the limit read at the wrong zone makes exactly the same-zone test fail, and restoring it makes it pass.
🤖 Generated with Claude Code