Repair the contract tests #8943 left behind on main by danielhanchen · Pull Request #9067 · unslothai/unsloth · GitHub
Skip to content

Repair the contract tests #8943 left behind on main - #9067

Merged
danielhanchen merged 5 commits into
mainfrom
fix-contract-anchors-8943
Aug 17, 2026
Merged

Repair the contract tests #8943 left behind on main#9067
danielhanchen merged 5 commits into
mainfrom
fix-contract-anchors-8943

Conversation

@danielhanchen

Copy link
Copy Markdown
Member

Repair the contract tests #8943 left behind, and correct one that pinned the old behaviour

Repo tests (CPU) has been failing on main since #8943 landed: 54 tests across
tests/studio/test_new_chat_context_recount.py and
tests/studio/test_model_picker_contracts.py. #9026, immediately before it, is
clean. Nothing in the batch of test fixes merged after it caused this.

Three separate problems, and the third is a behaviour change, not a stale anchor.

  1. test_new_chat_context_recount.py sliced loadModel's already-resident branch by
    a comment studio: stop prompting to stop chats for a model already loaded #8943 rewrote, so 52 tests died at the slice. That branch is also no
    longer a short early return: it now IS the residency decision, reconciling GPU
    pools, speculative type, managed llama flags and per-model config through 17
    imported collaborators. Re-anchoring to the top of it would have meant stubbing
    all 17, and a replay under 17 stubs asserts against a construction rather than
    the product.

    The slice is now the adoption TAIL, which is what this file is about: what
    happens after the model is judged resident. The judgement itself is stubbed as
    adoptable and is covered on its own by the resident-model-match and
    resident-config-match suites studio: stop prompting to stop chats for a model already loaded #8943 added. Six stubs, each derived exactly the
    way the source derives it, and the two enclosing conditions restated so the
    entry conditions are visible rather than implied.

    setParams is stubbed to the merge the tail depends on rather than sliced: the
    real reducer reaches preset policy, per-turn counters and loaded-context
    bookkeeping, none of which this file measures.

  2. test_external_readoption_drops_a_pin_taken_for_another_model sliced on a
    condition that no longer exists. The contract survives in a better form: the
    branch used to clear the pin to null, and now adopts this pick's own pin by the
    rule a completed load writes it, which drops a stale pin the same way and keeps
    a pinned cached row loadable. Rewritten against that, ordering requirement
    unchanged.

  3. test_hydration_keeps_the_slot_control_when_readopting_the_running_model was
    asserting behaviour studio: stop prompting to stop chats for a model already loaded #8943 deliberately reversed, and studio: stop prompting to stop chats for a model already loaded #8943 is right. Its commit
    says why: the adopt path rolls the outgoing model's config back into the store
    before it hydrates, so the slot and batch controls sitting there belong to the
    model the tab just left, and suppressing the reseed left a resident model
    running 4 slots showing the outgoing count for the next Apply to save over it.
    The test now pins the new contract, asserts nothing reintroduces a same-model
    exemption, and additionally pins the ordering that makes the reseed necessary:
    the rollback must precede the hydration.

tests/studio: 3896 passed, 4 skipped
reinstating the readoptingSameModel guard, or moving the rollback after the
hydration, fails the rewritten test

danielhanchen and others added 4 commits August 17, 2026 05:20
…ned the old behaviour

Repo tests (CPU) has been failing on main since #8943 landed: 54 tests across
tests/studio/test_new_chat_context_recount.py and
tests/studio/test_model_picker_contracts.py. #9026, immediately before it, is
clean. Nothing in the batch of test fixes merged after it caused this.

Three separate problems, and the third is a behaviour change, not a stale anchor.

1. test_new_chat_context_recount.py sliced loadModel's already-resident branch by
   a comment #8943 rewrote, so 52 tests died at the slice. That branch is also no
   longer a short early return: it now IS the residency decision, reconciling GPU
   pools, speculative type, managed llama flags and per-model config through 17
   imported collaborators. Re-anchoring to the top of it would have meant stubbing
   all 17, and a replay under 17 stubs asserts against a construction rather than
   the product.

   The slice is now the adoption TAIL, which is what this file is about: what
   happens after the model is judged resident. The judgement itself is stubbed as
   `adoptable` and is covered on its own by the resident-model-match and
   resident-config-match suites #8943 added. Six stubs, each derived exactly the
   way the source derives it, and the two enclosing conditions restated so the
   entry conditions are visible rather than implied.

   setParams is stubbed to the merge the tail depends on rather than sliced: the
   real reducer reaches preset policy, per-turn counters and loaded-context
   bookkeeping, none of which this file measures.

2. test_external_readoption_drops_a_pin_taken_for_another_model sliced on a
   condition that no longer exists. The contract survives in a better form: the
   branch used to clear the pin to null, and now adopts this pick's own pin by the
   rule a completed load writes it, which drops a stale pin the same way and keeps
   a pinned cached row loadable. Rewritten against that, ordering requirement
   unchanged.

3. test_hydration_keeps_the_slot_control_when_readopting_the_running_model was
   asserting behaviour #8943 deliberately reversed, and #8943 is right. Its commit
   says why: the adopt path rolls the outgoing model's config back into the store
   before it hydrates, so the slot and batch controls sitting there belong to the
   model the tab just left, and suppressing the reseed left a resident model
   running 4 slots showing the outgoing count for the next Apply to save over it.
   The test now pins the new contract, asserts nothing reintroduces a same-model
   exemption, and additionally pins the ordering that makes the reseed necessary:
   the rollback must precede the hydration.

  tests/studio: 3896 passed, 4 skipped
  reinstating the readoptingSameModel guard, or moving the rollback after the
  hydration, fails the rewritten test
Following up my own reasoning for accepting #8943's reversal. The reseed is
correct because the same flag gates the remembered lookup: it does not blank the
slot control, it re-reads THIS model's saved config through
resolveResidentInitialConfig. Without that the reversal would take the user's
saved slot count away for real, so it is asserted rather than left implicit.

Also confirmed the attribution exactly: #8943's direct parent is #9026, which is
clean, so there is no intervening commit and one introducer.

  184 passed, and dropping slotsModelChanged from the remembered gate fails it
Comments and docstrings only. The AST gate reports a difference because the
harness is a Python string holding TypeScript, so its // comments are inside a
string literal; every changed line is a comment line in that string or a
docstring, checked line by line, and the tests still pass.

  237 passed
@danielhanchen
danielhanchen merged commit 0575c27 into main Aug 17, 2026
21 of 22 checks passed
@danielhanchen
danielhanchen deleted the fix-contract-anchors-8943 branch August 17, 2026 06:58
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.

1 participant