Skip to content

fix(common): coerce empty Result to UNAVAILABLE by value class - #217

Merged
PatrickRitchie merged 14 commits into
TrakHound:masterfrom
ottobolyos:fix/agent-empty-result-unavailable
Aug 21, 2026
Merged

fix(common): coerce empty Result to UNAVAILABLE by value class#217
PatrickRitchie merged 14 commits into
TrakHound:masterfrom
ottobolyos:fix/agent-empty-result-unavailable

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Reshapes the empty-Result coerce on MTConnectAgent.AddObservation from an unconditional rewrite to a value-class-aware one, per @PatrickRitchie's discussion feedback (comment 5322277097). The MTConnect Standard, Part 2 - Devices Information Model, ties Result validity to the DataItem's category and (for Events) its controlled vocabulary; the SDK now coerces null / empty / whitespace Results only where the value class forbids the empty string.

Three value classes are derived from the DataItem's category, representation, and type:

  • Numeric - every VALUE-representation SAMPLE (Part 2 - "Sample MUST always be reported in float") plus the numeric-typed VALUE-representation Events enumerated in the SysML model (PART_COUNT, LINE_NUMBER, BLOCK_COUNT, HARDNESS, TOOL_OFFSET, and every kindred integer/float result attribute): empty Results are always coerced.
  • Enumeration - VALUE-representation EVENT DataItems whose Type has a controlled vocabulary (EXECUTION, CONTROLLER_MODE, AVAILABILITY, and the other Events whose value is defined by an MTConnect.Observations.Events.<Type> enum): empty Results are coerced by default; the new AllowEmptyResultForEnumEvents configuration flag preserves the empty Result when an integrator needs parity with adapters that emit empty values for these Events.
  • String - free-form VALUE-representation EVENT DataItems (PROGRAM, MESSAGE, TOOL_ID, ASSET_CHANGED, and every other non-vocabulary Type): empty Results are never coerced. The standard's Observation::result default value type is string, does not forbid the empty string, and the reference C++ agent accepts empty strings for these Events.

CONDITION observations are untouched (their state axis - Normal / Warning / Fault / Unavailable - is handled separately via ConditionLevel). DATA_SET / TABLE / TIME_SERIES representations carry structured payloads (Entries, Cells, Samples) rather than a single Result and are short-circuited to the String class - their inputs legitimately omit the Result key by design, so the coerce path leaves them untouched for both SAMPLE and EVENT categories.

Behavior change

Input Value class Post-fix wire payload
Sample AMPERAGE = "" Numeric UNAVAILABLE
Event PART_COUNT = "" Numeric (SysML integer) UNAVAILABLE
Event EXECUTION = "" (flag default) Enumeration UNAVAILABLE
Event EXECUTION = "" (flag = true) Enumeration "" (verbatim)
Event PROGRAM = "" String "" (verbatim)
Event MESSAGE = "" String "" (verbatim)
Event TOOL_ID = "" String "" (verbatim)
Event ASSET_CHANGED = "" String "" (verbatim)
SAMPLE X (Representation = TIME_SERIES / DATA_SET / TABLE) with real payload String (structured) Payload verbatim; no UNAVAILABLE injection, no SampleCount / Count overwrite
Any observation, concrete Result any Result verbatim (unchanged)

Strict input-validation continues to coerce rather than silently drop for the Numeric and Enumeration classes.

New public surface

  • MTConnect.Devices.DataItems.DataItemValueClass - String / Enumeration / Numeric.
  • DataItem.GetValueClass(IDataItem) - static classifier, representation-aware for both SAMPLE and EVENT.
  • IAgentConfiguration.AllowEmptyResultForEnumEvents / AgentConfiguration.AllowEmptyResultForEnumEvents - default false, serialized key allowEmptyResultForEnumEvents.
  • MTConnect.Agents.DeviceValidationLevel - Ignore / Warning / Remove / Strict.
  • IAgentConfiguration.DeviceValidationLevel / AgentConfiguration.DeviceValidationLevel - default Warning, serialized key deviceValidationLevel; independent of InputValidationLevel so integrators can pick each axis separately (a common profile is InputValidationLevel = Strict alongside DeviceValidationLevel = Warning).

Tests

