Skip to content

Commit a76bb43

Browse files
mattmilleraiguill
andauthored
chore(assets): drop vestigial tags.tag_type column (Comfy-Org#14248)
tag_type was always "user" in practice — no code path ever set it to anything else (no system/seeded classification was wired up) and nothing queried it. The column, its ix_tags_tag_type index, and the TagUsage.type API field were dead weight, so they're removed. Adds alembic migration 0004 to drop the column and index. Verified: asset-seeder tests pass; migration applies cleanly on a fresh SQLite (tags retains only name; tag_type column + index dropped). Co-authored-by: guill <jacob.e.segal@gmail.com>
1 parent f350acd commit a76bb43

11 files changed

Lines changed: 66 additions & 42 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
"""
2+
Drop the vestigial tags.tag_type column.
3+
4+
tag_type was always "user" in practice — no code path ever set it to anything
5+
else (no system/seeded classification was ever wired up) and nothing queried it.
6+
The column, its index (ix_tags_tag_type), and the corresponding API field were
7+
dead weight, so they are removed.
8+
9+
Revision ID: 0004_drop_tag_type
10+
Revises: 0003_add_metadata_job_id
11+
Create Date: 2026-06-03
12+
"""
13+
14+
from alembic import op
15+
import sqlalchemy as sa
16+
17+
revision = "0004_drop_tag_type"
18+
down_revision = "0003_add_metadata_job_id"
19+
branch_labels = None
20+
depends_on = None
21+
22+
23+
def upgrade() -> None:
24+
with op.batch_alter_table("tags") as batch_op:
25+
batch_op.drop_index("ix_tags_tag_type")
26+
batch_op.drop_column("tag_type")
27+
28+
29+
def downgrade() -> None:
30+
with op.batch_alter_table("tags") as batch_op:
31+
batch_op.add_column(
32+
sa.Column(
33+
"tag_type",
34+
sa.String(length=32),
35+
nullable=False,
36+
server_default="user",
37+
)
38+
)
39+
batch_op.create_index("ix_tags_tag_type", ["tag_type"])

app/assets/api/routes.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,8 +575,8 @@ async def get_tags(request: web.Request) -> web.Response:
575575
)
576576

