From 7b1df353cb2dde171bd78c31d337e0eea8b5425d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 22:19:01 +0000 Subject: [PATCH 1/3] Remove Swift Package CI workflow that can never pass swift.yml runs 'swift build' / 'swift test', which require a Package.swift. This repo is an Xcode app project with no Swift package, so the workflow has failed with 'Could not find Package.swift' on every run since it was added, marking every push and PR with a failing check. The Build Debug IPA workflow already compiles the project via xcodebuild on the same triggers, so build validation is unaffected. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013DNqCsHShA4yNqgZ6ZZQy3 --- .github/workflows/swift.yml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 .github/workflows/swift.yml diff --git a/.github/workflows/swift.yml b/.github/workflows/swift.yml deleted file mode 100644 index 21ae770..0000000 --- a/.github/workflows/swift.yml +++ /dev/null @@ -1,22 +0,0 @@ -# This workflow will build a Swift project -# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-swift - -name: Swift - -on: - push: - branches: [ "main" ] - pull_request: - branches: [ "main" ] - -jobs: - build: - - runs-on: macos-latest - - steps: - - uses: actions/checkout@v4 - - name: Build - run: swift build -v - - name: Run tests - run: swift test -v From e11a6062d11f490bf8f5340efbdb4712f64885af Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 22:26:11 +0000 Subject: [PATCH 2/3] Index long rural road segments instead of dropping them The route-speed spatial index skipped any OSM way segment whose bounding box spanned more than 8 cells (~3.5 km). Sparsely-noded rural highways can legitimately run much farther between geometry nodes, so those stretches were dropped from the index and silently fell back to average route pacing instead of their real speed limit. Raise the guard to 128 cells (~57 km at the equator), which still catches genuinely broken data such as antimeridian jumps (~90,000 cells) while keeping realistic long segments in the index. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013DNqCsHShA4yNqgZ6ZZQy3 --- StikDebug/Views/MapSelectionView.swift | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/StikDebug/Views/MapSelectionView.swift b/StikDebug/Views/MapSelectionView.swift index 54f9009..d4f54f7 100644 --- a/StikDebug/Views/MapSelectionView.swift +++ b/StikDebug/Views/MapSelectionView.swift @@ -579,10 +579,16 @@ private struct RouteSpeedIndex { let maxX = Int(floor(max(wayStart.longitude, wayEnd.longitude) / cellSize)) let minY = Int(floor(min(wayStart.latitude, wayEnd.latitude) / cellSize)) let maxY = Int(floor(max(wayStart.latitude, wayEnd.latitude) / cellSize)) - // A real road segment spans at most a couple of cells; a huge - // span means broken data (e.g. antimeridian jump) — skip it - // rather than flooding the index. - guard maxX - minX <= 8, maxY - minY <= 8 else { continue } + // Most road segments span a cell or two, but sparsely-noded + // rural highways can legitimately run tens of km between OSM + // geometry points, so allow a generous span (~57 km at the + // equator). Anything larger is broken data — e.g. an + // antimeridian jump spans ~90,000 cells — and is skipped rather + // than flooding the index. The earlier ≤8-cell cap (~3.5 km) + // silently dropped those long segments, so those stretches lost + // their real speed limit and fell back to average pacing. + let maxCellSpan = 128 + guard maxX - minX <= maxCellSpan, maxY - minY <= maxCellSpan else { continue } for x in minX...maxX { for y in minY...maxY { waySegments[SpatialCellKey(x: x, y: y), default: []].append(segment) From b394334d8ceed7fdc5c2a6839e2eaf9fa8089af2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 22:27:16 +0000 Subject: [PATCH 3/3] Add fork code-review report Documents the review of all fork changes since upstream divergence, the two defects fixed (broken swift.yml CI workflow, over-aggressive speed-index segment cap), and the areas verified as correct. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013DNqCsHShA4yNqgZ6ZZQy3 --- CODE_REVIEW_REPORT.md | 151 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 CODE_REVIEW_REPORT.md diff --git a/CODE_REVIEW_REPORT.md b/CODE_REVIEW_REPORT.md new file mode 100644 index 0000000..a540e8b --- /dev/null +++ b/CODE_REVIEW_REPORT.md @@ -0,0 +1,151 @@ +# Fork Code-Review Report — StikDebug-PLUS + +**Date:** 2026-07-14 +**Branch reviewed:** `claude/fork-code-review-33fkom` +**Scope:** All fork-specific changes since divergence from upstream StikDebug (commit `c5b23e2`). + +## Summary + +The fork adds four feature areas on top of upstream StikDebug: a location-simulator +overhaul (speed profiles, bus stops, map styles, long-route performance), a +cellular/Wi-Fi network path monitor with tunnel auto-reconnect, background +keep-alive improvements for debug sessions, and an experimental "hold app alive +in background" mode. 15 files changed, ~1,400 lines added. + +I reviewed the full diff against upstream, cross-checked every new FFI call +against `idevice.h`, traced the state machines in the changed managers, and +confirmed build status via GitHub Actions history. **Two genuine defects were +found and fixed; both are pushed to the review branch.** No crashes, compile +errors, or logic bugs remain in the reviewed code. + +--- + +## Defects fixed + +### 1. Always-failing CI workflow — `.github/workflows/swift.yml` (removed) + +**Severity:** Medium (broken CI signal on every push/PR) + +The workflow ran `swift build` / `swift test`, which require a `Package.swift`. +This repository is an Xcode app project with no Swift package, so the job failed +with `error: Could not find Package.swift` on **every run since it was added**, +marking each push and PR with a red ❌ check. + +The existing **Build Debug IPA** workflow already compiles the project with +`xcodebuild` on the same `push`/`pull_request` triggers and passes green, so +build validation is fully covered without it. + +- **Fix:** Deleted `swift.yml` (commit `7b1df35`). +- **Evidence:** GitHub Actions run `29341384965` (job `87113733956`) — + `Could not find Package.swift in this directory or any of its parent directories`. + +### 2. Long rural road segments dropped from the speed index — `MapSelectionView.swift` (fixed) + +**Severity:** Low (accuracy regression on long routes; no crash) + +`RouteSpeedIndex` bucketed each OSM way segment into ~440 m grid cells, but +skipped any segment whose bounding box spanned more than **8 cells (~3.5 km)**: + +```swift +guard maxX - minX <= 8, maxY - minY <= 8 else { continue } +``` + +Sparsely-noded rural highways can legitimately run tens of km between OSM +geometry nodes. Those segments were silently dropped from the index, so those +stretches lost their real speed limit and fell back to average-route pacing — +a behavior regression versus upstream, whose (slower) implementation scanned +every segment without this cap. + +- **Fix:** Raised the guard to **128 cells (~57 km at the equator)** (commit + `e11a606`). This still catches genuinely broken data such as antimeridian + jumps (which span ~90,000 cells) while keeping realistic long segments + indexed. Per-segment registration stays bounded, and the `nearestWayThreshold` + (40 m) distance filter means indexing extra cells never produces false matches. + +--- + +## Areas verified as correct (no change needed) + +### Build / compilation +- **Build Debug IPA is green** on the fork tip (`346bff6`), confirming the app + compiles and that the `xcode-version: 26.6.0` pin resolves on the runner. +- `packet.count` is correctly converted to `UInt` for the `uintptr_t` + `debug_proxy_send_raw` parameter; `fetchRouteSpeedContext` returns a + `RouteSpeedContext` on all paths (both were prior build-fix commits and hold up). + +### Experimental "hold app alive" / JIT keep-alive (`JITEnableContext`, `BackgroundAliveManager`, `DebugKeepAliveLease`) +- Every FFI call matches `idevice.h`: `process_control_launch_app` argument + order and `bool` flags, `process_control_disable_memory_limit`, and + `debug_proxy_send_raw(handle, ptr, uintptr_t)`. +- The hand-built RSP continue packet `$c#63` has the correct payload and + checksum (`0x63 % 256` → `63`), and is valid in no-ack mode. +- The hold sequence mirrors the proven `debugApp` path: drain acks → + `QStartNoAckMode` → disable ack mode → `vAttach` → continue → poll for + cancellation → interrupt (`0x03`) → detach (`D`). +- `HoldToken` is lock-guarded; `BackgroundAliveManager` balances start/stop and + its `stillCurrent` check prevents a late-finishing session from clobbering a + newer one. `DebugKeepAliveLease()` auto-activates in `init`, so the bare + construction in both `BackgroundAliveManager` and `HomeView` is correct. +- Background-task renewal re-checks `isActive` before re-arming, so an expiring + UIKit assertion no longer tears down long sessions. + +### Background audio / location (`BackgroundAudioManager`, `BackgroundLocationManager`) +- The new `force:` (session) vs. unforced (user-toggle) activity counters are + balanced on every start/stop path and never go negative (`max(… - 1, 0)`). +- `refreshRunningState()` correctly honors forced holds regardless of the user's + keep-alive toggles, and `refreshFromSettings()` re-evaluates on toggle change. +- The non-zero silent-audio fill is inaudible (~-80 dBFS, DC-free) and guarded + by `!format.isInterleaved` with the correct channel/frame iteration; the + interleaved branch is an unreachable fallback on the standard mixer format. + +### Network path monitor & tunnel reconnect (`NetworkPathMonitor`, `TunnelManager`) +- Path state is lock-guarded; change notifications are deduplicated by signature. +- Tunnel retry (3 attempts, 1s→2s backoff) correctly short-circuits on permanent + errors (invalid/expired/missing pairing `-9/-17`, bad target IP `-18`, parse + failures) instead of pointlessly retrying user-actionable failures. +- The reconnect observer is registered on the main queue, matching `start()`'s + main-thread expectation; the debounced work item cancels prior pending work. + +### Location simulator — speed profiles, bus stops, map styles (`MapSelectionView`) +- The Overpass query moved from a bounding-box GET to a corridor `around` POST; + the body is percent-encoded with a form-safe character set and the client + timeout (30 s) exceeds the server timeout (25 s). +- `OverpassResponse.Element` decodes both way `geometry` and node `lat`/`lon`, + which `out tags geom;` emits for nodes — bus-stop parsing is sound. +- Bus-stop detection covers both the legacy `highway=bus_stop` and the modern + `public_transport=platform/stop_position` + `bus=yes` schemas. +- Route state (`routeBusStops`, `isImportedRoute`, `lastFallbackSpeed`, + `routeRequestID`) is reset consistently across import / clear / reset / refresh, + and every async prefetch guards on `routeRequestID` before applying results. +- Profile switching re-plans directions only when walking-vs-driving changes and + the route is searched (not imported); otherwise it rebuilds pacing in place. +- All SwiftUI/MapKit symbols are valid for the iOS 17.4 deployment target + (`MapStyle`, `PointOfInterestCategories`, `Marker`, `.mapStyle`). + +--- + +## Minor observations (intentional trade-offs — not changed) + +- **Bus ETA** in the route summary uses MapKit's automobile travel time and does + not add the per-stop dwell time, so a bus route's ETA is slightly optimistic. +- **Bus stop dwell** (12 s) is applied to the sample nearest the stop (≤25 m), + so playback pauses just before arriving rather than exactly at the stop. +- **`build_ipa.yml`** has two stray blank lines from an earlier edit; harmless + (YAML ignores them). +- **Bus-stop cell lookup** assumes non-polar latitudes (documented); above ~80°N + a 90 m search radius could exceed a longitude cell. Not reachable in practice. +- **`keepAppAlive`** blocks a global-queue thread for the hold duration and opens + multiple concurrent debug tunnels; acceptable for an explicitly experimental + feature, but worth revisiting if it graduates out of experimental. + +--- + +## Commits on the review branch + +| Commit | Change | +|--------|--------| +| `7b1df35` | Remove Swift Package CI workflow that can never pass | +| `e11a606` | Index long rural road segments instead of dropping them | + +Both are pushed to `origin/claude/fork-code-review-33fkom`. The Build Debug IPA +workflow will validate compilation on the branch tip.