fix: address PR #47 review findings — column resolver, guard alerts, test coverage#49
Merged
Conversation
… coverage
Review fixes for the GOV.UK CSV schema-change PR:
- sponsorCsvColumns: ratingIdx no longer aliases the TierRating column
already claimed by typeIdx (returns -1 on the new format so callers
fall back to typeRating, the correct source)
- sponsorStateMachine: send admin alert when the fingerprint set is
below the trust threshold and Phases C2/D2 are skipped; correct the
circuit-breaker message (Phase C2 resurrections remain committed)
- sponsorListFetcher: remove dead legacy-only column resolver
(resolveColumnIndexes/rowToRecord had no callers and only matched
the pre-May-2026 layout)
- csvArchiver/sponsorMonitorJob: replace silent .catch(() => {}) on
admin alerts and the FAILED-status write with logged catches
- tests: fix qsvValidate/qsvCount mock shapes to match real contracts
New coverage (22 tests):
- sponsorCsvColumns.test.ts: both register layouts, predicate
precedence, ratingIdx-alias regression, unknown-header fallback
- stateMachineGuards.test.ts: mass-removal circuit breaker (trip,
under-threshold, SPONSOR_ALLOW_MASS_REMOVAL=1 bypass), Phase C2
self-heal sweep (event + suppressed/mass-repair branches), trust
gate on C2/D2, Phase D2 second-miss removal
- sponsorCsvNewFormat.test.ts: parseCsvFile throws on mass row
rejection; buildFingerprintedCsv throws and deletes gutted output
Resolves SonarQube typescript:S4325 (unnecessary type assertion) on the db.execute mock casts; converted all nine ReturnType<typeof vi.fn> casts to vi.mocked() for consistency.
|
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.
Follow-up to #47 (merged before review fixes landed). Addresses all Important findings from the multi-agent review.
Fixes
ratingIdxno longer aliases theTierRatingcolumn already claimed bytypeIdx— predicate now excludestier, so it returns -1 on the current format andderiveSponsorRowEnumsfalls back totypeRating(the correct source).resolveColumnIndexes/rowToRecord— zero callers, matched only the pre-May-2026 layout).downloadAndStreamToArraykept: it delegates toparseCsvFile, which uses the shared resolver..catch(() => {})(validation alert, abort alert, FAILED-status write) replaced with logged catches.qsvValidate/qsvCountmock shapes corrected to match real contracts.New coverage (22 tests, all green — 241/241 suite-wide)
sponsorCsvColumns.test.ts(9): both register layouts, predicate precedence, ratingIdx-alias regression, unknown-header fallbackstateMachineGuards.test.ts(7): mass-removal circuit breaker (trip / under-threshold /SPONSOR_ALLOW_MASS_REMOVAL=1bypass), Phase C2 self-heal sweep (event + suppressed mass-repair branches), trust gate on C2/D2, Phase D2 second-miss removalsponsorCsvNewFormat.test.ts(+2):parseCsvFilethrows on mass row rejection;buildFingerprintedCsvthrows and deletes the gutted output fileTest plan
npx vitest run— 241/241 across 26 files (pre-commit hook)npx tsc --noEmit— cleanSPONSOR_ALLOW_MASS_REMOVAL=1run)