Fix and harden the model-config Playwright job, and pin the hooks it drives by danielhanchen · Pull Request #7760 · unslothai/unsloth

danielhanchen · 2026-08-02T18:02:23Z

The model-config Playwright job has been failing on main since #7736, so it fails on every PR branched after it for reasons unrelated to what those PRs change. That is how I found it.

FAIL: could not open run-settings for a model matching 'gemma-3-270m'
FAIL: could not reopen run-settings after reload
FAIL: Reset button not found in run-settings

The behaviour is intentional and the test had not followed it. #7736 collapses a single-quant row and, as its own comment puts it, the row "loads it in one click". The helper clicked the row first and then went looking for the gear, so by that point the model was loading and the picker had closed. The three failures are one cause: the first is that click, and the other two follow from run-settings never opening.

The gear cannot be reached through the row -- it is the row button's sibling at 2 of its 11 render sites and an uncle at the other 9 -- so it is now matched by the model its own aria-label names. That drops the row click and fixes a second latent bug in the same line: .first pressed whichever gear came first in the popover rather than the one belonging to the model under test.

I reproduced the failure locally against a real Studio before changing anything, and the run now passes end to end. Getting there exposed what the cascade had been hiding.

What the cascade was hiding

A staged edit made in run-settings' first moments is silently discarded. The panel re-derives its baseline once mount-time work lands and drops whatever was staged, so Save reports "Default settings kept" and stores nothing. Measured on gemma-3-270m: fails at 0 ms, passes from 500 ms. Neither the input value, the Reset state nor the primary button label differs across that window, so there is nothing to poll -- hence a bounded settle wait with the measurement written down rather than a magic number. I have not changed the frontend for this: a person cannot open the panel, read it, type and click inside half a second, so it is reachable by a driver and not by a user. Worth its own look, not a fix wedged into a CI repair.

Steps 3 and 3b inherited the popover from step 2. That is why one root cause reported as three failures, and why the re-type-shown regression -- the only guard on the phantom-pin bug -- had been silently skipping. Each step opens its own state now, and the skip is a soft_fail rather than prose.

False passes

These let the job report PASS while proving nothing, which is worse than the failure being fixed:

The hidden-model step asserts an absence, so it passed for free when the picker rendered no rows at all -- exactly the state a broken picker is in. It now proves the picker is populated first.
Persistence, reset and migration scanned every stored entry, so a value belonging to a different model satisfied them. Scoped to the entry under test; the fallback only fires when no key has the versioned shape. Verified against the old assertion: it accepts another model's value, the scoped one rejects it.
read_configs() turned unreadable storage into {}, which several assertions treat as success.
_count() turned a closed page or a lost execution context into "0 matches", so an infrastructure failure read as a missing selector. That is precisely how this root cause surfaced as a misleading message.
primary_button() swept "Load model" first, which substring-matches "Reload model", so the reload case was found under the wrong name.

A miss now writes a diagnostics sidecar naming the selector that missed and the option rows and gear labels actually present, via dump_diagnostics() -- which already existed in _playwright_robust.py and no driver was calling.

Pinning the hooks

test_model_picker_contracts.py is a CPU-only source-contract suite whose docstring says it pairs with the Playwright checks, yet it pinned exactly one aria-label and none of the picker's test hooks. Renaming one used to type check, lint, pass every unit test, and then fail a 25-minute browser job. The new pins cover data-model-picker-option, the search input, the list container, the gear label prefix, Back to model list, Context Length, and the four primary-action labels. Confirmed against a scratch copy with three hooks renamed: they fail in about a second.

Why nobody noticed for 14 hours

Unsloth UI CI cancelled in-progress runs on main. Pushes land there in bursts, and four cancelled runs are why a deterministic failure went unreported. cancel-in-progress is now off for main only; PR branches still cancel. 32 other workflows share the pattern -- a separate PR, not a silent expansion of this one.

Verification

Against a real Studio on a local install, gemma-3-270m-it-GGUF UD-Q4_K_XL:

