CSV typed-column merge silently rewrites a typo'd enum in the TagConfigJson cell before any gate can see it #553

Closed
opened 2026-08-10 00:26:20 -04:00 by dohertj2 · 1 comment
Owner

Found by the #551 work; the CSV analogue of #549 (silent retype instead of a dead tag). CsvDriverTagMap.ApplyTypedColumns round-trips the free-form TagConfigJson cell through the driver model's FromJson/ToJson to merge the typed columns — and that lenient load turns "dataType":"Intt16" into Int16 before the #551 import gate (or any later gate) can judge it. The operator's typo is silently replaced by the default; only keys the model does not own (e.g. keys like Modbus stringByteOrder before #551, or unknown keys) survive the round-trip to be judged.

Related: the MTConnect CSV export writes driverDataType into the residual cell (the typed map doesn't own it), so a valid driverDataType re-imports and out-ranks the MtDataType column at the driver — an operator edits MtDataType in the exported CSV, re-imports, and the change is silently ignored (#550's editor normalisation only repairs it on the next editor save).

Fix direction: validate the raw pre-merge cell (run TagConfigValidator over the as-imported blob before ApplyTypedColumns), and/or make the merge normalise runtime-preferred aliases the same way the editors now do. A rejected row must be an ERROR, not a silent rewrite.

Found by the #551 work; the CSV analogue of #549 (silent retype instead of a dead tag). `CsvDriverTagMap.ApplyTypedColumns` round-trips the free-form `TagConfigJson` cell through the driver model's `FromJson`/`ToJson` to merge the typed columns — and that lenient load turns `"dataType":"Intt16"` into `Int16` **before** the #551 import gate (or any later gate) can judge it. The operator's typo is silently replaced by the default; only keys the model does not own (e.g. keys like Modbus `stringByteOrder` before #551, or unknown keys) survive the round-trip to be judged. Related: the MTConnect CSV export writes `driverDataType` into the residual cell (the typed map doesn't own it), so a *valid* `driverDataType` re-imports and out-ranks the `MtDataType` column at the driver — an operator edits `MtDataType` in the exported CSV, re-imports, and the change is silently ignored (#550's editor normalisation only repairs it on the next editor save). Fix direction: validate the raw pre-merge cell (run `TagConfigValidator` over the as-imported blob before `ApplyTypedColumns`), and/or make the merge normalise runtime-preferred aliases the same way the editors now do. A rejected row must be an ERROR, not a silent rewrite.
Author
Owner

FIXED — merged to master as 62fe6c83 (branch commit cf2af8a5).

Characterization on the current tree first (the issue was filed against pre-#550/#551 analysis), which narrowed the real defect: MTConnect's invalid-alias and invalid-cell cases were already caught (#550 flowing through the #551 gate — MTConnect uses a raw-key map that never round-trips), but the model-backed maps (Modbus/S7) genuinely laundered ("dataType":"Intt16" → silent Int16), and the valid-alias case plus the whole export side were still broken: ReadTypedColumns read only the canonical key, so export printed the ignored value in the typed column and left the winning alias in the residual — actively setting the re-import trap.

Fix — evidence preservation, not a second validator:

  • ModelBackedCsvDriverTagMap.ApplyTypedColumns restores the base cell's own tokens over the model's serialisation for every key no non-blank typed column overrode. Fragment + typed-columns composition is byte-identical; nothing in the CSV layer decides validity — the #551 gate (TagConfigValidator → each model's Validate()) stays the one authority, now judging unlaundered evidence.
  • CsvTypedColumn gained ReadKeys/OwnedJsonKeys — the full ordered spelling set the RUNTIME reads (MTConnect driverDataType→dataType, fullName→dataItemId→address, mirroring TryFromTagConfig). A firing column clears out-ranking aliases (so an edited MtDataType genuinely wins), the export residual strips them, and export cells show the effective value via precedence-ordered ReadEffective.
  • Override-contradiction semantics: column wins, openly — the review grid names each override (TypedOverrodeFallback extended to the whole owned key set). #549's doctrine is against silent rewrites; every token that reaches the stored blob is judged, and a token the operator overrode is discarded visibly.

Falsified: guard neutered → exactly the 7 defect-closing tests red, 8 acceptance controls green, and the #551 gate suites stayed 53/53 green under the neutered build (layer in front of the gate, not a replacement). Verified on the merged tree: AdminUI 1262, MTConnect 534, Modbus 373, S7 285, TwinCAT 205 — all green.

Residual found en route, filed separately: non-enum model-owned keys (e.g. Modbus address) are judged by the deploy-gate Inspect since #551 but by no authoring model (#554).

**FIXED** — merged to master as `62fe6c83` (branch commit `cf2af8a5`). Characterization on the current tree first (the issue was filed against pre-#550/#551 analysis), which narrowed the real defect: MTConnect's invalid-alias and invalid-cell cases were **already caught** (#550 flowing through the #551 gate — MTConnect uses a raw-key map that never round-trips), but the model-backed maps (Modbus/S7) genuinely laundered (`"dataType":"Intt16"` → silent `Int16`), and the **valid**-alias case plus the whole export side were still broken: `ReadTypedColumns` read only the canonical key, so export printed the *ignored* value in the typed column and left the *winning* alias in the residual — actively setting the re-import trap. Fix — evidence preservation, not a second validator: - `ModelBackedCsvDriverTagMap.ApplyTypedColumns` restores the base cell's own tokens over the model's serialisation for every key no non-blank typed column overrode. Fragment + typed-columns composition is byte-identical; nothing in the CSV layer decides validity — the #551 gate (`TagConfigValidator` → each model's `Validate()`) stays the one authority, now judging unlaundered evidence. - `CsvTypedColumn` gained `ReadKeys`/`OwnedJsonKeys` — the full ordered spelling set the RUNTIME reads (MTConnect `driverDataType`→`dataType`, `fullName`→`dataItemId`→`address`, mirroring `TryFromTagConfig`). A firing column clears out-ranking aliases (so an edited `MtDataType` genuinely wins), the export residual strips them, and export cells show the *effective* value via precedence-ordered `ReadEffective`. - Override-contradiction semantics: **column wins, openly** — the review grid names each override (`TypedOverrodeFallback` extended to the whole owned key set). #549's doctrine is against *silent* rewrites; every token that reaches the stored blob is judged, and a token the operator overrode is discarded visibly. Falsified: guard neutered → exactly the 7 defect-closing tests red, 8 acceptance controls green, and the #551 gate suites stayed 53/53 green under the neutered build (layer in front of the gate, not a replacement). Verified on the merged tree: AdminUI 1262, MTConnect 534, Modbus 373, S7 285, TwinCAT 205 — all green. Residual found en route, filed separately: non-enum model-owned keys (e.g. Modbus `address`) are judged by the deploy-gate `Inspect` since #551 but by no authoring model (#554).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#553