Skip to content

Fix unit_labels override writing to raw_limits in TxtLoader#603

Merged
jepegit merged 1 commit into
masterfrom
claude/reverent-hellman-78168f
Jul 20, 2026
Merged

Fix unit_labels override writing to raw_limits in TxtLoader#603
jepegit merged 1 commit into
masterfrom
claude/reverent-hellman-78168f

Conversation

@jepegit

@jepegit jepegit commented Jul 20, 2026

Copy link
Copy Markdown
Owner

What

TxtLoader.parse_loader_parameters had a copy-paste slip in its override handling: the unit_labels= branch updated config_params.raw_limits instead of config_params.unit_labels.

if unit_labels := kwargs.get("unit_labels", None):
    logging.critical(f"overriding unit_labels: {unit_labels}")
-   self.config_params.raw_limits.update(unit_labels)
+   self.config_params.unit_labels.update(unit_labels)

Why

  • Passing unit_labels= to a loader had no effect on unit labels, so vendor column-name templates (e.g. neware's Current({{ current }})) could not be overridden this way.
  • The unit-label strings silently polluted raw_limits instead.

The bug is bracketed by a correct raw_units branch above and a correct raw_limits branch immediately below, which is what makes it a copy-paste slip rather than intent. git blame traces it to a9de15c4"implement renaming original headers based on unit labels" — whose stated purpose was to wire unit_labels.

Test

Adds test_unit_labels_override_lands_in_unit_labels_not_raw_limits in tests/test_loader_two_stage.py: passes unit_labels={"current": "A"} to a neware loader and asserts it lands in config_params.unit_labels while raw_limits is unchanged. Verified it fails on the pre-fix code and passes after; the surrounding two-stage and port-parity suites stay green.

Notes for reviewer

Found while porting loader post-processors for #560 (PR #601); kept out of that PR to keep it scoped to the port.

🤖 Generated with Claude Code

`parse_loader_parameters` had a copy-paste slip: the `unit_labels=`
override branch updated `config_params.raw_limits` instead of
`config_params.unit_labels`. Introduced in a9de15c ("implement renaming
original headers based on unit labels"), whose intent was clearly to wire
`unit_labels`.

Effect of the bug: passing `unit_labels=` to a loader had no effect on unit
labels (vendor column-name templates like neware's `Current({{ current }})`
could not be overridden this way), and the unit-label strings silently
polluted `raw_limits`.

Add a regression test asserting a `unit_labels=` override lands in
`config_params.unit_labels` and leaves `raw_limits` untouched.

Found while porting loader post-processors for #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 20, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@jepegit
jepegit merged commit 5468059 into master Jul 20, 2026
5 checks passed
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.

1 participant