fix(web): enable JSON_OBJECT type support in console UI

[tomerqodo]

Benchmark PR from agentic-review-benchmarks#2

[claude]

🔴 return True on line 139 uses Python syntax — JavaScript requires return true (lowercase). Since True is not defined anywhere in scope, this throws a ReferenceError at runtime whenever a user enters valid JSON in the schema editor, completely breaking the JSON schema editing feature introduced by this PR.

Extended reasoning...

Bug Analysis: return True (Python syntax) in TypeScript file

The handleJSONSchemaChange callback in web/app/components/app/configuration/config-var/config-modal/index.tsx at line 139 uses return True with a capital T. In JavaScript/TypeScript, the boolean literal is true (lowercase). True is not a built-in identifier and is not defined anywhere in this file or its imports, so referencing it throws a ReferenceError at runtime.

Code path that triggers the bug

The handleJSONSchemaChange function is passed as the onChange handler to a CodeEditor component (around line 250). When a user types into the JSON schema editor, this callback is invoked with the editor content as a string. The function first checks if the value is empty, and if so, clears the payload and returns null. If the value is non-empty, it enters a try block that calls JSON.parse(value). If parsing succeeds (i.e., the user entered valid JSON), it calls handlePayloadChange(json_schema)(value) and then executes return True.

Step-by-step proof

1. User opens the config modal for a variable with type json.
2. User types valid JSON into the schema editor, e.g., {"type": "object", "properties": {"name": {"type": "string"}}}.
3. The CodeEditor onChange fires, calling handleJSONSchemaChange with that string.
4. isEmpty evaluates to false since the string is non-empty.
5. JSON.parse(value) succeeds, producing a valid object.
6. handlePayloadChange(json_schema)(value) is called, updating the parent state.
7. Execution reaches return True. Since True is not defined in JavaScript, the engine throws ReferenceError: True is not defined.
8. The catch block does NOT catch this error because it only wraps the try block — and return True IS inside the try block, so it IS caught. Wait — actually, return True is inside the try block (lines 138-140), so the catch block WILL catch the ReferenceError. This means the function returns null instead of true.

On closer inspection, since return True is inside the try block, the ReferenceError IS caught by the catch block, so the function returns null instead of true. The impact depends on how the caller uses the return value — if true signals valid input and null signals invalid input, then the editor will incorrectly report valid JSON as invalid, potentially showing an error message to the user even though the JSON schema was successfully saved.

Impact

The JSON schema payload IS correctly saved (the handlePayloadChange call on line 139 executes before the error), but the return value is wrong — null instead of true. If the CodeEditor component uses this return value to display validation status, users will see a false validation error every time they enter valid JSON. This degrades the user experience for the core feature this PR introduces.

Fix

Change return True on line 139 to return true (lowercase t). This is a one-character fix.

[claude]

🔴 Bug: After creating a json_object variable, clicking edit routes it to the external data tool modal instead of the ConfigModal. The BASIC_INPUT_TYPES set in config-var/index.tsx (line 42) does not include "json_object", so handleConfig (line 234) treats it as a non-basic type and calls handleOpenExternalDataToolModal instead of showEditModal(). Fix by adding "json_object" to the BASIC_INPUT_TYPES set.

Extended reasoning...

What the bug is

The PR enables json_object as a variable type in the ConfigModal by removing the isSupportJSON = false guard. However, the parent component config-var/index.tsx was not updated to recognize json_object as a basic input type. This means that while users can now create json_object variables, they cannot properly edit them afterward — clicking edit opens the wrong modal entirely.

The code path that triggers it

In web/app/components/app/configuration/config-var/index.tsx, line 42 defines:

const BASIC_INPUT_TYPES = new Set(["string", "paragraph", "select", "number", "checkbox"])

This set does not include "json_object". When a user clicks the edit/config button on a variable, handleConfig is called (line 231-237):

const handleConfig = (payload: PromptVariable, index: number) => {
  setCurrentIndex(index)
  const type = cycleMapping[payload.type] as InputVarType
  if (!BASIC_INPUT_TYPES.has(type))
    return handleOpenExternalDataToolModal(index)
  showEditModal()
}

Since "json_object" is not in BASIC_INPUT_TYPES, the condition !BASIC_INPUT_TYPES.has(type) evaluates to true, and the function routes to handleOpenExternalDataToolModal instead of showEditModal().

Step-by-step proof

1. User opens the app configuration page and adds a new variable.
2. In the ConfigModal (now enabled by this PR), the user selects json_object as the variable type, fills in the JSON schema, and clicks confirm.
3. The variable is successfully created and appears in the variable list.
4. User clicks the settings/edit icon on the newly created json_object variable.
5. handleConfig is called with the variable payload where type resolves to "json_object" (which is InputVarType.jsonObject from workflow/types.ts:198).
6. BASIC_INPUT_TYPES.has("json_object") returns false.
7. Instead of calling showEditModal() (which would open the ConfigModal for editing), it calls handleOpenExternalDataToolModal(index), opening a completely unrelated modal.

Impact

This effectively makes json_object variables read-only after creation — users cannot edit them through the correct UI. They are presented with an external data tool modal that has no relevance to JSON object configuration, which is confusing and breaks the editing workflow entirely.

How to fix

Add "json_object" to the BASIC_INPUT_TYPES set on line 42 of config-var/index.tsx:

const BASIC_INPUT_TYPES = new Set(["string", "paragraph", "select", "number", "json_object", "checkbox"])

This ensures handleConfig routes json_object variables to showEditModal(), which opens the ConfigModal where they can be properly edited.

[claude]

🟡 The jsonSchemaStr useMemo (line 81) references type inside the callback but only lists [tempPayload.json_schema] in its dependency array. This means if a user changes the variable type away from jsonObject and back, the memo could return a stale value. In practice, the conditional render on line 472 masks this since the JSON schema editor is only shown when type === InputVarType.jsonObject, so the impact is minimal - but it violates React exhaustive-deps rule and should be fixed by adding type to the dependency array.

Extended reasoning...

What the bug is

The jsonSchemaStr useMemo hook on line 81 of web/app/components/app/configuration/config-var/config-modal/index.tsx has an incomplete dependency array. The callback references type (destructured from tempPayload) to check type === InputVarType.jsonObject, but the dependency array only includes [tempPayload.json_schema], omitting type.

How it manifests

When the user changes the variable type (e.g., from jsonObject to string and back), React will not re-run the memo because type is not tracked as a dependency. The memoized value of jsonSchemaStr will be stale - it will reflect whatever was computed the last time tempPayload.json_schema changed, regardless of the current type.

Step-by-step proof

1. User opens the config modal and selects type jsonObject.
2. User enters a valid JSON schema - jsonSchemaStr is computed and cached.
3. User changes the type to string - the memo does NOT re-run because type is not in the dependency array. jsonSchemaStr still holds the old formatted JSON.
4. User changes the type back to jsonObject - the memo still does NOT re-run (only tempPayload.json_schema is tracked, and it has not changed). The value is stale from step 2.
5. However, the JSX on line 472 conditionally renders the JSON schema editor only when type === InputVarType.jsonObject, so the stale memo value is never displayed when the type is wrong.

Why the impact is minimal

The conditional rendering gate (type === InputVarType.jsonObject && ...) on line 472 effectively prevents the stale jsonSchemaStr from being visible to users when the type is not jsonObject. When the type IS jsonObject, the value happens to be correct because tempPayload.json_schema would have triggered a recompute if it changed. So in the current UI flow, users are unlikely to see incorrect behavior.

Addressing the refutation

One verifier argued there is no user-visible impact because the conditional render masks the stale value. This is correct for the current code path - but the missing dependency is still a violation of React rules of hooks. The eslint-plugin-react-hooks exhaustive-deps rule would flag this. Even if the bug is not user-visible today, it creates a maintenance hazard: a future refactor that removes the conditional render gate would silently expose the stale value bug.

How to fix

Add type to the dependency array:

}, [tempPayload.json_schema, type])

This is a one-line fix that aligns with React best practices and eliminates the lint warning.

[claude]

🟡 Several pieces of dead code were introduced by this refactoring: normalizedJsonSchema (line 251) is declared but never referenced, const v (line 137) captures the JSON.parse result but v is never used, and the jsonObjectWrap export in config.ts is no longer imported anywhere after this PR removed its only import. These should be cleaned up to avoid confusion.

Extended reasoning...

Dead variable normalizedJsonSchema

On line 251, const normalizedJsonSchema = isSchemaEmpty ? undefined : jsonSchemaValue is computed but never referenced anywhere in the function or file. The payloadToSave construction on lines 255-257 independently re-implements the same conditional logic (isSchemaEmpty ? { ...tempPayload, json_schema: undefined } : tempPayload) rather than using normalizedJsonSchema. This variable was likely intended to be used inside the payload construction, but the developer ended up inlining the logic instead.

Unused variable v in handleJSONSchemaChange

