Audit public mutable array state and ownership semantics

[0xC000005]

Summary

Several public properties expose mutable array instances from core interpolants:

• ChebyshevApproximation.Domain, NNodes, NodeArrays, TensorValues, Weights, DiffMatrices
• ChebyshevSpline.Domain, NNodes, Knots
• ChebyshevSlider.Domain, NNodes, Partition, PivotPoint
• ChebyshevTT.Domain, NNodes
• record wrappers such as Domain(double[][] Bounds), Ns(int[] Counts), and SpecialPoints(double[][] Points)

Because arrays are mutable even when the property setter is internal or absent, callers can corrupt object invariants after construction. Examples include mutating a built object's domain bounds to NaN, changing NNodes away from tensor shape, or changing stored tensor values before evaluation or serialization.

Why this is architectural

This should not be fixed as a small one-off validation patch. Binary write-side validation can prevent one malformed output path, but the same mutated state can affect JSON save, evaluation, algebra compatibility, roots/min/max, integration, and diagnostics.

Microsoft CA1819 explicitly warns that array-returning properties are not write-protected and says keeping an array tamper-proof requires returning a copy or changing the API shape. Microsoft also marks this as a breaking fix, which means it should be planned as a versioned public API hardening change rather than bundled into unrelated bug PRs.

References:

• https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1819
• https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2227

Proposed direction

1. Decide the public contract:
   • Snapshot APIs: properties return cloned arrays or read-only views; mutation requires explicit methods.
   • Performance APIs: keep raw-array access but clearly mark as advanced/unsafe and add safe getters for normal users.
   • Hybrid: keep current properties for compatibility in a minor release, add safe clone/read-only accessors, obsolete mutable properties, then remove/change in the next major version.
2. Add regression tests showing that external mutation cannot corrupt:
   • evaluation,
   • binary/JSON save/load,
   • algebra compatibility,
   • TT rank/domain metadata,
   • spline knot routing.
3. Update docs to explain ownership/copy semantics.
4. Audit DTO/serialization-state classes separately; returning arrays from internal DTOs may be acceptable, but public runtime objects should not expose mutable invariant storage accidentally.

Audit context

This was found while auditing .pcb binary load/save boundaries after PR #72. PR #72 intentionally remains narrow: it hardens read-side parsing of untrusted binary payloads and does not attempt a broad public API redesign.

[0xC000005]

Additional surface from the same code search: SobolResult(double[] FirstOrder, double[] TotalOrder, double Variance) also exposes mutable arrays. It may be lower risk than runtime interpolant state because it is result data, but it should be included in the same ownership/immutability audit.

[0xC000005]

Merged the result/wrapper ownership slice in #80.

What changed:

• Domain, Ns, and SpecialPoints now snapshot constructor/init inputs and return copied arrays from properties, implicit conversions, and deconstruction.
• SobolResult now snapshots FirstOrder/TotalOrder inputs and returns copied arrays while preserving the public init-only record API.
• Package validation initially caught the unsafe version of this change because removing positional-record init setters would have broken API compatibility; the merged version preserves them.

Validation:

• Focused RecordTypesTests: 7/7 passed.
• Sobol tests: 35/35 passed.
• Sensitivity convention tests: 2/2 passed.
• Full Release suite: 1539/1539 passed.
• Release build, format verification, diff check, NuGet package validation, PR Actions, codecov/patch, and post-merge main workflows all passed.

Remaining ownership surfaces I still have tracked:

• ChebyshevApproximation.GetSpecialPoints()
• cached GetEvaluationPoints() buffers, which need an explicit policy decision: snapshot by default, documented live cache, or separate unsafe/performance-returning API.

[0xC000005]

Merged the approximation special-points ownership slice in #81.

What changed:

• ChebyshevApproximation.GetSpecialPoints() now returns a deep copy instead of the live JSON-loaded metadata array.
• Added a regression test that loads valid SpecialPoints metadata from JSON, mutates the returned array, and verifies later reads are unchanged.

Validation:

• Focused ApproxPublicStateOwnershipTests: 8/8 passed.
• SpecialPoints tests: 12/12 passed.
• JSON migration/load tests: 8/8 passed.
• Full Release suite: 1540/1540 passed.
• Release build, format verification, diff check, NuGet package validation, PR Actions, codecov/patch, Codecov comment, and post-merge main workflows all passed.

Remaining ownership surface tracked for #73:

• cached GetEvaluationPoints() buffers across the interpolants. This needs an explicit policy decision because existing tests currently assert cached same-instance behavior.

[0xC000005]

Final ownership-hardening slice landed in #82.

Summary:

• ChebyshevApproximation.GetEvaluationPoints(), ChebyshevSpline.GetEvaluationPoints(), ChebyshevSlider.GetEvaluationPoints(), and ChebyshevTT.GetEvaluationPoints() now keep lazy caching internal while returning caller-owned snapshots.
• XML docs now describe the snapshot-return contract for these APIs.
• The final public-array scan after fix: snapshot cached evaluation points #82 found remaining public array returns are result-producing APIs, already-snapshot properties, internal/serialization DTOs, or factory/node-info payloads outside the invariant-ownership defect class.

Validation for #82:

• TDD red: the four focused snapshot tests failed before the implementation because repeated calls returned the same mutable array instance.
• Focused snapshot tests: 4/4 passed.
• All GetEvaluationPoints tests: 9/9 passed.
• Full Release suite: 1540/1540 passed.
• Release build, format check, git diff --check, and package validation all passed.
• Pre-merge GitHub Actions and codecov/patch passed; Codecov reported all modified and coverable lines covered.
• Post-merge main Tests and Deploy Docs workflows passed for merge commit 5978576484837dfc1fc84674c35ced8a9b25fc2d.

This completes the tracked #73 surfaces across #75, #77, #78, #79, #80, #81, and #82. Reopen or file a follow-up only if new public mutable state is added, serialization DTOs become runtime invariant storage, or an intentional high-performance unsafe view API is introduced.