Techx 612 - with sdk now fixed, we updated API calls to match and fixed state management

[shawn-bwell]

This provides a working UI for the consents page in the iOS SDK Sample App.

• now maintaining state of consents
• now successfully getting consents on load.

[claude]

🔴 loadConsents unconditionally wipes localProvisions after every successful getConsents (line 266). When the detached refresh fired immediately after createConsent (line 296) hits the server before the new consent has propagated, consentsByCategory returns the pre-create state AND the optimistic cache gets wiped — the row visibly reverts to the old badge, making the user's tap appear to fail. Fix: only delete keys where the fetched provision now matches the expected optimistic value: for (k, v) in localProvisions where map[k]?.provision?.type == v { localProvisions.removeValue(forKey: k) }.

Extended reasoning...

What's wrong

loadConsents ends with an unconditional localProvisions = [:] on line 266. The accompanying comment ("clear optimistic cache once server data arrives") encodes an assumption — "the data that arrived contains my pending write" — that is never enforced. Whenever that assumption is false, the optimistic cache is wiped against a stale snapshot and the UI reverts the user's visible action.

Step-by-step proof (single tap, no rapid taps required)

1. Category A has no existing consent. consentsByCategory["TOS"] == nil, localProvisions["TOS"] == nil. Badge shows None.
2. User taps Permit. submitConsent calls createConsent (line 289). It succeeds.
3. Line 291 sets localProvisions["TOS"] = "permit" → provisionType resolves to "permit" → badge updates to Permit. Good.
4. Line 296 spawns a detached Task { await loadConsents(showLoading: false) }.
5. That task calls sdk.user.getConsents(nil) on line 257. If the FHIR consent store / read replica / GraphQL cache has any propagation lag between the create endpoint accepting the write and a subsequent read seeing it (a routine property of read-replica architectures), the bundle returned will not yet include the new Consent resource.
6. Line 258–264 builds map — it does not contain "TOS".
7. Line 265 sets consentsByCategory = map (still no "TOS").
8. Line 266 wipes localProvisions = [:] — including "TOS".
9. SwiftUI re-renders. existing?.provision?.type is nil, localProvisions["TOS"] is nil → badge falls back to None. The user sees their tap visibly fail despite the server having accepted it.

Eventually a later refresh sees the propagated state and the badge corrects itself, but in the QA flow this manifests as a flicker that exactly mimics a broken Permit/Deny button — which is the central thing DCON-4083 is trying to validate.

Concurrent-tap amplification

When the user taps A then B before A's refresh completes (rows are independent, buttons remain available), A's in-flight refresh can wipe B's optimistic value before B's own refresh has even fired its read. Both rows revert.

Addressing the refutations

• "Eventual consistency is unverified." The very existence of localProvisions as an optimistic cache demonstrates the developer already designed around this gap (see the comment on line 290: "Optimistic update while background refresh is in-flight"). If reads were guaranteed read-your-writes, the optimistic cache would be unnecessary. FHIR consent stores routinely run behind read replicas, and the SDK's GraphQL layer adds another caching tier.
• "State self-corrects." It does — eventually — but during the gap the UI shows the wrong badge. The whole purpose of the optimistic cache is to prevent that flicker; wiping the cache against a snapshot that doesn't contain the write defeats it.
• "Intentional design / comment says so." The comment encodes an unenforced precondition, not a verified invariant. The fix doesn't change the intent — it just enforces the precondition.
• "QA-only code, no impact." The file header explicitly says this view is for DCON-4083 SDK validation — QA flicker that mimics a real failure is the worst possible outcome here.
• "Fix is disproportionate." The fix is one line, no new state, no timestamps:

  for (k, v) in localProvisions where map[k]?.provision?.type == v {
      localProvisions.removeValue(forKey: k)
  }

[shawn-bwell]

@claude is this still and issue? please re-review.

[claude]

🔴 The optimistic update at ConsentsView.swift:138 is masked by the existing server state. existing?.provision?.type ?? localProvisions[key] short-circuits to the stale server value whenever a consent already exists, so for return users (TOS, healthMatch, etc.) tapping Permit/Deny shows no UI change until the background loadConsents() refresh completes — defeating the cache the comment on lines 79-80 explicitly says is intended to win during the refresh window. Swap the operands to localProvisions[key] ?? existing?.provision?.type.

Extended reasoning...

The bug. ConsentsView.swift:138 resolves the row's displayed provision type with:

let provisionType = existing?.provision?.type ?? localProvisions[key]

The nil-coalescing operator returns the left operand when it is non-nil. So whenever the server already has a consent record for this category (existing != nil), the server-stored type wins and the optimistic local override in localProvisions[key] is completely masked. This is precisely the opposite of what the doc comment on lines 79-80 declares:

