Log of Changes

[MBtheOtaku]

Settings page UI Update, Auto Unmute, Failed MIssing Translations, and Moved Deduplications script to non_osm_scripts

Summary by CodeRabbit

• New Features

  • User data deletion control in Settings
  • Persian (fa‑IR) and Russian (ru) localizations
  • New callout categories: transportation, intersection, safety

• Enhancements

  • Guided NaviLens integration from beacons with improved flows
  • Redesigned Settings with collapsible callout sections
  • Search bar layout fixed for iOS 26+
  • Minimum iOS deployment increased to 15.0

• Localization

  • Multiple string edits for clarity and accessibility

[JJGatchalian]

It looks like this can't be merged yet because some of the settings have been reverted, e.g. Audio Mute Distance and Tile Server URL.

[steinbro]

Please apply the moved deduplication script to the branch in #177 -- it's best to keep all the changes for a feature to the same pull request so we can merge them at once.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors callout state handling from an Objective-C state machine to a Swift enum, removes AppCenter, expands NaviLens support (new guide/launch flows), restructures settings and accessibility, adds CI/TestFlight build/upload scripts, updates localizations (fa-IR, ru), and adds data/visualization utilities and tests.

Changes

Cohort / File(s)	Summary
CI / TestFlight \.github/workflows/build-for-testflight.yml, \.github/workflows/ios-tests.yml, apps/ios/Scripts/ci/archive-upload.sh, apps/ios/Scripts/ci/ExportOptions.plist	New TestFlight workflow triggered from ios-tests, macos-14 runner, Xcode setup, code-signing and upload script; ios-tests adds LocalizationLinter and concurrency cancellation.
State machine removal & refactor apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift, apps/ios/GuideDogs/Code/Behaviors/GDAStateMachine/*	Removed Objective-C GDAStateMachine headers/impl; replaced callout logic with a private Swift State enum and direct transitions in CalloutStateMachine.
NaviLens integration apps/ios/GuideDogs/Code/Visual UI/Helpers/Integrations.swift, .../LocationAction.swift, .../BeaconToolbarView.swift, multiple view controllers & views`	Added guideToNaviLens/detail flows (throwing), safeGuideToNaviLens, updated action text to combined beacon_or_navilens keys, set home beacon focus and pop-to-root after guiding.
Project & runtime config apps/ios/GuideDogs.xcodeproj/..., apps/ios/UnitTests.xctestplan	Removed AppCenter references, bumped deployment targets/marketing version, added fa-IR/ru locales, enabled thread sanitizer and NSZombie for scheme/test plan.
Settings UI & user data apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/*, apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift, apps/ios/GuideDogs/Code/Visual UI/Views/Settings.storyboard	Collapsible Callouts sections, replaced mobility with transportation/intersection/safety, added User Data section and deleteUserData action, removed servicesHostName, added autoUnmuteEnabled setting.
Accessibility cleanup apps/ios/GuideDogs/Code/.../String+Extension.swift, CalloutOrigin.swift, multiple view controllers`	Removed accessibilityString/localizedStringForAccessibility and many explicit accessibilityLabel assignments across help, settings, and feature UI.
Audio & concurrency fixes apps/ios/GuideDogs/Code/Audio/AudioEngine.swift, DiscreteAudioPlayer.swift	Added shouldStart guards, moved callbacks to MainActor, serialized buffer state on player queue to avoid races.
Markers & lists UI apps/ios/GuideDogs/Code/Visual UI/Views/Markers & Routes/*, DeleteModifier.swift (removed)	Replaced VStack scrolls with SwiftUI List for markers/routes, centralized onDelete handling, removed custom DeleteModifier, added contentShape to cells.
Localization & linter apps/ios/GuideDogs/Assets/Localization/*, apps/ios/Scripts/LocalizationLinter/main.swift	Updated en-GB strings, added fa-IR and ru InfoPlist.strings, removed some locale keys (pt), changed privacy URL, linter now accumulates failures instead of early-exit.
Build metadata & docs apps/ios/fastlane/metadata/en-GB/privacy_url.txt, docs/ios-client/ReferenceEntity_model.md	Privacy URL update and new documentation for ReferenceEntity/markers/waypoints.
Search & Map utilities SearchResultsUpdater.swift, LocalizationContext.swift, various svcs/data scripts, visualize_tiles_map.py`	Stored MKLocalSearch instance, reduced search regions, locale fallback by language code, added Python utilities for non-OSM validation and tile visualization.
Telemetry & build settings GDATelemetry.swift, BuildSettings.swift	Removed AppCenter Analytics calls; replaced appCenter Source with adhoc.
Tests apps/ios/UnitTests/*	Enabled thread sanitizer in tests; adjusted AudioEngine tests to use shared engine; added navilens filter test case.

Sequence Diagram(s)

sequenceDiagram
    participant UI as UI (Caller)
    participant App as App Integrations
    participant Location as LocationService
    participant Beacon as BeaconManager
    participant NaviLens as NaviLens App / App Store

    UI->>App: request guideToNaviLens(detail)
    App->>Location: get current location (optional)
    alt location available
        Location-->>App: coordinates
        App->>Beacon: attempt beaconing with coords and detail
        Beacon-->>App: success / error
        alt beacon success
            App->>NaviLens: open in-app NaviLens (if available)
            NaviLens-->>App: launched
        else beacon error
            App->>NaviLens: fallback launch (open app or App Store)
            NaviLens-->>App: launched / shown App Store
        end
    else no location
        App->>NaviLens: launch directly (app or App Store)
        NaviLens-->>App: launched
    end
    App-->>UI: result / throw on error

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• Remove GDAStateMachine implementation and references #219: Same removal of GDAStateMachine and refactor to Swift-based callout state handling.
• fix scrolling in markers list and move search back to top for iOS 26+ #230: Overlaps UI changes for markers/routes, DeleteModifier removal, and iOS 26 search-bar handling.
• Remove callout-specific accessibility overrides in Help & Tutorials #236: Overlapping accessibility cleanup (removal of accessibility helpers and label overrides).

Suggested reviewers

• RDMurray

"🐰 I hopped through state and strings today,
AppCenter hopped out, Swift took the sway.
NaviLens now guides and settings unfold,
New locales and scripts in CI’s bold.
A little hop forward — eager to play!" 🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.28% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Log of Changes' is vague and does not convey meaningful information about the primary changes in this substantial pull request.	Use a more descriptive title that summarizes the main changes, such as 'Settings UI updates, auto unmute feature, and deduplication script migration' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

[coderabbitai]

Actionable comments posted: 20

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (8)

apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift (1)

391-407: ⚠️ Potential issue | 🔴 Critical

Add validation to prevent enterImmediateVicinityDistance from exceeding leaveImmediateVicinityDistance.

The removal of cross-coupling logic between these properties creates a risk. The geofence logic in DestinationManager.swift (lines 614–620) relies on enterImmediateVicinityDistance <= leaveImmediateVicinityDistance to function correctly:

• Exit occurs when distance >= leaveImmediateVicinityDistance
• Entry occurs when distance <= enterImmediateVicinityDistance

Without this constraint, invalid configurations are possible (e.g., enterImmediateVicinityDistance = 50m, leaveImmediateVicinityDistance = 30m), which could cause unexpected geofence transitions. Either enforce the invariant via validation in the setter or reintroduce the synchronized logic.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift` around lines 391
- 407, The two distance properties can become inconsistent; enforce the
invariant enterImmediateVicinityDistance <= leaveImmediateVicinityDistance in
the setters: in the setter for enterImmediateVicinityDistance (backed by
Keys.enterImmediateVicinityDistance) validate newValue and if it is greater than
the current leaveImmediateVicinityDistance either clamp newValue down to
leaveImmediateVicinityDistance or update leaveImmediateVicinityDistance
(Keys.leaveImmediateVicinityDistance) to newValue so the invariant holds;
similarly, in the setter for leaveImmediateVicinityDistance ensure the new leave
value is at least the current enterImmediateVicinityDistance (adjust/clamp the
other value or reject) so DestinationManager's expectation is always satisfied.

apps/ios/GuideDogs/Code/Visual UI/Views/Settings.storyboard (1)

784-805: ⚠️ Potential issue | 🟡 Minor

Resolve Auto Layout ambiguity in "Mix with Other Apps" cell.

The ambiguous="YES" attributes on the label, switch, and imageView indicate unresolved Auto Layout constraints that can cause unpredictable layout behavior across device sizes and with Dynamic Type enabled.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Settings.storyboard around lines 784
- 805, The label (id="VSq-h5-uRj"), switch (id="xAo-Ds-vYv") and imageView
(id="43Y-tn-4fp") are marked ambiguous; add explicit Auto Layout constraints to
remove ambiguity: constrain 43Y-tn-4fp to the cell leading and centerY with
fixed width/height (or aspect constraint) so its size is resolvable; constrain
VSq-h5-uRj centerY to the imageView and add a leading constraint from VSq-h5-uRj
to 43Y-tn-4fp with standard spacing and a trailing (<=) constraint from
VSq-h5-uRj to xAo-Ds-vYv; constrain xAo-Ds-vYv to the cell trailing and centerY
and set its contentHugging/contentCompressionResistance priorities (increase
horizontal hugging for the switch) so the label can truncate; remove or update
ambiguous="YES" once constraints are satisfied.

apps/ios/GuideDogs/Code/Visual UI/Helpers/Integrations.swift (1)

9-24: ⚠️ Potential issue | 🔴 Critical

Add the missing framework imports.

This file uses URL, UINavigationController, UIApplication, and location.distance(from:) without importing Foundation or UIKit. The file will fail to compile. Other files in the same directory all include explicit imports at the top (e.g., import Foundation).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Helpers/Integrations.swift around lines 9
- 24, The file is missing required framework imports causing unresolved symbols
used in launchNaviLens (e.g., URL, UIApplication, UINavigationController) and
any usage of location.distance(from:) on LocationDetail; add the appropriate
imports at the top of the file (import Foundation and import UIKit) so symbols
like URL, UIApplication, UINavigationController and distance(from:) are
available to AppContext, HomeViewController and LocationDetail.

apps/ios/GuideDogs.xcodeproj/project.pbxproj (1)

5539-5558: ⚠️ Potential issue | 🟠 Major

Confirm the iOS 15 floor is intentional.

Lines 5539, 5832, and 5893 raise the shipped app’s minimum OS from iOS 14.1 to iOS 15.0. That immediately drops iOS 14 devices from install/upgrade eligibility, so this needs an explicit product/release decision rather than a silent project-file change.

Also applies to: 5832-5851, 5893-5912

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs.xcodeproj/project.pbxproj` around lines 5539 - 5558, The
project file shows IPHONEOS_DEPLOYMENT_TARGET = 15.0 in multiple build
configurations (search for IPHONEOS_DEPLOYMENT_TARGET occurrences around the
changed blocks) which raises the app minimum OS from 14.1 to 15.0; confirm this
is an intentional product decision and if not revert those
IPHONEOS_DEPLOYMENT_TARGET entries back to 14.1 in the affected build
configurations (the three occurrences noted) or add a comment/PR note
documenting the release decision and upgrade rationale so reviewers know this
change is deliberate.

apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/StatusTableViewController.swift (1)

295-299: ⚠️ Potential issue | 🟡 Minor

Don't toggle automatic callouts from the cancel action.

This cancel handler reads reenableCalloutsAfterReload, which is controller state reused across previous cleanup attempts. If that flag is still true from an earlier run, tapping Cancel here can silently turn automatic callouts back on even though this sheet never disabled them.

Proposed fix

-        alert.addAction(UIAlertAction(title: GDLocalizedString("general.alert.cancel"), style: .cancel, handler: { [weak self] _ in
-            if self?.reenableCalloutsAfterReload ?? false {
-                SettingsContext.shared.automaticCalloutsEnabled = true
-            }
-        }))
+        alert.addAction(UIAlertAction(title: GDLocalizedString("general.alert.cancel"), style: .cancel, handler: nil))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/StatusTableViewController.swift around lines 295 - 299, The
cancel action in StatusTableViewController currently reads
reenableCalloutsAfterReload and may flip
SettingsContext.shared.automaticCalloutsEnabled on cancel; remove that behavior
so the Cancel handler does not modify automaticCalloutsEnabled or depend on
reenableCalloutsAfterReload. Update the alert.addAction for the .cancel action
to only dismiss/return without touching reenableCalloutsAfterReload or
SettingsContext.shared.automaticCalloutsEnabled (or explicitly reset
reenableCalloutsAfterReload earlier in the cleanup flow instead), ensuring only
the confirmed path alters automaticCalloutsEnabled.

apps/ios/Scripts/LocalizationLinter/main.swift (1)

736-766: ⚠️ Potential issue | 🟠 Major

hasMissingKeys never flips, so missing base keys still exit 0.

Line 739 only logs a warning, but nothing ever sets hasMissingKeys = true, so Lines 764-766 are unreachable. A file can reference a non-existent base localization key and the linter still succeeds.

Suggested fix

 var hasMissingKeys = false
 
 codeFiles.forEach { (codeFile) in
     codeFile.missingKeys(from: baseLanguageFile).forEach({ (key) in
-        printWarning("Missing translation: '\(codeFile.filename)' uses a localization key which is not found in the base language file (or the key format is invalid): \"\(key)\"")
+        printError("Missing translation: '\(codeFile.filename)' uses a localization key which is not found in the base language file (or the key format is invalid): \"\(key)\"")
+        hasMissingKeys = true
     })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/Scripts/LocalizationLinter/main.swift` around lines 736 - 766, The
variable hasMissingKeys is never set to true when missing base keys are found,
so the final exit(1) never runs; inside the codeFiles.forEach where you iterate
codeFile.missingKeys(from: baseLanguageFile) and call printWarning, set
hasMissingKeys = true (and also consider setting it in the .noTranslations case
of codeFile.checkDynamicKeys(from:) alongside didFail) so that the final if
hasMissingKeys { exit(1) } path can trigger.

apps/ios/GuideDogs/Code/Audio/DiscreteAudioPlayer.swift (1)

203-237: ⚠️ Potential issue | 🟠 Major

Keep wasPaused queue-confined in the reschedule callback.

This function still reads wasPaused off the player's serial queue at Line 224. That reopens the race this patch is trying to close: a render callback can observe stale pause state and dequeue from bufferQueue while resume logic is rebuilding it, which can drop a rescheduled buffer.

Proposed fix

         while let buffer = oldQueue.dequeue() {
             newQueue.enqueue(buffer)
             layers[layer].player.scheduleBuffer(buffer, completionCallbackType: .dataRendered) { [weak self] (_) in
                 guard let `self` = self else {
                     return
                 }
-                
-                // Only log completion and remove from the buffer queue if the audio actually played back
-                guard !self.wasPaused else {
-                    return
-                }
-                
                 self.queue.async {
+                    guard !self.wasPaused else {
+                        return
+                    }
                     _ = self.layerStates[layer].bufferQueue.dequeue()
                 }
             }
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Audio/DiscreteAudioPlayer.swift` around lines 203 -
237, schedulePendingBuffers(forChannel:) currently reads the shared property
wasPaused from inside the player's render callback, reintroducing a race; fix it
by snapshotting wasPaused on the existing serial queue before scheduling (e.g.
let wasPausedSnapshot = wasPaused) and capture that immutable snapshot in the
layers[layer].player.scheduleBuffer completion closure instead of reading
self.wasPaused, leaving other accesses to layerStates[layer].bufferQueue and
self.queue as-is so dequeue continues to run on the queue.

apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift (1)

124-157: ⚠️ Potential issue | 🟠 Major

The .callouts switch is incomplete and should reuse the helper method.

The inline switch rowType is missing cases for .safety and .intersection that now exist in CalloutsRow. The identifier ?? "default" pattern is redundant (emits a compiler warning, not an error), but more importantly, the stale switch logic should be replaced by calling configureCalloutCell(_:at:), which correctly handles all callout row types and serves as the single source of truth.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/SettingsViewController.swift around lines 124 - 157, The
.callouts case currently re-dequeues a CalloutSettingsCellView and contains a
stale inline switch missing .safety and .intersection and uses the redundant
identifier ?? "default"; replace that block by dequeuing the cell using the same
identifier pattern as above (use
SettingsViewController.cellIdentifiers[indexPath] to get identifier) cast to
CalloutSettingsCellView, set cell.delegate = self, then call
configureCalloutCell(cell, at: indexPath) instead of the inline switch so all
CalloutsRow cases are handled by the existing helper (remove the redundant
identifier ?? "default" usage and the inline switch over CalloutsRow).

🟡 Minor comments (16)

apps/ios/GuideDogs/Code/Visual UI/Controls/New Feature Announcement/NewFeaturesViewController.swift-116-116 (1)

116-116: ⚠️ Potential issue | 🟡 Minor

Localize the VoiceOver page announcement.

Line 116 hard-codes "of" into the accessibility string, so non-English locales will get a mixed-language announcement even though the visible count already uses a localized format on Line 144. Please build this from GDLocalizedString("settings.new_feature.num_of_num", ...) or a dedicated accessibility key instead.

♻️ Suggested change

             } else {
                 let total = self?.featurePages.count ?? index + 1
-                UIAccessibility.post(notification: UIAccessibility.Notification.screenChanged, argument: "\(index + 1) of \(total). \(vc.header ?? "")")
+                let pageText = GDLocalizedString("settings.new_feature.num_of_num", String(index + 1), String(total))
+                UIAccessibility.post(notification: UIAccessibility.Notification.screenChanged, argument: "\(pageText). \(vc.header ?? "")")
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Controls/New Feature
Announcement/NewFeaturesViewController.swift at line 116, The VoiceOver
announcement uses a hard-coded "of" in the UIAccessibility.post call
(UIAccessibility.post(notification:argument:)) which causes mixed-language
output; replace the string construction to use the localized format key (e.g.
GDLocalizedString("settings.new_feature.num_of_num", index + 1, total)) or a
specific accessibility localization key and then append the localized header
(vc.header) so the argument passed to UIAccessibility.post is fully localized;
update the code that builds the argument for UIAccessibility.post(notification:
UIAccessibility.Notification.screenChanged, argument: ...) to use the localized
format instead of the literal "of".

apps/ios/GuideDogs/Code/Visual UI/Helpers/Search/SearchResultsUpdater.swift-41-41 (1)

41-41: ⚠️ Potential issue | 🟡 Minor

Missing cancellation of previous search before starting a new one.

The localSearch instance is stored but never cancelled before reassignment. When a new search starts (lines 130, 175), the previous MKLocalSearch may still be in progress, leading to potential race conditions where stale callbacks fire unexpectedly.

🛡️ Proposed fix to cancel previous search

 class SearchResultsUpdater: NSObject {
     // ...
     private var localSearch: MKLocalSearch?
+    
+    private func cancelLocalSearch() {
+        localSearch?.cancel()
+        localSearch = nil
+    }

Then call cancelLocalSearch() before starting a new search in both partialSearchWithText and searchWithText, and also in deinit:

     deinit {
         searchRequestToken?.cancel()
+        localSearch?.cancel()
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Helpers/Search/SearchResultsUpdater.swift
at line 41, The stored MKLocalSearch instance (localSearch) isn't cancelled
before starting new searches, causing stale callbacks; implement a
cancelLocalSearch() helper that calls localSearch?.cancel() and sets localSearch
= nil, then invoke cancelLocalSearch() at the start of
partialSearchWithText(...) and searchWithText(...), and also call it from deinit
to ensure any in-flight MKLocalSearch is properly cancelled before reassignment
or teardown.

svcs/data/utilities/visualize_tiles_map.py-65-66 (1)

65-66: ⚠️ Potential issue | 🟡 Minor

Computed map center is unused; map defaults to hardcoded NYC coordinates.

center_lat and center_lon are computed from the data (lines 65-66) but the map is initialized with hardcoded NYC coordinates [40.7128, -74.0060] (line 84). This appears to be a bug—the map should center on the actual data.

Proposed fix

 # Create map
-m = folium.Map(location=[40.7128, -74.0060], zoom_start=4)
+m = folium.Map(location=[center_lat, center_lon], zoom_start=4)

Also applies to: 84-84

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/utilities/visualize_tiles_map.py` around lines 65 - 66, The
computed center_lat and center_lon are never used — replace the hardcoded NYC
coordinates when initializing the map with the computed values; update the map
creation in the function (where folium.Map or map = Map(...) is instantiated) to
use location=[center_lat, center_lon] (ensuring center_lat/center_lon are
computed beforehand) so the map centers on the actual data instead of [40.7128,
-74.0060].

svcs/data/non_osm_scripts/check_non_osm_ingested.py-47-48 (1)

47-48: ⚠️ Potential issue | 🟡 Minor

Add timeout to requests.get() to prevent indefinite hangs.

The HTTP request lacks a timeout parameter. If the tile server is unresponsive, the script will hang indefinitely. Adding a timeout improves reliability for CI/CD pipelines and interactive use.

Proposed fix

-        response = requests.get(url)
+        response = requests.get(url, timeout=30)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/non_osm_scripts/check_non_osm_ingested.py` around lines 47 - 48,
The GET call using requests.get(url) can hang indefinitely; update the request
to include a sensible timeout (e.g., requests.get(url, timeout=10)) and handle
timeout/network errors around that call (catch requests.exceptions.Timeout /
requests.exceptions.RequestException) so failures are surfaced instead of
blocking; adjust the logic that reads response.json() / assigns features to
handle the error path when the request fails.

svcs/data/utilities/visualize_tiles_map.py-30-33 (1)

30-33: ⚠️ Potential issue | 🟡 Minor

Positional arguments with defaults are still required; use optional flags instead.

The usage comment (lines 5-7) describes output, min_radius, and max_radius as optional, but they're defined as positional arguments. In argparse, positional arguments are always required regardless of default values. Use -- prefix to make them truly optional.

Proposed fix

 parser.add_argument("json_file", help="Path to tile log JSON file")
-parser.add_argument("output", default="tiles_map_v2.html", help="Output HTML map file name")
-parser.add_argument("min_radius", type=int, default=3, help="Minimum marker radius")
-parser.add_argument("max_radius", type=int, default=15, help="Maximum marker radius")
+parser.add_argument("--output", default="tiles_map_v2.html", help="Output HTML map file name")
+parser.add_argument("--min-radius", type=int, default=3, dest="min_radius", help="Minimum marker radius")
+parser.add_argument("--max-radius", type=int, default=15, dest="max_radius", help="Maximum marker radius")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/utilities/visualize_tiles_map.py` around lines 30 - 33, The three
parameters are currently added as positional arguments via
parser.add_argument("output", ...), parser.add_argument("min_radius", ...),
parser.add_argument("max_radius", ...) which makes them required despite having
defaults; change them to optional flags (e.g., parser.add_argument("--output",
"-o", default="tiles_map_v2.html", help=...),
parser.add_argument("--min-radius", type=int, default=3, help=...),
parser.add_argument("--max-radius", type=int, default=15, help=...)) so they
become truly optional and preserve types and defaults; update any code that
references args.min_radius/args.max_radius/args.output if you change names
(hyphenated flags map to args.min_radius etc.) and keep args =
parser.parse_args() unchanged.

apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift-242-270 (1)

242-270: ⚠️ Potential issue | 🟡 Minor

Same state ordering issue: state set after async call.

Similar to the start() method, state = .announcingCallout at line 270 is set after audioEngine.play() at line 242. If the completion handler fires synchronously, the state checks would evaluate against the previous state.

Consider moving the state assignment before the audioEngine.play() call for consistency and correctness.

Proposed fix

+        state = .announcingCallout
+        
         self.audioEngine.play(sounds) { (success) in
             calloutGroup.delegate?.calloutFinished(callout, completed: success)
             
             if self.state == .stopping {
                 GDLogVerbose(.stateMachine, "Callout interrupted. Stopping...")
                 self.complete()
                 calloutGroup.onComplete?(false)
                 return
             }
             
             if self.state == .off {
                 GDLogVerbose(.stateMachine, "Callouts immediately interrupted. Cleaning up...")
                 calloutGroup.onComplete?(false)
                 return
             }
             
             guard success else {
                 GDLogVerbose(.stateMachine, "Callout did not finish playing successfully. Terminating state machine...")
                 calloutGroup.onComplete?(false)
                 self.complete(failed: true)
                 return
             }
             
             self.eventDelayCalloutAnnounced()
         }
         
         CalloutStateMachine.log(callout: callout, context: self.calloutGroup?.logContext)
-        
-        state = .announcingCallout

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift` around
lines 242 - 270, The state assignment for announcing is performed after calling
audioEngine.play(...), which can synchronously invoke its completion handler and
read the old state; move the state = .announcingCallout line to immediately
before calling audioEngine.play(...) so the completion handler and any
early-return checks (e.g., self.state == .stopping / .off) observe the correct
state, keep the existing CalloutStateMachine.log(callout:context:) and
CalloutStateMachine.eventDelayCalloutAnnounced() calls in place, and ensure
calloutGroup.delegate?.calloutFinished(...) and calloutGroup.onComplete? are
left unchanged.

apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift-104-133 (1)

104-133: ⚠️ Potential issue | 🟡 Minor

State set after async call could cause race condition if completion fires synchronously.

The state = .playingPrefixSounds assignment at line 130 occurs after the audioEngine.play() call at line 105. If the completion handler fires synchronously (e.g., on error or zero-length audio), the state checks in lines 106-117 would evaluate against the previous state (.start) rather than .playingPrefixSounds.

Consider setting the state before initiating the async operation:

Proposed fix

         if sounds.count > 0 {
+            // Transition to the state to allow mode sounds and prefix sounds to play
+            self.state = .playingPrefixSounds
+            
             self.audioEngine.play(Sounds(sounds)) { (success) in
                 if self.state == .stopping {
                     GDLogVerbose(.stateMachine, "Callout interrupted. Stopping...")
                     self.complete()
                     callouts.onComplete?(false)
                     return
                 }
 
                 if self.state == .off {
                     GDLogVerbose(.stateMachine, "Callouts immediately interrupted. Cleaning up...")
                     callouts.onComplete?(false)
                     return
                 }
                 
                 guard success else {
                     GDLogVerbose(.stateMachine, "Callout did not finish playing successfully. Terminating state machine...")
                     self.complete(failed: true)
                     return
                 }
                 
                 GDLogVerbose(.stateMachine, "Enter mode sound played")
                 self.announceCallout()
             }
-            
-            // Transition to the state to allow mode sounds and prefix sounds to play
-            self.state = .playingPrefixSounds
         } else {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift` around
lines 104 - 133, The state transition to .playingPrefixSounds must occur before
starting the async play to avoid a race if the audioEngine.play completion is
invoked synchronously; move the assignment self.state = .playingPrefixSounds to
immediately before calling audioEngine.play(Sounds(sounds)) and keep the
existing completion handler logic (checks of self.state, calls to complete(),
callouts.onComplete?, and announceCallout()) unchanged so synchronous
completions observe the correct state.

apps/ios/GuideDogs/Code/App/Logging/GDATelemetry.swift-22-25 (1)

22-25: ⚠️ Potential issue | 🟡 Minor

Formatting issue: Excessive indentation on closing brace.

The closing brace on line 24 has inconsistent indentation compared to the rest of the code style.

         set {
             SettingsContext.shared.telemetryOptout = !newValue
-                    }
+        }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/App/Logging/GDATelemetry.swift` around lines 22 - 25,
The setter for the telemetryOptIn property has an incorrectly indented closing
brace; update the setter block (where SettingsContext.shared.telemetryOptout =
!newValue is assigned) so the closing brace aligns with the start of the setter
(remove the excessive indentation) to match surrounding code style and restore
consistent indentation for the set { ... } block.

apps/ios/GuideDogs/Code/App/Logging/GDATelemetry.swift-8-8 (1)

8-8: ⚠️ Potential issue | 🟡 Minor

Typo: "depricated" should be "deprecated".

-// This is just a stub, MS used AppCenter which is being depricated.
+// This is just a stub, MS used AppCenter which is being deprecated.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/App/Logging/GDATelemetry.swift` at line 8, Typo in
the top-of-file comment: replace the misspelled word "depricated" with the
correct spelling "deprecated" in the comment string in GDATelemetry.swift so the
comment reads "MS used AppCenter which is being deprecated."

apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift-329-338 (1)

329-338: ⚠️ Potential issue | 🟡 Minor

Remove unused property or integrate it if planned for future use.

The autoUnmuteEnabled property is defined but not used anywhere in the codebase. If this is newly added code awaiting integration, it should be connected to the actual functionality. If it's no longer needed, it should be removed.

Additionally, note that among audio-related settings, only beaconVolume posts a notification; audioSessionMixesWithOthers does not, so notifications are not universally used for this category of settings.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift` around lines 329
- 338, The property autoUnmuteEnabled in SettingsContext (backed by userDefaults
with Keys.autoUnmuteEnabled) is unused—either remove this property or wire it
into the audio flow; to fix, either delete the autoUnmuteEnabled getter/setter
from SettingsContext (and remove Keys.autoUnmuteEnabled) or integrate it by
updating relevant code paths that handle audio unmute behavior and mirror
beaconVolume's behavior by posting the same notification (or a new descriptive
Notification.Name) when setting autoUnmuteEnabled; also ensure consistency with
audioSessionMixesWithOthers so audio-related settings uniformly post
notifications if other listeners rely on them.

apps/ios/GuideDogs/Assets/Localization/nl-NL.lproj/Localizable.strings-4134-4134 (1)

4134-4134: ⚠️ Potential issue | 🟡 Minor

Replace the double-escaped paragraph breaks.

Line 4134 uses \\n\n between quoted testimonials. In a .strings value, this renders as a literal \n character before each paragraph break. These should be \n\n instead.

Proposed fix

- ... de verkeerde kant op liep.\\n\nZelfs in de stad waar ik al 3 jaar woon ...
+ ... de verkeerde kant op liep.\n\nZelfs in de stad waar ik al 3 jaar woon ...

- ... van wat er Om me heen is.\\n\nHet 3D-geluid geeft een betere beleving ...
+ ... van wat er Om me heen is.\n\nHet 3D-geluid geeft een betere beleving ...

- ... nu eerder een nieuwe route uitproberen.\\n\nAls je gewoon rondloopt ...
+ ... nu eerder een nieuwe route uitproberen.\n\nAls je gewoon rondloopt ...

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Assets/Localization/nl-NL.lproj/Localizable.strings` at
line 4134, The localized string value for "faq.when_to_use_soundscape.answer"
contains double-escaped paragraph breaks written as \\n\n which render a literal
backslash-n then newline; update the string so each intended paragraph break
uses the proper escape sequence \n\n (replace all occurrences of \\n\n with
\n\n) so the testimonials render as real paragraph breaks in the .strings value.

apps/ios/GuideDogs/Assets/Localization/ru.lproj/InfoPlist.strings-1-7 (1)

1-7: ⚠️ Potential issue | 🟡 Minor

Missing localization content.

This Russian InfoPlist.strings file contains only header comments but no actual localized strings. InfoPlist.strings typically includes translations for system permission prompts (e.g., NSLocationWhenInUseUsageDescription, NSMotionUsageDescription).

If this is intentional as a placeholder for future translations, consider adding a TODO comment. Otherwise, the missing translations will cause the app to fall back to the base language for these system prompts when the device is set to Russian.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Assets/Localization/ru.lproj/InfoPlist.strings` around
lines 1 - 7, The Russian InfoPlist.strings currently contains only header
comments and no localized keys; add the missing localized entries such as
"NSLocationWhenInUseUsageDescription",
"NSLocationAlwaysAndWhenInUseUsageDescription", "NSMotionUsageDescription", and
any other permission keys used by the app with their Russian translations, or if
this file is intentionally left empty add a clear TODO comment like "TODO: add
Russian translations for InfoPlist permission strings" so reviewers know it’s a
placeholder; update the file (InfoPlist.strings) to include these keys and
translated values or the TODO comment accordingly.

apps/ios/GuideDogs/Code/Visual UI/View Controllers/POI Table/SearchTableViewController.swift-355-359 (1)

355-359: ⚠️ Potential issue | 🟡 Minor

Misleading comment - copy-paste from beacon case.

Same issue as in MarkersAndRoutesListNavigationHelper.swift - the comment says "Set a beacon" but this is the NaviLens case.

📝 Suggested fix

 case .navilens:
-    // Set a beacon on the given location
-    // and segue to the home view
+    // Launch NaviLens navigation for the given location
+    // and return to the home view
     try guideToNaviLens(detail: detail)
     self.navigationController?.popToRootViewController(animated: true)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View Controllers/POI
Table/SearchTableViewController.swift around lines 355 - 359, The comment above
the .navilens case is incorrect (a copy-paste from the beacon case); update the
comment in SearchTableViewController.swift near the .navilens branch (where try
guideToNaviLens(detail: detail) and popToRootViewController are called) to
accurately describe the action (e.g., "Navigate to the given NaviLens location
and return to the home view" or similar) so it no longer mentions setting a
beacon.

apps/ios/GuideDogs/Code/Visual UI/Views/Markers & Routes/MarkersAndRoutesListNavigationHelper.swift-90-94 (1)

90-94: ⚠️ Potential issue | 🟡 Minor

Misleading comment - copy-paste from beacon case.

The comment says "Set a beacon on the given location" but this is the NaviLens case, not the beacon case. The comment should describe the NaviLens-specific behavior.

📝 Suggested fix

 case .navilens:
-    // Set a beacon on the given location
-    // and segue to the home view
+    // Launch NaviLens navigation for the given location
+    // and return to the home view
     try guideToNaviLens(detail: detail)
     self.popToRootViewController(animated: true)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Markers &
Routes/MarkersAndRoutesListNavigationHelper.swift around lines 90 - 94, The
comment above the case .navilens block is a leftover copy-paste referencing a
beacon; update it to describe the NaviLens behavior instead: replace the "Set a
beacon on the given location" text with a concise note that this branch calls
guideToNaviLens(detail:) to start navigation via NaviLens and then returns to
the home view by calling popToRootViewController(animated:). Ensure the comment
references the NaviLens action so it accurately documents the
guideToNaviLens(detail:) and popToRootViewController usage.

docs/ios-client/ReferenceEntity_model.md-15-15 (1)

15-15: ⚠️ Potential issue | 🟡 Minor

Contradictory information about temporary entity persistence.

Line 15 states isTemp = true entities are "Not persisted; used dynamically" in the Notes column, but the "Stored in Realm?" column shows "Yes". This is contradictory. If temporary points are not persisted, the column should indicate "No" or clarify that they are stored transiently in Realm but cleaned up.

📝 Suggested fix

-| **Temporary Point** | `ReferenceEntity` with `isTemp = true` | Yes | Hidden (used for beacons/navigation only) | Not persisted; used dynamically |
+| **Temporary Point** | `ReferenceEntity` with `isTemp = true` | Yes (transient) | Hidden (used for beacons/navigation only) | Stored temporarily; garbage collected when no longer in use |

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/ios-client/ReferenceEntity_model.md` at line 15, The table row for
ReferenceEntity with isTemp = true is contradictory: the "Stored in Realm?"
column says "Yes" while the Notes claim "Not persisted; used dynamically."
Update the row so the "Stored in Realm?" column either reads "No" or a clarified
value like "Transient (stored briefly, cleaned up)" and adjust the Notes to
match; ensure the change references the ReferenceEntity isTemp behavior and, if
choosing the transient option, add a short parenthetical explaining the cleanup
policy so both columns are consistent.

apps/ios/GuideDogs/Code/Visual UI/View Controllers/Settings/SettingsViewController.swift-78-86 (1)

78-86: ⚠️ Potential issue | 🟡 Minor

These new section descriptions need localization keys.

These literals bypass GDLocalizedString, so expanded sections will regress to English in every non-English locale. Please store keys here and resolve them when the footer text is rendered.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/SettingsViewController.swift around lines 78 - 86, The
sectionDescriptions dictionary currently contains hard-coded English literals;
replace each value with a localization key string (e.g.
"settings.section.general", "settings.section.audio", etc.) instead of
user-facing text, and ensure the UI resolves those keys via GDLocalizedString
when the footer/description is rendered (e.g., in the method that builds the
expanded section footer in SettingsViewController). Update the Section -> String
mapping (symbol: sectionDescriptions) to contain keys and change the rendering
code that reads sectionDescriptions to call GDLocalizedString(key) before
displaying.

🧹 Nitpick comments (13)

apps/ios/GuideDogs/Code/Visual UI/Helpers/Search/SearchResultsUpdater.swift (1)

209-209: Pre-existing force unwraps could cause crashes on incomplete placemark data.

While not introduced in this PR, the force unwraps on lat!, lon!, and name! are risky. MapKit placemarks can have nil values for location or name. Consider defensive handling.

♻️ Suggested safer approach

-                pois.append(GenericLocation(lat: lat!, lon: long!, name: result.name!, address: address))
+                if let lat = lat, let long = long, let name = result.name {
+                    pois.append(GenericLocation(lat: lat, lon: long, name: name, address: address))
+                }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Helpers/Search/SearchResultsUpdater.swift
at line 209, The line appending GenericLocation uses force unwraps (lat!, long!,
name!) which can crash when CLPlacemark data is incomplete; in
SearchResultsUpdater.swift replace this with defensive unwrapping by extracting
the coordinate via result.location?.coordinate.latitude/longitude and the name
via result.name, using a guard-let or if-let to skip/continue when any is nil
(or provide sensible defaults) before calling pois.append(GenericLocation(...));
ensure you reference the same GenericLocation initializer and avoid force
unwraps.

svcs/data/non_osm_scripts/find_duplicate_points.py (2)

6-6: Unused import: geodesic is imported but never used.

The geodesic function from geopy.distance is imported but not utilized anywhere in the script. Consider removing this unused import.

Proposed fix

-from geopy.distance import geodesic

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/non_osm_scripts/find_duplicate_points.py` at line 6, The import
geodesic from geopy.distance is unused; remove the "from geopy.distance import
geodesic" import statement in find_duplicate_points.py (ensure no other
references to geodesic remain in the file) and run the linter/unit tests to
confirm there are no missing dependencies.

---

42-49: Geographic distance approximation is latitude-dependent and may cause inaccurate matching.

The conversion 5 / 111320 assumes 1 degree ≈ 111.32 km, which is only accurate for latitude. Longitudinal degrees vary significantly by latitude (e.g., ~111 km at the equator but ~78 km at 45°N). Since the KDTree uses Euclidean distance on (lat, lon) pairs, this approximation could miss matches or create false positives depending on the geographic region.

For bus stop data typically within a single city/region, this may be acceptable, but consider documenting this limitation or using a proper geodesic distance calculation for more accurate results.

Alternative: Use BallTree with haversine metric for geodesic accuracy

from sklearn.neighbors import BallTree
import numpy as np

# Convert to radians for haversine
osm_coords_rad = np.radians(np.array(osm_stops))
navilens_coords_rad = np.radians(navilens_coords)

# Build BallTree with haversine metric
osm_tree = BallTree(osm_coords_rad, metric='haversine')

# Query with distance in radians (5 meters / Earth radius in meters)
threshold_rad = 5 / 6371000
distances, indices = osm_tree.query_radius(navilens_coords_rad, r=threshold_rad, return_distance=True)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/non_osm_scripts/find_duplicate_points.py` around lines 42 - 49, The
current fixed-degree conversion (distance_threshold_deg) is latitude-dependent
and can mislead osm_tree.query on (lat, lon) Euclidean coords; replace this with
a geodesic-aware approach: convert osm_stops and navilens_coords to radians,
rebuild osm_tree as a BallTree with metric='haversine', compute threshold_rad =
5 / 6371000, query the BallTree using that radius and then set
navilens_df["in_osm"] based on returned distances < threshold_rad (use osm_tree,
osm_stops, navilens_coords, distance threshold variable, and
navilens_df["in_osm"] from the diff to locate the changes). Ensure you remove
the 5/111320 conversion and update the query call accordingly (or if you decide
not to change logic, add a short comment near distance_threshold_deg documenting
the latitude-dependence limitation).

svcs/data/utilities/visualize_tiles_map.py (1)

56-57: Silently swallowing all exceptions hides parsing errors.

The bare except Exception: pass suppresses all errors during JSON parsing, making it difficult to diagnose malformed log entries. Consider logging a warning or at minimum catching only expected exceptions like json.JSONDecodeError and KeyError.

Proposed fix with specific exception handling

             try:
                 log = json.loads(buffer)
                 uri = log["uri"]
                 ts = log["ts"]
                 parts = uri.strip("/").split("/")
                 if len(parts) >= 4 and parts[0] == "tiles":
                     z = int(parts[1])
                     x = int(parts[2])
                     y = int(parts[3].replace(".json", ""))
                     lat, lon = num2deg(x, y, z)
                     rows.append({"ts": ts, "lat": lat, "lon": lon})
-            except Exception:
-                pass
+            except (json.JSONDecodeError, KeyError, ValueError):
+                pass  # Skip malformed or non-tile log entries
             buffer = ""

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/utilities/visualize_tiles_map.py` around lines 56 - 57, Replace the
bare "except Exception: pass" in visualize_tiles_map.py with targeted exception
handling: catch json.JSONDecodeError and KeyError (and optionally TypeError)
around the JSON parsing/lookup code (the try block that surrounds the JSON
load/field access), and emit a warning that includes the exception message and
the raw line/content that failed to parse (use the module logger or
logging.warning). This preserves other exceptions, surfaces parsing issues, and
provides context for debugging while still handling expected malformed entries.

svcs/data/non_osm_scripts/check_non_osm_ingested.py (2)

71-72: Consider using a standard exit code for failure.

Using sys.exit(-1) is unconventional. POSIX exit codes are typically 0-255, and -1 wraps to 255 on most systems. Using sys.exit(1) is more portable and conventional for indicating failure.

Proposed fix

             print("FAIL")
-            sys.exit(-1)
+            sys.exit(1)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/non_osm_scripts/check_non_osm_ingested.py` around lines 71 - 72,
The exit path currently calls sys.exit(-1) after printing "FAIL"; change this to
use a conventional nonzero exit code by replacing sys.exit(-1) with sys.exit(1)
(or raise SystemExit(1)) so the failure returns a portable POSIX-compatible
status; update the code around the print("FAIL") / sys.exit(-1) call to use
sys.exit(1) and run tests to confirm behavior.

---

52-52: Remove unused pprint import.

The pprint import inside the loop is unused (the pprint(feature) call is commented out). Either remove the import or move it to the top of the file if you plan to keep the debug code.

Proposed fix

         # Check that some feature in the tile matches our row
-        from pprint import pprint
         for feature in features:
-            `#pprint`(feature)
             if (

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/non_osm_scripts/check_non_osm_ingested.py` at line 52, Remove the
unused local import "from pprint import pprint" inside the loop: either delete
that import statement and any commented-out "pprint(feature)" call, or if you
want to keep the debug printing, move the "from pprint import pprint" to the
module top and re-enable the pprint usage; target the local import and the
commented pprint call near the loop that iterates over "feature".

apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift (2)

170-170: Minor formatting: missing space before brace.

Proposed fix

-        switch state{
+        switch state {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift` at line
170, In CalloutStateMachine.swift locate the switch statement written as "switch
state{" and fix the minor formatting by adding a space before the brace so it
reads "switch state {" (ensure you update any other similar occurrences in the
same file to match the project's Swift style).

---

104-104: Prefer !sounds.isEmpty over sounds.count > 0.

Swift idiom favors isEmpty for checking collection emptiness as it's more expressive and potentially more efficient for some collection types.

Proposed fix

-        if sounds.count > 0 {
+        if !sounds.isEmpty {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Behaviors/Helpers/CalloutStateMachine.swift` at line
104, Replace the explicit count check with Swift's idiomatic emptiness test: in
CalloutStateMachine (file contains the if using variable sounds), change the
condition from "sounds.count > 0" to use "!sounds.isEmpty" so the code reads a
clear, expressive emptiness check and matches Swift best practices.

apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift (1)

65-66: Minor: Extra blank line after key declaration.

There's an unnecessary blank line after the autoUnmuteEnabled key declaration. Consider removing for consistency with other key declarations in this struct.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/App/Settings/SettingsContext.swift` around lines 65 -
66, Remove the extra blank line immediately after the fileprivate static let
autoUnmuteEnabled = "GDAAutoUnmuteEnabled" declaration in the SettingsContext
struct so it matches the spacing of the other key declarations; simply delete
the blank line to keep consistent formatting around the autoUnmuteEnabled
constant.

apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/Soundscape.xcscheme (1)

106-112: Make Zombies opt-in instead of the shared default.

NSZombieEnabled=YES is useful when you’re actively chasing a use-after-free crash, but Apple positions Zombies as a targeted debugging aid you enable during investigation. Committing it in the shared scheme means every developer now runs that special diagnostic mode by default, which is better handled in a local or dedicated crash-debug scheme. (developer.apple.com)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/Soundscape.xcscheme`
around lines 106 - 112, The shared scheme currently enables NSZombies by default
(the <AdditionalOption key="NSZombieEnabled" value="YES" isEnabled="YES"> entry
in the Soundscape scheme's AdditionalOptions); remove or disable that
AdditionalOption from the shared xcscheme so Zombies are not on for all
developers and instead add it to a local/user scheme or a separate debug scheme
as an opt-in diagnostic. Locate the AdditionalOption with key "NSZombieEnabled"
in the Soundscape xcscheme and either delete it or set isEnabled="NO" there, and
if desired create a non-shared scheme (or add to the user-specific scheme) that
contains NSZombieEnabled=YES for targeted debugging.

apps/ios/GuideDogs/Code/Visual UI/View Controllers/Home/HomeViewController.swift (1)

127-131: Minor: Inconsistent capitalization in comment.

The comment uses lowercase "ios 26" while other files in this PR use "iOS 26". Consider updating for consistency:

Suggested fix

-        // Don't allow ios 26 to move the search to the bottom.
+        // Don't allow iOS 26 to move the search to the bottom.

Same iOS 26 API verification applies as noted in SearchWaypointViewController.swift.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Home/HomeViewController.swift around lines 127 - 131, Update the
comment above the iOS availability check in HomeViewController (the block
modifying navigationItem.searchBarPlacementAllowsToolbarIntegration) to use
consistent capitalization "iOS 26" instead of "ios 26" to match other files (see
SearchWaypointViewController.swift for the same check); only change the comment
text, leaving the conditional and property assignment unchanged.

apps/ios/GuideDogs/Code/Behaviors/Default/Callouts/DestinationCallout.swift (1)

94-99: Use a single localized key for this full spoken message.

This now stitches two translated fragments together with a hard-coded space, which makes the TTS harder to localize naturally in non-English locales. Prefer dedicated localized strings for the “muted only” and “muted + NaviLens suggestion” variants.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Behaviors/Default/Callouts/DestinationCallout.swift`
around lines 94 - 99, Replace the string concatenation in
DestinationCallout.swift (the variable text built from
GDLocalizedString("beacon.beacon_location_within_audio_beacon_muted",
formattedDistance) + optional " " +
GDLocalizedString("beacon.suggest_navilens")) with a single localized lookup
that returns the fully composed TTS sentence; detect the NaviLens case by
checking poi / LocationDetail(entity: poi!).source.hasNaviLens and call
GDLocalizedString with two distinct keys (e.g.
"beacon.beacon_location_within_audio_beacon_muted_only" and
"beacon.beacon_location_within_audio_beacon_muted_with_navilens"), passing
formattedDistance as any format argument, then pass that single result into
TTSSound(text, at: markerLocation).

apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM Entity/GDASpatialDataResultEntity+Typeable.swift (1)

80-83: Consider using SuperCategory enum for consistency.

isTransitStop() (line 46-51) uses SuperCategory(rawValue: superCategory) to leverage the enum for type-safe comparisons. The new isNaviLens() method uses a raw string comparison instead. For consistency and type safety, consider using the enum pattern.

♻️ Suggested refactor

 private func isNaviLens() -> Bool {
-    return superCategory == "navilens"
+    guard let category = SuperCategory(rawValue: superCategory) else {
+        return false
+    }
+    return category == .navilens
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Data/Models/Extensions/OSM`
Entity/GDASpatialDataResultEntity+Typeable.swift around lines 80 - 83, Replace
the raw-string check in isNaviLens() with the same enum-based pattern used in
isTransitStop(): convert the superCategory string to SuperCategory via
SuperCategory(rawValue: superCategory) and compare to the appropriate case
(e.g., .navilens) so the method uses the SuperCategory enum for type-safe
comparison; update the isNaviLens() implementation to mirror the style of
isTransitStop() for consistency.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

For a GitHub Actions workflow triggered by workflow_run, what ref/commit does actions/checkout@v4use by default, and how do you explicitly check outgithub.event.workflow_run.head_sha?

💡 Result:

By default, actions/checkout@v4 checks out the ref/SHA that triggered the current workflow run (i.e., effectively github.ref / github.sha), otherwise it falls back to the repo’s default branch. [1]
For workflows triggered by workflow_run, that typically means you’ll end up on the default branch commit for the workflow_run-triggered workflow, not the commit that the upstream workflow ran on (this is part of GitHub’s “run from default branch for certain events” behavior). [2]

To explicitly check out the triggering workflow’s commit, use the workflow_run payload’s head_sha (the workflow-run object includes head_sha). [3]

- name: Checkout the triggering commit
  uses: actions/checkout@v4
  with:
    ref: ${{ github.event.workflow_run.head_sha }}

Sources: [1] [2] [3]

---

🏁 Script executed:

cat -n .github/workflows/build-for-testflight.yml | head -30

Repository: soundscape-community/soundscape

Length of output: 930

---

Checkout should be pinned to the triggering SHA.

For workflow_run, this job builds the commit that passed ios-tests. Without an explicit ref, a later push to main will make this workflow archive and upload untested code instead.

Suggested fix

     - uses: actions/checkout@v4
       with:
+        ref: ${{ github.event_name == 'workflow_run' && github.event.workflow_run.head_sha || github.sha }}
         fetch-depth: 0
         persist-credentials: true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-for-testflight.yml around lines 21 - 24, The
checkout step is not pinned to the triggering commit SHA, so change the
actions/checkout@v4 invocation to explicitly checkout the workflow_run commit by
adding a ref under the with block (e.g., add ref: ${{
github.event.workflow_run.head_sha }}) so the job builds the exact commit that
passed ios-tests; update the checkout step where "uses: actions/checkout@v4" is
declared.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Is the xcprettygem preinstalled on GitHub-hostedmacos-14 runners, or does a workflow need to install it explicitly?

💡 Result:

xcpretty is not preinstalled on GitHub-hosted macos-14 runners. The official macos-14-arm64 runner image “Installed Software” list includes Xcbeautify but does not list xcpretty, so your workflow should install xcpretty explicitly (e.g., gem install xcpretty or via your Gemfile) if you depend on it. [1]

Sources:
[1] actions/runner-images – macos-14-arm64-Readme.md (Installed Software list) (github.com)

Citations:

• 1: https://github.com/actions/runner-images/blob/main/images/macos/macos-14-arm64-Readme.md

---

Install xcpretty before using it in the upload step.

This workflow pipes to xcpretty on line 61 but never installs it. xcpretty is not preinstalled on GitHub-hosted macos-14 runners. The step will fail with xcpretty: command not found. Compare with ios-tests.yml, which explicitly installs the gem before use.

Code snippet

    - name: build and upload to testflight
      env:
        APPSTORE_CONNECT_API_KEY: ${{ secrets.APP_STORE_P8_BASE64 }}
        APPSTORE_CONNECT_ISSUER_ID: ${{ secrets.APP_STORE_ISSUER_ID }}
        APPSTORE_CONNECT_KEY_ID: ${{ secrets.APP_STORE_KEY_ID }}
        KEYCHAIN_PASSWORD: ${{ steps.install-certificates.outputs.keychain-password }}
      run: "apps/ios/Scripts/ci/archive-upload.sh | xcpretty && exit ${PIPESTATUS[0]}"

Add a step to install xcpretty (e.g., gem install xcpretty) before this job runs.

🧰 Tools

🪛 actionlint (1.7.11)

[error] 61-61: shellcheck reported issue in this script: SC2086:info:1:58: Double quote to prevent globbing and word splitting

(shellcheck)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build-for-testflight.yml around lines 55 - 61, The "build
and upload to testflight" job pipes output to xcpretty but never installs it,
causing "xcpretty: command not found"; add a preceding step that installs the
gem (for example a step named "install xcpretty" that runs gem install xcpretty
--no-document) before the run that calls "apps/ios/Scripts/ci/archive-upload.sh
| xcpretty" so the xcpretty binary is available on the macos-14 runner.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

The new CI gate never turns on the missing-translation check.

main.swift only validates per-locale gaps when "missing" is present in CommandLine.arguments. As written, this step will still pass PRs with missing translations in *.lproj files.

Suggested fix

-                swift Scripts/LocalizationLinter/main.swift
+                swift Scripts/LocalizationLinter/main.swift missing

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	- name: Run LocalizationLinter
	working-directory: apps/ios
	run: |
	swift Scripts/LocalizationLinter/main.swift
	- name: Run LocalizationLinter
	working-directory: apps/ios
	run: |
	swift Scripts/LocalizationLinter/main.swift missing

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ios-tests.yml around lines 30 - 33, The CI step invoking
the LocalizationLinter doesn't enable the missing-translation check because
main.swift only performs per-locale gap validation when "missing" is present in
CommandLine.arguments; update the workflow step that runs Swift (the Run
LocalizationLinter step) to pass the "missing" argument (e.g., invoke main.swift
with the literal "missing" or the required flag) so that main.swift sees
CommandLine.arguments includes "missing" and turns on the missing-translation
check for the lproj files.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check if the file exists and examine its structure
if [ -f "apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/Soundscape.xcscheme" ]; then
  echo "=== File exists ==="
  # Get context around line 82
  sed -n '75,90p' "apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/Soundscape.xcscheme"
  echo -e "\n=== Full LaunchAction section ==="
  # Try to find the LaunchAction context
  sed -n '/<LaunchAction/,/<\/LaunchAction>/p' "apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/Soundscape.xcscheme" | head -40
else
  echo "File not found"
fi

Repository: soundscape-community/soundscape

Length of output: 2240

---

🏁 Script executed:

# Check if there are other schemes in the directory
ls -la "apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/" 2>/dev/null || echo "Directory not found"

Repository: soundscape-community/soundscape

Length of output: 593

---

🌐 Web query:

Thread Sanitizer iOS device support Apple WWDC

💡 Result:

Apple introduced Thread Sanitizer (TSan) for Xcode/iOS at WWDC 2016, and explicitly noted that TSan is not supported on physical iOS devices—it’s intended for use in the iOS Simulator (and other supported simulated environments). [1], [2]

Apple has expanded TSan’s simulator coverage over time (for example, Xcode 12 added TSan when debugging on simulated watchOS devices), but that does not change iOS on-device support. [3]

Bottom line: No—TSan does not support running on an iPhone/iPad device; use the iOS Simulator. [1], [2]

Sources:
[1] WWDC 2016 Session 412 (Thread Sanitizer and Static Analysis) – “It is not supported on the device.”
[2] Stack Overflow: “You can’t use TSan on a device” (Xcode scheme diagnostics / platform limits)
[3] Apple Xcode 12 Release Notes (TSan support for simulated watchOS)

---

🌐 Web query:

Xcode Thread Sanitizer platform support simulator device macOS

💡 Result:

• macOS apps: Thread Sanitizer (TSan) is supported when running your app on macOS. [1]
• iOS/tvOS: TSan is supported when running on the Simulator, but not supported on physical devices. [2]
• Architectures / platforms: In Xcode, TSan is supported for 64‑bit macOS and 64‑bit iOS/tvOS Simulators; watchOS isn’t supported. [3]

Sources:
[1] Apple WWDC18 “Understanding Crashes and Crash Logs” (TSan works on macOS and in the Simulator) [1] (developer.apple.com)
[2] Apple WWDC19 “Getting the Most Out of Simulator” (TSan supported in Simulator, not on devices) (developer.apple.com)
[3] Stack Overflow summary of Apple docs (64-bit macOS + 64-bit iOS/tvOS simulators; not device; no watchOS) (stackoverflow.com)

Citations:

• 1: https://developer.apple.com/videos/play/wwdc2018/414/?utm_source=openai
• 2: https://developer.apple.com/videos/play/wwdc2019/418/?utm_source=openai
• 3: https://stackoverflow.com/questions/39021334/what-is-required-to-make-the-thread-sanitizer-and-address-sanitizer-options-avai?utm_source=openai

---

Move Thread Sanitizer to a debug-only scheme.

This scheme is checked in under xcshareddata, so Run settings apply to all developers. Thread Sanitizer is not supported on physical iOS devices—only on the iOS Simulator and macOS. Enabling it here forces a simulator-only configuration on everyone in the shared default scheme. Use a dedicated debug scheme (e.g., Soundscape-Debug-TSan) instead, allowing contributors to opt in when needed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs.xcodeproj/xcshareddata/xcschemes/Soundscape.xcscheme` at
line 82, The shared Xcode scheme (Soundscape.xcscheme) currently has
enableThreadSanitizer = "YES" which forces simulator-only runs for all devs;
change the shared scheme to disable/remove enableThreadSanitizer (set to "NO" or
remove the key) so Run uses default settings for everyone, and create a separate
debug-only scheme (e.g., Soundscape-Debug-TSan) that contains
enableThreadSanitizer = "YES" so contributors can opt in; update the scheme
metadata to ensure the new Soundscape-Debug-TSan is the one checked in for
thread-sanitizer use and leave Soundscape.xcscheme free of TSan settings.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

The Terms checkbox hints are reversed.

When the checkbox is on, the action should be to untick it; when it is off, the action should be to tick it. The current strings announce the opposite action to VoiceOver users.

Suggested string swap

-"terms_of_use.accept_checkbox.on.acc_hint" = "Double tap to tick the \"Accept Terms of Use\" checkbox";
-"terms_of_use.accept_checkbox.off.acc_hint" = "Double tap to untick the \"Accept Terms of Use\" checkbox";
+"terms_of_use.accept_checkbox.on.acc_hint" = "Double tap to untick the \"Accept Terms of Use\" checkbox";
+"terms_of_use.accept_checkbox.off.acc_hint" = "Double tap to tick the \"Accept Terms of Use\" checkbox";

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"terms_of_use.accept_checkbox.on.acc_hint" = "Double tap to tick the \"Accept Terms of Use\" checkbox";
	/* */
	"terms_of_use.accept_checkbox.off.acc_hint" = "Double tap to untick the \"accept the terms of use\" checkbox.";
	"terms_of_use.accept_checkbox.off.acc_hint" = "Double tap to untick the \"Accept Terms of Use\" checkbox";
	"terms_of_use.accept_checkbox.on.acc_hint" = "Double tap to untick the \"Accept Terms of Use\" checkbox";
	/* */
	"terms_of_use.accept_checkbox.off.acc_hint" = "Double tap to tick the \"Accept Terms of Use\" checkbox";

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Assets/Localization/en-GB.lproj/Localizable.strings`
around lines 3432 - 3437, The accessibility hint strings for the Terms checkbox
are reversed: update the values for terms_of_use.accept_checkbox.on.acc_hint and
terms_of_use.accept_checkbox.off.acc_hint so the "on" key instructs users to
"Double tap to untick the \"Accept Terms of Use\" checkbox" and the "off" key
instructs users to "Double tap to tick the \"Accept Terms of Use\" checkbox";
locate and swap the string literals for those two keys in Localizable.strings
accordingly.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Can UITableViewHeaderFooterViewinstances be reused across multiple calls totableView(_:willDisplayHeaderView:forSection:), and is it safe to add gesture recognizers and subviews there unconditionally on every invocation?

💡 Result:

Yes. UITableViewHeaderFooterView is a reusable view type (it has a reuseIdentifier, is dequeued via dequeueReusableHeaderFooterView(withIdentifier:), and has prepareForReuse() specifically for recycling) and the same instance may be displayed, end displaying, and later be displayed again for the same or a different section as the table scrolls. [1]

Because of that, tableView(_:willDisplayHeaderView:forSection:) can be invoked multiple times for the same UIView instance (it’s called “prior to the display of a header view”, not “once per creation”), so it is not safe to unconditionally add subviews or gesture recognizers there on every call—doing so commonly results in duplicated subviews / multiple recognizers firing. [2][3]

Recommended pattern

• Add permanent subviews / gesture recognizers once in the header’s initializer (init(reuseIdentifier:)) / awakeFromNib, or guard with a one-time flag.
• Use willDisplayHeaderView only to configure (set text, colors, targets/state) for the current section.
• If you do attach anything that must be reset/removed between uses, do it in prepareForReuse(). [1]

Sources:
[1] WWDC “What’s New in Cocoa Touch” (UITableViewHeaderFooterView API list incl. reuseIdentifier, dequeue, prepareForReuse) (docs.huihoo.com)
[2] Microsoft (UIKit binding) docs: WillDisplayHeaderView “Called prior to the display of a header view for a section.” (learn.microsoft.com)
[3] Stack Overflow: willDisplayHeaderView may not be the first time you’ve received that message for that header view (reuse implication). (stackoverflow.com)

Citations:

• 1: https://docs.huihoo.com/apple/wwdc/2012/session_200__whats_new_in_cocoa_touch.pdf
• 2: https://learn.microsoft.com/en-us/dotnet/api/uikit.iuitableviewdelegate.willdisplayheaderview?view=net-ios-26.0-10.0&utm_source=openai
• 3: https://stackoverflow.com/questions/35046293/swift-table-section-header-duplicating-on-scroll?utm_source=openai

---

Don't append gesture recognizers and views on every display pass.

UITableViewHeaderFooterView is reused across multiple invocations of tableView(_:willDisplayHeaderView:forSection:), so unconditionally adding gesture recognizers, chevron views, and constraints here will accumulate them on the same header instance. This causes multiple recognizers to fire on a single tap and stacks duplicate accessories on top of each other.

Move permanent attachments (gesture recognizers, subviews) to the header's initializer or awakeFromNib(). Use willDisplayHeaderView() only to configure (update text, colors, state) for the current section. Clean up any non-permanent state in prepareForReuse().

Also applies to: 239-248

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/SettingsViewController.swift around lines 223 - 226, The
header view is accumulating gesture recognizers and accessory subviews because
header.addGestureRecognizer(...) and subview/constraint setup are being done in
tableView(_:willDisplayHeaderView:forSection:); move permanent setup into the
header class (e.g., override awakeFromNib() or the header's initializer) and add
the UITapGestureRecognizer(target:self, action:`#selector`(handleHeaderTap(_:)))
there, create and constrain the chevron/subviews once in that header class, and
use willDisplayHeaderView(_:) only to configure per-section state (text, colors,
expanded flag). Also implement prepareForReuse() on the custom
UITableViewHeaderFooterView to reset any transient state so duplicate
recognizers/subviews and stacked accessories are not created.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd 'StatusTableViewController.swift$' -x sed -n '176,323p' {}
echo '---'
rg -n -C2 'preferredStyle:\s*\.actionSheet|popoverPresentationController' --type swift

