Skip to content

Commit 700348a

Browse files
authored
Merge pull request #1156 from org2AI/dev/model-properties-auto-update
feat(model-picker): apply effort changes automatically
2 parents 7be1097 + edd9086 commit 700348a

11 files changed

Lines changed: 592 additions & 221 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# ModelPropertiesDropdown UI audit
2+
3+
| Line | Element | Verdict | Reason | Suggested change |
4+
| -------------------------------------------------------------------------------------------- | --------------------------------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------- |
5+
| `src/components/ModelPropertiesDropdown/index.tsx:381` | Section separators after removing the footer | fix | The former footer separator would remain below the final control | Applied `last:border-b-0` while keeping the shared section token |
6+
| `src/components/ModelPropertiesDropdown/index.tsx:455` | Thinking/Fast switches and native range keyboard behavior | fix | Switches need accessible names; menu arrow navigation must not intercept native range keys | Applied `ariaLabel` and disabled list-style keyboard handling for this control popover |
7+
| `src/components/ModelPropertiesDropdown/EffortSlider.tsx:142` | Native range input | keep with reason | Preserves native range semantics, accessible effort text, focus, and pointer capture; local previews coalesce gestures before the shared save callback | None |
8+
| `src/components/ModelPropertiesDropdown/index.tsx:372` | Shared panel with existing 260px width | keep with reason | Uses the dropdown design tokens; width matches this popup's custom anchor calculations, which now use measured height without a footer | None |
9+
| `src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/ModelVariantInlineCard.tsx:435` | Selected-version trigger | keep with reason | Continues to use the shared dropdown and existing table trigger token; reads saved selection instead of maintaining a second preview map | None |
10+
11+
Verdict totals: **2 fix**, **3 keep with reason**, **0 abstract**.

docs/verification-2026-08-31/Gpt56Effort.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ This implements ORGII's delegation guidance; it does not establish full parity
4949
with Codex's internal orchestration.
5050

5151
Ultra's slider fill, label, and focus ring use the existing purple theme token.
52-
Other levels retain the primary accent. The popup retains its existing
53-
Apply/Cancel workflow in this change.
52+
Other levels retain the primary accent. The subsequent automatic-update change
53+
removes Cancel/Apply; see [automatic-update verification](ModelPropertiesAutoUpdate.md).
5454

