Conversation
|
@codex review this pull request at head |
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: fef019f231
ℹ️ 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".
| continue | ||
| if not state.is_usable or (now - state.timestamp) > timeout: | ||
| # The channel is stale/unusable — there is no current value. | ||
| if (now - state.timestamp) > timeout: |
There was a problem hiding this comment.
When a pressure channel or calibrated T11/T12 channel continuously delivers TIMEOUT/SENSOR_ERROR/NaN readings, each update refreshes its timestamp, so this timestamp-only condition keeps data_loss_pressure and data_loss_temperature clear indefinitely. The claimed replacement coverage does not exist: calibrated_sensor_fault skips non-usable readings, sensor_fault_intermittent covers only the uncalibrated group, and pressure has no validity alarm. Thus these configured CRITICAL alarms disappear precisely while the measurement is unusable; add validity coverage for every affected channel before excluding delivered faults from stale/data-loss evaluation.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| is_stale=unusable, | ||
| # Delivery freshness and measurement validity are independent. A | ||
| # received fault remains observable but is not missing data. | ||
| is_valid=not unusable, |
There was a problem hiding this comment.
For a fresh invalid T12 reading, this change creates a state with is_stale == False but is_usable == False; however, engine.py's cooldown_alarm.status handler at line 6269 still exposes the value whenever only is_stale is false. Consequently NaN/inf is sent as t_cold, and alarm_panel.py treats it as present and renders nan K/inf K instead of unavailable. Update that status consumer to use the new usability contract.
AGENTS.md reference: AGENTS.md:L533-L536
Useful? React with 👍 / 👎.
1 participant
Laboratory failure corrected
A temperature channel that delivered
OVERRANGE,SENSOR_ERROR, or a non-finite value every second was classified as stale and then as total data loss. The operator therefore saw "no data" while transport was healthy and the instrument was reporting a fault.This change separates two observations:
Stale alarms now use their own configured delivery deadlines. A fresh invalid reading remains unusable and fault-counted, but it does not claim that delivery stopped. Cooldown and vacuum consumers fail closed on unusable input, so this correction cannot clear a fired watchdog or erase sustained safety evidence.
Evidence
On exact master, the five new semantic controls produced four intended failures: fresh invalid readings were marked stale, and the 30-second warning deadline contaminated the independent 120-second total-loss deadline.
With this change:
git diff --checkpassed.The open pull-request queue was checked by changed file before opening this pull request. No open pull request changes the invalid-reading, stale-alarm, cooldown, or vacuum-guard files in this slice.
This pull request remains a draft until the exact-head Codex review has no findings. Full hosted CI is intentionally deferred until then.