Repository navigation
Conversation
|
🎉 Great job! Your PR title follows the correct format. 🚀 |
calmacx
added this pull request to stack #131
September 17, 2026 09:59
Captures the parity baseline for the Express -> React Router v7 rewrite
(GAT-9016): what production TRASER actually returns today, across 6 real
ACTIVE Gateway datasets plus 20 dataset-independent calls.
Adds scripts/harvest-regression-fixtures.mjs (plain Node ESM, node: builtins
only, no package.json change) and the 104 fixtures it produced. The replay
harness that asserts against them is a later PR in the migration stack.
The sample is deliberately small — 112 files, ~900 KB. Parity drift is
per-code-path, not per-dataset, so many datasets sharing an input schema
exercise one code path many times over. Six datasets are the minimum that
still span every input-schema bucket production yields: HDRUK 3.0.0 x2,
HDRUK 4.0.0 x2, GWDM 2.0 x1 (canonical form) and one input matching no known
schema. Every behavioural class from a wider trial harvest survives at this
size: the working targets (GWDM 2.0, GWDM 2.1, SchemaOrg GoogleRecommended),
both known-failing 500 paths, and the blanket CRUK rejection.
Model and version live in the directory path, not the filename:
cases/translate/{Schema}/{Version}/[{variant}/]{pid}__{form}.json. The
filename carries only the dataset and which form of it was used.
Thirteen fixtures under cases/common/err/ cover the express-validator error
envelope specifically. The rewrite replaces express-validator with
hand-rolled shaping in app/lib/errors.server.ts, making the 4xx envelope the
highest-drift-risk surface in the migration; it previously had no baseline
coverage at all.
stratify() alternates the rarest shape bucket with the most common one.
Ordering rarest-first alone starved the dominant GWDM 2.0 shape out of a
small sample; the alternating order spans both.
Five datasets carrying named-individual contact emails are excluded on
data-protection grounds, each with its reason recorded in index.json. The
addresses remaining are role or organisational mailboxes already published
on the Gateway, plus fictional placeholders.
The script carries no explanatory comments, per house style. What a reader
needs — the two-hash scheme, the envelope rules, what the sample is and is
not, and which validator each error fixture trips — is in
tests/data/regression-fixtures/README.md.
Lands the React Router v7 build toolchain on the GAT-9016 integration branch
so that everything above it can have green CI: package.json + lockfile,
tsconfig, vite and react-router config, Dockerfiles, public assets, a minimal
app/root.tsx shell, an empty app/routes.ts, an ESLint 9 flat config, and CI
wiring.
React Router cannot build without a root module, and routes.ts hard-fails on
any entry whose file is missing, so the toolchain has to land as one floor
before any application code. The root.tsx here is a deliberate stub — Layout,
Outlet and ErrorBoundary only, no loader, no middleware, no ~/lib import. The
real one arrives with the UI shell: it dynamically imports auth.server,
retention.server and schema.server, which Vite resolves statically at build
time, so it cannot land before the background-jobs PR.
Three fixes beyond the toolchain itself:
- npm start bound port 3000, not 3001. react-router-serve defaults to 3000 and
does not read .env at runtime (dotenv is loaded by vite.config.ts, which only
runs at build/dev time). The chart routes targetPort 3001 and the old Express
service defaulted to 3001, so shipping this as-is would have deployed a
container listening on the wrong port. The start script now defaults PORT to
3001, still overridable from the environment.
- env.example on the POC branch points SCHEMA_LOCATION at HDRUK/schemata; dev
correctly uses HDRUK/schemata-2. CI copies this file, so the POC value would
have pointed schema loading at the wrong repo. Kept dev's value and carried
over the POC additions.
- The Express service called require('dotenv').config() at startup
(src/app.js:15), so .env was read by the running server. In the rewrite
dotenv is only invoked from vite.config.ts, which does not run under
react-router-serve, so a built server saw none of its configuration: with
SCHEMA_LOCATION unset the schema loader falls back to a relative path, tries
to open /available.json, and every schema-backed endpoint 500s. npm start now
uses node --env-file-if-exists, so a deployment with no .env file
(Kubernetes, where config arrives as real environment variables) is
unaffected. This does not remove the need to populate deployment.yaml, which
currently sets no environment variables at all.
CI gains Typecheck, Lint and Build steps, and runs the suite against a started
server. Its branch filters match feat/GAT-* so that every PR in the migration
stack is covered regardless of which ticket it lands under.
Ports the ten app/lib modules that make up the translation engine (layers
L0-L4) verbatim from poc/GAT-XXXX: TTL cache, dataset cache, diff, error
shaping, audit, schema/AJV, templates, translation graph, translate, and the
re-export barrel.
The set is closed under its own dependencies — every relative import resolves
to another module in the set — so it compiles and typechecks standalone with
no stubs and no forward references. Internal edges are exactly:
schema->ttlCache, templates->ttlCache, graph->templates,
translation->{schema,templates,graph}, traser->{schema,translation}.
Nothing is reachable over HTTP: no app/routes file and no routes.ts entry.
That keeps this review about engine semantics rather than HTTP contracts.
Engine exercised directly without HTTP. All four schema families load
(19 schemas, 0 failures). Single-hop HDRUK 2.1.2 -> GWDM 1.0 translates clean.
Multi-hop chains genuinely route via GWDM: HDRUK 4.0.0 -> SchemaOrg
GoogleRecommended is 3 nodes, HDRUK 2.1.2 -> CRUK 1.0.0 is 5. getPath to an
unreachable node returns a 400 object rather than throwing.
Two things the plan expected that turned out otherwise, both recorded rather
than worked around:
- The build does not exercise Vite's ssr.external handling of the CJS-only
packages. With nothing importing app/lib, build/server/index.js is byte-for
-byte the scaffolding bundle and contains no ajv or jsonata at all. That
config is first exercised by the API routes PR. The CJS interop is instead
verified here at runtime via vite-node, which does load the modules through
the SSR pipeline.
- validateMetadata mutates its input. The documented "rewrite structuredClones
before validating" divergence is not implemented anywhere on the POC branch:
the sole structuredClone is inside findMatchingSchemas, which needs it to
probe candidate schemas. The regression harness must not build its
comparison predicate around a divergence that may not exist.
The ported files keep the comments they arrived with. Nothing was added.
calmacx
force-pushed
the
feat/GAT-9591-api-routes
branch
from
September 17, 2026 10:06
3bfa760 to
c478de7
Compare
calmacx
force-pushed
the
feat/GAT-9591-api-contract-fixes
branch
from
September 17, 2026 10:06
aed7d07 to
45ea12e
Compare
calmacx
removed this pull request from stack #131
September 17, 2026 10:54
calmacx
added this pull request to stack #133
September 17, 2026 10:54
cache.server.ts and diff.server.ts have no counterpart in the Express service being migrated: src/ has no dataset cache, and nothing under src/routes reads or writes a data/ directory. They exist only to serve the admin tooling built on the POC branch. WS1 was rescoped to the Express migration, its parity proof and the docs page, so both modules move to WS4 PR01 along with the endpoints and pages that call them. After this commit nothing in app/ or tests/ imports either one, so the cut is clean rather than an untangling. diff.server.ts had exactly one consumer, benchmark.server.ts, which is itself WS4. Refs: upgrade-plans/07-strip-admin-surface-from-ws1.md
Ports the twelve JSON API resource routes into app/routes/api/ byte-identical
to poc/GAT-XXXX and registers them in app/routes.ts: /translate, /validate,
/find, /list/{schemas,templates,translations,datasets},
/get/{schema,map,form_hydration,dataset} and /openapi.json.
These are the live contracts Gateway API, gateway-web-2 and the federation
service call today. They land together because all twelve depend only on the
ported module set, and because the spec endpoint enumerates its siblings.
Also fixes /openapi.json for production. openapi.json.ts globbed
app/routes/api/*.ts at runtime, but Dockerfile.prod's final stage copies only
package.json, package-lock.json, node_modules and build/ -- app/ is not in the
image, so the glob matched nothing and the spec shipped with zero paths. Local
runs and CI could never see it because the repo is the working directory.
The swagger definition moves to app/lib/openapi-definition.json so that the
route and a new scripts/build-openapi.mjs share one source, and npm run build
now generates build/openapi.json after react-router build (which clears
build/). The generator exits non-zero rather than writing a spec with no
paths, so the failure can never be silent again. The loader reads that
artefact; outside production it falls back to globbing sources so npm run dev
still works, and in production it throws rather than serving an empty spec.
The @openapi JSDoc annotations are untouched — they are the generator's input,
not commentary. Only when and where the spec is compiled has changed.
/list/datasets and /get/dataset read the dataset file cache, which the Express service does not have: src/routes/list.js serves only templates, schemas and translations, and src/routes/get.js only schema, map and form_hydration. Both endpoints are new surface built for the admin tooling, so they move to WS4 PR01 with cache.server.ts. Neither carried an @openapi block, so the generated spec is unchanged by this commit. Refs: upgrade-plans/07-strip-admin-surface-from-ws1.md
Fixes the five divergences that replaying the production fixtures against the
ported routes turned up. Kept separate from the port itself so that PR remains
reviewable as a verbatim port and this one is reviewed as behaviour.
- GET /status was missing entirely and answered 404. Express defines it
(src/routes/index.js:13) and it is the likely liveness-probe target. Restored
as a four-line resource route that touches no schemas, templates or cache, so
it cannot fail for reasons unrelated to liveness.
- /translate lost the express-validator error envelope, returning
{message} instead of {message, errors:[{type,msg,path,location}]}. A consumer
doing errors[0].msg got a TypeError. /validate had the same defect. Both
restored, including express-validator's quirk that a missing metadata key
trips isObject() and notEmpty() separately and so produces two identical
entries, while a non-object non-empty value produces one.
- /translate stopped validating validate_input / validate_output, silently
accepting anything that was not "0". Now 1/true are true, 0/false are false,
and anything else is a 400 carrying production's exact message. Accepting
true/false is a deliberate widening over production, which took only "1"/"0";
no fixture sends them and "2" still errors identically. An empty value keeps
defaulting to true, matching express-validator's .default("1").
- /translate stopped validating that extra is an object, forwarding a string
into the JSONata binding where templates dereference extra.*.
- /get/map returned 400 where production returned 200 with
translation_map: null. A consumer branching on res.ok took the error path
instead of seeing null. Restored to 200, and since most such pairs are still
reachable by chaining templates, the response additively gains
translation_path and translation_maps. Every field production returned is
unchanged in name and type.
Fixture replay goes from 6 exact / 2 status-mismatch to 13 exact / 0.
calmacx
force-pushed
the
feat/GAT-9591-api-routes
branch
from
September 21, 2026 09:36
c478de7 to
39a726d
Compare
calmacx
force-pushed
the
feat/GAT-9591-api-contract-fixes
branch
from
September 21, 2026 09:36
45ea12e to
1b6791b
Compare
calmacx
force-pushed
the
feat/GAT-9591-api-routes
branch
from
September 21, 2026 14:12
39a726d to
6a7a699
Compare
Collaborator
Author
|
Superseded — folded into the API routes PR. The five contract fixes ( This PR only existed because #128 was already open when the divergences were found, and its rationale was the production fixture replay — which is being removed from the repo. Closing rather than rebasing. |
This branch was successfully deployed
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.
What
Fixes the five API contract divergences that replaying the production fixtures against the ported routes turned up: a missing
GET /status, a losterrors[]envelope on/translateand/validate, two dropped query/body validations on/translate, and/get/mapreturning 400 where production returned 200.Why
These are live contracts. A consumer doing
errors[0].msgwas getting aTypeError; one branching onres.okfor/get/mapwas taking the error path instead of seeingtranslation_map: null. Kept separate from the port PR so that one stays reviewable as a verbatim port and this one is reviewed as behaviour.Testing
Fixture replay against production goes from 6 exact / 2 status-mismatch to 13 exact / 0 status-mismatch.
/get/mapverified on all three branches: direct pair → map populated, path null; multi-hop → 4-node path with 3 real templates; unroutable → 200 with all three fields null. Boolean flag matrix (1/0/true/false/empty accepted,2/yes→ 400 with production's exact message) checked case by case.npm run typecheck,npm run lintandnpm run buildexit 0.Notes
errors[]envelope meant reproducing an express-validator quirk, not just the shape: a missingmetadatakey tripsisObject()andnotEmpty()separately and yields two identical entries, while a non-object non-empty value yields one.true/falseforvalidate_input/validate_outputis a deliberate widening — production took only"1"/"0". No fixture sends them and"2"still errors identically, so the baseline is unaffected, but it is a real divergence and is recorded as one./get/mapgainstranslation_pathandtranslation_maps. Both are additive; no field production returned changed name or type.