Conversation
…uppressed The owner ruled on 2026-08-28 that every interlock, emergency_off included, is optional: on by default and turnable off by the operator. His reason is that this is a science laboratory with a new prerequisite every time, and that the operator is a trained professional who must not be blocked from operating. The standing safety argument is why suppression never means silence: when CryoDAQ refuses a legitimate action, the operators wire the hardware outside CryoDAQ, which does not prevent the action but removes this system's ability to see, control and record it. So a disabled row suppresses the ACTION and never the OBSERVATION. It keeps evaluating, and a violation it would have acted on is raised as an operator-facing warning naming the row, the value and the threshold, not a debug line. The bottom bar lists what is currently off, because a warning nobody sees is the same as deleting the check. Every transition writes a durable operator-log receipt -- identity, UTC time, the exact notice shown, request id -- and the receipt commits BEFORE the enabled state changes, so a crash cannot leave a guard off with no record of who turned it off. State survives a restart, and re-enabling re-evaluates immediately against the latest observation rather than waiting for the next one. Disable intervals are written into the experiment metadata, so a week-long dataset can say which guards were off while its points were taken.
|
@codex review Head under review: This is a direction-reversing safety-adjacent change, so the things worth your
Two limits stated openly rather than discovered: worker-thread persistence |
![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: 83fcc65200
ℹ️ 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".
| if record.condition.matches_reading(reading, descriptor_envelope) | ||
| ] | ||
| matching = [record for record in identity_matching if record.enabled and record.state == InterlockState.ARMED] | ||
| protected_matching = identity_matching |
There was a problem hiding this comment.
When a disabled row receives persistent non-usable readings while a source is active, protected_matching still includes it, so _handle_nonusable eventually invokes on_interlock_dead_channel, which latches a fault and performs the full OFF path. Thus disabling an emergency_off row suppresses threshold crossings but can still shut down the experiment when that row's sensor fails. Either make this escalation respect the row's disabled state or represent the dead-channel protection as a separate, explicitly non-optional guard so the operator-visible contract is truthful.
Useful? React with 👍 / 👎.
| self._last_disabled_interlocks = tuple(disabled_raw) | ||
| self._bottom_bar.set_disabled_interlocks(self._last_disabled_interlocks) |
There was a problem hiding this comment.
The disabled list is installed before the packet's timestamp and ordering checks. If an older or duplicate READY safety packet arrives after a newer one, the branch at lines 846–849 returns without invalidating the presentation, so an old disabled_interlocks: [] can replace a current non-empty list and be rendered as “none” rather than stale or unavailable. Defer this assignment until the safety packet has passed the same freshness/order cut as the rest of its metadata.
AGENTS.md reference: AGENTS.md:L533-L536
Useful? React with 👍 / 👎.
| ) -> None: | ||
| """Publish one loud observation while deliberately suppressing action.""" | ||
| condition = record.condition | ||
| record.last_suppressed_warning = now |
There was a problem hiding this comment.
If the suppressed-warning handler fails to establish any operator-facing surface—for example, both alarm-event dispatch and operator-log persistence fail—this timestamp is already advanced, the exception is swallowed below, and condition_active remains true. Subsequent hazardous readings therefore do not retry until the configured cooldown (or the 60-second fallback), turning a transient publication failure into a silent interval even though the disabled condition remains active. Only mark the warning delivered after a confirmed surface succeeds, or retain immediate retry authority on total failure.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| next_state[interlock_name] = {"enabled": enabled, "receipt": receipt} | ||
| next_state.update(self._retired_operator_state) | ||
| await asyncio.to_thread(self._write_operator_state, next_state) | ||
| record.enabled = enabled |
There was a problem hiding this comment.
If an interlock trips, is disabled while still TRIPPED, then is re-enabled after its latest reading becomes safe, this assignment reports the row as enabled but leaves its latch state TRIPPED. Future hazardous readings are excluded from matching because they are not ARMED, and they also receive no suppressed warning because the row is enabled, so the interlock becomes completely silent until a separate acknowledgement happens. Re-enable should either require/perform the explicit re-arm transition or refuse to report success while the row remains tripped.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| not previous | ||
| and record.latest_reading is not None | ||
| and record.condition.is_triggered(record.latest_reading.value) | ||
| ): | ||
| await self._trip(record, record.latest_reading) |
There was a problem hiding this comment.
Production interlocks bind multiple channels, but latest_reading stores only the last reading seen for the entire row. If T1 remains above threshold and a later T2–T8 reading is safe, re-enabling checks only that safe final reading and does not fire immediately, even though the interlock condition still holds on T1; the action is delayed until T1's next poll. Retain and evaluate the latest observation for every bound channel rather than a single row-wide value.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| for name, item in self._interlocks.items() | ||
| if name != interlock_name and item.last_transition_receipt is not None | ||
| } | ||
| next_state[interlock_name] = {"enabled": enabled, "receipt": receipt} |
There was a problem hiding this comment.
Each toggle replaces the state file's only receipt for the row. If disabling succeeds but record_interlock_operator_state fails, and the operator later re-enables before restart, this write overwrites the disable receipt with the re-enable receipt; sync_interlock_operator_provenance then sees an enabled row with no open interval and cannot reconstruct the missed disable interval. The operator log still contains the events, but the experiment metadata permanently omits the period during which its guard was disabled, so retain transition history or reconcile from the durable log before discarding the prior receipt.
Useful? React with 👍 / 👎.
| async def publish_interlock_operator_state(self) -> None: | ||
| """Refresh the existing safety-state carrier after an operator toggle.""" | ||
| await self._publish_state("interlock_operator_state_changed") |
There was a problem hiding this comment.
This is the only update sent to MainWindowV2 after a toggle, but _publish_state catches broker failures and returns success, and the GUI never polls the new interlock_status endpoint. If the publication fails—or the GUI reconnects after the one-shot packet—the durable state can be disabled while the bottom bar indefinitely retains the prior empty list as current and appears to say all guards are armed. Require a publication receipt/retry or have the GUI periodically reconcile from the read endpoint instead of treating this best-effort packet as convergent state.
AGENTS.md reference: AGENTS.md:L490-L491
Useful? React with 👍 / 👎.
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: This round answers your findings, plus one the coordinator measured that was not Verified by the coordinator: the regression passes 13 nodes and the working tree If a finding is unaddressed rather than fixed, name it rather than treating |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08d4e48067
ℹ️ 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".
| receipt_result = await _submit_operator_log_entry(log_cmd, context) | ||
| entry = receipt_result.get("entry") if type(receipt_result) is dict else None | ||
| commit_receipt = receipt_result.get("commit_receipt") if type(receipt_result) is dict else None | ||
| if receipt_result.get("committed") is not True or type(entry) is not dict or type(commit_receipt) is not dict: |
There was a problem hiding this comment.
When an exact request is replayed after a later opposite toggle, _submit_operator_log_entry returns the original committed receipt, but this path unconditionally calls set_enabled again. For example, disable request A, enable request B, then a delayed replay of A leaves the interlock disabled even though B is the newest operator decision. Persist or recognize toggle settlement by request ID and return the original result without reapplying stale state.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| provenance_recorded = await asyncio.to_thread( | ||
| context.experiment_manager.record_interlock_operator_state, | ||
| interlock_name=name, |
There was a problem hiding this comment.
If an experiment stop/start runs after the operator-log append releases its experiment CAS but before this call, record_interlock_operator_state writes through whichever experiment is currently active rather than the experiment_id captured above. The toggle can therefore be logged against experiment A while its disable interval is written to experiment B; the pending-receipt recovery path also lacks an experiment identity with which to repair this. Serialize the whole toggle with experiment lifecycle changes or pass and enforce the captured experiment ID.
AGENTS.md reference: AGENTS.md:L490-L491
Useful? React with 👍 / 👎.
| target_receipt=receipt, | ||
| target_pending=pending, | ||
| ) | ||
| await asyncio.to_thread(self._write_operator_state, next_state) |
There was a problem hiding this comment.
If the command task is cancelled while this to_thread write is in progress, the worker thread continues and can durably write the new enabled value, but the coroutine exits before updating record.enabled, its receipt journal, or the publication surface. This leaves disk and the running interlock authority disagreeing until restart; a cancelled enable can therefore leave the live guard disabled even though the durable file says it is enabled. Shield and settle the write, then complete the in-memory transition before propagating cancellation.
AGENTS.md reference: AGENTS.md:L470-L471
Useful? React with 👍 / 👎.
| or type(state.get("enabled")) is not bool | ||
| or type(state.get("receipt")) is not dict | ||
| or ( | ||
| schema_version == 2 | ||
| and ( | ||
| type(state.get("pending_receipts")) is not list | ||
| or len(state["pending_receipts"]) > _MAX_PENDING_OPERATOR_TRANSITIONS | ||
| or any(type(receipt) is not dict for receipt in state["pending_receipts"]) |
There was a problem hiding this comment.
When the state file is corrupted or hand-edited but retains the expected outer keys, this validation accepts any dictionaries as receipts; for example, {"enabled": false, "receipt": {}, "pending_receipts": []} is loaded successfully and disables the interlock without an operator, timestamp, request ID, notice, or commit proof. Validate the complete receipt schema and timestamp before applying a persisted disabled value so malformed persistence fails closed instead of silently removing guard authority.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| if action == "interlock_set_enabled": | ||
| return await _set_interlock_operator_state(cmd, context=context) |
There was a problem hiding this comment.
At the reviewed head, the ordinary ZMQ mutation endpoint accepts interlock_set_enabled without checking verified-OFF state or routing the disable through a dedicated safety adapter, so any admitted mutation client can suppress an emergency_off guard while a source is active. A repository-wide search found no separately approved hazard analysis or the required host-death and physical-bench evidence for this new control path. Keep disabling unavailable until that approval stack exists, or enforce the approved adapter and operating-state gate here.
AGENTS.md reference: AGENTS.md:L33-L35
Useful? React with 👍 / 👎.
| if state["enabled"] is False and not record.condition.operator_disableable: | ||
| raise InterlockConfigError(f"persisted disabled interlock {name!r} is no longer operator-disableable") | ||
| record.enabled = state["enabled"] |
There was a problem hiding this comment.
When interlocks.yaml changes between restarts, this loader applies a prior disabled decision solely by row name. If that name now has a different threshold, action, or physical channel binding, the old receipt—whose notice described the previous condition—silently disables the newly reviewed guard without a fresh operator decision. Persist and verify a fingerprint of the exact interlock policy before restoring enabled=False, and refuse stale disabled receipts on mismatch.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: This round answers your findings AND fixes the defect behind ten red checks, The coordinator verified the round by reverting the production change and One node fails here that also fails identically on master Docs gate green at this head; derived pair regenerated as the last commit. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a412247640
ℹ️ 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".
| if enabled: | ||
| for interval in reversed(intervals): | ||
| if interval.get("interlock_name") == interlock_name and interval.get("reenabled_at") is None: |
There was a problem hiding this comment.
When an interlock is disabled during experiment A, A is finalized, experiment B starts, and the interlock is re-enabled, this branch searches only B's metadata for an open interval. Consequently A's interval remains permanently open and B contains no record that it began while the guard was disabled. Fresh evidence at this head is this experiment-spanning lifecycle sequence, rather than the previously reported concurrent experiment-binding race; close or split the interval explicitly when experiment ownership changes.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| disabled_interlocks = self._disabled_interlocks() | ||
| if disabled_interlocks: | ||
| return False, "Software interlock(s) disabled: " + ", ".join(disabled_interlocks) |
There was a problem hiding this comment.
If an interlock is disabled while SafetyManager is already READY, this new precondition rejects the eventual RUN request but neither demotes the FSM nor adds a blocker to _operator_safety_snapshot. The typed snapshot therefore continues to publish READY, and MainWindowV2 continues treating its source-control gate as ready even though the backend will refuse the action; project the provider state into the authoritative snapshot so operator readiness agrees with the enforced precondition.
AGENTS.md reference: AGENTS.md:L533-L536
Useful? React with 👍 / 👎.
| or type(operator) is not str | ||
| or not operator.strip() |
There was a problem hiding this comment.
For an operator value between 513 and 4096 UTF-8 bytes, this admission accepts the command and the operator-log layer durably commits it, but InterlockEngine.set_enabled() subsequently rejects the receipt because _validated_operator_transition_receipt() caps the operator at 512 bytes. The state change therefore never occurs and no pending transition is persisted for restart reconciliation, leaving a committed enable/disable decision permanently detached from interlock state; apply the receipt's 512-byte bound before _submit_operator_log_entry().
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| if not triggered: | ||
| record.condition_active = False |
There was a problem hiding this comment.
For a multi-channel row such as overheat_cryostat, if T1 remains above threshold while a later T2–T8 reading is safe, this row-wide assignment clears condition_active even though the interlock condition is still active on T1. The next T1 sample then satisfies not record.condition_active and emits another alarm/operator-log warning regardless of the configured cooldown, so one persistently hot channel can generate a loud warning on every poll; derive the aggregate active state from all latest bound-channel readings or track it per channel.
Useful? React with 👍 / 👎.
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: This round answers your four findings. Coordinator's control: 5 nodes red Three of the five reddening nodes are worth your eye because they are about
Docs gate green at this head; derived pair regenerated as the last commit. If a finding is unaddressed rather than fixed, please name it rather than |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c63cac159f
ℹ️ 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".
| if (transitions or state.get("enabled") is False) and not any( | ||
| _interlock_receipt_request_id(receipt) == _interlock_receipt_request_id(last_receipt) | ||
| for receipt in transitions | ||
| ): | ||
| transitions.append(dict(last_receipt)) |
There was a problem hiding this comment.
When experiment B starts while the row remains disabled, this branch re-adds the last receipt but later resolves it to the receipt's original experiment A and skips it as already recorded; because the new split logic runs only on a later re-enable, B receives no interval if the guard stays disabled through finalization, and if re-enabling occurs after experiment C begins, B is omitted entirely. Fresh evidence at this head is that the new test always re-enables during B, so it does not cover a disabled guard spanning multiple boundaries or never being re-enabled; seed and split the interval as part of each lifecycle boundary instead.
AGENTS.md reference: AGENTS.md:L468-L469
Useful? React with 👍 / 👎.
| f"interlocks.yaml at {config_path}: 'interlocks' must be a list, got {type(entries).__name__}" | ||
| ) | ||
|
|
||
| operator_disableable_default = raw.get("operator_disableable", True) |
There was a problem hiding this comment.
When an existing deployment selects a pre-upgrade interlocks.local.yaml that lacks the new root key, this fallback silently makes every configured row operator-disableable. _engine_config_path() prefers that complete machine-local override, and the new mutation endpoint then accepts toggles for those rows, so merely upgrading grants authority to suppress safety guards without an explicit reviewed policy change; require the key or default omission to False.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| if len(record.transition_receipts) >= _MAX_OPERATOR_TRANSITION_RECEIPTS: | ||
| raise RuntimeError("interlock operator idempotency journal is full") |
There was a problem hiding this comment.
The 1,024-entry bounds apply independently to every row and to both the pending and retained arrays, while _write_operator_state() imposes no aggregate byte limit and _load_operator_state() rejects any resulting file over 4 MiB. With the three tracked rows, legally bounded receipts using the accepted 512-byte operator identity already produce a retained state file above 4 MiB (and pending receipts duplicate that history), so a file written successfully by this code can make the next engine restart fail before provenance recovery runs; prune or impose a serialized aggregate budget that cannot exceed the loader's limit.
Useful? React with 👍 / 👎.
|
Correction to my earlier comments on this pull request. I said that The failure was in MY environment. That test spawns a subprocess to prove there With The operational advice was still right — that node is not this branch's problem |
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: Reversion control, run this turn on Ubuntu 22.04. Production reverted, tests kept: Documentation gate: Context you should hold while reviewing. The owner's standing ruling is that nothing may So please check specifically that The third node is a memory-shape guard — serialized operator journals must not outgrow what the Disclosure: this change was produced and verified with AI assistance. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35441a643f
ℹ️ 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".
| blockers.append( | ||
| SafetyBlocker( | ||
| "software_interlock_disabled", | ||
| OperatorPresentationState.WARNING, | ||
| f"Software interlocks are disabled: {disabled_names}", | ||
| "Re-enable every named software interlock before requesting RUN", |
There was a problem hiding this comment.
When any interlock is disabled, this SafetyBlocker makes the authoritative snapshot report BLOCKED; MainWindowV2._apply_operator_snapshot() then clears safety readiness and KeithleyPanel._update_control_enablement() disables the Start button. Fresh evidence at this head is the owner's explicit requirement in this review that a disabled interlock must warn without refusing RUN, so project this as warning-only rather than a readiness blocker.
Useful? React with 👍 / 👎.
| serialized = json.dumps(payload, ensure_ascii=False, indent=2, allow_nan=False) | ||
| if len(serialized.encode("utf-8")) > _MAX_OPERATOR_STATE_BYTES: | ||
| raise RuntimeError("interlock operator state exceeds its size bound") |
There was a problem hiding this comment.
Once the append-only transition_receipts arrays approach this aggregate limit, every subsequent toggle is rejected instead of old settled idempotency entries being retired. This includes an attempt to re-enable a currently disabled guard; because the operator-log intent is committed before this write, the operator receives only a reconciliation failure while the guard remains disabled, and retries cannot recover because no path reduces the retained journal. Bound the retained history without discarding pending provenance, and warn before capacity exhaustion.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| for experiment_id in sorted(changed_experiments): | ||
| atomic_write_text( | ||
| self._metadata_path(experiment_id), | ||
| json.dumps(payloads[experiment_id], ensure_ascii=False, indent=2), |
There was a problem hiding this comment.
When a disabled guard crosses an experiment boundary, the new seeding path queues both the prior experiment's closure and the current experiment's continuation, but this loop persists those files independently. If the second write fails, retry cannot converge: a persisted forward pointer with no target interval raises active interlock disable interval is unavailable, while a persisted target with an unclosed source hits the early matching-target return and never repairs the source. Use a recoverable transaction/journal or make reconciliation repair either one-sided state.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| elif not any( | ||
| interval.get("interlock_name") == interlock_name and _interlock_interval_is_open(interval) | ||
| for interval in intervals | ||
| ): |
There was a problem hiding this comment.
If an operator submits a new disable request while the row is already disabled, this condition leaves the existing open interval bound to the first request even though set_enabled() replaces the engine's disable_receipt with the second request. At the next experiment boundary, _seed_disabled_interlock_boundary() searches the prior metadata using that second request ID, raises prior interlock disable interval is unavailable, and the new experiment loses its disabled-guard provenance. Treat same-state disables as no-ops or update the open interval and its receipt identity consistently.
AGENTS.md reference: AGENTS.md:L468-L469
Useful? React with 👍 / 👎.
| if action == "interlock_set_enabled": | ||
| return await _set_interlock_operator_state(cmd, context=context) |
There was a problem hiding this comment.
At the inspected head, a repository-wide search for interlock_set_enabled, interlock_status, and operator_disableable under src/cryodaq/gui finds no caller or control; the only GUI addition is the explicitly passive bottom-status display. Consequently an operator using the shipped application can see that a guard is disabled but cannot disable or re-enable it, leaving this advertised operator option available only to bespoke ZMQ clients and encouraging control outside the supported UI. Add a reviewed, accessible GUI route that invokes this mutation and presents its receipt/outcome.
Useful? React with 👍 / 👎.
|
Hold the review of Measured this turn by running the node on Windows directly, which is where it fails:
Why this is a real defect and not a layout detail. 1280 is the width of the screen the The threshold will not be raised. Lowering a threshold to make a test pass is prohibited here, A lane is working it now, instructed to reuse the existing mechanism rather than truncate the The rest of the round stands: the three interlock-optionality nodes pass, and the reversion |
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: The status bar now fits, and the threshold was not touched. Measured this turn on Ubuntu The lane made the node stricter rather than passing it: it now feeds a 1000-character A regression this round caused, and closed. The first attempt fixed the width and broke Suites, measured this turn: What I would like you to attack. The visible label is now elided while the full text lives
The owner's rule here is that an interlock may be switched off and the source may be turned Disclosure: this change was produced and verified with AI assistance. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30e8dd79e7
ℹ️ 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".
| visible_name = _bounded_visible(canonical[0], _MAX_VISIBLE_INTERLOCK_CHARS) | ||
| visible = f"⚠ Блок.: {visible_name}{suffix}" | ||
| else: | ||
| visible = f"⚠ Блок.: {len(canonical)}{suffix}" |
There was a problem hiding this comment.
When two or more interlocks are disabled, this branch renders only ⚠ Блок.: N; a repository-wide GUI search shows that the identities then exist only in this label's tooltip and accessible description. A sighted operator who does not hover can therefore see that protection is reduced but cannot determine which guards are unavailable or act on that information. Keep the identities—or an immediately visible, non-hover detail surface—in the persistent presentation while still bounding the bar width.
AGENTS.md reference: AGENTS.md:L529-L532
Useful? React with 👍 / 👎.
| result = self._suppressed_handler(condition, reading, message) | ||
| if asyncio.iscoroutine(result): | ||
| await result |
There was a problem hiding this comment.
When a disabled row crosses its threshold, the sole _check_loop consumer waits here for the complete production warning handler before reading another broker item. That handler awaits SQLiteWriter.append_operator_log(), which queues work on the shared write executor without a timeout, so a slow or stuck persistence write for this observational warning also delays processing a subsequent hazardous reading for an enabled interlock. Preserve warning retries and settlement under a separately owned task rather than blocking the serial protection loop.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| self._interlocks[condition.name] = _InterlockRecord( | ||
| condition=condition, | ||
| enabled=condition.enabled_by_default, |
There was a problem hiding this comment.
When a valid configuration sets enabled_by_default: false, this assignment creates a disabled record without a last_transition_receipt or disable_receipt. ExperimentManager.sync_interlock_operator_provenance() consequently skips the row, and a later re-enable has neither a prior receipt nor an open interval to close, so every experiment spanning this supported default-disabled state permanently omits the fact that its guard was unavailable. Either reject disabled initialization or seed durable, policy-bound provenance for it.
Useful? React with 👍 / 👎.
|
One red on Windows at Over the bound by exactly 13 bytes, which is the number of newlines in the file. Why. The bound is checked on the JSON string, and then a different thing is written to disk: # src/cryodaq/core/interlock.py::_write_operator_state
serialized = json.dumps(payload, ensure_ascii=False, indent=2, allow_nan=False)
if len(serialized.encode("utf-8")) > _MAX_OPERATOR_STATE_BYTES: # LF newlines
raise RuntimeError("interlock operator state exceeds its size bound")
atomic_write_text(path, serialized)
# src/cryodaq/core/atomic_write.py::atomic_write_text
with os.fdopen(fd, "w", encoding=encoding) as f: # text mode, no newline=""Text mode turns every This is wider than this pull request. The fix is one argument — I have not changed anything on this branch for it yet. The rest of the round stands: the status |
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
Landed by the batch lander. The lane's own report and the coordinator's verification are recorded on the pull request.
|
@codex review Head under review: The Windows red is closed, and it was a real cross-platform defect rather than a platform Verified on Windows, because that is the only platform where this change alters behaviour: The new guard writes a known string through the helper and asserts the file's bytes equal the The 32 KiB bound was not raised. The bound is right; the accounting was wrong. The blast radius, checked rather than assumed.
What I would like you to attack: this changes bytes on disk for nine callers on Windows. If any The rest of the round stands: the status bar fits 1280 with the full interlock text in the Disclosure: this change was produced and verified with AI assistance. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b5020856
ℹ️ 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".
| elif identity_matching: | ||
| self._nonusable_windows.pop(reading.channel, None) | ||
| if not ( | ||
| math.isinf(reading.value) | ||
| and any(record.condition.is_triggered(reading.value) for record in identity_matching) | ||
| ): | ||
| return |
There was a problem hiding this comment.
When every interlock matching a channel is disabled and the sensor reports NaN or a finite error-status value, this branch clears the non-usable window and returns instead of calling _handle_nonusable. The disabled row therefore loses both the immediate diagnostic alarm and persistent dead-channel observation; the general sensor-diagnostics alarm is delayed and configurable, so disabling an action can make this safety-relevant sensor failure substantially quieter. Keep the observational warning/debounce path active while suppressing only the eventual control action or fault latch.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
|
One Windows red remains at Measured on Windows this turn, both ways:
So the label this branch added is fine on its own. The aggregate is over, and only on Windows. That is a real defect rather than a platform quirk: a width budget that is correct only by luck The laboratory machine is Ubuntu 22.04, so the instrument itself is not affected today; Windows The rest of this head stands and is verified: |
1 participant
Every software interlock becomes operator-optional: on by default, turnable off
by the operator, and warning instead of going quiet when it is off.
Why
The owner ruled this on 2026-08-28. His reason is that this is a science
laboratory with a new prerequisite every time, and that the operator is a
trained professional who must not be blocked from operating.
The standing rule behind it is a safety argument, not a convenience one: when
CryoDAQ refuses a legitimate action, the operators wire the hardware outside
CryoDAQ. The refusal does not prevent the action — it only removes this
system's ability to see, control and record it.
What it does
A disabled row suppresses the action and never the observation:
operator-facing warning naming the row, the value and the threshold;
the same as deleting the check;
the exact notice shown, request id — and the receipt commits before the
enabled state changes, so a crash cannot leave a guard off with no record
of who turned it off;
against the latest observation rather than waiting for the next one;
dataset can say which guards were off while its points were taken.
emergency_offis included, by explicit owner decision.set_disabled_interlocksdistinguishes "no interlock is disabled" from "nobodytold me which interlocks are disabled". The second renders as
—, never as anempty list, because only the first is safe to read as all-guards-armed.
Verification
The control is reversion, not a passing test. All eight production paths were
reverted to their base with the new regression kept: 7 tests failed.
Restoring production and re-running: 7 passed. Per-property results are in
the lane's report; the properties covered are optionality of every row
including
emergency_off, evaluation-while-disabled, warning content, disableand re-enable receipts, immediate firing on re-enable, restart persistence, the
bottom-bar listing through real telemetry routing, and the run-provenance
interval.
The regression adds 46 assertions and deletes none. No existing test was
weakened, skipped or restated.
Docs gate green at this head; the derived pair was regenerated to a fixed point
as the last commit.
Not determined
sandbox stalls filesystem work inside
asyncio.to_thread, so the behaviouraltests call the same synchronous persistence function inline. Production keeps
the off-event-loop boundary.
filesystem tests and one transport test that cannot open an
AF_INETsocketunder the sandbox. The focused partitions ran: 92, 35 and 1 passed, plus ruff
check and format.
touched.
Disclosure
Written by an AI lane (
gpt-5.6-sol) under an agent brief, verified and landedby the coordinating agent. The reversion control above was run independently of
the lane's own report.