Merge pull request #41 from kipp-ing/review-pr40-followups

Post-merge review fixes for #40: unreachable German strings, weakened tests, tier-collapse doc drift

Author: kipp-ing

CLAUDE.md

@@ -141,9 +141,11 @@ Active feature: `1100-purchase-gate` (branch `1100-purchase-gate`, cut from `mai
 spec at `specs/1100-purchase-gate/spec.md`, current plan at `specs/1100-purchase-gate/plan.md`
 (+ research/data-model/contracts/quickstart, 2026-07-19). Purchase gate /
 one-time unlocks: the free core stays whole (all sources + full core playback, basic
-transitions); paid tiers **Pro** (ambience — launch composition **Ken Burns motion + clock
-overlay**, amended 2026-07-19; never-publicly-shipped features only) and **Automation** (HA/MQTT
-+ App Intents) plus an optional everything-bundle; one-time purchases only, no subscriptions ever,
+transitions); one paid **Supporter Unlock** grants everything gated — ambience (**Ken Burns
+motion + clock overlay**, amended 2026-07-19; never-publicly-shipped features only) *and*
+automation (HA/MQTT + App Intents). The former Pro/Automation tiers and the everything-bundle
+were **collapsed into that single unlock on 2026-07-23** (PR #40): `Entitlement` has one case,
+`ProductCatalog.unlocks` is `[.supporter]`. One-time purchases only, no subscriptions ever,
 never the word "lifetime"; Family Sharing + universal purchase (incl. tvOS); on-device
 entitlement caching so unattended frames work offline indefinitely; never-claw-back
 (FR-1100-13); **sequencing is release-blocking: the gated build must be the first version the
@@ -154,19 +156,23 @@ anywhere in this public repo — pricing is decided in App Store Connect at subm
 **real `StoreKitClient` StoreKit 2 adapter**, `LockedRow`/`UnlockScreenView`/`TipJarView`), gates
 at the point of effect in both apps, Unlocks settings section (Restore + tip jar), US5 broker
 degradation (masked config behind a locked banner), and launch `refresh()` + `listenForUpdates()`
-now wired on both apps' production entry points. PurchaseKit 110 host tests + full iOS suite
-**153/0/9** green on iOS 18.6. **T030 fully done — the old "caveat" was a misdiagnosis, corrected
+now wired on both apps' production entry points. **Current measured gate (2026-07-25, iPad Pro
+11-inch (M4) sim): PurchaseKit 106 host tests green, full iOS suite 163/0/5** — the 5 skips are
+the ASC-screenshot, live-smoke, and 3 device-rig items. (The older "110 host / 153/0/9" figures
+predate the tier collapse and the post-PR-#40 review; re-measure before quoting any count.)
+**T030 fully done — the old "caveat" was a misdiagnosis, corrected
 2026-07-21.** It held that `SKTestSession` serves 0 products under headless `xcodebuild` ("the
 runner, not the runtime") so its 7 cases needed the Xcode IDE or a device. Actually two setup bugs
 in the test: `configurationFileNamed:` resolves against `Bundle.main` (the *host app* bundle, which
 lacks `Configuration.storekit`) and fails **silently**; and `resetToDefaultState()` clears
 `disableDialogs`, so setting it first left Ask-to-Buy blocking on a dialog. Both fixed, skip-guard
 replaced by a hard assertion, all 7 passing on the iOS 18.6 sim + Framepad (17.7.10) + FramePhone
 (26.0.1). Runs headlessly in CI; nothing folds into T042. See `docs/testing.md`; issue #16 closed.
-**Also note:** a 2026-07-21 full-suite run found two failures **pre-existing on `main`** and
-unrelated to 1100 — `BrokerSetupUITests` (#21) and `ShareSheetIncomingUITests` (#22, order-dependent)
-— so the "153/0/9 green" line above no longer reproduces as stated. **T033 done (2026-07-20):** the tvOS unlock surface — new `TVSettingsView` (gear
-destination) with the Ambience/Pro locked row, an Automation-gated Home-Assistant row
+**Resolved:** the two failures a 2026-07-21 run found pre-existing on `main` — `BrokerSetupUITests`
+(#21) and `ShareSheetIncomingUITests` (#22, order-dependent) — were fixed in PR #36; both issues are
+closed and all six of their cases pass in the 2026-07-25 run. **T033 done (2026-07-20):** the tvOS unlock surface — new `TVSettingsView` (gear
+destination) with the ambience locked row, a Home-Assistant row (both Supporter-gated since the
+2026-07-23 collapse; they were Pro- and Automation-gated when T033 landed)
 (`TVLockedBrokerView` masked-config banner when unentitled), and an Unlocks section (Restore +
 tip), reusing PurchaseKit UI via `fullScreenCover`; Apple-TV-simulator screenshot-verified under
 the `--uitest-entitlements` seams; the shared unlock/tip screens gained a tvOS-only opaque

OwnFrame/Intents/FrameIntents.swift

@@ -17,9 +17,12 @@
 import AppIntents
 import AppIntentsKit
 
-/// The contract's user-facing error copy (English-only, FR-300-30), mapped 1:1
-/// from the package's closed taxonomy. Parameter details (like the rejected
-/// percent) stay out of the copy by design — the message names the rule.
+/// The contract's user-facing error copy, mapped 1:1 from the package's closed
+/// taxonomy. The wording ships localized through the app's String Catalog
+/// (FR-300-30; German since 2026-07-23) — the glue tests pin the locale to English
+/// so they assert the contract wording rather than the runner's language.
+/// Parameter details (like the rejected percent) stay out of the copy by design —
+/// the message names the rule.
 enum FrameIntentError: Error, Equatable, CustomLocalizedStringResourceConvertible {
     case notConfigured
     case frameNotOpen

OwnFrame/Localizable.xcstrings

@@ -2552,12 +2552,12 @@
         }
       }
     },
-    "Where your money goes: the unlocks cover the project's running costs — developer account, AI tools, test hardware — and everything beyond that goes back to open-source projects that serve the community. The free frame stays whole, forever." : {
+    "Where your money goes: the Supporter Unlock covers the project's running costs — developer account, AI tools, test hardware — and everything beyond that goes back to open-source projects that serve the community. The free frame stays whole, forever." : {
       "localizations" : {
         "de" : {
           "stringUnit" : {
             "state" : "translated",
-            "value" : "Wohin dein Geld fließt: Die Freischaltungen decken die laufenden Kosten des Projekts — Entwicklerkonto, KI-Tools, Testgeräte — und alles darüber hinaus geht zurück an Open-Source-Projekte, die der Gemeinschaft dienen. Die kostenlose App bleibt für immer voll funktionsfähig."
+            "value" : "Wohin dein Geld fließt: Die Supporter-Freischaltung deckt die laufenden Kosten des Projekts — Entwicklerkonto, KI-Tools, Testgeräte — und alles darüber hinaus geht zurück an Open-Source-Projekte, die der Gemeinschaft dienen. Die kostenlose App bleibt für immer voll funktionsfähig."
           }
         }
       }

OwnFrame/Onboarding/AlbumPickerView.swift

@@ -105,7 +105,11 @@ struct AlbumPickerView: View {
     static func subtitle(for album: Album) -> String? {
         var parts: [String] = []
         if let dateText = dateText(album.startDate, album.endDate) { parts.append(dateText) }
-        if let count = album.assetCount { parts.append(count == 1 ? "1 photo" : "\(count) photos") }
+        // Built as a String and joined, so the count needs an explicit lookup — a bare literal
+        // here would ship the English text into an otherwise localized subtitle.
+        if let count = album.assetCount {
+            parts.append(count == 1 ? String(localized: "1 photo") : String(localized: "\(count) photos"))
+        }
         return parts.isEmpty ? nil : parts.joined(separator: " · ")
     }
 

OwnFrame/Onboarding/PhotoAlbumPickerView.swift

@@ -117,8 +117,14 @@ struct PhotoAlbumPickerView: View {
 
     @ViewBuilder
     private func limitedContent(_ pool: SourceCollection?) -> some View {
-        let label = pool?.title ?? String(localized: "Selected Photos")
-        let isAdded = sourceLibrary.sources.contains { $0.label == label }
+        // The pool's model-side title is a fixed English identifier (PhotoLibraryKit ships no
+        // catalog); the row shows — and persists — the localized name instead. Identity is the
+        // sentinel collection ID, not the label, so neither a rename nor a language switch can
+        // make an already-added pool look un-added.
+        let label = String(localized: "Selected Photos")
+        let isAdded = sourceLibrary.sources.contains {
+            $0.kind == .photoLibrary(collectionID: PhotoLibrarySource.selectedPhotosID)
+        }
         List {
             Section {
                 Button {

OwnFrame/Slideshow/SlideshowSettingsView.swift

@@ -374,7 +374,9 @@ struct SlideshowSettingsView: View {
                 } footer: {
                     VStack(alignment: .leading, spacing: 10) {
                         Text("Restore purchases you already own. Tips are optional and unlock nothing — they just say thanks.")
-                        Text("Where your money goes: the unlocks cover the project's running costs — developer account, AI tools, test hardware — and everything beyond that goes back to open-source projects that serve the community. The free frame stays whole, forever.")
+                        // Transparency statement (docs/where-the-money-goes.md) — word-for-word the
+                        // tvOS copy in TVSettingsView, so both platforms share one catalog entry.
+                        Text("Where your money goes: the Supporter Unlock covers the project's running costs — developer account, AI tools, test hardware — and everything beyond that goes back to open-source projects that serve the community. The free frame stays whole, forever.")
                             .accessibilityIdentifier("settings.unlocks.moneyPledge")
                     }
                 }

OwnFrameTests/FrameIntentGlueTests.swift

@@ -68,6 +68,18 @@ struct FrameIntentGlueTests {
 
     // MARK: - Error mapping (contract copy, state untouched)
 
+    /// The contract copy resolved in English, whatever the runner's locale is.
+    ///
+    /// This copy ships translated (spec 300), so a bare `String(localized:)` renders German on a
+    /// German device and the comparison against the contract text goes red for the wrong reason.
+    /// Pinning the locale keeps these assertions about the *contract wording*, not the tester's
+    /// language — the translations themselves are covered by the catalogs.
+    private func contractCopy(_ error: FrameIntentError) -> String {
+        var resource = error.localizedStringResource
+        resource.locale = Locale(identifier: "en")
+        return String(localized: resource)
+    }
+
     @Test func outOfRangeBrightnessThrowsTheContractCopyAndRecordsNothing() async throws {
         let fixture = try Fixture()
         defer { fixture.restore() }
@@ -79,7 +91,7 @@ struct FrameIntentGlueTests {
         }
         #expect(fixture.surface.calls.isEmpty)
         #expect(
-            String(localized: FrameIntentError.brightnessOutOfRange.localizedStringResource)
+            contractCopy(.brightnessOutOfRange)
                 == "Brightness must be between 0 and 100 percent."
         )
     }
@@ -92,18 +104,18 @@ struct FrameIntentGlueTests {
             _ = try await PauseSlideshowIntent().perform()
         }
         #expect(
-            String(localized: FrameIntentError.notConfigured.localizedStringResource)
+            contractCopy(.notConfigured)
                 == "Set up the frame first — open OwnFrame and add a source."
         )
     }
 
     @Test func remainingContractCopyMatches() {
         #expect(
-            String(localized: FrameIntentError.frameNotOpen.localizedStringResource)
+            contractCopy(.frameNotOpen)
                 == "OwnFrame must be open on the frame device for this."
         )
         #expect(
-            String(localized: FrameIntentError.sourceMissing.localizedStringResource)
+            contractCopy(.sourceMissing)
                 == "This source no longer exists in the frame's library."
         )
     }

Packages/PurchaseKit/Sources/PurchaseKit/Localizable.xcstrings

@@ -342,6 +342,17 @@
         }
       }
     },
+    "The purchase could not be completed." : {
+      "extractionState" : "manual",
+      "localizations" : {
+        "de" : {
+          "stringUnit" : {
+            "state" : "translated",
+            "value" : "Der Kauf konnte nicht abgeschlossen werden."
+          }
+        }
+      }
+    },
     "Tip" : {
       "extractionState" : "manual",
       "localizations" : {

Packages/PurchaseKit/Sources/PurchaseKit/PurchaseViewModel.swift

@@ -194,9 +194,12 @@ public final class PurchaseViewModel {
     ///
     /// The store's own description is appended when it has one, because "why" is the entire point
     /// of the `.failed` case; the leading sentence guarantees the message is never empty or raw.
-    /// Not localized — the app ships English-only by design (CLAUDE.md).
+    ///
+    /// The lead resolves against `.module` — a package's strings do not live in the app bundle,
+    /// and this one renders under an already-translated title. The appended store description
+    /// comes from StoreKit and is localized by the system.
     private static func failureMessage(for error: any Error) -> String {
-        let lead = "The purchase could not be completed."
+        let lead = String(localized: "The purchase could not be completed.", bundle: .module)
         guard let description = (error as? any LocalizedError)?.errorDescription,
               !description.isEmpty
         else { return lead }

Packages/PurchaseKit/Sources/PurchaseKit/UI/LockedRow.swift

@@ -8,8 +8,9 @@ extension Entitlement {
     /// The user-facing unlock name, as it appears on locked badges and unlock screens.
     ///
     /// Presentation only, which is why it lives beside the view rather than in the model: the
-    /// entitlement itself is a capability, not a marketing name. Not localized — the repo ships
-    /// English-only by design (CLAUDE.md).
+    /// entitlement itself is a capability, not a marketing name. Deliberately *not* localized:
+    /// "Supporter" is the product's proper name, the fixed morpheme every locale keeps — the
+    /// German copy builds on it too ("Supporter-Freischaltung").
     public var displayName: String {
         switch self {
         case .supporter: "Supporter"