fix(ablegacy): encode array writes at the data file's element width

Review r0 I3. The array encoder used the AUTHORED dataType's width while both runtimes size their buffer from the FILE: an Int array on F8 wrote four shorts into a 16-byte float buffer and sent it all (garbage neighbours, answered Good); Long/Float on N overran it. - The write path resolves the element width from the data file the address names (N/B/A/I/O/S 16-bit word, L 32-bit, F float) and encodes at THAT width. - An authored type that does not match its file is refused BadTypeMismatch before any runtime exists (warned once, no health write). - The same rule (AbLegacyTagDefinitionFactory.ArrayFileMismatch) backs an Inspect deploy warning and the AdminUI model's Validate error; FromTagConfig still accepts the tag so persisted configs keep loading. - M6: the skipped F1 parity test gains the shapes the write path refuses (/bit, .sub, indirect, type/file mismatch, PLC-5 native). Tests: one case per file type, a mismatch per pairing with nothing created, and a positive control. Refs #738 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 15:20:10 -04:00
parent 17ae2eea0b
commit d758dd1e16
6 changed files with 175 additions and 5 deletions
@@ -177,6 +177,9 @@ public static class AbLegacyTagDefinitionFactory
        {
            if (AbLegacyAddress.Validate(accepted.Address, family) is { } addressError)
                warnings.Add($"AbLegacy tag address: {addressError} The driver reads this tag as BadNodeIdUnknown.");
+            else if (accepted.ArrayLength is not null
+                     && ArrayFileMismatch(accepted.DataType, accepted.Address) is { } widthError)
+                warnings.Add(widthError);
            return warnings;
        }

@@ -252,6 +255,54 @@ public static class AbLegacyTagDefinitionFactory
        $"AbLegacy {type} is not supported as an array element (String, Timer, Counter and Control elements are "
        + "structured); author a scalar tag per element instead. The driver skips this tag and it reads BadNodeIdUnknown.";

+    /// <summary>
+    ///     The element width an array on <paramref name="address"/>'s data file has, as the data type that decodes /
+    ///     encodes it: N, B, A and the I/O/S word files are 16-bit words (<see cref="AbLegacyDataType.Int"/>), L is
+    ///     32-bit (<see cref="AbLegacyDataType.Long"/>), F is IEEE float (<see cref="AbLegacyDataType.Float"/>).
+    ///     <see langword="null"/> for a file with no flat array layout (ST, T, C, R, function files) or an address that
+    ///     does not parse.
+    /// </summary>
+    /// <param name="address">The authored address.</param>
+    /// <returns>The file's element type, or <see langword="null"/>.</returns>
+    public static AbLegacyDataType? ArrayFileElementType(string address) =>
+        AbLegacyAddress.TryParse(address) is { } a ? ArrayFileElementType(a) : null;
+
+    /// <inheritdoc cref="ArrayFileElementType(string)"/>
+    public static AbLegacyDataType? ArrayFileElementType(AbLegacyAddress a) => a.FileLetter switch
+    {
+        "N" or "B" or "A" or "I" or "O" or "S" => AbLegacyDataType.Int,
+        "L" => AbLegacyDataType.Long,
+        "F" => AbLegacyDataType.Float,
+        _ => null,
+    };
+
+    /// <summary>
+    ///     Why an ARRAY authored as <paramref name="type"/> on <paramref name="address"/> does not match its data
+    ///     file's element width (review 07 r0 I3), or <see langword="null"/> when it does. Int / AnalogInt need a 16-bit
+    ///     word file (N/B/A/I/O/S), Long an L file, Float an F file; Bit arrays are bit-packed words and fit any 16-bit
+    ///     word file. A mismatch reads the file at the wrong width (garbage) and the driver refuses to WRITE it
+    ///     (<c>BadTypeMismatch</c>), because encoding at the authored width would corrupt the neighbouring elements.
+    /// </summary>
+    /// <param name="type">The authored element data type.</param>
+    /// <param name="address">The authored address.</param>
+    /// <returns>The mismatch message, or <see langword="null"/>.</returns>
+    public static string? ArrayFileMismatch(AbLegacyDataType type, string address)
+    {
+        if (!IsArrayElementTypeSupported(type)) return null; // structured arrays have their own refusal
+        if (AbLegacyAddress.TryParse(address) is not { } a || a.IsIndirect) return null;
+        var file = ArrayFileElementType(a);
+        var ok = type switch
+        {
+            AbLegacyDataType.Int or AbLegacyDataType.AnalogInt or AbLegacyDataType.Bit => file == AbLegacyDataType.Int,
+            _ => file == type,
+        };
+        return ok
+            ? null
+            : $"AbLegacy {type} array on '{address}' does not match the {a.FileLetter} file's element width "
+              + "(Int/AnalogInt/Bit need an N/B/A word file, Long an L file, Float an F file): reads decode the wrong width "
+              + "and writes are refused BadTypeMismatch.";
+    }
+
    private static int ReadInt(JsonElement o, string name)
        => o.TryGetProperty(name, out var e) && e.ValueKind == JsonValueKind.Number
           && e.TryGetInt32(out var v) ? v : 0;
