From bb8685605046b135b6a6d3c9ba65a2fb59269b71 Mon Sep 17 00:00:00 2001 From: Jakob Langdal Date: Thu, 22 Jan 2026 16:03:36 +0100 Subject: [PATCH] Move change calculation into separate reducer --- .changeset/silent-feet-breathe.md | 5 + .../calculate-change-reducer.test.ts | 131 ++++++++++++++++++ .../experiment/calculate-change-reducer.ts | 10 ++ .../context/experiment/experiment-reducers.ts | 3 - .../src/context/experiment/reducers.test.ts | 12 +- .../core/src/context/experiment/reducers.ts | 5 +- 6 files changed, 161 insertions(+), 5 deletions(-) create mode 100644 .changeset/silent-feet-breathe.md create mode 100644 packages/core/src/context/experiment/calculate-change-reducer.test.ts create mode 100644 packages/core/src/context/experiment/calculate-change-reducer.ts diff --git a/.changeset/silent-feet-breathe.md b/.changeset/silent-feet-breathe.md new file mode 100644 index 00000000..542b3c3e --- /dev/null +++ b/.changeset/silent-feet-breathe.md @@ -0,0 +1,5 @@ +--- +'@boostv/process-optimizer-frontend-core': patch +--- + +Fix a 'race condition' in reducer where changes were not detected when a datapoint changes from invalid to valid diff --git a/packages/core/src/context/experiment/calculate-change-reducer.test.ts b/packages/core/src/context/experiment/calculate-change-reducer.test.ts new file mode 100644 index 00000000..756bb0df --- /dev/null +++ b/packages/core/src/context/experiment/calculate-change-reducer.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest' +import { ExperimentType } from 'common' +import { calculateChangeReducer } from './calculate-change-reducer' +import { createFetchExperimentResultRequest } from './api' +import { emptyExperiment } from './store' +import md5 from 'md5' + +describe('calculateChangeReducer', () => { + it('should set changedSinceLastEvaluation to false when hash matches', () => { + const experiment: ExperimentType = { + ...emptyExperiment, + id: 'test-id', + } + const hash = md5( + JSON.stringify(createFetchExperimentResultRequest(experiment)) + ) + const experimentWithHash: ExperimentType = { + ...experiment, + lastEvaluationHash: hash, + } + + const result = calculateChangeReducer(experimentWithHash) + + expect(result.changedSinceLastEvaluation).toBe(false) + }) + + it('should set changedSinceLastEvaluation to true when hash does not match', () => { + const experiment: ExperimentType = { + ...emptyExperiment, + id: 'test-id', + lastEvaluationHash: 'outdated-hash', + } + + const result = calculateChangeReducer(experiment) + + expect(result.changedSinceLastEvaluation).toBe(true) + }) + + it('should set changedSinceLastEvaluation to true when lastEvaluationHash is empty', () => { + const experiment: ExperimentType = { + ...emptyExperiment, + id: 'test-id', + lastEvaluationHash: '', + } + + const result = calculateChangeReducer(experiment) + + expect(result.changedSinceLastEvaluation).toBe(true) + }) + + it('should detect change when optimizer config changes', () => { + const baseExperiment: ExperimentType = { + ...emptyExperiment, + id: 'test-id', + } + const hash = md5( + JSON.stringify(createFetchExperimentResultRequest(baseExperiment)) + ) + const experimentWithHash: ExperimentType = { + ...baseExperiment, + lastEvaluationHash: hash, + } + + expect( + calculateChangeReducer(experimentWithHash).changedSinceLastEvaluation + ).toBe(false) + + const modifiedExperiment: ExperimentType = { + ...experimentWithHash, + optimizerConfig: { + ...experimentWithHash.optimizerConfig, + initialPoints: 10, + }, + } + + const result = calculateChangeReducer(modifiedExperiment) + + expect(result.changedSinceLastEvaluation).toBe(true) + }) + + it('should detect change when variables are added', () => { + const baseExperiment: ExperimentType = { + ...emptyExperiment, + id: 'test-id', + } + const hash = md5( + JSON.stringify(createFetchExperimentResultRequest(baseExperiment)) + ) + const experimentWithHash: ExperimentType = { + ...baseExperiment, + lastEvaluationHash: hash, + } + + expect( + calculateChangeReducer(experimentWithHash).changedSinceLastEvaluation + ).toBe(false) + + const modifiedExperiment: ExperimentType = { + ...experimentWithHash, + valueVariables: [ + { + type: 'continuous', + name: 'TestVar', + description: 'Test variable', + min: 0, + max: 100, + enabled: true, + }, + ], + } + + const result = calculateChangeReducer(modifiedExperiment) + + expect(result.changedSinceLastEvaluation).toBe(true) + }) + + it('should not mutate the original experiment object', () => { + const experiment: ExperimentType = { + ...emptyExperiment, + id: 'test-id', + changedSinceLastEvaluation: false, + lastEvaluationHash: 'some-hash', + } + + const result = calculateChangeReducer(experiment) + + expect(result).not.toBe(experiment) + expect(experiment.changedSinceLastEvaluation).toBe(false) + expect(result.changedSinceLastEvaluation).toBe(true) + }) +}) diff --git a/packages/core/src/context/experiment/calculate-change-reducer.ts b/packages/core/src/context/experiment/calculate-change-reducer.ts new file mode 100644 index 00000000..d442a5b9 --- /dev/null +++ b/packages/core/src/context/experiment/calculate-change-reducer.ts @@ -0,0 +1,10 @@ +import { ExperimentType } from 'common' +import { produce } from 'immer' +import md5 from 'md5' +import { createFetchExperimentResultRequest } from './api' + +export const calculateChangeReducer = produce((state: ExperimentType): void => { + state.changedSinceLastEvaluation = + state.lastEvaluationHash !== + md5(JSON.stringify(createFetchExperimentResultRequest(state))) +}) diff --git a/packages/core/src/context/experiment/experiment-reducers.ts b/packages/core/src/context/experiment/experiment-reducers.ts index 41048e39..9e78c7c2 100644 --- a/packages/core/src/context/experiment/experiment-reducers.ts +++ b/packages/core/src/context/experiment/experiment-reducers.ts @@ -458,9 +458,6 @@ export const experimentReducer = produce( default: assertUnreachable(action) } - state.changedSinceLastEvaluation = - state.lastEvaluationHash !== - md5(JSON.stringify(createFetchExperimentResultRequest(state))) state.info.version = state.info.version + 1 } ) diff --git a/packages/core/src/context/experiment/reducers.test.ts b/packages/core/src/context/experiment/reducers.test.ts index 9179750e..e6c14577 100644 --- a/packages/core/src/context/experiment/reducers.test.ts +++ b/packages/core/src/context/experiment/reducers.test.ts @@ -10,7 +10,11 @@ import { scoreNames, ValueVariableType, } from '@core/common/types' -import { emptyExperiment, State } from '@core/context/experiment' +import { + createFetchExperimentResultRequest, + emptyExperiment, + State, +} from '@core/context/experiment' import { versionInfo } from '@core/common' import _ from 'lodash' import { produce } from 'immer' @@ -21,6 +25,7 @@ import { createValueVariable, dummyPayloads, } from '@core/context/experiment/test-utils' +import md5 from 'md5' describe('experiment reducer', () => { const initState: State = { @@ -116,6 +121,7 @@ describe('experiment reducer', () => { const payload: ExperimentType = { id: '5678', changedSinceLastEvaluation: false, + lastEvaluationHash: '5b4247caaa4e9a5a0c519a9017ccc547', info: { swVersion: versionInfo.version, name: 'Not cake', @@ -169,6 +175,10 @@ describe('experiment reducer', () => { type: 'updateExperiment', payload, } + const hash = md5( + JSON.stringify(createFetchExperimentResultRequest(payload)) + ) + expect(hash).toEqual(payload.lastEvaluationHash) const actual = rootReducer(initState, action) expect(actual).toEqual({ diff --git a/packages/core/src/context/experiment/reducers.ts b/packages/core/src/context/experiment/reducers.ts index 1e969290..b47f7329 100644 --- a/packages/core/src/context/experiment/reducers.ts +++ b/packages/core/src/context/experiment/reducers.ts @@ -3,6 +3,7 @@ import { State } from './store' import { ExperimentAction, experimentReducer } from './experiment-reducers' import { validateExperiment, ValidationViolations } from './validation' import { validationReducer } from './validation-reducer' +import { calculateChangeReducer } from './calculate-change-reducer' export type Action = ExperimentAction @@ -36,7 +37,9 @@ export const rootReducer = (state: State, action: Action) => { validateExperiment(experiment) return { ...state, - experiment: validationReducer(experiment, validationViolations), + experiment: calculateChangeReducer( + validationReducer(experiment, validationViolations) + ), } } default: