Add a test verifying `GET FIX - #210
Merged
Jagadeeshftw merged 1 commit intoJul 27, 2026
Merged
Conversation
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
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.
================================================================================
PULL REQUEST
TITLE
Guard CSV_COLUMNS against silent CSV export drift (tests + compile-time check)
BASE BRANCH: main
COMPARE BRANCH: fix/csv-column-drift-guard
TYPE: test + hardening + docs
BREAKING: No. No response body, route, or status code changes.
================================================================================
SUMMARY
routes/anchors.ts declared CSV_COLUMNS = ["id", "name", "registeredAt",
"active"] as a plain string[] and passed it to toCsv. routes/settlements.ts did
the same for Settlement.
toCsv renders exactly the columns it is handed and ignores every model field it
is not handed. Nothing connected those constants to the Anchor / Settlement
types, so adding a field to a model without updating the constant would have
silently dropped that field from the CSV export. No compile error, no failing
test, and consumers downloading the CSV quietly lose data. This is the same
class of problem as the existing OpenAPI schema-drift issue, applied to export
column coverage.
This PR closes that gap with two independent guardrails.
================================================================================
WHAT CHANGED
Tests now parse the first line of the real ?format=csv response and assert the
exact column list AND order:
GET /api/v1/anchors?format=csv
-> id, name, registeredAt, active
GET /api/v1/settlements?format=csv
-> id, anchor, asset, amount, fee, status, createdAt, cancelReason
GET /api/v1/anchors/:id/settlements?format=csv
-> same as settlements, and additionally pinned to equal the top-level
settlement export so the two separate column constants cannot diverge
Beyond the literal hardcoded list, each suite also asserts the header against
Object.keys(...) of a real serialized API response object rather than against a
second hardcoded list. That is the part that actually resists drift: a field
added to the model and surfaced in the JSON response fails the suite even if
nobody remembers to update the expected-column array. The settlement version
uses a cancelled settlement so the optional cancelReason field is present.
Also covered: the header is still emitted when the result set is empty, and
each data row has exactly one cell per header column.
The issue asked whether CSV_COLUMNS could instead be derived from keyof Anchor
/ keyof Settlement to make drift structurally impossible. It can, so it is
implemented here rather than deferred as a follow-up:
csvColumnsFor() lives in utils/csv.ts. It is a pure type-level helper: it
returns its tuple unchanged at runtime and has zero effect on any response
body. It constrains the tuple to (keyof T & string)[] and additionally requires
the tuple to be exhaustive, so both directions of drift now fail npm run build:
field, or removed field), and
uncovered field via the CSV_COLUMNS_IS_MISSING_MODEL_FIELDS property.
Optional model fields such as cancelReason? are treated exactly like required
ones, because omitting them truncates the export just the same.
toCsv's columns parameter was widened from string[] to readonly string[] so the
const tuples can be passed directly.
docs/ARCHITECTURE.md gains a "CSV Export Column Coverage" section documenting
the silent-drift risk and both guardrails, written in the same "locked in by a
test" style as the existing persistence-swap section. CHANGELOG.md gains an
[Unreleased] entry.
================================================================================
VALIDATION
Full CI pipeline (lint -> build -> test) run locally, all green:
Coverage: 97.11% statements overall, against the 95% guideline.
src/utils/csv.ts at 100%.
Acceptance criterion "new tests pass against the current CSV_COLUMNS constants"
was verified in isolation: the test-only commit (ba5fea9) was checked out on its
own, against completely unmodified production code, and gives 40 suites /
449 tests green. So the tests are not passing merely because the production
code was changed alongside them.
DRIFT WAS DELIBERATELY SIMULATED AND REVERTED (not present in this diff)
Per the acceptance criteria, drift was introduced locally during review to
confirm the tests actually fail, then reverted.
Simulated drift Result
1 Added required field tier to Anchor, Build fails, error names
left CSV_COLUMNS untouched tier
2 Added OPTIONAL field tier? to Anchor and Build fails naming tier.
populated it in the service With the type guard
bypassed, the runtime test
fails with - "tier"
3 Removed "active" from the anchor Build fails, error names
CSV_COLUMNS active
4 Removed "cancelReason" from the settlement Build fails naming it. With
CSV_COLUMNS the guard bypassed, 4
runtime tests fail
Case 2 is the important one. It confirms the runtime tests catch drift on their
own, which proves the two guardrails are genuinely independent rather than the
type guard simply masking weak tests.
All simulations were reverted. grep for tier / SIMULATED / ghostColumn across
src/ and docs/ returns nothing, and the working tree is clean.
================================================================================
FILES CHANGED
src/routes/anchors.test.ts +99 Header-coverage tests for the anchor
export and the nested settlement
export, plus a parseHeaderRow helper
src/routes/settlements.test.ts +88 Header-coverage tests for the
settlement export, plus a
parseHeaderRow helper
src/utils/csv.ts +44 Added csvColumnsFor(); toCsv now
accepts readonly string[]
src/utils/csv.test.ts +69 Unit tests for csvColumnsFor
src/routes/anchors.ts +22 CSV_COLUMNS and SETTLEMENT_CSV_COLUMNS
built via csvColumnsFor
src/routes/settlements.ts +11 CSV_COLUMNS built via csvColumnsFor
docs/ARCHITECTURE.md +54 New "CSV Export Column Coverage"
section
CHANGELOG.md +19 [Unreleased] entry
Total: 8 files, 396 insertions(+), 10 deletions(-)
================================================================================
COMMITS
ba5fea9 test: assert exact CSV header columns for anchor and settlement
exports
8efd189 feat: derive CSV_COLUMNS from the model type to make drift a build
error
749b9b9 docs: document the CSV export column-coverage guarantees
The commits are deliberately split. The issue framed itself as test-only, so if
you would prefer to keep it that way, commit 8efd189 can be dropped and the
tests still stand on their own and still pass.
================================================================================
NOTES FOR THE REVIEWER
When adding a field to Anchor or Settlement, add it to the corresponding
CSV_COLUMNS, and for settlements add it to BOTH routes/settlements.ts and the
nested SETTLEMENT_CSV_COLUMNS list in routes/anchors.ts, plus the
expected-column lists in the tests. The build will name the field you missed.
Follow-up worth considering, intentionally out of scope here:
SETTLEMENT_CSV_COLUMNS is duplicated across two route files. The compile-time
guard keeps both honest against the Settlement model, and a test pins the two
exports to each other, so they cannot silently diverge today. Extracting a
single shared constant would still remove the duplication.
Security: no runtime behaviour change. csvColumnsFor is erased at compile time
and returns its input unchanged. No new dependencies were added.
Closes #145