@@ -885,12 +885,29 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag

                // Array writes (review 07 U10): convert and length-check BEFORE any runtime exists, then encode every
                // element. A bit / sub-element array has no flat encoding.
+                // The encode width is the DATA FILE's element width (review 07 r0 I3), never the authored type's: an
+                // authored type that does not match its file is refused BadTypeMismatch before anything is encoded.
                Array? arrayValue = null;
+                AbLegacyDataType arrayFileType = default;
                if (IsArrayTag(def))
                {
-                    var conversion = parsed.BitIndex is not null || parsed.SubElement is not null
-                        ? AbLegacyStatusMapper.BadNotSupported
-                        : AbLegacyArrayWriteEncoder.TryConvert(def.DataType, w.Value, EffectiveArrayLength(def), out arrayValue);
+                    var fileType = AbLegacyTagDefinitionFactory.ArrayFileElementType(parsed);
+                    var widthRefusal = AbLegacyTagDefinitionFactory.ArrayFileMismatch(def.DataType, def.Address)
+                                       ?? (fileType is null ? $"{def.Address} has no flat array layout." : null);
+                    uint conversion;
+                    if (parsed.BitIndex is not null || parsed.SubElement is not null || def.DataType == AbLegacyDataType.Bit)
+                        conversion = AbLegacyStatusMapper.BadNotSupported;
+                    else if (widthRefusal is not null || fileType is null)
+                    {
+                        WarnRefusalOnce(device, def, "write", widthRefusal ?? "no flat array layout", AbLegacyStatusMapper.BadTypeMismatch);
+                        results[i] = new WriteResult(AbLegacyStatusMapper.BadTypeMismatch);
+                        continue;
+                    }
+                    else
+                    {
+                        arrayFileType = fileType.Value;
+                        conversion = AbLegacyArrayWriteEncoder.TryConvert(arrayFileType, w.Value, EffectiveArrayLength(def), out arrayValue);
+                    }
                    if (conversion != AbLegacyStatusMapper.Good)
                    {
                        if (conversion == AbLegacyStatusMapper.BadNotSupported)
@@ -925,7 +942,7 @@ public sealed partial class AbLegacyDriver : IDriver, IReadable, IWritable, ITag
                await opLock.WaitAsync(cancellationToken).ConfigureAwait(false);
                try
                {
-                    if (arrayValue is not null) runtime.EncodeArray(def.DataType, arrayValue);
+                    if (arrayValue is not null) runtime.EncodeArray(arrayFileType, arrayValue);
                    else runtime.EncodeValue(def.DataType, parsed.BitIndex, w.Value);
                    await runtime.WriteAsync(cancellationToken).ConfigureAwait(false);
                    status = runtime.GetStatus();
@@ -92,6 +92,11 @@ public sealed class AbLegacyTagConfigModel
        if (isArray && arrayLength >= 1 && !AbLegacyTagDefinitionFactory.IsArrayElementTypeSupported(DataType))
            return AbLegacyTagDefinitionFactory.ArrayElementTypeError(DataType);

+        // An array must match its data file's element width (review 07 r0 I3): the driver refuses to write a
+        // mismatched one, and reads it at the wrong width.
+        if (isArray && arrayLength >= 1 && AbLegacyTagDefinitionFactory.ArrayFileMismatch(DataType, Address.Trim()) is { } widthError)
+            return widthError;
+
        // Parse-gate backstop: the checks above mirror the reader's whole rejection surface today, so this is
        // expected to be silent for AbLegacy. It is here so a future reader rule cannot become a gap.
        return TagConfigParseGate.Describe(
@@ -43,7 +43,24 @@ public sealed class AbLegacyArrayWriteSupportParityTests
        DriverArrayWriteSupport.Supports(DriverTypeNames.AbLegacy, "Int16", json).ShouldBe(encoderAccepts, dataType.ToString());
    }

-    /// <summary>What the encoder accepts today — the row F1 must add (Int, AnalogInt, Long, Float).</summary>
+    /// <summary>
+    ///     The shapes the WRITE PATH refuses although their element type alone looks writable (review 07 r0 M6): a
+    ///     <c>/bit</c> or <c>.sub</c> address, an indirect address, a type that does not match its data file (I3), and
+    ///     a native PLC-5 device. The F1 row must answer false for every one — keyed on <c>dataType</c> alone it would
+    ///     advertise them writable. Skipped with the parity theory until F1 lands.
+    /// </summary>
+    [Theory(Skip = "F1: DriverArrayWriteSupport has no AbLegacy row yet (Core.Abstractions is outside this lane)")]
+    [InlineData("""{"address":"N7:0/3","dataType":"Int","isArray":true,"arrayLength":2}""", "{}")]
+    [InlineData("""{"address":"T4:0.ACC","dataType":"Int","isArray":true,"arrayLength":2}""", "{}")]
+    [InlineData("""{"address":"N7:[N7:0]","dataType":"Int","isArray":true,"arrayLength":2}""", "{}")]
+    [InlineData("""{"address":"F8:0","dataType":"Int","isArray":true,"arrayLength":2}""", "{}")]
+    [InlineData("""{"address":"N7:0","dataType":"Long","isArray":true,"arrayLength":2}""", "{}")]
+    [InlineData("""{"address":"N7:0","dataType":"Int","isArray":true,"arrayLength":2}""",
+        """{"transport":"Native","devices":[{"hostAddress":"ab://10.0.0.5/1,0","plcFamily":"Plc5"}]}""")]
+    public void Table_refuses_the_shapes_the_write_path_refuses(string tagConfig, string driverConfig) =>
+        DriverArrayWriteSupport.Supports(DriverTypeNames.AbLegacy, "Int16", tagConfig, driverConfig).ShouldBeFalse();
+
+    /// <summary>What the encoder accepts today — the row F1 must add (Int, AnalogInt, Long, Float on a matching file).</summary>
    [Theory]
    [InlineData(AbLegacyDataType.Int, true)]
    [InlineData(AbLegacyDataType.AnalogInt, true)]
@@ -72,6 +72,68 @@ public sealed class AbLegacyArrayWriteTests
        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
    }

+    /// <summary>Review 07 r0 I3: every data file type encodes at ITS element width (one case per file).</summary>
+    [Theory]
+    [InlineData("N7:0", AbLegacyDataType.Int, AbLegacyDataType.Int)]
+    [InlineData("N7:0", AbLegacyDataType.AnalogInt, AbLegacyDataType.Int)]
+    [InlineData("B3:0", AbLegacyDataType.Int, AbLegacyDataType.Int)]
+    [InlineData("A9:0", AbLegacyDataType.Int, AbLegacyDataType.Int)]
+    [InlineData("L9:0", AbLegacyDataType.Long, AbLegacyDataType.Long)]
+    [InlineData("F8:0", AbLegacyDataType.Float, AbLegacyDataType.Float)]
+    public async Task Each_file_type_encodes_at_the_file_element_width(string address, AbLegacyDataType authored, AbLegacyDataType fileWidth)
+    {
+        var (drv, f) = await NewAsync(authored, address, 2);
+
+        var r = await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, 2 })], CancellationToken.None);
+
+        r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.Good);
+        var tag = (RecordingTag)f.Tags[address];
+        tag.EncodedType.ShouldBe(fileWidth);
+        tag.Encoded!.GetType().ShouldBe(fileWidth switch
+        {
+            AbLegacyDataType.Long => typeof(int[]),
+            AbLegacyDataType.Float => typeof(float[]),
+            _ => typeof(short[]),
+        });
+        tag.WriteCount.ShouldBe(1);
+    }
+
+    /// <summary>
+    ///     Review 07 r0 I3: an array whose authored type does not match its data file (Int on F8, Long on N7, Float on
+    ///     L9) used to be encoded at the AUTHORED width into the file-typed buffer and sent — garbage in the
+    ///     neighbouring elements, answered Good. It is now refused BadTypeMismatch before any runtime exists.
+    /// </summary>
+    [Theory]
+    [InlineData("F8:0", AbLegacyDataType.Int)]
+    [InlineData("N7:0", AbLegacyDataType.Long)]
+    [InlineData("N7:0", AbLegacyDataType.Float)]
+    [InlineData("L9:0", AbLegacyDataType.Float)]
+    [InlineData("L9:0", AbLegacyDataType.Int)]
+    public async Task A_type_that_does_not_match_its_file_is_refused_and_never_written(string address, AbLegacyDataType authored)
+    {
+        var (drv, f) = await NewAsync(authored, address, 4);
+
+        var r = await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, 2, 3, 4 })], CancellationToken.None);
+
+        r[0].StatusCode.ShouldBe(AbLegacyStatusMapper.BadTypeMismatch);
+        f.Tags.ShouldBeEmpty(); // nothing created, nothing on the wire
+        drv.GetHealth().State.ShouldBe(DriverState.Healthy);
+        AbLegacyTagDefinitionFactory.Inspect(
+                $$"""{"address":"{{address}}","dataType":"{{authored}}","isArray":true,"arrayLength":4}""")
+            .ShouldContain(w => w.Contains("element width")); // and the deploy gate names it
+    }
+
+    [Fact]
+    public async Task Positive_control_a_Float_array_on_an_F_file_is_written()
+    {
+        var (drv, f) = await NewAsync(AbLegacyDataType.Float, "F8:0", 4);
+        (await drv.WriteAsync([new WriteRequest("Arr", new[] { 1, 2, 3, 4 })], CancellationToken.None))[0]
+            .StatusCode.ShouldBe(AbLegacyStatusMapper.Good);
+        ((RecordingTag)f.Tags["F8:0"]).Encoded.ShouldBe(new[] { 1f, 2f, 3f, 4f });
+        AbLegacyTagDefinitionFactory.Inspect("""{"address":"F8:0","dataType":"Float","isArray":true,"arrayLength":4}""")
+            .ShouldBeEmpty();
+    }
+
    [Fact]
    public async Task Float_and_Long_arrays_encode_at_their_element_width()
    {
@@ -136,6 +136,24 @@ public sealed class AbLegacyTagConfigModelTests
        AbLegacyTagConfigModel.FromJson($$"""{"address":"{{address}}","dataType":"{{dataType}}"}""").Validate().ShouldBeNull();
    }

+    /// <summary>Review 07 r0 I3: an array must match its data file's element width.</summary>
+    [Theory]
+    [InlineData("F8:0", "Int", false)]
+    [InlineData("N7:0", "Long", false)]
+    [InlineData("N7:0", "Int", true)]
+    [InlineData("L9:0", "Long", true)]
+    [InlineData("F8:0", "Float", true)]
+    public void Validate_refuses_an_array_whose_type_does_not_match_its_file(string address, string dataType, bool valid)
+    {
+        var m = AbLegacyTagConfigModel.FromJson(
+            $$"""{"address":"{{address}}","dataType":"{{dataType}}","isArray":true,"arrayLength":4}""");
+
+        if (valid) m.Validate().ShouldBeNull();
+        else m.Validate().ShouldNotBeNull().ShouldContain("element width");
+        // A SCALAR of the same shape is not this rule's business.
+        AbLegacyTagConfigModel.FromJson($$"""{"address":"{{address}}","dataType":"{{dataType}}"}""").Validate().ShouldBeNull();
+    }
+
    [Fact]
    public void Validate_returns_null_for_scalar_tag_regardless_of_no_array_length()
    {