feat(database): tag taxonomy update, storage padding, and alias canonicalization#713
Conversation
…icalization Three related changes to the tag system: 1. Storage-only numeric padding: purely-numeric tag values are zero-padded to width 4 in SQLite (e.g. disc:1 → disc:0001) so ORDER BY sorts correctly. PadTagValue is applied at every DB write site; UnpadTagValue strips at every read site. Public API, NFC tokens, and ZapScript remain in natural form. 2. Net-new upstream tags from PigSaint/GameDataBase: keyboard, touchscreen, positional:4 (input); barcode namespace (addon); vibration:rumble and accelerometer (embedded); vicdual/g80/h1/model1-3/naomi and new manufacturers nichibutsu/taiyo/tecfri:ambush/tourvision (arcadeboard); gameboy:infrared and gameboy:gba (compatibility); archimedes/atari:falcon/ sega:32x/nintendo:disksystem/nintendo:gameandwatch/wonderswan (port); vr and keyword:ubikey (search); comicclassics (reboxed); pcemini/ ninjajajamaru/zeldacollection and 3dfukkoku:01/02 (rerelease); rev:f, set:f1/f2, alt:4/5/6 (range fills); ca (lang); ddrgb/fullchanger (addon controller); mobileadaptergb (link); glasses:mvd, led:powerantenna/bugsensor, pocketsakura, spectrumcommunicator (addon misc); seganet (reboxed). 3. Deprecated alias canonicalization: addon:barcodeboy rewrites to addon:barcode:barcodeboy; addon:controller:jcart to embedded:slot:jcart; addon:controller:rumble to embedded:vibration:rumble. Old NFC tokens and ZapScript files using the former names resolve transparently at query time via CanonicalizeTagAlias in resolveFilter.
📝 WalkthroughWalkthroughThe PR introduces comprehensive tag value normalization by implementing padding/unpadding functions for numeric tag segments, adding tag alias canonicalization for deprecated formats, and systematically applying these transformations across database queries, caching, and indexing operations. Canonical tag definitions are expanded with new hardware, arcade board, and input device tags. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/database/mediadb/tagfilter_test.go (1)
292-311: Assert the exact alias-argument count.
require.Greater(t, len(args), 7)will still pass ifBuildTagFilterSQLstarts emitting extra duplicated args. This test is checking a fixed three-filter layout, sorequire.Len(t, args, 12)would catch that regression.🧪 Tighten the assertion
- require.Greater(t, len(args), 7) + require.Len(t, args, 12)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/database/mediadb/tagfilter_test.go` around lines 292 - 311, The test currently uses require.Greater(t, len(args), 7) which allows extra duplicated args to slip by; update the assertion to require the exact expected count by replacing that check with require.Len(t, args, 12) (this targets the args returned from BuildTagFilterSQL), so the test will fail if BuildTagFilterSQL emits more or fewer than the expected 12 alias/argument entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pkg/database/mediadb/tagfilter_test.go`:
- Around line 292-311: The test currently uses require.Greater(t, len(args), 7)
which allows extra duplicated args to slip by; update the assertion to require
the exact expected count by replacing that check with require.Len(t, args, 12)
(this targets the args returned from BuildTagFilterSQL), so the test will fail
if BuildTagFilterSQL emits more or fewer than the expected 12 alias/argument
entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9140a644-24a8-4301-ab6a-7a4cf8c1e425
📒 Files selected for processing (14)
pkg/database/mediadb/sql_cache.gopkg/database/mediadb/sql_search.gopkg/database/mediadb/sql_tags.gopkg/database/mediadb/tag_cache.gopkg/database/mediadb/tagfilter.gopkg/database/mediadb/tagfilter_test.gopkg/database/mediascanner/indexing_pipeline.gopkg/database/tags/aliases.gopkg/database/tags/aliases_test.gopkg/database/tags/storage.gopkg/database/tags/storage_test.gopkg/database/tags/tag_mappings.gopkg/database/tags/tag_values.gopkg/database/tags/tags.go
Summary
Storage-only numeric padding: purely-numeric tag values are zero-padded to width 4 in SQLite (
disc:1→ stored asdisc:0001) soORDER BYsorts correctly.PadTagValueapplied at every DB write site;UnpadTagValuestrips at every read site. Public API, NFC tokens, and ZapScript remain in natural form — no external contract change.Net-new upstream tags from PigSaint/GameDataBase:
keyboard,touchscreen,positional:4(input);barcodenamespace withbarcodeboy/barcodereader(addon);vibration:rumble,accelerometer(embedded);vicdual,g80,h1,model1–model3/variants,naomi, and manufacturersnichibutsu,taiyo,tecfri:ambush,tourvision(arcadeboard);gameboy:infrared,gameboy:gba(compatibility);archimedes,atari:falcon,sega:32x,nintendo:disksystem,nintendo:gameandwatch,wonderswan(port);vr,keyword:ubikey(search);comicclassics(reboxed);pcemini,ninjajajamaru,zeldacollection,3dfukkoku:01/:02(rerelease);rev:f,set:f1/f2,alt:4/5/6(range fills);ca(lang);ddrgb,fullchanger(addon controller);mobileadaptergb(link);glasses:mvd,led:powerantenna/bugsensor,pocketsakura,spectrumcommunicator(addon misc);seganet(reboxed).Deprecated alias canonicalization:
addon:barcodeboy→addon:barcode:barcodeboy;addon:controller:jcart→embedded:slot:jcart;addon:controller:rumble→embedded:vibration:rumble. Old NFC tokens and ZapScript files using former names resolve transparently at query time viaCanonicalizeTagAliasinresolveFilter. MediaDB is ephemeral — no migration needed; reindex produces canonical rows.Summary by CodeRabbit
Release Notes
New Features
Improvements