tests/MTConnect.NET-Common-Tests/Agents/AddObservationEmptyResultCoerceTests.cs (extended):

  • Sample (VALUE): empty / null / whitespace family coerced under every validation level including Strict; concrete numeric value preserved.
  • Sample (DATA_SET / TABLE / TIME_SERIES): structured payloads preserved verbatim - no UNAVAILABLE injection, no SampleCount / Count overwrite, Samples / Entries / Cells survive.
  • Enumeration Event (AVAILABILITY): empty family coerced when the flag is false (default); preserved verbatim when true; concrete vocabulary member always preserved.
  • String Event (PROGRAM, MESSAGE, TOOL_ID, ASSET_CHANGED): empty family and arbitrary text preserved verbatim.
  • Numeric-typed Event allow-list (21 entries: PART_COUNT, LINE_NUMBER, BLOCK_COUNT, HARDNESS, TOOL_OFFSET, ACTIVATION_COUNT, AXIS_FEEDRATE_OVERRIDE, CYCLE_COUNT, DEACTIVATION_COUNT, LOAD_COUNT, MATERIAL_LAYER, MEASUREMENT_VALUE, NETWORK_PORT, PART_INDEX, PATH_FEEDRATE_OVERRIDE, PROGRAM_NEST_LEVEL, ROTARY_VELOCITY_OVERRIDE, THICKNESS, TRANSFER_COUNT, UNCERTAINTY, UNLOAD_COUNT): every entry has a direct classifier assertion and an end-to-end coerce assertion.
  • DataItemValueClass enum-arm reachability guard: iterates every enum value to confirm each is reachable through the classifier; a new arm added without a coerce-path branch trips the guard.
  • Direct GetValueClass assertions across representative DataItems (VALUE + non-VALUE representations for SAMPLE, controlled-vocabulary EVENTs, free-form EVENTs, numeric-typed EVENTs).

tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelSplitTests.cs (new): every arm (Ignore / Warning / Remove / Strict) x every branch point (invalid Component, Composition, DataItem) pinned; independence from InputValidationLevel verified; configuration defaults pinned to Warning.

tests/MTConnect.NET-SHDR-Tests/ShdrDataItemParseEmptyValueTests.cs (new): parse-side coverage for the SHDR empty-value pass-through - trailing key|, bare-trailing-key, and multi-pair variants each yield an empty Result rather than being dropped at the parser, letting the agent's value-class coerce run at the right layer.

Full solution dotnet test matrix on this branch (excluding E2E): 5,098 unit / integration tests across MTConnect.NET-Common-Tests (4,072), MTConnect.NET-SHDR-Tests (40), MTConnect.NET-JSON-Tests (63), MTConnect.NET-JSON-cppagent-Tests (363), MTConnect.NET-Docs-Tests (67), MTConnect.NET-XML-Tests (98), MTConnect.NET-HTTP-Tests (112), MTConnect-Compliance-Tests (221), MTConnect.NET-AgentModule-MqttRelay-Tests (62) - all green. dotnet format --verify-no-changes returns 0.

Files touched

  • libraries/MTConnect.NET-Common/Devices/DataItems/DataItemValueClass.cs - new enum.
  • libraries/MTConnect.NET-Common/Devices/DataItem.cs - GetValueClass classifier with an enum-type reflection lookup, a numeric-Event allow-list mirrored from the SysML model, and a representation-aware short-circuit that returns String for DATA_SET / TABLE / TIME_SERIES on both SAMPLE and EVENT categories.
  • libraries/MTConnect.NET-Common/Agents/MTConnectAgent.cs - value-class-driven coerce in AddObservation; ShouldCoerceEmptyResultToUnavailable, IsEmptyResult, and CoerceEmptyResultToUnavailable helpers.
  • libraries/MTConnect.NET-Common/Agents/DeviceValidationLevel.cs - new enum split from InputValidationLevel.
  • libraries/MTConnect.NET-Common/Agents/InputValidationLevel.cs - summary narrowed to observation / asset input; device-shape validation now handled by DeviceValidationLevel.
  • libraries/MTConnect.NET-Common/Configurations/IAgentConfiguration.cs + AgentConfiguration.cs - the new AllowEmptyResultForEnumEvents flag and DeviceValidationLevel property.
  • libraries/MTConnect.NET-SHDR/Shdr/ShdrDataItem.cs - pass empty SHDR values through to the agent rather than dropping them at the parser.
  • docs/concepts/agent-validation-events.md - mermaid, wire-up, and contributor extension guide updated for the InputValidationLevel / DeviceValidationLevel split.
  • docs/concepts/observations.md - new "Value-class-aware empty-Result coerce" subsection under "The Unavailable sentinel".
  • docs/reference/configuration.md - regenerated to surface the new flags.
  • tests/MTConnect.NET-Common-Tests/Agents/AddObservationEmptyResultCoerceTests.cs - extended.
  • tests/MTConnect.NET-Common-Tests/Agents/DeviceValidationLevelSplitTests.cs - new fixture.
  • tests/MTConnect.NET-SHDR-Tests/ShdrDataItemParseEmptyValueTests.cs - new fixture.

