Add support for Qwen3-4B-Instruct and add model-template mappings#85
Open
Mohammed-Faizzzz wants to merge 6 commits intoFLock-io:mainfrom
Open
Add support for Qwen3-4B-Instruct and add model-template mappings#85Mohammed-Faizzzz wants to merge 6 commits intoFLock-io:mainfrom
Mohammed-Faizzzz wants to merge 6 commits intoFLock-io:mainfrom
Conversation
Contributor
WalkthroughAdds "Qwen/Qwen3-4B-Instruct-2507" to supported base models and introduces a public MODEL_TEMPLATE_MAP that aliases many model IDs to existing template keys; template initialization in template.py now copies templates for mapped model IDs at import time. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App Startup
participant T as src/core/template.py
participant C as src/core/constant.py
App->>T: import template module
T->>T: register_template("qwen1.5"), register_template("qwen3"), register_template("yi"), ...
T->>C: read MODEL_TEMPLATE_MAP
Note right of C: MODEL_TEMPLATE_MAP: { model_id -> template_key }
loop For each (model_id, template_key)
T->>T: template_dict[model_id] = template_dict[template_key]
alt template_key missing
T-->>App: raises KeyError during import
else template_key present
T-->>App: model_id aliases template_key
end
end
App-->>App: Templates available (original + aliases)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/core/template.py (1)
187-188: Guard the mapping and avoid aliasing the same Template instance.
- Missing/typo’ed template keys will raise a KeyError at import; raise a clear error.
- Assigning the same object means in-place mutations (if any) affect all aliases. Clone to be safe.
-for model_name, template_name in MODEL_TEMPLATE_MAP.items(): - template_dict[model_name] = template_dict[template_name] +for model_name, template_name in MODEL_TEMPLATE_MAP.items(): + tmpl = template_dict.get(template_name) + if tmpl is None: + raise KeyError(f"MODEL_TEMPLATE_MAP references unknown template '{template_name}'") + # copy to avoid shared-instance side effects + template_dict[model_name] = Template(**vars(tmpl))Quick check: run a small prompt through a Qwen3-4B-Instruct model to confirm the qwen1.5 tokenization works end-to-end.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/constant.py(2 hunks)src/core/template.py(2 hunks)
🔇 Additional comments (1)
src/core/constant.py (1)
17-17: LGTM: added Qwen/Qwen3-4B-Instruct to supported models.Entry placement under Qwen is consistent.
Comment on lines
+55
to
+109
| MODEL_TEMPLATE_MAP = { | ||
| # Qwen | ||
| "Qwen/Qwen2.5-0.5B": "qwen1.5", | ||
| "Qwen/Qwen2.5-0.5B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen2.5-1.5B": "qwen1.5", | ||
| "Qwen/Qwen2.5-1.5B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen2.5-3B": "qwen1.5", | ||
| "Qwen/Qwen2.5-3B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen2.5-7B": "qwen1.5", | ||
| "Qwen/Qwen2.5-7B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen2.5-14B": "qwen1.5", | ||
| "Qwen/Qwen2.5-14B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen2.5-32B": "qwen1.5", | ||
| "Qwen/Qwen2.5-32B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen2.5-72B": "qwen1.5", | ||
| "Qwen/Qwen2.5-72B-Instruct": "qwen1.5", | ||
| "Qwen/Qwen3-4B-Instruct": "qwen1.5", | ||
| # Yi | ||
| "01-ai/Yi-1.5-6B": "yi", | ||
| "01-ai/Yi-1.5-6B-Chat": "yi", | ||
| "01-ai/Yi-1.5-9B": "yi", | ||
| "01-ai/Yi-1.5-9B-Chat": "yi", | ||
| "01-ai/Yi-1.5-34B": "yi", | ||
| "01-ai/Yi-1.5-34B-Chat": "yi", | ||
| # Mistral | ||
| "mistralai/Mistral-7B-v0.3": "mistral", | ||
| "mistralai/Mistral-7B-Instruct-v0.3": "mistral", | ||
| "mistralai/Ministral-8B-Instruct-2410": "mistral", | ||
| # Mixtral | ||
| "mistralai/Mixtral-8x7B-v0.1": "mixtral", | ||
| "mistralai/Mixtral-8x7B-Instruct-v0.1": "mixtral", | ||
| # Gemma 2 | ||
| "google/gemma-2-2b": "gemma", | ||
| "google/gemma-2-9b": "gemma", | ||
| "google/gemma-2-27b": "gemma", | ||
| "google/gemma-2-2b-it": "gemma", | ||
| "google/gemma-2-9b-it": "gemma", | ||
| "google/gemma-2-27b-it": "gemma", | ||
| # LLaMA 3 + 3.1 | ||
| "meta-llama/Meta-Llama-3-8B": "llama3", | ||
| "meta-llama/Meta-Llama-3-8B-Instruct": "llama3", | ||
| "meta-llama/Meta-Llama-3-70B": "llama3", | ||
| "meta-llama/Meta-Llama-3-70B-Instruct": "llama3", | ||
| "meta-llama/Meta-Llama-3.1-8B": "llama3", | ||
| "meta-llama/Meta-Llama-3.1-8B-Instruct": "llama3", | ||
| "meta-llama/Meta-Llama-3.1-70B": "llama3", | ||
| "meta-llama/Meta-Llama-3.1-70B-Instruct": "llama3", | ||
| # Phi 3 | ||
| "microsoft/Phi-3.5-mini-instruct": "phi3", | ||
| "microsoft/Phi-3-mini-4k-instruct": "phi3", | ||
| "microsoft/Phi-3-medium-4k-instruct": "phi3", | ||
| # Phi 4 | ||
| "microsoft/Phi-4-mini-instruct": "phi4", | ||
| "microsoft/phi-4": "phi4", | ||
| } |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify mapping coverage and consistency with SUPPORTED_BASE_MODELS and registered templates.
- Map includes Mixtral keys that aren’t in SUPPORTED_BASE_MODELS in this file; confirm intent (either add those models to SUPPORTED or prune the entries).
- Double-check that Qwen/Qwen3-4B-Instruct indeed uses the qwen1.5 chat format.
🏁 Script executed:
#!/bin/bash
# Verify: MODEL_TEMPLATE_MAP keys ⊆ SUPPORTED_BASE_MODELS
# and template names exist in src/core/template.py register_template(...) calls.
set -euo pipefail
python - << 'PY'
import ast, pathlib, sys, json
const_p = pathlib.Path("src/core/constant.py")
templ_p = pathlib.Path("src/core/template.py")
def get_supported_and_map():
mod = ast.parse(const_p.read_text(encoding="utf-8"))
supported, mapping = [], {}
for node in mod.body:
if isinstance(node, ast.Assign):
for tgt in node.targets:
if isinstance(tgt, ast.Name) and tgt.id == "SUPPORTED_BASE_MODELS":
if isinstance(node.value, ast.List):
supported = [
e.s for e in node.value.elts if isinstance(e, ast.Str)
]
if isinstance(tgt, ast.Name) and tgt.id == "MODEL_TEMPLATE_MAP":
if isinstance(node.value, ast.Dict):
mapping = {
(k.s if isinstance(k, ast.Str) else None):
(v.s if isinstance(v, ast.Str) else None)
for k, v in zip(node.value.keys, node.value.values)
if isinstance(k, ast.Str) and isinstance(v, ast.Str)
}
return supported, mapping
def get_registered_templates():
mod = ast.parse(templ_p.read_text(encoding="utf-8"))
names = set()
for node in ast.walk(mod):
if isinstance(node, ast.Call):
fn = node.func
if isinstance(fn, ast.Name) and fn.id == "register_template":
for kw in node.keywords:
if kw.arg == "template_name" and isinstance(kw.value, ast.Str):
names.add(kw.value.s)
return names
supported, mapping = get_supported_and_map()
registered = get_registered_templates()
missing_in_supported = sorted(set(mapping.keys()) - set(supported))
unknown_templates = sorted(set(mapping.values()) - registered)
print(json.dumps({
"missing_in_SUPPORTED_BASE_MODELS": missing_in_supported,
"unknown_template_names_in_map": unknown_templates
}, indent=2))
# Non-zero exit if inconsistencies found
if missing_in_supported or unknown_templates:
sys.exit(1)
PYLength of output: 164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import ast, pathlib, json, sys
C = pathlib.Path("src/core/constant.py")
T = pathlib.Path("src/core/template.py")
if not C.exists() or not T.exists():
print(json.dumps({"error":"missing files", "constant_exists": C.exists(), "template_exists": T.exists()}))
sys.exit(2)
c_src = C.read_text(encoding="utf-8")
t_src = T.read_text(encoding="utf-8")
# parse constant.py
modc = ast.parse(c_src)
supported = []
model_map = {}
for node in modc.body:
if isinstance(node, ast.Assign):
for tgt in node.targets:
if isinstance(tgt, ast.Name) and tgt.id == "SUPPORTED_BASE_MODELS":
if isinstance(node.value, (ast.List, ast.Tuple)):
for e in node.value.elts:
if isinstance(e, ast.Constant) and isinstance(e.value, str):
supported.append(e.value)
elif isinstance(e, ast.Str):
supported.append(e.s)
if isinstance(tgt, ast.Name) and tgt.id == "MODEL_TEMPLATE_MAP":
if isinstance(node.value, ast.Dict):
for k,v in zip(node.value.keys, node.value.values):
if isinstance(k, ast.Constant) and isinstance(k.value, str):
key = k.value
elif isinstance(k, ast.Str):
key = k.s
else:
continue
if isinstance(v, ast.Constant) and isinstance(v.value, str):
val = v.value
elif isinstance(v, ast.Str):
val = v.s
else:
continue
model_map[key] = val
missing = sorted([k for k in model_map.keys() if k not in supported])
# parse template.py for register_template calls of interest
modt = ast.parse(t_src)
def is_register_call(node):
if not isinstance(node, ast.Call): return False
fn = node.func
if isinstance(fn, ast.Name) and fn.id == "register_template": return True
if isinstance(fn, ast.Attribute) and fn.attr == "register_template": return True
return False
results = []
for node in ast.walk(modt):
if is_register_call(node):
# get template_name kw
tmpl = None
kw_repr = {}
for kw in node.keywords:
k = kw.arg
val = None
if isinstance(kw.value, ast.Constant):
val = kw.value.value
elif isinstance(kw.value, ast.Str):
val = kw.value.s
else:
try:
val = ast.unparse(kw.value)
except Exception:
val = "<expr>"
kw_repr[k] = val
if k == "template_name" and isinstance(val, str):
tmpl = val
if tmpl in ("qwen1.5", "mixtral"):
start = node.lineno - 1
end = getattr(node, "end_lineno", node.lineno)
snippet = "\n".join(t_src.splitlines()[start:end])
results.append({"template_name": tmpl, "lineno": node.lineno, "end_lineno": end, "keywords": kw_repr, "src_snippet": snippet})
out = {
"missing_in_SUPPORTED_BASE_MODELS": missing,
"model_map_keys_count": len(model_map),
"supported_count": len(supported),
"sample_supported": supported[:40],
"templates_inspected": results
}
print(json.dumps(out, indent=2))
PYLength of output: 3439
Fix MODEL_TEMPLATE_MAP / SUPPORTED_BASE_MODELS mismatch
- Two MODEL_TEMPLATE_MAP keys are not in SUPPORTED_BASE_MODELS: "mistralai/Mixtral-8x7B-v0.1" and "mistralai/Mixtral-8x7B-Instruct-v0.1". Add them to SUPPORTED_BASE_MODELS in src/core/constant.py or remove/prune these keys from MODEL_TEMPLATE_MAP.
- The qwen1.5 template is registered in src/core/template.py (lines 59–69) and uses chat-style markers (<|im_start|>/<|im_end|>), so mapping "Qwen/Qwen3-4B-Instruct" → "qwen1.5" is consistent.
| @@ -1,5 +1,6 @@ | |||
| from dataclasses import dataclass | |||
| from typing import Dict | |||
| from constant import MODEL_TEMPLATE_MAP | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use package-relative import to avoid import errors with src/ layout.
Direct from constant import ... will fail when imported as a package. Prefer a relative import (optionally keep a fallback for ad-hoc scripts).
-from constant import MODEL_TEMPLATE_MAP
+try:
+ from .constant import MODEL_TEMPLATE_MAP # package-relative
+except ImportError: # pragma: no cover
+ from constant import MODEL_TEMPLATE_MAP # fallback for direct execution📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from constant import MODEL_TEMPLATE_MAP | |
| try: | |
| from .constant import MODEL_TEMPLATE_MAP # package-relative | |
| except ImportError: # pragma: no cover | |
| from constant import MODEL_TEMPLATE_MAP # fallback for direct execution |
🤖 Prompt for AI Agents
In src/core/template.py around line 3, the direct top-level import "from
constant import MODEL_TEMPLATE_MAP" will break when the package is imported;
change to a package-relative import and optionally provide a fallback for ad-hoc
execution: use "from .constant import MODEL_TEMPLATE_MAP" as the primary import
and wrap it in a try/except that falls back to "from constant import
MODEL_TEMPLATE_MAP" if the relative import fails, so both package usage and
standalone/script runs work.
for more information, see https://pre-commit.ci
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/core/template.py (1)
3-3: Use package-relative import with fallback.Prevents import errors under src/ layout and keeps ad‑hoc script compatibility.
-from constant import MODEL_TEMPLATE_MAP +try: + from .constant import MODEL_TEMPLATE_MAP # package-relative +except ImportError: # pragma: no cover + from constant import MODEL_TEMPLATE_MAP # fallback for direct execution
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/core/constant.py(2 hunks)src/core/template.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/constant.py
Comment on lines
+71
to
+88
| register_template( | ||
| template_name="qwen3", | ||
| system_format="<|im_start|>system\n{content}<|im_end|>\n", | ||
| user_format="<|im_start|>user\n{content}<|im_end|>\n<|im_start|>assistant\n", | ||
| assistant_format="{content}<|im_end|>\n", | ||
| tool_format = ( | ||
| "# Tools\n\n" | ||
| "You may call one or more functions to assist with the user query.\n\n" | ||
| "You are provided with function signatures within <tools></tools> XML tags:\n" | ||
| "<tools>\n{content}\n</tools>\n\n" | ||
| "For each function call, return a json object with function name and arguments within <tool_call></tool_call> XML tags:\n" | ||
| "<tool_call>\n{\"name\": <function-name>, \"arguments\": <args-json-object>}\n</tool_call>" | ||
| ), | ||
| function_format="<tool_call>\n{content}\n</tool_call><|im_end|>\n", | ||
| observation_format="<|im_start|>user\n<tool_response>\n{content}\n</tool_response><|im_end|>\n<|im_start|>assistant\n", | ||
| system="You are a helpful assistant.", | ||
| stop_word="<|im_end|>", | ||
| ) |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix unescaped braces in qwen3 tool_format; .format() will crash.
The JSON example in tool_format includes bare {} which collide with str.format. Escape them.
- tool_format = (
+ tool_format = (
"# Tools\n\n"
"You may call one or more functions to assist with the user query.\n\n"
"You are provided with function signatures within <tools></tools> XML tags:\n"
"<tools>\n{content}\n</tools>\n\n"
"For each function call, return a json object with function name and arguments within <tool_call></tool_call> XML tags:\n"
- "<tool_call>\n{\"name\": <function-name>, \"arguments\": <args-json-object>}\n</tool_call>"
+ "<tool_call>\n{{\"name\": <function-name>, \"arguments\": <args-json-object>}}\n</tool_call>"
),If you want, I can add a quick unit test that formats every template field with content="x" to catch such issues proactively.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- show src/core/template.py (1-240) ----"
nl -ba src/core/template.py | sed -n '1,240p' || true
echo
echo "---- rg register_template calls ----"
rg -n "register_template\(" || true
echo
echo "---- rg def register_template ----"
rg -n "def register_template" || true
echo
echo "---- rg .format( occurrences (2 lines context) ----"
rg -n --hidden --no-ignore-vcs "\.format\(" -S -g '!**/node_modules/**' -C2 || true
echo
echo "---- rg {content} occurrences (2 lines context) ----"
rg -n --hidden --no-ignore-vcs "\{content\}" -S -C2 -g '!**/node_modules/**' || true
echo
echo "---- rg tool_format occurrences ----"
rg -n "tool_format" -S || trueLength of output: 2408
Escape braces in qwen3 tool_format — fix str.format() crash
Unescaped JSON braces in the qwen3 tool_format will be parsed by str.format() and raise a ValueError at runtime (see src/core/dataset.py:49 where .format(...) is called). Fix by doubling the literal braces in the JSON example.
File: src/core/template.py (register_template qwen3, ~lines 71-88)
- tool_format = (
+ tool_format = (
"# Tools\n\n"
"You may call one or more functions to assist with the user query.\n\n"
"You are provided with function signatures within <tools></tools> XML tags:\n"
"<tools>\n{content}\n</tools>\n\n"
"For each function call, return a json object with function name and arguments within <tool_call></tool_call> XML tags:\n"
- "<tool_call>\n{\"name\": <function-name>, \"arguments\": <args-json-object>}\n</tool_call>"
+ "<tool_call>\n{{\"name\": <function-name>, \"arguments\": <args-json-object>}}\n</tool_call>"
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| register_template( | |
| template_name="qwen3", | |
| system_format="<|im_start|>system\n{content}<|im_end|>\n", | |
| user_format="<|im_start|>user\n{content}<|im_end|>\n<|im_start|>assistant\n", | |
| assistant_format="{content}<|im_end|>\n", | |
| tool_format = ( | |
| "# Tools\n\n" | |
| "You may call one or more functions to assist with the user query.\n\n" | |
| "You are provided with function signatures within <tools></tools> XML tags:\n" | |
| "<tools>\n{content}\n</tools>\n\n" | |
| "For each function call, return a json object with function name and arguments within <tool_call></tool_call> XML tags:\n" | |
| "<tool_call>\n{\"name\": <function-name>, \"arguments\": <args-json-object>}\n</tool_call>" | |
| ), | |
| function_format="<tool_call>\n{content}\n</tool_call><|im_end|>\n", | |
| observation_format="<|im_start|>user\n<tool_response>\n{content}\n</tool_response><|im_end|>\n<|im_start|>assistant\n", | |
| system="You are a helpful assistant.", | |
| stop_word="<|im_end|>", | |
| ) | |
| register_template( | |
| template_name="qwen3", | |
| system_format="<|im_start|>system\n{content}<|im_end|>\n", | |
| user_format="<|im_start|>user\n{content}<|im_end|>\n<|im_start|>assistant\n", | |
| assistant_format="{content}<|im_end|>\n", | |
| tool_format = ( | |
| "# Tools\n\n" | |
| "You may call one or more functions to assist with the user query.\n\n" | |
| "You are provided with function signatures within <tools></tools> XML tags:\n" | |
| "<tools>\n{content}\n</tools>\n\n" | |
| "For each function call, return a json object with function name and arguments within <tool_call></tool_call> XML tags:\n" | |
| "<tool_call>\n{{\"name\": <function-name>, \"arguments\": <args-json-object>}}\n</tool_call>" | |
| ), | |
| function_format="<tool_call>\n{content}\n</tool_call><|im_end|>\n", | |
| observation_format="<|im_start|>user\n<tool_response>\n{content}\n</tool_response><|im_end|>\n<|im_start|>assistant\n", | |
| system="You are a helpful assistant.", | |
| stop_word="<|im_end|>", | |
| ) |
🤖 Prompt for AI Agents
In src/core/template.py around lines 71 to 88, the qwen3 template's tool_format
contains unescaped JSON braces which are consumed by str.format() and cause a
ValueError at runtime; fix by doubling all literal braces in the JSON example
inside tool_format (replace { with {{ and } with }} for the JSON snippet and any
other literal braces) so that str.format() treats them as literal characters
while leaving the format placeholders like {content} intact.
Comment on lines
+206
to
+207
| for model_name, template_name in MODEL_TEMPLATE_MAP.items(): | ||
| template_dict[model_name] = template_dict[template_name] |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Copy and validate when aliasing templates to avoid KeyError and shared-state bugs.
- Guard against mappings to unknown template names.
- Create a new Template instance per alias to avoid accidental cross-mutation.
-for model_name, template_name in MODEL_TEMPLATE_MAP.items():
- template_dict[model_name] = template_dict[template_name]
+missing = []
+for model_name, template_name in MODEL_TEMPLATE_MAP.items():
+ base = template_dict.get(template_name)
+ if base is None:
+ missing.append((model_name, template_name))
+ continue
+ # copy to avoid shared object between aliases
+ template_dict[model_name] = replace(base)
+if missing:
+ raise KeyError(f"MODEL_TEMPLATE_MAP points to unknown template(s): {missing}")Add import (outside the changed hunk) to support replace:
-from dataclasses import dataclass
+from dataclasses import dataclass, replace📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for model_name, template_name in MODEL_TEMPLATE_MAP.items(): | |
| template_dict[model_name] = template_dict[template_name] | |
| # (update import at top of file) | |
| from dataclasses import dataclass, replace |
Suggested change
| for model_name, template_name in MODEL_TEMPLATE_MAP.items(): | |
| template_dict[model_name] = template_dict[template_name] | |
| missing = [] | |
| for model_name, template_name in MODEL_TEMPLATE_MAP.items(): | |
| base = template_dict.get(template_name) | |
| if base is None: | |
| missing.append((model_name, template_name)) | |
| continue | |
| # copy to avoid shared object between aliases | |
| template_dict[model_name] = replace(base) | |
| if missing: | |
| raise KeyError(f"MODEL_TEMPLATE_MAP points to unknown template(s): {missing}") |
🤖 Prompt for AI Agents
In src/core/template.py around lines 206-207, the aliasing loop blindly assigns
references causing KeyError for unknown targets and shared-state bugs; change it
to first check that template_name exists in template_dict and raise or log a
clear KeyError with context if not, then assign a fresh copy for the alias
(e.g., template_dict[model_name] = deepcopy(template_dict[template_name]] ) or
construct a new Template instance from the source so the alias does not share
mutable state; also add the needed import at the top (from copy import deepcopy)
to support the replacement.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
requirements.txt (1)
2-3: Reproducibility: move broad ranges behind a constraints file.Version ranges here improve flexibility but reduce determinism across environments. Recommend locking via a constraints/lock file in CI while keeping user‑facing ranges.
I can draft constraints.txt derived from your current solver output if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
requirements.txt(1 hunks)src/core/constant.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/core/constant.py
🔇 Additional comments (1)
requirements.txt (1)
2-2: Resolved — huggingface-hub 0.34.0 requires Python >=3.8.0 (no 3.8 drop). PyPI metadata shows requires_python=">=3.8.0" for v0.34.0, so this upgrade does not remove Python 3.8 support; no change required unless you intend to require 3.9+ (then update docs/classifiers).
| huggingface-hub==0.29.1 | ||
| transformers==4.49.0 | ||
| huggingface-hub>=0.34.0,<1.0 | ||
| transformers>=4.51.0 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Add an upper bound to transformers to prevent future major‑version breakage.
Guard against a v5 jump changing APIs.
Apply:
-transformers>=4.51.0
+transformers>=4.51.0,<5.0Also, verify 4.51.x remains compatible with your pinned Torch (torch>=1.13.1) and accelerate (>=0.27.2); Transformers’ optional PyTorch features may expect newer Torch.
🌐 Web query:
What PyTorch versions are officially supported by Transformers 4.51.x, and does it require Python >=3.9?
💡 Result:
Short answer: Transformers 4.51.x requires Python >= 3.9 and is intended to be used with PyTorch 2.0 or newer (i.e., PyTorch 2.0+). [1][2]
Sources:
- transformers 4.51.3 (PyPI) — Requires: Python >=3.9; "Transformers works with ... PyTorch 2.0+." [1]
- Transformers installation docs (nearby 4.x releases) confirming PyTorch 2.x compatibility and Python 3.9+ testing. [2]
Add upper bound to transformers and resolve PyTorch/Python compatibility
Apply:
-transformers>=4.51.0
+transformers>=4.51.0,<5.0- File: requirements.txt (line 3).
- Rationale: Transformers 4.51.x requires Python >=3.9 and PyTorch 2.0+, so the current torch>=1.13.1 is incompatible. Either upgrade torch to 2.0+ (and confirm accelerate compatibility) or pin transformers to a 4.x release that supports torch 1.13.1. Verify CI/runtime Python version is >=3.9.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| transformers>=4.51.0 | |
| transformers>=4.51.0,<5.0 |
🤖 Prompt for AI Agents
In requirements.txt around line 3, transformers is pinned to >=4.51.0 which
requires Python >=3.9 and PyTorch 2.0+, causing incompatibility with the repo's
torch>=1.13.1; either (A) upgrade torch to >=2.0.0 (and ensure accelerate is
compatible) and confirm CI/runtime Python >=3.9, or (B) constrain transformers
to a 4.x release compatible with torch 1.13.1 (e.g., <=4.50.*) and verify
CI/runtime Python is still supported; update the requirements.txt accordingly
and run CI to ensure all dependency combinations pass.
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
1 participant
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.
Qwen3-4B-Instructto SUPPORTED_BASE_MODELSSummary by CodeRabbit
New Features
Chores