Move device definitions from Tableland to the R2 template catalog and narrow VIN decodes to one trim - #314
Open
zer0stars wants to merge 61 commits into
Open
Move device definitions from Tableland to the R2 template catalog and narrow VIN decodes to one trim#314zer0stars wants to merge 61 commits into
zer0stars wants to merge 61 commits into
Conversation
Add BatteryCapacityKwh to DrivlyVINResponse, Vincario metadata map, and VehicleInfo struct so EV battery capacity is a first-class field.
Japanese chassis numbers were producing garbage on-chain device definitions
(e.g. Toyota Crown GWS214-6014148 landed as toyota_4d_2017 because the 17vin
adapter picked "4D" body-style from Model Name). Two fixes:
1. japan_17vin_api.go: robust model extraction. Try "Model Name", "Model name",
"车型" in priority order; reject body-style codes matching ^\d+[A-Za-z]{0,3}$;
fall through to first non-body-style token in "Additional Vehicle Infomation".
2. decode_vin.go: gate auto-create of on-chain DD for low-confidence sources
(japan17vin, carvxvin, autoiso, eleva). On tableland miss from these providers,
return NotFoundError so the client falls back to the manual picker instead
of minting a bad DD. High-confidence providers (Drivly, Vincario, DATGroup,
Tesla) retain existing auto-create behavior.
Locks in that CROWN/HYBRID resolves to CROWN (not 4D) against the real 17vin response that caused the toyota_4d_2017 on-chain DD.
Platform-shared Toyota chassis (ZWR90, ZRR80) return
Model Name="NOAH/VOXY" or "NOAH/VOXY/ESQUIRE". Picking the first
slash token produced "NOAH" and broke indexing — prior production
DDs were all toyota_voxy_*. The Additional Vehicle Infomation column
names the actual built trim ("VOXY 07S ..."), so use it as a hint
to pick the right candidate. When the hint doesn't match any
candidate (e.g. "CROWN/HYBRID" + "4D HTWC") fall back to first,
which still rejects body-style tokens.
Adds a data-driven test suite covering 16 real payloads from prod.
Two 17vin responses produced malformed model slugs:
- "LEXUS NX SERIES(TMMC" carried the factory code suffix into the
slug (toyota_lexus-nx-series(tmmc_2022 — invalid, parens aren't
legal slug chars).
- data.model_list[0].Model_en carries 17vin's standardized product
name ("NX 350") which is cleaner than the raw EPC attribute;
prefer it when populated.
Adds sanitizeModelName (truncate at first paren or comma, trim) and
two new test cases. Non-paren payloads unchanged.
Device definitions now live as JSON documents in R2 behind a CDN, written through the definitions-worker. This service reads docs and the catalog manifest over HTTP and sends writes to the worker with a bearer token. - New DeviceDefinitionCatalogService gateway replaces the on-chain service; same method surface, no chain, no KMS, no tableland-node. - Deleted contracts bindings, sender package, and all Ethereum settings. - DD gRPC RPCs (create/update/filtered-list) return Unimplemented; the worker and identity-api are the new write/read paths. VIN decoder, integrations, device styles, and device types gRPC stay. - decode-vin now creates missing definitions through the worker. - Typesense sync job reads the R2 manifest instead of tableland tables. - vin_repo uses the manufacturer token id from identity instead of a registry contract call.
Catalog ids can contain characters like & + ( ) " (e.g. dodge_town-&-country_2012), which must be percent-encoded in URL paths.
- catalogDoc now overrides UnmarshalJSON: the embedded tableland model's custom unmarshaler was promoted and silently dropped the manufacturer field, so every catalog read returned token id 0. Regression tests cover doc, empty-metadata, and manifest decoding. - Create/Update/Delete always return the definition id on success so callers (decode-vin NewTrxHash deref) can't hit a nil pointer when the worker URL is unset. - delete-dd CLI no longer polls Polygonscan for a transaction that is now a synchronous worker call; dead CheckTransactionStatus helper and POLYGON_SCAN_API_KEY setting/secret removed.
- Success/InternalError counters with CatalogRead / CatalogManifestRead / CatalogWrite method labels replace the retired Tableland* series. - Update() now reads its merge base through the worker (uncached) when DEFINITIONS_WORKER_URL is set, so read-modify-write can't merge a stale CDN-cached document.
- upsert-vin-decoding and device-style lookups now return NotFound instead of nil-dereferencing when a definition is missing from the catalog (the catalog returns nil, nil on 404). - Create re-gains the old already-exists guard, using a CDN-bypassing read, so a create can never silently overwrite a curated definition. - bulk-update-powertrain: inverted nil-check (pre-existing) wiped all device attributes on every processed row and panicked on absent metadata; now initializes only when metadata is actually nil.
The Typesense sync pass only ever upserted, and the prod CronJob runs it
without -create-index, so the index was never rebuilt from scratch. A
definition deleted from R2 therefore stayed searchable forever: the worker's
delete-on-write was the only path that removed it, and if that Typesense call
failed the orphan became permanent, with search returning a definition_id that
404s.
runSearchSync now collects every id it upserts and deletes anything else the
index still holds. Two guards, because this deletes from a live index:
- an empty catalog set skips the prune entirely, so a failed or empty
catalog read can never empty the index;
- a prune larger than 10% of the index (floor 100) is refused and reported,
since that is far more likely to mean a partial catalog read than a real
deletion of that size. -allow-bulk-prune overrides it.
Orphans are removed one id at a time rather than through a filter_by set:
definition ids legitimately contain & + ( ) and ", which would need escaping
inside a filter expression.
The prune pass built its keep-set from the documents it upserts, which are filtered to year >= minSearchYear. Anything older therefore looked like an orphan, so a definition that exists in the catalog but sits below the index cutoff would be deleted from the search index. Measured against prod: definitions_prod carries 1,216 documents that are not in the year >= 2007 catalog set, and 1,173 of them are real definitions from 2005 and 2006 -- acura_mdx_2005 and the like -- indexed before the cutoff existed and searchable today. Only 43 are genuinely absent from the catalog. The bulk guard would not have caught it either: the limit at that index size is 1,791, and 1,216 slides underneath. buildManufacturerDocuments now returns every id it saw alongside the documents it built, and the keep-set uses those. A definition below the cutoff is not an orphan -- it exists, it is simply not indexed going forward. Whether to drop pre-cutoff documents is a product decision, not something a cleanup pass should do silently on its first run.
…mpty sync Three ways a failure was being reported as success. An empty DEFINITIONS_WORKER_URL made every write a no-op that still answered success. workerRequest returned (false, nil) and all three callers discarded the bool, so Create returned an id, Delete logged "Deleted device definition", and a VIN decode answered 200 with a definitionId that had never been written to R2 -- traceable only through an Info log. A missing write endpoint is a misconfiguration, not a mode of operation, so it is now an error. catalogPageSize was declared twice, in package main and in package gateways. The sync loop terminated on len(page) < the main copy while pages actually came from the gateway copy. Lowering the gateway constant would have ended paging early for the 11 manufacturers holding more than 500 definitions -- BMW alone has 1618 -- leaving the rest out of the prune keep-set and deleting their search documents. Against a 16,697-document index the bulk guard sits at 1,669, so it would not have tripped. The gateway constant is now exported and used by both. identity-api answers HTTP 200 with a populated `errors` array and null data when it fails, which the client surfaces as (nil, nil). The daily cron then printed "Index Updated" having indexed nothing. Zero manufacturers is now a hard error.
…eSize panic bulk-update-powertrain read each definition through the CDN, mutated its metadata and wrote the result back. Documents are served with max-age=86400, so an edit made through the worker earlier that day was read as its pre-edit version and silently overwritten -- Update fetches a fresh base and then has it replaced by the caller's stale copy. Added GetDefinitionByIDFresh, which reads through the worker, and the command now uses it. Read-modify-write callers cannot use the cached path. decode-vin put the definition slug into NewTrxHash, surfaced as new_trx_hash / NewTransactionHash. Definitions are no longer written on-chain, so there is no transaction, and any consumer treating that field as a hash -- checking a 0x prefix, polling an explorer, storing it in a tx_hash column -- gets a slug instead. It stays empty now; the id was already returned in DefinitionId. manifest() counted only its decode failure, so a catalog outage made CatalogManifestRead drop to zero rather than spike, which no "error rate > X" alert would fire on. Every exit now counts itself. Finally, fiber's Query default applies only when a parameter is absent, so an explicit pageSize=0 or pageSize=abc reached the search handler as 0 and divided by zero computing totalPages. Pre-existing on main, but this PR is what makes admin call the endpoint. Both page and pageSize are clamped.
…eal worker host cc3bfd2 clamped the definitions search handler and missed the byte-identical one sixty lines above it, which feeds the same (found + pageSize - 1) / pageSize arithmetic. GET /device-definitions/search-r1?pageSize=0 still divided by zero; fiber's recover middleware turned it into a 500 rather than a crash, but it was the same defect the commit claimed to fix. DEFINITIONS_WORKER_URL also pointed at definitions-worker.dimo.workers.dev in both values files. That host has no A record -- wrangler disables workers.dev once custom domains are configured -- so every write would have failed on DNS.
The prune's keep-set decides what gets deleted and it was built from whatever `manifest()` happened to return. Two ways that was wrong. It came from the CDN, which serves manifest.json with max-age=300, so any definition created in the five minutes before a sync was absent from the keep-set and its search document deleted. And the per-page cache expires after a minute while a full run takes several, so pages could come from different snapshots -- a definition that shifted position between pages was returned by neither and deleted too. PinCatalogSnapshot fetches once, bypassing the CDN, and holds it for the run. The keep-set was also assembled by walking identity-api's manufacturer list, which made it only as complete as that list. One make missing meant every one of its definitions looked like an orphan: BMW alone is 1,618 documents, and the bulk guard at that index size is 1,669, so it would have deleted them all without tripping. The keep-set now comes from the manifest, which is the catalog. The walk still drives indexing, where an incomplete list only means less gets refreshed. Also: add_vin dereferenced a definition that is nil both for a missing id and for one owned by another manufacturer; an identity blip could cache an empty manufacturer map for ten minutes, turning a brief outage into ten minutes of silently skipped rows; and the prune now logs every id it removes, because documents below the index year cutoff are never re-upserted and would otherwise vanish without a record.
…l deletes PinCatalogSnapshot's justification was wrong. It claimed to bypass a CDN serving the manifest with max-age=300, but the worker generates every response for definitions.dimo.org -- no cf-cache-status, no age, and the date header advances per request across colos -- so there is no edge cache in front of it, and after the URL fix the worker and catalog hosts are the same anyway. The pin still earns its place for the other reason: the per-request cache expires after a minute while a full sync takes several, so without it pages come from different manifests and a definition that shifts between them is returned by neither and deleted as an orphan. The comments now say that instead. The prune guard was calibrated above the failure it exists to catch. Ten percent of the live index is ~1,790 deletions, and one large manufacturer missing from the keep-set is 1,618 documents for BMW alone, so the motivating disaster slid underneath it. Two percent, against routine churn of tens. An empty keep-set is now a hard error rather than a silent no-op sync, matching the treatment identity's manufacturer list already got -- and this is the input that drives deletions. The manifest's own count is checked against the definitions it carries. DeleteDocuments now reports how many it removed and logs each one as it succeeds. It deletes one at a time and stops on the first error, so logging the whole batch up front claimed ids that still existed -- in the audit trail for deletions that cannot be undone.
The guard was a percentage of the index with a floor underneath it, which meant two constants, a helper to combine them, and a regime where each silently took over from the other. Below ~5,000 documents the floor decided; above it the fraction did. Both existing bulk-prune tests sat in the floor regime, so the suite stayed green for any fraction between 0.02 and 0.99 -- which is how the last change to it shipped in a commit message and not in the code. The ratio was never doing the work. What matters is one number above routine churn and below the smallest failure worth catching: prod carries 41 orphans, and a keep-set that lost a whole manufacturer is ~1,615 documents. 500 sits between them and says so directly. The test now pins that relationship rather than an arithmetic result.
A VIN decode narrows a multi-trim template to one trim and reports how confident the match is, instead of asserting one flat record for every configuration. Adds template-shaped catalog reads, a pure trim matcher, and additive proto fields for trim, template version and match quality.
GetDefinitionByID/GetDefinitionByIDFresh become GetTemplateByID and GetTemplateByIDFresh, reading t/<id>.json instead of definitions/<id>.json. No fallback to the old path: a 404 now fails loudly rather than being served as the pre-migration flat record. Attribute values decode as map[string]any so typed values (fuel_tank_capacity_gal: 15.8) stay typed instead of getting stringified. Updates every caller of the renamed methods to consume the new Template shape directly, without shimming it back into the old model.
bulk-update-powertrain read templates (typed attributes) then wrote them back through Update(), which stringified everything and hit the legacy definitions/<id> route -- undoing the point of the template migration and mixing the new no-fallback read path with the old flat write path in one runnable subcommand. Powertrain is now a per-trim value derived during extraction anyway, so a flat bulk override no longer matches the model. Deleted the subcommand, its main.go registration, Update() (its only caller), and DeviceDefinitionUpdateInput (Update()'s only consumer). fetchDoc/fetchDocFresh/fetchDocFrom/catalogDoc and workerPutBody stay: they're still live behind GetDeviceDefinitionByID/GetDefinition (add_vin), Create (decode_vin, create_dd), and the manifest reader.
GetTemplateByID/GetTemplateByIDFresh returned one generic error for both a genuine 404 and any other catalog failure, so get_ds_by_id.go and upsert_vin_decode.go both collapsed the two into NotFoundError. A catalog outage then looked exactly like a missing vehicle, and the plausible response to that spurious 404 -- creating the definition -- would write a duplicate for a vehicle that already exists, at the worst possible moment. Add ErrTemplateNotFound, a typed sentinel returned only for a literal 404; every other failure (bad status, transport error, decode failure) stays a distinct error. Both callers now branch on errors.Is(err, ErrTemplateNotFound) rather than treating any error as not-found, with a test on each side. Also: make TestTemplateHasNoLegacyFields actually exercise the Template type instead of asserting on its own fixture, and add a no-fallback test for the Fresh/worker-backed read path, which previously only had the CDN path under direct test.
…tage decode_vin.go's main Handle() flow treated any GetTemplateByID error -- outage, timeout, 500, decode failure, not just a genuine 404 -- as "this definition doesn't exist" and fell through to Create() with a nil template. On the VIN-decode hot path, at decode volume, a catalog outage meant every decode for an already-existing vehicle attempted to write a duplicate definition. Only errors.Is(err, gateways.ErrTemplateNotFound) (or a nil error with a nil template) is now allowed to fall through to the create-if-missing path; any other error aborts the decode and returns before any writes. Checked the other two GetTemplateByID call sites in this file (hydrateResponseFromVinNumber, vinInfoFromKnown) -- neither calls Create or any other write on failure, so neither carries this risk. Added a test proving Create() is never called when the catalog fails for a non-not-found reason, and confirmed it fails against the pre-fix code before restoring the fix.
Extraction emits only manufacturerCode selectors, and decode_vin could only ever supply styleName, so no trim could match and match_quality was permanently model-only. Drivly is the one provider that carries a manufacturer code (DrivlyVINResponse.ManufacturerCode); it was being dropped during normalization to VINDecodingInfoData. Carry it through and forward it into MatchSignals so drivly-decoded VINs -- the population the templates were built from -- can actually resolve.
Create and Delete were the last writers on /definitions/<id>, which the new worker does not serve at all. Deployed as-is, every catalog miss on the decode path and every delete would have failed: Create reads first, and fetchDocFrom turned the worker's catch-all 404 into "does not exist", so the existence guard always passed and the PUT then 404'd. Create now builds a template -- one trim named "Base", matching the extraction pipeline's own fallback, because the schema requires at least one and a model-year sold in a single configuration still has a trim. The body is a dedicated type rather than coremodels.Template: marshalling that would send "version": 0, and the worker rejects a client-supplied server-owned field by name instead of stripping it. Attributes are deliberately written empty. dd.Metadata holds stringified values under source-specific names, while the contract requires typed values from the DeviceType vocabulary; translating them needs that vocabulary and the per-field precedence the extraction applies. Dumping them in untyped would write exactly the unvalidated values this migration exists to remove. A definition created here exists so a decode can resolve to it -- its attributes arrive from the extraction import or from Console. The existence check moves to GetTemplateByIDFresh, which was added for this and had no caller. It distinguishes ErrTemplateNotFound from every other failure, so a catalog 500 aborts rather than authorising a duplicate. Tests pin the route and the body from outside the process, since neither is observable from within: no /definitions/ path, no server-owned fields, no ksuid, exactly one trim, and no write at all when the template exists or the catalog is down.
definitions-worker builds and publishes the search index by alias swap, and dd-api's cron sync writes the same Typesense collection. Two writers, and dd-api's is the destructive one: pruneOrphans deletes documents from the live collection, which is precisely what the worker's search.ts warns against -- "Do not reintroduce a document-level prune against the aliased collection." The name collision recorded in the migration's verify notes is this: a real collection holds the name the worker wants for its alias. The worker's model supersedes it rather than duplicating it. It indexes per-trim documents built from templates, publishes by swapping an alias, and never deletes from what is being served -- which is what removes the orphan and prune bug class instead of managing it. Removed: the sync command and its Typesense indexer, and with them the only callers of CatalogIDs, PinCatalogSnapshot and QueryDefinitionsByManufacturer, which existed to page the manifest for exactly this job. The cronjobs go with them; leaving them would crash-loop a pod on an unknown subcommand. Querying Typesense is untouched -- only indexing moves. Cutover ordering this imposes: the worker must have built and published an index before dd-api ships without the sync, and the collection currently squatting the alias name has to be dropped so the alias can take it.
Supersedes the previous commit's decision to write attributes empty. A
definition created on a catalog miss now carries what the decode actually
learned, folded onto the DeviceType vocabulary the same way the extraction
import folds it.
internal/core/vocabulary is a port of the extraction's vocabulary.mjs. The two
must agree: both write into the same contract, and a value one folds and the
other drops produces a template whose attributes depend on which path created
it -- unobservable from either side afterwards. The port carries the rename
table, the value tables that fold 14 spellings of fuel_type and 17 of
driven_wheels, and the deliberate non-mappings ('4x2' names a driven wheel
count, not an axle; the '6L'/'5L'/'4L'/'U' epa_class codes have no identified
meaning) with their own drop reasons, so a decision never reads as a typo.
It lives in its own leaf package because internal/core/services imports
gateways, and gateways is the caller.
Parity is tested against the JavaScript, not asserted: testdata carries the
original's output over 22 cases run on the real vocabulary, with the command
that regenerates it. A diff there is the two implementations disagreeing.
Verified the guard fails when the Go side is perturbed.
Dropped values are logged rather than discarded -- report what was read and not
carried, not only what failed to parse. A vocabulary that cannot be fetched
aborts the create: writing fewer attributes because a fetch failed is
indistinguishable afterwards from the source not having carried them.
Removes the last two routes the new worker does not serve. dd-api now requests only /t/<id> and /schema/<device-type>, which clears the deploy gate this branch set out to close. GetDeviceDefinitionByID reads the template and flattens it for the callers that still take the legacy shape. Only attributes SHARED by every trim cross over: that shape has one slot per attribute, and filling it from an arbitrary trim is exactly what produced the record claiming powertrain ICE while carrying a hybrid's tank size. Absent is honest, a guess is not. KSUID stays empty rather than invented -- consumers read it as an identifier. /manifest.json is gone with GetDeviceDefinitions, and so are its only two callers: GetDeviceDefinitionByMakeModelYearQuery and GetDeviceDefinitionByDynamicFilterQuery. Both were registered in api.go and dispatched by nothing -- the same unreachable-but-wired state the branch notes flagged on BulkValidateVinCommand, and leaving them would have kept a catalog-wide scan alive to serve code no route reaches. If make/model/year lookup is wanted again it no longer needs a listing at all: the id is derivable (DeviceDefinitionSlug), so it is one template read. Tests cover the flattening rule directly, using the Camry fixture whose ICE and HEV trims disagree on tank size, plus the not-found and outage paths -- a catalog 500 must not be reclassified as "does not exist".
/device-definitions/search answered 500 for every hit. The handler read each Typesense document with direct type assertions, and the first one it reached was doc["device_definition_id"].(string): the definitions-worker owns the index now and its documents carry no such field (legacy ksuids are gone), so the assertion was on a nil interface, which panics, which fiber's recover turns into a 500. dimo-driver's MintVehicle hook and dimo-login both call this route. Every field is now read through docString/docInt, which return the zero value for a key the index does not carry. ID is definition_id and the legacy_ksuid wire field carries the same value: it stays for compatibility, and the slug id is the only id there is. The worker indexes ONE DOCUMENT PER TRIM, so a query for "camry" would also have returned toyota_camry_2020 once per trim. The search now sends group_by=definition_id with group_limit=1, and the handler takes each group's first hit, falling back to the flat hits when the result is not grouped. Facet parsing is unchanged apart from nil guards on the pointer fields. Tests: a handler test with worker-shaped grouped hits (no device_definition_id, empty image_url, two Camry trims) yields one item per definition with ID equal to legacy_ksuid; a second test with year and image_url missing altogether does not panic; a wire-level test against a fake Typesense server pins group_by and group_limit on the request.
definitions-worker accepts template ids matching
^[a-z0-9][a-z0-9._&+-]*_[a-z0-9._&+-]+_[0-9]{4}$ (src/template.ts ID_RE) and
answers 422 for anything else. dd-api built ids from SlugString output, which
keeps ! ( ) " and other punctuation, so a VIN decoding to Volkswagen "up!"
produced volkswagen_up!_2025: GetTemplateByID 404s, Create PUTs the id, the
worker answers 422, and the decode returns "error creating new device
definition" on every retry, forever. subaru_tribeca-(ny/nj)_2008 is the other
shape seen in production.
DeviceDefinitionSlug is the single place every id is built (decode_vin,
get_compatibility_r1, create_dd, add_vin, the cmd decoder), so it now reduces
both parts to the worker's character class after the existing replacements:
strip everything outside [a-z0-9._&+-], collapse dash runs, trim dangling
dashes. Each part goes through SlugString first, which is idempotent on the
already-slugged input the decode path passes and lowercases and dashes the
whitespace of the raw make and model cmd/device-definitions-api passes. An
underscore can never survive inside a part: it is the separator, and
templateFromDefinition reads the manufacturer slug back as everything before
the first one.
The table test covers up!, Tribeca (NY/NJ), "Special" Edition, Model 3 and
C/D 4.5 in both the slugged and raw shapes and asserts every output against a
copy of the worker's regex. The existing table is unchanged: no prior expected
id carried a stripped character, and nothing in the fixtures does either.
manufacturer.tokenId is optional in the template schema (minimum 1 when present), and templateFromDefinition wrote templates without one whenever its best-effort identity lookup failed: an unminted make, or a single identity blip on the live decode path. GetTemplateByID then reported the absent id as big.NewInt(0), and every consumer treated 0 as a real manufacturer. UpsertDecoding asked identity for token id 0, failed with an untyped "no manufacturer found for token id 0" and never wrote the vin_numbers row. addvin compared 0 against the caller's manufacturer, got (nil, nil) and reported "no device definition found" for a template that exists. The on-chain create this replaced hard-failed instead. GetTemplateByID and GetTemplateByIDFresh now return a nil token id when the template carries none, and each consumer handles unknown: - GetDeviceDefinitionByID compares ownership only when both sides are known, so a tokenless template still resolves by id. - UpsertDecoding resolves the name through a helper that keeps identity for a known token id and otherwise uses the template's own manufacturer name, returning a bare NotFoundError when there is neither. - GetManufacturerNameByID rejects a nil or non-positive id up front rather than panicking or round-tripping to identity. Create restores the gate: templateFromDefinition resolves the manufacturer through identity before fetching the vocabulary or writing, and fails with ErrManufacturerUnresolved when identity cannot resolve the make to a token id. The decode handler already returns Create's error, so a decode for such a make fails rather than writing a template nobody can use.
… template Create emulated create-only semantics by reading the template through the worker and, on a 404, sending an unconditional PUT. The worker's PUT is an upsert, so the check and the write were two steps with a window between them. When a Console curator saved toyota_camry_2026 with its trims while a first decode of the same model-year sat in that window, the decode's PUT wrote version 2 as a single "Base" trim, the curated trims were lost, and Create reported success. Create now drops the read and sends the PUT with If-None-Match: *, so the existence check happens at the worker, atomically with the write. A 412 maps to the new ErrTemplateExists sentinel, with nothing written. Every other refusal, a 422 for an invalid template or an outage, stays a plain error. workerRequest delegates to a header-taking variant and returns the worker's status code instead of an unused bool, so Delete is unchanged. The decode path treats ErrTemplateExists as success by someone else: createOrAdoptTemplate reads the stored template back through the worker (the CDN may still serve the 404 the decode just saw) and the handler narrows it like any template it found, instead of failing or overwriting. A create that lands returns no template, and the decode continues exactly as before. GetTemplateByIDFresh stays: its only caller was Create's pre-check, and the read-back after a 412 needs the same cache-bypassing read.
9059dcc reduced both id parts to the worker's character class and then collapsed dash runs and trimmed dangling dashes. The last two steps rewrote ids the worker accepts and the catalog already holds. Model "ID. Buzz" slugs to id--buzz, so a new decode computed volkswagen_id-buzz_2024, missed the stored volkswagen_id--buzz_2024, and created a duplicate template with a single Base trim. vin_numbers and newly minted vehicles then pointed at the duplicate while curated trims and existing vehicles stayed on the original. ford_ranger---ra_2022, bmw_x3-_2026 and several Mercedes vans hit the same path, as did create_dd, add_vin and get_compatibility_r1. DeviceDefinitionSlug now builds the id exactly as it always has and returns it untouched when it matches the worker's id pattern. Only an id the worker would refuse is repaired, by slugging each part and dropping characters outside the worker's class. There is no dash collapsing or trimming, so the raw input cmd/device-definitions-api passes and the slugged input the decode path passes still produce the same id. TestDeviceDefinitionSlugKeepsIDsTheWorkerAccepts pins the catalog ids above. TestDeviceDefinitionSlugAgainstManifest replays a manifest when DEFINITIONS_MANIFEST is set. Against the live manifest, all 17,993 ids were checked: 17,930 the worker accepts come out unchanged and 63 it refuses are repaired into valid ids. Both tests fail against the previous utils.go.
…trim The worker indexes one document per trim and each document's name carries the trim, for example "2020 Toyota Camry LE". Search groups by definition_id and returns one item per definition, but Name was read from whichever trim document ranked first in the group, so the same definition could be named for a different trim from one query to the next. Name is now built from the year, make and model the way definitions have always been named, falling back to the document's name when a part is missing. The test fixture's names now carry their trims the way the worker's do; the old handler fails against it. Facet counts stay per document, which here means per trim, since Typesense counts documents rather than groups. A comment above the facet parsing says so.
The manifest test from 61d9dff split each stored id into its own make and model parts and fed them back in. For any model part without a comma, slash or dot the legacy builder returns that id unchanged by construction, so its "17,930 unchanged" figure did not show that decode inputs keep their ids. TestDeviceDefinitionSlugAgainstManifestNames builds every definition's id from its manufacturer name and model, the inputs a decode starts from, through both call shapes: slugged first, as the decode path passes them, and raw, as cmd/device-definitions-api does. It fails when the two disagree, when an id the legacy builder produced and the worker accepts comes out different, or when a refused id is not repaired. Against the live manifest of 17,993 definitions it passes with: definitions=17993 checked=17993 raw-agrees=17993 valid-unchanged=17930 repaired=63 rebuilds-stored-id new=17887 legacy=17950 Against the utils.go from before 61d9dff it fails on volkswagen_id--buzz and the other ids whose model slug carries a dash run.
…r RPCs Device definitions are edited through the definitions-worker and the Console template editor once this ships, so dimo-admin's definition editing is retired. The three RPCs it used, CreateDeviceDefinition, UpdateDeviceDefinition and GetFilteredDeviceDefinition, only answered Unimplemented on this branch. They are removed from device_definition.proto together with the messages nothing else used: CreateDeviceDefinitionRequest and its response, UpdateDeviceDefinitionRequest with its nested DeviceStyles, FilterDeviceDefinitionRequest, GetFilteredDeviceDefinitionsResponse, FilterDeviceDefinitionsReponse, DeviceTypeAttributeRequest and ExternalID. The timestamp import only those messages needed goes too, as does ExternalIDsToGRPC, which existed to fill ExternalID. A client still calling one of these methods gets Unimplemented from the server, as it did from the stubs. dimo-admin pins v1.5.6 of this module, so its build is unchanged until it upgrades, and its definition list, create and edit calls fail as they already did. devices-api does not call them. The Go code is regenerated with protoc-gen-go v1.34.1 and protoc-gen-go-grpc v1.3.0, the versions the files were generated with, so the generated diff is the removal and the renumbering it causes.
…ice-definitions #312 was written against the Tableland-era decode, which auto-created an on-chain definition on a miss. That code is gone on this branch: a miss now creates a catalog template, create-only, through createOrAdoptTemplate. Conflict in internal/core/queries/decode_vin.go, resolved by keeping this branch's create path and putting #312's gate in front of it. A catalog miss from a low-confidence source (japan17vin, carvxvin, autoiso, eleva) answers a NotFoundError, so the client opens the manual picker, and never reaches the create. The helper's comment now says catalog template instead of on-chain definition. The japan17vin model-name extraction and its tests merged cleanly.
This was referenced Sep 14, 2026
hydrateResponseFromVinNumber builds its response from five fields -- manufacturer, year, style id, source and definition id -- and never sets Model. The fresh path in Handle sets it from vinInfo.Model, so the same VIN answered "Camry" the first time it was decoded and "" on every decode afterwards, depending only on whether a vin_numbers row existed. That is the majority of production traffic: the function's own doc comment calls this "the path most decodes take" and promises it gives the same answer a fresh decode would. vin_numbers stores the manufacturer name and the definition id but not the model, so the value comes from the template the function already reads to run the trim matcher. Going field by field over the two paths, Model was the only field the fresh path sets and this one did not: both set manufacturer, year, source, definition id, device style id, trim, template version, match quality, match candidates, match by, hardware template id and powertrain; neither sets new_trx_hash or the deprecated device_make_id.
hydrateResponseFromVinNumber treated every error from GetTemplateByID as
proof that the template does not exist: `if err != nil || tblDef == nil`
logged "vin decoded for unexistent device definition" and answered OK
with empty trim, match quality, match candidates, powertrain and
hardware template id. A catalog 502, a TLS blip or a truncated body was
therefore indistinguishable from a genuinely template-less definition,
on the path most decodes take, and emitted the undocumented fourth
match_quality ("") the response contract forbids.
Check the sentinel with errors.Is(err, gateways.ErrTemplateNotFound), as
the live path already does, and surface anything else as an error. The
gateway's own doc says callers must check it this way "so a catalog
outage is never mistaken for 'this vehicle does not exist'". Behaviour
for a genuine not-found is unchanged.
hydrateResponseFromVinNumber now returns (*DecodeVinResponse, error) and
Handle reports that error instead of returning a successful decode.
…ontext
vinInfoFromKnown is the smartcar / software-connection fallback: every
provider failed and the decode is rebuilt from the VIN's WMI plus a model
and year the caller supplied. It read the wmis rows with sqlboiler's
.All(), which -- unlike .One() -- reports no rows as an empty slice with
a nil error, then indexed element zero. A VIN whose WMI has no row is
exactly the case this fallback exists for, so it panicked the decode
handler. The error message on the err path ("unknown WMI") shows no-rows
was expected to arrive as an error; it never did.
An unknown WMI is now &exceptions.NotFoundError, the typed not-found the
rest of the decode path uses, and the wmis read error keeps its own
message instead of borrowing the not-found one.
The same function passed context.Background() to both the database query
and GetTemplateByID, so a cancelled decode kept holding a connection and
issuing catalog requests. Both now take the caller's context, which
vinInfoFromKnown accepts as its first parameter.
The WMI-to-manufacturer choice moves to makeFromWMIRows, which takes the
rows it decides between, so it is testable without Postgres.
GET /device-definitions/:id answered 500 for any id with no template. GetTemplateByID reports a genuine not-found as the wrapped ErrTemplateNotFound sentinel; both by-id handlers returned it raw; the mediator panics with it, fiber's recover hands an error value to the error handler untouched, and internal/api/common/config.go matches *exceptions.NotFoundError by type assertion, which a wrapped sentinel never satisfies. The pre-migration on-chain path returned (nil, nil) here and produced a 404, which also left the handlers' `if dd == nil` block unreachable. Both handlers now check the sentinel with errors.Is and map it to exceptions.NotFoundError, and report anything else -- a catalog outage, a timeout, a decode failure -- as exceptions.InternalError, so a 502 is never reclassified as "this vehicle does not exist". This is what get_ds_by_id.go and upsert_vin_decode.go already do. Every other caller of GetTemplateByID in the repo was checked: decode_vin.go's live path, get_ds_by_id.go, upsert_vin_decode.go and the gateway's own GetDeviceDefinitionByID all check the sentinel; vin_decoding_service.go's 0SC test-VIN shortcut reads a fixed default definition whose absence is a deployment fault, not a missing vehicle, and keeps reporting an internal error.
BulkValidateVinCommandHandler asserted the result of GetDeviceDefinitionByIDQuery to *coremodels.GetDeviceDefinitionQueryResult -- twice, unchecked. That handler answers the catalog's *coremodels.Template on this branch, so the assertion panicked on the SUCCESS path: every VIN that decoded and whose definition was in the catalog took down the request. The R2 migration is what last changed that return type, and the same line also indexed DeviceStyles[0] without checking the slice, which is empty for any definition with no styles. The row build moves to decodedVINFrom, which uses the checked form and reports an unexpected or nil result as an error rather than a panic, so a future change to the handler's contract is reported instead of crashing. The manufacturer now comes from the template's own manufacturer (tokenId is optional in the contract and absent reads as 0, which the response already allowed), and DeviceModel is the template's model rather than the first style's sub-model -- a different thing from what the field is named. Every other type assertion on a handler result was checked against the handlers this branch changed: grpc_definitions_service.go's device-style assertions match get_ds_by_id.go and get_ds_by_filter.go, which still answer coremodels.GetDeviceStyleQueryResult and a slice of it; UpsertDecodingQuery's result is discarded by its only caller; CreateDeviceDefinitionCommandResult is asserted nowhere; and the two handlers this branch deleted have no callers left to mismatch.
DeviceDefinitionSlug repairs an id the worker would refuse by dropping every character outside definitions-worker's ID_RE class, and never looks at what the drop produced. A model written entirely in a script with no character in that class -- Japanese or Cyrillic, which reach a decode through vinInfoFromKnown's KnownModel from smartcar and software connections, a source isLowConfidenceSource does not gate -- leaves the id's model segment empty: toyota_ハイエース_2020 repairs to toyota__2020 and lada_Нива_2021 to lada__2021. ID_RE requires at least one character there, so no template can ever be stored at either id. The catalog read 404s, the create 422s, and so does every retry of every VIN of that model-year, forever, while the decode reports a 500. Add common.ValidateDefinitionID and the ErrUnmintableDefinitionID sentinel it wraps, and gate the decode on it in a new definitionIDForDecode, called where Handle used to build tid inline. A decode that cannot build a usable id now stops before the read, with *exceptions.NotFoundError -- the same type the low-confidence gate returns, translated to HTTP 404 and codes.NotFound by the layers that already handle it -- so the client falls back to the manual make/model/year picker instead of retrying a write that cannot land. The other half of this finding is the worker's id-body parity check, and it is fixed in definitions-worker by normalizing both sides through the same repair before comparing. dd-api's half of that contract is to keep sending the model as decoded: repairing it to match the id would store "up" as the name of a car called "up!". Pin it here too, so neither side can move alone -- TestCreateSendsTheDecodedModelAlongsideTheRepairedID asserts the body-and-id pair the create produces for up! and Tribeca (NY/NJ).
createOrAdoptTemplate returned (nil, nil) on the path where this call's own create landed, so Handle's whole `if tblDef != nil` block was skipped for the first-ever decode of a definition. That response carried trim "", template_version 0, match_quality "", match_by [], hardware_template_id "" and the powertrain from the old pt heuristic -- and saveVinDecodeNumber then persisted the row, so the very next decode of the same VIN took hydrateResponseFromVinNumber, read the stored selector-less Base trim and answered trim "Base" with quality "exact" at version 1. Two calls a second apart gave different answers for one VIN, which is precisely what hydrateResponseFromVinNumber's own doc comment says must not happen. observeTrimMatch never fired on that path either, so TrimMatchQuality -- the counter the plan calls its riskiest assumption -- under-reported every newly created definition. definitions-worker answers a PUT with the document it stored, version stamped, so the template was already in hand and being thrown away. Create now returns *coremodels.Template instead of the id it was handed: workerRequestWithHeader returns the response body, Create decodes it, and createOrAdoptTemplate hands it back so the created path runs the same MatchTrim the adopt path runs. No extra request, no read-back. A write whose response cannot be read back as this template is (nil, nil) rather than an error -- the write did land, and failing it would have the next decode create a duplicate -- and createOrAdoptTemplate falls back to GetTemplateByIDFresh for that case so the decode still carries match data. create_dd.go keeps answering with the id, which is the value Create used to return and what TransactionID has carried since definitions stopped being written on-chain.
DEFINITIONS_CATALOG_URL and DEFINITIONS_WORKER_URL are the same hostname in both dev and prod, because definitions-worker is what serves definitions.dimo.org -- there is no separate origin to read through. GetTemplateByIDFresh therefore built a byte-identical request to the cached read, against documents served `public, max-age=86400, stale-while-revalidate=604800`, and delivered no bypass at all. The read-back after a create-only conflict was served the very 404 the decode had seen moments earlier, so the decode failed with "template %s was created by another writer but could not be read back" for a template that plainly exists, and a curator's save could be up to a day stale in the same read. Keep the one hostname and miss the cache by cache key instead: the fresh read now carries a unique `fresh` query parameter. definitions-worker routes on url.pathname alone (src/index.ts's templateMatch, verified against the worker at fb81596), so the query reaches nothing there and the request lands on exactly the handler the cached read lands on, while Cloudflare -- whose cache key is the whole URL -- cannot answer it from cache. A request header cannot do this job: the edge does not honour a client's Cache-Control on a cached response. The value is time plus an atomic counter, so two fresh reads inside one clock tick still differ; it only has to be unique, never unguessable. With no worker URL configured the parameter goes on the catalog URL rather than silently falling back to the cached read. decode_vin_create_test.go's fake deliberately used two servers, a shape the charts never produce. It keeps that fake -- it makes "which host was this" unambiguous -- and gains newOneHostFakeWorker, one host with an edge cache in front of it, which reproduced the stale-404 failure exactly before this fix. The charts now say in both values files why the two URLs are one hostname.
Handle opened a SERIALIZABLE transaction on the writer to read the cached vin_numbers row, and held it across hydrateResponseFromVinNumber. That function was pure in-memory when the transaction was placed around it. It now issues a catalog GET through a client with a 30 second timeout and a device_styles query against the reader, and returns on a cache hit -- so the path the code itself calls "the one most decodes take" held a writer connection and a serializable snapshot open across a full CDN round trip, and took a reader connection while holding it. A two second catalog slowdown pinned every writer connection for two seconds per decode; at steady volume the writer pool saturated and unrelated writes queued behind it, and a thirty second stall exhausted it outright. Read the row, end the transaction, then hydrate. The read moves into readCachedVinNumber, which opens the transaction, runs the one query, and closes it before returning -- deferred Rollback, so a failed query and a failed begin close it too, not only the path that finds a row. Nothing slow can run inside it any more because there is no inside. The isolation level is unchanged: what this row is read at is a separate decision from how long the read is held, and the transaction was never committed before either. Tested with sqlmock rather than Postgres, so this runs without Docker: each case asserts that begin, query and rollback have all been satisfied by the time the call returns.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidates the former stack #314 → #315 → #316 into one PR against
main(2026-09-14).
r2-device-definitionswas fast-forwarded toshrink-dd-api;#315 and #316 are closed with empty diffs. Commit history is still linear in
that order, so
git logwalks the three layers bottom-up.Why
Adding a manufacturer required an on-chain table + privilege grants + a service
restart (see the Ineos incident, manufacturer 147). The catalog is ~18k
near-static reference docs; object storage + search index is the right tool.
And the flat record could not describe a car:
toyota_camry_2020in productiondeclares
powertrain_type: ICEwhile carrying the hybrid's fuel economy, tanksize and OEM code — one row standing for several real cars. definitions-worker#1
emits that model-year as ten trims; this PR makes a VIN decode narrow it to one
and say how confident it was.
Layer 1 — dd-api onto R2 (
internal/infrastructure/gateways/device_definition_catalog_service.go)Removes the entire on-chain/Tableland device-definition path. Net −14k lines
and every chain dependency (contract bindings, KMS sender, tableland-node proxy,
Ethereum settings).
DeviceDefinitionCatalogServicegateway: reads over HTTP with Prometheuscounters, writes via the worker with a bearer token.
Update()reads itsmerge base through the worker (uncached) so it can never merge a stale CDN copy.
Unimplemented— dimo-adminmoves to the worker/REST in DIMO-Network/dimo-admin#315. Deploy together
with that PR, admin first — new admin works against old dd-api, old admin
breaks against this. VIN decoder, integrations, device styles and device types
gRPC are untouched.
DEFINITIONS_CATALOG_URL/DEFINITIONS_WORKER_URLenvs +DEFINITIONS_WORKER_TOKENsecret replace the Ethereum/Tableland config.Opt-in E2E (
CATALOG_E2E_URL=... go test -run TestCatalogServiceE2E) passedagainst a full local backfill of all 17,996 live Tableland rows.
Layer 2 — narrow a VIN decode to one trim (
internal/core/services/trim_match.go,decode_vin*.go)t/<id>.json— the template shape — with no fallback todefinitions/<id>.json. A missing template means the import hasn't run, andthat must fail loudly rather than silently serving the pre-migration record.
manufacturerCode,styleNameor a VINpattern. Several matches is
ambiguous, not "take the first" — it emitsonly the attributes every candidate agrees on and names the candidates.
Nothing matching is
model-only, which carries no trim's values at all.DecodeVinResponsegainstrim,template_version,match_quality,match_candidates,match_by,hardware_template_id— fields 12–17,additive only, nothing renumbered.
quality/source-labelled counter, so how often decodes land onmodel-onlyis measurable rather than assumed.Bugs caught in review, all of the silent kind: a catalog outage reported as
"vehicle not found", so
decode_vinwrote a duplicate definition on everydecode during an outage (now a typed sentinel, checked by identity); trim
matching was structurally unreachable because
buildFromDrivlydroppedmanufacturerCodeduring normalisation; and the cached path (Handlereturning early for a VIN already in
vin_numbers— most production traffic)never ran the matcher, so the same VIN resolved
exactonce and an unqualifiedguess forever after.
Layer 3 — onto the template routes (
device_definition_catalog_service.go,internal/core/vocabulary/)dd-api now requests only
/t/…and/schema/…, both of whichdefinitions-worker serves. Previously
Create()readGET /definitions/<id>,got the worker's catch-all 404, treated it as "does not exist", then
PUTto aroute that does not exist — loud rather than corrupting, but every catalog miss
on the decode path would have failed.
Create()→PUT /definitions/<id>PUT /t/<id>, a template with oneBasetrimDelete()→DELETE /definitions/<id>DELETE /t/<id>fetchDocFreshGetTemplateByIDFresh, on theErrTemplateNotFoundsentinelGetDeviceDefinitionByID→/definitions/<id>.jsonmanifest(),CatalogIDs,PinCatalogSnapshot, Typesense sync jobGetDeviceDefinitions+ its two query handlersThree judgement calls:
collection and dd-api's was the destructive one:
pruneOrphansdeleted fromthe live collection. The worker publishes by alias swap and never deletes from
what is being served.
internal/core/vocabularyportsthe extraction's
vocabulary.mjs— the rename table, the value tables folding14 spellings of
fuel_typeand 17 ofdriven_wheels, and the deliberatenon-mappings (
4x2/6x4name a driven-wheel count, not an axle; the6L/5L/4L/Uepa_class codes have no identified meaning) with their owndrop reasons so a decision never reads as a typo. Parity is tested against the
JavaScript's own output in
testdata/vocabulary_parity.json, with thecommand that regenerates it. A vocabulary that cannot be fetched aborts the
create: writing fewer attributes because a fetch failed is indistinguishable
afterwards from the source not having carried them.
one slot per attribute, and filling it from an arbitrary trim is precisely
what produced the blended Camry. Absent is honest; a guess is not.
GetDeviceDefinitionByMakeModelYearQueryandGetDeviceDefinitionByDynamicFilterQuerywere wired inapi.goand constructedby nothing. If make/model/year lookup is wanted again, the id is derivable via
DeviceDefinitionSlug, so it is one template read.Verification
go build ./...,go vet ./...clean; full suite green with-race;golangci-lint at baseline; CI green. No
/definitions/or/manifest.jsoncall sites remain anywhere in the repo.
Cutover ordering this imposes
Full order, blockers and decisions taken:
definitions-worker/docs/superpowers/2026-09-14-trim-template-cutover.md.Folded in: #311 and #312
Both PRs were merged into this branch so device-definitions-api has one open PR for this work. Their commits are preserved through merge commits ede41b8 and 5694b45.
battery_capacity_kwhtoVehicleInfo, the Vincario metadata map andDrivlyVINResponse, mirroringfuel_tank_capacity_gal. It merged cleanly. A follow-up commit gofmt-aligns the struct.Since the layer review
CreateDeviceDefinition,UpdateDeviceDefinitionandGetFilteredDeviceDefinitionRPCs are removed with the messages only they used. Admin definition editing is retired. Callers get Unimplemented, as they did from the stubs, and dimo-admin pins v1.5.6 of this module.