Skip to content

Refactor/status dict#108

Open
MathomJohnson wants to merge 5 commits intodevelopfrom
refactor/status-dict
Open

Refactor/status dict#108
MathomJohnson wants to merge 5 commits intodevelopfrom
refactor/status-dict

Conversation

@MathomJohnson
Copy link
Copy Markdown
Member

  • Replaces 9 flat, inconsistently-named cathode keys in dict_logger with a single nested "cathode" key grouped by label (A/B/C) and subfield (heater_current, heater_voltage, clamp_temperature)
  • Adds update_cathode_field(cathode_label, subfield, value) to Logger with input validation on both arguments — raises a descriptive KeyError for invalid labels or subfields, consistent with update_field
  • Updates all three callsites in cathode_heating.py to use the new method
  • Cleans up trailing whitespace across 8 files (no logic changes)

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.

@MathomJohnson
Copy link
Copy Markdown
Member Author

@codex

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ 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".

@bwalkerMIR
Copy link
Copy Markdown
Collaborator

@codex review

@bwalkerMIR
Copy link
Copy Markdown
Collaborator

@MathomJohnson can you work with @sbasu107 and @ColeMov to resolve the conflicts in this PR?

@bwalkerMIR bwalkerMIR marked this pull request as draft April 30, 2026 02:57
@bwalkerMIR
Copy link
Copy Markdown
Collaborator

Converted to draft until merge conflicts resolved

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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".

@bwalkerMIR
Copy link
Copy Markdown
Collaborator

@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?

MathomJohnson and others added 2 commits April 30, 2026 13:20
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>
@bwalkerMIR
Copy link
Copy Markdown
Collaborator

@codex review

Copy link
Copy Markdown

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

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: 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".

Comment thread utils.py
Comment on lines +156 to +157
cathode = self.dict_logger["cathode"]
if cathode_label not in cathode:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

@bwalkerMIR bwalkerMIR marked this pull request as ready for review April 30, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants