fix(ui): allow in-place editing of classifier numeric inputs (#38803)

Backspacing the last digit of Context Window Size instantly refilled the default (3), since onChange mapped empty input to null and the handler coalesced null back to the default. The same defect affected Timeout (ms) and Context Character Budget. Add per-field raw draft state so an empty or partial value stays visible while focused, commit only finite values (rounded, clamped to each field's minimum), and clear the draft on blur so an abandoned edit falls back to the committed value. 0 stays a valid committed value for both context controls. Add stable ids and label associations; update tests to query by label
This commit is contained in:
tin-berri 2026-08-29 14:43:20 -07:00 committed by GitHub
parent 20cfccaf5f
commit 5c034fda74
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 114 additions and 43 deletions

View file

@ -40,6 +40,10 @@ const DEFAULT_SCORING_EXPLANATION =
"The router scores each request across 7 dimensions: token count, code presence, reasoning markers, technical " +
"terms, simple indicators, multi-step patterns, and question complexity. The weighted score determines the tier:";
const CLASSIFIER_TIMEOUT_ID = "classifier-timeout-ms";
const CLASSIFIER_CONTEXT_WINDOW_SIZE_ID = "classifier-context-window-size";
const CLASSIFIER_CONTEXT_BUDGET_CHARS_ID = "classifier-context-budget-chars";
const CUSTOM_PROMPT_WITH_HEURISTIC_FALLBACK =
"This router classifies with your own prompt, so the tier comes from whatever rubric it states. The four tier " +
"names stay fixed. The scoring below is the heuristic, which now runs only when the classifier call fails:";
@ -204,6 +208,7 @@ const ClassificationMethodConfig: React.FC<ClassificationMethodConfigProps> = ({
showValidationErrors = false,
defaultModel,
}) => {
const [draft, setDraft] = React.useState<{ id: string; raw: string } | null>(null);
const hasDefaultModel = Boolean(defaultModel);
const classifierType = effectiveClassifierType(value);
const classifierModelMissing =
@ -261,13 +266,13 @@ const ClassificationMethodConfig: React.FC<ClassificationMethodConfigProps> = ({
});
};
const handleClassifierTimeoutChange = (timeoutMs: number | null) => {
const handleClassifierTimeoutChange = (timeoutMs: number) => {
onChange({
...value,
classifier_llm_config: {
...value.classifier_llm_config,
model: value.classifier_llm_config?.model ?? "",
timeout_ms: timeoutMs ?? DEFAULT_CLASSIFIER_TIMEOUT_MS,
timeout_ms: timeoutMs,
},
});
};
@ -300,20 +305,32 @@ const ClassificationMethodConfig: React.FC<ClassificationMethodConfigProps> = ({
onChange({ ...value, classifier_fallback: fallback });
};
const handleClassifierContextWindowSizeChange = (windowSize: number | null) => {
const handleClassifierContextWindowSizeChange = (windowSize: number) => {
onChange({
...value,
classifier_context_window_size: windowSize ?? DEFAULT_CLASSIFIER_CONTEXT_WINDOW_SIZE,
classifier_context_window_size: windowSize,
});
};
const handleClassifierContextBudgetCharsChange = (budgetChars: number | null) => {
const handleClassifierContextBudgetCharsChange = (budgetChars: number) => {
onChange({
...value,
classifier_context_budget_chars: budgetChars ?? DEFAULT_CLASSIFIER_CONTEXT_BUDGET_CHARS,
classifier_context_budget_chars: budgetChars,
});
};
const handleClassifierIntegerChange = (
id: string,
raw: string,
minimum: number,
onCommit: (value: number) => void,
) => {
setDraft({ id, raw });
const parsed = Number(raw);
if (raw.trim() === "" || !Number.isFinite(parsed)) return;
onCommit(Math.max(minimum, Math.round(parsed)));
};
const handleClassifierContextIncludeAssistantTurnsChange = (includeAssistantTurns: boolean) => {
onChange({
...value,
@ -366,14 +383,27 @@ const ClassificationMethodConfig: React.FC<ClassificationMethodConfigProps> = ({
{classifierModelMissing && <span className="text-xs text-destructive">A classifier model is required</span>}
</div>
<div>
<strong className="block mb-1 font-semibold">Timeout (ms)</strong>
<Label htmlFor={CLASSIFIER_TIMEOUT_ID} className="block mb-1 font-semibold">
Timeout (ms)
</Label>
<Input
type="number"
value={value.classifier_llm_config?.timeout_ms ?? DEFAULT_CLASSIFIER_TIMEOUT_MS}
onChange={(event) =>
handleClassifierTimeoutChange(event.target.value === "" ? null : event.target.valueAsNumber)
id={CLASSIFIER_TIMEOUT_ID}
type="text"
inputMode="numeric"
value={
draft?.id === CLASSIFIER_TIMEOUT_ID
? draft.raw
: String(value.classifier_llm_config?.timeout_ms ?? DEFAULT_CLASSIFIER_TIMEOUT_MS)
}
min={1}
onChange={(event) =>
handleClassifierIntegerChange(
CLASSIFIER_TIMEOUT_ID,
event.target.value,
1,
handleClassifierTimeoutChange,
)
}
onBlur={() => setDraft(null)}
className="w-full"
/>
<span className="text-xs text-muted-foreground">
@ -480,14 +510,27 @@ const ClassificationMethodConfig: React.FC<ClassificationMethodConfigProps> = ({
</span>
</RestrictedSection>
<div>
<strong className="block mb-1 font-semibold">Context Window Size</strong>
<Label htmlFor={CLASSIFIER_CONTEXT_WINDOW_SIZE_ID} className="block mb-1 font-semibold">
Context Window Size
</Label>
<Input
type="number"
value={value.classifier_context_window_size ?? DEFAULT_CLASSIFIER_CONTEXT_WINDOW_SIZE}
onChange={(event) =>
handleClassifierContextWindowSizeChange(event.target.value === "" ? null : event.target.valueAsNumber)
id={CLASSIFIER_CONTEXT_WINDOW_SIZE_ID}
type="text"
inputMode="numeric"
value={
draft?.id === CLASSIFIER_CONTEXT_WINDOW_SIZE_ID
? draft.raw
: String(value.classifier_context_window_size ?? DEFAULT_CLASSIFIER_CONTEXT_WINDOW_SIZE)
}
min={0}
onChange={(event) =>
handleClassifierIntegerChange(
CLASSIFIER_CONTEXT_WINDOW_SIZE_ID,
event.target.value,
0,
handleClassifierContextWindowSizeChange,
)
}
onBlur={() => setDraft(null)}
className="w-full"
/>
<span className="text-xs text-muted-foreground">
@ -497,14 +540,27 @@ const ClassificationMethodConfig: React.FC<ClassificationMethodConfigProps> = ({
</span>
</div>
<div>
<strong className="block mb-1 font-semibold">Context Character Budget</strong>
<Label htmlFor={CLASSIFIER_CONTEXT_BUDGET_CHARS_ID} className="block mb-1 font-semibold">
Context Character Budget
</Label>
<Input
type="number"
value={value.classifier_context_budget_chars ?? DEFAULT_CLASSIFIER_CONTEXT_BUDGET_CHARS}
onChange={(event) =>
handleClassifierContextBudgetCharsChange(event.target.value === "" ? null : event.target.valueAsNumber)
id={CLASSIFIER_CONTEXT_BUDGET_CHARS_ID}
type="text"
inputMode="numeric"
value={
draft?.id === CLASSIFIER_CONTEXT_BUDGET_CHARS_ID
? draft.raw
: String(value.classifier_context_budget_chars ?? DEFAULT_CLASSIFIER_CONTEXT_BUDGET_CHARS)
}
min={0}
onChange={(event) =>
handleClassifierIntegerChange(
CLASSIFIER_CONTEXT_BUDGET_CHARS_ID,
event.target.value,
0,
handleClassifierContextBudgetCharsChange,
)
}
onBlur={() => setDraft(null)}
className="w-full"
/>
<span className="text-xs text-muted-foreground">

View file

@ -131,10 +131,8 @@ describe("ComplexityRouterConfig", () => {
fireEvent.click(screen.getByText("Advanced: Classification Method"));
expect(screen.getByText("Classifier Model")).toBeInTheDocument();
expect(screen.getByText("Timeout (ms)")).toBeInTheDocument();
expect(screen.getByDisplayValue("750")).toBeInTheDocument();
expect(screen.getByText("Context Window Size")).toBeInTheDocument();
expect(screen.getByDisplayValue("5")).toBeInTheDocument();
expect(screen.getByLabelText("Timeout (ms)")).toHaveValue("750");
expect(screen.getByLabelText("Context Window Size")).toHaveValue("5");
expect(screen.queryByText("Context Per-Turn Character Limit")).not.toBeInTheDocument();
});
@ -148,11 +146,8 @@ describe("ComplexityRouterConfig", () => {
fireEvent.click(screen.getByText("Advanced: Classification Method"));
const windowSizeSection = screen.getByText("Context Window Size").closest("div") as HTMLElement;
expect(within(windowSizeSection).getByDisplayValue("3")).toBeInTheDocument();
const budgetSection = screen.getByText("Context Character Budget").closest("div") as HTMLElement;
expect(within(budgetSection).getByDisplayValue("8000")).toBeInTheDocument();
expect(screen.getByLabelText("Context Window Size")).toHaveValue("3");
expect(screen.getByLabelText("Context Character Budget")).toHaveValue("8000");
});
it("should warn when the budget is too small to quote any turn that does not already fit", () => {
@ -247,7 +242,11 @@ describe("ComplexityRouterConfig", () => {
expect(screen.queryByText("Context Per-Turn Character Limit")).not.toBeInTheDocument();
});
it("should call onChange with the updated classifier_context_window_size when edited", () => {
it.each([
["Timeout (ms)", "7", { classifier_llm_config: { model: "gpt-3.5-turbo", timeout_ms: 7 } }],
["Context Window Size", "0", { classifier_context_window_size: 0 }],
["Context Character Budget", "7", { classifier_context_budget_chars: 7 }],
])("keeps %s empty while it is being edited, then commits %s", (label, replacement, expected) => {
const onChange = vi.fn();
const llmValue: ComplexityRouterConfigValue = {
...defaultValue,
@ -257,14 +256,31 @@ describe("ComplexityRouterConfig", () => {
renderWithProviders(<ComplexityRouterConfig modelInfo={mockModelInfo} value={llmValue} onChange={onChange} />);
fireEvent.click(screen.getByText("Advanced: Classification Method"));
const windowSizeSection = screen.getByText("Context Window Size").closest("div") as HTMLElement;
const input = within(windowSizeSection).getByRole("spinbutton");
fireEvent.change(input, { target: { value: "7" } });
const input = screen.getByLabelText(label);
fireEvent.change(input, { target: { value: "" } });
expect(onChange).toHaveBeenCalledWith({
...llmValue,
classifier_context_window_size: 7,
});
expect(input).toHaveValue("");
expect(onChange).not.toHaveBeenCalled();
fireEvent.change(input, { target: { value: replacement } });
expect(onChange).toHaveBeenLastCalledWith({ ...llmValue, ...expected });
});
it("restores the committed context window size after an empty field loses focus", () => {
const llmValue: ComplexityRouterConfigValue = {
...defaultValue,
classifier_type: "llm",
classifier_llm_config: { model: "gpt-3.5-turbo", timeout_ms: 3000 },
};
renderWithProviders(<ComplexityRouterConfig modelInfo={mockModelInfo} value={llmValue} onChange={vi.fn()} />);
fireEvent.click(screen.getByText("Advanced: Classification Method"));
const input = screen.getByLabelText("Context Window Size");
fireEvent.change(input, { target: { value: "" } });
fireEvent.blur(input);
expect(input).toHaveValue("3");
});
it("should render the custom technical keywords field", () => {

View file

@ -295,8 +295,7 @@ describe("EditAutoRouterModal classifier context window", () => {
renderLlmModal();
await user.click(await screen.findByText("Advanced: Classification Method"));
const windowSizeSection = (await screen.findByText("Context Window Size")).closest("div") as HTMLElement;
const input = within(windowSizeSection).getByRole("spinbutton");
const input = await screen.findByLabelText("Context Window Size");
fireEvent.change(input, { target: { value: "8" } });
await user.click(screen.getByRole("button", { name: /save changes/i }));