src/Shared/Constants.cs (1)

115-117: Constant addition looks good; minor casing/doc nits

The new datasheet constant is appropriate and aligns with how other shared constants are exposed. Consider aligning the section header casing with nearby headers and adding a brief XML doc to aid discoverability in IDE hovers.

-        //Core datasheets
+        //Core Datasheets
+        /// <summary>
+        /// Datasheet name for the core Distribution Type (used for display-name lookups).
+        /// </summary>
         public const string DATASHEET_CORE_DISTRIBUTION_TYPE = "core_DistributionType";

src/Runtime/STSimTransformer.Distributions.cs (6)

86-92: Good guard; include value/context and treat NaN/∞ as invalid

The negative check is spot-on for the PR goal. Two small robustness wins:

• Include the sampled value and context (iteration/timestep for init can use MinimumIteration/MinimumTimestep) to speed up debugging.
• Treat NaN/Infinity as invalid too.
• Optional: rename colName to distName for clarity.

-                        if (t.CurrentValue.HasValue && (t.CurrentValue < 0.0))
-                        {
-                            string colName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
-                            throw new Exception(String.Format(
-                                "The following distribution produces a negative transition target: {0}", colName));
-                        }
+                        if (t.CurrentValue.HasValue)
+                        {
+                            double value = t.CurrentValue.Value;
+                            if (double.IsNaN(value) || double.IsInfinity(value) || value < 0.0)
+                            {
+                                string distName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
+                                throw new Exception(String.Format(
+                                    "Negative or invalid Transition Target during initialization (iter={0}, timestep={1}). value={2}, distributionType={3}",
+                                    this.MinimumIteration, this.MinimumTimestep, value, distName));
+                            }
+                        }

---

279-285: Mirror the same robustness on resample and add iteration/timestep to the error

Same feedback as init: include the offending value and resampling context, and treat NaN/∞ as invalid.

-                        if (t.CurrentValue.HasValue && (t.CurrentValue < 0.0))
-                        {
-                            string colName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
-                            throw new Exception(String.Format(
-                                "The following distribution produces a negative transition target: {0}", colName));
-                        }
+                        if (t.CurrentValue.HasValue)
+                        {
+                            double value = t.CurrentValue.Value;
+                            if (double.IsNaN(value) || double.IsInfinity(value) || value < 0.0)
+                            {
+                                string distName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
+                                throw new Exception(String.Format(
+                                    "Negative or invalid Transition Target after resampling (iter={0}, timestep={1}). value={2}, distributionType={3}",
+                                    iteration, timestep, value, distName));
+                            }
+                        }

---

402-413: Defensive lookup to avoid nulls and improve null-id text

If the datasheet or validation table is missing or the id is unknown, this can throw a NullReferenceException and mask the original error. Recommend a defensive fallback and a more user-friendly null-id label.

-        private string GetProjectItemName(string dataSheetName, int? id)
-        {
-            if (!id.HasValue)
-            {
-                return "NULL";
-            }
-            else
-            {
-                DataSheet ds = this.Project.GetDataSheet(dataSheetName);
-                return ds.ValidationTable.GetDisplayName(id.Value);
-            }
-        }
+        private string GetProjectItemName(string dataSheetName, int? id)
+        {
+            if (!id.HasValue)
+            {
+                return "Unspecified";
+            }
+
+            DataSheet ds = this.Project.GetDataSheet(dataSheetName);
+            string name = null;
+            if (ds != null && ds.ValidationTable != null)
+            {
+                name = ds.ValidationTable.GetDisplayName(id.Value);
+            }
+
+            return String.IsNullOrEmpty(name) ? id.Value.ToString() : name;
+        }

To verify safety across projects, please check at least one scenario file where the Core Distribution Type datasheet might be absent or customised. If that’s possible, the defensive code above prevents masking the primary error with a lookup failure.

---

279-285: Avoid duplication: extract a small validator helper

The init and resample checks are nearly identical. Consider extracting a private helper to keep the messages consistent and reduce future drift.

Example (outside the shown ranges):

private void ValidateTransitionTargetNonNegative(TransitionTarget t, string stage, int iteration, int timestep)
{
    if (!t.CurrentValue.HasValue) return;
    double value = t.CurrentValue.Value;
    if (double.IsNaN(value) || double.IsInfinity(value) || value < 0.0)
    {
        string distName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
        throw new Exception(String.Format(
            "Negative or invalid Transition Target {0} (iter={1}, timestep={2}). value={3}, distributionType={4}",
            stage, iteration, timestep, value, distName));
    }
}

Then call with:

• During init: ValidateTransitionTargetNonNegative(t, "during initialization", this.MinimumIteration, this.MinimumTimestep);
• During resample: ValidateTransitionTargetNonNegative(t, "after resampling", iteration, timestep);

I can push this refactor if you want.

Also applies to: 86-92

---

279-285: Consider a tiny tolerance to ignore floating-point underflow negatives

If distributions can produce values like -1e-15 due to numerical noise, a strict < 0.0 check will halt runs unnecessarily. If that’s a realistic scenario, consider tolerating tiny negatives (e.g., > -1e-12) and treating them as zero. If strictness is intentional per requirements, feel free to ignore this.

