Skip to content

Navigation Menu

Sign in
Sign up

feat: give the operator a control for each interlock, in the interface - #119

Draft
test1card wants to merge 4 commits into
feat/interlock-operator-optionality from
feat/interlock-control-in-the-gui
Draft

feat: give the operator a control for each interlock, in the interface #119
test1card wants to merge 4 commits into
feat/interlock-operator-optionality from
feat/interlock-control-in-the-gui

Conversation

@test1card

@test1card test1card commented Aug 28, 2026

Copy link
Copy Markdown
Owner

The owner asked for this today, in these words:

"BTW, those overheat cryostat and overheat compressor should be configurable in the gui,
because right now no sensors hooked to the compressor"

an hour after describing why he had left the program entirely:

"I want to turn on the source, but the button is disabled, because 2 of my sensors have
troubles with soldering, and they show their ceiling that registers as 380K. and now I have
to use the source by hand, bypassing cryodaq"

The gap this closes. #113 makes every interlock operator-optional in the engine — receipts,
survival across restart, the disabled interval carried as run provenance — and adds a status bar
that displays which are disabled. Measured on origin/master,
git grep -l interlock -- src/cryodaq/gui returns nothing: there is no interlock surface in the
interface at all. So the ruling was implemented where he could not reach it. He could see that
an interlock was off; he had no control to turn one off.

This adds that control: src/cryodaq/gui/shell/overlays/interlock_panel.py, reachable from
navigation and the tool rail, driving the engine-side path #113 already provides rather than a
second mechanism.

Scope, stated so it can be corrected. "Configurable" is implemented as ENABLE/DISABLE per
interlock. Editing thresholds and channel patterns from the interface is deliberately NOT
included — he did not clearly ask for it and it is a much larger surface. One word from him adds
it. The panel shows, for each interlock, which channels it watches and whether it cuts the
source or triggers an emergency shutdown, because those differ sharply in consequence.

Verification, run by the coordinator on Ubuntu 22.04, not taken from the lane's report.
Production reverted with the new tests kept:

production reverted -> 7 failed, 51 passed
 test_toggle_does_not_disable_any_control
 test_disabled_state_survives_restart_and_is_visible_on_both_surfaces
 test_control_is_discoverable_in_navigation_and_shows_engine_owned_details
 test_every_current_route_key_and_roadmap_destination_is_preserved
 test_labels_and_order_are_deterministic_and_operator_facing
 (and two more)
production restored -> 58 passed
module gate: every test file naming interlock, navigation or tool_rail -- 54 files
 -> 1685 passed, 5 skipped, 0 failed
documentation gate -> 68 passed
ruff check / ruff format --check -> clean

The owner's standing ruling is encoded as an executable guard, not a promise.
test_toggle_does_not_disable_any_control exists because his rule is that nothing may grey out
a control — a source may be turned off, but he must never lose the ability to act, since when he
does he drives the hardware by hand outside this program and nothing is recorded at all. A
direct search of the new panel for setEnabled(False) / setDisabled(True) returns nothing.

docs/CLAIM_CORRECTIONS.md moves its moving candidate count from 717 to 719, read out of the
freshness guard rather than counted, naming the two paths that entered.

Related but deliberately separate: a sibling branch is making a railed sensor read as a
faulty sensor rather than as 380 K, so that the interlock is never fed a broken reading in the
first place. This pull request is about giving him the switch; that one is about not needing it.

Opened as a draft. Disclosure: this change was produced and verified with AI assistance.

soak measurement added 4 commits August 28, 2026 14:55
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.

Copy link
Copy Markdown
Owner Author

@codex review

