Fix/default model backfill select best - #4045
Merged
Merged
Conversation
added 3 commits
September 30, 2026 09:51
… models The default-model backfill runs after EVERY model creation, and a slot it fills is treated as final. Batch adds create models one by one, so the first-created model permanently occupied the slot before better candidates landed -- the "available first, then larger context window" ranking never got to compare across the batch. Observed live: a 5-model batch import left a 256K-context model as the default LLM while two 1M-context models arrived right after it. Distinguish user choices from backfill placeholders via the config row's user_id: the UI save path (set_single_config) stamps the acting user on rows it writes, backfill-inserted rows leave it empty. Backfill now: - never touches a slot whose row carries a user_id (user's explicit choice) - re-evaluates a previously auto-configured slot on every create and swaps in the best candidate (available first, then larger context); the first user save flips the row to user-owned and locks it - repairs dangling rows and fills never-configured slots as before get_single_config_info now also returns the row's user_id for this classification.
The backfill used to pick the best candidate from ALL live models of a slot's type. When a user deliberately cleared a default slot and then added one new model, the backfill resurrected an older, larger-context model they had passed over -- silently overriding the clear. Pass the ids of the models created by the current call into the backfill: - an empty slot (never configured, or cleared by the user) is now filled only from those newly created models; if the call added none of the slot's type, the slot stays empty - dangling rows still repair from the full pool (the previous choice is gone, so the best remaining replacement is appropriate) - auto-configured slots keep re-evaluating among all candidates (the larger-context swap from the previous commit) - user-configured slots remain locked create_model_record returns only a bool, so the new ids are recovered via display-name lookup (_ids_for_created_models); multi_embedding creates include their embedding twin automatically.
…session The auto-slot swap from the earlier commit never expired: an auto-configured default could be replaced by a better model at any later create, so adding models months after an import could still move the default. Users expect a default that has been sitting in the slot to stay put -- only the batch import that is still in progress should converge on the best model. Gate the swap on occupant freshness: - swap candidates are the current occupant plus the models created in the current call; older models the user passed over are never resurrected through the swap path (previously the swap re-ranked ALL models of the type, so a cleared-then-refilled slot could drift back to an old giant) - the swap only runs while the occupant was created within _AUTO_SLOT_SWAP_WINDOW (5 minutes) of the newest model in the current call -- batch imports create their rows seconds apart, so mid-batch convergence still works; an occupant from an earlier session is frozen - timestamps come from the DB on both sides, so no clock/timezone skew; missing create_time disables the gate (permissive, legacy behaviour) - empty-slot fill (new-only), dangling repair and user-choice locking are unchanged
lijiayang619
requested review from
Dallas98,
WMC001,
YehongPan,
hhhhsc701 and
jeffwu-1999
as code owners
September 30, 2026 02:28
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Batch detection, ownership inference, and display-name ID recovery can select or overwrite incorrect defaults.
Review effort: Balanced
Findings: 3
Open (5)
What changed in this PR
Updates default-model backfilling after model creation.
Changes:
- Restricts empty-slot candidates to newly created models.
- Adds automatic-default replacement logic.
- Expands regression tests.
| File | Description |
|---|---|
backend/services/model_management_service.py |
Implements candidate selection and backfill behavior. |
backend/database/tenant_config_db.py |
Exposes config-row user ownership. |
test/backend/services/test_model_management_service.py |
Adds backfill regression tests. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The auto-slot swap was removed: a default slot occupied by ANY live model -- user-picked or system-backfilled -- is now never touched by later creates. Users expect "the slot already has a model" to mean exactly that; swapping in a better model months after an import silently moved defaults and compounded with the empty-slot rules into hard-to-predict behaviour. The previous commit's freshness window tried to reconcile this with mid-batch convergence via heuristics; explicit batch context replaces it. Batch imports now carry their own flow control: - ModelRequest gains an optional skip_default_backfill flag (popped by the app layer before the dict reaches the service/DB layer, same contract as the accept-signal fields). The batch dialog marks every per-row create with it, so no row claims empty slots as it lands. - A new POST /model/backfill_defaults endpoint finalizes the batch: it resolves the created display names to ids and runs the backfill ONCE with the whole batch as candidates, so an empty slot gets the best model of the batch (available first, then larger context) in a single decision. - The user-facing batch dialog calls the finalize after its loop; the manage-tenant path keeps its existing per-row behaviour (its request model ignores the flag and its frontend service does not forward it). Final slot semantics: occupied -> never touched; empty -> best of the current call's new models (or all models for legacy callers); dangling -> repaired from the full pool; user choice -> locked.
jeffwu-1999
approved these changes
Sep 30, 2026
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.
修改了默认模型配置逻辑，
1.没有默认模型的时候，批量添加直接选上下文最大的
2.如果已有默认模型，就添加模型的时候不会再去变动
3.如果默认模型是空的，那就从新添加的里面选
没有默认模型就选上下文最大
有模型但是没有默认模型，从新添加的模型里面选