{{ message }}
Repair the contract tests #8943 left behind on main - #9067
Merged
Conversation
…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
for more information, see https://pre-commit.ci
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

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.
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
adoptableand is covered on its own by the resident-model-match andresident-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.
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.
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