Skip to content

Commit dbb244a

Browse files
committed
fix(provider): skip top p when custom setup temperature is set
The schema already refuses both sampling fields on the wire, but the wizard still walked temperature then top p and only failed at save.
1 parent f4dfc0b commit dbb244a

3 files changed

Lines changed: 189 additions & 3 deletions

File tree

‎src/tui/provider/setup.ts‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,14 @@ import {
6969
stopRamp,
7070
teardownSurface,
7171
} from "./surface.js";
72-
import { stepsFor, type ProviderField, type SetupStep } from "./steps.js";
72+
import {
73+
nextSamplingStepIndex,
74+
previousSamplingStepIndex,
75+
stepsFor,
76+
topPAfterTemperatureAdvance,
77+
type ProviderField,
78+
type SetupStep,
79+
} from "./steps.js";
7380
import type {
7481
ProviderPreset,
7582
ProviderSetupConfig,
@@ -308,6 +315,10 @@ export async function runProviderSetup(
308315
state.submitting = true;
309316
state.submitPhase = "testing";
310317
clearError();
318+
state.values.topP = topPAfterTemperatureAdvance(
319+
state.values.temperature,
320+
state.values.topP,
321+
);
311322
surface.paint();
312323
stopRamp(state);
313324
state.rampTimer = setInterval(() => surface.paintStatus(), RAMP_TICK_MS);
@@ -449,7 +460,17 @@ export async function runProviderSetup(
449460
if (!stepReady(field, state.values[field])) return;
450461

451462
if (state.stepIndex < steps().length - 1) {
452-
state.stepIndex += 1;
463+
if (field === "temperature") {
464+
state.values.topP = topPAfterTemperatureAdvance(
465+
state.values.temperature,
466+
state.values.topP,
467+
);
468+
}
469+
state.stepIndex = nextSamplingStepIndex(
470+
steps(),
471+
state.stepIndex,
472+
state.values.temperature,
473+
);
453474
clearError();
454475
if (isListStep()) enterModelList();
455476
showStep();
@@ -466,7 +487,11 @@ export async function runProviderSetup(
466487
}
467488
if (isGoModelListStep()) discovery.abandonGoPrefetch();
468489
if (isZenModelListStep()) discovery.abandonZenPrefetch();
469-
state.stepIndex -= 1;
490+
state.stepIndex = previousSamplingStepIndex(
491+
steps(),
492+
state.stepIndex,
493+
state.values.temperature,
494+
);
470495
clearError();
471496
if (currentStep() === "provider") enterProviderList();
472497
else if (isListStep()) enterModelList();

‎src/tui/provider/steps.test.ts‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
import {
4+
CUSTOM_STEPS,
5+
OAUTH_STEPS,
6+
OLLAMA_STEPS,
7+
PRESET_STEPS,
8+
nextSamplingStepIndex,
9+
previousSamplingStepIndex,
10+
skipTopPWhenTemperatureSet,
11+
topPAfterTemperatureAdvance,
12+
} from "./steps.js";
13+
14+
const temperatureIndex = CUSTOM_STEPS.indexOf("temperature");
15+
const topPIndex = CUSTOM_STEPS.indexOf("topP");
16+
const effortsIndex = CUSTOM_STEPS.indexOf("efforts");
17+
const maxTokensIndex = CUSTOM_STEPS.indexOf("maxTokens");
18+
const defaultEffortIndex = CUSTOM_STEPS.indexOf("defaultEffort");
19+
20+
describe("custom provider sampling-step skip", () => {
21+
test("CUSTOM_STEPS includes sampling and effort steps", () => {
22+
expect(temperatureIndex).toBeGreaterThanOrEqual(0);
23+
expect(topPIndex).toBeGreaterThanOrEqual(0);
24+
expect(effortsIndex).toBeGreaterThanOrEqual(0);
25+
expect(maxTokensIndex).toBeGreaterThanOrEqual(0);
26+
expect(defaultEffortIndex).toBeGreaterThanOrEqual(0);
27+
});
28+
29+
test("nextSamplingStepIndex skips topP when temperature is set", () => {
30+
expect(nextSamplingStepIndex(CUSTOM_STEPS, temperatureIndex, "0.7")).toBe(
31+
effortsIndex,
32+
);
33+
expect(nextSamplingStepIndex(CUSTOM_STEPS, temperatureIndex, "0")).toBe(
34+
effortsIndex,
35+
);
36+
});
37+
38+
test("nextSamplingStepIndex does not skip topP when temperature is blank", () => {
39+
expect(nextSamplingStepIndex(CUSTOM_STEPS, temperatureIndex, "")).toBe(
40+
topPIndex,
41+
);
42+
expect(nextSamplingStepIndex(CUSTOM_STEPS, temperatureIndex, " ")).toBe(
43+
topPIndex,
44+
);
45+
});
46+
47+
test("nextSamplingStepIndex advances from topP to efforts", () => {
48+
expect(nextSamplingStepIndex(CUSTOM_STEPS, topPIndex, "")).toBe(
49+
effortsIndex,
50+
);
51+
expect(nextSamplingStepIndex(CUSTOM_STEPS, topPIndex, "0.7")).toBe(
52+
effortsIndex,
53+
);
54+
});
55+
56+
test("nextSamplingStepIndex does not skip the temperature step", () => {
57+
expect(nextSamplingStepIndex(CUSTOM_STEPS, maxTokensIndex, "0.7")).toBe(
58+
temperatureIndex,
59+
);
60+
});
61+
62+
test("nextSamplingStepIndex is +1 on lists without sampling steps", () => {
63+
expect(nextSamplingStepIndex(PRESET_STEPS, 0, "0.7")).toBe(1);
64+
expect(nextSamplingStepIndex(OAUTH_STEPS, 0, "0.7")).toBe(1);
65+
expect(nextSamplingStepIndex(OLLAMA_STEPS, 0, "0.7")).toBe(1);
66+
});
67+
68+
test("previousSamplingStepIndex skips topP when temperature is set", () => {
69+
expect(previousSamplingStepIndex(CUSTOM_STEPS, effortsIndex, "0.7")).toBe(
70+
temperatureIndex,
71+
);
72+
expect(previousSamplingStepIndex(CUSTOM_STEPS, effortsIndex, "0")).toBe(
73+
temperatureIndex,
74+
);
75+
});
76+
77+
test("previousSamplingStepIndex does not skip topP when temperature is blank", () => {
78+
expect(previousSamplingStepIndex(CUSTOM_STEPS, effortsIndex, "")).toBe(
79+
topPIndex,
80+
);
81+
expect(previousSamplingStepIndex(CUSTOM_STEPS, effortsIndex, " ")).toBe(
82+
topPIndex,
83+
);
84+
});
85+
86+
test("previousSamplingStepIndex from topP lands on temperature", () => {
87+
expect(previousSamplingStepIndex(CUSTOM_STEPS, topPIndex, "0.7")).toBe(
88+
temperatureIndex,
89+
);
90+
expect(previousSamplingStepIndex(CUSTOM_STEPS, topPIndex, "")).toBe(
91+
temperatureIndex,
92+
);
93+
});
94+
95+
test("previousSamplingStepIndex does not skip two steps", () => {
96+
expect(
97+
previousSamplingStepIndex(CUSTOM_STEPS, defaultEffortIndex, "0.7"),
98+
).toBe(effortsIndex);
99+
});
100+
101+
test("previousSamplingStepIndex floors at 0", () => {
102+
expect(previousSamplingStepIndex(CUSTOM_STEPS, 0, "0.7")).toBe(0);
103+
});
104+
105+
test("topPAfterTemperatureAdvance clears topP only when temperature is set", () => {
106+
expect(topPAfterTemperatureAdvance("0.7", "0.9")).toBe("");
107+
expect(topPAfterTemperatureAdvance("0", "0.9")).toBe("");
108+
expect(topPAfterTemperatureAdvance("", "0.9")).toBe("0.9");
109+
expect(topPAfterTemperatureAdvance(" ", "0.9")).toBe("0.9");
110+
});
111+
112+
test("skipTopPWhenTemperatureSet treats 0 as set and whitespace as unset", () => {
113+
expect(skipTopPWhenTemperatureSet("0")).toBe(true);
114+
expect(skipTopPWhenTemperatureSet("0.7")).toBe(true);
115+
expect(skipTopPWhenTemperatureSet("")).toBe(false);
116+
expect(skipTopPWhenTemperatureSet(" ")).toBe(false);
117+
});
118+
});

‎src/tui/provider/steps.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,46 @@ export function stepsFor(choice: ProviderChoice | null): readonly SetupStep[] {
132132
if (isOllamaProviderId(choice.id)) return OLLAMA_STEPS;
133133
return choice.oauth !== null ? OAUTH_STEPS : PRESET_STEPS;
134134
}
135+
136+
// Inference options XOR temperature and topP on the wire. The wizard still
137+
// lists both steps so either can be filled; skip/clear is navigation, not
138+
// a range check (stepReady stays range-only).
139+
export function skipTopPWhenTemperatureSet(temperature: string): boolean {
140+
return temperature.trim().length > 0;
141+
}
142+
143+
export function nextSamplingStepIndex(
144+
steps: readonly SetupStep[],
145+
currentIndex: number,
146+
temperature: string,
147+
): number {
148+
const next = currentIndex + 1;
149+
if (
150+
skipTopPWhenTemperatureSet(temperature) &&
151+
steps[currentIndex] === "temperature" &&
152+
steps[next] === "topP"
153+
) {
154+
return next + 1;
155+
}
156+
return next;
157+
}
158+
159+
export function previousSamplingStepIndex(
160+
steps: readonly SetupStep[],
161+
currentIndex: number,
162+
temperature: string,
163+
): number {
164+
if (currentIndex <= 0) return 0;
165+
const prev = currentIndex - 1;
166+
if (skipTopPWhenTemperatureSet(temperature) && steps[prev] === "topP") {
167+
return Math.max(0, prev - 1);
168+
}
169+
return prev;
170+
}
171+
172+
export function topPAfterTemperatureAdvance(
173+
temperature: string,
174+
topP: string,
175+
): string {
176+
return skipTopPWhenTemperatureSet(temperature) ? "" : topP;
177+
}

0 commit comments

Comments
 (0)