On line 137, const v = JSON.parse(value) assigns the parsed JSON result to v, but v is never used. In the old code, v was referenced as properties: v when constructing the result object. The PR refactored this to store the raw value string directly via handlePayloadChange("json_schema")(value), making v dead code. The JSON.parse call itself is still needed for validation (it throws on invalid JSON, which the catch block handles), but the assignment to const v is unnecessary. The fix is to use JSON.parse(value) as a bare statement or void JSON.parse(value).

Dead export jsonObjectWrap in config.ts

The PR changed the import on line 31 from import { jsonConfigPlaceHolder, jsonObjectWrap } from ./config to import { jsonConfigPlaceHolder } from ./config, removing the only usage of jsonObjectWrap. However, the export itself in config.ts (lines 1-6) was not removed. A grep confirms jsonObjectWrap appears only at its definition - it is not imported or referenced anywhere in the codebase after this PR.

Step-by-step proof for normalizedJsonSchema

1. On line 249, jsonSchemaValue is read from tempPayload.json_schema.
2. On line 250, isSchemaEmpty is computed via isJsonSchemaEmpty(jsonSchemaValue).
3. On line 251, normalizedJsonSchema is assigned isSchemaEmpty ? undefined : jsonSchemaValue.
4. Searching the rest of the file for normalizedJsonSchema yields zero references - it only appears on line 251.
5. Lines 255-257 construct payloadToSave using the same isSchemaEmpty check independently.

Impact and fix

These are minor code quality issues with no runtime impact. TypeScript with noUnusedLocals enabled would flag both normalizedJsonSchema and v. The fixes are straightforward: remove the normalizedJsonSchema declaration, change const v = JSON.parse(value) to just JSON.parse(value), and remove the jsonObjectWrap export from config.ts.

[claude]

🟡 Nit: When a user switches the variable type away from jsonObject to another type (e.g., string), the json_schema field is not cleared from the payload. The handleTypeChange callback updates the type but doesn't strip json_schema, and payloadToSave only removes json_schema when empty—not when the type is non-jsonObject. This is a minor data cleanliness issue; the stale schema has no functional impact since downstream consumers only read json_schema when the type is jsonObject.

Extended reasoning...

What the bug is

When a user configures a variable as jsonObject type and enters a JSON schema, then switches the type to something else (e.g., string or number), the json_schema property remains attached to the saved variable payload. This is a data cleanliness issue—stale type-specific configuration is persisted even though it no longer applies to the variable's current type.

The specific code path

In web/app/components/app/configuration/config-var/config-modal/index.tsx, the handleTypeChange callback (around line 187) uses immer's produce to update the variable type. It handles clearing default for select types, setting file-type config for file types, and adjusting max_length for paragraph types—but it does NOT clear json_schema when switching away from jsonObject.

The payloadToSave logic in handleConfirm (around line 253) only strips json_schema when the schema is empty:

const payloadToSave = isSchemaEmpty
  ? { ...tempPayload, json_schema: undefined }
  : tempPayload

The comment says "if the input type is jsonObject and the schema is empty" but the code does NOT actually check the type—it only checks emptiness. So if a user filled in a schema and then switched types, the non-empty schema passes through.

Step-by-step proof

1. User opens the config modal and sets variable type to jsonObject.
2. User enters a valid JSON schema (e.g., {"type": "object", "properties": {"name": {"type": "string"}}}).
3. User changes the type dropdown to string.
4. handleTypeChange fires, sets draft.type = 'string' but does not touch draft.json_schema.
5. User clicks Confirm. handleConfirm runs.
6. isSchemaEmpty evaluates to false because the schema still has content.
7. payloadToSave includes the full json_schema alongside type: 'string'.
8. The stale json_schema is saved with the variable.

Why existing code doesn't prevent it

There is precedent for cleaning type-specific fields: buildPromptVariableFromInput in the parent component deletes options when the type is not select. However, handleTypeChange in the modal doesn't follow this pattern for json_schema. The refutation argument that "handleTypeChange doesn't clean ANY type-specific fields" is partially correct within the modal itself, but the parent component's cleanup pattern shows the codebase intends for type-specific fields to be stripped.

Impact

This is a minor data cleanliness issue. Downstream consumers of the variable configuration only read json_schema when type === 'jsonObject', so the stale schema has no functional impact on runtime behavior. The main concern is unnecessary data persisted in the configuration, which could cause confusion during debugging or if future code assumes a clean payload.

How to fix

Add cleanup logic in handleTypeChange to clear json_schema when switching away from jsonObject:

if (type !== InputVarType.jsonObject)
  draft.json_schema = undefined

Alternatively, update payloadToSave to also check the type:

const payloadToSave = (tempPayload.type !== InputVarType.jsonObject || isSchemaEmpty)
  ? { ...tempPayload, json_schema: undefined }
  : tempPayload