BUG: Enforce array-kind contracts in ArraySelectionParameter #1833

Problem

Track the parameter-level implementation follow-up to #1821. That issue remains the original defect report and contains the compatibility investigation. This follow-up records the shared validation work and the decisions required before a separate implementation PR.

This work does not block #1817 or #1826. Keep the local typed-array checks in #1817 while the shared parameter contract is developed. Document their temporary purpose and link this issue from the code. Do not remove the guards until parameter validation provides equivalent protection and regression tests pass.

Numeric-only filters can accept a StringArray or NeighborList through ArraySelectionParameter and then throw during preflight. The selection should produce an invalid preflight result with an actionable diagnostic before the filter accesses a numeric array.

Verified at simplnx commit 58f9f72c8917a2e07361048549e355fb81960221 on Windows with MSVC 2022, Release, Catch2 2.13.6. In ConvertDataFilter, both array kinds produced an unexpected Bad dynamic_cast! exception from filter.preflight(...). This report establishes an exception escaping the filter API; application-level handling was not tested.

Steps to Reproduce

1. Check out the reported commit with testing enabled.
2. Add the complete test source below to src/Plugins/SimplnxCore/test/ConvertDataTest.cpp, using the existing test runner. Keep the existing tests in that file.
3. Build SimplnxCoreUnitTest in Release.
4. Run the conversion rejection test through CTest:

cmake --build <build> --config Release --target SimplnxCoreUnitTest
ctest --test-dir <build> -C Release -R "ArraySelectionIssue: conversion rejects unsupported kinds without throwing" -j 1 --output-on-failure

The fixture creates two-tuples of numeric arrays, strings, and neighbor lists. No external input files are required. Each rejection section selects one unsupported array kind for Convert AttributeArray DataType and calls preflight.

Cause

ArraySelectionParameter::validatePath accepts any IArray, then applies allowed numeric types, component shapes, and storage-location checks only inside the IDataArray branch. StringArray and NeighborList skip that branch and return success.

ConvertDataFilter declares this generic selection parameter with all numeric types. Its preflight implementation then calls getDataRefAs<IDataArray>. That accessor uses a reference dynamic_cast, which throws for either non-IDataArray kind. IFilter::preflight calls preflightImpl without catching this exception.

Generic consumers need different acceptance rules. ReshapeDataArrayFilter and SplitDataArrayByTupleFilter deliberately support StringArray and NeighborList. A blanket IDataArray requirement in the shared parameter would remove supported behavior.

The proposed shortcut dataArray == nullptr && !m_AllowedTypes.empty() is not safe as a global fix. The constructor requires nonempty AllowedTypes, so this condition would reject every non-IDataArray selection. Reshape Data Array and Split Data Array by Tuple intentionally support StringArray and NeighborList and also pass GetAllDataTypes(). Convert Data uses the same declaration but requires IDataArray. The existing numeric type set therefore does not identify which array kinds a consumer supports.

An array kind and its element type are different requirements. A NeighborList can contain int32 values without being a scalar int32 DataArray. StringArray has no numeric DataType value. The shared contract must represent the consumer's actual requirements without inferring them from tuple counts or numeric type declarations alone.

Verification Unit Test

The complete source below contains three passing compatibility controls, one failing conversion test with two sections, and two additional policy probes. The policy probes document currently bypassed restrictions; the final cross-kind type/shape contract remains a design decision.

Observed results from an isolated published-head checkout and a snapshot of its previously validated Release libraries:

• Numeric type/shape validation, numeric conversion preflight, and generic StringArray/NeighborList reshape controls passed.
• The conversion test failed in both sections: 15 assertions, 13 passed, 2 failed.
• The shape/element-type probes failed three assertions: fixed-one-component StringArray and NeighborList selections passed a three-component restriction, and a float NeighborList passed an int32-only restriction. Their controls passed.

Actual conversion failure excerpt (the same failure occurred for each array kind):

REQUIRE_NOTHROW( result = filter.preflight(data, args) )
due to unexpected exception with message:
  Bad dynamic_cast!

test cases:  1 |  0 passed | 1 failed
assertions: 15 | 13 passed | 2 failed

The isolated harness was built from the reported worktree headers and a copied runtime/import-library set taken before unrelated review fixes were rebuilt. The tests ran sequentially through CTest. No production fix was applied.

