Feat/sw ssn correction cuid - #1866
Draft
landonshumway-ia wants to merge 13 commits into
Draft
Conversation
Phase 1 of porting the SSN-correction feature from compact-connect. This adds the field at the edge only; nothing downstream reads it yet. - LicensePostRequestSchema accepts an optional previousSSN, validated to only be meaningful alongside an ssn. This compact allows SSN-less uploads identified by license number, so a previousSSN with no ssn is a real possibility and would silently do nothing. - LicenseIngestSchema accepts an optional previousProviderId, set by the preprocessor when a previousSSN resolves to a different provider. No feature flag: neither social work nor cosmetology is in production, so there is nothing to gate. The bulk upload path needed no change - the CSV reader drops blank cells per row, so a correction file's mostly-empty previousSSN column is only read on the rows a state actually filled in. Covered by a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2. Selection logic only - nothing calls these yet. get_records_associated_with_license is scoped to jurisdiction and license type rather than to a single license, so the single-state and multi-state licenses of one type are always selected together. They are a validated pair: a multi-state license with no matching single-state license reports a validation error back to the uploading state, so moving one without the other would break that pairing on both the source and destination providers, and SQS ordering would decide whether a state saw a spurious error. get_person_level_records returns the provider update history alone. This compact has no military affiliation records, so unlike compact-connect that is the whole of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverses the both-scopes bundling from the previous commit. A state may legitimately need to correct the SSN on only one scope's row - the other scope's row may have been uploaded under the correct SSN all along - so the migration must move exactly the license the correction row identifies: jurisdiction, license type, AND scope. The pairing concern that motivated bundling turns out to be a feature here, not a bug: the missing-single-state check fires at multi-state ingest as a notification event, so a pair split across two providers mid-correction prompts the state to correct the remaining row rather than silently breaking anything. This also restores compact-connect's single-target-license migration structure for Phase 3: one target license key, a three-item final transaction group, and a single-record idempotency guard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3. Nothing calls the migration yet - that is Phase 5. migrate_provider_for_ssn_correction moves one license record and its dependent records from the provider id an incorrect SSN resolved to onto the corrected one, keeping compact-connect's structure: the dateOfUpdate concurrency fence on the old provider record, the idempotency guard that short-circuits a replay once the target license has left, the abort-before-writing check that a full migration would orphan nothing, and the create/delete/final batching for migrations too large for a single atomic transaction. Dropped relative to compact-connect, none of which exists here: privileges, military affiliations, S3 document moves, payment transaction re-pointing, and Cognito/email side effects. cuid_ownership.py is new and social-work-only. A correction moves one license at a time while a CUID is earned by a single-state/multi-state pair, so the ownership rule is applied by simulating both sides of the move and asking which holds the older qualifying pair. A pair completes at the LATER of its two members' firstUploadDate - comparing earliest uploads would credit a pair for a date at which it did not yet qualify. Kept as a pure function so all branches are unit-testable, and so the cosmetology port can delete the file whole. Two behaviours worth noting for review: - CUID removal from a surviving provider is done by omitting the field from the partial-migration repopulation Put, not a separate write. - A CUID stranded by a full migration is returned as retired_cuid so Phase 5 can alarm on it; nothing else records it ever existed. Refactor: _generate_put_transaction_item now takes an optional condition and is the single place a Put item is shaped; _build_put_transaction_item is the data-class-shaped wrapper over it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 4. The preprocessor is the only place that can turn a previousSSN into something the ingest handler can act on, since the ingest handler has no SSN access. Both SSNs are popped before anything is published, so neither reaches the event bus - asserted over the whole serialized detail rather than by key, so a full SSN cannot slip through under some other field name. previousProviderId is only set when the previous SSN resolves to a genuinely different provider. A previousSSN equal to the ssn, or one resolving to the same provider, is not a correction. A previousSSN that was never uploaded creates a mapping resolving to a provider with no records, which the migration treats as a no-op. A failure resolving it re-raises, so SQS redelivers rather than publishing the row as an ordinary upload and silently skipping the migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phases 5 and 6. This is the commit that turns the feature on. The migration runs before the provider records are read, so everything that follows sees the migrated state - which is also what lets the ordinary CUID assignment rule work correctly after a correction. CUID assignment is suppressed for any upload carrying a correction, gated on previousProviderId being present rather than on whether the migration moved anything. A state can resend the same correction, and on the resend the idempotency guard reports no migration performed while the corrected record now holds a qualifying pair; minting there would lock in a new CUID that the ownership rule then protects, permanently retiring the original. Over-minting is irreversible; under-minting is fixed by the next ordinary upload. Verified by removing the gate and confirming three tests fail. Four metrics and alarms: full, partial, and no-migration outcomes, plus a retired-CUID alarm for the externally visible case where a public identifier stops resolving. The alarm definitions are a table rather than compact-connect's three near-identical blocks. Their namespace and dimensions are asserted against what the handler publishes, since a mismatch would leave them permanently in INSUFFICIENT_DATA and the feature running unobserved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes Phase 6. Adds previousSSN to the state API request model and documents it for state operational staff. The docs cover the two things per-license correction asks of a state: that correcting both scopes of a license type means submitting a corrected row for each, and that the unpaired-multi-state notification they may see partway through is expected rather than an error. Also documents the CUID ordering constraint - an ordinary upload submitted partway through a multi-row correction can retire the practitioner's identifier - and that a practitioner mid-correction may briefly have no CUID and so not appear in public search. Not done here: docs/api-specification/latest-oas30.json and the postman collection are generated by bin/download_oas30.py from a *deployed* API Gateway, so they cannot be regenerated before this change ships. Both need regenerating against an environment that has it deployed; hand editing generated artifacts would only drift them further. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Applies review feedback. Drops the _UNDATED fallback. Every license carries a firstUploadDate from the moment it is ingested, so firstUploadDate is now read directly - an absent value means something is wrong upstream, and failing loudly beats silently mis-assigning a public identifier that can never be reassigned. Replaces the ownership algorithm. Check 2 now asks whether the *corrected* practitioner qualifies once this license lands, rather than whether the old record still qualifies without it; check 3 compares the migrating license's upload date against everything remaining, rather than comparing pair-completion times on both sides. The behavioural change is that qualification of the destination gates the move. A CUID no longer follows licenses onto a record that cannot yet hold a pair, and the old record keeps its identifier even when it no longer qualifies itself. That makes cases 1, 4 and 5 consistent with each other: a state that accidentally attached licenses to an existing practitioner never strips that practitioner of the identifier their own, older licenses earned, and the corrected practitioner is assigned one by the ordinary rule on their next upload. One consequence worth watching: a full migration whose destination does not qualify now retires the identifier rather than moving it. That was already reported as retired_cuid and alarmed on, and is covered by its own test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
When something older stays behind, the identifier now only stays with it while the old record still holds a qualifying pair. Once it does not, nothing left there could have earned the identifier, so it moves to the corrected practitioner rather than being stranded on a record that no longer qualifies while the corrected one, which does, has none. This restores the outcome the client questionnaire records for case 4 - a practitioner holding a lone license onto whom a state mistyped a full pair - which the previous revision had inverted. Cases 1 and 5 are unaffected: there the remainder is still a qualifying pair, so check 4 takes its yes branch and the identifier stays where it was earned. Verified load-bearing by removing the check and confirming its unit and function tests both fail. One existing test needed its fixture changed: it was asserting check 3's "before all remaining" comparison with an unpaired remainder, which check 4 now short-circuits, so its remaining licenses are a qualifying pair spanning the migrating license's date. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verifies the previousSSN correction end to end against a deployed environment, focused on the CUID behaviour a correction has to get right - it is a public search key, so it must be neither duplicated nor silently retired. A correction moves one license record at a time, so a practitioner holding a matching single-state and multi-state license takes two uploads to correct. The test uploads both under one incorrect SSN, waits for a CUID to be assigned, then corrects them one at a time: - After the first correction the corrected practitioner holds a lone license and does not qualify, so the CUID must stay on the original record and must not be minted on the new one. - After the second the pair is whole again and nothing remains behind, so the original CUID must move across unchanged. Also asserts the original partition is emptied, every record arrived intact field-for-field, ssnLastFour is corrected on the moved licenses but untouched on the one left behind, and an ssnCorrection audit record exists under the corrected provider id. Two things this test could not reuse from smoke_common: - query_provider_by_name returns only the first match, on the stated assumption that each test names its practitioner uniquely. A correction breaks that by design - mid-test the practitioner exists under two provider ids - so this collects every match instead. - That query also needs to refresh the staff token on a 401, since the polls here can outlive it. Cleanup of both provider partitions runs in the test's own finally, so a failure part way through still clears what it created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Inserted inline rather than regenerated from a deployed API Gateway, since the change is a single property. This is not cosmetic: the license POST schema sets additionalProperties=false, so as it stood the documented contract would have rejected a request carrying previousSSN. The description and pattern are copied from the CDK model so the two stay in step, and the field is correctly absent from the required list. The postman collection is generated from this spec by bin/update_postman_collection.py and is still out of date. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The Social Work compact needs to support allowing states to correct SSNs for practitioners, similar to what we support for JCC. In many respects this SSN correction process is simpler for Social Work, as there are no privilege transaction records or practitioner cognito accounts to manage when SSNs are corrected. However, we do have to account for the CUID, which is tracked at the provider level when both a single and multi-state license has been uploaded for a practitioner.
This adds support for the 'previousSSN' field, similar to the contract used by JCC for correcting SSNs on a license record. When an SSN is corrected for a specific license, we move over the license and any records associated with it (ie adverse actions or investigations). We then make a check for the following conditions when determining what to do with the CUID:
yes -> move the license over, do not overwrite the existing CUID
no -> proceed to question 2
no -> move record over, do not move CUID
yes -> proceed to question 3
yes -> move the CUID over to the corrected practitioner record, remove CUID from the original record. A new CUID will be generated for the original practitioner when a state performs another qualifying license upload for one of the remaining licenses.
no -> proceed to question 4
no -> move CUID over with license records
yes -> move over the license records, but do not move the CUID and do not generate a new one. The new practitioner record will be created without a CUID. For states that accidentally added license records to an existing practitioner (case 1 and 5) a new CUID will be generated when the state performs a subsequent upload for those licenses after the SSN has been corrected for them.
Effectively, the CUID will be associated with the set of license records that were uploaded first which caused the CUID to be generated initially, so if they move due to a SSN correction, the CUID moves with them.
Requirements List
Testing List
yarn test:unit:allshould run without errors or warningsyarn serveshould run without errors or warningsyarn buildshould run without errors or warningsbackend/compact-connect/tests/unit/test_api.pyrun compact-connect/bin/download_oas30.pyCloses #1857