diff --git a/packages/check-core/src/comparison/_shared/comparison-resolved-types.ts b/packages/check-core/src/comparison/_shared/comparison-resolved-types.ts
index 178b879d..e7019823 100644
--- a/packages/check-core/src/comparison/_shared/comparison-resolved-types.ts
+++ b/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. */
diff --git a/packages/check-core/src/comparison/config/resolve/comparison-resolver.spec.ts b/packages/check-core/src/comparison/config/resolve/comparison-resolver.spec.ts
index 6888818f..4aff7173 100644
--- a/packages/check-core/src/comparison/config/resolve/comparison-resolver.spec.ts
+++ b/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)],
diff --git a/packages/check-core/src/comparison/config/resolve/comparison-resolver.ts b/packages/check-core/src/comparison/config/resolve/comparison-resolver.ts
index cf5568ea..6229111d 100644
--- a/packages/check-core/src/comparison/config/resolve/comparison-resolver.ts
+++ b/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'
}
}
}
diff --git a/packages/check-core/src/comparison/config/resolve/comparison-scenario-specs.ts b/packages/check-core/src/comparison/config/resolve/comparison-scenario-specs.ts
index e07f04d5..670464be 100644
--- a/packages/check-core/src/comparison/config/resolve/comparison-scenario-specs.ts
+++ b/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
}
diff --git a/packages/check-core/src/comparison/report/comparison-grouping.spec.ts b/packages/check-core/src/comparison/report/comparison-grouping.spec.ts
index 686605d1..546afb99 100644
--- a/packages/check-core/src/comparison/report/comparison-grouping.spec.ts
+++ b/packages/check-core/src/comparison/report/comparison-grouping.spec.ts
@@ -4,12 +4,18 @@ import { describe, expect, it } from 'vitest'
import type { DatasetKey } from '../../_shared/types'
import type {
- ComparisonResolverInvalidValueError,
ComparisonResolverUnknownInputError,
ComparisonScenario,
ComparisonScenarioKey
} from '../_shared/comparison-resolved-types'
-import { allAtPos, inputVar, scenarioWithInput, scenarioWithInputVar } from '../_shared/_mocks/mock-resolved-types'
+import {
+ allAtPos,
+ inputVar,
+ scenarioWithInput,
+ scenarioWithInputVar,
+ scenarioWithInputs
+} from '../_shared/_mocks/mock-resolved-types'
+import { inputAtValueSpec } from '../../_shared/scenario-specs'
import type { BundleModel, LoadedBundle, ModelSpec } from '../../bundle/bundle-types'
import type { OutputVar } from '../../bundle/var-types'
@@ -241,10 +247,20 @@ describe('categorizeComparisonGroups', () => {
const d: ComparisonResolverUnknownInputError = { kind: 'unknown-input' }
const dAtMax = scenarioWithInput('7', 'd', 'at-maximum', d, d, undefined, undefined)
- // In this scenario, the input is valid, but the value is out of range, so the scenario should
- // be invalid for both sides
- const bInvalid: ComparisonResolverInvalidValueError = { kind: 'invalid-value' }
- const bAt666 = scenarioWithInput('8', 'b', 666, bInvalid, bInvalid, undefined, undefined)
+ // In this scenario, the input is valid but the value is out of range; this is a non-fatal
+ // warning so the scenario still runs (with the requested out-of-range value) on both sides
+ const bAt666 = scenarioWithInputs(
+ '8',
+ [
+ {
+ requestedName: 'b',
+ stateL: { inputVar: b, value: 666, warning: { kind: 'value-out-of-range' } },
+ stateR: { inputVar: b, value: 666, warning: { kind: 'value-out-of-range' } }
+ }
+ ],
+ inputAtValueSpec('_b', 666),
+ inputAtValueSpec('_b', 666)
+ )
const scenarios = [baseline, aAtMin, aAtMax, bAtMin, bAtMax, bAt20, dAtMax, bAt666]
@@ -258,9 +274,10 @@ describe('categorizeComparisonGroups', () => {
// - a at {min,max}
// - b at {min,max}
// - b at 20
- // - 2 invalid scenarios:
- // - d at max (input is unknown for both sides)
- // - b at 666 (value is invalid for both sides)
+ // - 1 invalid scenario:
+ // - d at max (input is unknown for both sides; fatal error, scenario does not run)
+ // - 1 valid scenario with a non-fatal warning:
+ // - b at 666 (value is out of range for both sides; scenario still runs)
const allSummaries = [
testSummary(dAtMax, w),
testSummary(dAtMax, x),
@@ -336,7 +353,10 @@ describe('categorizeComparisonGroups', () => {
const groupsByScenario = groupComparisonTestSummaries(allSummaries, 'by-scenario')
const groupSummaries = categorizeComparisonGroups(comparisonConfig, [...groupsByScenario.values()], 'max-diff')
- // Verify that scenarios with unknown inputs and invalid values get grouped into the "with errors" category
+ // Verify that scenarios with unknown inputs (fatal error, scenario does not run) get
+ // grouped into the "with errors" category. Scenarios with out-of-range values are
+ // non-fatal warnings: the scenario still runs, so it gets grouped with the other
+ // valid scenarios (see `withoutDiffs` assertion below).
expect(groupSummaries.withErrors).toEqual([
groupSummary('7', [
testSummary(dAtMax, w),
@@ -344,13 +364,6 @@ describe('categorizeComparisonGroups', () => {
testSummary(dAtMax, y),
testSummary(dAtMax, z),
testSummary(dAtMax, v)
- ]),
- groupSummary('8', [
- testSummary(bAt666, w),
- testSummary(bAt666, x),
- testSummary(bAt666, y),
- testSummary(bAt666, z),
- testSummary(bAt666, v)
])
])
@@ -426,6 +439,26 @@ describe('categorizeComparisonGroups', () => {
])
expect(groupSummaries.withoutDiffs).toEqual([
+ // b at 666 carries a non-fatal `value-out-of-range` warning but still runs; its
+ // group iteration order places it ahead of the baseline since the dAtMax summaries
+ // (key '7') are inserted before the bAt666 summaries (key '8') which are inserted
+ // before the baseline summaries (key '1').
+ groupSummary(
+ '8',
+ [
+ testSummary(bAt666, w),
+ testSummary(bAt666, x),
+ testSummary(bAt666, y),
+ testSummary(bAt666, z),
+ testSummary(bAt666, v)
+ ],
+ {
+ totalDiffCount: 5,
+ totalDiffByBucket: [0, 0, 0, 0, 0, 0],
+ diffCountByBucket: [5, 0, 0, 0, 0, 0],
+ diffPercentByBucket: [100, 0, 0, 0, 0, 0]
+ }
+ ),
groupSummary(
'1',
[
diff --git a/packages/check-ui-shell/src/components/compare/_shared/annotations.spec.ts b/packages/check-ui-shell/src/components/compare/_shared/annotations.spec.ts
index 744ec68a..354e4c5d 100644
--- a/packages/check-ui-shell/src/components/compare/_shared/annotations.spec.ts
+++ b/packages/check-ui-shell/src/components/compare/_shared/annotations.spec.ts
@@ -213,6 +213,60 @@ describe('getAnnotationsForScenario', () => {
])
})
+ it('should return correct annotation when value is out of range on both sides', () => {
+ const i1 = inputVar('1', 'Input1')[1]
+ const spec = inputAtPositionSpec('uid1', '_input1', 'at-maximum')
+ const s: ComparisonScenario = {
+ kind: 'scenario',
+ key: 's1',
+ id: 'sg1',
+ title: 'sg1',
+ settings: {
+ kind: 'input-settings',
+ inputs: [
+ {
+ requestedName: 'i1',
+ stateL: { inputVar: i1, value: 500, warning: { kind: 'value-out-of-range' } },
+ stateR: { inputVar: i1, value: 500, warning: { kind: 'value-out-of-range' } }
+ }
+ ]
+ },
+ specL: spec,
+ specR: spec
+ }
+ const annotations = getAnnotationsForScenario(s, bundleNameL, bundleNameR)
+ expect(annotations).toEqual([
+ `‼ warning: value out of range for 'i1'`
+ ])
+ })
+
+ it('should return correct annotation when value is out of range on one side', () => {
+ const i1 = inputVar('1', 'Input1')[1]
+ const spec = inputAtPositionSpec('uid1', '_input1', 'at-maximum')
+ const s: ComparisonScenario = {
+ kind: 'scenario',
+ key: 's1',
+ id: 'sg1',
+ title: 'sg1',
+ settings: {
+ kind: 'input-settings',
+ inputs: [
+ {
+ requestedName: 'i1',
+ stateL: { inputVar: i1, value: 90 },
+ stateR: { inputVar: i1, value: 500, warning: { kind: 'value-out-of-range' } }
+ }
+ ]
+ },
+ specL: spec,
+ specR: spec
+ }
+ const annotations = getAnnotationsForScenario(s, bundleNameL, bundleNameR)
+ expect(annotations).toEqual([
+ `‼ warning for current: value out of range for 'i1'`
+ ])
+ })
+
it('should return correct annotation when settings differ between the two models', () => {
const s: ComparisonScenario = {
kind: 'scenario',
diff --git a/packages/check-ui-shell/src/components/compare/_shared/annotations.ts b/packages/check-ui-shell/src/components/compare/_shared/annotations.ts
index e9c0dfb7..6d682462 100644
--- a/packages/check-ui-shell/src/components/compare/_shared/annotations.ts
+++ b/packages/check-ui-shell/src/components/compare/_shared/annotations.ts
@@ -45,26 +45,36 @@ export function getAnnotationsForScenario(
return []
}
- type InputErrorKind = 'unknown-input' | 'unknown-input-setting-group' | 'invalid-value'
+ type InputErrorKind = 'unknown-input' | 'unknown-input-setting-group'
interface InputError {
requestedName: string
kind: InputErrorKind
}
+ type InputWarningKind = 'value-out-of-range'
+ interface InputWarning {
+ requestedName: string
+ kind: InputWarningKind
+ }
const errorsInBoth: InputError[] = []
const errorsInL: InputError[] = []
const errorsInR: InputError[] = []
+ const warningsInBoth: InputWarning[] = []
+ const warningsInL: InputWarning[] = []
+ const warningsInR: InputWarning[] = []
for (const input of scenario.settings.inputs) {
- const kindL = input.stateL.error?.kind
- const kindR = input.stateR.error?.kind
+ const errKindL = input.stateL.error?.kind
+ const errKindR = input.stateR.error?.kind
+ const warnKindL = input.stateL.warning?.kind
+ const warnKindR = input.stateR.warning?.kind
- const uisgL = kindL === 'unknown-input-setting-group'
- const uisgR = kindR === 'unknown-input-setting-group'
+ const uisgL = errKindL === 'unknown-input-setting-group'
+ const uisgR = errKindR === 'unknown-input-setting-group'
- const uiL = kindL === 'unknown-input'
- const uiR = kindR === 'unknown-input'
+ const uiL = errKindL === 'unknown-input'
+ const uiR = errKindR === 'unknown-input'
- const ivL = kindL === 'invalid-value'
- const ivR = kindR === 'invalid-value'
+ const oorL = warnKindL === 'value-out-of-range'
+ const oorR = warnKindR === 'value-out-of-range'
if (uisgL || uisgR) {
const err: InputError = { requestedName: input.requestedName, kind: 'unknown-input-setting-group' }
@@ -86,62 +96,80 @@ export function getAnnotationsForScenario(
errorsInR.push(err)
}
}
- if (ivL || ivR) {
- const err: InputError = { requestedName: input.requestedName, kind: 'invalid-value' }
- if (ivL && ivR) {
- errorsInBoth.push(err)
- } else if (ivL) {
- errorsInL.push(err)
- } else if (ivR) {
- errorsInR.push(err)
+ if (oorL || oorR) {
+ const warn: InputWarning = { requestedName: input.requestedName, kind: 'value-out-of-range' }
+ if (oorL && oorR) {
+ warningsInBoth.push(warn)
+ } else if (oorL) {
+ warningsInL.push(warn)
+ } else if (oorR) {
+ warningsInR.push(warn)
}
}
}
// unknown inputs 'X', 'Y'
// unknown input setting group 'Z'
- // value out of range for 'X', 'Y'
function messageForErrorKind(errors: InputError[], kind: InputErrorKind): string | undefined {
const inputs = errors.filter(e => e.kind === kind).map(e => `'${e.requestedName}'`)
if (inputs.length === 0) {
return undefined
+ } else if (kind === 'unknown-input') {
+ const subject = inputs.length === 1 ? 'input' : 'inputs'
+ return `unknown ${subject} ${inputs.join(', ')}`
} else {
- if (kind === 'unknown-input') {
- const subject = inputs.length === 1 ? 'input' : 'inputs'
- return `unknown ${subject} ${inputs.join(', ')}`
- } else if (kind === 'unknown-input-setting-group') {
- return `unknown input setting group ${inputs[0]}`
- } else {
- return `value out of range for ${inputs.join(', ')}`
- }
+ return `unknown input setting group ${inputs[0]}`
}
}
- // If there are "unknown input" errors, those take precedence over other errors like
- // "invalid value"
- function message(errors: InputError[]): string {
+ // If there are "unknown input setting group" errors, those take precedence over other
+ // errors like "unknown input"
+ function errorMessage(errors: InputError[]): string {
const parts = [
messageForErrorKind(errors, 'unknown-input-setting-group'),
- messageForErrorKind(errors, 'unknown-input'),
- messageForErrorKind(errors, 'invalid-value')
+ messageForErrorKind(errors, 'unknown-input')
]
return parts.filter(p => p !== undefined).join('; ')
}
- // If there are any errors in both, those take precendence over errors in one side only
+ // value out of range for 'X', 'Y'
+ function warningMessage(warnings: InputWarning[]): string {
+ const inputs = warnings.map(w => `'${w.requestedName}'`)
+ return `value out of range for ${inputs.join(', ')}`
+ }
+
+ // If there are any fatal errors in both, those take precedence over errors in one side
+ // only:
// invalid scenario: {inputMessages}
// scenario not valid in {left}: {inputMessages}
// scenario not valid in {right}: {inputMessages}
if (errorsInBoth.length > 0) {
- annotations.push(annotationSpan('err', `invalid scenario: ${message(errorsInBoth)}`))
+ annotations.push(annotationSpan('err', `invalid scenario: ${errorMessage(errorsInBoth)}`))
}
if (errorsInL.length > 0) {
const firstPart = `scenario not valid in ${datasetSpan(bundleNameL, 'left')}`
- annotations.push(annotationSpan('warn', `${firstPart}: ${message(errorsInL)}`))
+ annotations.push(annotationSpan('warn', `${firstPart}: ${errorMessage(errorsInL)}`))
}
if (errorsInR.length > 0) {
const firstPart = `scenario not valid in ${datasetSpan(bundleNameR, 'right')}`
- annotations.push(annotationSpan('warn', `${firstPart}: ${message(errorsInR)}`))
+ annotations.push(annotationSpan('warn', `${firstPart}: ${errorMessage(errorsInR)}`))
+ }
+
+ // Non-fatal warnings: the scenario still runs, so always render as yellow. If there are
+ // any warnings in both, those take precedence over errors in one side only:
+ // warning: {inputMessages}
+ // warning for {left}: {inputMessages}
+ // warning for {right}: {inputMessages}
+ if (warningsInBoth.length > 0) {
+ annotations.push(annotationSpan('warn', `warning: ${warningMessage(warningsInBoth)}`))
+ }
+ if (warningsInL.length > 0) {
+ const firstPart = `warning for ${datasetSpan(bundleNameL, 'left')}`
+ annotations.push(annotationSpan('warn', `${firstPart}: ${warningMessage(warningsInL)}`))
+ }
+ if (warningsInR.length > 0) {
+ const firstPart = `warning for ${datasetSpan(bundleNameR, 'right')}`
+ annotations.push(annotationSpan('warn', `${firstPart}: ${warningMessage(warningsInR)}`))
}
// Add a warning if the settings differ between the two models