Run all cases after adding this source to the normal test file:

ctest --test-dir <build> -C Release -R "ArraySelectionIssue:" -j 1 --output-on-failure

// Standalone Catch2 v2 regression source for simplnx SHA 58f9f72c8917a2e07361048549e355fb81960221.
// Compile with the existing SimplnxCoreUnitTest runner, or define CATCH_CONFIG_MAIN
// on this translation unit when linking a separate executable against SimplnxCore and simplnx.
#include "SimplnxCore/Filters/ConvertDataFilter.hpp"
#include "SimplnxCore/Filters/ReshapeDataArrayFilter.hpp"

#include "simplnx/Common/DataTypeUtilities.hpp"
#include "simplnx/DataStructure/DataArray.hpp"
#include "simplnx/DataStructure/DataStore.hpp"
#include "simplnx/DataStructure/DataStructure.hpp"
#include "simplnx/DataStructure/NeighborList.hpp"
#include "simplnx/DataStructure/StringArray.hpp"
#include "simplnx/Parameters/ArraySelectionParameter.hpp"
#include "simplnx/Parameters/DynamicTableParameter.hpp"
#include "simplnx/UnitTest/UnitTestCommon.hpp"

#include <catch2/catch.hpp>

using namespace nx::core;

namespace
{
void PopulateArrays(DataStructure& data)
{
  REQUIRE(DataArray<int32>::CreateWithStore<DataStore<int32>>(data, "IntScalar", ShapeType{2}, ShapeType{1}) != nullptr);
  REQUIRE(DataArray<int32>::CreateWithStore<DataStore<int32>>(data, "IntVector", ShapeType{2}, ShapeType{3}) != nullptr);
  REQUIRE(DataArray<float32>::CreateWithStore<DataStore<float32>>(data, "FloatScalar", ShapeType{2}, ShapeType{1}) != nullptr);
  REQUIRE(StringArray::CreateWithValues(data, "Strings", ShapeType{2}, {"alpha", "beta"}) != nullptr);
  auto* ints = NeighborList<int32>::Create(data, "IntNeighbors", ShapeType{2});
  REQUIRE(ints != nullptr);
  ints->setList(0, std::vector<int32>{1, 7});
  ints->setList(1, std::vector<int32>{3});
  REQUIRE(NeighborList<float32>::Create(data, "FloatNeighbors", ShapeType{2}) != nullptr);
}
} // namespace

TEST_CASE("ArraySelectionIssue: numeric restrictions remain enforced", "[ArraySelectionIssue][controls]")
{
  DataStructure data;
  PopulateArrays(data);
  const ArraySelectionParameter scalar("input", "Input", "", {}, {DataType::int32}, {{1}});
  CHECK(scalar.validatePath(data, DataPath({"IntScalar"})).valid());
  CHECK(scalar.validatePath(data, DataPath({"FloatScalar"})).invalid());
  CHECK(scalar.validatePath(data, DataPath({"IntVector"})).invalid());
}

// Exploratory policy probes: these assert one possible common shape/element-type
// contract. They do not encode an approved API change or blanket kind rejection.
TEST_CASE("ArraySelectionIssue: shared component shape restriction", "[ArraySelectionIssue][regression][policy]")
{
  DataStructure data;
  PopulateArrays(data);
  const ArraySelectionParameter vector("input", "Input", "", {}, GetAllDataTypes(), {{3}});
  CHECK(vector.validatePath(data, DataPath({"IntVector"})).valid());
  CHECK(vector.validatePath(data, DataPath({"IntScalar"})).invalid());
  SECTION("StringArray has one component")
  {
    REQUIRE_NOTHROW(data.getDataRefAs<StringArray>(DataPath({"Strings"})));
    REQUIRE(data.getDataRefAs<StringArray>(DataPath({"Strings"})).getComponentShape() == ShapeType{1});
    CHECK(vector.validatePath(data, DataPath({"Strings"})).invalid());
  }
  SECTION("NeighborList has one component")
  {
    REQUIRE_NOTHROW(data.getDataRefAs<NeighborList<int32>>(DataPath({"IntNeighbors"})));
    REQUIRE(data.getDataRefAs<NeighborList<int32>>(DataPath({"IntNeighbors"})).getComponentShape() == ShapeType{1});
    CHECK(vector.validatePath(data, DataPath({"IntNeighbors"})).invalid());
  }
}

