Skip to content

Commit 317d6e6

Browse files
committed
fix(playground): clean stale row on app swap + drop diagnostic logs (#4525)
Two follow-ups now that the row-reconciliation fix is verified working: 1. Clean-on-swap: the evaluator playground selects an app via changePrimaryNode + connectDownstreamNode (ConfigureEvaluator), not a setEntityIds positional swap — so the previously-wired swap-time prune never fired there. Add playgroundController.actions.reconcileRowsToPrimary and call it from connectAppToEvaluatorAtom AFTER connectDownstreamNode, so the shared testcase row is cleaned the instant the app changes (not only at run time). Running after the downstream connect means the evaluator's referenced columns (correct_answer_key → ground_truth, etc.) are protected from the strict app-contract clean. pruneTestcaseRowsForEntity now: - collects downstream-evaluator protected columns, - returns a status ('acted' | 'noop' | 'unresolved'). reconcileRowsToPrimary handles the hydration race: if the new primary's inputPorts aren't resolved yet AND the entity isn't loaded, it subscribes to inputPorts and retries once, then unsubscribes. If the entity is loaded but genuinely has no variables, it doesn't subscribe (no dangling sub). The run-time reconciliation in webWorkerIntegration remains the backstop. 2. Remove diagnostic logs added while tracing the bug: [executionRunner.filter], [webWorker.reconcile], [playgroundController.prune]. The reconcile + writeback logic stays; only the console.warn telemetry is dropped.
1 parent 123cc1c commit 317d6e6

4 files changed

Lines changed: 83 additions & 53 deletions

File tree

web/oss/src/components/Evaluators/components/ConfigureEvaluator/atoms.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,15 @@ export const connectAppToEvaluatorAtom = atom(
188188
},
189189
})
190190

191+
// Clean the shared testcase row against the newly-selected app's input
192+
// contract so stale keys from a previously-selected app (e.g. chat
193+
// `messages`/`context` after swapping a chat app for a completion app)
194+
// are dropped immediately — not only at run time (#4525 / AGE-3793).
195+
// Runs AFTER connectDownstreamNode so the evaluator is in the graph and
196+
// its referenced columns (correct_answer_key → ground_truth, etc.) are
197+
// protected from the strict app-contract clean.
198+
set(playgroundController.actions.reconcileRowsToPrimary)
199+
191200
// Persist only after both graph mutations succeeded. The picker
192201
// display label is derived from the depth-0 node's `label` via
193202
// `selectedAppLabelAtom`, so no extra write needed here.

web/packages/agenta-playground/src/state/controllers/playgroundController.ts

Lines changed: 73 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ import {
8383
newTestcaseDataHashAtom,
8484
} from "../execution/selectors"
8585
import {pruneDanglingConnections} from "../helpers/connectionGraph"
86-
import {reconcileRowDataForEntity, resolveEntityInputContract} from "../helpers/entityInputContract"
86+
import {
87+
collectDownstreamReferencedColumns,
88+
reconcileRowDataForEntity,
89+
resolveEntityInputContract,
90+
} from "../helpers/entityInputContract"
8791
import {extractAndLoadChatMessagesAtom} from "../helpers/extractAndLoadChatMessages"
8892
import {normalizeTestcaseRowsForLoad} from "../helpers/testcaseRowNormalization"
8993
import type {EntitySelection, PlaygroundNode, RunnableType} from "../types"
@@ -2112,88 +2116,113 @@ function relinkLoadableSessions(
21122116
}
21132117
}
21142118