Repository: soundscape-community/soundscape

Length of output: 13273

---

Anchor this action sheet before presenting it on iPad.

UIAlertController with .actionSheet style crashes on iPad when popoverPresentationController is not configured. The deleteUserDataTouchUpInside() method presents an action sheet without setting a popover source. Since this is triggered from a table cell button, add a popover anchor to prevent the crash.

Proposed fix

-            cell.button.addTarget(self, action: `#selector`(deleteUserDataTouchUpInside), for: .touchUpInside)
+            cell.button.addTarget(self, action: `#selector`(deleteUserDataTouchUpInside(_:)), for: .touchUpInside)
@@
-    `@objc` func deleteUserDataTouchUpInside() {
+    `@objc` func deleteUserDataTouchUpInside(_ sender: UIButton) {
@@
         let alert = UIAlertController(title: GDLocalizedString("settings.clear_cache.markers.alert_title"),
                                       message: GDLocalizedString("settings.clear_cache.markers.alert_message"),
                                       preferredStyle: UIAlertController.Style.actionSheet)
+        if let popover = alert.popoverPresentationController {
+            popover.sourceView = sender
+            popover.sourceRect = sender.bounds
+        }
@@
         present(alert, animated: true, completion: nil)
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/View
Controllers/Settings/StatusTableViewController.swift around lines 176 - 185, The
action sheet presented by deleteUserDataTouchUpInside can crash on iPad because
its UIAlertController (style .actionSheet) has no popoverPresentationController
anchor; update the presentation to set popoverPresentationController?.sourceView
and sourceRect (or barButtonItem) anchored to the originating button from the
cell (the ButtonTableViewCell.button used in Section.userData), e.g. obtain the
button (or the cell) in deleteUserDataTouchUpInside, unwrap
popoverPresentationController safely, set sourceView = button and sourceRect =
button.bounds and optionally permittedArrowDirections, then present the alert;
this ensures the action sheet is anchored on iPad and prevents the crash.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the new localization keys for intersection callouts exist
echo "Checking for callouts.intersection key:"
rg -l 'callouts\.intersection[^.]' --type-add 'strings:*.strings' --type strings

echo ""
echo "Checking for callouts.intersection.info key:"
rg -l 'callouts\.intersection\.info' --type-add 'strings:*.strings' --type strings

Repository: soundscape-community/soundscape

Length of output: 1438

---

🏁 Script executed:

#!/bin/bash
# Check the intersection-related keys in a few localization files
echo "=== Checking en-US.lproj ==="
rg 'callouts\.intersection' apps/ios/GuideDogs/Assets/Localization/en-US.lproj/Localizable.strings -A 1

echo ""
echo "=== Checking fr-FR.lproj (French) ==="
rg 'callouts\.intersection' apps/ios/GuideDogs/Assets/Localization/fr-FR.lproj/Localizable.strings -A 1

echo ""
echo "=== Checking de-DE.lproj (German) ==="
rg 'callouts\.intersection' apps/ios/GuideDogs/Assets/Localization/de-DE.lproj/Localizable.strings -A 1

echo ""
echo "=== Checking ja-JP.lproj (Japanese) ==="
rg 'callouts\.intersection' apps/ios/GuideDogs/Assets/Localization/ja-JP.lproj/Localizable.strings -A 1

Repository: soundscape-community/soundscape

Length of output: 576

---

Add localization keys for the new Intersections cell to all supported language files.

The cell uses callouts.intersection and callouts.intersection.info as localization keys, but these are only defined in en-US.lproj/Localizable.strings. All other 17+ supported language files (French, German, Spanish, Japanese, etc.) are missing these keys, causing the UI to display English text as a fallback for non-English locales.

Add translations for both keys to all language-specific Localizable.strings files before merging.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Settings.storyboard around lines 317
- 364, The new Intersections table cell uses the localization keys
callouts.intersection and callouts.intersection.info which currently exist only
in English; add these two keys with appropriate translated strings to every
supported language-specific Localizable.strings file (one entry per language,
preserving the exact keys) so non-English locales no longer fall back to
English—after adding, run your usual localization/strings validation (or build)
to confirm no missing-key warnings.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd 'GuideDogs\.(xcworkspace|xcodeproj)$' apps/ios
rg -n 'GuideDogs\.xcworkspace|GuideDogs\.xcodeproj' .github/workflows apps/ios/Scripts/ci

Repository: soundscape-community/soundscape

Length of output: 476

---

🏁 Script executed:

sed -n '1,50p' apps/ios/Scripts/ci/archive-upload.sh

Repository: soundscape-community/soundscape

Length of output: 2014

---

🏁 Script executed:

# Check if CocoaPods or other workspace management is in use
ls -la apps/ios/GuideDogs.xcworkspace/
cat apps/ios/GuideDogs.xcworkspace/contents.xcworkspacedata 2>/dev/null | head -20
ls -la apps/ios/ | grep -i podfile
rg -l "pod " apps/ios/Podfile* 2>/dev/null | head -5

Repository: soundscape-community/soundscape

Length of output: 623

---

Archive uses the project while CI tests the workspace.

The test workflow builds apps/ios/GuideDogs.xcworkspace, but this release script uses GuideDogs.xcodeproj instead. The TestFlight artifact is built from a different Xcode container than the one that passed CI testing, which can lead to divergent behavior if the workspace has different scheme configurations or build settings.

Suggested fix

-APP_PROJECT="$SCRIPTDIR/../../GuideDogs.xcodeproj"
+APP_WORKSPACE="$SCRIPTDIR/../../GuideDogs.xcworkspace"
@@
 xcodebuild archive  \
-    -project "$APP_PROJECT" \
+    -workspace "$APP_WORKSPACE" \
     -scheme "$APP_SCHEME" \

Also applies to: 41-48

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/Scripts/ci/archive-upload.sh` around lines 13 - 15, The script is
using APP_PROJECT="$SCRIPTDIR/../../GuideDogs.xcodeproj" which mismatches CI
(which builds the workspace); update the script to reference the workspace
(e.g., set APP_WORKSPACE="$SCRIPTDIR/../../GuideDogs.xcworkspace") and ensure
subsequent use of APP_PROJECT/APP_ARCHIVE_PATH or build commands (where
APP_PROJECT or xcodebuild is invoked) are changed to pass -workspace
"$APP_WORKSPACE" and -scheme "$APP_SCHEME" (or otherwise replace APP_PROJECT
usage) so the archive step uses the same Xcode workspace as tests.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Compare the two files to verify they are duplicates
diff svcs/data/non_osm_scripts/find_duplicate_points.py svcs/data/non_osm_scripts/script.py
echo "Exit code: $? (0 = identical)"

Repository: soundscape-community/soundscape

Length of output: 103

---

Remove duplicate file script.py.

script.py is identical to find_duplicate_points.py. Maintaining duplicate files creates unnecessary maintenance burden and risk of divergent changes over time.

Remove script.py and keep find_duplicate_points.py as the single source of truth for this deduplication logic.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@svcs/data/non_osm_scripts/script.py` around lines 1 - 67, Delete the
duplicate module script.py and retain find_duplicate_points.py as the single
implementation of the deduplication logic; remove script.py from the repo and
any packaging/CI manifests referencing it, and verify that any imports, CLI docs
or tests that referenced script.py now point to find_duplicate_points.py (or its
CLI entry) so there are no dangling references.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

apps/ios/GuideDogs/Code/Visual UI/Views/Settings.storyboard (1)

784-805: Resolve the ambiguous layout flags in mixAudio subviews.

Lines 784, 794, and 804 mark the label, switch, and imageView as ambiguous="YES", indicating incomplete layout constraints. With the label configured for Dynamic Type (adjustsFontForContentSizeCategory="YES") and variable line wrapping (numberOfLines="0"), these views will break under longer localized strings or larger accessibility font sizes. The Swift implementation does not add runtime constraints to compensate.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Settings.storyboard around lines 784
- 805, The label, switch and imageView in the mixAudio row are left with
ambiguous="YES" (ids VSq-h5-uRj, xAo-Ds-vYv, 43Y-tn-4fp) so they break under
long/localized strings and large Dynamic Type; fix by adding explicit Auto
Layout constraints in the Settings.storyboard for the "mixAudio" cell: constrain
the imageView (43Y-tn-4fp) to a fixed width/height and leading margin and
centerY, constrain the label (VSq-h5-uRj) leading to the imageView trailing,
trailing to the switch leading with a standard spacing and centerY alignment,
and constrain the switch (xAo-Ds-vYv) to a fixed trailing margin and centerY;
also set appropriate contentCompressionResistance and contentHugging priorities
on the label and switch so the switch keeps its size and the label wraps (keep
numberOfLines="0" and adjustsFontForContentSizeCategory="YES"), then verify
ambiguous becomes NO and remove any runtime reliance on onSettingValueChanged:
to add constraints.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Settings.storyboard:
- Line 1187: The storyboard has a duplicated localization key: change the
userDefinedRuntimeAttribute with keyPath "localization" and value
"settings.language.screen_title.2" to use the unified key
"settings.language.screen_title" so both occurrences reference the same
translation key; update the attribute value in the Settings.storyboard element
that currently contains "settings.language.screen_title.2" to
"settings.language.screen_title" to ensure consistent localization and fallback
behavior.

---

Nitpick comments:
In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Settings.storyboard:
- Around line 784-805: The label, switch and imageView in the mixAudio row are
left with ambiguous="YES" (ids VSq-h5-uRj, xAo-Ds-vYv, 43Y-tn-4fp) so they break
under long/localized strings and large Dynamic Type; fix by adding explicit Auto
Layout constraints in the Settings.storyboard for the "mixAudio" cell: constrain
the imageView (43Y-tn-4fp) to a fixed width/height and leading margin and
centerY, constrain the label (VSq-h5-uRj) leading to the imageView trailing,
trailing to the switch leading with a standard spacing and centerY alignment,
and constrain the switch (xAo-Ds-vYv) to a fixed trailing margin and centerY;
also set appropriate contentCompressionResistance and contentHugging priorities
on the label and switch so the switch keeps its size and the label wraps (keep
numberOfLines="0" and adjustsFontForContentSizeCategory="YES"), then verify
ambiguous becomes NO and remove any runtime reliance on onSettingValueChanged:
to add constraints.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 47cd8288-fcd5-4493-90fb-a3ae45cbb2f3

📥 Commits

Reviewing files that changed from the base of the PR and between b4fc4e7 and 9526c5b.

📒 Files selected for processing (1)

• apps/ios/GuideDogs/Code/Visual UI/Views/Settings.storyboard

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unify the language-screen localization key.

Line 1187 still references settings.language.screen_title.2 while Line 39 now uses settings.language.screen_title. Using two keys for the same title path can cause inconsistent translations/fallback behavior.

🔧 Proposed fix

-                        <userDefinedRuntimeAttribute type="string" keyPath="localization" value="settings.language.screen_title.2"/>
+                        <userDefinedRuntimeAttribute type="string" keyPath="localization" value="settings.language.screen_title"/>

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	<userDefinedRuntimeAttribute type="string" keyPath="localization" value="settings.language.screen_title.2"/>
	<userDefinedRuntimeAttribute type="string" keyPath="localization" value="settings.language.screen_title"/>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/ios/GuideDogs/Code/Visual` UI/Views/Settings.storyboard at line 1187,
The storyboard has a duplicated localization key: change the
userDefinedRuntimeAttribute with keyPath "localization" and value
"settings.language.screen_title.2" to use the unified key
"settings.language.screen_title" so both occurrences reference the same
translation key; update the attribute value in the Settings.storyboard element
that currently contains "settings.language.screen_title.2" to
"settings.language.screen_title" to ensure consistent localization and fallback
behavior.