TEST_CASE("ArraySelectionIssue: neighbor element type restriction", "[ArraySelectionIssue][regression][policy]")
{
  DataStructure data;
  PopulateArrays(data);
  const ArraySelectionParameter ints("input", "Input", "", {}, {DataType::int32}, {{1}});
  // Acceptance of matching NeighborList inputs is a compatibility choice. This
  // test chooses preservation, then requires the declared element restriction.
  CHECK(ints.validatePath(data, DataPath({"IntNeighbors"})).valid());
  CHECK(ints.validatePath(data, DataPath({"FloatNeighbors"})).invalid());
}

TEST_CASE("ArraySelectionIssue: generic reshape keeps supported array kinds", "[ArraySelectionIssue][controls]")
{
  UnitTest::LoadPlugins();
  DataStructure data;
  PopulateArrays(data);
  const ArraySelectionParameter generic("input", "Input", "", {}, GetAllDataTypes());
  CHECK(generic.validatePath(data, DataPath({"Strings"})).valid());
  CHECK(generic.validatePath(data, DataPath({"IntNeighbors"})).valid());

  ReshapeDataArrayFilter filter;
  auto args = filter.getDefaultArguments();
  args.insertOrAssign(ReshapeDataArrayFilter::k_TupleDims_Key, std::make_any<DynamicTableParameter::ValueType>(DynamicTableParameter::ValueType{{1.0, 2.0}}));

  SECTION("StringArray remains supported")
  {
    const DataPath path({"Strings"});
    args.insertOrAssign(ReshapeDataArrayFilter::k_Input_Array_Key, std::make_any<DataPath>(path));
    REQUIRE(filter.preflight(data, args).outputActions.valid());
    REQUIRE(filter.execute(data, args).result.valid());
    REQUIRE_NOTHROW(data.getDataRefAs<StringArray>(path));
    const auto& output = data.getDataRefAs<StringArray>(path);
    CHECK(output.getTupleShape() == ShapeType{1, 2});
    CHECK(output[0] == "alpha");
    CHECK(output[1] == "beta");
  }
  SECTION("NeighborList remains supported")
  {
    const DataPath path({"IntNeighbors"});
    args.insertOrAssign(ReshapeDataArrayFilter::k_Input_Array_Key, std::make_any<DataPath>(path));
    REQUIRE(filter.preflight(data, args).outputActions.valid());
    REQUIRE(filter.execute(data, args).result.valid());
    REQUIRE_NOTHROW(data.getDataRefAs<NeighborList<int32>>(path));
    const auto& output = data.getDataRefAs<NeighborList<int32>>(path);
    CHECK(output.getTupleShape() == ShapeType{1, 2});
    CHECK(output.getList(0) == std::vector<int32>{1, 7});
    CHECK(output.getList(1) == std::vector<int32>{3});
  }
  UnitTest::CheckArraysInheritTupleDims(data);
}

TEST_CASE("ArraySelectionIssue: conversion accepts numeric arrays", "[ArraySelectionIssue][controls]")
{
  UnitTest::LoadPlugins();
  DataStructure data;
  PopulateArrays(data);
  ConvertDataFilter filter;
  auto args = filter.getDefaultArguments();
  args.insertOrAssign(ConvertDataFilter::k_ArrayToConvertPath_Key, std::make_any<DataPath>(DataPath({"IntScalar"})));
  REQUIRE(filter.preflight(data, args).outputActions.valid());
}

TEST_CASE("ArraySelectionIssue: conversion rejects unsupported kinds without throwing", "[ArraySelectionIssue][regression][preflight]")
{
  UnitTest::LoadPlugins();
  DataStructure data;
  PopulateArrays(data);
  ConvertDataFilter filter;
  auto args = filter.getDefaultArguments();
  DataPath path;
  SECTION("StringArray")
  {
    path = DataPath({"Strings"});
  }
  SECTION("NeighborList")
  {
    path = DataPath({"IntNeighbors"});
  }
  args.insertOrAssign(ConvertDataFilter::k_ArrayToConvertPath_Key, std::make_any<DataPath>(path));
  IFilter::PreflightResult result;
  REQUIRE_NOTHROW(result = filter.preflight(data, args));
  REQUIRE(result.outputActions.invalid());
}

