fix: treat out-of-range values in comparison tests as a warning instead of error

packages/check-core/src/comparison/_shared/comparison-resolved-types.ts

@@ -48,22 +48,40 @@ export interface ComparisonDataset {
 /** A unique key for a `ComparisonScenario`, generated internally for use by the library. */
 export type ComparisonScenarioKey = string & { _brand?: 'ComparisonScenarioKey' }
 
+/** A fatal error indicating that no input variable matched the requested name. */
 export interface ComparisonResolverUnknownInputError {
   kind: 'unknown-input'
 }
 
+/** A fatal error indicating that no input setting group matched the requested ID. */
 export interface ComparisonResolverUnknownInputSettingGroupError {
   kind: 'unknown-input-setting-group'
 }
 
-export interface ComparisonResolverInvalidValueError {
-  kind: 'invalid-value'
-}
-
+/**
+ * A fatal resolution error.  When this is set on an input state, the scenario
+ * cannot be run for the affected side: spec construction for that side is
+ * skipped and downstream UI annotations render it as an error.
+ */
 export type ComparisonResolverError =
   | ComparisonResolverUnknownInputError
   | ComparisonResolverUnknownInputSettingGroupError
-  | ComparisonResolverInvalidValueError
+
+/**
+ * A non-fatal warning indicating that the resolved value falls outside the
+ * declared `[minValue, maxValue]` range of a slider (or is not one of the
+ * declared values of a switch).  The scenario still runs with the requested
+ * value; consumers (e.g. UI annotations) should flag it as a warning.
+ */
+export interface ComparisonResolverValueOutOfRangeWarning {
+  kind: 'value-out-of-range'
+}
+
+/**
+ * A non-fatal resolution warning.  Warnings do not prevent the scenario from
+ * running; they are surfaced as annotations alongside the resolved value.
+ */
+export type ComparisonResolverWarning = ComparisonResolverValueOutOfRangeWarning
 
 /** Describes the resolution state for a scenario input relative to a specific model. */
 export interface ComparisonScenarioInputState {
@@ -73,8 +91,14 @@ export interface ComparisonScenarioInputState {
   position?: InputPosition
   /** The value of the input, for the given position or explicit value. */
   value?: number
-  /** The error info if the input could not be resolved. */
+  /** The fatal error info if the input could not be resolved. */
   error?: ComparisonResolverError
+  /**
+   * Non-fatal advisory info about the resolved input.  When set (without an
+   * accompanying `error`), the input still resolved and the scenario can run;
+   * consumers should surface the warning alongside the resolved value.
+   */
+  warning?: ComparisonResolverWarning
 }
 
 /** A scenario input that has been checked against both "left" and "right" model. */

packages/check-core/src/comparison/config/resolve/comparison-resolver.spec.ts

@@ -70,7 +70,10 @@ import { resolveComparisonSpecs } from './comparison-resolver'
 
 const errUnknownInput: ComparisonScenarioInputState = { error: { kind: 'unknown-input' } }
 const errUnknownSettingGroup: ComparisonScenarioInputState = { error: { kind: 'unknown-input-setting-group' } }
-const errInvalidValue: ComparisonScenarioInputState = { error: { kind: 'invalid-value' } }
+
+function warnOutOfRange(inputVar: InputVar, value: number): ComparisonScenarioInputState {
+  return { inputVar, value, warning: { kind: 'value-out-of-range' } }
+}
 
 function mockModelSpec(kind: 'L' | 'R'): ModelSpec {
   //
@@ -280,10 +283,14 @@ describe('resolveComparisonSpecs', () => {
           scenarioWithInputsSpec([inputAtValueSpec('id 2', 40)]),
           // Match by alias (slider name)
           scenarioWithInputsSpec([inputAtValueSpec('S3', 60)]),
-          // Error if value is out of range on both sides
+          // Warning if value is out of range on both sides; the scenario still runs
+          // with the requested value
           scenarioWithInputsSpec([inputAtValueSpec('ivarA', 500)]),
-          // Error if value is out of range on one side
-          scenarioWithInputsSpec([inputAtValueSpec('id 2', 90)])
+          // Warning if value is out of range on one side; the scenario still runs
+          // with the requested value on each side
+          scenarioWithInputsSpec([inputAtValueSpec('id 2', 90)]),
+          // Error if input is unknown on both sides; the scenario does not run
+          scenarioWithInputsSpec([inputAtValueSpec('ivarX', 10)])
         ]
       }
 
@@ -317,24 +324,36 @@ describe('resolveComparisonSpecs', () => {
             atValSpec('_ivarc', 60),
             atValSpec('_ivard', 60)
           ),
-          scenarioWithInput(
+          // Out-of-range on both sides: scenario still runs at value 500 on each side,
+          // both states carry a `value-out-of-range` warning
+          scenarioWithInputs(
             '4',
-            'ivarA',
-            500,
-            { kind: 'invalid-value' },
-            { kind: 'invalid-value' },
-            undefined,
-            undefined
+            [
+              {
+                requestedName: 'ivarA',
+                stateL: warnOutOfRange(lVar('IVarA'), 500),
+                stateR: warnOutOfRange(rVar('IVarA'), 500)
+              }
+            ],
+            atValSpec('_ivara', 500),
+            atValSpec('_ivara', 500)
           ),
-          scenarioWithInput(
+          // Out-of-range on right side only: scenario still runs at value 90 on each
+          // side, right state carries a `value-out-of-range` warning
+          scenarioWithInputs(
             '5',
-            'id 2',
-            90,
-            lVar('IVarB'),
-            { kind: 'invalid-value' },
+            [
+              {
+                requestedName: 'id 2',
+                stateL: { inputVar: lVar('IVarB'), value: 90 },
+                stateR: warnOutOfRange(rVar('IVarB_Renamed'), 90)
+              }
+            ],
             atValSpec('_ivarb', 90),
-            undefined
-          )
+            atValSpec('_ivarb_renamed', 90)
+          ),
+          // Unknown input on both sides remains fatal: no specs produced
+          scenarioWithInput('6', 'ivarX', 10, undefined, undefined, undefined, undefined)
         ],
         scenarioGroups: [],
         viewGroups: []
@@ -364,9 +383,11 @@ describe('resolveComparisonSpecs', () => {
           scenarioWithDistinctInputsSpec([inputAtValueSpec('S3', 60)], [inputAtValueSpec('S3', 70)]),
           // Error if input is not available on requested side
           scenarioWithDistinctInputsSpec([inputAtValueSpec('ivarA', 20)], [inputAtValueSpec('unknown', 600)]),
-          // Error if value is out of range on both sides
+          // Warning if value is out of range on both sides; the scenario still runs
+          // with the requested values on each side
           scenarioWithDistinctInputsSpec([inputAtValueSpec('ivarA', 500)], [inputAtValueSpec('ivarA', 600)]),
-          // Error if value is out of range on one side
+          // Warning if value is out of range on one side; the scenario still runs
+          // with the requested values on each side
           scenarioWithDistinctInputsSpec([inputAtValueSpec('id 2', 90)], [inputAtValueSpec('id 2', 600)])
         ]
       }
@@ -378,13 +399,25 @@ describe('resolveComparisonSpecs', () => {
           scenarioWithInputs('2', [], atValSpec('_ivarb', 40), atValSpec('_ivarb_renamed', 50)),
           scenarioWithInputs('3', [], atValSpec('_ivarc', 60), atValSpec('_ivard', 70)),
           scenarioWithInputs('4', [resolvedInput('unknown', {}, errUnknownInput)], undefined, undefined),
+          // Out-of-range on both sides: scenario still runs with the requested values,
+          // both states carry a `value-out-of-range` warning
           scenarioWithInputs(
             '5',
-            [resolvedInput('ivarA', errInvalidValue, {}), resolvedInput('ivarA', {}, errInvalidValue)],
-            undefined,
-            undefined
+            [
+              resolvedInput('ivarA', warnOutOfRange(lVar('IVarA'), 500), {}),
+              resolvedInput('ivarA', {}, warnOutOfRange(rVar('IVarA'), 600))
+            ],
+            atValSpec('_ivara', 500),
+            atValSpec('_ivara', 600)
           ),
-          scenarioWithInputs('6', [resolvedInput('id 2', {}, errInvalidValue)], undefined, undefined)
+          // Out-of-range on right side only: scenario still runs with the requested
+          // values, right state carries a `value-out-of-range` warning
+          scenarioWithInputs(
+            '6',
+            [resolvedInput('id 2', {}, warnOutOfRange(rVar('IVarB_Renamed'), 600))],
+            atValSpec('_ivarb', 90),
+            atValSpec('_ivarb_renamed', 600)
+          )
         ],
         scenarioGroups: [],
         viewGroups: []
@@ -434,10 +467,10 @@ describe('resolveComparisonSpecs', () => {
           // id=sg6        id=sg6      (ERROR: setting with unknown input variable)
           //   IVarZ=40      IVarZ=40
           scenarioWithSettingGroupSpec('sg6', opts('sg6')),
-          // id=sg7        id=sg7      (ERROR: setting with out-of-range value)
+          // id=sg7        id=sg7      (WARNING: setting with out-of-range value on both sides)
           //   IVarA=500    IVarA=500
           scenarioWithSettingGroupSpec('sg7', opts('sg7')),
-          // id=sg8        id=sg8      (ERROR: setting with out-of-range value on one side)
+          // id=sg8        id=sg8      (WARNING: setting with out-of-range value on one side, settings differ)
           //   IVarA=40     IVarA=500
           scenarioWithSettingGroupSpec('sg8', opts('sg8')),
           // id=sg9        id=sg9      (ERROR: setting group not found on either side)
@@ -479,14 +512,26 @@ describe('resolveComparisonSpecs', () => {
             undefined,
             opts('sg6')
           ),
+          // sg7: out-of-range on both sides; scenario runs at value 500 on each side
           scenarioWithInputs(
             '7',
-            [resolvedInput('IVarA', errInvalidValue, {}), resolvedInput('IVarA', {}, errInvalidValue)],
-            undefined,
-            undefined,
+            [
+              resolvedInput('IVarA', warnOutOfRange(lVar('IVarA'), 500), {}),
+              resolvedInput('IVarA', {}, warnOutOfRange(rVar('IVarA'), 500))
+            ],
+            atValSpec('_ivara', 500),
+            atValSpec('_ivara', 500),
             opts('sg7')
           ),
-          scenarioWithInputs('8', [resolvedInput('IVarA', {}, errInvalidValue)], undefined, undefined, opts('sg8')),
+          // sg8: out-of-range on right side only; scenario runs at value 40 on left and
+          // 500 on right, so settingsDiffer is true
+          scenarioWithInputs(
+            '8',
+            [resolvedInput('IVarA', {}, warnOutOfRange(rVar('IVarA'), 500))],
+            atValSpec('_ivara', 40),
+            atValSpec('_ivara', 500),
+            { ...opts('sg8'), settingsDiffer: true }
+          ),
           scenarioWithInputs(
             '9',
             [resolvedInput('sg9', errUnknownSettingGroup, errUnknownSettingGroup)],

packages/check-core/src/comparison/config/resolve/comparison-resolver.ts

@@ -489,18 +489,21 @@ function resolveScenarioForDistinctInputSpecs(
 ): ComparisonScenario {
   // TODO: Unlike the more typical "scenario with inputs" case, when we have "distinct"
   // inputs (separate sets of inputs for the two models) we only include the `settings`
-  // array in the resulting `ComparisonScenario` if there are errors in resolving the
-  // inputs.  If all inputs were resolved successfully, then the `settings` array will
-  // be empty.  This is probably fine for now but it could stand to be redesigned.
-  const inputsWithErrors: ComparisonScenarioInput[] = []
+  // array in the resulting `ComparisonScenario` for inputs that have an error or warning
+  // (so the UI can flag them).  If all inputs were resolved cleanly, the `settings` array
+  // will be empty.  This is probably fine for now but it could stand to be redesigned.
+  const inputsWithIssues: ComparisonScenarioInput[] = []
 
   // Resolve the input settings for the left and right sides separately
   const settingsL: InputSetting[] = []
   const settingsR: InputSetting[] = []
 
-  // Helper function that resolves an input for the given model/side.  If the input is
-  // resolved successfully, an `InputSetting` will be saved for that side.  Otherwise,
-  // a `ComparisonScenarioInput` describing the error will be saved.
+  // Helper function that resolves an input for the given model/side.  If the resolved
+  // input has no fatal `error`, an `InputSetting` will be saved for that side (this
+  // includes inputs with a non-fatal `warning`, e.g. out-of-range values, which the
+  // model can still be run with).  If the resolved input has a fatal `error` or a
+  // `warning`, a `ComparisonScenarioInput` describing the issue is also added to
+  // `inputsWithIssues` so the UI can annotate it.
   function resolveInputSpec(
     side: 'left' | 'right',
     modelInputs: ModelInputs,
@@ -518,19 +521,24 @@ function resolveScenarioForDistinctInputSpecs(
         assertNever(inputSpec)
     }
 
-    if (inputState.error !== undefined) {
-      // The input could not be resolved, so add it to the set of error inputs
+    if (inputState.error !== undefined || inputState.warning !== undefined) {
+      // The input has a fatal error or a non-fatal warning; record it so the UI can
+      // annotate it
       // TODO: For now we include an empty object (with undefined properties) for the
       // "other" side.  Maybe we can make the state properties optional, or maybe we
       // just need a less awkward way of handling these input states in the "distinct"
       // inputs case.
-      inputsWithErrors.push({
+      inputsWithIssues.push({
         requestedName: inputSpec.inputName,
         stateL: side === 'left' ? inputState : {},
         stateR: side === 'right' ? inputState : {}
       })
-    } else {
-      // The input was resolved, so create a scenario that works for this side
+    }
+
+    if (inputState.error === undefined && inputState.inputVar !== undefined) {
+      // The input was resolved (possibly with a warning), so create a scenario setting
+      // that works for this side.  Warnings are non-fatal — the model still runs with
+      // the requested (possibly out-of-range) value.
       const inputSetting = inputSettingFromResolvedInputState(inputState)
       if (side === 'left') {
         settingsL.push(inputSetting)
@@ -544,10 +552,14 @@ function resolveScenarioForDistinctInputSpecs(
   inputSpecsL.forEach(inputSpec => resolveInputSpec('left', modelInputsL, inputSpec))
   inputSpecsR.forEach(inputSpec => resolveInputSpec('right', modelInputsR, inputSpec))
 
-  // Create a `ScenarioSpec` for each side if there were no errors
+  // Create a `ScenarioSpec` for each side if there were no fatal errors.  Warnings on
+  // their own do not prevent spec construction.
+  const hasFatalError = inputsWithIssues.some(
+    input => input.stateL.error !== undefined || input.stateR.error !== undefined
+  )
   let specL: ScenarioSpec
   let specR: ScenarioSpec
-  if (inputsWithErrors.length === 0) {
+  if (!hasFatalError) {
     specL = inputSettingsSpec(settingsL)
     specR = inputSettingsSpec(settingsR)
   }
@@ -561,7 +573,7 @@ function resolveScenarioForDistinctInputSpecs(
     subtitle,
     settings: {
       kind: 'input-settings',
-      inputs: inputsWithErrors
+      inputs: inputsWithIssues
     },
     specL,
     specR
@@ -794,6 +806,11 @@ function resolveInputVarAtPosition(inputVar: InputVar, position: InputPosition):
 
 /**
  * Return a `ComparisonScenarioInputState` for the input at the given value.
+ *
+ * If the value is out of range for the input variable, the returned state
+ * still includes the resolved `inputVar` and the requested `value` (so the
+ * scenario can still run with that value) and attaches a non-fatal
+ * `value-out-of-range` warning that consumers can surface as an annotation.
  */
 function resolveInputVarAtValue(inputVar: InputVar, value: number): ComparisonScenarioInputState {
   let inRange: boolean
@@ -814,8 +831,10 @@ function resolveInputVarAtValue(inputVar: InputVar, value: number): ComparisonSc
     }
   } else {
     return {
-      error: {
-        kind: 'invalid-value'
+      inputVar,
+      value,
+      warning: {
+        kind: 'value-out-of-range'
       }
     }
   }

packages/check-core/src/comparison/config/resolve/comparison-scenario-specs.ts

@@ -52,8 +52,11 @@ function scenarioSpecFromInputs(inputs: ComparisonScenarioInput[], side: 'left'
   for (const input of inputs) {
     const state = side === 'left' ? input.stateL : input.stateR
 
-    // If any inputs on this side could not be resolved, return undefined so that we don't try
-    // to fetch data for this side
+    // If any inputs on this side could not be resolved (i.e. fatal error like an
+    // unknown input variable), return undefined so that we don't try to fetch data
+    // for this side.  Note that non-fatal warnings (e.g. out-of-range values) still
+    // produce a resolved `inputVar`, so the scenario can still run with the
+    // requested value in that case.
     if (state.inputVar === undefined) {
       return undefined
     }