fix: derive what an emptied form field means from its schema

A cleared text box hands back "", and for a field like group_column an
empty string is not "no value", it is a column named "". That reached
scikit-learn, or got persisted and raised a KeyError in a Huey worker much
later. 77 fields accept string-or-null today, including every class_weight
and max_features in the sklearn wrappers.

The int half of this was fixed years ago in the leaf inputs, where
IntegerInput and NumberInput each coerce "" to null themselves, and it was
the visible half because "" fails int validation loudly. The string half
was never fixed, because it fails silently.

The historical attempt went the other way, translating "" to None during
validation, and there was no good place for it: from a bare string the
schema layer cannot tell whether the author meant "unset" or "the empty
string". What made it decidable is that the schema already says both
things, and nobody was reading them:

  - the field does not admit null -> "" is a value, leave it
  - it admits null and its placeholder is "" -> empty means "", which is the
    14 negative_prompt fields across the diffusion models and exactly why a
    global rule would have been wrong for them
  - it admits null and its placeholder is anything else -> empty means unset

So emptyValueFor derives it per field with no new keyword, no authoring
burden and no backend change, and normalizeEmptyValue is applied at the one
point where a form reports a change, so every input type is covered and no
leaf component has to know about it. The pipelines dispatcher calls the same
shared helper rather than growing its own copy, since the two must not
disagree.

Also removes the display of the literal word "none" for a null value
(TextInput, a work-in-progress line from March 2024). It stayed harmless
only because the null branch renders that input disabled: with the input
enabled it would be a submittable string, and pydantic does not parse
"none" to None, it keeps it as the string 'none'. And passes the
`placeholder` prop that FormSchemaFieldWithOptions documents and reads but
neither of its two call sites ever provided, which is why leaving the Null
chip used to set the value to undefined.

Not changed: the backend still accepts "" for a nullable string. Tightening
that would reject params already persisted, so it is a separate decision
with a data-migration dimension rather than something to ride along here.

Author: cristian-tamblay

DashAI/front/src/components/configurableObject/Inputs/TextInput.jsx