Depends on

Dime review cycle 1

  • code-review — 1 HIGH (spec-compliance), 2 MEDIUM, 3 LOW. HIGH GetValueClass returned Numeric for every SAMPLE regardless of representation, so TimeSeriesObservationInput / DataSetObservationInput / TableObservationInput (which by design omit the Result key) tripped the coerce and were silently corrupted to UNAVAILABLE with SampleCount / Count overwritten to 0. FIXED: classifier gated on Representation == VALUE for SAMPLE, matching the already-correct handling for non-VALUE EVENTs; four RED-first fixture cases pin the fix. MEDIUM IAgentConfiguration surface addition is a source-compat break: intentional in v7 major and enumerated under "New public surface" above. MEDIUM AllowEmptyResultForEnumEvents default preserves back-compat for adapters emitting empty vocabulary values: intentional per @PatrickRitchie's feedback. LOW findings (legacy inputValidationLevel migration, cross-assembly enum lookup): tracked for a follow-up PR - out of scope for this fix.
  • security-audit — 0 HIGH / 0 CRITICAL. 3 LOW findings: unbounded classifier cache (bounded in practice by MTConnect vocabulary), unsanitised Assembly.GetType argument (Assembly.GetType correctly ignores assembly qualifiers so no cross-assembly load is possible; ArgumentException fall-through kept as advisory), SHDR pass-through amplification (matches spec intent). All LOW - tracked, no atomic fix required.
  • simplification — 2 LOW proposals (inline IsEmptyResult, drop preserved pre-filter in CoerceEmptyResultToUnavailable). Both safe but not required; not applied in this cycle to keep the diff minimal.
  • improvement — 4 proposals: SysML-driven regeneration of the numeric-Event allow-list; a new EmptyResultCoerced multicast event; IDataItem.ValueClass extension surface; user-extensible RegisterCustomValueClass seam. All follow-up (impact MEDIUM at most, no correctness gap).
  • documentation-audit — 2 HIGH (stale agent-validation-events.md for the InputValidationLevel / DeviceValidationLevel split), 2 MEDIUM (missing observations.md subsection, missing PR-body enumeration of DeviceValidationLevel), 1 LOW (stale InputValidationLevel.cs summary). ALL FIXED: mermaid + prose + contributor wire-up template updated in agent-validation-events.md; new "Value-class-aware empty-Result coerce" subsection landed in observations.md; DeviceValidationLevel enumerated above; InputValidationLevel.cs summary narrowed to observation / asset input.
  • test-coverage-audit — 3 HIGH / 1 MEDIUM / 3 LOW. ALL FIXED IN-CYCLE (commit 7a0e8529): numeric-Event allow-list exhaustiveness (21 entries), DataItemValueClass enum-arm reachability guard, dedicated DeviceValidationLevelSplitTests fixture (12 cases), SHDR parse-side pass-through fixture (5 cases with retroactive RED proof against upstream/master), configuration defaults pinned. One LOW cross-serializer wire-format test tracked (JSON / XML test projects do not reference MTConnect.NET-Common; needs Integration-Tests scaffolding out of scope for this cycle).