577577
tags = [
578-
schemas_out.TagUsage(name=name, count=count, type=tag_type)
579-
for (name, tag_type, count) in rows
578+
schemas_out.TagUsage(name=name, count=count)
579+
for (name, count) in rows
580580
]
581581
payload = schemas_out.TagsList(
582582
tags=tags, total=total, has_more=(query.offset + len(tags)) < total

app/assets/api/schemas_out.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ class AssetsList(BaseModel):
4646
class TagUsage(BaseModel):
4747
name: str
4848
count: int
49-
type: str
5049

5150

5251
class TagsList(BaseModel):

app/assets/database/models.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,6 @@ class Tag(Base):
227227
__tablename__ = "tags"
228228

229229
name: Mapped[str] = mapped_column(String(512), primary_key=True)
230-
tag_type: Mapped[str] = mapped_column(String(32), nullable=False, default="user")
231230

232231
asset_reference_links: Mapped[list[AssetReferenceTag]] = relationship(
233232
back_populates="tag",
@@ -240,7 +239,5 @@ class Tag(Base):
240239
overlaps="asset_reference_links,tag_links,tags,asset_reference",
241240
)
242241

243-
__table_args__ = (Index("ix_tags_tag_type", "tag_type"),)
244-
245242
def __repr__(self) -> str:
246243
return f"<Tag {self.name}>"

app/assets/database/queries/tags.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,11 @@ def validate_tags_exist(session: Session, tags: list[str]) -> None:
5555
raise ValueError(f"Unknown tags: {missing}")
5656

5757

58-
def ensure_tags_exist(
59-
session: Session, names: Iterable[str], tag_type: str = "user"
60-
) -> None:
58+
def ensure_tags_exist(session: Session, names: Iterable[str]) -> None:
6159
wanted = normalize_tags(list(names))
6260
if not wanted:
6361
return
64-
rows = [{"name": n, "tag_type": tag_type} for n in list(dict.fromkeys(wanted))]
62+
rows = [{"name": n} for n in list(dict.fromkeys(wanted))]
6563
ins = (
6664
sqlite.insert(Tag)
6765
.values(rows)
@@ -97,7 +95,7 @@ def set_reference_tags(
9795
to_remove = [t for t in current if t not in desired]
9896

9997
if to_add:
100-
ensure_tags_exist(session, to_add, tag_type="user")
98+
ensure_tags_exist(session, to_add)
10199
session.add_all(
102100
[
103101
AssetReferenceTag(
@@ -142,7 +140,7 @@ def add_tags_to_reference(
142140
return AddTagsResult(added=[], already_present=[], total_tags=total)
143141

144142
if create_if_missing:
145-
ensure_tags_exist(session, norm, tag_type="user")
143+
ensure_tags_exist(session, norm)
146144

147145
current = set(get_reference_tags(session, reference_id))
148146

@@ -289,7 +287,6 @@ def list_tags_with_usage(
289287
q = (
290288
select(
291289
Tag.name,
292-
Tag.tag_type,
293290
func.coalesce(counts_sq.c.cnt, 0).label("count"),
294291
)
295292
.select_from(Tag)
@@ -331,7 +328,7 @@ def list_tags_with_usage(
331328
rows = (session.execute(q.limit(limit).offset(offset))).all()
332329
total = (session.execute(total_q)).scalar_one()
333330

334-
rows_norm = [(name, ttype, int(count or 0)) for (name, ttype, count) in rows]
331+
rows_norm = [(name, int(count or 0)) for (name, count) in rows]
335332
return rows_norm, int(total or 0)
336333

337334

app/assets/scanner.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ def insert_asset_specs(specs: list[SeedAssetSpec], tag_pool: set[str]) -> int:
355355
return 0
356356
with create_session() as sess:
357357
if tag_pool:
358-
ensure_tags_exist(sess, tag_pool, tag_type="user")
358+
ensure_tags_exist(sess, tag_pool)
359359
result = batch_insert_seed_assets(sess, specs=specs, owner_id="")
360360
sess.commit()
361361
return result.inserted_refs

app/assets/services/schemas.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ class IngestResult:
5656

5757
class TagUsage(NamedTuple):
5858
name: str
59-
tag_type: str
6059
count: int
6160

6261

app/assets/services/tagging.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def list_tags(
7575
owner_id=owner_id,
7676
)
7777

78-
return [TagUsage(name, tag_type, count) for name, tag_type, count in rows], total
78+
return [TagUsage(name, count) for name, count in rows], total
7979

8080

8181
def list_tag_histogram(

tests-unit/assets_test/queries/test_tags.py

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ def _make_reference(session: Session, asset: Asset, name: str = "test", owner_id
4040

4141
class TestEnsureTagsExist:
4242
def test_creates_new_tags(self, session: Session):
43-
ensure_tags_exist(session, ["alpha", "beta"], tag_type="user")
43+
ensure_tags_exist(session, ["alpha", "beta"])
4444
session.commit()
4545

4646
tags = session.query(Tag).all()
4747
assert {t.name for t in tags} == {"alpha", "beta"}
4848

4949
def test_is_idempotent(self, session: Session):
50-
ensure_tags_exist(session, ["alpha"], tag_type="user")
51-
ensure_tags_exist(session, ["alpha"], tag_type="user")
50+
ensure_tags_exist(session, ["alpha"])
51+
ensure_tags_exist(session, ["alpha"])
5252
session.commit()
5353

5454
assert session.query(Tag).count() == 1
@@ -65,13 +65,6 @@ def test_empty_list_is_noop(self, session: Session):
6565
session.commit()
6666
assert session.query(Tag).count() == 0
6767

68-
def test_tag_type_is_set(self, session: Session):
69-
ensure_tags_exist(session, ["system-tag"], tag_type="system")
70-
session.commit()
71-
72-
tag = session.query(Tag).filter_by(name="system-tag").one()
73-
assert tag.tag_type == "system"
74-
7568

7669
class TestGetReferenceTags:
7770
def test_returns_empty_for_no_tags(self, session: Session):
@@ -193,7 +186,7 @@ class TestMissingTagFunctions:
193186
def test_add_missing_tag_for_asset_id(self, session: Session):
194187
asset = _make_asset(session, "hash1")
195188
ref = _make_reference(session, asset)
196-
ensure_tags_exist(session, ["missing"], tag_type="system")
189+
ensure_tags_exist(session, ["missing"])
197190

198191
add_missing_tag_for_asset_id(session, asset_id=asset.id)
199192
session.commit()
@@ -204,7 +197,7 @@ def test_add_missing_tag_for_asset_id(self, session: Session):
204197
def test_add_missing_tag_is_idempotent(self, session: Session):
205198
asset = _make_asset(session, "hash1")
206199
ref = _make_reference(session, asset)
207-
ensure_tags_exist(session, ["missing"], tag_type="system")
200+
ensure_tags_exist(session, ["missing"])
208201

209202
add_missing_tag_for_asset_id(session, asset_id=asset.id)
210203
add_missing_tag_for_asset_id(session, asset_id=asset.id)
@@ -216,7 +209,7 @@ def test_add_missing_tag_is_idempotent(self, session: Session):
216209
def test_remove_missing_tag_for_asset_id(self, session: Session):
217210
asset = _make_asset(session, "hash1")
218211
ref = _make_reference(session, asset)
219-
ensure_tags_exist(session, ["missing"], tag_type="system")
212+
ensure_tags_exist(session, ["missing"])
220213
add_missing_tag_for_asset_id(session, asset_id=asset.id)
221214

222215
remove_missing_tag_for_asset_id(session, asset_id=asset.id)
@@ -237,7 +230,7 @@ def test_returns_tags_with_counts(self, session: Session):
237230

238231
rows, total = list_tags_with_usage(session)
239232

240-
tag_dict = {name: count for name, _, count in rows}
233+
tag_dict = {name: count for name, count in rows}
241234
assert tag_dict["used"] == 1
242235
assert tag_dict["unused"] == 0
243236
assert total == 2
@@ -252,7 +245,7 @@ def test_exclude_zero_counts(self, session: Session):
252245

253246
rows, total = list_tags_with_usage(session, include_zero=False)
254247

255-
tag_names = {name for name, _, _ in rows}
248+
tag_names = {name for name, _ in rows}
256249
assert "used" in tag_names
257250
assert "unused" not in tag_names
258251

@@ -262,7 +255,7 @@ def test_prefix_filter(self, session: Session):
262255

263256
rows, total = list_tags_with_usage(session, prefix="alph")
264257

265-
tag_names = {name for name, _, _ in rows}
258+
tag_names = {name for name, _ in rows}
266259
assert tag_names == {"alpha", "alphabet"}
267260

268261
def test_order_by_name(self, session: Session):
@@ -271,7 +264,7 @@ def test_order_by_name(self, session: Session):
271264

272265
rows, _ = list_tags_with_usage(session, order="name_asc")
273266

274-
names = [name for name, _, _ in rows]
267+
names = [name for name, _ in rows]
275268
assert names == ["alpha", "middle", "zebra"]
276269

277270
def test_owner_visibility(self, session: Session):
@@ -287,13 +280,13 @@ def test_owner_visibility(self, session: Session):
287280

288281
# Empty owner sees only shared
289282
rows, _ = list_tags_with_usage(session, owner_id="", include_zero=False)
290-
tag_dict = {name: count for name, _, count in rows}
283+
tag_dict = {name: count for name, count in rows}
291284
assert tag_dict.get("shared-tag", 0) == 1
292285
assert tag_dict.get("owner-tag", 0) == 0
293286

294287
# User1 sees both
295288
rows, _ = list_tags_with_usage(session, owner_id="user1", include_zero=False)
296-
tag_dict = {name: count for name, _, count in rows}
289+
tag_dict = {name: count for name, count in rows}
297290
assert tag_dict.get("shared-tag", 0) == 1
298291
assert tag_dict.get("owner-tag", 0) == 1
299292

tests-unit/assets_test/services/test_tagging.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ def test_returns_tags_with_counts(self, mock_create_session, session: Session):
141141

142142
rows, total = list_tags()
143143

144-
tag_dict = {name: count for name, _, count in rows}
144+
tag_dict = {name: count for name, count in rows}
145145
assert tag_dict["used"] == 1
146146
assert tag_dict["unused"] == 0
147147
assert total == 2
@@ -155,7 +155,7 @@ def test_excludes_zero_counts(self, mock_create_session, session: Session):
155155

156156
rows, total = list_tags(include_zero=False)
157157

158-
tag_names = {name for name, _, _ in rows}
158+
tag_names = {name for name, _ in rows}
159159
assert "used" in tag_names
160160
assert "unused" not in tag_names
161161

@@ -165,7 +165,7 @@ def test_prefix_filter(self, mock_create_session, session: Session):
165165

166166
rows, _ = list_tags(prefix="alph")
167167

168-
tag_names = {name for name, _, _ in rows}
168+
tag_names = {name for name, _ in rows}
169169
assert tag_names == {"alpha", "alphabet"}
170170

171171
def test_order_by_name(self, mock_create_session, session: Session):
@@ -174,7 +174,7 @@ def test_order_by_name(self, mock_create_session, session: Session):
174174

175175
rows, _ = list_tags(order="name_asc")
176176

177-
names = [name for name, _, _ in rows]
177+
names = [name for name, _ in rows]
178178
assert names == ["alpha", "middle", "zebra"]
179179

180180
def test_pagination(self, mock_create_session, session: Session):
@@ -185,7 +185,7 @@ def test_pagination(self, mock_create_session, session: Session):
185185

186186
assert total == 5
187187
assert len(rows) == 2
188-
names = [name for name, _, _ in rows]
188+
names = [name for name, _ in rows]
189189
assert names == ["b", "c"]
190190

191191
def test_clamps_limit(self, mock_create_session, session: Session):

0 commit comments

Comments
 (0)