From a2cc072a570fcdc5d8cc7a211fbf5047ad778848 Mon Sep 17 00:00:00 2001 From: Chris Campbell Date: Wed, 27 May 2026 13:44:28 -0700 Subject: [PATCH] fix: treat out-of-range values in comparison tests as a warning instead of error --- .../_shared/comparison-resolved-types.ts | 36 +++++- .../resolve/comparison-resolver.spec.ts | 105 +++++++++++++----- .../config/resolve/comparison-resolver.ts | 53 ++++++--- .../resolve/comparison-scenario-specs.ts | 7 +- .../report/comparison-grouping.spec.ts | 67 ++++++++--- .../compare/_shared/annotations.spec.ts | 54 +++++++++ .../components/compare/_shared/annotations.ts | 98 ++++++++++------ 7 files changed, 313 insertions(+), 107 deletions(-) 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