2119+
type PruneStatus = "acted" | "noop" | "unresolved"
2120+
21152121
/**
2116-
* Reconcile every testcase row against the new primary entity's input
2117-
* contract.
2122+
* Reconcile every testcase row against the given entity's input contract.
21182123
*
21192124
* Why this exists: the testcase row store (`testcaseMolecule`) is shared
21202125
* across loadables. When the user swaps the primary app in the LLM-as-a-
2121-
* judge playground (anchor swap in `setEntityIdsAtom`), the row data keeps
2122-
* every key the *previous* primary populated — chat `messages`, completion
2123-
* template variables that the new app doesn't declare, etc. Without
2124-
* reconciliation, those stale keys leak into the new app's request body
2125-
* via the downstream "spread all keys" fallbacks.
2126-
*
2127-
* Handling at the row layer (here) makes the UI immediately reflect the new
2128-
* app's variables and is the primary fix; execution-time reconciliation in
2129-
* `executionRunner.ts` is only a hydration-window safety net.
2126+
* judge playground, the row data keeps every key the *previous* primary
2127+
* populated — chat `messages`, completion template variables that the new
2128+
* app doesn't declare, etc. Without reconciliation, those stale keys leak
2129+
* into the new app's request body and the downstream evaluator's envelope.
21302130
*
21312131
* Allow-list source is `inputPorts` (via `resolveEntityInputContract`), NOT
21322132
* `inputSchema.properties` — completion apps surface their variables as
21332133
* prompt template placeholders through `inputPorts` and have an EMPTY static
21342134
* input schema, so schema-based filtering keeps everything. Policy:
2135-
* - App with a resolved contract → strict: keep only declared keys.
2135+
* - App with a resolved contract → strict: keep only declared (or
2136+
* downstream-evaluator-protected) keys.
21362137
* - Evaluator → chat-transport only: evaluators spread extra testcase
21372138
* columns, so we never strict-filter them.
2138-
* - Unresolved contract (ports mid-hydration) → skip; the execution-time
2139-
* reconciliation catches it. A console.warn is emitted so we can see
2140-
* when the hydration window is hit.
2139+
* - Unresolved contract (ports mid-hydration) → no-op; returns
2140+
* `"unresolved"` so the caller can retry once the contract resolves. The
2141+
* run-time reconciliation in `webWorkerIntegration` is the backstop.
2142+
*
2143+
* Columns referenced by downstream evaluator `<input>_key` settings (e.g.
2144+
* `correct_answer_key → ground_truth`) are protected so a strict clean
2145+
* against the app contract doesn't drop intentional evaluation inputs.
21412146
*
21422147
* Mutations go through `testcaseMolecule.actions.batchUpdate` setting stale
21432148
* keys to `undefined`, which the store's update reducer interprets as a
21442149
* delete. Drafts are created as needed (one per affected row).
21452150
*/
2146-
function pruneTestcaseRowsForEntity(get: Getter, set: Setter, entityId: string) {
2151+
function pruneTestcaseRowsForEntity(get: Getter, set: Setter, entityId: string): PruneStatus {
21472152
const contract = resolveEntityInputContract(get, entityId)
21482153

21492154
// Unresolved, non-evaluator contract → we can't strict-filter safely yet.
21502155
// The evaluator path is always "resolved enough" (chat-transport strip
21512156
// works without a variable list), so only bail for non-evaluator apps.
21522157
if (!contract.isEvaluator && !contract.resolved) {
2153-
console.warn("[playgroundController.prune] contract-not-resolved on swap", {
2154-
entityId,
2155-
// Without resolved inputPorts we can't decide what to drop. The
2156-
// execution-time reconciliation catches the leak window for now.
2157-
})
2158-
return
2158+
return "unresolved"
21592159
}
21602160

21612161
const displayRowIds = get(testcaseMolecule.atoms.displayRowIds)
2162-
if (!Array.isArray(displayRowIds) || displayRowIds.length === 0) return
2162+
if (!Array.isArray(displayRowIds) || displayRowIds.length === 0) return "noop"
2163+
2164+
const protectedColumns = collectDownstreamReferencedColumns(get, get(playgroundNodesAtom))
21632165

21642166
const updates: {id: string; updates: {data: Record<string, unknown>}}[] = []
2165-
const droppedPerRow: Record<string, string[]> = {}
2166-
let strategyUsed: string | null = null
21672167

21682168
for (const rowId of displayRowIds) {
21692169
const row = get(testcaseMolecule.data(rowId))
21702170
const data = (row as {data?: Record<string, unknown>} | null)?.data
21712171
if (!data || typeof data !== "object") continue
21722172

2173-
const {dropped, strategy} = reconcileRowDataForEntity(get, entityId, data)
2173+
const {dropped} = reconcileRowDataForEntity(get, entityId, data, {
2174+
protectedKeys: protectedColumns,
2175+
})
21742176
if (dropped.length === 0) continue
2175-
strategyUsed = strategy
21762177

21772178
const undefinedData: Record<string, unknown> = {}
21782179
for (const key of dropped) {
21792180
undefinedData[key] = undefined
21802181
}
21812182
updates.push({id: rowId, updates: {data: undefinedData}})
2182-
droppedPerRow[rowId] = dropped
21832183
}
21842184

2185-
if (updates.length === 0) return
2186-
2187-
console.warn("[playgroundController.prune] dropped stale keys after primary swap", {
2188-
entityId,
2189-
rowsAffected: updates.length,
2190-
strategy: strategyUsed,
2191-
droppedPerRow,
2192-
})
2185+
if (updates.length === 0) return "noop"
21932186

21942187
set(testcaseMolecule.actions.batchUpdate, updates)
2188+
return "acted"
21952189
}
21962190

2191+
/**
2192+
* Reconcile all testcase rows against the CURRENT primary (depth-0) entity's
2193+
* input contract, on demand — call this right after a primary swap so the
2194+
* shared row is cleaned the instant the app changes, without waiting for a
2195+
* run. The run-time reconciliation in `webWorkerIntegration` is the backstop.
2196+
*
2197+
* Hydration handling: the new primary's input ports may not be resolved at
2198+
* call time (the workflow is still loading). When the prune reports
2199+
* `"unresolved"` AND the entity isn't loaded yet, we subscribe to its
2200+
* `inputPorts` and retry once they resolve, then unsubscribe. If the entity
2201+
* is already loaded but has no resolvable variables, there's nothing to wait
2202+
* for, so we don't subscribe (avoids a dangling subscription).
2203+
*/
2204+
const reconcileRowsToPrimaryAtom = atom(null, (get, set) => {
2205+
const nodes = get(playgroundNodesAtom)
2206+
const primary = nodes.find((node) => node.depth === 0)
2207+
if (!primary) return
2208+
const entityId = primary.entityId
2209+
2210+
const status = pruneTestcaseRowsForEntity(get, set, entityId)
2211+
if (status !== "unresolved") return
2212+
2213+
// Unresolved: either the workflow is still loading, or it's a genuinely
2214+
// no-variable app. Only wait if it hasn't loaded yet.
2215+
const entityLoaded = get(workflowMolecule.selectors.data(entityId)) != null
2216+
if (entityLoaded) return
2217+
2218+
const store = getDefaultStore()
2219+
const unsub = store.sub(workflowMolecule.selectors.inputPorts(entityId), () => {
2220+
const retryStatus = pruneTestcaseRowsForEntity(store.get, store.set, entityId)
2221+
const nowLoaded = store.get(workflowMolecule.selectors.data(entityId)) != null
2222+
if (retryStatus !== "unresolved" || nowLoaded) unsub()
2223+
})
2224+
})
2225+
21972226
/**
21982227
* Switch one entity for another in the displayed selection.
21992228
* Handles both single and comparison mode. The loadable-scoped re-link
@@ -2315,6 +2344,13 @@ export const playgroundController = {
23152344
/** Change the primary node */
23162345
changePrimaryNode: changePrimaryNodeAtom,
23172346

2347+
/**
2348+
* Reconcile all testcase rows against the current primary entity's
2349+
* input contract. Call after a primary swap to clean stale keys from a
2350+
* previous app off the shared row immediately (#4525 / AGE-3793).
2351+
*/
2352+
reconcileRowsToPrimary: reconcileRowsToPrimaryAtom,
2353+
23182354
/** Disconnect from testset and reset to local mode */
23192355
disconnectAndResetToLocal: disconnectAndResetToLocalAtom,
23202356

web/packages/agenta-playground/src/state/execution/executionRunner.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -241,15 +241,7 @@ function reconcileEntityInputData(
241241
data: Record<string, unknown>,
242242
entityId: string,
243243
): Record<string, unknown> {
244-
const {data: next, dropped, strategy} = reconcileRowDataForEntity(get, entityId, data)
245-
if (dropped.length > 0) {
246-
console.warn("[executionRunner.filter] reconciled stale row keys", {
247-
entityId,
248-
strategy,
249-
dropped,
250-
})
251-
}
252-
return next
244+
return reconcileRowDataForEntity(get, entityId, data).data
253245
}
254246

255247
function createConcurrencyLimiter(concurrency: number) {

web/packages/agenta-playground/src/state/execution/webWorkerIntegration.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -345,13 +345,6 @@ export const triggerExecutionAtom = atom(
345345
// Persist the cleaned row (deletes the dropped keys via the
346346
// testcase store's undefined-means-delete semantics).
347347
set(loadableController.actions.updateRow, loadableId, logicalRowId, undefinedPatch)
348-
console.warn("[webWorker.reconcile] cleaned stale row keys before run", {
349-
rootEntityId,
350-
rowId: logicalRowId,
351-
strategy: reconciledRow.strategy,
352-
dropped: reconciledRow.dropped,
353-
protectedColumns: Array.from(protectedColumns),
354-
})
355348
}
356349

357350
// In comparison mode, filter nodes to only include the effective variant's

0 commit comments

Comments
 (0)