Skip to content

Commit 4045d7d

Browse files
atheurerclaude
andcommitted
Add per-iteration sample selection and fix varyingKeys crash
- Server accepts sampleIndex as either a number (all iterations) or an object { iterationId: index } for per-iteration sample selection - Client computes best sample per iteration (closest to mean) instead of using first iteration's best for all - Expandable "Per-iteration samples" section in metric controls lets users override sample selection per iteration, with best marked (*) - Global "Best (auto)" resets all to per-iteration auto-best - Fix crash when rendering per-iteration sample labels: include varyingKeys in chart result object (was undefined, causing .has() error on buildIterLabel) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5c946c1 commit 4045d7d

3 files changed

Lines changed: 150 additions & 33 deletions

File tree

queries/cdmq/server.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,9 @@ app.post('/api/v1/iterations/supplemental-metric', async (req, res) => {
11281128
const { runIds, iterations: reqIterations, start, end, source, type, breakout, filter, sampleIndex } = req.body;
11291129
var breakoutArr = Array.isArray(breakout) ? breakout : [];
11301130
var filterVal = filter || null;
1131+
// sampleIndex can be a number (same for all iterations) or an object { iterationId: index }
11311132
var requestedSampleIdx = (typeof sampleIndex === 'number') ? sampleIndex : null;
1133+
var perIterSampleIdx = (typeof sampleIndex === 'object' && sampleIndex !== null && !Array.isArray(sampleIndex)) ? sampleIndex : null;
11321134
if (!source || !type) {
11331135
return res.status(400).json({ code: 'MISSING_PARAMS', error: 'source and type are required' });
11341136
}
@@ -1208,8 +1210,14 @@ app.post('/api/v1/iterations/supplemental-metric', async (req, res) => {
12081210
var iterRanges = (periodRanges[i]) || [];
12091211
if (iterPeriodIds.length === 0) continue;
12101212

1211-
// Use requested sample index, defaulting to 0
1212-
var selIdx = (requestedSampleIdx !== null && requestedSampleIdx < iterPeriodIds.length) ? requestedSampleIdx : 0;
1213+
// Use per-iteration sample index if available, otherwise global, otherwise 0
1214+
var selIdx = 0;
1215+
if (perIterSampleIdx && perIterSampleIdx[allIterIds[i]] != null) {
1216+
selIdx = perIterSampleIdx[allIterIds[i]];
1217+
} else if (requestedSampleIdx !== null) {
1218+
selIdx = requestedSampleIdx;
1219+
}
1220+
if (selIdx >= iterPeriodIds.length) selIdx = 0;
12131221

12141222
if (!iterPeriodIds[selIdx]) continue;
12151223
var range = iterRanges[selIdx];

queries/cdmq/web-ui/src/components/CompareView.jsx

Lines changed: 98 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -612,8 +612,8 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
612612
restoredMetricsApplied.current = true;
613613
var ctx = getRunContext();
614614
restoredMetrics.forEach(function (rm) {
615-
var bestIdx = computeBestSampleIndex();
616-
var sIdx = rm.sampleIndex != null ? rm.sampleIndex : bestIdx;
615+
var bestIndices = computeBestSampleIndices();
616+
var sIdx = rm.sampleIndex != null ? rm.sampleIndex : bestIndices;
617617
timeWork('Restore ' + rm.source + '::' + rm.type, function () {
618618
return api.getSupplementalMetric({
619619
iterations: ctx.iterations, start: ctx.start, end: ctx.end,
@@ -666,36 +666,48 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
666666
}
667667
}, [iterations]);
668668

669-
// Compute best sample index from primary metric values (closest to mean)
670-
function computeBestSampleIndex() {
671-
// Find the most common sample count and compute best index from first iteration with values
672-
var bestIdx = 0;
669+
// Compute best sample index per iteration from primary metric values (closest to mean)
670+
function computeBestSampleIndices() {
671+
var indices = {};
673672
for (var itId in metricValues) {
674673
var mv = metricValues[itId];
675674
if (mv && mv.sampleValues && mv.sampleValues.length > 1) {
676675
var sum = 0;
677676
for (var v = 0; v < mv.sampleValues.length; v++) sum += mv.sampleValues[v];
678677
var mean = sum / mv.sampleValues.length;
679678
var bestDiff = Infinity;
679+
var bestIdx = 0;
680680
for (var s = 0; s < mv.sampleValues.length; s++) {
681681
var diff = Math.abs(mv.sampleValues[s] - mean);
682682
if (diff < bestDiff) { bestDiff = diff; bestIdx = s; }
683683
}
684-
break; // Use the first iteration's best sample as default
684+
indices[itId] = bestIdx;
685+
} else {
686+
indices[itId] = 0;
685687
}
686688
}
687-
return bestIdx;
689+
return indices;
690+
}
691+
692+
// Backward-compatible: single best index (from first iteration with multiple samples)
693+
function computeBestSampleIndex() {
694+
var indices = computeBestSampleIndices();
695+
for (var itId in indices) {
696+
var mv = metricValues[itId];
697+
if (mv && mv.sampleValues && mv.sampleValues.length > 1) return indices[itId];
698+
}
699+
return 0;
688700
}
689701

690702
var handleAddMetric = useCallback(function () {
691703
if (!addMetricSource || !addMetricType) return;
692704
var exists = supplementalMetrics.some(function (m) { return m.source === addMetricSource && m.type === addMetricType; });
693705
if (exists) { setShowAddMetric(false); return; }
694706
var ctx = getRunContext();
695-
var bestIdx = computeBestSampleIndex();
707+
var bestIndices = computeBestSampleIndices();
696708
setAddMetricLoading(true);
697709
timeWork('Fetch ' + addMetricSource + '::' + addMetricType, function () {
698-
return api.getSupplementalMetric({ iterations: ctx.iterations, start: ctx.start, end: ctx.end, source: addMetricSource, type: addMetricType, sampleIndex: bestIdx });
710+
return api.getSupplementalMetric({ iterations: ctx.iterations, start: ctx.start, end: ctx.end, source: addMetricSource, type: addMetricType, sampleIndex: bestIndices });
699711
}).then(function (res) {
700712
setSupplementalMetrics(function (prev) {
701713
return prev.concat([{
@@ -705,7 +717,7 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
705717
display: addMetricDisplay,
706718
chartType: 'bar', // 'bar', 'stacked', 'line'
707719
filter: '', // e.g., 'gt:0.01', 'lt:100'
708-
sampleIndex: bestIdx, // client-computed best sample
720+
sampleIndex: bestIndices, // per-iteration best sample indices
709721
breakouts: [], // active breakout dimensions
710722
remainingBreakouts: res.remainingBreakouts || [],
711723
loading: false,
@@ -799,17 +811,29 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
799811
});
800812
}, [iterations, supplementalMetrics]);
801813

802-
var handleSampleChange = useCallback(function (si, newSampleIndex) {
803-
var idx = newSampleIndex === 'auto' ? computeBestSampleIndex() : parseInt(newSampleIndex, 10);
814+
var handleSampleChange = useCallback(function (si, newSampleIndex, iterationId) {
804815
var sm = supplementalMetrics[si];
816+
var newIndices;
817+
if (newSampleIndex === 'auto') {
818+
newIndices = computeBestSampleIndices();
819+
} else if (iterationId) {
820+
// Per-iteration override: update just this iteration's sample index
821+
newIndices = typeof sm.sampleIndex === 'object' && sm.sampleIndex ? Object.assign({}, sm.sampleIndex) : computeBestSampleIndices();
822+
newIndices[iterationId] = parseInt(newSampleIndex, 10);
823+
} else {
824+
// Global override: set all iterations to the same index
825+
var idx = parseInt(newSampleIndex, 10);
826+
newIndices = {};
827+
iterations.forEach(function (it) { newIndices[it.iterationId] = idx; });
828+
}
805829
setSupplementalMetrics(function (prev) {
806830
var next = prev.slice();
807-
next[si] = Object.assign({}, next[si], { sampleIndex: idx, loading: true });
831+
next[si] = Object.assign({}, next[si], { sampleIndex: newIndices, loading: true });
808832
return next;
809833
});
810834
var ctx = getRunContext();
811835
timeWork('Switch sample for ' + sm.source + '::' + sm.type, function () {
812-
return api.getSupplementalMetric({ iterations: ctx.iterations, start: ctx.start, end: ctx.end, source: sm.source, type: sm.type, breakout: sm.breakouts, filter: sm.filter, sampleIndex: idx });
836+
return api.getSupplementalMetric({ iterations: ctx.iterations, start: ctx.start, end: ctx.end, source: sm.source, type: sm.type, breakout: sm.breakouts, filter: sm.filter, sampleIndex: newIndices });
813837
}).then(function (res) {
814838
setSupplementalMetrics(function (prev) {
815839
var next = prev.slice();
@@ -1094,7 +1118,7 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
10941118
}
10951119
}
10961120

1097-
result.push({ metricName: metricName, data: chartData, commonItems: commonItems, groupInfo: groupInfo });
1121+
result.push({ metricName: metricName, data: chartData, commonItems: commonItems, groupInfo: groupInfo, varyingKeys: varyingKeys });
10981122
});
10991123

11001124
return result;
@@ -1229,22 +1253,24 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
12291253
</select>
12301254
)}
12311255
{(function () {
1232-
var sampleVals = null;
1233-
for (var ii = 0; ii < iterations.length; ii++) {
1234-
var mv2 = metricValues[iterations[ii].iterationId];
1235-
if (mv2 && mv2.sampleValues && mv2.sampleValues.length > 1) { sampleVals = mv2.sampleValues; break; }
1236-
}
1237-
if (!sampleVals || sampleVals.length <= 1) return null;
1256+
// Check if any iteration has multiple samples
1257+
var hasMultiSample = iterations.some(function (it) {
1258+
var mv2 = metricValues[it.iterationId];
1259+
return mv2 && mv2.sampleValues && mv2.sampleValues.length > 1;
1260+
});
1261+
if (!hasMultiSample) return null;
1262+
// Determine if all iterations use auto-best
1263+
var currentIndices = typeof sm.sampleIndex === 'object' && sm.sampleIndex ? sm.sampleIndex : null;
1264+
var bestIndices = computeBestSampleIndices();
1265+
var isAuto = !currentIndices || iterations.every(function (it) {
1266+
return (currentIndices[it.iterationId] == null || currentIndices[it.iterationId] === bestIndices[it.iterationId]);
1267+
});
12381268
return (
12391269
<span className="compare-filter-group">
12401270
<label className="compare-filter-label">Sample:</label>
1241-
<select className="compare-breakout-select" value={sm.sampleIndex != null ? sm.sampleIndex : 'auto'} onChange={function (e) { handleSampleChange(si, e.target.value); }}>
1271+
<select className="compare-breakout-select" value={isAuto ? 'auto' : 'custom'} onChange={function (e) { if (e.target.value === 'auto') handleSampleChange(si, 'auto'); }}>
12421272
<option value="auto">Best (auto)</option>
1243-
{sampleVals.map(function (pmv, idx2) {
1244-
var label2 = 'Sample ' + (idx2 + 1);
1245-
if (pmv != null) label2 += ' (' + formatValue(pmv) + ')';
1246-
return <option key={idx2} value={idx2}>{label2}</option>;
1247-
})}
1273+
<option value="custom" disabled>Per-iteration</option>
12481274
</select>
12491275
</span>
12501276
);
@@ -1284,6 +1310,47 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
12841310
<button className="btn btn-sm btn-secondary" onClick={function () { handleApplyBreakoutFilter(si); }} disabled={sm.loading} style={{ fontSize: 10, padding: '2px 6px' }}>Apply</button>
12851311
</div>
12861312
)}
1313+
{/* Per-iteration sample overrides (expandable) */}
1314+
{(function () {
1315+
var hasMultiSample = iterations.some(function (it) {
1316+
var mv2 = metricValues[it.iterationId];
1317+
return mv2 && mv2.sampleValues && mv2.sampleValues.length > 1;
1318+
});
1319+
if (!hasMultiSample) return null;
1320+
var currentIndices = typeof sm.sampleIndex === 'object' && sm.sampleIndex ? sm.sampleIndex : {};
1321+
var bestIndices = computeBestSampleIndices();
1322+
var hasOverride = iterations.some(function (it) {
1323+
return currentIndices[it.iterationId] != null && currentIndices[it.iterationId] !== bestIndices[it.iterationId];
1324+
});
1325+
return (
1326+
<details className="compare-sample-details">
1327+
<summary className="compare-sample-summary">
1328+
Per-iteration samples{hasOverride ? ' (customized)' : ''}
1329+
</summary>
1330+
<div className="compare-sample-list">
1331+
{iterations.map(function (it) {
1332+
var mv2 = metricValues[it.iterationId];
1333+
if (!mv2 || !mv2.sampleValues || mv2.sampleValues.length <= 1) return null;
1334+
var currentIdx = currentIndices[it.iterationId] != null ? currentIndices[it.iterationId] : (bestIndices[it.iterationId] || 0);
1335+
var label = buildIterLabel(it, charts.length > 0 ? charts[0].varyingKeys : new Set(), new Set());
1336+
return (
1337+
<div key={it.iterationId} className="compare-sample-row">
1338+
<span className="compare-sample-iter-label" title={it.iterationId}>{label || it.iterationId.substring(0, 8)}</span>
1339+
<select className="compare-breakout-select" value={currentIdx} onChange={function (e) { handleSampleChange(si, e.target.value, it.iterationId); }}>
1340+
{mv2.sampleValues.map(function (pmv, idx2) {
1341+
var slabel = 'Sample ' + (idx2 + 1);
1342+
if (pmv != null) slabel += ' (' + formatValue(pmv) + ')';
1343+
if (idx2 === (bestIndices[it.iterationId] || 0)) slabel += ' *';
1344+
return <option key={idx2} value={idx2}>{slabel}</option>;
1345+
})}
1346+
</select>
1347+
</div>
1348+
);
1349+
})}
1350+
</div>
1351+
</details>
1352+
);
1353+
})()}
12871354
</div>
12881355
);
12891356
}
@@ -1809,19 +1876,19 @@ const CompareView = forwardRef(function CompareView({ selected, groupByList, set
18091876
<div className="compare-primary-controls">
18101877
<button className="btn btn-sm btn-secondary" onClick={function () {
18111878
var ctx = getRunContext();
1812-
var bestIdx = computeBestSampleIndex();
1879+
var bestIndices = computeBestSampleIndices();
18131880
setAddMetricLoading(true);
18141881
timeWork('Add primary metric refinement ' + pmStr, function () {
18151882
return api.getSupplementalMetric({
18161883
iterations: ctx.iterations, start: ctx.start, end: ctx.end,
1817-
source: pmParts[0], type: pmParts[1], sampleIndex: bestIdx,
1884+
source: pmParts[0], type: pmParts[1], sampleIndex: bestIndices,
18181885
});
18191886
}).then(function (res) {
18201887
setSupplementalMetrics(function (prev) {
18211888
return prev.concat([{
18221889
source: pmParts[0], type: pmParts[1],
18231890
values: res.values || {}, display: 'panel',
1824-
chartType: 'bar', filter: '', sampleIndex: bestIdx,
1891+
chartType: 'bar', filter: '', sampleIndex: bestIndices,
18251892
breakouts: [], remainingBreakouts: res.remainingBreakouts || [],
18261893
loading: false,
18271894
}]);

queries/cdmq/web-ui/src/index.css

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,6 +1376,48 @@ a.run-id:hover {
13761376
color: var(--danger);
13771377
}
13781378

1379+
.compare-sample-details {
1380+
margin-top: 4px;
1381+
padding-left: 8px;
1382+
}
1383+
1384+
.compare-sample-summary {
1385+
font-size: 11px;
1386+
color: var(--text-muted);
1387+
cursor: pointer;
1388+
user-select: none;
1389+
}
1390+
1391+
.compare-sample-summary:hover {
1392+
color: var(--text);
1393+
}
1394+
1395+
.compare-sample-list {
1396+
display: flex;
1397+
flex-direction: column;
1398+
gap: 3px;
1399+
margin-top: 4px;
1400+
padding-left: 8px;
1401+
}
1402+
1403+
.compare-sample-row {
1404+
display: flex;
1405+
align-items: center;
1406+
gap: 8px;
1407+
font-size: 11px;
1408+
}
1409+
1410+
.compare-sample-iter-label {
1411+
color: var(--text-secondary);
1412+
font-family: 'SF Mono', ui-monospace, Consolas, monospace;
1413+
font-size: 10px;
1414+
min-width: 120px;
1415+
max-width: 200px;
1416+
overflow: hidden;
1417+
text-overflow: ellipsis;
1418+
white-space: nowrap;
1419+
}
1420+
13791421
.compare-metric-breakouts {
13801422
display: flex;
13811423
flex-wrap: wrap;

0 commit comments

Comments
 (0)