Wip/modeling commons/nanoid - #59
Open
omargfh wants to merge 18 commits into
Open
Conversation
findByIdOrName used a UUID-shaped regex to decide between an id lookup and a name lookup. A NanoID is an ordinary-looking string that a tag name could also be, so the heuristic misroutes any NanoID-shaped tag name to a 404. Look up by id first and fall back to name on a miss; ids are unique, so a hit is authoritative. This costs one extra query on the name path, which is acceptable.
Switch x-correlation-id and Fastify genReqId generation from randomUUID to newId. Rename validateUUIDv4 to validateRequestId and widen it to accept either a canonical UUID or a NanoID, since these headers commonly arrive as UUIDs from upstream proxies and tracing systems. This is the only place in the codebase that still accepts UUID-shaped ids, and it is deliberate upstream interop, not backward compatibility.
Hash the namespaced key with SHA-256 and map 21 digest bytes into the nanoid alphabet via byte & 63, instead of bit-forcing a UUIDv5 shape from SHA-1. Determinism and idempotency are unchanged; only the output encoding moves from UUID to nanoid.
Replace randomUUID with newId in initial-import.ts and apply-diff.ts for every generated row id. Idempotency comes from legacyId columns, not id determinism, so this is a pure generator swap. Rename NodeMigrationDeps.newUuid to newId and update all callers and specs. Rename derivedUuid to storagePathHash and add a pinned-output test: its uuid-shaped S3 key format is frozen on purpose and must stay byte-for-byte identical to avoid orphaning existing storage objects.
Swap every remaining randomUUID() entity-id generator for the shared
newId() helper (model, permission, tag, additional-file, interaction,
draft domains, plus the draft-file entries embedded in draft JSON) and
point Better Auth's generateId at the same helper.
Widen storage-key path segments from randomUUID().substring(0, 8) (32
bits of entropy) to nanoid(10), applied consistently across every
storage-key call site since none of them parse the segment back.
Replace every Type.String({ format: 'uuid' }) validator with idSchema()
across the 18 remaining DTO/schema files, drop the now-unused 'uuid'
ajv format, and fix ID_EXAMPLE to a real NanoID literal so Swagger
stops advertising a UUID-shaped example that fails its own pattern.
Tighten existing domain and storage-key specs to assert the NanoID
shape instead of the old UUID/hex regex.
filenameFromKey still sliced a fixed 37 chars off the last path segment, a leftover assumption from the 36-char UUID prefix the writer no longer emits. Against the new nanoid(10) staging-key prefix this either leaked the prefix into the filename (short keys) or truncated a genuine long filename (keys over 37 chars). Three shapes actually reach this reader: createStorageKey keys put the random segment in its own path directory, so the last segment is already the bare filename; legacy pre-migration keys and the current stagingKey both join the random segment and filename with a dash in one segment. Strip a canonical-UUID-shaped prefix for the legacy case, and a fixed STAGING_KEY_RANDOM_SEGMENT_LENGTH-char prefix only when the key path is staging-shaped, since the nanoid alphabet includes '-' and an ordinary filename could otherwise be mistaken for a random segment. The length is now a named constant shared with stagingKey instead of a magic number. Also drop the stale 'Tag UUID or case-insensitive name' description in tag.schemas.ts; the param is a NanoID now.
Replace the UUID/hex id regexes in the e2e timing collector with a matcher anchored to the 21-char nanoid alphabet, plus the legacy dashed 36-char format for older captured reports, so requests to routes like /v1/models/:id collapse correctly again instead of fragmenting the perf report. Also swap the nil UUID literal in the user profile auth feature for a nanoid-shaped placeholder so that scenario still exercises 401 instead of tripping the id-format 400.
Add the id convention to backend AGENTS.md/CLAUDE.md and record the NanoID decision plus its two deliberate UUID exceptions in apps/modeling-commons-backend/.claude/DECISIONS.md. Update the six plan docs still describing UUID ids as the convention, fixing the runtime-breaking ::uuid cast in the search spec and rewriting the discussion-archive importer's id-mapping algorithm for NanoID.
The rehearsal dump's uuid regex only matched the old 36-char shape, so it silently stopped normalising the nanoid(21) and nanoid(10) segments that storage keys now contain, making two dumps of identical data diff on every key. Extract the normalisation into lib/normalize-key.ts so it covers all three shapes that can appear in a key: bare nanoid(21)/nanoid(10) path segments, the nanoid(10)-filename fused segment from stagingKey, and the hex-hyphenated shape shared by legacy pre-migration rows and the frozen storagePathHash output. Matches are anchored to segment boundaries and exact lengths so real path components and filenames are never mistaken for an id.
…ation These names and comments claimed UUIDs that no longer exist now that row ids are NanoIDs: modelUuid, fileUuid, ownerUuid, tagUuid, userUuid and related test fixtures. Mechanical rename only, no behaviour change. storagePathHash and its pinned-output test are untouched.
samePreviewObject stripped a 36-char uuid segment to ignore the random
nonce when deciding whether a resync's preview already matches the
database. Since createModelFromNode now mints a nanoid(21) nonce while
the resync still derives the frozen storagePathHash (36 chars), the
strip only removed one of the two, so every previously-imported model
compared as changed on its first resync even when nothing changed.
Both buildPreviewFileKey and the live app's createStorageKey share the
same tail convention, .../{y}/{m}/{d}/{nonce}/{filename}, so the nonce
is reliably the second-to-last path segment regardless of what shape
it is. Compare by that position instead of by shape, and move the
function into lib/preview-key.ts so it is unit tested.
The plugins autoload had no filter, so a spec file placed beside its subject was imported as a Fastify plugin and ran describe() outside a test runner, killing the app at boot. The routes autoload already filters this way.
Fastify compiles route schemas with an ajv instance it builds itself, so the format registered on the exported ajv never reached route validation and every route carrying an idSchema failed to build. Both server builders now share one registration function.
The support glob swept in any .ts under tests/support, so a spec beside its subject ran describe() outside a test runner and aborted the run before any scenario started. Verified with --dry-run: 158 scenarios, no undefined steps.
The fused-prefix strip fired on any ten characters before a dash, so wolf-sheep-predation.nlogo normalised to <id>-predation.nlogo and two different objects could compare identical in a rehearsal dump. Bare ids now require a following slash, and the fused strip is gated on a staging path.
Both producers of the fused staging key must agree with the reader that strips it; one still had a bare literal.
omargfh
marked this pull request as ready for review
August 13, 2026 18:52
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.
No description provided.