@@ -30,7 +30,11 @@ function TextInput({
         size="small"
         name={name}
         label={label}
-        value={value === null ? "none" : value}
+        // A null value is an empty box, not the word "none". Displaying the
+        // literal was a work-in-progress line from 2024 that only stayed
+        // harmless because the null branch renders this input disabled: with
+        // the input enabled it would be a submittable string.
+        value={value ?? ""}
         onChange={onChange}
         autoComplete="off"
         error={!!showError}

DashAI/front/src/components/pipelines/ParamsSettings.jsx

@@ -15,7 +15,7 @@ import FormSchemaFieldWithOptions from "../../components/shared/FormSchemaFieldW
 import FormSchemaFieldWithCollapse from "../../components/shared/FormSchemaFieldWithCollapse";
 import FormSchemaFieldWithOptimizers from "../../components/shared/FormSchemaFieldWithOptimizers";
 import FormSchemaFieldWithParent from "../../components/shared/FormSchemaFieldWithParent";
-import { getModelFromSubform } from "../../utils/schema";
+import { getModelFromSubform, normalizeEmptyValue } from "../../utils/schema";
 
 function ParamsSettings({
   open,
@@ -64,7 +64,14 @@ function ParamsSettings({
       const value = localValues?.[objName];
 
       const fieldOnChange = (fieldValue) => {
-        handleFieldChange(objName, fieldValue);
+        // Same shared helper as the main renderer: this fork exists only
+        // because the shared one needs a provider context, so the empty-value
+        // semantics must not differ between the two. When the two dispatchers
+        // are collapsed, this call site goes with them.
+        handleFieldChange(
+          objName,
+          normalizeEmptyValue(fieldValue, fieldSchema),
+        );
       };
 
       if ("anyOf" in fieldSchema) {
@@ -76,6 +83,7 @@ function ParamsSettings({
             options={fieldSchema.anyOf}
             required={fieldSchema.required}
             objName={objName}
+            placeholder={fieldSchema.placeholder}
             field={{
               value: value,
               onChange: fieldOnChange,

DashAI/front/src/components/shared/FormSchemaRenderFields.jsx

@@ -5,7 +5,11 @@ import FormSchemaFieldWithOptions from "./FormSchemaFieldWithOptions";
 import FormSchemaFieldWithCollapse from "./FormSchemaFieldWithCollapse";
 import FormSchemaFieldWithOptimizers from "./FormSchemaFieldWithOptimizers";
 import FormSchemaFieldWithParent from "./FormSchemaFieldWithParent";
-import { getModelFromSubform, getSchemaRules } from "../../utils/schema";
+import {
+  getModelFromSubform,
+  getSchemaRules,
+  normalizeEmptyValue,
+} from "../../utils/schema";
 import { evaluateRules } from "../../utils/ruleEngine";
 import { Stack } from "@mui/material";
 import PropTypes from "prop-types";
@@ -63,8 +67,15 @@ function FormSchemaRenderFields({
   if (!modelSchema) return null;
 
   const handleChange = useCallback(
-    (name, subName) => (value) => {
+    (name, subName) => (rawValue) => {
       const fieldPath = subName ? `${name}.${subName}` : name;
+      // One place decides what an emptied input means, so no leaf input has to
+      // know: a cleared box on a nullable field submits null instead of the
+      // empty string that used to reach sklearn as a column named "".
+      const fieldSchema = subName
+        ? modelSchema?.[name]?.properties?.[subName]
+        : modelSchema?.[name];
+      const value = normalizeEmptyValue(rawValue, fieldSchema);
       formik.setFieldValue(fieldPath, value, true);
       // Always pass complete formik.values so handleUpdateSchema receives
       // ALL fields regardless of whether the context store has been
@@ -74,7 +85,7 @@ function FormSchemaRenderFields({
         autoSave ? onFormSubmit : null,
       );
     },
-    [formik, handleUpdateSchema, autoSave, onFormSubmit],
+    [formik, handleUpdateSchema, autoSave, onFormSubmit, modelSchema],
   );
 
   // Which fields the schema's own rules say are meaningful right now. The
@@ -144,6 +155,7 @@ function FormSchemaRenderFields({
             setError={setError}
             field={baseField}
             disabled={disabled}
+            placeholder={fieldSchema.placeholder}
           />,
         );
       } else if (isOptimizable) {

DashAI/front/src/components/shared/FormSchemaRenderFields.test.jsx

@@ -16,7 +16,7 @@
  */
 
 import React from "react";
-import { screen, within } from "@testing-library/react";
+import { screen, waitFor, within } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 
 // react-markdown ships ESM only and Create React App's jest does not transform
@@ -317,3 +317,80 @@ describe("relevance on a union field, the SimpleImputer case", () => {
     expect(inputFor(container, "strategy")).not.toBeDisabled();
   });
 });
+
+describe("an emptied nullable field", () => {
+  // The oldest defect on this path: a cleared box handed back "" and the
+  // backend stored a column named "", which sklearn then choked on. The
+  // historical workaround displayed the word "none" for a null value, which
+  // survived only because the null branch renders the input disabled.
+  const nullableSchema = (placeholder) => ({
+    title: "S",
+    type: "object",
+    required: ["group_column"],
+    properties: {
+      group_column: {
+        anyOf: [{ type: "string" }, { type: "null" }],
+        placeholder,
+        title: "Group column",
+        description: "The column to group by.",
+      },
+    },
+  });
+
+  async function renderNullable(value, placeholder = null) {
+    const modelSchema = await formattedModel(nullableSchema(placeholder));
+    const formik = fakeFormik({ group_column: value });
+    const result = renderWithProviders(
+      <FormSchemaRenderFields
+        modelSchema={modelSchema}
+        formik={formik}
+        handleUpdateSchema={jest.fn()}
+      />,
+    );
+    return { ...result, formik };
+  }
+
+  it("shows an empty box for a null value, not the word none", async () => {
+    const { container } = await renderNullable(null);
+    const input = inputFor(container, "group_column");
+    expect(input).toHaveValue("");
+    expect(screen.queryByDisplayValue("none")).not.toBeInTheDocument();
+  });
+
+  it("stores null when the user clears it", async () => {
+    const { container, formik } = await renderNullable("some_column");
+    const input = inputFor(container, "group_column");
+    expect(input).not.toBeDisabled();
+
+    await userEvent.clear(input);
+
+    await waitFor(() =>
+      expect(formik.setFieldValue).toHaveBeenCalledWith(
+        "group_column",
+        null,
+        true,
+      ),
+    );
+    // Never the empty string, which is the value that used to reach sklearn.
+    expect(formik.setFieldValue).not.toHaveBeenCalledWith(
+      "group_column",
+      "",
+      true,
+    );
+  });
+
+  it("keeps the empty string when the author chose it as the default", async () => {
+    // The diffusion models' negative_prompt: same wire shape, opposite
+    // meaning, which is why this is derived per field instead of globally.
+    const { container, formik } = await renderNullable("a prompt", "");
+    await userEvent.clear(inputFor(container, "group_column"));
+
+    await waitFor(() =>
+      expect(formik.setFieldValue).toHaveBeenCalledWith(
+        "group_column",
+        "",
+        true,
+      ),
+    );
+  });
+});

DashAI/front/src/utils/schema.js

@@ -43,6 +43,57 @@ export async function resolveDefaults(
   }
 }
 
+/**
+ * What an emptied input means for a given field.
+ *
+ * The problem this answers is old: a cleared text box hands back `""`, and for
+ * a field like `group_column` an empty string is not "no value", it is a column
+ * named "". sklearn then fails on it, or the backend stores it and a Huey worker
+ * raises a KeyError much later. The historical patch went the other way, trying
+ * to translate `""` to None during validation, and there was no good place for
+ * it: the schema layer cannot tell, from a bare string, whether the author meant
+ * "unset" or "the empty string".
+ *
+ * It can be derived instead, from what the schema already says on the wire:
+ *
+ *  - The field does not admit null: `""` is a value like any other, so it stays.
+ *    A required field then reports itself empty, which is correct.
+ *  - It admits null and its placeholder is `""`: the author chose the empty
+ *    string as the default, so that is what empty means. This is the case of the
+ *    14 `negative_prompt` fields across the diffusion models, and it is exactly
+ *    why a single global rule would have been wrong.
+ *  - It admits null and its placeholder is anything else: empty means unset.
+ *
+ * So there is no new keyword, no authoring burden and no backend change. The
+ * answer was already in the schema; nobody was reading it.
+ *
+ * @param {object} subSchema one property of a formatted schema
+ * @returns {null|string} the value an emptied input should submit
+ */
+export const emptyValueFor = (subSchema) => {
+  const branches = Array.isArray(subSchema?.anyOf) ? subSchema.anyOf : [];
+  const admitsNull =
+    branches.some((branch) => branch.type === "null") ||
+    subSchema?.type === "null";
+  if (!admitsNull) return "";
+  return subSchema?.placeholder === "" ? "" : null;
+};
+
+/**
+ * Replace an emptied input's value with what empty means for that field.
+ *
+ * Applied at the single point where a form reports a change, so every input
+ * type is covered and no leaf component has to know about it.
+ *
+ * @param {*} value the value the input handed back
+ * @param {object} subSchema the schema of the field that changed
+ * @returns {*} the value to store
+ */
+export const normalizeEmptyValue = (value, subSchema) => {
+  if (value !== "" && value !== undefined) return value;
+  return emptyValueFor(subSchema);
+};
+
 // Generate a Yup schema from a JSON schema object based on the JSON schema specification from the api, it also generates the initial values of the form
 export const generateYupSchema = (schemaObj) => {
   const schema = {};

DashAI/front/src/utils/schema.test.js

@@ -14,9 +14,11 @@ import { validateYupSchema, yupToFormErrors } from "formik";
 
 import {
   SCHEMA_RULES,
+  emptyValueFor,
   formattedModel,
   generateYupSchema,
   getSchemaRules,
+  normalizeEmptyValue,
 } from "./schema";
 import {
   holdoutWireSchema,
@@ -177,3 +179,58 @@ describe("generateYupSchema enforces the declared rules", () => {
     expect(errors.validation).toMatch(/must sum to 1/);
   });
 });
+
+describe("what an emptied input means, derived from the schema", () => {
+  // The old question, answered from data instead of by a global rule. The
+  // schema already says whether null is allowed and what the author chose as
+  // the default; between those two facts there is no ambiguity left.
+  const nullableColumn = {
+    anyOf: [{ type: "string" }, { type: "null" }],
+    placeholder: null,
+  };
+  const nullablePrompt = {
+    anyOf: [{ type: "string" }, { type: "null" }],
+    placeholder: "",
+  };
+  const plainString = { type: "string", placeholder: "abc" };
+  const nullableInt = {
+    anyOf: [{ type: "integer" }, { type: "null" }],
+    placeholder: null,
+  };
+
+  it("means unset for a nullable field whose default is not the empty string", () => {
+    // group_column and the other 76 string-or-null fields: an empty string
+    // here is a column named "", which is what reached sklearn.
+    expect(emptyValueFor(nullableColumn)).toBeNull();
+    expect(emptyValueFor(nullableInt)).toBeNull();
+  });
+
+  it("means the empty string when the author chose it as the default", () => {
+    // The 14 negative_prompt fields across the diffusion models. A single
+    // global "empty means null" rule would be wrong for exactly these.
+    expect(emptyValueFor(nullablePrompt)).toBe("");
+  });
+
+  it("leaves a non-nullable field alone", () => {
+    expect(emptyValueFor(plainString)).toBe("");
+  });
+
+  it("tolerates a field it knows nothing about", () => {
+    expect(emptyValueFor(undefined)).toBe("");
+    expect(emptyValueFor({})).toBe("");
+  });
+
+  it("only rewrites empty values, never real ones", () => {
+    expect(normalizeEmptyValue("abc", nullableColumn)).toBe("abc");
+    expect(normalizeEmptyValue(0, nullableInt)).toBe(0);
+    expect(normalizeEmptyValue(false, nullableColumn)).toBe(false);
+    expect(normalizeEmptyValue(null, nullableColumn)).toBeNull();
+  });
+
+  it("treats a cleared box and an absent value the same way", () => {
+    expect(normalizeEmptyValue("", nullableColumn)).toBeNull();
+    expect(normalizeEmptyValue(undefined, nullableColumn)).toBeNull();
+    expect(normalizeEmptyValue("", nullablePrompt)).toBe("");
+    expect(normalizeEmptyValue(undefined, nullablePrompt)).toBe("");
+  });
+});