Skip to content

Commit 9bd1960

Browse files
OriNachumclaude
andcommitted
fix(profiles): validate RoleProfile knob types, normalise operator profile names
RoleProfile.from_dict validated key names only, so a TOML typo like feasible = "false" (a truthy string) silently passed construction and the renderer's truthiness check flipped a role feasible; now every knob's TYPE is checked (bool strictly for feasible, int/float-not-bool for numeric knobs, str-or-None for string knobs) and a bad value raises ModelGearError naming the role, knob, expected and got types. discover_operator_profiles() keyed profiles by the raw filename stem, so an operator file like Thor.toml never matched resolve_profile()'s .strip().lower() lookup and silently failed to override the builtin; keys are now normalised the same way, with a clear error on a post-normalisation collision (Thor.toml + thor.toml). Also fixed resolve_init_profile's --profile-vs-detected-card comparison to use normalised forms so a casing-only difference (--profile Spark on a detected "spark" card) no longer triggers a spurious mismatch warning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJc5yvfweHP2AEccKNeaVd
1 parent aa2c33e commit 9bd1960

5 files changed

Lines changed: 235 additions & 3 deletions

File tree

lobes/cli/_runtime_ops.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,11 @@ def resolve_init_profile(
143143
f"--profile {name!r} used on an undetected card ({facts}) — "
144144
"proceeding, but this profile was not validated for this box."
145145
)
146-
elif card.resolved != name:
146+
elif card.resolved != name.strip().lower():
147+
# Compare NORMALISED forms: card.resolved is always the registry's
148+
# lowercase canonical name, but `name` is the raw --profile value
149+
# as typed — `--profile Spark` on a detected "spark" card must not
150+
# warn on casing alone (resolve_profile() normalises the same way).
147151
warning = (
148152
f"--profile {name!r} overrides the detected card "
149153
f"{card.resolved!r} ({facts}) — proceeding, but this profile "

lobes/profiles/loader.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,45 @@ def discover_operator_profiles(deploy_dir: Path | str) -> dict[str, Profile]:
121121
122122
Absent/empty directory -> ``{}`` (never raises just for "no operator
123123
profiles here" — that is the common case, not an error).
124+
125+
Keyed by the filename stem NORMALISED (``.strip().lower()``), matching how
126+
:func:`resolve_profile` normalises the requested name — an operator file
127+
named e.g. ``Thor.toml`` must still be found when a caller asks for
128+
``thor`` (or ``THOR``, or `` thor ``). Two files that collide after
129+
normalisation (``Thor.toml`` and ``thor.toml`` both present) is an
130+
unresolvable ambiguity — which one wins is a coin flip an operator would
131+
never want silently made for them — so it is a LOAD ERROR rather than a
132+
silently-picked winner.
124133
"""
125134
operator_dir = _operator_dir(deploy_dir)
126135
if not operator_dir.is_dir():
127136
return {}
128137
found: dict[str, Profile] = {}
138+
raw_names_by_key: dict[str, str] = {}
129139
for path in sorted(operator_dir.glob(f"*{PROFILE_SUFFIX}")):
130-
name = path.stem
140+
raw_name = path.stem
141+
key = raw_name.strip().lower()
142+
if key in found:
143+
raise ModelGearError(
144+
code=EXIT_USER_ERROR,
145+
message=(
146+
f"operator profile name collision in {operator_dir}: "
147+
f"{raw_names_by_key[key]!r} and {raw_name!r} both normalise "
148+
f"to {key!r}"
149+
),
150+
remediation=(
151+
"rename one of the files so their names differ after "
152+
"case-folding (profile names are matched case-insensitively)"
153+
),
154+
)
131155
data = _parse(path.read_text(encoding="utf-8"), source=str(path))
132-
found[name] = Profile.from_dict(name, data)
156+
# Pass the RAW stem (not the normalised key) as the profile's declared
157+
# identity — the file's own casing is what an embedded `name = "..."`
158+
# field (if present) must match, per Profile.from_dict's mismatch
159+
# check; only the DICT KEY used for lookup is normalised, matching how
160+
# resolve_profile() normalises the requested name.
161+
found[key] = Profile.from_dict(raw_name, data)
162+
raw_names_by_key[key] = raw_name
133163
return found
134164

135165

lobes/profiles/schema.py

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,51 @@ def _profile_error(message: str, remediation: str) -> ModelGearError:
5353
return ModelGearError(code=EXIT_USER_ERROR, message=message, remediation=remediation)
5454

5555

56+
def _is_strict_bool(value: Any) -> bool:
57+
return isinstance(value, bool)
58+
59+
60+
def _is_optional_bool(value: Any) -> bool:
61+
return value is None or isinstance(value, bool)
62+
63+
64+
def _is_optional_str(value: Any) -> bool:
65+
return value is None or isinstance(value, str)
66+
67+
68+
def _is_optional_number(value: Any) -> bool:
69+
# bool is a subclass of int in Python — reject it explicitly BEFORE the
70+
# isinstance(value, (int, float)) check, or `feasible = "false"`-style
71+
# TOML mistakes (here, a stray `true`/`false` on a numeric knob) would
72+
# silently pass as a number.
73+
if isinstance(value, bool):
74+
return False
75+
return value is None or isinstance(value, (int, float))
76+
77+
78+
def _is_optional_int(value: Any) -> bool:
79+
if isinstance(value, bool):
80+
return False
81+
return value is None or isinstance(value, int)
82+
83+
84+
# Per-field type validator + human-readable "expected" description, used by
85+
# RoleProfile.from_dict to reject a value of the wrong TYPE (not just an
86+
# unknown key) — e.g. `feasible = "false"` (a truthy STRING) must fail loudly
87+
# rather than silently flip a role to feasible via Python truthiness.
88+
_FIELD_VALIDATORS: dict[str, tuple[Any, str]] = {
89+
"feasible": (_is_strict_bool, "bool"),
90+
"model": (_is_optional_str, "str or None"),
91+
"gpu_mem_util": (_is_optional_number, "int/float or None"),
92+
"max_model_len": (_is_optional_int, "int or None"),
93+
"quantization": (_is_optional_str, "str or None"),
94+
"kv_cache_dtype": (_is_optional_str, "str or None"),
95+
"attention_backend": (_is_optional_str, "str or None"),
96+
"enforce_eager": (_is_optional_bool, "bool or None"),
97+
"max_num_seqs": (_is_optional_int, "int or None"),
98+
}
99+
100+
56101
@dataclass(frozen=True)
57102
class RoleProfile:
58103
"""One role's serving declaration within a :class:`Profile`.
@@ -99,6 +144,19 @@ def from_dict(role: str, data: Mapping[str, Any]) -> "RoleProfile":
99144
message=f"unknown knob(s) {sorted(unknown)!r} for role {role!r}",
100145
remediation=f"known knobs: feasible, model, {', '.join(KNOB_NAMES)}",
101146
)
147+
for key, value in data.items():
148+
validator, expected = _FIELD_VALIDATORS[key]
149+
if not validator(value):
150+
got = type(value).__name__
151+
raise _profile_error(
152+
message=(
153+
f"role {role!r}: knob {key!r} must be {expected}, " f"got {got} ({value!r})"
154+
),
155+
remediation=(
156+
f"fix the value's type for {key!r} in role {role!r} "
157+
f"(expected {expected})"
158+
),
159+
)
102160
return RoleProfile(**dict(data))
103161

104162

tests/test_init_profile.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,3 +285,30 @@ def test_resolve_init_profile_returns_no_warning_on_clean_match(tmp_path) -> Non
285285
assert profile.name == "spark"
286286
assert card.resolved == "spark"
287287
assert warning is None
288+
289+
290+
def test_resolve_init_profile_explicit_profile_casing_only_difference_warns_not(
291+
tmp_path,
292+
) -> None:
293+
# The bug this guards: card.resolved is always the registry's lowercase
294+
# canonical name, but an explicit --profile is compared RAW — so
295+
# `--profile Spark` on a detected "spark" card used to warn on casing
296+
# alone. Compare normalised forms instead.
297+
profile, card, warning = _runtime_ops.resolve_init_profile(
298+
"Spark", tmp_path, detect_fn=lambda: _fake_card("spark")
299+
)
300+
assert profile.name == "spark"
301+
assert card.resolved == "spark"
302+
assert warning is None
303+
304+
305+
def test_resolve_init_profile_explicit_profile_genuine_mismatch_still_warns(
306+
tmp_path,
307+
) -> None:
308+
profile, card, warning = _runtime_ops.resolve_init_profile(
309+
"Spark", tmp_path, detect_fn=lambda: _fake_card("thor")
310+
)
311+
assert profile.name == "spark"
312+
assert card.resolved == "thor"
313+
assert warning is not None
314+
assert "detected card 'thor'" in warning

tests/test_profile_schema.py

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,86 @@ def test_profile_from_dict_rejects_unknown_knob() -> None:
7777
assert "not_a_knob" in exc.value.message
7878

7979

80+
def test_role_profile_from_dict_rejects_string_false_for_feasible() -> None:
81+
# The bug this guards: `feasible = "false"` is a non-empty STRING, which
82+
# is truthy in Python — the renderer's `if not rp.feasible` must never
83+
# see a value like this pass validation and silently flip a role to
84+
# feasible.
85+
with pytest.raises(ModelGearError) as exc:
86+
RoleProfile.from_dict("cortex", {"feasible": "false"})
87+
assert exc.value.code == EXIT_USER_ERROR
88+
assert "cortex" in exc.value.message
89+
assert "feasible" in exc.value.message
90+
assert "bool" in exc.value.message
91+
assert "str" in exc.value.message
92+
93+
94+
def test_role_profile_from_dict_rejects_string_false_for_enforce_eager() -> None:
95+
# Same bug for enforce_eager: a truthy string would render `--enforce-eager`
96+
# from a TOML value the operator intended as "off".
97+
with pytest.raises(ModelGearError) as exc:
98+
RoleProfile.from_dict("reranker", {"enforce_eager": "false"})
99+
assert exc.value.code == EXIT_USER_ERROR
100+
assert "reranker" in exc.value.message
101+
assert "enforce_eager" in exc.value.message
102+
103+
104+
def test_role_profile_from_dict_accepts_none_for_enforce_eager() -> None:
105+
rp = RoleProfile.from_dict("cortex", {"enforce_eager": None})
106+
assert rp.enforce_eager is None
107+
108+
109+
def test_role_profile_from_dict_rejects_none_for_feasible() -> None:
110+
# feasible has no Optional in the schema (default True, never None).
111+
with pytest.raises(ModelGearError) as exc:
112+
RoleProfile.from_dict("cortex", {"feasible": None})
113+
assert exc.value.code == EXIT_USER_ERROR
114+
assert "feasible" in exc.value.message
115+
116+
117+
def test_role_profile_from_dict_rejects_bool_for_gpu_mem_util() -> None:
118+
# bool is a subclass of int in Python — must be rejected explicitly for a
119+
# numeric knob, not silently accepted as 0.0/1.0.
120+
with pytest.raises(ModelGearError) as exc:
121+
RoleProfile.from_dict("cortex", {"gpu_mem_util": True})
122+
assert exc.value.code == EXIT_USER_ERROR
123+
assert "gpu_mem_util" in exc.value.message
124+
assert "cortex" in exc.value.message
125+
126+
127+
def test_role_profile_from_dict_accepts_int_and_float_for_gpu_mem_util() -> None:
128+
assert RoleProfile.from_dict("cortex", {"gpu_mem_util": 1}).gpu_mem_util == 1
129+
assert RoleProfile.from_dict("cortex", {"gpu_mem_util": 0.3}).gpu_mem_util == 0.3
130+
131+
132+
def test_role_profile_from_dict_rejects_bool_for_max_model_len() -> None:
133+
with pytest.raises(ModelGearError) as exc:
134+
RoleProfile.from_dict("cortex", {"max_model_len": False})
135+
assert exc.value.code == EXIT_USER_ERROR
136+
assert "max_model_len" in exc.value.message
137+
138+
139+
def test_role_profile_from_dict_rejects_bool_for_max_num_seqs() -> None:
140+
with pytest.raises(ModelGearError) as exc:
141+
RoleProfile.from_dict("cortex", {"max_num_seqs": True})
142+
assert exc.value.code == EXIT_USER_ERROR
143+
assert "max_num_seqs" in exc.value.message
144+
145+
146+
def test_role_profile_from_dict_rejects_wrong_type_for_string_knobs() -> None:
147+
for knob in ("model", "quantization", "kv_cache_dtype", "attention_backend"):
148+
with pytest.raises(ModelGearError) as exc:
149+
RoleProfile.from_dict("cortex", {knob: 123})
150+
assert exc.value.code == EXIT_USER_ERROR
151+
assert knob in exc.value.message
152+
153+
154+
def test_role_profile_from_dict_accepts_none_for_string_knobs() -> None:
155+
for knob in ("model", "quantization", "kv_cache_dtype", "attention_backend"):
156+
rp = RoleProfile.from_dict("cortex", {knob: None})
157+
assert getattr(rp, knob) is None
158+
159+
80160
def test_profile_from_dict_rejects_unknown_top_level_key() -> None:
81161
with pytest.raises(ModelGearError):
82162
Profile.from_dict("bogus", {"nope": True})
@@ -268,6 +348,39 @@ def test_operator_profile_overrides_a_builtin_of_the_same_name(tmp_path) -> None
268348
assert builtin_spark.role("cortex").model == "sakamakismile/Qwen3.6-27B-Text-NVFP4-MTP"
269349

270350

351+
def test_mixed_case_operator_file_overrides_the_builtin(tmp_path) -> None:
352+
# The bug this guards: discover_operator_profiles() used to key profiles
353+
# by the RAW filename stem, so `profiles/Thor.toml` never matched a
354+
# resolve_profile("thor") lookup (which normalises with .strip().lower())
355+
# and silently failed to override the builtin.
356+
profiles_dir = tmp_path / "profiles"
357+
profiles_dir.mkdir()
358+
(profiles_dir / "Thor.toml").write_text(
359+
'[roles.cortex]\nmodel = "operator/mixed-case-override"\n',
360+
encoding="utf-8",
361+
)
362+
found = loader.discover_operator_profiles(tmp_path)
363+
assert set(found.keys()) == {"thor"}
364+
assert found["thor"].role("cortex").model == "operator/mixed-case-override"
365+
366+
resolved = loader.resolve_profile("thor", deploy_dir=tmp_path)
367+
assert resolved.role("cortex").model == "operator/mixed-case-override"
368+
369+
resolved_upper = loader.resolve_profile("THOR", deploy_dir=tmp_path)
370+
assert resolved_upper.role("cortex").model == "operator/mixed-case-override"
371+
372+
373+
def test_operator_profile_case_collision_raises_user_error(tmp_path) -> None:
374+
profiles_dir = tmp_path / "profiles"
375+
profiles_dir.mkdir()
376+
(profiles_dir / "Thor.toml").write_text('[roles.cortex]\nmodel = "a"\n', encoding="utf-8")
377+
(profiles_dir / "thor.toml").write_text('[roles.cortex]\nmodel = "b"\n', encoding="utf-8")
378+
with pytest.raises(ModelGearError) as exc:
379+
loader.discover_operator_profiles(tmp_path)
380+
assert exc.value.code == EXIT_USER_ERROR
381+
assert "thor" in exc.value.message.lower()
382+
383+
271384
def test_discover_operator_profiles_missing_dir_returns_empty(tmp_path) -> None:
272385
assert loader.discover_operator_profiles(tmp_path / "nonexistent") == {}
273386

0 commit comments

Comments
 (0)