Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0ed7dd9fc8
|
@@ -16,6 +16,23 @@ for 1.0.0 have something to be assembled from.
|
||||
|
||||
## Unreleased
|
||||
|
||||
## 1.3.1
|
||||
|
||||
- Fixed: **updating to 1.2.0 or later broke every page that lists models**, with
|
||||
a 500 and nothing but the error page to show for it. The per-model reasoning
|
||||
effort list added in 1.2.0 was the first list-shaped setting this application
|
||||
had ever added to a table that already had rows in it, and the code that fills
|
||||
in such a column on existing rows could not tell a list from a dictionary — so
|
||||
it wrote the wrong kind of empty value into every model, and reading one back
|
||||
raised rather than returning nothing.
|
||||
|
||||
A fresh install was never affected, which is exactly why it was not caught:
|
||||
the column is only filled in that way on a database that already existed.
|
||||
|
||||
This release both stops it happening and **puts right the rows already
|
||||
written**, on start, with nothing to run by hand. If your instance is showing
|
||||
the error page, updating is the whole fix.
|
||||
|
||||
## 1.3.0
|
||||
|
||||
- **A model's reasoning efforts can now be detected rather than known.** There
|
||||
|
||||
@@ -1,3 +1,3 @@
|
||||
"""LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
|
||||
|
||||
__version__ = "1.3.0"
|
||||
__version__ = "1.3.1"
|
||||
|
||||
@@ -39,6 +39,29 @@ log = logging.getLogger(__name__)
|
||||
MANUAL_STEPS: list[str] = []
|
||||
|
||||
|
||||
def _default_shape(column: Column) -> type | None:
|
||||
"""`list` or `dict`, from the column's own Python-side default.
|
||||
|
||||
`default=list` and `default=dict` are how the two JSON flavours are
|
||||
declared, and SQLAlchemy keeps the callable. Calling it is cheap and is the
|
||||
only way to tell a MutableList column from a MutableDict one -- see the note
|
||||
in `_literal_default`.
|
||||
"""
|
||||
default = column.default
|
||||
if default is None or not getattr(default, "is_callable", False):
|
||||
return None
|
||||
try:
|
||||
# SQLAlchemy wraps a zero-argument callable to take a context.
|
||||
produced = default.arg(None)
|
||||
except Exception: # noqa: BLE001 - a default we cannot call tells us nothing
|
||||
return None
|
||||
if isinstance(produced, list):
|
||||
return list
|
||||
if isinstance(produced, dict):
|
||||
return dict
|
||||
return None
|
||||
|
||||
|
||||
def _literal_default(column: Column) -> str | None:
|
||||
"""A SQL literal to backfill an existing row's new column with.
|
||||
|
||||
@@ -63,8 +86,22 @@ def _literal_default(column: Column) -> str | None:
|
||||
if "JSON" in affinity:
|
||||
# MutableList columns must start as [] and MutableDict as {}; guessing
|
||||
# wrong makes the first read blow up rather than return empty.
|
||||
python_type = getattr(column.type, "python_type", None)
|
||||
return "'[]'" if python_type is list else "'{}'"
|
||||
#
|
||||
# 🚨 NOT `column.type.python_type`. `MutableList.as_mutable(JSON)`
|
||||
# returns the *same* JSON type object with an event listener attached --
|
||||
# it does not subclass or wrap it -- so the type cannot tell you which
|
||||
# of the two it is, and `JSON.python_type` is `dict` for both. That read
|
||||
# as "this is a dict column" for every list column, and the first one
|
||||
# ever added by a migration (`Model.reasoning_efforts`, 1.2.0) arrived
|
||||
# as `'{}'` on every existing row. `MutableList` refuses a dict, so the
|
||||
# failure was not an empty list but a ValueError on *load* -- every page
|
||||
# that lists models, 500, on an instance that had simply been updated.
|
||||
#
|
||||
# The Python-side default is the only honest signal: a JSONList column
|
||||
# is declared `default=list` and a JSONDict one `default=dict`, and
|
||||
# calling it says which. Anything that cannot be called or produces
|
||||
# neither falls back to `{}`, which is what this always assumed.
|
||||
return "'[]'" if _default_shape(column) is list else "'{}'"
|
||||
if "BOOL" in affinity:
|
||||
return "0"
|
||||
if any(token in affinity for token in ("INT", "FLOAT", "NUMERIC", "DECIMAL")):
|
||||
@@ -190,6 +227,50 @@ def ensure_fts(engine: Engine) -> list[str]:
|
||||
return created
|
||||
|
||||
|
||||
def repair_json_shapes(engine: Engine) -> list[str]:
|
||||
"""Put right any JSON column backfilled with the wrong empty value.
|
||||
|
||||
`_literal_default` used to read the shape off `column.type.python_type`,
|
||||
which is `dict` for a MutableList column as well as a MutableDict one -- so
|
||||
the first list-shaped JSON column ever added by a migration arrived as
|
||||
`'{}'` on every row that already existed. `MutableList` refuses a dict, and
|
||||
refuses it while *loading*, so the symptom was not an empty list but a
|
||||
`ValueError` and a 500 on every page that touched the table.
|
||||
|
||||
Converges, like `ensure_fts` beside it: it runs on every start, it is
|
||||
idempotent, and on a database that was never damaged it does nothing. Only
|
||||
the exact wrong value is rewritten -- `'{}'` in a column whose default
|
||||
produces a list -- because `{}` cannot be a legitimate value there, while
|
||||
anything else in that column might be somebody's data.
|
||||
"""
|
||||
fixed: list[str] = []
|
||||
inspector = inspect(engine)
|
||||
known = set(inspector.get_table_names())
|
||||
|
||||
with engine.begin() as connection:
|
||||
for table in Base.metadata.sorted_tables:
|
||||
if table.name not in known:
|
||||
continue
|
||||
for column in table.columns:
|
||||
if "JSON" not in column.type.__class__.__name__.upper():
|
||||
continue
|
||||
if _default_shape(column) is not list:
|
||||
continue
|
||||
result = connection.execute(
|
||||
text(
|
||||
f'UPDATE "{table.name}" SET "{column.name}" = \'[]\' '
|
||||
f'WHERE "{column.name}" = \'{{}}\''
|
||||
)
|
||||
)
|
||||
if result.rowcount:
|
||||
fixed.append(f"{table.name}.{column.name} ({result.rowcount} row(s))")
|
||||
log.warning(
|
||||
"repaired %s.%s on %d row(s): was '{}' in a list column",
|
||||
table.name, column.name, result.rowcount,
|
||||
)
|
||||
return fixed
|
||||
|
||||
|
||||
def sync_schema(engine: Engine) -> list[str]:
|
||||
"""Bring the database up to the declared schema. Returns what it changed."""
|
||||
import lembas.db.models # noqa: F401 (registers every table on the metadata)
|
||||
@@ -219,6 +300,14 @@ def sync_schema(engine: Engine) -> list[str]:
|
||||
changes.append(f"add column {table.name}.{column.name}")
|
||||
log.info("schema: %s", statement)
|
||||
|
||||
# Before the search indexes, and before anything can try to load a row:
|
||||
# a column left holding the wrong empty value makes the ORM raise on read.
|
||||
try:
|
||||
for repair in repair_json_shapes(engine):
|
||||
changes.append(f"repair {repair}")
|
||||
except Exception: # noqa: BLE001 - a repair that fails must not stop a start
|
||||
log.exception("could not repair JSON column shapes")
|
||||
|
||||
try:
|
||||
for index in ensure_fts(engine):
|
||||
changes.append(f"create search index {index}")
|
||||
|
||||
@@ -230,3 +230,124 @@ def test_each_new_table_is_usable_after_the_upgrade(db, table):
|
||||
|
||||
declared = {column.name for column in Base.metadata.tables[table].c}
|
||||
assert _columns(engine, table) == declared
|
||||
|
||||
|
||||
# --- A list-shaped JSON column added to a database that already had rows -----
|
||||
#
|
||||
# Reported as a 500 on a live instance the moment it updated:
|
||||
#
|
||||
# ValueError: Attribute 'reasoning_efforts' does not accept objects
|
||||
# of type <class 'dict'>
|
||||
#
|
||||
# `_literal_default` read the shape off `column.type.python_type`, and
|
||||
# `MutableList.as_mutable(JSON)` returns the *same* JSON type object with a
|
||||
# listener attached -- it does not subclass it -- so `python_type` is `dict` for
|
||||
# both flavours. Every existing row got `'{}'` in a list column, and MutableList
|
||||
# refuses a dict while *loading*, so every page that listed models raised.
|
||||
#
|
||||
# The suite never caught it because `conftest.py` builds a fresh database, where
|
||||
# the column is created from the model rather than backfilled by a migration.
|
||||
# These tests exercise the path that actually ran.
|
||||
def test_a_list_column_is_backfilled_with_a_list():
|
||||
from lembas.db.migrations import _default_shape, _literal_default
|
||||
from lembas.db.models import Model
|
||||
|
||||
columns = {c.name: c for c in Model.__table__.columns}
|
||||
assert _default_shape(columns["reasoning_efforts"]) is list
|
||||
assert _literal_default(columns["reasoning_efforts"]) == "'[]'"
|
||||
|
||||
|
||||
def test_a_dict_column_still_gets_a_dict():
|
||||
from lembas.db.migrations import _literal_default
|
||||
from lembas.db.models import Model
|
||||
|
||||
columns = {c.name: c for c in Model.__table__.columns}
|
||||
assert _literal_default(columns["capabilities_json"]) == "'{}'"
|
||||
assert _literal_default(columns["params_json"]) == "'{}'"
|
||||
|
||||
|
||||
def _seed_model(engine, **overrides):
|
||||
"""A real row, made the way the application makes one.
|
||||
|
||||
Built through the ORM rather than a hand-written INSERT: the table has
|
||||
several NOT NULL columns and a test that enumerates them is a test that
|
||||
breaks every time one is added, for reasons having nothing to do with what
|
||||
it is checking.
|
||||
"""
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from lembas.db.models import Connection, Model
|
||||
|
||||
with Session(engine) as session:
|
||||
connection = Connection(
|
||||
name="local", base_url="http://127.0.0.1:1", api_key_encrypted=""
|
||||
)
|
||||
session.add(connection)
|
||||
session.flush()
|
||||
model = Model(connection_id=connection.id, model_id="bonsai", **overrides)
|
||||
session.add(model)
|
||||
session.commit()
|
||||
return model.id
|
||||
|
||||
|
||||
def test_the_damage_already_written_is_repaired_on_start(tmp_path):
|
||||
"""The fix to `_literal_default` helps the next instance. This is the one
|
||||
that helps the instance that has already updated."""
|
||||
from sqlalchemy import create_engine, text
|
||||
|
||||
from lembas.db.migrations import repair_json_shapes, sync_schema
|
||||
|
||||
engine = create_engine(f"sqlite:///{tmp_path}/repair.db")
|
||||
sync_schema(engine)
|
||||
model_id = _seed_model(engine)
|
||||
|
||||
# Exactly what the broken backfill left behind on a row that predated the
|
||||
# column: the wrong empty value, in a column that refuses it on load.
|
||||
with engine.begin() as connection:
|
||||
connection.execute(
|
||||
text("UPDATE models SET reasoning_efforts = '{}' WHERE id = :id"),
|
||||
{"id": model_id},
|
||||
)
|
||||
|
||||
assert repair_json_shapes(engine)
|
||||
|
||||
with engine.begin() as connection:
|
||||
stored = connection.execute(
|
||||
text("SELECT reasoning_efforts FROM models WHERE id = :id"), {"id": model_id}
|
||||
).scalar()
|
||||
assert stored == "[]"
|
||||
|
||||
# And the row loads again, which is the whole point -- the failure was a
|
||||
# ValueError while reading, not a wrong value sitting harmlessly.
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from lembas.db.models import Model
|
||||
|
||||
with Session(engine) as session:
|
||||
assert session.get(Model, model_id).reasoning_efforts == []
|
||||
|
||||
# Converges: a second run finds nothing left to do.
|
||||
assert repair_json_shapes(engine) == []
|
||||
|
||||
|
||||
def test_the_repair_leaves_a_dict_column_alone(tmp_path):
|
||||
"""`{}` is a legitimate value in a MutableDict column and must survive."""
|
||||
from sqlalchemy import create_engine, text
|
||||
|
||||
from lembas.db.migrations import repair_json_shapes, sync_schema
|
||||
|
||||
engine = create_engine(f"sqlite:///{tmp_path}/keep.db")
|
||||
sync_schema(engine)
|
||||
model_id = _seed_model(engine)
|
||||
|
||||
with engine.begin() as connection:
|
||||
connection.execute(
|
||||
text("UPDATE models SET capabilities_json = '{}' WHERE id = :id"),
|
||||
{"id": model_id},
|
||||
)
|
||||
repair_json_shapes(engine)
|
||||
with engine.begin() as connection:
|
||||
stored = connection.execute(
|
||||
text("SELECT capabilities_json FROM models WHERE id = :id"), {"id": model_id}
|
||||
).scalar()
|
||||
assert stored == "{}"
|
||||
|
||||
Reference in New Issue
Block a user