From d339139fb6f8a30408c962df8a4ff72704986e31 Mon Sep 17 00:00:00 2001 From: kingbri <8082010+kingbri1@users.noreply.github.com> Date: Thu, 3 Jul 2025 12:17:09 -0400 Subject: [PATCH] Config: Deep merge model overrides Anything below the first level of kwargs was not being merged properly. A more bulletproof solution would be to refactor the loading code to separate draft and normal model parameters. Signed-off-by: kingbri <8082010+kingbri1@users.noreply.github.com> --- common/model.py | 5 +++-- common/tabby_config.py | 4 ++-- common/utils.py | 23 +++++++++++++++++------ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/common/model.py b/common/model.py index 6534951..a26aa17 100644 --- a/common/model.py +++ b/common/model.py @@ -18,7 +18,7 @@ from common.networking import handle_request_error from common.tabby_config import config from common.optional_dependencies import dependencies from common.transformers_utils import HFModel -from common.utils import unwrap +from common.utils import deep_merge_dict, unwrap # Global variables for model container container: Optional[BaseModelContainer] = None @@ -104,7 +104,8 @@ async def apply_inline_overrides(model_dir: pathlib.Path, **kwargs): overrides["draft_model"] = {**draft_inline_config} # Merge the override and model kwargs - merged_kwargs = {**overrides, **kwargs} + # No need to preserve the original overrides dict + merged_kwargs = deep_merge_dict(overrides, kwargs) return merged_kwargs diff --git a/common/tabby_config.py b/common/tabby_config.py index d41cc64..865ae28 100644 --- a/common/tabby_config.py +++ b/common/tabby_config.py @@ -11,7 +11,7 @@ from ruamel.yaml.comments import CommentedMap, CommentedSeq from ruamel.yaml.scalarstring import PreservedScalarString from common.config_models import BaseConfigModel, TabbyConfigModel -from common.utils import merge_dicts, filter_none_values, unwrap +from common.utils import deep_merge_dicts, filter_none_values, unwrap yaml = YAML(typ=["rt", "safe"]) @@ -36,7 +36,7 @@ class TabbyConfig(TabbyConfigModel): # Remove None (aka unset) values from the configs and merge them together # This should be less expensive than pruning the entire merged dictionary configs = filter_none_values(configs) - merged_config = merge_dicts(*configs) + merged_config = deep_merge_dicts(*configs) # validate and update config merged_config_model = TabbyConfigModel.model_validate(merged_config) diff --git a/common/utils.py b/common/utils.py index 13668f4..b0d7ad2 100644 --- a/common/utils.py +++ b/common/utils.py @@ -32,21 +32,32 @@ def filter_none_values(collection: Union[dict, list]) -> Union[dict, list]: return collection -def merge_dict(dict1: Dict, dict2: Dict) -> Dict: - """Merge 2 dictionaries""" +def deep_merge_dict(dict1: Dict, dict2: Dict, copy: bool = False) -> Dict: + """ + Merge 2 dictionaries. If copy is true, the original dictionary isn't modified. + """ + + if copy: + dict1 = dict1.copy() + for key, value in dict2.items(): if isinstance(value, dict) and key in dict1 and isinstance(dict1[key], dict): - merge_dict(dict1[key], value) + deep_merge_dict(dict1[key], value, copy=False) else: dict1[key] = value + return dict1 -def merge_dicts(*dicts: Dict) -> Dict: - """Merge an arbitrary amount of dictionaries""" +def deep_merge_dicts(*dicts: Dict) -> Dict: + """ + Merge an arbitrary amount of dictionaries. + We wanna do in-place modification for each level, so do not copy. + """ + result = {} for dictionary in dicts: - result = merge_dict(result, dictionary) + result = deep_merge_dict(result, dictionary) return result