Proposed Fix

Implement this in a separate PR. The proposed design replaces ArraySelectionParameter's current set of DataType values with a set of a new enum. Each enum value identifies both the array kind and its element type. IDataArray and neighbor-list entries are distinct even when their elements have the same DataType. StringArray has its own entry.

Use the more precise allowed set to add checks to validate / validatePath. The public validate method currently delegates to validatePath; both entry points must enforce the same contract. Reject an unlisted combined type before a filter obtains a typed reference.

The following enum records the proposed coverage. Its name and enumerator names are illustrative, not final:

enum class AllDataType : uint8
{
  DataArray_int8,
  DataArray_uint8,
  DataArray_int16,
  DataArray_uint16,
  DataArray_int32,
  DataArray_uint32,
  DataArray_int64,
  DataArray_uint64,
  DataArray_float32,
  DataArray_float64,
  DataArray_boolean,
  NeighborListArray_int8,
  NeighborListArray_uint8,
  NeighborListArray_int16,
  NeighborListArray_uint16,
  NeighborListArray_int32,
  NeighborListArray_uint32,
  NeighborListArray_int64,
  NeighborListArray_uint64,
  NeighborListArray_float32,
  NeighborListArray_float64,
  string
};

The current neighbor-list base class is named INeighborList; IArray::ArrayType::NeighborListArray identifies that kind. The example's NeighborListArray entries refer to that family.

Explore shorter names before implementation. For example, ArraySelectionType with DataInt32, ListInt32, and String makes the selection contract explicit. This is a naming candidate only. Keep a distinct name from the existing IArray::ArrayType, which identifies only the array kind.

Expected validation behavior:

1. Keep the existing empty-path, missing-object, and IArray validation.
2. Determine the selected kind from its array metadata. For IDataArray and INeighborList, also obtain the element DataType. Map StringArray to the dedicated string entry. These interfaces expose the required metadata; validation does not need array contents.
3. Map the kind and element type to the new enum and require that exact value in the allowed set. An int32 NeighborList does not satisfy DataArray_int32. A numeric DataArray does not satisfy the corresponding NeighborListArray entry. StringArray requires the string entry.
4. Return an actionable invalid Result for an unsupported kind, element type, or unmapped combination. Do not let unsupported objects reach typed reference access. Retain the existing parameter error-code contracts where applicable; specify any necessary change.
5. Apply the separately defined component-shape and storage-location rules. Membership in the new set does not prove that shape or location is valid.

Examples for tests and documentation:

Migration details to settle:

• Change the AllowedTypes set, constructor/accessors, cloning, helpers, and dependent callers to carry the new enum consistently. Do not silently translate an old numeric-only declaration into acceptance of every array kind.
• Provide clear helper-set semantics for all DataArray element types, all supported NeighborList element types, and all supported kinds. The current GetAllDataTypes() returns the 11 DataType values; it cannot by itself describe the new distinction. Preserve that utility's other consumers as needed.
• Migrate numeric-only filters to DataArray entries. Migrate intentional generic consumers to explicit entries for every supported kind. Confirm the existing behavior of saved pipelines and any affected Python/GUI parameter consumers.
• Keep the combined-enum direction above. Final naming, migration/default compatibility, component-shape rules, storage-location rules, and the behavior of an empty allowed set still need explicit decisions.

The proposed enum has 22 values: 11 DataArray entries including Boolean, 10 NeighborList entries excluding Boolean, and one StringArray entry. Boolean NeighborLists remain unsupported. This update must preserve existing NeighborList type handling; adding Boolean NeighborList support is outside its scope.

Required behavior and compatibility constraints:

