Refactor/status dict#108
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ 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". |
|
@codex review |
|
@MathomJohnson can you work with @sbasu107 and @ColeMov to resolve the conflicts in this PR? |
|
Converted to draft until merge conflicts resolved |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ 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". |
|
@MathomJohnson there are a lot of unnecessary whitespace changes in the diff complicating the files changed can. Can you get rid of unnecessary white space changes today? |
Reverts whitespace-only noise from dashboard.py, g9_driver.py, power_supply_9104.py, README.md, interlocks.py, and vtrx.py back to develop. For cathode_heating.py and utils.py, strips the whitespace changes while keeping the intentional refactor to the nested cathode dict and update_cathode_field method. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Calling clear_value("cathode") would silently replace the nested dict
with None, crashing any subsequent update_cathode_field call. Raise a
clear KeyError instead and cover it with a test.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5c96d4f8
ℹ️ 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".
| cathode = self.dict_logger["cathode"] | ||
| if cathode_label not in cathode: |
There was a problem hiding this comment.
Prevent schema corruption when updating cathode root
update_cathode_field assumes self.dict_logger["cathode"] is always a mapping, but update_field still permits field == "cathode" and can overwrite that subtree with a scalar or None. After that, the membership check in update_cathode_field throws TypeError, and CathodeHeatingSubsystem.update_data() invokes this path for clamp-temperature updates outside its local try/except, so periodic updates can break once the root key is overwritten. Rejecting top-level "cathode" writes in update_field (consistent with clear_value) or validating the schema before membership checks would avoid this failure mode.
Useful? React with 👍 / 👎.
Motivation
The old flat keys had two problems: inconsistent naming conventions ("Cathode A - Heater Current:" with a colon vs "clamp_temperature_A") and colons embedded in key names, which are unfriendly for JSON querying in Supabase. The new nested structure is easier to iterate over per-cathode and stores cleanly as JSON.