From f820a3f1d7797640f1a21c04773af119561fb2e0 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sun, 9 Aug 2026 16:13:57 +0200 Subject: fix: reject non-finite prices and escape a model named DEFAULT float() accepts "nan" and "inf", so a hand-edited models.ini could feed either into the cost arithmetic and render "$nan". Rejected once in _get rather than at each downstream call site. A model whose id is literally DEFAULT wrote a [DEFAULT] section, which a stock configparser reader treats as defaults inherited by every other model. Since the file is documented for hand-editing, that reintroduces the contamination the renamed default_section exists to prevent, so the id is escaped on write and mapped back on read. Also drops the marker branch in save(), leaving mark_skipped() as the sole writer of the "marker present iff no real keys" invariant, and prunes two test blocks that could not fail. Co-Authored-By: Claude Opus 5 --- test_llamachat.py | 66 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 24 deletions(-) (limited to 'test_llamachat.py') diff --git a/test_llamachat.py b/test_llamachat.py index 8b53d55..ae59d1d 100755 --- a/test_llamachat.py +++ b/test_llamachat.py @@ -1425,35 +1425,35 @@ def test_models_store(): again.save("together:org/name:v2", models.ModelInfo(ctx_size=4096)) assert models.ModelStore(path).get("together:org/name:v2").ctx_size == 4096 - # Real ids carry slashes, dots and mixed case. Section names are - # case sensitive, unlike keys, so the id must come back verbatim. - for real in ( - "together:Qwen/Qwen2.5-72B-Instruct-Turbo", - "together:deepseek-ai/DeepSeek-V3", - "together:meta-llama/Llama-3.3-70B", - ): - again.save(real, models.ModelInfo(ctx_size=128000, vision=True)) - back = models.ModelStore(path).get(real) - assert back.ctx_size == 128000, real - assert back.vision is True, real - - # vision=False must not be written as "False" and read back as None: - # bool is an int subclass, so the write order matters. + # False must survive as False, not degrade to None: "no vision" is a + # real answer that shadows a provider default, unlike "unknown". + # Pins the lowercase wire format too, which Task 15 documents for + # hand-editing. (Collapsing the bool branch in save() would still + # pass, since _get_bool lowercases: this guards the meaning, not + # that one branch.) again.save("together:novision", models.ModelInfo(vision=False)) assert models.ModelStore(path).get("together:novision").vision is False + assert "vision = false" in path.read_text(encoding="utf-8") - # Cancelling a dialog over a model we already know must not erase it. + # Cancelling a dialog over a model we already know must not erase it, + # nor mark it skipped: the marker means "no real keys", so a section + # holding both would be a state no reader is written to expect. again.mark_skipped("together:Qwen/Qwen2.5") assert models.ModelStore(path).get("together:Qwen/Qwen2.5").ctx_size == 32768 - - # An all-blank save is still an answer: asked, learned nothing. It - # must not read back as never offered, or the dialog reopens forever. - # configparser does round-trip a keyless section, so the marker is - # belt and braces against an empty section being dropped by hand. - again.save("together:blank", models.ModelInfo()) - reread = models.ModelStore(path) - assert reread.get("together:blank") is None - assert reread.was_offered("together:blank") is True + assert models.SKIPPED not in path.read_text(encoding="utf-8").split( + "[together:Qwen/Qwen2.5]" + )[1].split("[")[0] + + # A model literally named DEFAULT must not write a [DEFAULT] section: + # a stock-configparser reader, which Task 15 invites by documenting + # this file, would read it as inherited defaults for every model. + again.save("DEFAULT", models.ModelInfo(ctx_size=2048)) + assert "[DEFAULT]" not in path.read_text(encoding="utf-8") + escaped = models.ModelStore(path) + assert escaped.get("DEFAULT").ctx_size == 2048 + assert escaped.was_offered("DEFAULT") is True + # The escape must not swallow a neighbouring id. + assert escaped.was_offered("DEFAULTS") is False # A hand-edited file must degrade, not raise: "32k" is not an int. # The [DEFAULT] value is deliberately a *valid* int, so this catches @@ -1471,9 +1471,27 @@ def test_models_store(): # configparser's real default section. assert edited.get("together:empty") is None + # nan and inf parse cleanly through float(), so they would reach the + # cost arithmetic and render as "$nan". Unknown is the honest answer. + path.write_text( + "[together:nan]\nprice_in = nan\nprice_out = inf\n\n" + "[together:neg]\nprice_in = -inf\nctx_size = 8192\n", + encoding="utf-8", + ) + weird = models.ModelStore(path) + assert weird.get("together:nan") is None + # A bad price must not take the good ctx_size down with it. + assert weird.get("together:neg").price_in is None + assert weird.get("together:neg").ctx_size == 8192 + # A file that is not ini at all reads as empty rather than raising. path.write_text("this is not an ini file\n", encoding="utf-8") assert models.ModelStore(path).was_offered("together:junk") is False + + # Nor may a binary file take the app down at startup: that raises + # UnicodeDecodeError, which is not a configparser.Error. + path.write_bytes(b"\xff\xfe\x00not utf-8 at all\x00") + assert models.ModelStore(path).was_offered("together:junk") is False print("ok models.ini storage") -- cgit v1.2.3