Optimistic provision overrides — applied immediately after createConsent while the background getConsents refresh is in-flight.

Step-by-step proof. Suppose consentsByCategory["healthMatch"].provision.type == "deny" (the user's existing server state). User taps Permit:

1. submitConsent inserts "healthMatch" into submitting → row shows spinner.
2. createConsent awaits successfully on the SDK.
3. localProvisions["healthMatch"] = "permit" is set (line 270 area).
4. Task { await loadConsents(showLoading: false) } is launched — fire-and-forget, completes ~100–500 ms later.
5. defer { submitting.remove(key) } fires, submitConsent returns.
6. SwiftUI re-renders the row. isSubmittingThis == false, so the buttons reappear. existing is still consentsByCategory["healthMatch"] from the previous load (the background refresh hasn't landed). existing?.provision?.type == "deny" is non-nil, so provisionType resolves to "deny".
7. Result: the Permit button still has the faded green background (Color.green.opacity(0.12)), the Deny button is highlighted as if it were still active, and the badge still reads Deny. The optimistic update is invisible to the user for the entire window between createConsent success and getConsents refresh.

Why no existing code prevents this. loadConsents does clear localProvisions on success (line ~263), which is correct as a confirm-or-replace step. But during the in-flight window, the local override is shadowed by stale server data because of the operand order. For brand-new categories where existing == nil, the optimistic update works because the left operand is nil and the coalesce falls through — so the bug only manifests for the already-touched case, which is precisely the case QA will hit when validating DCON-4083.

Impact. This is the central UX for the screen this PR adds. The reviewer flow for DCON-4083 — flipping an existing consent and confirming the UI reflects the change — exhibits a 100–500 ms (or longer, if the network is slow) period where the button highlighting and badge appear unchanged, and rapid double-taps look like no-ops. It's a textbook optimistic-update bug.

Fix. Swap the operands so the local override wins until loadConsents confirms or replaces it:

let provisionType = localProvisions[key] ?? existing?.provision?.type

This is the inversion the original intent calls for. It is one line, fully backwards-compatible with the existing loadConsents clear-on-success behavior, and matches the lifecycle described in the comment.

[shawn-bwell]

@claude is this still and issue? please re-review.

[claude]

🟡 Dead localization entry: the string "Current consent status unavailable — getConsents requires SDK v1.4.3 (DCON-4083). Permit/Deny still works for QA." (Localizable.xcstrings:268-271) is the pre-1.4.3 workaround warning. Commit 0dbd599 in this branch removed the call site from ConsentsView but left the catalog entry behind — grep confirms no Swift source references it. Nit cleanup: delete the entry to keep the catalog consistent with the code change in this same branch and avoid translators wasting effort on a never-shown warning.

Extended reasoning...

What the bug is. Localizable.xcstrings (lines 268-271) contains the auto-extracted entry:\n\njson\n"Current consent status unavailable — getConsents requires SDK v1.4.3 (DCON-4083). Permit/Deny still works for QA." : {\n "comment" : "A warning message that explains the current consent status is unavailable.",\n "isCommentAutoGenerated" : true\n}\n\n\nThis string is the pre-1.4.3 fallback warning that ConsentsView was meant to display when getConsents was unavailable.\n\nWhy it's orphaned. The most recent commit on this branch is 0dbd599 fix(DCON-4083): remove pre-1.4.3 workarounds from ConsentsView. The PR description confirms the new behavior — "now successfully getting consents on load" — so the workaround branch was deleted from Swift sources, but the corresponding catalog entry was never cleaned up. The branch also bumps the SDK to 1.4.3-beta (project.pbxproj, Package.resolved), which is exactly the version the warning referenced as missing.\n\nStep-by-step proof.\n\n1. The string lives in Localizable.xcstrings:268-271 (see modified file in this PR).\n2. grep -r 'Current consent status unavailable' bwell-swift-ios/ matches only Localizable.xcstrings — zero Swift source references.\n3. grep -r 'getConsents requires SDK' bwell-swift-ios/ likewise matches only the catalog.\n4. git log --oneline shows commit 0dbd599 fix(DCON-4083): remove pre-1.4.3 workarounds from ConsentsView on this branch, which deleted the call site that produced this entry.\n5. The new ConsentsView.swift added in this PR contains no reference to this string — isLoading/errorMessage use "Loading consents..." and "Failed to load consents." instead.\n\nImpact. None at runtime — Xcode's strings catalog doesn't crash on unreferenced keys; the file already contains other unreferenced entries with no localizations. The downsides are purely housekeeping: (a) translators may waste effort translating a warning that will never display, and (b) the catalog drifts further from the code it's supposed to mirror.\n\nFix. Delete the four-line entry at Localizable.xcstrings:268-271. No other change needed — Xcode will regenerate auto-extracted entries on the next build, and since no source references this key, it won't come back.

[shawn-bwell]

@claude is this still and issue? please re-review.

[claude]

🟡 consentsByCategory builds with map[code] = resource (no precedence rule), so when multiple Consent resources share the same category code, the retained record is whichever the SDK iterates last — arbitrary, not 'most recent' or 'active'. Since submitConsent calls createConsent (not update), repeated Permit/Deny toggles on the same category accumulate Consent resources, and the displayed provision/ID may disagree with the user's last action. Recommend filtering for status == .active then picking the one with the latest dateTime/meta.lastUpdated.

Extended reasoning...

What the bug is

In loadConsents() (ConsentsView.swift:258-264), the code collapses the consent bundle into a dictionary keyed by category code:

var map: [String: BWell.GetConsentBundleEntry.GetConsentResource] = [:]
for entry in result?.entry ?? [] {
    guard let resource = entry.resource else { continue }
    if let code = resource.category?.first?.coding?.first?.code {
        map[code] = resource   // overwrites silently, no precedence
    }
}

There is no precedence rule — whichever entry the SDK enumerates last for a given category wins, regardless of status, date, or relevance.

Why this is reachable in this PR

This isn't purely a hypothetical FHIR concern. The PR's own workflow drives the duplication:

1. submitConsent calls sdk.user.createConsent(request) — a CREATE, not an update — so each tap on Permit/Deny appends a new Consent resource server-side.
2. The file header documents this view's purpose: validating Swift SDK consent behavior by toggling Permit/Deny on categories like healthMatch.
3. After a QA flow like Permit → Deny → Permit on a single category, the FHIR Consent store contains multiple resources for that category. getConsents returns a bundle with no documented contract here that it filters to a single "current" record per category.

Step-by-step proof

1. QA opens the Consents screen. loadConsents populates consentsByCategory["healthMatch"] with whatever the server returns.
2. QA taps Permit on Health Match. submitConsent calls createConsent(status: .active, type: .permit, category: .healthMatch). A new Consent resource (call it R1, provision=permit) is created. Optimistic UI sets localProvisions["healthMatch"] = "permit". Background reload runs.
3. QA taps Deny on Health Match. Another createConsent runs; resource R2 (provision=deny) is created. Optimistic UI sets localProvisions["healthMatch"] = "deny". Background reload runs.
4. The bundle now contains at least R1 (permit) and R2 (deny). loadConsents iterates result.entry. The loop assigns map["healthMatch"] = R1, then map["healthMatch"] = R2, then (in a different iteration order) map["healthMatch"] = R1 — whichever the SDK happened to return last wins.
5. On reload, localProvisions is cleared, so the UI shows existing?.provision?.type from the arbitrarily-chosen resource. The displayed provision and the truncated ID (line 202) may both disagree with the user's last action.

Why existing code doesn't prevent it

The optimistic-update path (localProvisions[key] = …) masks this transiently but is cleared as soon as the background loadConsents completes. There is no client-side sort, no status == .active filter, no dateTime/meta.lastUpdated tiebreaker.

Addressing the refutation

The refutation argues this is unproven without concrete evidence that getConsents returns duplicates, and notes this is a QA sample app. That's fair — the SDK or backend may be filtering today. But the comment is explicit that this view exists to validate consent behavior in a sample app whose primary action is repeated toggling, and createConsent is provably append-style. Locking the client into an undocumented server contract via map[code] = resource is exactly the kind of silent assumption that bites a QA workflow. A 2-line defensive sort costs nothing.

How to fix

Replace the overwrite with a deterministic winner. Something like:

var map: [String: BWell.GetConsentBundleEntry.GetConsentResource] = [:]
for entry in result?.entry ?? [] {
    guard let resource = entry.resource,
          let code = resource.category?.first?.coding?.first?.code else { continue }
    if let existing = map[code] {
        // Prefer active; among same status, prefer latest dateTime
        let preferIncoming = (resource.status == .active && existing.status != .active)
            || (resource.status == existing.status && (resource.dateTime ?? "") > (existing.dateTime ?? ""))
        if preferIncoming { map[code] = resource }
    } else {
        map[code] = resource
    }
}

(Exact field names may differ in the SDK — the principle is status == .active first, latest dateTime/meta.lastUpdated as tiebreaker.)

Severity rationale

Filing as nit: we don't have visibility into whether the SDK/backend already filters to active+latest, and the QA workflow generally tests one category at a time. But for a view whose explicit purpose is validating consent behavior, depending on undocumented server response ordering is worth tightening up before this pattern is copied into a production consent UI.

[shawn-bwell]

@claude is this still and issue? please re-review.