Unmodified driver at main: the same three failures as CI.
After: RESULT: PASS, every step running rather than skipping.
test_model_picker_contracts.py: 114 passed; 3 fail when the hooks are renamed.
ruff check tests/studio/: clean.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9bec712b5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-02T18:06:03Z

+        if find_on_device_row(popover, hint) is None:
            return None


Expand multi-quant rows before locating their gear

When the target repository has multiple downloaded quantizations—or Show all quantizations is enabled—its On Device entry is an expandable parent row, and renderDownloadedGgufRow does not mount any inference-settings button until that row is expanded. This helper now only confirms that the parent exists and immediately searches for the absent gear, so every subsequent configuration check fails with could not open run-settings; retain the direct gear path for collapsed single-quant rows, but click expandable rows before locating the variant gear.

Useful? React with 👍 / 👎.

Right about the DOM: renderDownloadedGgufRow mounts no gear on the parent -- there is an aria-hidden spacer where it would go -- and the variant gears only exist inside GgufVariantExpander under {expanderOpen && ...}. So a gear lookup against an unexpanded multi-quant row finds nothing.

Not what fails CI today, though. That job downloads one quant of gemma-3-270m-it-GGUF, showAllQuantizations defaults false, and the test never toggles it, so the collapsed sole-quant path is the only one exercised -- which is why the bisect showed main+fix passing on a real runner. Latent rather than live.

Fixed anyway, since the helper should not depend on the test model having exactly one quant on disk. Gear first, and only if there is none, click the row and look again.

The ordering is what makes that safe, and it is safe only in that order: the expandable parent's onClick is toggleGgufExpanded, which only sets expansion state, while the collapsed row that loads and closes the picker is exactly the row that already had a gear. So the fallback can never reach the row whose click would dismiss the popover.

I also made the lookup polled. A row still waiting on its sole-quant probe renders with no gear anywhere, and the one-shot read would have taken that as the model having no run-settings -- a nearer-term fragility than the multi-quant case.

Review on #7760 is right that a multi-quant parent mounts no inference-settings button until it is expanded, so a gear lookup against it finds nothing. It is not what fails CI today -- that job downloads one quant and Show all quantizations defaults off, so the collapsed path is the only one exercised -- but the helper should not depend on the test model having exactly one quant on disk. Gear first, and only if there is none, click the row and look again. Safe in that order and only in that order: the expandable parent's onClick is toggleGgufExpanded, while the collapsed single-quant row that loads and closes the picker is exactly the row that already had a gear. So the fallback can never reach the row whose click would dismiss the popover. The lookup is also polled now. A row still waiting on its sole-quant probe renders with no gear at all for a moment, and a single miss there would have read as the model having no run-settings.

danielhanchen · 2026-08-03T05:45:39Z

@codex review

The model-config Playwright job has been failing on main since #7736, on every PR branched after it, for reasons unrelated to what those PRs change. FAIL: could not open run-settings for a model matching 'gemma-3-270m' FAIL: could not reopen run-settings after reload FAIL: Reset button not found in run-settings The behaviour is intentional and the test had not followed it. #7736 collapses a single-quant row and, as its own comment says, "loads it in one click". The helper clicked the row first and then looked for the gear, so by then the model was loading and the picker had closed. The three failures are one cause: the first is that click, and the other two follow from run-settings never opening. data-model-picker-option sits on the row button and the gear is its sibling, so the gear cannot be reached through the row at all. It is now matched by the model its own aria-label names, which drops the row click and also fixes a second latent bug: `.first` pressed whichever gear came first in the popover rather than the one belonging to the model under test. Bisected on a runner, one commit per run, nothing else changed: 3742317 (parent of #7736) pass 63307b1 (#7736) fail origin/main fail origin/main + this change pass

Review on #7760 is right that a multi-quant parent mounts no inference-settings button until it is expanded, so a gear lookup against it finds nothing. It is not what fails CI today -- that job downloads one quant and Show all quantizations defaults off, so the collapsed path is the only one exercised -- but the helper should not depend on the test model having exactly one quant on disk. Gear first, and only if there is none, click the row and look again. Safe in that order and only in that order: the expandable parent's onClick is toggleGgufExpanded, while the collapsed single-quant row that loads and closes the picker is exactly the row that already had a gear. So the fallback can never reach the row whose click would dismiss the popover. The lookup is also polled now. A row still waiting on its sole-quant probe renders with no gear at all for a moment, and a single miss there would have read as the model having no run-settings.

for more information, see https://pre-commit.ci

…nding Two defects in the fallback, both found reviewing my own change. find_on_device_row matches with Playwright has_text, which is case-insensitive, while find_gear matched with a CSS attribute substring, which is not. So a hint whose case differs from the repo id would find the row and miss the gear -- and then take the new fallback and click a row that loads, which is exactly the bug this PR exists to prevent, now reachable through an env var. Both matchers are case-insensitive now. After expanding, every variant mounts its own gear and all of them carry the repo id, so `.first` over the popover picked an arbitrary quant, possibly one not on disk, and the test would then configure and try to load it. The comment above it claimed the opposite. The second lookup is scoped to the row's own group, with the popover kept only as a fallback. The poll is now a wait_for, which returns the moment the gear mounts instead of sleeping out six fixed intervals -- and the old loop slept after its last attempt too, so a miss cost 3s and the fallback path paid it twice.

Reproduced the main failure locally first: the same three FAILs, from one cause. With the gear fix the run now passes end to end, which exposed what the cascade had been hiding. - A staged edit made in run-settings' first moments is discarded: the panel re-derives its baseline once mount-time work lands, so Save reports "Default settings kept" and stores nothing. Measured on gemma-3-270m: fails at 0ms, passes from 500ms, and neither the input value, the Reset state nor the primary button label differs across that window, so there is nothing to poll. Bounded settle wait, with the measurement written down. - Steps 3 and 3b inherited the popover from step 2, which is why one root cause reported as three failures and why the re-type-shown regression had been silently skipping. Each step opens its own now, and the skip is a soft_fail rather than prose. - The hidden-model step asserts an absence, so it passed for free when the picker rendered nothing at all. It now proves the picker is populated first. - Persistence, reset and migration scanned every stored entry, so another model's value satisfied them. Scoped to the entry under test; the fallback only fires when no key has the versioned shape. - read_configs turned unreadable storage into {}, which several assertions treat as success, and _count turned a closed page into "selector missing". Both say what happened now. - primary_button swept "Load model" first, which substring-matches "Reload model", so the reload case was found under the wrong name. - A miss now writes a diagnostics sidecar naming the selector, the option rows and the gear labels actually present. Also pinned the hooks the driver depends on in the CPU contract suite. Renaming one used to type check, lint, pass every unit test and then fail a 25-minute browser job; the pins catch it in a second. Unsloth UI CI no longer cancels in-progress runs on main. Pushes land in bursts there, and four cancelled runs are why this break went unreported for 14 hours.

for more information, see https://pre-commit.ci

danielhanchen · 2026-08-03T07:14:24Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76425a7b84

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-03T07:20:44Z

+            group = row.locator("xpath=ancestor::div[1]")
+            gear = find_gear(group, hint, timeout = 2000) or find_gear(popover, hint)


Select the requested quantization's settings gear

When the repository has multiple downloaded quantizations and GGUF_VARIANT is not the first displayed variant, this lookup still passes only the repository hint to find_gear, whose .first selects an arbitrary variant even though GgufVariantExpander labels each gear with both repo and quant. Fresh evidence in the revised code is that the new expansion path never includes GGUF_VARIANT in its selector, so the job can configure and load a different quantization than the one named by the environment; match the gear label on both the repository and GGUF_VARIANT.

Useful? React with 👍 / 👎.

Right, and the fallback I added is what made it reachable: expanding mounts one gear per variant, all carrying the repo id, so scoping to the row's group narrowed it to the right repo and left .first to choose the quant. The labels are <repo> <quant>, so it names GGUF_VARIANT now. Kept the repo-only match as a last resort so a label format change degrades to the previous behaviour instead of failing outright. 135156f.

Agreed. Scoping to the row was not enough on its own: the expanded group holds one gear per variant and every label carries the repo id, so .first among them is whichever the expander lists first, which need not be the downloaded one. The lookup names GGUF_VARIANT now, with repo-only kept as a fallback.

chatgpt-codex-connector · 2026-08-03T07:20:44Z

+            return [
+                cfg[k] for k in recognised if needle in str(k).lower() and isinstance(cfg[k], dict)
+            ]


Scope stored configurations to the target quantization

When another quantization of GGUF_REPO already has the distinctive value, this repo-only substring filter returns that entry as well, although storage keys identify configurations by both model and quantization. Consequently a failed save, reset, or migration for GGUF_VARIANT can be masked by stale settings for a different quant; parse the versioned key and require exact normalized matches for both the repository and GGUF_VARIANT rather than searching the serialized key.

Useful? React with 👍 / 👎.

Yes, and this is a real gap in the fix rather than a leftover: I scoped away other models but not other quants of the same repo, which the substring match cannot separate. It parses the key now and requires both halves to match, normalised for case and whitespace, with the fallback firing only when no key has the versioned shape.

Checked it is not vacuous: an entry for the same repo at a different quant is accepted by the substring version and rejected by this one. 135156f.

Correct, and the consequence is the part worth naming: a stale entry for another quant of the same repo could satisfy a check that this quant had saved, reset or migrated, so the test would pass on evidence belonging to a different configuration. Both halves of the key are matched now.

chatgpt-codex-connector · 2026-08-03T07:20:44Z

+def test_picker_popover_and_trigger_keep_their_tour_hooks():
+    chat = _read("features/chat/chat-page.tsx")
+    assert "chat-model-selector-popover" in chat
+    assert 'dataTour="chat-model-selector"' in chat or "chat-model-selector" in chat


Pin the actual model-selector trigger hook

If triggerDataTour="chat-model-selector" is removed or renamed while the popover hook remains, this test still passes because the fallback substring "chat-model-selector" is contained in "chat-model-selector-popover". The Playwright driver would then fail at page.locator(TRIGGER).first.click(), defeating the stated purpose of catching hook changes in this fast contract test; assert the actual triggerDataTour prop used by chat-page.tsx instead of the shared substring.

Useful? React with 👍 / 👎.

Correct, and worse than a weak fallback: the first term named dataTour="chat-model-selector", which does not appear in chat-page.tsx at all. The prop is triggerDataTour, so the assertion only ever passed on the bare substring, and the popover's own value contains it. The pin would have gone green with the trigger hook deleted, while page.locator(TRIGGER).first.click() is the driver's very first action.

Both props are named exactly now. Confirmed against a scratch tree with triggerDataTour renamed: it fails. 135156f.

Right, and this is the worse of the two because the assertion was there specifically to stop a hook rename reaching the browser job. "chat-model-selector" is a prefix of "chat-model-selector-popover", which the line above already asserts, so the or held after triggerDataTour was deleted -- and the driver clicks the trigger before it can ever reach the popover. Both props are now asserted exactly, at head.

chatgpt-codex-connector · 2026-08-03T07:20:44Z

+    assert '"Back to model list"' in page or "Back to model list" in page
+    assert 'ariaLabel="Context Length"' in page
+    assert 'aria-label="Context Length"' in page
+    assert ">\n            Reset" in page or "Reset" in page


Verify the Reset button rather than any Reset text

If the run-settings Reset button is removed or renamed, this contract still passes because the fallback searches the entire raw source for Reset, and model-config-page.tsx already contains that word in unrelated explanatory comments. The Playwright reset gate would therefore reach the expensive browser job before discovering the missing control; constrain this assertion to the button's JSX instead of accepting any source occurrence.

Useful? React with 👍 / 👎.

Agreed, and this file is the specific reason it matters: Reset appears in its own comments at lines 923 and 939, so the raw-source fallback passes with the control gone. Pinned to the button's JSX (the setConfig({ ...DEFAULT_PER_MODEL_CONFIG }) handler and the Reset label together), which fails when I rename the label in a scratch tree. 135156f.

Correct. model-config-page.tsx explains Reset in three separate comments, so the fallback passed whether or not the control existed. It now matches the Button element itself, anchored on the setConfig({ ...DEFAULT_PER_MODEL_CONFIG }) handler, so a rename of either the text or the behaviour fails here rather than 25 minutes into a browser job.

Four review items, all mine, all correct. The two contract pins I added were passing vacuously. The trigger hook is carried by triggerDataTour, and my assertion's first term named a prop that does not exist, so it only ever passed on the bare substring -- which the popover's own value contains. It would have gone green with the trigger hook deleted, and the driver's very first click is the thing that would then fail. Same shape for Reset: the fallback searched the whole file, and this file says Reset in two of its own comments, so the pin passed with the button gone. Both now name what they mean, and both fail against a scratch tree with the hook renamed. The expanded-variant lookup passed only the repo hint. Every variant mounts its own gear and all of them carry the repo id, so .first picked an arbitrary quant, possibly one not on disk. The labels are "<repo> <quant>", so it names GGUF_VARIANT now, falling back to the repo-only match so a label format change degrades rather than breaks. entries_for_model matched the repo as a substring of the serialised key, which also matches this repo's other quants -- so a stale entry for one quant could stand in for the one under test and mask its failed save. It parses the key and requires repo and quant to match. Verified it rejects a sibling quant that the substring version accepted.

danielhanchen · 2026-08-03T07:41:16Z

@codex review

danielhanchen · 2026-08-03T07:44:58Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 135156f5a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-03T07:52:13Z

            return None
-        gear = popover.locator('button[aria-label^="Inference settings for"]').first
-        if _count(gear) == 0:
+        gear = find_gear(popover, hint)


Match the quant before accepting an existing gear

When Expand quantizations is enabled and the repository contains multiple downloaded quants, the expander is already mounted, so this repo-only lookup succeeds and the quant-specific branch below is skipped. GgufVariantExpander orders variants by fit and recommendation rather than GGUF_VARIANT, meaning .first can open, configure, and reload a different quantization. Fresh evidence relative to the earlier review is that the revised code applies quant = GGUF_VARIANT only inside if gear is None, leaving already-expanded rows on this unqualified path; include the quant in the initial lookup as well.

Useful? React with 👍 / 👎.

Right, and I had left the hole in the obvious place: the quant was named only inside if gear is None. With Expand quantizations on the expander is already mounted, so the repo-only lookup succeeds immediately and that branch is never reached -- and since the expander orders variants by fit and recommendation rather than by GGUF_VARIANT, .first opens whichever it happens to list first. The first lookup names the quant now, with repo-only behind it for the single-quant row.

chatgpt-codex-connector · 2026-08-03T07:52:13Z

+    render sites and an uncle at the rest -- so it matches on this label, and the model
+    name in it is what keeps it off another row's gear."""
+    pickers = _read("features/model-picker/components/model-selector/pickers.tsx")
+    assert "Inference settings for ${" in pickers


Pin the variant gear label rather than any interpolated gear

If only GgufVariantExpander stops including the repository or quant in its settings label, this assertion still passes because pickers.tsx contains many unrelated Inference settings for ${...} labels. The Playwright helper specifically relies on the expander label containing both repoId and v.quant, so this fast contract would miss the exact hook regression it claims to prevent and defer it to the browser job; assert the expander's complete ariaLabel expression.

Useful? React with 👍 / 👎.

Correct. pickers.tsx interpolates that label at five sites and only the expander's names both the repo and the quant, which is the part the driver depends on to tell one variant from another -- so a file-wide match was green precisely when the label it cares about had lost what makes it useful. The expander's complete ariaLabel is asserted now.

chatgpt-codex-connector · 2026-08-03T07:52:13Z

+    assert '"data-model-picker-option": true;' in pickers
+    assert '"data-model-picker-option": true,' in pickers


Verify the row button receives the automation props

If ModelRow stops spreading optionProps onto its <button>, both assertions still pass because they only inspect the ModelRowOptionProps type and the object returned by getOptionProps. The component separately reads optionProps?.onKeyDown, so the prop can remain used and type-check while data-model-picker-option disappears from the DOM, causing every row lookup in the Playwright driver to fail; pin the {...optionProps} spread on the row button.

Useful? React with 👍 / 👎.

Right, and this is the sharpest of the five. The two assertions proved the props are declared and returned; ModelRow reads optionProps?.onKeyDown separately, so the prop stays used and the file type checks while data-model-picker-option leaves the DOM and every row lookup in the driver fails. The spread on the row button is pinned now, and dropping it fails this test where it did not before.

chatgpt-codex-connector · 2026-08-03T07:52:13Z

+    for label in ('"Save settings"', '"Forget settings"', '"Reload model"', '"Load model"'):
+        assert label in page, label


Verify the primary labels remain on an actionable button

If the primary <Button onClick={handleRun}> is removed or replaced with a non-button element while the primaryActionLabel calculation remains, all four string assertions still pass even though primary_button() can no longer find or click the control. This recreates the same delayed browser-only failure that these source contracts are intended to prevent; assert that {primaryActionLabel} is rendered inside the button wired to handleRun, not merely that its possible label literals exist somewhere in the file.

Useful? React with 👍 / 👎.

Correct. The four literals live in the primaryActionLabel calculation, so they outlive the button being deleted or turned into something get_by_role cannot find. The label now has to be rendered inside the handleRun Button. Matched as whole elements rather than through the opening tag, since disabled={...} puts braces there first.

chatgpt-codex-connector · 2026-08-03T07:52:13Z

+    assert 'aria-hidden="true"' in pickers
+    assert "w-[42px]" in pickers


Assert that the multi-quant parent actually lacks a gear

If a ModelLoadSettingsAction is added to the multi-quant parent while the alignment spacer remains, these assertions still pass because they only prove that an aria-hidden 42px element exists somewhere in the file. The Playwright helper uses the absence of a parent gear as its signal that clicking the row is safe and expands variants, so this unguarded change would bypass the quant-specific expansion path; inspect the renderDownloadedGgufRow parent block and assert that it contains the spacer instead of a settings action.

Useful? React with 👍 / 👎.

Agreed, and the failure mode is the one worth spelling out: the driver reads the absence of a gear as "this row expands rather than loads", so adding a gear to the parent inverts that inference silently, and the spacer staying put is exactly what a file-wide search keeps seeing. It now reads the renderDownloadedGgufRow parent block and requires both the spacer and no ModelLoadSettingsAction.

Five review items, all the same shape: an assertion that reads the whole file and so stays green through the change it exists to catch. These pins were added to turn a 25-minute browser failure into a two-second one, and four of them could not have done it. - The automation-attribute test proved the props are declared and returned, but not that ModelRow spreads them onto the button. It reads optionProps?.onKeyDown separately, so the prop stays used and type checks while data-model-picker-option leaves the DOM and every row lookup fails. Now pins {...optionProps} on the row button. - The gear-label test matched "Inference settings for ${". pickers.tsx interpolates that at five sites, and only the expander's names both the repo and the quant, which is what tells one variant from another. Now pins that label. - The parent-row test proved an aria-hidden 42px element exists somewhere. Adding a gear to the parent while leaving the spacer satisfies it -- and that is precisely the change that inverts the driver's "no gear means this row expands rather than loads". Now reads the parent block and requires the spacer and no ModelLoadSettingsAction. - The primary-action test proved four string literals exist. They live in the primaryActionLabel calculation, so they outlive the button being deleted or made a non-button. Now requires the label to be rendered inside the handleRun Button. Each verified by making the change it names: dropping the spread, removing the quant from the expander label, giving the parent a gear, and unwiring handleRun each fail now. None did before. The fifth is the driver. The quant was named only inside `if gear is None`, so with "Expand quantizations" on the expander is already mounted, a repo-only lookup succeeds immediately and that branch is never reached -- and the expander orders variants by fit, not by GGUF_VARIANT, so it configures an arbitrary one. The first lookup names the quant now, with repo-only behind it.

for more information, see https://pre-commit.ci

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b8162df4d8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-03T08:17:41Z

+    populated = _count(popover.locator("[data-model-picker-option]"))
+    if populated == 0:
+        fail("picker shows no rows at all, so the hidden-model check below proves nothing")


Wait for inventory before treating the picker as empty

When the On Device inventory is still loading, the picker renders the loading state with zero [data-model-picker-option] rows until cachedReady flips, so this immediate count after a fixed 400 ms can add a hard failure on slow cache scans even though the target row appears moments later. Please wait for either the expected row or the loading state to settle before treating an empty count as a broken picker.

Useful? React with 👍 / 👎.

Correct. Until cachedReady flips the picker renders its loading state with no rows at all, so counting once after a fixed 400ms turns a slow cache scan into a hard failure about nothing true -- and this is CI on a runner that has just downloaded a model, which is exactly when a scan is slow.

It waits for the first row to attach now. A populated picker attaches one as soon as it has it, so the normal case returns immediately and the timeout is only spent when there genuinely is nothing, which is the case the check exists for.

Worth noting the pairing, since the two are easy to read separately: this check exists because the hidden-model assertion above it asserts an absence, so it passes for free against an empty picker. A false failure here and a false pass there are the same bug seen from either end, and both halves have to be right for either to mean anything.

The populated check counted rows once, 400ms after clicking On Device. Until cachedReady flips, the picker renders its loading state with no rows at all, so a slow cache scan would be reported as a broken picker -- a hard failure on timing rather than on anything true. It waits for the first row to attach instead. A populated picker attaches one as soon as it has it, so the normal case returns immediately and the timeout is only spent when there really is nothing, which is the case the check exists for. Note this guards the assertion added above it: the hidden-model check asserts an absence, so it passes for free against an empty picker. Both halves have to be right for either to mean anything.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bdceeda919

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-03T08:48:54Z

+        # variant and they all carry the repo id.
+        sel = GEAR.format(h = hint)
+        if quant:
+            sel += f'[aria-label*="{quant}" i]'


Match the quantization label exactly

When GGUF_VARIANT is contained in another downloaded quant name, this *= selector matches both labels; for example, requesting F16 also matches BF16, and .first may open the latter because expanded variants are independently ordered. The job then configures/reloads the wrong quant and its exact-key storage assertions fail despite the requested quant working correctly. Fresh evidence after the earlier quant fix is that the quant is now included but still compared as an unbounded substring; match the full ${GGUF_REPO} ${GGUF_VARIANT} gear label or otherwise enforce a token boundary.

Useful? React with 👍 / 👎.

Right, and I confirmed it in the real engine rather than by reading the selector. With four gears mounted, [aria-label*="F16" i] returns ['BF16', 'F16'] in that DOM order, so .first opens BF16, and the exact-key storage assertions then fail on a quant that was working. Since every label is Inference settings for <repo> <quant> (pickers.tsx:1199), the quant is the final token, so I pinned it with [aria-label$=" {quant}" i]. Same check after the change: F16 returns ['F16'], BF16 returns ['BF16'].

The leading space matters as much as the anchor: without it $="F16" still matches BF16. And every quant-narrowed lookup keeps its repo-only fallback, so the tightening can only ever fall through to the previous behaviour, never turn a found gear into a missing one.

Fixed in f1d6f70.

chatgpt-codex-connector · 2026-08-03T08:48:54Z

+                continue
+            got = tuple(str(x).strip().lower() for x in (list(parts) + [""])[:2])
+            if got == want and isinstance(cfg[key], dict):
+                matched.append(cfg[key])


Preserve case for POSIX model paths

When this configurable driver targets a POSIX path, lowercasing the parsed model-id key merges paths that the application deliberately treats as distinct (for example, /models/Foo.gguf and /models/foo.gguf). If the case-differing file already has the distinctive value, persistence, reset, or migration for the target can falsely pass using that other entry. Fresh evidence after the earlier storage-scoping fix is that entries_for_model calls .lower() unconditionally, whereas normalizeModelIdentity preserves case for POSIX paths; mirror the application's identity normalization instead.

Useful? React with 👍 / 👎.

Correct, and my first attempt at the fix was wrong in an instructive way. I read normalizeCaseInsensitivePath's minLength as "fold this many leading segments" and wrote a helper that folded only the root. It is not: minLength bounds the trailing-separator trim, and the whole path is lowercased. So //Server/Share/Foo.gguf folds entirely, which my version left untouched.

Rather than re-read it more carefully, I ported looksLikeLocalPath plus normalizeModelIdentity line for line and differential-tested the port against the TypeScript itself under node: 37 hand-picked cases (hub ids, POSIX, C:\, UNC in both slash directions, /mnt/c including the bare and trailing-slash forms, ~, ~user, ./.., redundant separators) and 4000 random strings over an alphabet of the separators and drive letters that drive every branch. 4037 cases, 0 mismatches.

The behaviour that matters here: /models/Foo.gguf and /models/foo.gguf stay distinct, which is what the unconditional .lower() was collapsing.

Fixed in f1d6f70.

chatgpt-codex-connector Bot reviewed Aug 2, 2026

View reviewed changes

danielhanchen and others added 5 commits August 3, 2026 06:47

[pre-commit.ci] auto fixes from pre-commit.com hooks

7c42d99

for more information, see https://pre-commit.ci

danielhanchen force-pushed the fix-modelcfg-collapsed-row branch from 7461756 to 3060952 Compare August 3, 2026 07:09

danielhanchen changed the title ~~Open run-settings from the gear, not by selecting the collapsed row~~ Fix and harden the model-config Playwright job, and pin the hooks it drives Aug 3, 2026

[pre-commit.ci] auto fixes from pre-commit.com hooks

76425a7

for more information, see https://pre-commit.ci

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

danielhanchen mentioned this pull request Aug 3, 2026

[staging CI] unslothai/unsloth#7760 danielhanchen/unsloth-staging-2#679

Open

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

danielhanchen and others added 2 commits August 3, 2026 07:57

[pre-commit.ci] auto fixes from pre-commit.com hooks

b8162df

for more information, see https://pre-commit.ci

chatgpt-codex-connector Bot reviewed Aug 3, 2026

View reviewed changes

Pin the gear to the exact quant and mirror the app's identity folding

f1d6f70

		group = row.locator("xpath=ancestor::div[1]")
		gear = find_gear(group, hint, timeout = 2000) or find_gear(popover, hint)

		assert '"data-model-picker-option": true;' in pickers
		assert '"data-model-picker-option": true,' in pickers

		for label in ('"Save settings"', '"Forget settings"', '"Reload model"', '"Load model"'):
		assert label in page, label

		assert 'aria-hidden="true"' in pickers
		assert "w-[42px]" in pickers

Uh oh!

Uh oh!

Conversation

danielhanchen commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What the cascade was hiding

False passes

Pinning the hooks

Why nobody noticed for 14 hours

Verification

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 2, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Aug 3, 2026

Uh oh!

danielhanchen commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Aug 3, 2026

Uh oh!

danielhanchen commented Aug 3, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 3, 2026

danielhanchen commented Aug 2, 2026 •

edited

Loading