Head under review: a71c92e873171512968f53668351c62e4247e91c (verified equal to
origin/feat/interlock-control-in-the-gui at the moment of writing). Based on
feat/interlock-operator-optionality (#113), whose engine-side machinery this drives.

The verification is in the pull request body; the short form is that reverting production with
the new tests kept gives 7 failed / 51 passed, restoring gives 58 passed, and the 54-file module
gate over everything naming interlock, navigation or tool_rail is 1685 passed with no failures.

What I want you to attack, in order of how badly it would hurt the operator.

  1. Can this control, or anything it wires up, ever leave a control unavailable? The owner's
    standing ruling is that nothing may grey out the Start button — a source may be turned off,
    but he must never lose the ability to act, because when he does he drives the hardware by
    hand outside this program and nothing is recorded. test_toggle_does_not_disable_any_control
    asserts it and a direct search for setEnabled(False) in the new panel finds nothing, but
    the panel is wired into navigation, the tool rail and main_window_v2, and I care more about
    the paths I did not think to test than the one I did.

  2. Does the panel ever disagree with the status bar? feat: make every software interlock operator-optional, warning when suppressed #113 's bottom status bar lists which
    interlocks are disabled. A control that says OFF while the bar says ON is worse than no
    control at all, because he would then trust the wrong one. They must agree after a toggle,
    after a restart, and after an Engine reconnect.

  3. Is a disabled interlock still visible after a restart, and does the run record show it?
    The engine side writes a receipt and carries the disabled interval as run provenance. If the
    interface can put an interlock back on without that being recorded, or leave one off
    invisibly, then a measurement could later be read as if it had been fully guarded.

  4. Disabling a temperature interlock is a real reduction in protection when the sensors are
    sound.
    I have accepted that deliberately, because his two badly soldered sensors and his
    unconnected compressor line make those interlocks fire falsely today. Tell me if the panel
    makes that consequence too easy to reach without understanding it — but note that a
    confirmation he can get trapped behind is NOT an acceptable answer here, and neither is a
    refusal.

Scope note: enable/disable only. Thresholds and channel patterns are deliberately not editable
from the interface; that was not clearly asked for and is a much larger surface.

Disclosure: this change was produced and verified with AI assistance.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a71c92e873

i️ 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 +319 to +323
"cmd": "interlock_set_enabled",
"interlock_name": name,
"enabled": enabled,
"operator": self._operator_edit.text().strip(),
"request_id": uuid.uuid4().hex,

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep Start available after disabling an interlock

When a disable command sent here succeeds, the production SafetyManager adds a software_interlock_disabled readiness blocker; the next typed OperatorSnapshot makes MainWindowV2 call KeithleyPanel.set_safety_ready(False), which disables every per-channel Start button and Start A+B. test_toggle_does_not_disable_any_control misses this because its command stub never delivers the resulting typed snapshot, so the exact operator scenario called out in the review still greys out Start.

AGENTS.md reference: AGENTS.md:L380-L386

Useful? React with 👍 / 👎.

Comment on lines +503 to +508
if name == "interlocks":
widget.interlock_committed.connect(self._apply_interlock_commit)
stale = self._last_safety_observed_at is None or self._last_safety_observed_at < datetime.now(
UTC
) - timedelta(seconds=_SAFETY_MAX_SOURCE_AGE_S)
widget.set_interlocks(self._last_interlocks, stale=stale)

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Populate the panel from a production state source

On a normal launch this always passes None: production SafetyManager._publish_state() publishes only disabled_interlocks, while the complete rows are available only from the interlock_status command, which neither this panel nor the window requests. Consequently _last_interlocks is never populated, _rows stays empty, and the operator sees no controls after startup or Engine reconnect; the tests hide this by manually adding engine.get_operator_state() to synthetic safety telemetry, a condition production never supplies.

AGENTS.md reference: AGENTS.md:L380-L386

Useful? React with 👍 / 👎.

Comment on lines +330 to +333
def _on_toggle_result(self, name: str, result: dict) -> None:
self._last_results[name] = dict(result) if type(result) is dict else {}
state = result.get("interlock") if type(result) is dict else None
if result.get("ok") is True and type(state) is dict:

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reconcile replies that report a committed transition

When interlock_set_enabled persists and applies the transition but later experiment-provenance settlement fails, the engine deliberately returns ok: false, committed: true with interlock_toggle_reconciliation_failed. This branch treats that as an ordinary rejection and neither updates nor re-queries state, so a successfully disabled interlock can remain displayed as enabled and the operator may retry under a false premise; committed/unknown outcomes need reconciliation against engine-owned state instead of retaining the old row.

AGENTS.md reference: AGENTS.md:L490-L491

Useful? React with 👍 / 👎.

Comment on lines +327 to +328
worker = ZmqCommandWorker(command, parent=self)
self._register_worker(worker, lambda result, target=name: self._on_toggle_result(target, result))

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Make repeated Disable clicks idempotent

During ordinary command latency the row keeps its old enabled state, so a double-click starts two workers with different request IDs and both request enabled=False. With an active experiment the first transition creates the open provenance interval, but the second same-state transition replaces the engine's persisted disable_receipt while record_interlock_operator_state() leaves the existing interval bound to the first receipt; on restart, sync_interlock_operator_provenance() cannot find an interval for the second receipt and Engine startup fails. Coalesce an in-flight same-row action or make the command/state owner reject same-state transitions without creating a new receipt.

AGENTS.md reference: AGENTS.md:L20-L22

Useful? React with 👍 / 👎.

Comment on lines +88 to +92
"channel_ids": tuple(canonical_channels),
"enabled": enabled,
"operator_disableable": disableable,
"action": action,
}

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Show the exact trigger condition before allowing Disable

The engine state already includes comparison and threshold, but canonicalization drops both, so the panel shows channels and eventual action without the boundary whose enforcement is being suppressed. The shipped descriptions do not supply those values—for example the operator cannot see that detector_warmup is the 10 K guard or distinguish the 320 K and 350 K overheat guards—yet Disable executes in one click; displaying these fields read-only would preserve the intended non-editable scope while making the reduction in protection understandable.

AGENTS.md reference: AGENTS.md:L529-L536

Useful? React with 👍 / 👎.

Comment on lines +318 to +323
command = {
"cmd": "interlock_set_enabled",
"interlock_name": name,
"enabled": enabled,
"operator": self._operator_edit.text().strip(),
"request_id": uuid.uuid4().hex,

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bind toggles to the displayed engine policy

After an Engine disconnect the window deliberately retains these rows as stale and leaves their buttons actionable, but this command binds only the interlock name and desired boolean. If the reconnected Engine reuses that name with changed channels, threshold, or protective action, clicking the retained row applies the new policy while the operator is still looking at the old consequence; transport mutation compatibility does not bind the displayed policy. Include the displayed policy fingerprint and Engine generation in the request and reject a mismatch without greying out the control.

AGENTS.md reference: AGENTS.md:L533-L536

Useful? React with 👍 / 👎.

Comment on lines +954 to +955
updated = tuple({**row, "enabled": enabled} if row["name"] == name else dict(row) for row in snapshot)
self._set_interlock_presentation(updated, stale=False)

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject telemetry older than an accepted toggle receipt

This applies the command reply without advancing any state revision or timestamp high-water mark. Because command replies and analytics/safety_state arrive over independent ZMQ paths, a fresh-but-pre-command telemetry packet can be processed afterward; _dispatch_safety_evidence() compares it only with the last telemetry timestamp and will overwrite the newly disabled row with its older enabled snapshot. Preserve changed_at or a monotonic state revision from the accepted reply and reject older snapshots so a confirmed OFF state cannot transiently render as ON.

AGENTS.md reference: AGENTS.md:L490-L491

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

#119's five Windows reds are inherited from its base, not caused by this branch.

This is stacked on feat/interlock-operator-optionality (#113). Both failing nodes belong to
that base:

tests/core/test_interlock_operator_optionality.py::test_serialized_operator_journals_cannot_outgrow_the_restart_loader
tests/gui/shell/test_bottom_status_bar_fault_beep.py::test_protocol_maxima_fit_1280_with_full_evidence_in_accessible_detail

The first is already fixed on #113's newer head 47b502085 — a size bound was checked against a
JSON string and then enforced against a file that text mode had rewritten with CRLF on Windows;
atomic_write_text now takes newline="". This branch is based on the older 30e8dd79e and
simply predates that fix.

The second is a genuine open defect on #113, measured this turn: the bar is 960 px with only the
interlock string set, but 1306 px with every field at maximum, and only on Windows — the elision
budget is tuned to one platform's font metrics. A lane is on it.

Nothing here is this pull request's own defect, and I am not going to rebase it onto a
moving base while both are under review. Once #113 settles, this rebases onto it and both nodes
come with it.

The interlock control itself remains verified as posted: production reverted gives 7 failed /
51 passed, restored gives 58 passed, and the 54-file module gate is 1685 passed with no
failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

AltStyle によって変換されたページ (->オリジナル) /