fix(import): rethrow cancellation in Android CSV import by 9thLevelSoftware · Pull Request #1355 · 9thLevelSoftware/Project-Phoenix-MP

9thLevelSoftware · 2026-10-27T08:32:29Z

Summary

Android CSV import catches Exception, and CancellationException is an Exception. On current main (622e6033; the audit was against 525e5bc7) a cancelled import kept going:

A cancelled saveSession was stored as Failed to save session (...): ... and later rows were still written.
A cancelled file read came back as Import failed: ... instead of stopping.
The analytics import caller then opened the import-result dialog for that failure.

AndroidCsvImporter now rethrows cancellation from both catches with rethrowIfCancellation(), so no further rows are written. The analytics caller rethrows it too, so leaving the screen does not show an import-failed message. A real read or save failure still returns CsvImportResult as before.

AnalyticsScreen.kt is also in open PR #1326, which only passes polarBleManager into the workout tab. This change is limited to the CSV import catch. It does not touch the Polar files or drafts #1347, #1348, and #1353.

Test

./gradlew -Pskip.supabase.check=true :shared:testAndroidHostTest --tests 'com.devil.phoenixproject.util.AndroidCsvImporterCancellationTest'

Before the fix the two cancellation tests returned a result (imported=2 with Failed to save session (Squat): import scope cancelled, and Import failed: left screen). After the fix those tests throw CancellationException and do not write later rows.

Full host suite after the fix:

./gradlew -Pskip.supabase.check=true :shared:testAndroidHostTest

518 suites, 5346 tests, 0 failures, 0 errors, 0 skipped. BUILD SUCCESSFUL.

CancellationException extends Exception, so the Android CSV importer treated a cancelled import as a failed row and kept writing. Rethrow it from both catches and from the analytics import caller. Co-authored-by: Devil <9thLevelSoftware@users.noreply.github.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

kilo-code-bot · 2026-10-28T16:07:14Z

                        importResult = result
                        showImportResultDialog = true
                    } catch (e: Exception) {
+                        e.rethrowIfCancellation()


WARNING: This rethrow never fires on iOS - CsvImporter.ios.kt still swallows the cancellation

AnalyticsScreen lives in phoneMain, so it is the caller on both platforms, but the iOS importer keeps the two catch (e: Exception) blocks this PR is fixing on Android:

shared/src/iosMain/kotlin/com/devil/phoenixproject/util/CsvImporter.ios.kt:72 records the cancelled row as a Failed to save session (...) error and keeps writing the remaining rows.

shared/src/iosMain/kotlin/com/devil/phoenixproject/util/CsvImporter.ios.kt:93 turns a cancelled read into a CsvImportResult whose only error is Import failed: ....

Because the iOS importer returns a result instead of throwing, this line never sees a CancellationException on iOS, so a cancelled iOS import still opens the import-failed dialog. Add e.rethrowIfCancellation() to both iOS catches (identical to CsvImporter.android.kt:73 and :95) so the two platforms behave the same.

kilo-code-bot · 2026-10-28T16:07:14Z

+        2026-03-12,09:00:00,Row,OldSchool,8,0,8,8,40.0,0,30,No,100
+    """.trimIndent().byteInputStream()
+
+    private class ThrowingStream(private val error: Exception) : InputStream() {


SUGGESTION: ThrowingStream is a verbatim copy of the one in UriContentReaderCancellationTest.kt:132

Both are the same 4-line private helper. Move it to shared/src/androidHostTest/kotlin/com/devil/phoenixproject/testutil/ (which already holds the Android-only test helpers) and use it from both tests. Optional cleanup - the duplication is harmless.

kilo-code-bot · 2026-10-28T16:07:14Z

+        repository.saveSessionAttempts.map { it.exerciseName.orEmpty() }
+
+    private fun historyCsv(): InputStream = """
+        Date,Time,Exercise,Mode,Target Reps,Warmup Reps,Working Reps,Total Reps,Weight,Progression,Duration (s),Just Lift,Eccentric Load


SUGGESTION: The history header is hardcoded although PhoenixCsvCodec.HISTORY_HEADER already exists

PhoenixCsvCodec is in the same package, so the fixture can use ${PhoenixCsvCodec.HISTORY_HEADER} instead of the duplicated literal and stay in step with the codec. Optional - the literal currently matches.

kilo-code-bot · 2026-10-28T16:07:29Z

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Issue Details (click to expand)

WARNING

File	Line	Issue
`shared/src/phoneMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt`	816	The shared caller rethrow is a no-op on iOS: `CsvImporter.ios.kt` still swallows `CancellationException`, so a cancelled iOS import still records a failed row, keeps writing later rows and opens the import-failed dialog

SUGGESTION

File	Line	Issue
`shared/src/androidHostTest/kotlin/com/devil/phoenixproject/util/AndroidCsvImporterCancellationTest.kt`	115	`ThrowingStream` duplicates the identical helper in `UriContentReaderCancellationTest.kt:132`
`shared/src/androidHostTest/kotlin/com/devil/phoenixproject/util/AndroidCsvImporterCancellationTest.kt`	109	History header hardcoded although `PhoenixCsvCodec.HISTORY_HEADER` exists

Files Reviewed (3 files)

shared/src/androidHostTest/kotlin/com/devil/phoenixproject/util/AndroidCsvImporterCancellationTest.kt - 2 issues
shared/src/androidMain/kotlin/com/devil/phoenixproject/util/CsvImporter.android.kt - 0 issues
shared/src/phoneMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt - 1 issue

Verdict

Comment only - the Android cancellation fix is correct and the new Robolectric test covers it; the open items are an iOS parity gap and two optional test nits.

Correctness / Safety Findings

important: shared/src/phoneMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt:L816: the caller-side rethrow only reaches Android. shared/src/iosMain/kotlin/com/devil/phoenixproject/util/CsvImporter.ios.kt:72 and :93 still catch Exception and swallow CancellationException, so a cancelled iOS import records a failed row, keeps writing the remaining rows and returns a CsvImportResult whose only error is Import failed: ..., which opens the import-failed dialog. Required fix: add e.rethrowIfCancellation() as the first statement of both iOS catches.

No other correctness or safety findings. The Android change itself is sound:

CsvImporter.android.kt:73 and :95 - both catches rethrow before converting an exception into a result. The outer catch at :94 needs its own rethrow, otherwise it re-swallows the rethrow from :73; both are present.
AnalyticsScreen.kt:816 - finally { isImporting = false } still runs, and a rethrown CancellationException is normal coroutine completion, so there is no dialog and no uncaught-exception crash.
AndroidCsvImporterCancellationTest.kt - the four tests assert the right things: the cancelled row is attempted but not stored, the Row row is never attempted, the saved session carries the profile id, and the non-cancellation failure paths still return a CsvImportResult.

Ponytail Review

shared/src/androidHostTest/kotlin/com/devil/phoenixproject/util/AndroidCsvImporterCancellationTest.kt:L115: reuse - ThrowingStream is a verbatim copy of UriContentReaderCancellationTest.kt:132. Move it to shared/src/androidHostTest/kotlin/com/devil/phoenixproject/testutil/ and use it from both tests.
shared/src/androidHostTest/kotlin/com/devil/phoenixproject/util/AndroidCsvImporterCancellationTest.kt:L109: reuse - the history header literal duplicates PhoenixCsvCodec.HISTORY_HEADER. Use the constant in the fixture.
shared/src/iosMain/kotlin/com/devil/phoenixproject/util/CsvImporter.ios.kt:L72: shrink - the iOS importer is a near-verbatim copy of CsvImporter.android.kt; moving the shared import loop into commonMain behind a platform file reader would make every future cancellation fix land on both platforms at once. Optional - the 2-line local fix above is the smaller change.

Ponytail net: -4 lines.

Suggested Minimal Patch

One file, two lines: shared/src/iosMain/kotlin/com/devil/phoenixproject/util/CsvImporter.ios.kt

add e.rethrowIfCancellation() as the first statement of the catch (e: Exception) at line 72
add e.rethrowIfCancellation() as the first statement of the catch (e: Exception) at line 93

Optional, not required: move ThrowingStream to shared/src/androidHostTest/kotlin/com/devil/phoenixproject/testutil/ and use PhoenixCsvCodec.HISTORY_HEADER in the new test's CSV fixture.

No other files need to change.

Final Merge Guidance

Can merge; the Android fix is correct and the new tests cover the changed behavior. This PR was already merged at 2026-10-28T16:02:07Z, so the iOS parity gap (2 lines) should land as a follow-up. The Ponytail items are optional cleanup.

Fix these issues in Kilo Cloud

_{Reviewed by minimax-m3 · Input: 98K · Output: 33.3K · Cached: 1.5M}

_{Review guidance: REVIEW.md from base branch main}

9thLevelSoftware marked this pull request as ready for review October 28, 2026 16:01

Copilot AI balanced review requested due to automatic review settings October 28, 2026 16:01

Copilot AI reviewed Oct 28, 2026

9thLevelSoftware merged commit 1c964b9 into main Oct 28, 2026
13 checks passed

9thLevelSoftware deleted the cursor/csv-import-cancellation-e3c6 branch October 28, 2026 16:02

kilo-code-bot Bot reviewed Oct 28, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(import): rethrow cancellation in Android CSV import - #1355

fix(import): rethrow cancellation in Android CSV import#1355
9thLevelSoftware merged 1 commit into
mainfrom
cursor/csv-import-cancellation-e3c6

9thLevelSoftware commented Oct 27, 2026 •

edited by cursor Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

kilo-code-bot Bot Oct 28, 2026

Uh oh!

kilo-code-bot Bot Oct 28, 2026

Uh oh!

kilo-code-bot Bot Oct 28, 2026

Uh oh!

kilo-code-bot Bot commented Oct 28, 2026 •

edited

Loading

WARNING

SUGGESTION

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

9thLevelSoftware commented Oct 27, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kilo-code-bot Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

kilo-code-bot Bot commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Overview

WARNING

SUGGESTION

Verdict

Correctness / Safety Findings

Ponytail Review

Suggested Minimal Patch

Final Merge Guidance

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

9thLevelSoftware commented Oct 27, 2026 •

edited by cursor Bot

Loading

kilo-code-bot Bot commented Oct 28, 2026 •

edited

Loading