Finding
Correction.triggered_by is declared on the class but never read anywhere in the framework. It is a parallel piece of information that duplicates the check↔correction binding already established by _register_check.
Three references in the entire codebase:
Nowhere reads it. Not base.py, not the tests, not _assemble_report, not the default correct(). The framework routes corrections through _registered_checks (populated by _register_check), so the binding source-of-truth is the registration call, not the class attribute.
This means an author can set triggered_by = "snr" and then register the correction against a check named "peak_exists" and the framework will silently use it for "peak_exists" — the triggered_by string just sits there, potentially lying.
Options
- (A) Remove
triggered_by from the Correction base class entirely. No behavior changes — existing subclasses that set it would just need the attribute removed. The check name is already known to the framework at correction-application time (it's the check.name from the failing CheckResult), so anywhere reports want to surface the binding, they can read it from the CheckResult instead.
- (B) Keep
triggered_by but populate it automatically at registration time. _register_check would set correction.triggered_by = name so the value can never drift from what was actually registered.
- (C) Actually start using
triggered_by in the report, ideally combined with (B) so authored values can't drift from registered ones.
Recommended: (A). Smallest surface area, removes a footgun, no real loss because the registered name is already discoverable.
Tasks
Background
Surfaced while writing the protocols user-guide page. The user guide currently documents triggered_by as informational metadata, which is technically true but misleading because the value is never displayed anywhere — the doc should not have to teach a piece of unused state.
Finding
Correction.triggered_byis declared on the class but never read anywhere in the framework. It is a parallel piece of information that duplicates the check↔correction binding already established by_register_check.Three references in the entire codebase:
src/labcore/protocols/base.py:241— the class attribute declaration onCorrectionsrc/labcore/protocols/base.py:224— an example inside a docstring (not executed)src/labcore/testing/protocol_dummy/gaussian_with_correction.py:68— set on the dummy correctionNowhere reads it. Not
base.py, not the tests, not_assemble_report, not the defaultcorrect(). The framework routes corrections through_registered_checks(populated by_register_check), so the binding source-of-truth is the registration call, not the class attribute.This means an author can set
triggered_by = "snr"and then register the correction against a check named"peak_exists"and the framework will silently use it for"peak_exists"— thetriggered_bystring just sits there, potentially lying.Options
triggered_byfrom theCorrectionbase class entirely. No behavior changes — existing subclasses that set it would just need the attribute removed. The check name is already known to the framework at correction-application time (it's thecheck.namefrom the failingCheckResult), so anywhere reports want to surface the binding, they can read it from theCheckResultinstead.triggered_bybut populate it automatically at registration time._register_checkwould setcorrection.triggered_by = nameso the value can never drift from what was actually registered.triggered_byin the report, ideally combined with (B) so authored values can't drift from registered ones.Recommended: (A). Smallest surface area, removes a footgun, no real loss because the registered name is already discoverable.
Tasks
labcore.CQEDToolboxoperations for anyCorrectionsubclasses that currently set or readtriggered_by, and update them in lockstep with the labcore change so nothing breaks. (CQEDToolbox is the largest known consumer of the corrections API.)docs/user_guide/protocols/operations.md, "Corrections" section) to remove or rephrase the bullet that currently describestriggered_by.Background
Surfaced while writing the protocols user-guide page. The user guide currently documents
triggered_byas informational metadata, which is technically true but misleading because the value is never displayed anywhere — the doc should not have to teach a piece of unused state.