Proposed pattern:

const double Epsilon = 1e-12;
if (value < -Epsilon) { /* throw as you do */ }
// else if (value < 0) value = 0; // optional clamp

Also applies to: 86-92

---

279-285: Add targeted tests for negative/NaN/∞ cases

To lock behaviour, add tests that assert:

• Initialisation throws when a negative is sampled.
• Resampling throws when a negative appears mid-run.
• Optional: throws on NaN and Infinity if you adopt that check.

I can scaffold tests if you share the preferred test harness for STSim runtime-level checks (there isn’t one in this diff). Otherwise, I can propose a minimal harness that injects a stub distribution provider returning specific values.

Also applies to: 86-92

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/Runtime/STSimTransformer.Distributions.cs (1)

281-289: Same issue as initialization: “Throw” vs “clamp”, and duplicate logic

This block repeats the same negative-value handling as above and has the same behavioural mismatch with the PR title. Consolidate via the helper and decide on throw vs clamp once.

Apply:

-                        if (t.CurrentValue.HasValue && (t.CurrentValue < 0.0))
-                        {
-                            t.m_CurrentValue = 0.0;
-                            string distName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
-                            this.RecordStatus(StatusType.Warning, String.Format(
-                                "The following distribution produces a negative transition target that has been truncated to 0: {0}",
-                                distName));
-                        }
+                        EnforceNonNegativeTransitionTarget(t, iteration, timestep);

src/Statistics/STSimDistributionBase.cs (1)

23-23: Prefer encapsulated setter over exposing an internal field

Making m_CurrentValue internal leaks invariants and allows bypassing CheckDisabled(). Keep the field private and add an internal setter API for sanctioned writes from the transformer.

Apply:

-        internal double? m_CurrentValue;
+        private double? m_CurrentValue;

Add inside the class (near other members):

+        // Allows sanctioned updates from within the assembly without exposing the field.
+        internal void SetCurrentValue(double value)
+        {
+            this.m_CurrentValue = value;
+        }

Follow-up: update call sites in STSimTransformer to use t.SetCurrentValue(0.0) instead of t.m_CurrentValue = 0.0.

src/Runtime/STSimTransformer.Distributions.cs (3)

86-94: DRY: extract negative-target handling into a helper

This logic is duplicated here and in Resample. Extract into a single private helper and call it from both places.

Apply in-place change here:

-                        if (t.CurrentValue.HasValue && (t.CurrentValue < 0.0))
-                        {
-                            t.m_CurrentValue = 0.0;
-                            string distName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);
-                            this.RecordStatus(StatusType.Warning, String.Format(
-                                "The following distribution produces a negative transition target that has been truncated to 0: {0}", 
-                                distName));
-                       }
+                        EnforceNonNegativeTransitionTarget(t, this.MinimumIteration, this.MinimumTimestep);

Add this helper (adjust throw vs clamp per chosen policy):

// Place near other private helpers
private void EnforceNonNegativeTransitionTarget(TransitionTarget t, int iteration, int timestep)
{
    if (!t.CurrentValue.HasValue)
    {
        return;
    }

    if (t.CurrentValue.Value < 0.0)
    {
        string distName = GetProjectItemName(Constants.DATASHEET_CORE_DISTRIBUTION_TYPE, t.DistributionTypeId);

        // Option A: throw (per PR title)
        // throw new ArgumentException($"Negative Transition Target value ({t.CurrentValue.Value}) for distribution '{distName}' at Iteration {iteration}, Timestep {timestep}.");

        // Option B: clamp + warn (current behaviour)
        t.SetCurrentValue(0.0); // if you keep the field, this would be: t.m_CurrentValue = 0.0;
        this.RecordStatus(StatusType.Warning,
            $"The following distribution produces a negative transition target that has been truncated to 0: {distName}");
    }
}

---

406-417: Make GetProjectItemName null-safe (avoid potential NRE) and more resilient

Project.GetDataSheet(...) or ValidationTable could be null and cause an NRE. Add a safe fallback string.

Apply:

-        private string GetProjectItemName(string dataSheetName, int? id)
+        private string GetProjectItemName(string dataSheetName, int? id)
         {
             if (!id.HasValue)
             {
-                return "NULL";
+                return "NULL";
             }
             else
             {
-                DataSheet ds = this.Project.GetDataSheet(dataSheetName);
-                return ds.ValidationTable.GetDisplayName(id.Value);
+                DataSheet ds = this.Project.GetDataSheet(dataSheetName);
+                if (ds?.ValidationTable == null)
+                {
+                    return $"{dataSheetName}:{id.Value}";
+                }
+                return ds.ValidationTable.GetDisplayName(id.Value);
             }
         }

Optional: cache DataSheet lookups in a Dictionary<string, DataSheet> if this runs hot.

---

86-94: Add unit tests for negative TransitionTarget handling

Please add tests covering: (a) negative sample on initialization, (b) negative sample on resample, (c) near-zero negative within tolerance if you add epsilon, (d) message content (warning vs exception) aligned with the decided policy.

I can draft the tests with a fake STSimDistributionProvider that returns a negative sample—say, -5.0—to assert the expected behaviour.

Also applies to: 281-289

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.