• Reject array kinds that a numeric-only consumer cannot process before that consumer obtains an IDataArray or a concrete typed reference. Return an invalid Result with the selected path, actual kind, expected kind/type, and corrective action. Do not let a dynamic_cast exception escape preflight.
• Preserve intentional StringArray and NeighborList support in generic consumers, including Reshape Data Array and Split Data Array by Tuple. Confirm their output values and tuple shapes as well as successful preflight.
• Enforce kind and element type through the combined enum. DataArray and NeighborList values with the same element type remain distinct. Accept StringArray only when the string entry is allowed.
• Define whether and how required component shapes apply to each accepted array kind. StringArray and NeighborList report one component; variable neighbor-list length is not the component count.
• Define storage-location validation for each supported kind and its EmptyDataStore/preflight behavior. The current bypass includes location checks; the existing reproducer does not establish the correct cross-kind storage policy.
• Audit ArraySelectionParameter consumers before changing defaults. Numeric-only and deliberately generic consumers currently share the same constructor configuration. Preserve supported saved-pipeline behavior, parameter cloning, and Python/GUI selection behavior where the selected design affects them.
• Use metadata for selection validation. Avoid reading all array values or creating full-array resident buffers. If storage-specific behavior changes, verify actual disk-backed stores separately from resident algorithm-path tests.
• Review existing MultiArraySelectionParameter array-kind validation and NeighborListSelectionParameter element-type validation as established patterns. Reuse compatible behavior where appropriate; these references do not prescribe the final API.

Acceptance and regression coverage:

• Implement the combined-enum allowed set and additional validation checks described above; settle final names and migration policy.
• Test each supported enum mapping and the exact-match examples above through both validate and validatePath. Include numeric-only, list-only, string-only, and mixed allowed sets.
• Keep Boolean DataArray support. Do not add a Boolean NeighborList enum entry or change existing NeighborList type handling.
• Document and test shape, location, and empty-set behavior independently of enum membership. Verify cloning and affected selection interfaces preserve the allowed set.
• Numeric-only ConvertDataFilter rejects StringArray and NeighborList with invalid preflight Results and no exceptions. Both sections of the reproduced failure pass.
• Valid numeric inputs remain accepted; wrong numeric types and component shapes remain rejected.
• Generic ReshapeDataArrayFilter and SplitDataArrayByTupleFilter continue to accept and correctly process supported StringArray and NeighborList inputs.
• Update the exploratory shape/type policy tests only after the contract is agreed. Their currently proposed expectations are not an approved API design.
• Audit representative typed preflight consumers and update declarations or validation as required by the agreed contract. Include shared-parameter unit tests and filter-level regressions.
• Check configured formatting and applicable static analysis. Run relevant in-core and OOC tests; distinguish metadata/resident checks from actual disk-backed behavior.
• Once equivalent shared protection is available, revisit the temporary ComputeSurfaceAreaToVolume guards and their expected errors. Preserve the no-throw, no-created-output behavior and update documentation.

Notes

• #1821 contains the original failure report and full compatibility investigation. Keep its historical reproduction evidence; this issue records the parameter-level follow-up.
• PR #1817 review and its inline request prompted this decision. The shared fix is explicitly deferred and is not a merge prerequisite for BUG: Correct surface-area and sphericity calculations #1817.
• At BUG: Correct surface-area and sphericity calculations #1817 head 008acec895cf5b4ea9a53925e7686d27413564f4, ComputeSurfaceAreaToVolumeFilter already rejects unsupported FeatureIds kinds with -12803 and unsupported NumCells kinds with -12801. Its regression test covers StringArray and NeighborList at both inputs, requires preflight not to throw, and verifies that no output arrays are created. Retain those protections until the shared replacement is ready.
• PR #1826 migrates tuple-count checks in three filters to ValidateCellTupleCount. Its migration commit does not add the -12803 array-kind guard to those filters. Count validation and array-kind validation are separate concerns.
• Closed issue #555 records earlier StringArray-selection requirements. This compatibility history matters when choosing the parameter contract.
• PR #1766, inspected at d0724ecd641247d3d15282e9abf3db398e60da9d, changes filter type/shape declarations but does not change ArraySelectionParameter or ConvertDataFilter. Those declarations alone do not address the non-IDataArray bypass.
• The baseline reproducer was compiled and run on Windows with MSVC 2022 Release and Catch2 2.13.6. Its source SHA is 58f9f72c8917a2e07361048549e355fb81960221. It uses isolated headers and a copied runtime/import-library snapshot from the validated baseline. This is not a fresh full rebuild, a GUI crash reproduction, or cross-platform proof.
• The preserved isolated harness was rerun on 2026-10-19 after verifying its source SHA and the simplnx/SimplnxCore binary hashes against its provenance record. Compatibility controls passed; both unsupported-kind conversion sections still failed with Bad dynamic_cast!. The policy probes above describe the original 2026-10-15 run and were not rerun for this follow-up.