Skip to content

Commit 5257fa1

Browse files
committed
Refactor AddToTestsetDrawer to fix cursor loss during mapping input and improve trace data entity synchronization
- Add stable mapping IDs using createMappingId() to prevent React key changes that cause cursor loss during typing - Add useRef-based trace data load tracking to trigger entity sync when trace data finishes loading - Update getValueAtPath to parse stringified JSON values before path navigation (handles outputs: '{"Response": "value"}') - Add debug logging to mapping extraction in update
1 parent f5b359d commit 5257fa1

File tree

9 files changed

+96
-17
lines changed

9 files changed

+96
-17
lines changed

web/oss/src/components/SharedDrawers/AddToTestsetDrawer/TestsetDrawer.tsx

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useEffect, useState} from "react"
1+
import {useCallback, useEffect, useRef, useState} from "react"
22

33
import {WarningCircle} from "@phosphor-icons/react"
44
import {Button, Input, Typography} from "antd"
@@ -57,6 +57,28 @@ const TestsetDrawer = ({open, spanIds, onClose, initialPath = "ag.data"}: Testse
5757
}
5858
}, [spanIds, initializeWithSpanIds])
5959

60+
// Track if trace data has loaded to trigger entity sync
61+
// When trace data entities finish loading (have non-empty data), sync local entities
62+
const hasTraceDataLoaded = useRef(false)
63+
useEffect(() => {
64+
// Check if trace data has actually loaded (has non-empty data objects)
65+
const hasData = drawer.traceData.some((t) => t.data && Object.keys(t.data).length > 0)
66+
67+
// If we haven't synced yet AND data has now loaded AND we have a revision selected
68+
if (!hasTraceDataLoaded.current && hasData && drawer.selectedRevisionId) {
69+
hasTraceDataLoaded.current = true
70+
// Trigger entity sync by calling onNewColumnBlur which updates local entities
71+
// with the now-loaded trace data
72+
console.log("[TestsetDrawer] Trace data loaded, syncing local entities")
73+
drawer.onNewColumnBlur()
74+
}
75+
76+
// Reset the flag when drawer closes
77+
if (!open) {
78+
hasTraceDataLoaded.current = false
79+
}
80+
}, [drawer.traceData, drawer.selectedRevisionId, drawer.onNewColumnBlur, open])
81+
6082
const elemRef = useResizeObserver<HTMLDivElement>((rect) => {
6183
drawer.setIsDrawerExtended(rect.width > 640)
6284
})

web/oss/src/components/SharedDrawers/AddToTestsetDrawer/assets/types.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
import {KeyValuePair} from "tailwindcss/types/config"
22

33
export interface Mapping {
4+
/** Stable unique ID for React key - prevents cursor loss during typing */
5+
id: string
46
data: string
57
column: string
68
newColumn?: string
79
}
10+
11+
/** Generate a unique ID for a new mapping */
12+
export function createMappingId(): string {
13+
return `mapping-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`
14+
}
815
export interface Preview {
916
key: string
1017
data: KeyValuePair[]

web/oss/src/components/SharedDrawers/AddToTestsetDrawer/atoms/drawerState.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
selectedRevisionIdAtom as sharedSelectedRevisionIdAtom,
1616
} from "@/oss/state/testsetSelection"
1717

18-
import type {Mapping, TestsetTraceData} from "../assets/types"
18+
import {createMappingId, type Mapping, type TestsetTraceData} from "../assets/types"
1919

