Shrink dd-api onto the template routes - #316
Open
zer0stars wants to merge 5 commits into
Open
Conversation
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".
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.
Stacked on #315 (
resolved-decode). Clears the deploy gate that branch recorded: dd-api now requests only/t/…and/schema/…, both of which definitions-worker serves.Why this was blocking
The worker's route table is
/t/:id,/schema/:doc, the three/adminroutes, and a 404 for everything else. dd-api still called/definitions/<id>and/manifest.json.Create()readGET /definitions/<id>, got the catch-all 404, andfetchDocFromtranslated that to(nil, nil)— "does not exist" — so the existence guard always passed; it thenPUTto a route that does not exist. Loud rather than corrupting, but every catalog miss on the decode path and every search sync would have failed.What changed
Create()→PUT /definitions/<id>PUT /t/<id>, a template with oneBasetrimDelete()→DELETE /definitions/<id>DELETE /t/<id>fetchDocFreshGetTemplateByIDFresh, on theErrTemplateNotFoundsentinelGetDeviceDefinitionByID→/definitions/<id>.jsonmanifest(),CatalogIDs,PinCatalogSnapshotGetDeviceDefinitions+ its two query handlersNet −1,597/+1,389 across 21 files.
The three judgement calls
Search indexing moves to the worker. Both sides were writing the same Typesense collection, and dd-api's was the destructive one:
pruneOrphansdeletes documents from the live collection, whichsearch.tsexplicitly warns against. The worker publishes by alias swap and never deletes from what is being served.Created templates carry real attributes.
internal/core/vocabularyports the extraction'svocabulary.mjs— the rename table, the value tables folding 14 spellings offuel_typeand 17 ofdriven_wheels, and the deliberate non-mappings (4x2names a driven wheel count, not an axle; the6L/5L/4L/Uepa_class codes have no identified meaning) with their own drop reasons so a decision never reads as a typo. The two implementations write into the same contract, so parity is tested against the JavaScript's own output on the real vocabulary rather than asserted —testdata/vocabulary_parity.json, with the command 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.The legacy flat shape carries only attributes shared by every trim. It has one slot per attribute, and filling it from an arbitrary trim is precisely what produced the record claiming powertrain
ICEwhile carrying a hybrid's tank size. Absent is honest; a guess is not.Deleting two registered handlers
GetDeviceDefinitionByMakeModelYearQueryandGetDeviceDefinitionByDynamicFilterQuerywere wired inapi.goand constructed by nothing — the same unreachable-but-registered state the branch notes flagged onBulkValidateVinCommand. Keeping them meant keeping a catalog-wide scan alive to serve code no route reaches. If make/model/year lookup is wanted again it needs no listing at all: the id is derivable viaDeviceDefinitionSlug, so it is one template read.Cutover ordering this imposes
Verification
go build ./...,go vet ./...clean; full suite passes. Route surface checked directly — no/definitions/or/manifest.jsoncall sites remain anywhere in the repo.