Reduce redundant intent writes while preserving liveness - #106
ThinkOffApp wants to merge 6 commits into
Conversation
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>
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. |
![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: c0ed66d5b2
ℹ️ 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".
| // One timer owns both telemetry and liveness. Default server device TTL is 90s. | ||
| this.#pollIntervalMs = Math.min(pollIntervalMs, 60000); | ||
| this.#publisher = new StatePublisher(fields => this.#client.patchDevice(fields), { | ||
| refreshMs: this.#pollIntervalMs, |
There was a problem hiding this comment.
When the desktop payload remains unchanged, StatePublisher records its checkpoint after the request completes, so any nonzero request latency makes the next 60-second timer fire less than refreshMs after that checkpoint and get suppressed. The following write then occurs roughly 120 seconds after the previous one, exceeding the explicit 90-second TTL and making stable devices—such as Linux hosts or Macs whose idle sensor is unavailable—repeatedly appear stale. Use a refresh interval comfortably below the TTL or calculate refresh eligibility from the send's scheduled/start time.
Useful? React with 👍 / 👎.
| publish(state) { | ||
| this.#pending = structuredClone(state); | ||
| if (!this.#running) { | ||
| this.#running = this.#drain().finally(() => { this.#running = undefined; }); |
There was a problem hiding this comment.
If another publish() queues a state while a request is in flight and that request rejects, #drain() exits with #pending still populated, but this finalizer only clears #running; the queued caller receives the rejected shared promise and its state is never attempted until an unrelated future publish. This is especially visible during daemon shutdown: an offline transition queued behind a failing active write is caught and followed immediately by process.exit(), leaving the agent active until its TTL expires. Clear the running marker and start another drain whenever pending state remains.
Useful? React with 👍 / 👎.
Discover local serving models in fleet telemetry
2 participants
DesktopAdapter started a 30-second heartbeat alongside its telemetry timer, so a daemon configured for 120 seconds still made 2,880 extra requests daily. Use one bounded device publisher, retain the 90-second device TTL, and refresh unchanged agent status at 120 seconds with its 300-second TTL. Status/task changes send immediately; in-flight updates coalesce to the latest state and failed writes do not advance the checkpoint.
For the existing 120-second primary daemon configuration, scheduled requests fall from 4,320 to 2,160/day (50%), excluding state changes/retries. This is schedule arithmetic, not measured billing. Room/command polling and standalone vitals scripts are unchanged. Device telemetry is sampled at least every 60 seconds, so liveness does not depend on the removed timer.
Validation: all 50 user-intent-kit tests pass, including simulated-day counts, immediate transitions, failed-write retries, coalescing and a single bounded desktop timer. Local MacBook daemon rollout performed; fleet-wide deployment remains separate.