2020
import {
2121
cascaderValueAtom,
@@ -837,11 +837,16 @@ export const applyAutoMappingAtom = atom(
837837
const matchedMappings = matchColumnsWithSuggestions(suggestions, columns)
838838

839839
// Convert to mapping format
840-
const newMappings: Mapping[] = matchedMappings.map((match, index) => ({
841-
...currentMappings[index],
842-
data: match.data,
843-
column: match.column,
844-
}))
840+
const newMappings: Mapping[] = matchedMappings.map((match, index) => {
841+
const existingMapping = currentMappings[index]
842+
return {
843+
// Use existing mapping ID if available, otherwise generate new one
844+
id: existingMapping?.id || createMappingId(),
845+
data: match.data,
846+
column: match.column,
847+
newColumn: existingMapping?.newColumn,
848+
}
849+
})
845850

846851
// Only update mappings if different
847852
const isSame =
@@ -1017,6 +1022,17 @@ export const onRevisionSelectAtom = atom(
10171022
return {success: false, reason: "no_trace_data"}
10181023
}
10191024

1025+
// Check if trace data has loaded (entities have non-empty data)
1026+
// If not loaded yet, log a warning - the data will be synced when entities load
1027+
const hasLoadedData = traceData.some((t) => t.data && Object.keys(t.data).length > 0)
1028+
if (!hasLoadedData) {
1029+
console.warn(
1030+
"[onRevisionSelectAtom] Trace data entities not fully loaded yet. " +
1031+
"Local entities will have empty data until onNewColumnBlur or mapping change triggers update.",
1032+
{traceDataCount: traceData.length, mappingCount: mappingData.length},
1033+
)
1034+
}
1035+
10201036
// Import selectRevisionAtom dynamically to avoid circular dependency
10211037
// eslint-disable-next-line @typescript-eslint/no-require-imports
10221038
const {selectRevisionAtom} = require("./localEntities")

web/oss/src/components/SharedDrawers/AddToTestsetDrawer/atoms/localEntities.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,14 @@ export const updateAllLocalEntitiesAtom = atom(
357357
if (!targetColumn) continue
358358

359359
const value = getValueAtPath(trace, mapping.data)
360+
console.log("[updateAllLocalEntitiesAtom] Mapping extraction", {
361+
mappingPath: mapping.data,
362+
targetColumn,
363+
traceDataKeys: Object.keys(trace.data || {}),
364+
traceDataPreview: JSON.stringify(trace.data).slice(0, 200),
365+
extractedValue: value,
366+
valueType: typeof value,
367+
})
360368
// Preserve objects/arrays as-is, only convert null/undefined to empty string
361369
updates[targetColumn] = value === undefined || value === null ? "" : value
362370
}
@@ -537,6 +545,14 @@ export const selectRevisionAtom = atom(
537545
if (!targetColumn) continue
538546

539547
const value = getValueAtPath(trace, mapping.data)
548+
console.log("[selectRevisionAtom] Mapping extraction", {
549+
mappingPath: mapping.data,
550+
targetColumn,
551+
traceDataKeys: Object.keys(trace.data || {}),
552+
traceDataPreview: JSON.stringify(trace.data).slice(0, 200),
553+
extractedValue: value,
554+
valueType: typeof value,
555+
})
540556
// Preserve objects/arrays as-is, only convert null/undefined to empty string
541557
mappedData[targetColumn] = value === undefined || value === null ? "" : value
542558
}

web/oss/src/components/SharedDrawers/AddToTestsetDrawer/components/MappingSection.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {useMemo} from "react"
33
import {ArrowRight, Crosshair, Plus, Trash} from "@phosphor-icons/react"
44
import {AutoComplete, Button, Select, Tooltip, Typography} from "antd"
55

6-
import {Mapping, TestsetColumn} from "../assets/types"
6+
import {createMappingId, Mapping, TestsetColumn} from "../assets/types"
77

88
interface MappingSectionProps {
99
mappingData: Mapping[]
@@ -98,7 +98,7 @@ export function MappingSection({
9898
<div className="flex flex-col gap-2">
9999
{mappingData.map((mapping, idx) => (
100100
<div
101-
key={`mapping-${idx}-${mapping.data || ""}`}
101+
key={mapping.id}
102102
className="flex gap-2 items-center"
103103
>
104104
{/* Inputs container - takes remaining space */}
@@ -221,7 +221,12 @@ export function MappingSection({
221221
className="mt-1"
222222
style={{width: elementWidth}}
223223
icon={<Plus />}
224-
onClick={() => setMappingData([...mappingData, {data: "", column: ""}])}
224+
onClick={() =>
225+
setMappingData([
226+
...mappingData,
227+
{id: createMappingId(), data: "", column: ""},
228+
])
229+
}
225230
>
226231
Add field
227232
</Button>

web/oss/src/components/SharedDrawers/AddToTestsetDrawer/hooks/useTestsetDrawer.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {currentColumnsAtom} from "@/oss/state/entities/testcase/columnState"
99
import {getValueAtPath} from "@/oss/state/entities/trace"
1010
import {projectIdAtom} from "@/oss/state/project"
1111

12-
import {Mapping, TestsetColumn, TestsetTraceData} from "../assets/types"
12+
import {createMappingId, Mapping, TestsetColumn, TestsetTraceData} from "../assets/types"
1313
import {onCascaderChangeAtom} from "../atoms/actions"
1414
import {
1515
allTracePathsSelectOptionsAtom,
@@ -192,15 +192,15 @@ export function useTestsetDrawer(): UseTestsetDrawerResult {
192192
if (column === "create") {
193193
setMappingData((prev) => [
194194
...prev,
195-
{data: dataPath, column: "create", newColumn: ""},
195+
{id: createMappingId(), data: dataPath, column: "create", newColumn: ""},
196196
])
197197
// Scroll to mapping section so user can fill in the column name
198198
setTimeout(() => {
199199
const mappingSection = document.querySelector('[data-testid="mapping-section"]')
200200
mappingSection?.scrollIntoView({behavior: "smooth", block: "center"})
201201
}, 100)
202202
} else {
203-
setMappingData((prev) => [...prev, {data: dataPath, column}])
203+
setMappingData((prev) => [...prev, {id: createMappingId(), data: dataPath, column}])
204204
// Trigger entity update to sync columns to preview table
205205
executeNewColumnBlur(getValueAtPath)
206206
}

web/oss/src/components/TestcasesTableNew/components/TestcaseSelectionCell.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@ const TestcaseSelectionCell = memo(function TestcaseSelectionCell({
2828
originNode,
2929
mode = "edit",
3030
}: TestcaseSelectionCellProps) {
31-
// Check if testcase has unsaved changes using controller selector (only in edit mode)
31+
// Check if testcase has unsaved changes using controller selector
3232
// This includes both draft edits AND pending column changes
3333
const isDirtyAtom = useMemo(() => testcase.selectors.isDirty(testcaseId || ""), [testcaseId])
34-
const isDirty = mode === "edit" ? useAtomValue(isDirtyAtom) : false
34+
// Always call useAtomValue (hooks must be unconditional), then check mode
35+
const isDirtyValue = useAtomValue(isDirtyAtom)
36+
const isDirty = mode === "edit" ? isDirtyValue : false
3537

3638
// New rows (not yet saved) are always dirty
3739
const isNewRow = testcaseId?.startsWith("new-") || testcaseId?.startsWith("local-") || false

web/oss/src/components/pages/evaluations/autoEvaluation/EvaluatorsModal/ConfigureEvaluator/DebugSection.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1118,7 +1118,7 @@ const DebugSection = () => {
11181118
]}
11191119
/>
11201120
</div>
1121-
shit
1121+
11221122
<EvaluatorVariantModal
11231123
variants={derivedVariants}
11241124
open={openVariantModal}

web/oss/src/state/entities/trace/selectors.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const splitPath = (path: string): string[] =>
2929

3030
/**
3131
* Get a value from an object using dot-notation path
32-
* Handles nested objects and escaped dots in keys
32+
* Handles nested objects, escaped dots in keys, and stringified JSON values
3333
*/
3434
export const getValueAtPath = (obj: any, rawPath: string): any => {
3535
if (obj == null || !rawPath) return undefined
@@ -43,6 +43,17 @@ export const getValueAtPath = (obj: any, rawPath: string): any => {
4343
for (let i = 0; i < parts.length; i++) {
4444
if (cur == null) return undefined
4545

46+
// If current value is a string, try to parse it as JSON
47+
// This handles cases where data is stringified (e.g., outputs: '{"Response": "value"}')
48+
if (typeof cur === "string") {
49+
try {
50+
cur = JSON.parse(cur)
51+
} catch {
52+
// Not valid JSON, can't navigate further into a string
53+
return undefined
54+
}
55+
}
56+
4657
const key = parts[i]
4758

4859
if (Object.prototype.hasOwnProperty.call(cur, key)) {

0 commit comments

Comments
 (0)