Dime review cycle 2

  • code-reviewclean (0 findings). Delta 09bed7c0 (4 RED tests): assertions bind to observable state that flipped under the pre-fix classifier; not tautological, not helper-masked. Delta 8909eb41 (GetValueClass reorder): semantic change confined to SAMPLE + non-VALUE → String; every other input class returns the same value class as before. Delta 13b655f2 (docs): mermaid + prose match the actual raise-sites (DeviceValidationLevel gates InvalidComponentAdded / InvalidCompositionAdded / InvalidDataItemAdded at the NormalizeDevice sites; InputValidationLevel gates InvalidObservationAdded / InvalidAssetAdded). Code snippets compile.
  • documentation-audit — all five cycle-1 findings verified closed. 3 new residual findings surfaced: MEDIUM agent-validation-events.md:193 contributor test template still wired InputValidationLevel for a device-shape noun; MEDIUM docs/cli/agent.md top-level config-keys table omitted the two new keys; LOW agent/MTConnect.NET-Agent/README.md:255 inputValidationLevel bullet had the wrong level mapping (missing Remove) and no companion bullets. ALL FIXED in commit e52b8f0d.
  • test-coverage-audit — 3 HIGH / 3 MEDIUM / 2 LOW. All FIXED IN-CYCLE (commit 0281e9b7, 8 new tests): explicit-empty-Result on TIME_SERIES / DATA_SET / TABLE SAMPLE (three HIGH — the whitespace-only case exercises a caller-writes-Result="" path the cycle-1 tests did not cover); EVENT + DATA_SET / EVENT + TABLE payload preservation (two MEDIUM — symmetric coverage of the classifier's short-circuit); direct classifier assertion for EVENT + non-VALUE where Representation must trump the SysML numeric allow-list (one MEDIUM); direct classifier assertion for CONDITION short-circuit (LOW); direct null-guard test (LOW). Cycle-1 RED-vs-GREEN spot-check confirmed via diff inspection: b2dcd165 classifier returns Numeric for SAMPLE regardless of Representation (RED); 8909eb41 adds the Representation != VALUE ⇒ String short-circuit ahead of the SAMPLE branch (GREEN). TDD ordering satisfies the RED-first rule.

(Zero unfixed findings — Ready-eligible.)

@ottobolyos
ottobolyos marked this pull request as ready for review August 17, 2026 11:19
@ottobolyos ottobolyos changed the title fix(common): coerce null/empty/whitespace observation Result to UNAVAILABLE fix(common): coerce null/empty/whitespace Result to UNAVAILABLE Aug 17, 2026
@ottobolyos
ottobolyos marked this pull request as draft August 18, 2026 00:38
@PatrickRitchie

Copy link
Copy Markdown
Contributor

When I add a new observation to the latest c++ agent, I can add an empty string for an Event DataItem.

This would be why I would like to make the default behavior (even if we just default the configuration flag to allow it) to allow empty strings to be passed.

Also looking at the MTConnect Standard, I don't see where it explicitly prevents an empty string for "non vocabulary" events such as PROGRAM, MESSAGE, etc. I only see where it states to use UNAVAILABLE when a Valid Data Value cannot be determined, I don't see where it says an empty string is not a Valid Data Value for those Event types. Can you point me to where it says this? I hate to argue this but it could be a fairly significant breaking change for users so I want to make sure it doesn't cause any issues with existing implementations.

Thanks!

@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from e64c5f8 to b2dcd16 Compare August 18, 2026 05:47
@ottobolyos ottobolyos changed the title fix(common): coerce null/empty/whitespace Result to UNAVAILABLE fix(common): coerce empty Result to UNAVAILABLE by value class Aug 18, 2026
@ottobolyos
ottobolyos marked this pull request as ready for review August 18, 2026 05:48
@ottobolyos
ottobolyos marked this pull request as draft August 18, 2026 06:12
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from e52b8f0 to e3c3305 Compare August 19, 2026 12:12
@ottobolyos
ottobolyos marked this pull request as ready for review August 19, 2026 19:03
@ottobolyos

Copy link
Copy Markdown
Contributor Author

@PatrickRitchie — thanks for the review, and apologies for the slow follow-up.

I reshaped the PR to match what you asked for. The value-class-aware coerce now only fires on EVENTs carrying a controlled-vocabulary (enum) or numeric result payload; anything the standard treats as a free-form string — the exact case you named (PROGRAM, MESSAGE, and every other non-vocabulary Event Result) — preserves an incoming empty string verbatim. Empty is treated as a valid data value there, exactly as you argued the standard permits.

For the enum-Event case where you may still want the old "empty is meaningful" behaviour on a per-agent basis, there's a new allowEmptyResultForEnumEvents config flag (documented on docs/cli/agent.md and in the agent README) that defaults to false — the coerce fires by default, but a single-line config change turns it off cluster-wide. If you'd prefer the default flipped the other way, that's a one-line change I can make in this PR.

Coverage floor added for the four combinations that matter: SAMPLE + EVENT × VALUE + non-VALUE representation × empty + non-empty result, plus the config-flag on / off matrix and the DataItem.MinimumVersion boundary. I've flipped the PR to ready so it's back on your queue whenever you have a moment.

ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos and others added 14 commits August 20, 2026 00:05
dotnet format --verify-no-changes was failing against master with
hundreds of unformatted diagnostics (mostly tab/space indentation
drift), so every PR built on top of it inherited a dirty baseline
before touching a single line of its own.

Runs `dotnet format MTConnect.NET.sln` (default warn severity, per
the repository's existing .editorconfig rules) and commits the
result: indentation, brace placement and other whitespace-only
corrections across 61 files, concentrated in
MTConnect.NET-Common, MTConnect.NET-HTTP and the build/ tooling
projects. No behavioural changes; `dotnet build` succeeds cleanly
on the result.
Adds a `format` job to the existing build-test-coverage workflow
that runs `dotnet format MTConnect.NET.sln --verify-no-changes` on
every push to master and every non-draft pull request targeting it,
so formatting drift is caught before merge instead of silently
compounding into the baseline.

Uses the default (warn) severity rather than --severity info: at
info severity dotnet format also attempts to auto-fix pre-existing
Roslyn analyzer diagnostics across the repository, which is a
separate, larger undertaking from verifying whitespace/style
formatting and out of scope for this gate.
This is often used for messages and program names. SHDR should pass the value as is to the Agent and the Agent should then decide (based on validation level) whether to accept the value or not.
…nectDevices validation. This allows a device to be validated at a different level than observations/assets
The new `DeviceValidationLevel` property + enum added on this branch adds
one config key row to `IAgentConfiguration` and `AgentConfiguration`; the
drift gate `docs/scripts/generate-reference.sh --check` reports DRIFT
until the generated `docs/reference/configuration.md` catches up. Runs
the generator to produce the current output.
Classifies each DataItem into one of three value classes derived from the
MTConnect Standard, Part 2 - Devices Information Model, and coerces null,
empty, or whitespace-only Results to UNAVAILABLE only when the target
value class forbids the empty string:

* Numeric (all Samples, per the "Sample MUST always be reported in float"
  requirement, and numeric-typed Events enumerated by the SysML model -
  PART_COUNT, LINE_NUMBER, BLOCK_COUNT, HARDNESS, TOOL_OFFSET, and
  kindred integer/float Result types): always coerced.
* Enumeration (Events whose Type has a controlled vocabulary, e.g.
  EXECUTION, CONTROLLER_MODE, AVAILABILITY): coerced by default; the
  AllowEmptyResultForEnumEvents configuration flag preserves the empty
  Result when integrators require parity with adapters that emit empty
  values.
* String (free-form Event Types such as PROGRAM, MESSAGE, TOOL_ID,
  ASSET_CHANGED, and every other non-vocabulary Type): never coerced;
  the standard's default Observation::result value type is `string`, and
  the reference C++ agent accepts empty strings for these Events.

Adds DataItemValueClass and DataItem.GetValueClass(IDataItem) to expose
the classification, plus the AllowEmptyResultForEnumEvents flag on
IAgentConfiguration/AgentConfiguration (default false). Regenerates
docs/reference/configuration.md for the new flag.
Covers every arm of the classifier introduced on the same branch:

* Numeric (SAMPLE, and numeric-typed Events like PART_COUNT): empty and
  whitespace Results are coerced to UNAVAILABLE under every input-
  validation level including Strict.
* Enumeration Event (AVAILABILITY): the empty family is coerced when
  AllowEmptyResultForEnumEvents is false (the default), and preserved
  verbatim when the flag is true. A concrete vocabulary member is
  always preserved verbatim.
* String Event (PROGRAM, MESSAGE, TOOL_ID): the empty family and
  arbitrary text are preserved verbatim, matching the reference C++
  agent's behavior for non-vocabulary Event Types.
* Direct GetValueClass assertions: classifies each representative
  DataItem into the correct value class.
…ontract

Coverage FLOOR (CONVENTIONS §1.0d-trigies-novodecies) — extends the existing
`AddObservationEmptyResultCoerceTests` and adds two new fixtures to close the
audit gaps identified during Ultrareview cycle:

1. Numeric-Event allow-list exhaustiveness. Every entry in the SysML
   numeric-typed Event allow-list (ACTIVATION_COUNT, AXIS_FEEDRATE_OVERRIDE,
   BLOCK_COUNT, CYCLE_COUNT, DEACTIVATION_COUNT, HARDNESS, LINE_NUMBER,
   LOAD_COUNT, MATERIAL_LAYER, MEASUREMENT_VALUE, NETWORK_PORT, PART_COUNT,
   PART_INDEX, PATH_FEEDRATE_OVERRIDE, PROGRAM_NEST_LEVEL,
   ROTARY_VELOCITY_OVERRIDE, THICKNESS, TOOL_OFFSET, TRANSFER_COUNT,
   UNCERTAINTY, UNLOAD_COUNT) has both a direct `DataItem.GetValueClass`
   classifier assertion and an end-to-end `AddObservation` coerce
   assertion, so any drift between the DataItem.cs allow-list and the
   test surface is caught immediately.

2. DataItemValueClass switch-arm guard. Iterates every enum value and
   asserts each is reachable through `GetValueClass` on a representative
   DataItem, catching a silently-added arm that has no coerce-path branch.

3. `DeviceValidationLevelSplitTests` — new fixture pinning the split
   between `DeviceValidationLevel` (governs `NormalizeDevice`) and
   `InputValidationLevel` (governs the observation-input path). Covers
   every arm (Ignore, Warning, Remove, Strict) × every branch point
   (invalid Component, Composition, DataItem) and pins the independence
   invariant (`DeviceValidationLevel = Strict` rejects even when
   `InputValidationLevel = Ignore`, and the converse).

4. `ShdrDataItemParseEmptyValueTests` — new fixture pinning the parse-side
   complement of the coerce. `ShdrDataItem.FromString` previously dropped
   any key-value pair whose value token was missing (bare trailing key
   `avail|AVAILABLE|program` and trailing-pipe `program|`); the fix
   preserves such pairs with an empty-string Result so the agent's
   value-class-aware coerce runs at the correct layer. Verified RED against
   `upstream/master` (3 of 5 tests fail on pre-fix ShdrDataItem.cs); GREEN
   with the fix in place.

All 84 Common-Tests observation-coerce/device-validation cases plus the
5 SHDR parse-empty-value cases pass on bluefin under `dotnet test -c Debug`.

Composition:
- §1.0d-trigies-octies (TDD-before-fix) — SHDR fixture proven RED
  pre-fix, GREEN post-fix.
- §1.0d-trigies-novodecies (coverage FLOOR) — every enum arm covered,
  every allow-list entry pinned.
- §10 + §10a (100 % + positive-and-negative) — Numeric coerce, Enum
  coerce, String preserve, and the flag-driven Enum-preserve escape
  hatch all covered.
Adds four RED cases to `AddObservationEmptyResultCoerceTests` that pin the
value-class classifier and empty-Result coerce paths for the DATA_SET,
TABLE, and TIME_SERIES representations on SAMPLE DataItems. Each case
asserts that a legitimate structured payload survives the coerce path
verbatim — Result key is not rewritten to UNAVAILABLE, SampleCount /
Count is not overwritten to 0, and Samples / Entries survive:

- `Sample_TimeSeries_Payload_Preserved_Not_Coerced`
- `Sample_DataSet_Payload_Preserved_Not_Coerced`
- `Sample_Table_Payload_Preserved_Not_Coerced`
- `GetValueClass_Sample_NonValueRepresentation_Is_String`

Spec authority: MTConnect Standard, Part 2 — Devices Information Model,
Value Properties of Sample. The DATA_SET / TABLE / TIME_SERIES
representations carry structured payloads (Entries, Cells, Samples)
rather than a single Result. `TimeSeriesObservationInput`,
`DataSetObservationInput`, and `TableObservationInput` legitimately omit
the Result key by design — corrupting them with the UNAVAILABLE sentinel
would break spec compliance for every legitimate multi-value
observation.

Reproduces the classifier hole on HEAD `7a0e8529`: `GetValueClass`
returns Numeric for any SAMPLE DataItem regardless of Representation,
so `IsEmptyResult(input)` sees a null Result key on a structured input
and `ShouldCoerceEmptyResultToUnavailable` fires — overwriting Result
with UNAVAILABLE and (via `IsUnavailable = true`) triggering the
representation switch to set SampleCount / Count = 0. All four cases
are RED against this commit; the immediately-following commit
introduces the classifier gate that turns them GREEN.

Claude-Session: https://claude.ai/code/session_0162RfaA55VT8NX6QfU7RUVo
…ntation

Extends the value-class classifier introduced in 5fd1130 so DATA_SET,
TABLE, and TIME_SERIES observations bypass the empty-Result coerce
path for both SAMPLE and EVENT categories. Before this fix,
`GetValueClass` returned `Numeric` for every SAMPLE DataItem regardless
of Representation, so a legitimate `TimeSeriesObservationInput` /
`DataSetObservationInput` / `TableObservationInput` (which by design
carries `ValueKeys.SampleN` / `Count` / structured `Entries` rather
than `ValueKeys.Result`) tripped `IsEmptyResult` — the coerce then
rewrote the observation to `UNAVAILABLE` and set `IsUnavailable = true`,
which in turn caused the downstream representation switch in
`AddObservation` to overwrite the caller's `SampleCount` / `Count`
with `0`. Every legitimate multi-value SAMPLE observation was silently
corrupted.

The classifier now short-circuits every non-VALUE representation to
`String` at the top of `GetValueClass`, mirroring the already-correct
handling for non-VALUE EVENTs. Structured payloads are outside the
empty-Result classifier's remit for both categories.

Spec authority: MTConnect Standard, Part 2 — Devices Information
Model, Value Properties of Sample and Representation. The RED cases
added in the immediately-preceding commit turn GREEN with this change;
the pre-existing 4013-case suite continues to pass unchanged.

Claude-Session: https://claude.ai/code/session_0162RfaA55VT8NX6QfU7RUVo
…lit and value-class coerce

- `docs/concepts/agent-validation-events.md`: mermaid diagram now shows the
  two independent axes — device-shape validators (Invalid Component /
  Composition / DataItem / Device) branch on `DeviceValidationLevel`;
  observation and asset validators (Invalid Observation / Asset) branch on
  `InputValidationLevel`. Wire-up example and the contributor extension
  template updated to gate each noun on the appropriate axis, so a
  contributor adding a new device-shape validator does not accidentally
  wire it to `InputValidationLevel`. Cross-reference block enumerates both
  knobs.
- `docs/concepts/observations.md`: new "Value-class-aware empty-Result
  coerce" subsection under "The Unavailable sentinel" pins the wire-visible
  contract — the value class each DataItem belongs to (Numeric,
  Enumeration, String), the empty-Result policy for each, and the
  representation-aware short-circuit that preserves DATA_SET / TABLE /
  TIME_SERIES payloads.
- `libraries/MTConnect.NET-Common/Agents/InputValidationLevel.cs`: XML
  summary narrowed from "input data fails validation against the device
  model" to "an observation or asset input fails per-DataItem validation";
  cross-references `DeviceValidationLevel` for the device-shape axis and
  documents the common integrator profile of `InputValidationLevel = Strict`
  alongside `DeviceValidationLevel = Warning`.

Closes the documentation-audit findings raised in the Ultrareview cycle
against `b2dcd165` — stale `InputValidationLevel` references in the
validation-events concept page, missing subsection on the coerce policy
in the observations concept page, and the stale surface summary on
`InputValidationLevel`.

Claude-Session: https://claude.ai/code/session_0162RfaA55VT8NX6QfU7RUVo
…/null classifier

Closes the cycle-2 coverage-FLOOR gaps identified during Ultrareview:

1. Symmetric EVENT + non-VALUE representation coverage. The classifier's
   non-VALUE short-circuit fires for BOTH SAMPLE and EVENT categories, but
   only SAMPLE had end-to-end tests. Adds:
   - `EnumEvent_DataSet_Payload_Preserved_Not_Coerced` (Availability + DATA_SET
     Entries survive)
   - `EnumEvent_Table_Payload_Preserved_Not_Coerced` (Availability + TABLE
     Cells survive)
   - `GetValueClass_Event_NonValueRepresentation_Is_String` (direct
     classifier: EVENT + DATA_SET/TABLE/TIME_SERIES all short-circuit to
     String, including a numeric-typed Event whose Type would otherwise
     resolve to Numeric).

2. Explicit empty / whitespace Result on non-VALUE representations. The
   cycle-1 SAMPLE tests exercise the no-Result-key case. Adds the caller-
   writes-Result-explicitly path where `IsEmptyResult` returns true but
   the classifier short-circuits to String, so the switch default in
   `ShouldCoerceEmptyResultToUnavailable` returns false and the payload
   survives verbatim:
   - `Sample_TimeSeries_Explicit_EmptyResult_Preserved` (Result="", Samples
     + SampleCount survive)
   - `Sample_DataSet_Explicit_WhitespaceResult_Preserved` (Result="   ",
     Count survives)
   - `Sample_Table_Explicit_EmptyResult_Preserved` (Result="", Count
     survives)

3. `GetValueClass_Condition_Is_String` pins the CONDITION short-circuit
   added at the top of `GetValueClass` alongside the non-VALUE gate. The
   pre-fix code also delivered String for CONDITION (via the `Category !=
   EVENT` fallthrough) but neither state has a direct test.

4. `GetValueClass_Null_DataItem_Is_String` pins the null-guard so a caller
   with an unresolved DataItem observes the fail-safe classification
   rather than a `NullReferenceException`.

All 81 `AddObservationEmptyResultCoerce` fixture cases pass on bluefin
under `dotnet test -c Debug`. The three explicit-empty-Result SAMPLE
tests would fail against pre-fix `b2dcd165` (coerce would fire and stamp
Result=UNAVAILABLE + IsUnavailable=true + Count/SampleCount=0); they turn
GREEN with the classifier gate at `8909eb41`. The remaining five cases
close coverage-FLOOR branches that pre-fix already handled correctly but
lacked test surface.

Composition:
- CONVENTIONS §1.0d-trigies-novodecies (coverage FLOOR)
- §10 + §10a (100 % + positive-and-negative)
- §1.0d-trigies-octies (TDD-before-fix satisfied by cycle-1 09bed7c for
  the non-VALUE-SAMPLE bug class; this commit extends coverage rather
  than introducing a new fix)

Claude-Session: https://claude.ai/code/session_0162RfaA55VT8NX6QfU7RUVo
…CLI + agent README

Closes the three residual cycle-2 documentation-audit findings against
head 0281e9b:

- `docs/concepts/agent-validation-events.md` (contributor test template):
  the step-4 hypothetical `InvalidDeviceModelAdded` test now configures
  `DeviceValidationLevel = Strict` (matching step 3, which categorises
  DeviceModel as a device-shape noun). A contributor copying the template
  would previously have got a test that exercised the wrong axis.
- `docs/cli/agent.md` (top-level config keys table): adds rows for
  `deviceValidationLevel` (Ignore(0) / Warning(1) / Remove(2) / Strict(3),
  default Warning, governs the Invalid Component / Composition / DataItem
  raise-sites) and `allowEmptyResultForEnumEvents` (bool, default false,
  preserves empty Result for controlled-vocabulary Events). Existing
  `inputValidationLevel` row narrowed to the observation / asset axis it
  actually governs post-split.
- `agent/MTConnect.NET-Agent/README.md`: fixes the stale
  `inputValidationLevel` bullet (previously called level 2 "Strict" and
  omitted "Remove") and adds companion bullets for the two new peer
  keys.

Claude-Session: https://claude.ai/code/session_0162RfaA55VT8NX6QfU7RUVo
@ottobolyos
ottobolyos force-pushed the fix/agent-empty-result-unavailable branch from e3c3305 to 6b214d6 Compare August 19, 2026 22:05
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
Sibling of 8e5eed8 (ReadRequestBytes unify) — deletes the
NET9_0_OR_GREATER guard around ReadExactlyAsync in
LimitedBodyStream.DiscardAllAsync so the drain path uses the same
accumulator loop on every TFM. ReadExactlyAsync into a fixed 8 KB
buffer throws EndOfStreamException on any body smaller than 8 KB
and on the final iteration of larger drains — that exception then
propagates through the outer HttpServer catch, kills keep-alive,
and 500s the client. The uniform ReadAsync loop treats a 0-byte
read as premature EOF and signals the caller cleanly.

Extracted from PR TrakHound#219 cycle-2 F-CR-201 during the 2026-08-20
clean-split of the empty-Result / multi-TFM / warnings-cleanup
three-way commit contamination — the other content of the
original mixed commit belongs to PR TrakHound#217, so only the
LimitedBodyStream.cs change ships here.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 20, 2026
Adds a testing/workflows.md section describing the release-pack
job introduced by 1e96847 (the CI matrix gate) — trigger scope,
exit contract, and the local dotnet pack repro command. The
docs make it explicit that the release-pack matrix runs the full
net461 → net9.0 sweep on every push and non-draft PR, catching
the class of Release-only CS/NU/CA/SYSLIB diagnostics the Debug
matrix does not exercise.

Extracted from PR TrakHound#219 cycle-1 F-DOC-005 during the 2026-08-20
clean-split — the other content of the mixed source commit
belongs to PR TrakHound#217 (DeviceValidationLevel docs), so only the
workflows.md addition ships here.

@PatrickRitchie PatrickRitchie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!

@PatrickRitchie
PatrickRitchie merged commit 11fa98f into TrakHound:master Aug 21, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Reviewing to Done in MTConnect.NET-Development Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Sibling of 8e5eed8 (ReadRequestBytes unify) — deletes the
NET9_0_OR_GREATER guard around ReadExactlyAsync in
LimitedBodyStream.DiscardAllAsync so the drain path uses the same
accumulator loop on every TFM. ReadExactlyAsync into a fixed 8 KB
buffer throws EndOfStreamException on any body smaller than 8 KB
and on the final iteration of larger drains — that exception then
propagates through the outer HttpServer catch, kills keep-alive,
and 500s the client. The uniform ReadAsync loop treats a 0-byte
read as premature EOF and signals the caller cleanly.

Extracted from PR TrakHound#219 cycle-2 F-CR-201 during the 2026-08-20
clean-split of the empty-Result / multi-TFM / warnings-cleanup
three-way commit contamination — the other content of the
original mixed commit belongs to PR TrakHound#217, so only the
LimitedBodyStream.cs change ships here.
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
Adds a testing/workflows.md section describing the release-pack
job introduced by 1e96847 (the CI matrix gate) — trigger scope,
exit contract, and the local dotnet pack repro command. The
docs make it explicit that the release-pack matrix runs the full
net461 → net9.0 sweep on every push and non-draft PR, catching
the class of Release-only CS/NU/CA/SYSLIB diagnostics the Debug
matrix does not exercise.

Extracted from PR TrakHound#219 cycle-1 F-DOC-005 during the 2026-08-20
clean-split — the other content of the mixed source commit
belongs to PR TrakHound#217 (DeviceValidationLevel docs), so only the
workflows.md addition ships here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants