Give the llama-flag fixtures the parallelSlotsClamped they now require by vineethsaivs · Pull Request #8837 · unslothai/unsloth · GitHub
Skip to content

Give the llama-flag fixtures the parallelSlotsClamped they now require - #8837

Merged
oobabooga merged 1 commit into
unslothai:mainfrom
vineethsaivs:fix/llama-flag-catalog-fixtures
Aug 14, 2026
Merged

Give the llama-flag fixtures the parallelSlotsClamped they now require#8837
oobabooga merged 1 commit into
unslothai:mainfrom
vineethsaivs:fix/llama-flag-catalog-fixtures

Conversation

@vineethsaivs

Copy link
Copy Markdown
Contributor

Problem

npm run typecheck fails on main right now:

tests/llama-extra-args-diagnostics.test.ts(20,7): error TS2741:
  Property 'parallelSlotsClamped' is missing in type '{ ... }' but required in type 'LlamaFlagCatalog'.
tests/llama-extra-args-diagnostics.test.ts(113,9): error TS2741: ... same
tests/llama-extra-args-diagnostics.test.ts(902,9): error TS2741: ... same
tests/llama-extra-args-diagnostics.test.ts(922,9): error TS2741: ... same

2098b7cd4 ("Add an extra llama-server arguments box to the model settings", #8702) added parallelSlotsClamped as a required field of LlamaFlagCatalog and set it in both real parsers, but four fixtures in llama-extra-args-diagnostics.test.ts build the catalogue by hand and were not updated.

studio-frontend-ci.yml runs npm run typecheck on pushes to main, so the Frontend CI gate is red for everyone until the fixtures catch up. The three most recent runs on main are failing.

Fix

Two parts, both small.

1. The four fixtures get the field. false is what the real parsers use when the backend does not report it, and the comment there calls it the safe read:

// Absent on a backend that predates the field, and false is the safe read:
// the floor then follows the asked-for count, exactly as it did before.
parallelSlotsClamped: Boolean(body.parallel_slots_clamped),

so false is what these fixtures mean: an unprobed or older build, which is exactly the scenario each of them is testing.

2. The interface contract test pins the field, next to the assertion that already pins defaultParallelSlots:

// The dynamic limits are what make it stale, so they have to be in that answer.
assert.match(flagsApi, /defaultParallelSlots: number;/);
// parallelSlotsClamped goes stale the same way and for the same reason: it is
// read off the same probe, so a cached answer can outlive the build it describes.
assert.match(flagsApi, /parallelSlotsClamped: boolean;/);

Both fields are read off the same --help probe and go stale together, which is the reason the neighbouring assertion exists. Had it covered both, this would have surfaced as a failing test rather than as a broken typecheck.

Test

                                          before        after
npm run typecheck                         exit 2        exit 0
                                          4 x TS2741
llama-extra-args-diagnostics.test.ts      66 passed     66 passed
whole suite (npm test)                    2702 / 0      2702 / 0

The new contract assertion was checked to actually bite, not just to pass: renaming both declarations of parallelSlotsClamped: boolean; in llama-flags.ts takes the file from 66 passed to 65 passed / 1 failed with

AssertionError: The input did not match the regular expression /parallelSlotsClamped: boolean;/

(Renaming only the first declaration is not enough, since the field is declared twice in that file, which is worth knowing if anyone re-checks this.)

Nothing outside the test file changes.

2098b7c added parallelSlotsClamped as a required field of LlamaFlagCatalog
and set it in both real parsers, but four fixtures in
llama-extra-args-diagnostics.test.ts still build the catalogue by hand
without it, so `npm run typecheck` fails on main:

    tests/llama-extra-args-diagnostics.test.ts(20,7): error TS2741:
      Property 'parallelSlotsClamped' is missing in type '{ ... }'
      but required in type 'LlamaFlagCatalog'.
    ... and the same at 113, 902 and 922

Frontend CI runs that script on pushes to main, so the gate is red for
everyone until the fixtures catch up.

false is the value the real parsers use when the backend does not report the
field, and its comment there calls it the safe read, so it is what these
fixtures mean.

Also pin the field in the interface contract test beside defaultParallelSlots.
Both are read off the same probe and go stale the same way, and the assertion
next to it already exists for exactly that reason; had it covered both, this
would have failed as a test rather than as a typecheck.
@oobabooga

Copy link
Copy Markdown
Member

@oobabooga
oobabooga merged commit 203007d into unslothai:main Aug 14, 2026
34 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants