From 363c0fc15d9d79eb1fa5c385df4d115c7f0b800e Mon Sep 17 00:00:00 2001 From: Jakob Langdal Date: Thu, 2 Jul 2026 21:21:42 +0000 Subject: [PATCH 1/2] fix(core): don't reset pareto selection when an unscored data point is added MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transferring a pareto point or suggestion to the data table appends an unscored (valid: false) row, which is excluded from the optimizer request and cannot move the front — yet the blanket invalidation policy cleared extras.selectedPoint on any dataPoints change, snapping the UI back to the default trade-off and (because the selection is part of the hashed request) triggering a needless re-evaluation. Move the invalidation post-validation into rootReducer (invalidateStaleParetoSelection) and compare dataPoints by their active (valid + enabled) subset — the same filter calculateData applies. Rows becoming active (score entered), removals, and variable/config changes still clear the selection as before. --- .changeset/pareto-selection-unscored-rows.md | 5 + .../experiment/experiment-reducers.test.ts | 165 ++++++++++++------ .../context/experiment/experiment-reducers.ts | 46 +++-- .../core/src/context/experiment/reducers.ts | 10 +- 4 files changed, 156 insertions(+), 70 deletions(-) create mode 100644 .changeset/pareto-selection-unscored-rows.md diff --git a/.changeset/pareto-selection-unscored-rows.md b/.changeset/pareto-selection-unscored-rows.md new file mode 100644 index 00000000..00b1b623 --- /dev/null +++ b/.changeset/pareto-selection-unscored-rows.md @@ -0,0 +1,5 @@ +--- +'@boostv/process-optimizer-frontend-core': patch +--- + +Keep the pareto front selection when an unscored data point is added. Transferring a pareto point ("Add as data point") or a suggestion to the data table appends a valid:false row that is excluded from the optimizer request and cannot move the front — it no longer clears the selection or triggers a re-evaluation. Rows becoming active (score entered), removals, and variable/config changes still invalidate the selection. diff --git a/packages/core/src/context/experiment/experiment-reducers.test.ts b/packages/core/src/context/experiment/experiment-reducers.test.ts index 9f915c0f..cc6d4590 100644 --- a/packages/core/src/context/experiment/experiment-reducers.test.ts +++ b/packages/core/src/context/experiment/experiment-reducers.test.ts @@ -1,83 +1,134 @@ import { describe, expect, it } from 'vitest' -import { experimentReducer } from './experiment-reducers' -import { emptyExperiment, initialState } from './store' import { produce } from 'immer' +import md5 from 'md5' +import { rootReducer } from './reducers' +import { emptyExperiment } from './store' +import { State } from './store' +import { createFetchExperimentResultRequest } from './api' +import { settings } from '@core/common' +import { DataEntry, scoreNames } from '@core/common/types' -describe('experimentReducer pareto selection invalidation', () => { - const withSelection = produce(initialState.experiment, draft => { - draft.extras.selectedPoint = [1, 2, 'Red'] +// A row with a score entered — active (valid + enabled) once validated. +const scoredRow = (id: number, x: number, score: number): DataEntry => ({ + meta: { id, enabled: true, valid: true }, + data: [ + { type: 'numeric', name: 'x', value: x }, + { type: 'score', name: scoreNames[0] ?? 'score', value: score }, + ], +}) + +// An unscored row, as produced by transferring a suggestion or a pareto point +// to the data table — validation keeps it invalid until a score is entered. +const unscoredRow = (id: number, x: number): DataEntry => ({ + meta: { id, enabled: true, valid: false }, + data: [{ type: 'numeric', name: 'x', value: x }], +}) + +// A fitted (post-initialization) state with a stored pareto selection whose +// last evaluation matches the current request hash — i.e. the idle state right +// after an evaluation, before the user acts. +const evaluatedState = (): State => { + const experiment = produce(emptyExperiment, draft => { + draft.id = 'exp' + draft.info.version = 2 + draft.valueVariables = [ + { + type: 'continuous', + name: 'x', + description: '', + min: 0, + max: 10, + enabled: true, + }, + ] + draft.optimizerConfig.initialPoints = 1 + // xi as the last updateDataPoints pass would have left it (best score 3) + draft.optimizerConfig.xi = Math.max(0.1, settings.maxRating - 3) + draft.dataPoints = [scoredRow(1, 5, 3)] + draft.results.next = [[6]] as unknown as typeof draft.results.next + draft.extras.selectedPoint = [1.5] + }) + const evaluated = produce(experiment, draft => { + draft.lastEvaluationHash = md5( + JSON.stringify(createFetchExperimentResultRequest(experiment)) + ) + draft.changedSinceLastEvaluation = false }) + return { experiment: evaluated } +} - it('clears selectedPoint when a structural action changes a structural field', () => { - // updateDataPoints mutates state.dataPoints — a structural key - const next = experimentReducer(withSelection, { +describe('pareto selection invalidation policy', () => { + it('keeps the selection and does not flag re-evaluation when an unscored row is appended (add-as-data-point)', () => { + // The row is excluded from the optimizer request (meta.valid: false), so it + // cannot move the front: transferring a pareto point to the data table must + // leave the selection and the evaluation state untouched. + const state = evaluatedState() + const actual = rootReducer(state, { type: 'updateDataPoints', - payload: [], + payload: [...state.experiment.dataPoints, unscoredRow(2, 7)], }) - expect('selectedPoint' in next.extras).toBe(false) + expect(actual.experiment.extras.selectedPoint).toEqual([1.5]) + expect(actual.experiment.changedSinceLastEvaluation).toBe(false) }) - it('keeps selectedPoint when setSelectedParetoPoint sets it', () => { - const next = experimentReducer(initialState.experiment, { - type: 'setSelectedParetoPoint', - payload: [1, 2, 'Red'], + it('keeps the selection when a suggestion is transferred via copySuggestedToDataPoints', () => { + const state = evaluatedState() + const actual = rootReducer(state, { + type: 'copySuggestedToDataPoints', + payload: { indices: [0], removeFromSuggestions: false }, }) - expect(next.extras.selectedPoint).toEqual([1, 2, 'Red']) + expect(actual.experiment.extras.selectedPoint).toEqual([1.5]) + expect(actual.experiment.changedSinceLastEvaluation).toBe(false) }) - it('keeps selectedPoint across a non-structural action', () => { - // updateExperimentName only mutates state.info.name — no structural field touched - const next = experimentReducer(withSelection, { - type: 'updateExperimentName', - payload: 'New Name', + it('clears the selection when a row becomes active (score entered)', () => { + const state = evaluatedState() + const withUnscored = produce(state, draft => { + draft.experiment.dataPoints.push(unscoredRow(2, 7)) + }) + const actual = rootReducer(withUnscored, { + type: 'updateDataPoints', + payload: withUnscored.experiment.dataPoints.map(dp => + dp.meta.id === 2 ? scoredRow(2, 7, 4) : dp + ), }) - expect(next.extras.selectedPoint).toEqual([1, 2, 'Red']) + expect('selectedPoint' in actual.experiment.extras).toBe(false) + expect(actual.experiment.changedSinceLastEvaluation).toBe(true) }) - it('updateExperiment clears selectedPoint (whole experiment replaced)', () => { - // updateExperiment replaces the entire experiment, which changes all - // structural fields — the policy must catch it even though it was not among - // the original 11 per-case clears. - const next = experimentReducer(withSelection, { + it('clears the selection when active data points change (row removed)', () => { + const state = evaluatedState() + const actual = rootReducer(state, { + type: 'updateDataPoints', + payload: [], + }) + expect('selectedPoint' in actual.experiment.extras).toBe(false) + }) + + it('clears the selection when the experiment is replaced (updateExperiment)', () => { + const state = evaluatedState() + const actual = rootReducer(state, { type: 'updateExperiment', payload: emptyExperiment, }) - expect('selectedPoint' in next.extras).toBe(false) + expect('selectedPoint' in actual.experiment.extras).toBe(false) }) - it('copySuggestedToDataPoints clears selectedPoint (appends data points)', () => { - // Build a state with one enabled numeric variable and one suggested next - // point — the minimum precondition for copySuggestedToDataPoints to push a - // new DataEntry. This action was not among the original 11 per-case clears - // and previously left a stale selection; the uniform policy fixes that. - const stateWithSuggestion = produce(emptyExperiment, draft => { - draft.extras.selectedPoint = [1, 2, 'Red'] - draft.valueVariables = [ - { - type: 'continuous', - name: 'x', - description: '', - min: 0, - max: 10, - enabled: true, - }, - ] - // results.next holds the suggested point values; index 0 selected via payload [0] - draft.results.next = [42] as unknown as typeof draft.results.next - }) - const next = experimentReducer(stateWithSuggestion, { - type: 'copySuggestedToDataPoints', - payload: { indices: [0], removeFromSuggestions: false }, + it('does not self-invalidate on setSelectedParetoPoint', () => { + const state = evaluatedState() + const actual = rootReducer(state, { + type: 'setSelectedParetoPoint', + payload: [2.5], }) - expect('selectedPoint' in next.extras).toBe(false) + expect(actual.experiment.extras.selectedPoint).toEqual([2.5]) }) - it('setSelectedParetoPoint with null removes selectedPoint (deselect path)', () => { - // null payload exercises the delete branch and must not leave the key present - const next = experimentReducer(withSelection, { - type: 'setSelectedParetoPoint', - payload: null, + it('keeps the selection across a non-structural action', () => { + const state = evaluatedState() + const actual = rootReducer(state, { + type: 'updateExperimentName', + payload: 'New name', }) - expect('selectedPoint' in next.extras).toBe(false) + expect(actual.experiment.extras.selectedPoint).toEqual([1.5]) }) }) diff --git a/packages/core/src/context/experiment/experiment-reducers.ts b/packages/core/src/context/experiment/experiment-reducers.ts index 76ea6656..e9c38278 100644 --- a/packages/core/src/context/experiment/experiment-reducers.ts +++ b/packages/core/src/context/experiment/experiment-reducers.ts @@ -511,25 +511,47 @@ const STRUCTURAL_KEYS = [ 'optimizerConfig', ] as const -// Single invalidation policy: ANY action that changes a structural field -// (variables, score variables, data points, or optimizer config) invalidates a -// stored pareto selection, whose coordinates would otherwise go stale. This -// replaces the 11 per-case clearParetoSelection(state) calls and intentionally -// also covers actions they missed (e.g. updateExperiment replacing the whole -// experiment, or copySuggestedToDataPoints appending data points). -// setSelectedParetoPoint is exempt so setting a selection isn't self-invalidated. export const experimentReducer = ( state: ExperimentType, action: ExperimentAction +): ExperimentType => experimentReducerInner(state, action) + +// Single invalidation policy: ANY action that changes a model-relevant +// structural field (variables, score variables, active data points, or +// optimizer config) invalidates a stored pareto selection, whose coordinates +// would otherwise go stale. This replaces the 11 per-case +// clearParetoSelection(state) calls and intentionally also covers actions they +// missed (e.g. updateExperiment replacing the whole experiment). +// setSelectedParetoPoint is exempt so setting a selection isn't self-invalidated. +// +// Data points are compared by their ACTIVE (valid + enabled) subset — the same +// filter `calculateData` applies to the optimizer request. An unscored row +// (meta.valid: false), e.g. a suggestion or pareto point transferred to the +// data table, is excluded from the request and cannot move the front, so +// appending one must not reset the selection (nor, since the selection is part +// of the hashed request, trigger a re-evaluation). meta.valid is only assigned +// by the validation reducer, so this must run AFTER validation (see +// rootReducer) — comparing pre-validation states would miss a row becoming +// valid when its score is entered. +export const invalidateStaleParetoSelection = ( + previous: ExperimentType, + next: ExperimentType, + action: ExperimentAction ): ExperimentType => { if (action.type === 'setSelectedParetoPoint') { - return experimentReducerInner(state, action) + return next + } + const changed = STRUCTURAL_KEYS.some(key => + key === 'dataPoints' + ? JSON.stringify(selectActiveDataPointsFromExperiment(next)) !== + JSON.stringify(selectActiveDataPointsFromExperiment(previous)) + : next[key] !== previous[key] + ) + if (!changed) { + return next } - const next = experimentReducerInner(state, action) return produce(next, draft => { - const changed = STRUCTURAL_KEYS.some(key => next[key] !== state[key]) - if (changed && 'selectedPoint' in draft.extras) { - // A stored pareto selection's coordinates go stale on any structural edit. + if ('selectedPoint' in draft.extras) { delete draft.extras.selectedPoint } }) diff --git a/packages/core/src/context/experiment/reducers.ts b/packages/core/src/context/experiment/reducers.ts index 59388890..eb101be9 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, + invalidateStaleParetoSelection, resetSuggestionCountOnModelFit, } from './experiment-reducers' import { validateExperiment, ValidationViolations } from './validation' @@ -41,10 +42,17 @@ export const rootReducer = (state: State, action: Action) => { const validationViolations: ValidationViolations = validateExperiment(experiment) const validated = validationReducer(experiment, validationViolations) + // Selection invalidation runs post-validation: meta.valid is only set by + // the validation reducer, and the active-data comparison depends on it. + const selectionChecked = invalidateStaleParetoSelection( + state.experiment, + validated, + action + ) return { ...state, experiment: calculateChangeReducer( - resetSuggestionCountOnModelFit(state.experiment, validated) + resetSuggestionCountOnModelFit(state.experiment, selectionChecked) ), } } From 49a2e58eb4fc71795ea9e09a977c8ecf5c879466 Mon Sep 17 00:00:00 2001 From: Jakob Langdal Date: Sat, 4 Jul 2026 08:23:02 +0000 Subject: [PATCH 2/2] perf(core): skip pareto-selection deep compare when dataPoints reference is unchanged invalidateStaleParetoSelection ran JSON.stringify over the active data-point subset of both states on every action, adding avoidable O(n) work even to clearly non-structural actions (e.g. updateExperimentName). The active subset is a pure function of experiment.dataPoints, and Immer keeps the same array reference when dataPoints is untouched, so an unchanged reference guarantees an identical subset. Short-circuit on next.dataPoints === previous.dataPoints and only fall back to the deep compare when the reference actually changes. Behavior-preserving: a row becoming valid (score entered) mutates dataPoints, yielding a new reference, so validation-driven invalidation still fires. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/src/context/experiment/experiment-reducers.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/core/src/context/experiment/experiment-reducers.ts b/packages/core/src/context/experiment/experiment-reducers.ts index e9c38278..bed1df0f 100644 --- a/packages/core/src/context/experiment/experiment-reducers.ts +++ b/packages/core/src/context/experiment/experiment-reducers.ts @@ -543,8 +543,14 @@ export const invalidateStaleParetoSelection = ( } const changed = STRUCTURAL_KEYS.some(key => key === 'dataPoints' - ? JSON.stringify(selectActiveDataPointsFromExperiment(next)) !== - JSON.stringify(selectActiveDataPointsFromExperiment(previous)) + ? // Fast path: Immer preserves the array reference when dataPoints is + // untouched (e.g. updateExperimentName), and the active subset is a pure + // function of dataPoints — so equal references guarantee an identical + // subset. Only fall back to the O(n) deep compare when the reference + // actually changes, avoiding two JSON.stringify passes on every action. + next.dataPoints !== previous.dataPoints && + JSON.stringify(selectActiveDataPointsFromExperiment(next)) !== + JSON.stringify(selectActiveDataPointsFromExperiment(previous)) : next[key] !== previous[key] ) if (!changed) {