Conversation
…control
The owner ruled on 2026-08-28: "nothing should disable start button. source may
be turned off if operator forgot, or if there is misbehaving, but not disable
the button".
Measured before this change: the end-to-end thermal run timed out because the
Source panel's Start action never became available. The cause was not the panel
wiring but `detector_warmup` holding `stop_source` at 77.5 K against a 10 K
threshold, which made Safety not-ready, which greyed out Start and the P/V/I
spin boxes and showed "Управление заблокировано".
The defect was never that the guard fired. It is that a guard could reach the
button at all.
`_safety_ready` carried two different meanings that a boolean cannot separate:
Safety authority is present and reports a current condition, and no
authoritative Safety state exists at all. Only the first is a guard judging the
operator. The second is the software being blind, which is a different question
and is still open with the owner.
So the cause is now explicit, as `SafetyGateCause`, and the two are treated
differently:
AUTHORITATIVE_NOT_READY - a condition. Start and the parameter controls stay
available, and the panel shows a WARNING naming the
condition and saying it may stop a running source.
AUTHORITY_UNAVAILABLE - blindness. Everything except emergency-off stays
disabled, exactly as before.
Nothing suppresses the guard itself. A `stop_source` interlock still stops a
source that is running; the operator may start it, be told why that is
questionable, and be stopped by the guard if the condition holds. His remedy is
to disable that interlock, which is the sibling change.
The decision is structural, not textual: no code branches on the wording of a
Safety reason, because that text is operator-facing prose that will be reworded.
|
@codex review Head under review: This reverses a safety-adjacent direction by owner ruling, so the useful review
Stated openly rather than left to be discovered: on this branch alone the probe |
![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: aade65d80d
ℹ️ 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".
| SafetyGateCause.AUTHORITATIVE_NOT_READY | ||
| if ( | ||
| not ready | ||
| and bridge_instance_id is not None | ||
| and not transport_stale |
There was a problem hiding this comment.
At aade65d80db29856a572eec665b78bd76f7380fc, a current typed snapshot whose Safety receipt is unavailable legitimately carries ReadinessTruth.UNKNOWN/SafetyLifecycle.UNKNOWN, a non-transport caution state, and no transport reason codes; this condition nevertheless selects AUTHORITATIVE_NOT_READY. The panel treats that cause permissively, enabling Start for an OFF channel and enabling all parameter controls, so missing Safety authority reaches the operator as a warning. Restrict the permissive cause to the explicitly authoritative blocked/interlock shape and classify UNKNOWN readiness/lifecycle as AUTHORITY_UNAVAILABLE.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| cause = ( | ||
| SafetyGateCause.AUTHORITATIVE_NOT_READY | ||
| if not ready and current_authority | ||
| else SafetyGateCause.AUTHORITY_UNAVAILABLE |
There was a problem hiding this comment.
When a STALE or DISCONNECTED typed snapshot arrives before the source overlay is constructed, _apply_operator_snapshot_safety retains its bridge/experiment bindings while setting readiness false; this lazy-open branch then checks only those bindings and labels the stale snapshot AUTHORITATIVE_NOT_READY. Opening the panel therefore enables Start for an OFF channel and all parameter controls, whereas the already-open-panel path explicitly maps the same transport-stale snapshot to AUTHORITY_UNAVAILABLE. Carry the transport/readiness classification into lazy construction rather than reconstructing authority from identity alone.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| condition = self._safety_reason or "Safety сообщает о действующем условии" | ||
| text = ( | ||
| f"ПРЕДУПРЕЖДЕНИЕ Safety: {condition}. " |
There was a problem hiding this comment.
The production snapshot composer sets readiness.status.operator_text to the generic text Backend readiness authority; the actual condition names and operator text are in readiness.blockers. Because this branch presents _safety_reason as the condition, a real typed interlock displays only ПРЕДУПРЕЖДЕНИЕ Safety: Backend readiness authority and persists that generic text in the Start receipt, unlike the test fixture that injects Interlock stop_source: detector_warmup directly into the summary status. Build the warning from the authoritative blocker evidence so the operator can see what may stop the source.
AGENTS.md reference: AGENTS.md:L533-L536
Useful? React with 👍 / 👎.
| interactive_ok = ( | ||
| self._connected | ||
| and self._safety_ready | ||
| and (self._safety_ready or self._safety_gate_cause is SafetyGateCause.AUTHORITATIVE_NOT_READY) |
There was a problem hiding this comment.
This new predicate reverses a reusable Keithley state semantic, but docs/design-system/cryodaq-primitives/keithley-panel.md still says every Safety-gated state disables Start/Stop/spins and still documents only the boolean set_safety_ready(ok, reason) API. Leaving the strict canonical component specification, examples, version, and changelog unchanged makes the owner-approved behavior look like a regression to later maintainers and design-system tests; update those governed artifacts in this slice.
AGENTS.md reference: AGENTS.md:L537-L539
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 reconciles the design-system documentation The coordinator's control: with the two production files reverted, 5 nodes Still worth your hardest look: over-severing. Docs gate green at this head, derived pair regenerated last. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 433a060134
ℹ️ 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 current_safe_off: | ||
| # The negative observation revokes readiness, not the | ||
| # still-current bridge/experiment identity binding. | ||
| self._typed_safety_ready = False | ||
| self._last_safety_gate_cause = SafetyGateCause.AUTHORITATIVE_NOT_READY |
There was a problem hiding this comment.
When a typed snapshot has been accepted and its bridge/experiment binding still matches, any fresh analytics/safety_state packet with state="safe_off" reaches this branch and assigns AUTHORITATIVE_NOT_READY, which enables Start and parameter controls. This feed is explicitly legacy observational telemetry, and SafetyManager._publish_state supplies neither a typed ReadinessBlocker nor bridge-binding evidence, so a routine SAFE_OFF event after a READY cut can create the permissive cause without the current authoritative blocker required by this change. Treat this packet only as revocation/AUTHORITY_UNAVAILABLE and wait for a newer typed BLOCKED cut before enabling controls.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| spec = (DESIGN_ROOT / "cryodaq-primitives" / "keithley-panel.md").read_text(encoding="utf-8") | ||
| version = (DESIGN_ROOT / "VERSION").read_text(encoding="utf-8").strip() | ||
| changelog = (DESIGN_ROOT / "CHANGELOG.md").read_text(encoding="utf-8") |
There was a problem hiding this comment.
This guard never reads either production file: it searches only the Markdown specification, version, and changelog. If the enum names, default cause, or permissive enablement predicate are removed or changed while the prose remains intact, every assertion still passes, so the test cannot prevent the code/documentation drift it claims to bind. Read or exercise the production implementation as part of this guard rather than restating only the documentation.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
…ranch #111 and #114 both edit the shell's Keithley safety wiring, for different reasons, and conflicted in main_window_v2.py. Resolved by intent rather than by hunk, keeping both properties: #111 - the panel must LEARN the source's confirmed state when it is opened lazily, after the state was already published. #114 - a current, authoritative Safety condition WARNS and leaves Start available; a missing Safety authority still disables it. They are not in tension: one is about what the panel knows, the other about what it permits. But they touch adjacent lines, and taking either side wholesale would have compiled, passed most tests, and silently deleted a property. Each conflicted hunk kept both sides. Where #114 sets a typed gate cause on invalidation, #111's typed snapshot reset is kept alongside it: taking #114 alone would have retained invalid typed replay evidence, and taking #111 alone would have lost the explicit fail-closed cause reset. The three properties are pinned by named nodes, all passing: a current authoritative condition leaves Start enabled; missing Safety authority leaves Start disabled; a lazily opened panel reaches confirmed OFF.
|
@codex review Head under review: #111 has been taken into this branch by merge. The two conflicted in Both properties are kept:
The hunk most worth your eye is safety-authority invalidation: #114's typed Three named nodes pin the properties and all pass: Measured by the coordinator on a tree carrying this integration plus #110, #115, Docs gate green at this head; derived pair regenerated as the last commit; the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e3912fd37
ℹ️ 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".
|
|
||
| def test_keithley_overlay_channel_state_replay_cleared_on_lifecycle_reset() -> None: | ||
| _app() | ||
| w = MainWindowV2() |
There was a problem hiding this comment.
Constructing MainWindowV2 without a live bridge leaves _current_bridge_instance_id() as None, so the OFF packet sent later is rejected by the production cache-admission condition and the cache is already empty before the experiment changes. This test therefore stays green even if lifecycle invalidation is absent; inject live_zmq_bridge, assert the OFF state was cached, and trigger the actual producer-lifecycle reset before checking that replay was cleared.
AGENTS.md reference: AGENTS.md:L380-L386
Useful? React with 👍 / 👎.
| warning_receipt = None | ||
| raw_warning_choice = cmd.get("operator_warning_choice") | ||
| try: | ||
| warning_choice = _keithley_warning_choice(raw_warning_choice) |
There was a problem hiding this comment.
When a compatibility-qualified caller sends keithley_start while Safety is in the warning-permissive SAFE_OFF/interlock condition, omitting operator_warning_choice makes _keithley_warning_choice(None) return None and this handler proceeds directly to request_run. The GUI currently supplies the field, but the authoritative command boundary therefore permits another or regressed client to energize the source without first persisting the operator-choice receipt that this change introduces; determine the warning requirement server-side and reject an unreceipted Start before dispatch.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| if self._current_bridge_instance_id() is not None: | ||
| source_state_key = channel.rsplit("/", maxsplit=1)[-1] | ||
| self._keithley_channel_state_snapshot[source_state_key] = reading |
There was a problem hiding this comment.
This admission check verifies only that some live bridge ID exists; DataBroker.publish() does not reserve channels by publisher, so any passive in-process producer can emit analytics/keithley_channel_state/smua with metadata["state"] = "off". If that packet arrives before the panel is opened, the new cache retains and replays it, and a warning-permissive Safety condition then enables Start from evidence that was never issued by SafetyManager; previously the unopened panel discarded the packet. Require producer-bound source-state evidence rather than granting actuator-state authority from the channel name and metadata shape.
AGENTS.md reference: AGENTS.md:L466-L469
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 removes a new identity-inference site the That is not cosmetic. A shell that spells the producer's name itself can drift Coordinator's measurements: the C2 sweep passes (14 nodes) with no unregistered Worth your scepticism: the check must still REJECT a reading that lacks the Docs gate green at this head; derived pair regenerated as the last commit. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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: What this closes: a safety decision could wait forever on a disk write. The branch's own node hung indefinitely —
Reversion control, run this turn on Ubuntu 22.04 — production reverted, tests kept: Note the shape of those node names: a stalled persistence does not block an operator-requested What I want you to attack. The timeout outcome is a judgement, not a mechanism: when the Related and deliberately not duplicated here: you raised the same shape on #117 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: 42db3be8d9
ℹ️ 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 error.strip(): | ||
| self._set_banner( | ||
| f"{description}: Engine подтвердил выполнение. {error.strip()}", | ||
| theme.STATUS_CAUTION, | ||
| auto_clear=False, |
There was a problem hiding this comment.
When Start A+B is in flight and the first channel returns ok with an unconfirmed-persistence warning, this branch displays the caution while the other command remains pending but does not retain it. If the second channel then returns an ordinary success, that callback reaches show_info() and overwrites the caution with a generic confirmation, so reply ordering can hide the only operator-visible indication that one Start choice was not durably recorded. Preserve the warning until all commands in the A+B batch settle and render the aggregate outcome.
AGENTS.md reference: AGENTS.md:L529-L535
Useful? React with 👍 / 👎.
| if not done: | ||
| commit_task.cancel() | ||
| commit_task.add_done_callback( | ||
| lambda task: task.exception() if not task.cancelled() else None, |
There was a problem hiding this comment.
When the real SQLiteWriter executor call is the operation that stalls, cancelling commit_task here only cancels the wrapper: append_operator_log_idempotent() has already created a retained owner whose _run_owned_executor() deliberately suppresses cancellation until the thread future finishes. The command can therefore return after one second, but the stalled owner remains in _owned_write_tasks, and SQLiteWriter.stop() later waits for that owner and calls executor.shutdown(wait=True), moving the same unbounded disk wait into engine shutdown (and accumulating more retained owners after repeated Starts). The timed-out write needs a bounded/isolated settlement that cannot leave shutdown waiting forever.
AGENTS.md reference: AGENTS.md:L20-L22
Useful? React with 👍 / 👎.
| if ( | ||
| type(command_warning_requirement) is not RunWarningRequirement | ||
| or command_warning_requirement._authority is not self._run_warning_authority | ||
| or command_warning_requirement.revision != self._operator_safety_snapshot.revision |
There was a problem hiding this comment.
When warning persistence takes long enough for any normal Safety reading or monitor tick to occur, this exact revision comparison rejects the subsequent Start even if the authoritative warning and every relevant Safety fact are unchanged. _collect_loop() advances the snapshot revision for every reading, and _run_checks() advances it every second; because the production persistence bound is also one second, the stalled-write path is especially likely to return keithley_warning_requirement_changed instead of ending in the operator-requested RUN state, while the 10 ms test timeout avoids the collision. Bind the choice to a generation that changes only when the warning decision changes, rather than the routinely refreshed presentation revision.
Useful? React with 👍 / 👎.
…he Start control Integrates #114 onto the candidate, and fixes one defect that only appears once #114 and #116 are in the same tree. The severing: main_window_v2 now distinguishes an authoritative blocked state from an unavailable one. A current interlock condition warns and may stop a running source; it does not grey out Start. Missing or stale Safety authority still disables ordinary controls, and emergency OFF stays available throughout. AUTHORITATIVE_NOT_READY and AUTHORITY_UNAVAILABLE stay distinct -- collapsing them to one boolean is what took the operator's button away. Conflicts were resolved per file rather than per side, because the two branches do not contest the same behaviour -- they have different bases. safety_manager and keithley_panel keep this candidate's A2 content (evidence staleness, the producer revision, replay-is-not-observation); main_window_v2 and its wiring tests take #114's severing. Taking either side wholesale would have silently reverted the other. THE INTEGRATION DEFECT, found by #114's test once both were present: the panel read the producer revision with `reading.metadata.get("source_observation_revision")`, which returns None when a producer stamps no revision. #116's stricter handling accepts the sentinel or an int, so None fell through both branches, the revision never advanced, and a genuine new observation could never reconcile an unknown command outcome -- leaving the operator's controls latched indefinitely. Absence now means unspecified, not invalid. Neither pull request could have found this alone: #116's tests always stamp a revision and #114's path does not. Verified on the candidate: 298 passed across the wiring, channel-state, panel, safety-manager, driver and sensor suites -- all three blocks' properties holding at once. Block A3. Semantic delta only; documentation and generated artifacts reconcile at freeze.
…s him Two merged-in-parallel branches rewrote the operator warning-choice persistence path in mutually exclusive ways, and neither referenced the other: #114 carried command_warning_requirement (x8 in engine.py, x0 of the committer), #117 carried warning_choice_committer (x6, x0 of the requirement). The candidate held #114's design only because #114 was integrated first, which is not a reason. Both keep the receipt write outside _cmd_lock, so that does not separate them. What separates them is what can refuse the operator. #114 validates a prepared RunWarningRequirement token inside _cmd_lock against the live snapshot, so a token that goes stale is a refusal path by construction - and it had already produced one: a routine one-second snapshot tick landing inside the one-second persistence bound returned keithley_warning_requirement_changed instead of starting the source. #117's committer has no token; its failure mode is a receipt recording "not confirmed". A refusal sends the operator to drive the source by hand outside this program, where nothing is recorded at all. So this takes #117's committer shape and ports #114's properties onto it, keeping #114's Start severing untouched. Four properties, each with a guard, each red before the change and green after: 1. a stalled persistence does not block an operator-requested start; 2. a missing or unconfirmed persistence is receipted, and the start is not refused - the operator can see his choice was not durably recorded; 3. an emergency OFF is not overtaken by an in-flight receipt write: no driver ON may occur after the OFF epoch; 4. a RUN already QUEUED when emergency OFF was pressed does not energize. It captures its abort epoch at ADMISSION, before it can wait behind another RUN at _run_request_lock, so an OFF while it is queued always changes the epoch it must present at both command-lock cuts. A malformed GUI correlation id no longer refuses the command either: it is reused when well-formed and otherwise replaced by a fresh token, so the receipt always carries an identity without that identity being authority. The operator-log entry for a confirmed start intent is now attributed to author="system"/source="auto" rather than to the operator, because the system composes the text; it keeps the operator_warning_choice tag and embeds the operator's warning verbatim. That is a deliberate change to what the run record asserts about a safety acknowledgment and is called out for the owner rather than settled here. Verified on this candidate, not inherited from the branch: - reversion control: production reverted to HEAD, the four guards go red (by interface absence - the previous design cannot express the committer), restored, green; - mutation control P4: moving the epoch capture inside _run_request_lock reddens exactly guard 4; - mutation control P3: disabling the post-receipt cut alone changes nothing because the check exists at both cuts; disabling both reddens guards 3 and 4. The property is defended in depth; - tests/core/test_experiment_commands.py: 72 passed, matching the candidate before the change - the stall the branch reported as pre-existing does not reproduce here and was a sandbox artifact; - test nodes in the two rewritten test files: 179 before, 179 after, none removed, none renamed, no skip or xfail introduced.
1 participant
An interlock may stop a running source. It may never grey out the Start control.
The ruling
What was actually wrong
The end-to-end thermal run timed out because Start never became available. The
cause was not the panel wiring:
detector_warmupheldstop_sourceat77.5 K against a 10 K threshold, Safety went not-ready, and that greyed out
Start, the P/V/I spin boxes, and showed "Управление заблокировано".
The defect was never that the guard fired. It is that a guard could reach the
button at all. Earlier attempts at this went after the guard — retiring the row,
then adjusting its threshold — which is treating the symptom.
The change
_safety_readycarried two meanings a boolean cannot separate:judging the operator;
Only the first is what the ruling severs. The second is a different question and
is still open with the owner, so it is deliberately left alone.
The cause is now explicit as
SafetyGateCause:AUTHORITATIVE_NOT_READYAUTHORITY_UNAVAILABLEemergency_offis unaffected and always available.Nothing suppresses the guard. A
stop_sourceinterlock still stops a sourcethat is running. The operator may start it, be told why that is questionable,
and be stopped by the guard if the condition holds — that loop is the intent,
not a bug. His remedy is to disable the interlock, which is #113.
No code branches on the wording of a Safety reason; that text is operator-facing
prose and will be reworded.
Verification
The lane's own control was a collection error, which proves a symbol is new and
little else, so the coordinator ran a behavioural mutation instead, in a
throwaway worktree:
(
and self._safety_ready) reddens 6 of 12 requested nodes; restoring makesall 12 pass.
authorization_reasonterm separately reddens the receipt node.test_missing_safety_authority_still_disables_controls_except_emergencyis thecontrol on this change — it must stay green, and it does, under both
mutations. Without it, over-severing would look like success.
Two assertions were changed, both in that one test: they pinned the old
fault_latchedreason string. OneisHidden()check was lost there; visibilityof the warning path is asserted by
test_authoritative_safety_condition_shows_visible_warning. Reviewer: pleaseconfirm you are content with that trade.
Docs gate green at this head; the derived pair was regenerated as the last commit.
Measured end-to-end, stated exactly
Running the source-gate probe against this branch alone:
The interlock term is severed — it is no longer what holds the button. Start
is still disabled, now by
channel_state == "unknown", which is a differentcondition: the panel has not yet been told the source's authoritative state.
That is what #111 addresses and it is not merged here. This branch does not,
on its own, make the end-to-end run pass, and it is not claimed to.
Not determined
probe worktree is the next measurement.
(
test_cancelled_reply_does_not_cancel_late_commit_or_completion_side_effects)hung during a broad run.
Disclosure
Written by an AI lane (
gpt-5.6-sol) under an agent brief; verified and landedby the coordinating agent. The mutation controls above were run by the
coordinator, independently of the lane's report, and contradicted it.