5555
## Architecture coverage
5656

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# Model properties automatic updates
2+
3+
## Behavior and source of truth
4+
5+
The shared effort popover no longer has Cancel/Apply. Fast and Thinking switches
6+
apply immediately. Slider values preview locally while dragging or holding a
7+
range key, then apply once on pointer/key release or blur. Assistive input that
8+
does not generate pointer/key events applies immediately. The popup stays open;
9+
Escape and outside clicks dismiss it without reverting completed changes.
10+
11+
The caller's model id is authoritative. Each completed control interaction
12+
resolves the full effort/thinking/fast combination before calling `onChange`.
13+
Unsupported Fast is cleared when moving to an effort without that variant;
14+
unavailable combinations and unchanged values do not reach the save callback.
15+
All four dropdown consumers keep their existing session/default-variant save
16+
paths. No provider calls, persistence formats, or backend behavior changed.
17+
18+
The old popover draft/Apply effects and model-table preview map were removed.
19+
A slider keeps only one local interaction and preview, with no pending save timer
20+
or queue. Pointer cancel/unmount drops unfinished previews. Refreshed parent
21+
values invalidate stale gestures; switching model families remounts the slider.
22+
Normal effort updates preserve the input node and keyboard focus.
23+
24+
The compact popup is left-aligned to its trigger so label width changes do not
25+
shift it sideways. It uses measured height for custom placement and omits the
26+
last section's bottom divider. The purple Ultra styling remains unchanged.
27+
28+
## Architecture coverage
29+
30+
Checked compilation, removal of dead preview state, callback naming, the shared
31+
selection/write boundary, invalid/default selections, all four entry points,
32+
and effort/Fast resolution. Provider wire formats, backend initialization, and
33+
cross-instance persistence were not changed or revalidated in this follow-up.
34+
Existing save-error and RPC ordering behavior remains owned by the callers;
35+
this change coalesces slider gestures before invoking those paths.
36+
37+
## Lifecycle evidence
38+
39+
| Area | Verdict | Evidence | Change or reason kept | Verification |
40+
| ------------------ | ------- | ---------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ------------------------------------------------------------------- |
41+
| Background work | keep | No new timer, listener, worker, or deferred save | Existing dropdown listeners are open-scoped; existing slider motion pauses while hidden | Slider visibility/reopen tests and popup overlay dismissal tests |
42+
| Memory | fix | Removed popover draft bookkeeping and per-model preview map | One bounded gesture/preview lives in the mounted slider | Cancel/unmount regression test |
43+
| Scope/isolation | fix | Gesture captures the original parent value; family changes replace its owner | A refreshed value cannot be overwritten by a stale unfinished drag | Stale-drag and external-refresh tests |
44+
| Rendering/hot path | fix | Native input updates only local preview during a gesture | One save callback per drag or held-key interaction; switches call once | Pointer/key coalescing, duplicate-release, and keyboard-focus tests |
45+
46+
## Verification
47+
48+
- `pnpm exec vitest run --config config/vitest.config.ts src/components/ModelPropertiesDropdown/ModelPropertiesDropdown.test.ts src/components/ModelPropertiesDropdown/EffortSlider.test.ts src/util/__tests__/variantEditOptions.test.ts src/components/SelectorPill/index.test.ts src/components/Dropdown/positioning.portalTransform.test.ts src/components/Dropdown/positioning.verticalFit.test.ts`: 32 tests passed
49+
- `pnpm exec eslint src/components/ModelPropertiesDropdown/index.tsx src/components/ModelPropertiesDropdown/EffortSlider.tsx src/components/ModelPropertiesDropdown/ModelPropertiesDropdown.test.ts src/components/ModelPropertiesDropdown/EffortSlider.test.ts src/components/ModelSelectorPill/index.tsx src/scaffold/GlobalSpotlight/palettes/UnifiedModelPalette/VariantPill.tsx src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/ModelVariantInlineCard.tsx src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/modelTableGroupColumns.tsx --max-warnings 0`: passed
50+
- `pnpm run typecheck`: passed; the first run caught an unsupported test assertion, which was replaced with this repo's supported assertions
51+
- `node scripts/quality/check-test-placement.mjs`: passed across 440 directories
52+
- `git diff --check`: passed
53+
54+
No desktop control, full-app visual inspection, actual account writes, or CPU/RSS
55+
measurements were used. Browser-native range event ordering in the packaged
56+
WebView was not manually verified. Tests use real React controls and the dropdown
57+
engine in jsdom, with controlled model updates at the public callback boundary.
58+
No measured runtime performance improvement is claimed.
59+
60+
Performance verdict: **pass for this scoped change**. The bounded interaction,
61+
no-background-save, cleanup, and stale-input invariants pass the targeted tests.
62+
Desktop resource measurements and end-to-end persistence are not claimed.
63+
64+
## WebKit drag regression (2026-09-01)
65+
66+
Explicitly calling `setPointerCapture` on the range input prevents WebKit's
67+
native thumb from dragging. The component now leaves capture to the native
68+
thumb. Native release delivery still handles releases outside the rail; the
69+
existing one-save-per-gesture, keyboard, cancellation, and stale-value behavior
70+
is unchanged. No timers, listeners, caches, or persistence paths were added.
71+
72+
A temporary headless Playwright fixture bundled the actual `EffortSlider.tsx`
73+
and compiled its SCSS, with a controlled parent and only translation stubbed.
74+
Real mouse movement reproduced the pre-fix WebKit failure: dragging Extra High
75+
to Ultra left the value at Extra High with no save. With the fix, WebKit 26.5
76+
and Chromium passed drags in both directions, an outside-rail release, a track
77+
click, and an arrow-key change. Each completed gesture saved exactly once;
78+
active drags saved nothing and keyboard focus stayed on the range.
79+
80+
The existing component regression now rejects capture on the input. Injected
81+
jsdom values only verify coalescing; they must not be used as proof that a
82+
native slider is draggable. The headless fixture is component-level evidence,
83+
not a packaged Tauri or full Spotlight end-to-end test. Actual account writes
84+
and the user's desktop remain untested.

src/components/ModelPropertiesDropdown/EffortSlider.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,100 @@ describe("EffortSlider", () => {
9898
});
9999
}
100100

101+
function pointer(type: string) {
102+
const event = new MouseEvent(type, { bubbles: true, button: 0 });
103+
Object.defineProperty(event, "pointerId", { value: 1 });
104+
act(() => range().dispatchEvent(event));
105+
}
106+
107+
it("leaves capture to the native thumb and commits the final preview once", () => {
108+
const onChange = vi.fn();
109+
render({ onChange });
110+
const capture = vi.fn();
111+
range().setPointerCapture = capture;
112+
pointer("pointerdown");
113+
// These injected values test coalescing, not native dragging. A drag
114+
// regression also needs real WebKit mouse gestures: input injection
115+
// bypasses the native thumb behavior that explicit capture broke.
116+
expect(capture).not.toHaveBeenCalled();
117+
changeValue("0");
118+
changeValue("2");
119+
expect(range().getAttribute("aria-valuetext")).toBe("Extra High");
120+
expect(onChange).not.toHaveBeenCalled();
121+
pointer("pointerup");
122+
pointer("lostpointercapture");
123+
expect(onChange).toHaveBeenCalledOnce();
124+
expect(onChange).toHaveBeenCalledWith(MODEL_REASONING_LEVEL.EXTRA_HIGH);
125+
});
126+
127+
it("drops canceled or unmounted drag previews without saving", () => {
128+
const onChange = vi.fn();
129+
render({ onChange });
130+
range().setPointerCapture = vi.fn();
131+
pointer("pointerdown");
132+
changeValue("2");
133+
pointer("pointercancel");
134+
pointer("lostpointercapture");
135+
expect(range().getAttribute("aria-valuetext")).toBe("High");
136+
pointer("pointerdown");
137+
changeValue("0");
138+
act(() => root.render(null));
139+
expect(onChange).not.toHaveBeenCalled();
140+
});
141+
142+
it("does not apply a stale drag after the parent changes the selection", () => {
143+
const onChange = vi.fn();
144+
render({ onChange });
145+
range().setPointerCapture = vi.fn();
146+
pointer("pointerdown");
147+
changeValue("2");
148+
render({ onChange, value: MODEL_REASONING_LEVEL.LOW });
149+
expect(range().getAttribute("aria-valuetext")).toBe("Light");
150+
pointer("pointerup");
151+
expect(onChange).not.toHaveBeenCalled();
152+
});
153+
154+
it("coalesces held arrow keys and commits on key release or blur", () => {
155+
const onChange = vi.fn();
156+
render({ onChange });
157+
act(() =>
158+
range().dispatchEvent(
159+
new KeyboardEvent("keydown", {
160+
key: "ArrowRight",
161+
bubbles: true,
162+
})
163+
)
164+
);
165+
changeValue("0");
166+
changeValue("2");
167+
expect(onChange).not.toHaveBeenCalled();
168+
act(() =>
169+
range().dispatchEvent(
170+
new KeyboardEvent("keyup", {
171+
key: "ArrowRight",
172+
bubbles: true,
173+
})
174+
)
175+
);
176+
expect(onChange).toHaveBeenCalledOnce();
177+
expect(onChange).toHaveBeenCalledWith(MODEL_REASONING_LEVEL.EXTRA_HIGH);
178+
onChange.mockClear();
179+
act(() =>
180+
range().dispatchEvent(
181+
new KeyboardEvent("keydown", {
182+
key: "Home",
183+
bubbles: true,
184+
})
185+
)
186+
);
187+
changeValue("0");
188+
act(() =>
189+
range().dispatchEvent(new FocusEvent("focusout", { bubbles: true }))
190+
);
191+
expect(onChange).toHaveBeenCalledOnce();
192+
expect(onChange).toHaveBeenCalledWith(MODEL_REASONING_LEVEL.LOW);
193+
});
194+
101195
it("keeps the discrete model contract and accessible label on native input changes", () => {
102196
const onChange = vi.fn();
103197
render({ onChange });

src/components/ModelPropertiesDropdown/EffortSlider.tsx

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useEffect, useRef } from "react";
1+
import React, { useEffect, useRef, useState } from "react";
22
import { useTranslation } from "react-i18next";
33

44
import {
@@ -10,6 +10,16 @@ import {
1010
import "./EffortSlider.scss";
1111

1212
const COMET_INDICES = Array.from({ length: 18 }, (_, index) => index);
13+
const RANGE_KEYS = new Set([
14+
"ArrowLeft",
15+
"ArrowRight",
16+
"ArrowUp",
17+
"ArrowDown",
18+
"Home",
19+
"End",
20+
"PageUp",
21+
"PageDown",
22+
]);
1323

1424
interface EffortSliderProps {
1525
levels: readonly ModelReasoningLevel[];
@@ -28,9 +38,39 @@ export const EffortSlider: React.FC<EffortSliderProps> = ({
2838
}) => {
2939
const { t } = useTranslation();
3040
const sliderRef = useRef<HTMLDivElement>(null);
41+
const [preview, setPreview] = useState<{
42+
level: ModelReasoningLevel;
43+
sourceValue: ModelReasoningLevel | undefined;
44+
}>();
45+
const interactionRef = useRef<{
46+
kind: "pointer" | "keyboard";
47+
pointerId?: number;
48+
level: ModelReasoningLevel | undefined;
49+
sourceValue: ModelReasoningLevel | undefined;
50+
} | null>(null);
51+
52+
const finishInteraction = (commit: boolean) => {
53+
const interaction = interactionRef.current;
54+
if (!interaction) return;
55+
interactionRef.current = null;
56+
setPreview(undefined);
57+
if (
58+
commit &&
59+
interaction.sourceValue === value &&
60+
interaction.level &&
61+
interaction.level !== value
62+
) {
63+
onChange(interaction.level);
64+
}
65+
};
66+
3167
const selectedIndex = Math.max(
3268
0,
33-
levels.findIndex((level) => level === value)
69+
levels.findIndex(
70+
(level) =>
71+
level ===
72+
(preview?.sourceValue === value ? (preview?.level ?? value) : value)
73+
)
3474
);
3575
const maxIndex = Math.max(0, levels.length - 1);
3676
const selectedLevel = levels[selectedIndex];
@@ -110,8 +150,58 @@ export const EffortSlider: React.FC<EffortSliderProps> = ({
110150
aria-valuetext={levelLabel}
111151
onChange={(event) => {
112152
const nextLevel = levels[event.currentTarget.valueAsNumber];
113-
if (nextLevel && nextLevel !== selectedLevel) onChange(nextLevel);
153+
if (!nextLevel || nextLevel === selectedLevel) return;
154+
if (interactionRef.current) {
155+
interactionRef.current.level = nextLevel;
156+
setPreview({
157+
level: nextLevel,
158+
sourceValue: interactionRef.current.sourceValue,
159+
});
160+
} else {
161+
// Assistive input without pointer/key events still commits.
162+
onChange(nextLevel);
163+
}
164+
}}
165+
// Keep rapid input local; persist only the completed gesture.
166+
// Let the native thumb own capture, including outside releases.
167+
// Capturing on the input prevents WebKit from dragging its thumb.
168+
onPointerDown={(event) => {
169+
if (event.button !== 0 || interactionRef.current) return;
170+
interactionRef.current = {
171+
kind: "pointer",
172+
pointerId: event.pointerId,
173+
level: value,
174+
sourceValue: value,
175+
};
176+
}}
177+
onPointerUp={(event) => {
178+
if (interactionRef.current?.pointerId === event.pointerId) {
179+
finishInteraction(true);
180+
}
181+
}}
182+
onPointerCancel={() => finishInteraction(false)}
183+
onLostPointerCapture={() => {
184+
if (interactionRef.current?.kind === "pointer")
185+
finishInteraction(true);
186+
}}
187+
onKeyDown={(event) => {
188+
if (RANGE_KEYS.has(event.key) && !interactionRef.current) {
189+
interactionRef.current = {
190+
kind: "keyboard",
191+
level: value,
192+
sourceValue: value,
193+
};
194+
}
195+
}}
196+
onKeyUp={(event) => {
197+
if (
198+
RANGE_KEYS.has(event.key) &&
199+
interactionRef.current?.kind === "keyboard"
200+
) {
201+
finishInteraction(true);
202+
}
114203
}}
204+
onBlur={() => finishInteraction(true)}
115205
/>
116206
<span
117207
className="effort-slider__thumb bg-white shadow-dropdown-soft"

0 commit comments

Comments
 (0)