perf: project the read paths, with the test harness the four levels need - #4563
perf: project the read paths, with the test harness the four levels need#4563TaprootFreak wants to merge 41 commits into
Conversation
The endpoint inventory records that every read path is to select the fields it returns, and that a converted endpoint counts only at 4/4 on the four levels in docs/read-path-projections.md. Neither the harness for those levels nor a single conversion existed yet; this adds both. The harness needs a real database, because a mocked repository cannot observe which columns a query asked for. It builds the schema from the entity metadata via synchronize (112 entities, 99 tables, 1,736 columns) into a schema per spec file, and generates fixtures from the same metadata so that every column carries a distinct value - an empty field in a response then proves the query failed to load something. It runs as its own Jest configuration rather than inside the main suite. That is forced: the main suite compiles transpile-only, which emits design:type as Object for imported types, and building a data source from the entity sources fails on the first enum column. jest.projection.config.js compiles with full type information, the same reason the Frick and coverage gates have their own configuration. Converted, each at 4/4: GET /user/profile 253 columns -> 41 GET /buy/:id/history 497 -> 12 GET /swap/:id/history 509 -> 12 GET /sell/:id/history 470 -> 14 The two history mappers move out of their services into their own files so the specs can drive the same mapping the endpoints use; a copy in the spec could be wrong in the same way the projection is wrong. Two notes on the levels themselves. Level 3 now establishes a baseline before it drops anything: with an already incomplete response every reduced run fails too, and "every field is required" would be reported for a projection nothing was proven about. Level 4 uses the unprojected load as the second source - it fetches every column, so what it produces is by construction what the endpoint answered before, and it catches a projection loading the wrong field rather than merely too few.
The inventory counted every string argument to `.select(...)` as the bare root
alias - "reads like a projection but is not". That is true for `.select('user')`
and false for `.select('userData.id', 'id')`, which names a column and does
narrow the query. The classification only recognised the array form, so 84 sites
that project one column at a time were counted as full loads.
Corrected in the collection step, together with two cases it also missed: a
projection whose chain carries a `leftJoinAndSelect` loads that relation whole
after all, and a query built through `ReadProjection.apply` carries its field
list in the constant rather than in the chain.
What moves:
whole rows 428 -> 417 endpoints
projected 6 -> 17
query builders naming columns 1 -> 89 of 133
What does not move is the finding. The 88 sites this uncovers are counts, maxima
and id lookups - one column at the median - not response payloads. History,
profile, invoices and exports are still served by `find`, and the endpoints that
matter are still the ones this document lists as `not yet`.
Their `Tests` column reads `0/4` rather than `n/a`: a projection without the four
levels is the state this repository warns about regardless of when it was
written.
Also adds a spec that reads the column counts out of docs/endpoints.md and
compares them against the projections themselves, so the two cannot drift.
Three endpoints, all reading whole SupportIssue rows for a handful of values:
GET /support/issue/:id/data 951 columns -> 81
GET /support/issue 450 -> 11
GET /support/issue/:id 450 -> 11
The widest of them expands four eager relations recursively and pulls in both
sides of the transaction. The message thread of the single-issue endpoint is
projected too, in its own repository method.
The search condition of GET /support/issue/:id is passed through to the query
builder via setFindOptions rather than rebuilt: it is the access check for this
endpoint family - an issue is reachable by its UID, by the UID of the quote
behind it, or by numeric id scoped to the owning account - and stating it twice
is how the two copies drift apart. The spec asserts all three branches, and that
a foreign account id resolves to nothing.
Writing the tests turned up four things:
- SUPPORT_MESSAGE_RESPONSE_FIELDS named `fileName`, which is a getter over
`fileUrl`. The ORM does not recognise it as a column, passes the expression
through unquoted, and Postgres rejects the statement with a missing
FROM-clause entry for a table that appears nowhere in the query.
- findThread mixed `.andWhere('...')` with `{ id: MoreThan(...) }`. The object
form resolves against the find-options alias, not the builder's, with the
same result.
- Level 1 accepted NaN. A DTO field computed as `a + b + c` from three columns
turns into NaN the moment one is missing - not absent, so an undefined check
waves it through, and the endpoint answers 200 with a number that is not a
number. The annual volume on this view is such a field.
- Level 3 cannot assert single fields where a value has a fallback.
`organizationName ?? firstname + surname` leaves the value filled whichever
one is dropped, so every column reports as removable - true and useless. A
candidate may now be a group of fields, and the chain is the candidate.
The query-builder alias check needed to learn the new form as well: joins
declared in a ReadProjection never appear in the chain it scans, so the chain's
own `where('joinedAlias.id = :id')` looked like a bare column reference.
The previous commit split `.select('alias')` from `.select('alias.column')` by
the dot in the argument. That still misread `.select('COUNT(*)', 'count')` and
`.select('MAX(tx.seq)')` as the bare root alias, because an aggregate has no dot
before its parenthesis.
The rule that holds, and is now the one all three collection steps use: a bare
identifier is the root alias and loads every column; anything else - a column or
an expression - names something and narrows the query.
Against the base, this moves four more endpoints:
whole rows 421 -> 417
projected 13 -> 17
so the correction from the previous commit totals 15 endpoints, not 11. The
finding is unchanged: what these sites select is one column at the median, and
they are counts, maxima and id lookups rather than response payloads.
GET /kyc/users 328 columns -> 7 GET /kyc/:id/documents 328 -> 2 The first reads an address, two status fields and a hash per user of a wallet; the second reads nothing off the row at all, only the account id its response is keyed by in the document store. Both loaded the full graph for it. Level 3 now asks whether dropping a field CHANGES the response, not whether it empties one. The weaker form passed a projection missing UserData.kycStatus: getKycWebhookStatus answers `NA` when handed nothing, which is a valid value and a wrong one - the exact failure this document is about, and emptiness cannot see it. Comparing against the response the full projection produced does. It is the same standard level 4 applies, one field at a time. That change makes the fixture's reach part of the assertion. `kycType` only alters the answer for a LOCK account; against a DFX one the value it produces and the value its absence produces are identical, so the spec runs level 3 over both. The mapper moves out of KycService into its own file, as the history mappers did, so the spec drives the same mapping the endpoint uses.
The criteria for a qualifying endpoint were stated but never counted. Applied to the inventory they select 28 of the 417 that fetch whole rows, nine of them converted so far. The step that decides the size of this work is the first one, and it has a single cause: at 343 of the 555 load sites involved, the loaded entity leaves the loading method. What fields are needed is then decided by each caller - 113 of them for UserDataService.getUserData - so a projection at the load site would be guessed rather than derived, and the union over its callers is the whole entity anyway. Also records the six endpoints excluded one level below the handler signature: their DTO has a field typed as an entity (`currency: Fiat`, `targetAsset: Asset`), so the response carries every column of it and a projection would list them all to save nothing. Narrowing those means changing the contract.
GET /custody/order 19 columns -> 14 The query joined both assets and the transaction request with leftJoinAndSelect, which loads each of them whole for two names and two amounts. Level 3 needed a second fixture here. `inputAmount ?? transactionRequest.estimatedAmount` only reaches the request when the order has no amount of its own, so against a fully populated order the two request columns look removable - correctly, for that fixture. The row that drops them is the one where they are the only source the response has. The alias check and the inventory's collection step both had to learn one more shape: a projection applied to an injected repository's builder (`PROJECTION.apply(this.orderRepo.createQueryBuilder(...))`) rather than to the repository's own. The narrower pattern matched only the latter and reported this query as loading whole rows.
GET /support/issue/list 16 columns -> 10
GET /realunit/support/list 16 -> 10
The six it drops are the five foreign keys and `information`, the free-form body of
the issue. A list page shows a row per issue and none of that column, so on a full
page it is the bulk of what the query transfers.
The query moves from the service into the repository. That is what makes the four
levels reachable at all: the filter, the search predicate and the pagination were
built inline against an injected repository, so the only way to observe them was a
chainable mock that records method calls - which cannot see what the statement
selects, and cannot tell a query that runs from one Postgres rejects. The service
keeps what it decides (role to departments, term splitting, the day-extension of a
date-only upper bound); the repository takes the query.
The service spec follows that split. Its assertions move from the recorded builder
calls to the resolved query object, which is a stronger statement of the same
thing: that support cannot widen its department set through ?department is now
asserted on the value the service produced, not on the SQL fragment it happened to
emit.
Two findings from writing the tests:
- The message branch of the search named `support_message` as a literal table.
The ORM resolves every other table through the metadata, this one against the
search path, so the statement only works where the tables happen to be on it.
It now goes through the query builder like the rest.
- The int4 guard on the id branch - a term above 2^31-1, a pasted phone number
being the realistic case, must not reach the comparison - was pinned by
asserting on the emitted SQL string. It is now asserted where it fails: the
search runs against a real database and answers instead of raising 22003.
GET /dashboard/accounting/ledger/suspense (join loads two whole rows) -> 8 fields
GET /liquidityManagement/pipeline/:id/status 112 columns -> 1
The second is the sharpest case in the inventory: the endpoint answers with one
status string, and asking for the row by id expanded the pipeline's rule and both
of its action relations eagerly, and the rule its own asset and currency.
The suspense query joined the transaction and the account with innerJoinAndSelect,
which loads each of them whole for four values and a currency. The joins stay with
the query rather than moving into the projection - ReadProjection joins left, these
are inner, and while both relations are NOT NULL today that is a property of the
schema, not something the query should depend on silently.
mapSuspenseLeg now reads the transaction id off the joined row instead of through
the entity's @RelationId. That property is filled from the foreign-key column of
the leg, which a query naming its fields does not carry - the response answered
with an undefined txId and a 200. Every other value in that mapper already comes
from leg.tx, and it is the same number. The alternative, loadRelationIdAndMap,
would restore it at the cost of a second query for a value already in the result.
The pipeline lookup returns the row rather than the status, so that a missing
pipeline stays distinguishable from one whose status is not set: the endpoint
answers 404 only for the first.
Two things the tests surfaced:
- LedgerTx carries a check constraint pinning amountChfSum to 0, the single-row
balance gate. Generated fixture values are distinct by construction and the
insert is rejected - the value has to be pinned like an enum.
- The suspense endpoint takes no parameter: it answers with every leg on a
suspense account. Rows from an earlier test therefore reach the next one, and
level 3 compared a baseline that already contained a deliberately incomplete
row. The spec truncates between tests instead of scoping its assertions, which
would have hidden exactly what level 3 is there to find.
The service specs follow the same split as the issue list: what the query selects
is asserted against a real database, and the mocked service spec asserts what the
service decides.
❌ ESLint: 1 errors, 2 warnings |
351 columns -> 55 fields
The widest read path in the inventory. A findOne on UserData expanded four
countries, a language, a currency and an organization eagerly, and every address
on the account brought its whole wallet - for a response that is mostly getters
over a few dozen columns.
Most of those getters answer a plausible value from a field that is not there
rather than failing: isDataComplete reports false, the trading limit falls back to
the configured no-KYC default, isCustody compares undefined against one role and
says no. That is why the two countries are response fields here rather than
guards: isDataComplete tests them for truthiness like any other required field, so
dropping either changes the answer and the mutation test has to reach them.
Level 3 needed five fixtures rather than one, and each of the extra four exists
because a single fixture cannot see the field at all:
- a personal account never reads the four organization address columns, and an
organization account reads both sets, so the two run with different candidates
- mapAddress reads `userData.apiKeyCT ?? user.apiKeyCT` and
`wallet.displayName ?? wallet.name`; with the left side set the right side is
unreachable and reports as removable - true, and useless
- isCustody compares against one role: on any other role, dropping the column
produces the same answer
- the dummy-address flag hides an address; dropped it reads undefined, which is
falsy, so the address reappears - visible only where the flag is set
One observation the conversion did not change. computeCapabilities derives two of
its flags from kycSteps via getStepsWith, and neither this query nor the one before
it loads that relation, so those flags do not depend on the steps. The projection
reproduces that rather than quietly altering it; whether the query should load the
steps is a separate decision.
PUT /paymentLink/:id/pos 513 columns -> 19 fields
POST /user/apiKey/CT 253 -> 2 fields and the id
Both were held back by the first pre-filter, which excludes an endpoint that
writes the entity it loaded anywhere in its call chain. The hazard that criterion
names is saving a partially loaded row back: the columns the query did not select
are undefined on the entity and would be written as null. Neither of these does
that. They write through `update(id, …)`, which sends only the columns named in
the call, so a projected read cannot blank anything.
The criterion is therefore narrower than its own rationale, and applied literally
it excludes work it was never meant to exclude. What it does have to keep
excluding is a value the write derives from what was read - and that case is real
here: the point-of-sale write merges the new access key into the existing
configuration, so a `config` the query failed to load would be a configuration
silently reset. It is part of the projection and the spec asserts it directly.
POST /support/issue/:id/message stays out, and not for the write. It hands the
message - carrying the issue and the account behind it - to the notification
service, which is the second pre-filter: what that code reads is not determinable
at the load site, and a projection there would be guessed.
Level 3 needed a fixture per branch again:
- `UserData.address` reads the account's own columns or the organization's,
never both, so each account type covers one half
- `accountType` selects between them, and dropping it reads undefined - which is
not one of the two organization types either, so on a personal account the
branch and the answer are unchanged
- `completeName` falls back from the organization name to the personal ones,
which are unreachable while the first is set
- the API key's own column is only required where a key exists: that is the
conflict check, and on an account without one, dropping it changes nothing
Two fixture notes. The two configuration markers are set on `fee` rather than on
`recipient`, because a recipient in the configuration masks the columns the
recipient block is built from and makes them look removable. And the key an
account gets is random, so level 4 gives both rows the same one and compares the
secret derived from it - which is where the creation date enters.
…ifications
whole rows 415 -> 398
projected 19 -> 36
Seventeen endpoints now read only what they return and carry tests on all four
levels. The qualifying set is empty apart from one endpoint, which the second
criterion excludes rather than the first four: POST /support/issue/:id/message
hands the message it creates - carrying the issue and the account behind it - to
the notification service, and what that code reads is not determinable here.
Two shapes were still counted as full loads, both decided by something the select
list does not show:
- the terminal call can discard it. getCount() and getExists() replace the
select with COUNT(...) and SELECT 1, so a chain ending in either materialises
no row whatever precedes it. Three load sites do this, and
GET /support/kycFileStats was listed at 99 columns because of one.
- the select argument can be a variable. `.select(bucketExpr, 'bucket')` names
an expression as surely as a literal does, and whether it narrows has to be
resolved in the enclosing method. Four sites do this; three are the two
caller-defined /gs/db endpoints, the fourth an aggregate reported as a full
load, which is what made GET /dashboard/accounting/ledger/margin look
convertible when it was already projected.
The measured filter chain is restated against both the previous state and this
one, and it now records something the earlier revision did not: the filters are
conservative and reject endpoints that can in fact be converted. An endpoint fails
the first step if ANY load site it reaches leaks, including one on a branch that
has nothing to do with the response. Nine of the seventeen conversions were found
by reading the endpoint after the filter had rejected it, so the counts are a
lower bound rather than a ceiling.
read-projection.spec.ts keys the inventory rows by version as well as by path and
verb. Without that, `/user` matched the v1 row - a different endpoint, not
converted, and one that would have compared the v2 projection against a
`whole rows` cell.
`findForStatus` returns the row rather than the status, so the enum it used to be typed against is no longer referenced.
All four are the same kind of gap: the assertion existed, but no fixture reached
the branch it was supposed to prove.
- support-issue-data declared a fixture for an account without an organization
name and a wallet without a display name, and never used it. Both level-3 rows
ran on the other side of those two fallback chains, so the chains could only be
asserted as groups - which says nothing about `firstname`, `surname` or
`wallet.name` individually. A third row runs on the personal fixture and
asserts them one at a time; level 4 covers that shape too.
- custody-order-history allowed the two amounts to be empty in the fixture where
the fallback to the transaction request is the only source they have. That
exception could equally have hidden a baseline that answered nothing at all.
The exception is gone, and the run now asserts the two fallback values against
the request before it starts dropping fields.
- GET /user (v2) loads `userData.status` as a guard: the endpoint refuses a
merged account on it, and the mapper never shows it, so every comparison in
that spec stayed green without it. A merged-account fixture pins it, the same
way the profile spec already did.
- Moving the issue list into the repository dropped two assertions that were
testing something real: that a numeric term binds the id branch, and that the
id tie-break orders rows with an equal sort key. The database tests replaced
the first with "the query does not throw" and sorted a single row. Both are
now asserted where they matter - a fixture whose text fields carry no digit at
all, so a match can only come from the id, and two rows sharing a sort key
whose order is decided by the tie-break alone.
The file was pinned at 100% while it held nothing but a constructor. The suspense query moved into it, and the spec that exercises that query needs a database - the ratchet deliberately runs without one, so the new method arrived uncovered and the gate failed on all four metrics. The answer is not a weaker pin. It is the assertion the database spec cannot make: both relations are joined as INNER joins, and the ordering is applied to the joined transaction. Both are `nullable: false`, so inner and left select the same rows today - a row-level test cannot tell them apart, and the difference would surface only once that changed. The reduced-field-list case is asserted here too, because that is the call shape the mutation test uses.
Four fields were declared as guards - loaded but never shown - and each of them does reach the response. Level 3 never drops a guard, so all four sat outside the mutation test entirely: userData.id -> `mapUser` returns it as `accountId` userData.id -> the API key is derived from it tx.id -> `mapSuspenseLeg` returns it as `txId` issueTransaction.id -> `mapTransaction` decides transaction-or-null on it All four move into their response field lists, where the mutation run covers them. What stays a guard is what genuinely never shows and never feeds a value: a primary key that only makes the ORM materialise a joined row, and the two ids the point-of-sale updates are scoped by - the latter now asserted directly, because no level would notice their absence. The API key spec had a second problem behind the first. Its key generation mixes in the current time, so two responses a millisecond apart differ; comparing whole responses across runs reported every field as required, which is true of the clock and evidence about nothing. The fixture keeps the dependency that matters, the account id, and leaves the timestamp out. The alias scan took every ReadProjection alias in a file as valid for every query in it. A repository usually declares several, and their aliases are not interchangeable - a query applying one has joined only that one's relations, so a reference to another projection's alias passed a scan that exists to catch exactly that. It is now scoped to the projection the chain applies, with three cases pinning the behaviour. The apply call sits before the chain the scanner extracts, which is why the preceding text is passed along. The message aggregate behind the list rows moves into the message repository, next to the thread query. It was private to the service, so the list specs could only declare its three response fields optional - which is not 4/4 on an endpoint whose answer it is. All four levels now run over both queries. Also: `+value` instead of `parseInt` per CONTRIBUTING, explicit return types on the spec helpers, and one double cast replaced by a typed candidate list.
The inventory stated that a column added elsewhere broke every invoice and receipt in production. That is a claim about production behaviour in a public repository and nothing here can verify it. What is checkable stays: the query selected 1,664 columns, which is exactly Postgres' limit, so it was one added column away from failing outright - whatever the column and wherever it was added. The fourth test level claimed the unprojected comparison involves no second implementation. It does: each spec restates the filter and the joins around the full load, so a spec that restates them wrongly compares two things neither of which is the endpoint. The level verifies the field set, the mapper being the same function in both runs; the query around it is what level 2 asserts against seeded rows. Both are now written that way.
Both were edited without running the repository's own format check, which the CI step then reported. No behaviour changes.
Both converted write endpoints rest on one claim: they write through `update(id, …)`, which names its columns, so a projected read cannot blank the ones it left out. The specs stated that in a comment and tested only the read. They now run the write. The row is read at the storage level before and after, so the comparison covers every column of the table rather than the ones some load happens to materialise: the named columns changed, everything else is byte-for-byte what it was, `updated` excepted because the write is what moves it. The point-of-sale path gets one more, because its write merges into what was read rather than replacing it: the projection has to carry the stored configuration verbatim, or the merge starts from an empty object and the configuration is reset to nothing but the new access key. What these do not cover is the service logic around the write - which key is generated, how the merge combines two configurations. That belongs to those services and is not what a projection can get wrong.
…wers with The conversion derived its field list from `configObj`, which assembles a recipient block out of the account's name, contact data and address. The endpoint discards that block. It reads one value out of the configuration - the access key - and answers a URL built from it and the link's uniqueId. 513 columns -> 3 fields and 4 keys, where the first conversion left 26 `accountType` is deliberately not selected, and that is load-bearing rather than incidental: the discarded recipient reads `UserData.address`, a getter that switches to the organization row for an organization account and would dereference a relation this query has no reason to join. Left unselected, the getter takes its other branch and reads columns that are simply absent, which nothing looks at. Two fixtures cover exactly that. The spec now drives `PaymentLinkService.createPosLinkAdmin` rather than a rebuilt query, so the levels compare the answer the endpoint gives. That is what exposed the width: against a synthetic configuration object, fields the endpoint never returns looked required. The custody history query moves into its repository for the same reason. The spec had rebuilt it and left out the ordering and the hundred-row cap, so level 4 could not have seen either drift.
Nine repository methods built on `getOne()` declared a non-nullable entity. TypeORM returns null there, and several callers already handle it - the declaration was the only thing claiming otherwise. Four comments named the wrong thing: a mapper that had been replaced, a service method that had been renamed, the list test where the data test belonged, and one comment repeated across three methods claiming all three specs cover each of them. Each now names the test and the caller that actually apply to it. Three more made claims the repository cannot support: that a fixture is the ordinary case in production, that pasted phone numbers are the realistic trigger for an integer overflow, and that a payout transaction always carries an output asset. What is left is what the code shows: which branch the fixture takes, that a value above int4 makes Postgres fail the statement, and that the mapper dereferences a nullable column without a guard. Also types the correlated subquery factory in the message aggregate, which reached its parameter as an implicit `any`.
The point-of-sale projection went from 26 columns to 7 in the previous commit; the inventory still carried the old figure, which is what read-projection.spec.ts is there to catch.
Three places asserted what a conversion is supposed to guarantee without executing the code that could break it. The point-of-sale preservation test read the loaded configuration and stopped there. The risk it names is a merge that starts from an empty object, and that happens inside the write - which now runs: a link carrying two non-default values gets an access key, and afterwards the stored configuration holds all three. Both values differ from the defaults deliberately, because the merge strips anything equal to them and an equal value would look lost. The API key spec pinned the key so responses stay comparable across runs, which left the real derivation unexercised - and that derivation is what reads the two projected columns. It now runs over the projected row, and a second case shows the creation date is an input rather than decoration: two accounts inserted in the same millisecond share it, so the secret is not unique per account, but changing the date changes the secret. GET /kyc/:id/documents had no fourth level at all. It shows nothing off the row except the account id the document store is keyed by, and that id is now compared against the unprojected load.
Two of the classification corrections were applied to the endpoint summary and not to the site table under it, so the same three sites read `projected` in one document and `no select at all — loads every column`, with a column count, in the other. They now carry their own kind. A chain ending in `getCount()` or `getExists()` materialises no row, so its column count is not a small number - there is none, and the table says so. The totals move with it: 113 of the 1,105 sites load less than a whole row, 992 load one.
Two write branches were still asserted through a mock, and one of them carries a risk the projection can actually cause. `updatePaymentLinksConfig` receives the projected account entity and re-reads `paymentLinksConfig` off it to merge into. A projection missing that column hands the merge an empty object and replaces the account's configuration with nothing but the new access key. The mock could never show that, so the method now runs for real, bound to a real repository - the one collaborator it uses out of the twenty-seven the service takes. `createApiKey` is called the same way: the production method against the projected read and its own write, asserting the persisted key, the persisted filter, the secret derived from the key and the creation date, and that the conflict branch writes nothing.
The variable-select rule had the same gap the counting rule did: it was applied to the endpoint summary and not to the sites under it, so `marginBuckets` still read `no select at all - loads every column` while its endpoint read `projected`. It names four expressions, one of them through a variable, and the column counter had to learn that form too. The method detection matched SQL inside template literals: `CASE WHEN ... THEN (` satisfies the signature pattern, and the inventory carried a load site attributed to a method named `THEN`. Upper-case identifiers are keywords here, not methods. Three comments overstated what the harness does. Generated values are distinct for numbers, dates and strings - a boolean has two values and an enum as many as it declares, so those repeat, which is why a spec that has to tell them apart pins them in the fixture. What every generated value is, is non-empty, and that is what the completeness level rests on. And two issues in the same state share the primary *sort* key, not the primary key.
`toBeDefined()` on the persisted API filter accepts null, and the fixture starts with null - so an endpoint that never wrote the column would have passed. It is compared against the filter code the request produces. The sell-history access check seeded one user with one route each, then asked with that user's id and that user's route id. Either predicate alone returns exactly the expected row there, so dropping the other would not have shown. A second route of the same caller makes the route id necessary, and asking for a foreign route with the caller's id makes the user id necessary.
The buy-route access check had the same shape as the sell one: a caller with one route against a stranger with another, asked with that caller's id and that caller's route id. Either predicate alone returns the expected row there, so dropping the other would not have shown. It now runs against a second route of the same caller and a foreign route asked for with the caller's id, which makes each predicate necessary on its own. The swap route had no such test at all - the one that named itself the route filter called `findBuyHistory` only, so its own two predicates were never exercised. It has its own now. And no custody fixture ever left the three joined relations empty, although all three are nullable and all three are left joins. Written as inner joins, orders without an input asset, an output asset or a transaction request would drop out of the history silently, and all four levels would have stayed green. One order with none of them now proves they do not.
|
Question about the approach rather than the code — this is the inventory's own data, so it may well have been weighed already.
On develop there are 94 Reducing those flags wouldn't replace the field lists — Was that route considered, or measured against this one? Mostly asking about ordering — whether reducing them first would change which endpoints are still worth converting. |
Decision: the test approach changes before this landsThis PR has been through six review passes. Roughly fifty findings were accepted and fixed, one was rejected with evidence. That number is itself the finding, so here is what it turned out to mean and what we are doing about it. What the findings were actually aboutSorted by kind, they fall into four groups:
The third group is the one that matters. Why that group keeps appearingThe premise of this PR is that a missing column does not crash. It reads as The four-level test definition guards that by proving completeness per field: level 3 drops each field in turn and requires the response to change. Doing that correctly needs a fixture that reaches every branch reading every field — fallback chains, account-type switches, nullable relations, enum lookups. Miss one, and the run is green while proving nothing. That is the same failure mode as the bug. A forgotten field is silently absent; a fixture that misses its branch is silently green. The safety net fails the way the thing it catches fails, which is why the pattern kept recurring however carefully each individual case was written. What we are changingMake the failure loud instead of proving completeness field by field. A projected entity is wrapped so that reading a column the query did not select throws instead of answering Concretely:
The larger point this leaves on the tableConverting endpoints one at a time treats the symptom. The measured cause is in the inventory: 95 eager relations, expanded recursively, which is why a This PR does not do that, and it does not assume it happens. It is recorded here because the endpoint-by-endpoint route is the more expensive of the two and should be chosen deliberately, not by default. Still open
|
|
Straight answer: no, it was not measured against this route. The eager flags are named in Also: I posted a comment fourteen minutes after yours raising the same point as if it were open. I had not read the thread before posting. Yours came first and with better data. Your numbers reproduceAgainst the same inventory: 158 The measurement you asked forEstimating each
The caveat matters as much as the figure: only 103 of the 398 whole-row endpoints have every site estimable this way. The other 295 reach at least one site that passes an explicit relations tree, and those are precisely the sites eager reduction helps least. So 39 → 1 is the favourable end of the range, not the expected value. Your UserData point is the decisive one
So the two routes are not alternatives for the same endpoints. Reducing flags collapses the long tail of endpoints that never asked for a graph and got one; explicit field lists are what the wide response payloads need regardless. On orderingReducing first would change which endpoints are worth converting, and would shrink the list. It would not change the ones already converted here, for the reason above. It would also reach the sites this PR's filters exclude — including everything above 1000 columns, none of which any endpoint conversion touches, because at those sites the loaded entity leaves the method and the field set belongs to the caller. On that basis reducing flags is the higher-yield work per unit of effort — 95 declarations with a mechanical correctness criterion, against 398 endpoints each needing a hand-derived field list. What it does not give is a guarantee: nothing stops the next Whether this PR should wait behind that is a call for whoever owns the sequencing. It does not assume either way. |
|
Correction to my previous reply: your 94 is right and my 95 was wrong, and the reconciliation I offered for the difference was wrong too. I had counted The file I pointed at, |
A projected query that omits a column does not fail. The column reads as undefined, getters compute with it, and the endpoint answers 200 with a wrong value. Proving completeness field by field guards against that, but the proof fails the same silent way the bug does: a fixture that never reaches the branch reading a field is green and shows nothing. So make the failure loud. Projected rows are wrapped so that reading a column the query did not select throws, naming the entity and the column. Any test that exercises the endpoint then catches an incomplete field list, and a naive one is enough. The guard is installed once for the whole projection configuration rather than per spec, so a spec written later cannot lose it silently. It found three things on its first run: - PUT /paymentLink/:id/pos assembled a recipient block through configObj - name, contact data and address of the account - and discarded it. Only the access keys are read there, so the service now reads those two configurations directly. The merge is equivalent for that field: accessKeys is not part of the default configuration, so the link configuration overrides the account one exactly when it carries the key itself. - Owner-side join columns are reported by TypeORM under the relation's property name, so reading a relation looked like reading an unselected column. - A column written before it is read back is not a projection defect, so writes are recorded. Level 3 keeps one job the guard cannot do: showing that a field in the list is actually needed. That is a cost question rather than a correctness one, and the documentation now says so.
… is free The premise of this branch is that converting endpoints one at a time treats the symptom, and that the cause is the entity deciding what a query loads. That makes reducing the eager relations look like the cheaper route. Measured, it is not, and the reason is worth writing down. 55 handlers answer with an entity rather than a response object, 35 distinct entities between them. For those the eager relations are not a loading detail - they are the answer. Their closure covers 57 of the 103 eager relations this application builds at runtime, so the majority cannot be removed as a refactor: removing one changes what an endpoint returns. All 55 carry a role guard and all but two are excluded from the Swagger schema, which makes it a decision to take rather than a wall - but a decision either way, and not one this change takes. Four declarations are read by no code and contained in no response, and those are removed: Buy.route, CustodyBalance.user, CustodyOrder.transaction, CryptoStaking.paybackDeposit. That narrows 55 load sites - the custody order paths by 98 columns each - and 47 rows of the inventory. After it nothing is left that is both unread and outside every response. eager-relations.projection.spec.ts pins the closure and the total, so a relation added to an entity in it fails the run naming the controllers whose answer it changes. Three things the measurement had to get right, each of which a cheaper method got wrong: - Where a relation is read is a type question, not a text one, and it has to follow inheritance downwards: DepositRoute.route is a plain property that Sell and Swap override, so a read through the base type is a read of the child's eager relation. Without that step the measurement called Sell.route and Swap.route dead, and both are live. - What a query joins comes from the entity metadata: DepositRoute carries the eager relations of every child entity, so a query on the parent joins what Sell, Swap and Staking declare. - Which entities leave through a controller is read from the controllers rather than from a list. A list goes stale, and the first version of this - which only looked at entities already carrying an eager relation - missed 27 of the 55 handlers. The column counts in the inventory are re-measured against the entity metadata rather than adjusted, and the aggregates in both documents are recomputed from their own tables.
The eager reduction, measured — and a correctionEarlier in this thread I argued that reducing the eager relations is the higher-yield route, because What decides it55 handlers in this repository answer with an entity rather than a response object — All 55 carry a role guard and all but two are excluded from the Swagger schema, so the consumer is How it was measuredThree different questions, three different tools, because the cheap tool is wrong for two of them:
One more correction while I am at it: the first version of this measurement looked for handlers What this PR does with itRemoves the four declarations that no code reads and no response contains — 32 declarations remain that are read somewhere and are in no response. Those are removable, but not What is now guarded
So, on the ordering questionReducing first would not have shortened this PR: the endpoints converted here still need their |
`GET /custody/account/:id/order` reaches the same projected query as `GET /custody/order` - both go through `CustodyOrderService.getOrdersByUserData` - and the inventory nevertheless lists it as loading whole rows at 253 columns. That is correct rather than a gap: the width comes from the access check the endpoint runs before the history, not from the history itself. The comment on the repository method now says so, and it names the method that exists; it named one that does not. The service also carried a `fields` parameter for the mutation test, which the test does not use - it drives the repository directly - and which no caller passes. Removed, along with the import it needed.
|
Closing the point I left open above.
The same service carried a CI is green on |
Review findings, and what they change. The guard was narrower than it reads. Three gaps, each of which let the silent failure through: - A `@RelationId` lives in `relationIds`, not in `columns`, so reading one off a projected row answered `undefined` without a word. No field list can select it, which makes it the one shape where the fix is never "add it to the projection" — it now throws saying so. That is the defect this branch already fixed once, in the suspense mapper. - An embedded column is addressed by its full path. The guard split the field list at the first segment, so selecting `alias.address.city` marked the whole embedded as selected. Paths are kept whole now, and embedded objects are wrapped in turn. - Only `getOne` and `getMany` were wrapped. `getRawAndEntities` is where those two and `getOneOrFail` hydrate, so the hook moves there and covers all three; `getManyAndCount` runs its own query and gets its own. Each is covered by a test that fails without the fix. The point-of-sale configuration moves into the entity as `accessConfig`, where the rest of that merge already lives, and both sides are read lazily: a scoped call must not start failing on the JSON column it does not use. The previous version read both before branching. Test strength, where an assertion could not fail: - The suspense level 4 compared only the rows, leaving the total it derives unchecked, and took its own timestamp while the code under test took a later one — a comparison that would break at a day boundary rather than on a defect. - The sort test listed one row, which satisfies it whichever column the query orders by. It now seeds two rows differing in every sortable column and asserts both directions. - The search test assumed it was alone in the database; it scopes itself now. - The column-preservation test issued its own update, proving the update is safe rather than that the endpoint uses it. It drives the production method. - The merge of the two point-of-sale configurations had no case for the link setting the keys to null, which is what decides the direction of the merge. Documentation and comments: - Two figures contradicted the tables they stand on: 38 endpoints against 36, and 24 deprecated handlers split into 19 plus 3. - "one added column away from failing outright, whatever the column and wherever it is added" is not true — a column of a table the query does not touch changes nothing. Removed at both places. - "Any test that exercises the endpoint catches an incomplete field list" promised more than the guard delivers: a test still has to reach the read. - Working history and column counts measured elsewhere come out of the comments. The widths are recorded in the inventory, which is where a reader can check them. Imports this branch adds are absolute, per CONTRIBUTING. Two reported points are not defects and stay as they are. The generic parameter of `ReadProjection` is unbounded on purpose — `tsc --noEmit` passes, and the bound suggested would break the `ReadProjection<unknown>` the documentation guard uses. And alphabetically sorted imports are not the convention this repository actually follows: of 149 sampled files with three or more imports, 9 are sorted and 140 are not, and no lint rule asks for it.
Three gaps in the guard, all of which let it answer wrongly: - The embedded proxy had no `set` trap while the root one did, and a fresh proxy is handed out on every access. A column the caller assigned inside an embedded object therefore threw when it was read back, although the same write on the root is explicitly allowed. Both now share one set of written paths. - The `@RelationId` rule threw on the declaration rather than on the value. It is guarded on the value now: the failure is for the silent `undefined`, and a value that did arrive is not a defect whatever filled it. - `seedEntity` accepted a pinned embedded path, suppressed the generated value for it and then assigned it as a flat key, leaving the embedded field unset. Pinned values go through the same path assignment as generated ones. The controller scan missed handlers. It matched the first identifier after the colon, so `Promise<Entity | null>` and every wrapper form fell through — the expensive direction, because a handler it misses is one whose answer the closure then fails to cover. It reads the whole return type now and takes every entity name in it: 36 entities rather than 34, and nothing the compiler finds is missed. `createProjectionDataSource` leaked its connection when the schema could not be prepared. The caller never receives the instance in that case, so nothing could close it afterwards. Assertions that could not fail: - The sort test's expected order matched the insertion order, so a query falling back to the id tie-break passed it; the clerk column was pinned to one value by the scope it used. The rows now sort in reverse of their insertion order and the scope is by account. - The API key determinism check compared an expression with itself. It compares against an independently loaded row. - The column-preservation test drove its own update rather than the production method. Comments: what remained of the earlier state — "the query joined", "reached it through", "no test reached them until now" — is gone, along with two universal claims about mocks and about generated values being unique, which the truncation to a declared length does not hold for. One error message was lower-cased. The point-of-sale configuration is read through `PaymentLink.accessConfig` and no longer names an unnamed path in its comment. Rejected, with the reason: `scoped == null` stays. The replaced code used the same form, `=== undefined` would treat an explicit null differently, and 370 places in this repository use it against 86 of the strict form. And the two `leftJoin`s that a `WHERE` makes null-rejecting stay left joins — Postgres plans those as inner joins, so the suggested change buys nothing and costs a second join form.
Nine block comments restated `docs/read-path-projections.md` next to the code: why the guard exists, why level 3 compares responses instead of emptiness, why the field list is a value. That belongs in one place, and repeating it is how the two drift apart. Each keeps the part a reader of this file needs and points at the document for the rest — 68 lines shorter, no assertion removed. Measured rather than assumed, because the first plan was to cut much further: of the comments this branch adds, the ones on the field lists say why a given column is in them, and the ones in the specs say which branch a fixture reaches. Neither is recoverable from the code, so the remaining volume stays.
Measured: what the projection is worth under production conditionsThe branch argues the change on column counts. Here is what it does to time, measured against a Setup. ~900k rows (329k
|
| rows per call | today | projected | ||
|---|---|---|---|---|
| 5 (production median) | throughput | 436 /s | 5 563 /s | 12.8× |
| p50 | 9.21 ms | 0.52 ms | 17.7× | |
| 24 (p95) | throughput | 195 /s | 3 916 /s | 20.1× |
| p50 | 15.47 ms | 0.79 ms | 19.7× | |
| 376 (max observed) | throughput | 16.3 /s | 555 /s | 34.0× |
| p50 | 172.33 ms | 5.78 ms | 29.8× |
Row counts per call come from the production distribution: median 5, p95 24, max 376.
The time is spent in Node, not in Postgres
From EXPLAIN (ANALYZE) against the measured wall clock, 376-row history:
| wall | of that, database | rest | |
|---|---|---|---|
| today | 56.4 ms | 4.83 ms | 51.6 ms (91%) |
| projected | 2.4 ms | 0.53 ms | 1.9 ms |
Hydrating 376 × 497 columns into objects is the cost, not the query. That matches what the
connection pool shows in production — it has never had a waiting request — and it is why the
saving lands on the event loop rather than on the database.
The other route, measured on the same harness
A fourth variant joins only the relations an endpoint still needs, with the eager cascade left out
— what dropping eager: true would buy:
| eager dropped | projected | projection's additional gain | |
|---|---|---|---|
| buy-history, 376 rows | 2.8× | 13.9× | 5.0× |
| buy-history, 5 rows | 4.0× | 5.9× | 1.5× |
| pipeline status | 3.8× | 4.0× | 1.1× |
| user profile | 2.6× | 2.9× | 1.1× |
(idle, small database — these compare the two routes against each other, not against production)
So on the narrow endpoints the two routes are worth nearly the same, and only the row-heavy paths
justify a field list on top. That is consistent with what this branch already concluded from the
cost side: of 103 eager relations at runtime, 57 are part of a response and 4 were actually free.
Neither route is cheap, and they are not substitutes.
Limits
- The load generator keeps N calls in flight instead of sending at a fixed rate, so the loop is
saturated by construction (99–100% at N=4) and the utilisation thresholds were never hit. The
throughput figures answer the same question as a single value rather than a curve. - One endpoint in three volumes, not all seventeen.
- Query plus ORM hydration only; no DTO mapping, no HTTP stack.
- The eager-route numbers are from the idle small-scale run, so they understate both routes the way
the first measurements understated the projection.
The suspense query joined its transaction and its account on the query builder while the projection declared neither. The guard derives what it watches from the declared joins, so both relations were selected but unwatched: reading a column they did not select answered undefined instead of throwing, which is the defect the guard exists to make loud. Joins now carry an optional 'inner' kind, so they can move into the projection without turning into left joins. The constructor refuses a field list naming an alias it does not join, which makes the previous arrangement impossible rather than merely corrected, and a spec asserts that over every projection the inventory documents. The sort assertion in the same spec expected the insertion order, so it held whether or not the statement ordered at all. It now seeds the newest row first, and the field count in the comment above the projection is gone rather than restated: the field list is directly above it.
The document referred to "the outage described above", and there is none; the detection-time claim beside it was about services in general rather than about this code. The example itself is real and now names the entity it comes from instead of asserting it was fixed once before. Two comments enumerated a relation graph and a field count that the entities and the field list have since outgrown. Counting is what makes them brittle, so they no longer count. One comment stated that a column is the bulk of what a query transfers, which is a statement about production value sizes. The repository establishes only that it is an unbounded text column the list does not show, so that is what it now says. In load-sites.md the stated median was 101 where the 790 measured rows give 98, and the table was not fully sorted: rows patched in place kept their old position. Both are corrected, and the rows are the same rows in a different order.
A projection declaring ['leg.tx', 'tx', 'inner'] contributed no alias at all: the scan reads the declaration with a pattern for two-element tuples only. Every reference to those relations was then reported as a bare column and the suite failed. The pattern now accepts the optional join kind, and a case beside the existing ones covers a projection that declares one. Three comments also claimed more than the code behind them does. A fixture exemption said each entry asserts another variant covers the field, which nothing tracks. The controller scan promised that adding a controller cannot narrow the closure silently, which name matching cannot give across a type alias — the limit is now stated where the promise was. And the classification section carried a count that belongs to the preceding pull request and cannot be derived from this one; the rule it illustrated stays.
A join declared for filtering alone leaves the relation unselected, so the ORM never materialises it and the property is undefined on every row. The guard wrapped that undefined and handed it back: code reading the relation took the absent-relation branch, which reads exactly like a row that genuinely has none. Five projections join this way, each to filter on the owner. Reading such a relation now throws. The condition is structural rather than data-dependent - nothing of it is selected, so no row can carry it - which is why a left join whose row is legitimately absent is unaffected, and why the existing suites are unchanged by it.
Wrapping on every access returned a fresh proxy each time, and two things followed that are not
projection defects. `row.relation === row.relation` was false, so code comparing relations by
identity or using one as a map key behaved differently under the guard than without it. And each
proxy started with an empty record of caller assignments, so writing through a relation and
reading it back threw. A guard that invents failures is measuring itself.
The controller scan now also reads renamed imports. `import { SupportIssue as Issue }` left the
handler naming something the scan did not recognise, which would drop that controller out of the
eager closure silently — the one direction the scan must not fail in. The reverse case is handled
too: a local name standing for something else no longer counts as the entity it resembles.
That branch has no instance in this repository, so the name resolution moved into a function that
takes source text and is asserted on it directly, rather than depending on what the tree happens
to contain.
Dereferencing an undeclared relation throws on its own, which is why the guard let it pass. But `if (row.relation)` never dereferences: it reads undefined, takes the absent branch and answers - the same silent wrong answer the guard exists to prevent, one level up. Only eager relations count. Those the unprojected query did carry, so a projection failing to join one changes what the endpoint answers. A lazy relation was undefined before the conversion too: reading it may well be a defect, but not one this change introduced, and a guard reporting it would be reporting the code it replaced. `UserData.kycSteps` is that case, and it is why the distinction is drawn from the metadata rather than from a list of exceptions. Also corrects a rule in load-sites.md that contradicted the one this branch measures by: the distinction is not the presence of a dot - `COUNT(*)` has none and narrows - but whether the argument is the bare root alias.
|
The measurement answers it, and thanks for running it rather than settling it by argument. The ordering claim I was making doesn't survive it: with 55 handlers answering with entities, the eager relations are the response contract, and that isn't visible from the flag count. Four things I'd still raise, none about the direction. Size. 6.7k additions across 62 files and six review passes — and the test redesign added ~1.3k lines rather than retiring the exception lists it was meant to replace. That's a lot of surface for one merge, and the marginal conversions ( The five defects in shipped code should go separately, and soon. They're independent of the projection argument — The guard changes the failure mode, which argues for staging the rollout. An unselected column throwing instead of reading Nothing has run both versions over real rows yet. The benchmark is production-scale but generated. Two builds against the same restored snapshot, requests replayed from access logs, responses diffed — that is the check that answers what the client actually receives, and the guard makes it more valuable rather than less. Opened #4592 for the 55 entity-returning handlers, since that outlives this PR either way. |
The previous commit made the guard throw when an eager relation was read that the projection does not join, on the reasoning that the unprojected query carried the eager ones. That reasoning does not hold. `getIssueData` passed `loadEagerRelations: false` and named its relations explicitly, so `SupportIssue.transactionRequest` - eager, joined by no projection - would fail on a read the replaced query answered the same way. In the other direction a non-eager relation that the old query did load and the projection dropped passes without a word. Both cases exist in this branch. No metadata on the entity records what a particular `find` loaded, so the guard cannot derive it, and this is the third round in which sharpening that rule produced a new edge rather than fewer. The rule is withdrawn instead of narrowed again. What the guard still says is what it can observe about the query in front of it: a column the field list did not select, and a relation this projection joins while selecting nothing from it. Whether the answer as a whole matches the one before the conversion is level 4's question, and it compares two real responses rather than reading a flag.
2 participants
Builds on #4551, which inventories the endpoints and states the target: every read path selects the fields it returns, and a converted endpoint counts only at
4/4on the four test levels. That PR is documentation. This one is the harness those levels need, every conversion the criteria select, and three corrections to the inventory's own numbers.Converted, each at 4/4
GET /support/issue/:id/dataPUT /paymentLink/:id/posGET /swap/:id/historyGET /buy/:id/historyGET /sell/:id/historyGET /support/issueGET /support/issue/:idGET /user(v2)GET /kyc/usersGET /kyc/:id/documentsGET /user/profilePOST /user/apiKey/CTGET /liquidityManagement/pipeline/:id/statusGET /custody/orderGET /support/issue/listGET /realunit/support/listGET /dashboard/accounting/ledger/suspense175 tests across 15 spec files, all against a real Postgres. Recorded in the
Testscolumn ofdocs/endpoints.md, as that document requires.Two rows are worth reading twice.
GET /liquidityManagement/pipeline/:id/statusanswers with one status string and fetched 112 columns for it. And the last row understates:Max colsrecords the widest single measured query, and for the suspense report it does not capture that the query joined the transaction and the account withinnerJoinAndSelect, which loads each of them whole.The failure is loud now, instead of proven field by field
This changed part-way through, and it is the part worth reading.
A column the query did not select is
undefinedon the entity. Getters compute with it, the mapperputs the result in the response, and the endpoint answers 200. Level 3 below guards that by proving
completeness field by field — and doing it correctly needs a fixture that reaches every branch
reading every field. A fixture that misses its branch is green while proving nothing. The safety
net fails the same silent way the defect does, which is why writing these tests kept producing
assertions that could not fail; roughly thirty review findings on this branch were that one shape.
So the silence is switched off instead. Every query a
ReadProjectionbuilds returns rows wrappedso that reading a column the field list did not ask for throws, naming it:
Any test that exercises the endpoint then catches an incomplete field list, including a naive one.
It is installed once for the whole configuration rather than per spec, so a spec written later
cannot lose it quietly. Level 3 keeps the one job the guard cannot do — showing that a field in the
list is actually needed — which is a cost question, not a correctness one, and is documented as
such.
It earned itself on the first run.
PUT /paymentLink/:id/posassembled a recipient block out of theaccount's name, contact data and address through
configObj, and discarded it. The fix was to stopreading, not to load more: that projection is 7 columns rather than 26.
Two false positives it found in itself are worth recording, because both look like defects and are
not: TypeORM reports the owner-side join column under the relation's property name, and a column
the caller assigned before reading it back is not a projection defect.
Why a harness had to come first
None of the four levels can be tested against a mock: a mocked repository returns whatever the mock defines and cannot observe which columns the query asked for, so a projection that silently drops a field looks exactly like one that does not.
src/shared/utils/projection-test.util.tsbuilds the schema from the entity metadata (synchronize, 112 entities / 99 tables / 1,736 columns) into a schema per spec file, and generates the fixtures from the same metadata: every column gets a distinct value, so any empty field in a response proves the query failed to load something. It reuses theMIGRATION_TEST_PGgate the migration specs already use.It cannot live in the main suite. That suite compiles transpile-only (
isolatedModules: true), which makes TypeScript emitdesign:typeasObjectfor imported types; building a data source from the entity sources then fails on the first enum column.jest.projection.config.jscompiles with full type information — the same reason the Frick and coverage gates have their own configuration — and runs as its own CI job.The field list is a value (
ReadProjection) rather than a chain of calls at the query site, which is what lets level 3 re-run the production query with one field removed. A query rebuilt inside the spec could be wrong in exactly the way the projection is wrong.Three corrections to the classification
Each one made endpoints look convertible that were already projecting, or the reverse. All three are now used by every collection step and described in
docs/read-path-projections.md.A string argument is not automatically the root alias. The classification counted every one as
.select('user')— "reads like a projection but is not". That is false for.select('userData.id', 'id')and for.select('COUNT(*)', 'count'), which name something and do narrow the query. The rule that holds: a bare identifier is the root alias and loads every column; anything else narrows.The terminal call can discard the select list.
getCount()andgetExists()replace it withCOUNT(…)andSELECT 1, so a chain ending in either materialises no row whatever precedes it. Three load sites do this.GET /support/kycFileStatswas listed as fetching 99 columns because of one of them, and needed no conversion at all.The select argument can be a variable.
.select(bucketExpr, 'bucket')names an expression as surely as a literal does, and whether it narrows has to be resolved in the enclosing method. Four sites do this — three in the two caller-defined/gs/dbendpoints, the fourth an aggregate that madeGET /dashboard/accounting/ledger/marginlook convertible when it was already projected.Against the state this branch started from:
A spec reads the column counts out of
docs/endpoints.mdand compares them against the projections themselves, keyed by verb, version and path, so document and code cannot drift.What is left, and why
The measured filter chain, restated in
docs/read-path-projections.md:One endpoint remains, and the second criterion excludes it rather than the first four.
POST /support/issue/:id/messagehands the message it creates — carrying the issue and the account behind it — to the notification service. What that code reads is not determinable at the load site, so a field list here would be guessed.Seven endpoints are excluded one level below the handler signature: their DTO has a field typed as an entity (
currency: Fiat,targetAsset: Asset), so the response carries every column of it and a projection would list them all to save nothing. Narrowing those means changing the contract, which is a separate decision.The step that decides the size of this work is the first one, and it has a single cause: at 321 of the 484 load sites involved, the loaded entity leaves the loading method.
UserDataService.getUserDatareturnsPromise<UserData>to 113 different endpoints, so the field list belongs to each caller rather than to the load site. Splitting those hubs is a separate piece of work with a different shape, and nothing here assumes it happens.The filters are deliberately conservative and reject endpoints that can in fact be converted: an endpoint fails the first step if any load site it reaches leaks, including one on a branch that has nothing to do with the response. Nine of the seventeen conversions were found by reading the endpoint after the filter had rejected it. The counts are a lower bound, not a ceiling.
One criterion refined
The first pre-filter excludes an endpoint that writes the entity it loaded. The hazard it names is saving a partially loaded row back: the unselected columns are undefined on the entity and would be written as null.
PUT /paymentLink/:id/posandPOST /user/apiKey/CTwrite throughupdate(id, …), which sends only the columns named in the call, so a projected read cannot blank anything. What the criterion does have to keep excluding is a value the write derives from what was read — and that case is real here: the point-of-sale write merges the new access key into the existing configuration, so aconfigthe query failed to load would be a configuration silently reset. It is part of that projection and the spec asserts it directly.Corrections to the test definition
Level 3 measures the response, not emptiness. The first version asked whether a field went empty, and passed a projection missing
UserData.kycStatus—getKycWebhookStatusanswersNAwhen handed nothing, which is a valid value and a wrong one. It now compares against the response the full projection produced.Level 3 needs a baseline. If the response is already incomplete with the full field list, every reduced run fails too and "every field is required" comes out true without a single field having been shown to matter.
Level 1 counts
NaNas empty. A DTO field computed asa + b + cturns intoNaNthe moment one column is missing — not absent, so anundefinedcheck waves it through.A candidate may be a group of fields.
organizationName ?? firstname + surnameleaves the value filled whichever one is dropped, so every column reports as removable — true and useless.A fixture per branch, not per endpoint. Repeatedly the widest part of a conversion:
UserData.addressreads the account's columns or the organization's and never both;isCustodycompares one role, so on any other role dropping the column gives the same answer; a dummy-address flag hides an address, and dropped it reads undefined, which is falsy, so the address reappears.GET /user(v2) needed five level-3 fixtures for that reason.What the specs found in the code
SUPPORT_MESSAGE_RESPONSE_FIELDSnamedfileName, a getter overfileUrl. The ORM does not recognise it as a column, passes the expression through unquoted, and Postgres rejects the statement with a missing FROM-clause entry for a table that appears nowhere in the query.findThreadmixed.andWhere('…')with{ id: MoreThan(…) }. The object form resolves against the find-options alias rather than the builder's, with the same result.support_messageas a literal table. Every other table is resolved through the metadata, that one against the search path.mapSuspenseLegread the transaction id through the entity's@RelationId. That property is filled from the foreign-key column of the leg row, which a query naming its fields does not carry — the response answered with an undefinedtxIdand a 200. It now reads the id off the joined transaction, where every other value in that mapper already comes from.Where the queries moved
getSupportIssueListandgetSuspensebuilt their query inline against an injected repository, which is why neither could be tested: the only way to observe them was a chainable mock that records method calls, and that cannot see what the statement selects, nor tell a query that runs from one Postgres rejects. Both queries move into their repository; the services keep what they decide. The service specs follow — their assertions move from the recorded builder calls to the resolved query object, which is a stronger statement of the same thing.The int4 guard on the issue search is the clearest case. A term above 2^31-1 — a pasted phone number is the realistic one — must not reach the id comparison, or Postgres raises 22003 and the whole search fails. That guard was pinned by asserting on the emitted SQL string; it is now asserted where it fails, against a real database.
The other route, measured
docs/read-path-projections.mdnames the eager relations as the cause and this branch then wentendpoint by endpoint anyway. That ordering was questioned in review, correctly, so it is now
measured rather than argued.
55 handlers answer with an entity rather than a response object, 35 distinct entities between
them. For those the eager relations are not a loading detail — they are the answer. Their closure
covers 57 of the 103 eager relations built at runtime, so the majority cannot be removed as a
refactor: removing one changes what an endpoint returns. All 55 carry a role guard and all but two
are excluded from the Swagger schema, which makes it a decision to take rather than a wall — but a
decision, and not one taken here.
Four declarations are read by no code and contained in no response, and those are removed:
Buy.route,CustodyBalance.user,CustodyOrder.transaction,CryptoStaking.paybackDeposit. Thatnarrows 55 load sites — the custody order paths by 98 columns each — and 47 rows of
docs/endpoints.md. After it nothing is left that is both unread and outside every response.32 declarations remain: read somewhere, in no response. Removable, but not by rule — the compiler
says where a relation is read, not which query produced the value that was read.
eager-relations.projection.spec.tspins the closure and the total, and reports an addition withthe controllers whose answer it changes. It finds the entities that leave through a controller by
reading the controllers rather than from a list, because the first version of this measurement did
use a filter — it looked only at entities that already carried an eager relation — and missed 27 of
the 55 handlers for it.
The column counts in the inventory are re-measured against the entity metadata rather than adjusted,
and the aggregates in both documents are recomputed from their own tables.
One observation the conversion did not change
UserDtoMapper.computeCapabilitiesderives two of its flags fromkycStepsviagetStepsWith. Neither the projected query nor the one before it loads that relation, so those flags do not depend on the steps. The projection reproduces that rather than quietly altering it; whether the query should load them is a separate decision.