Address XML workout import review feedback

Author: dzarlax

cmd/import/main.go

@@ -23,7 +23,7 @@ import (
 
 func main() {
 	filePath := flag.String("file", "", "Apple Health export (.zip or export.xml) — required")
-	batchSize := flag.Int("batch", 500, "metric points per DB transaction")
+	batchSize := flag.Int("batch", 500, "metric points or workouts per DB transaction")
 	pauseDur := flag.Duration("pause", 150*time.Millisecond, "sleep between batches (rate-limits DB load)")
 	dryRun := flag.Bool("dry-run", false, "parse only — do not write to DB")
 	flag.Parse()

docs/ai-plans/2026-07-01-apple-health-xml-workout-import.html

@@ -133,7 +133,7 @@ <h3>Storage is ready</h3>
   <section class="ok">
     <h2>Real Export Findings</h2>
     <p>
-      The unpacked export at <code>/Users/dzarlax/Downloads/apple_health_export/</code> was inspected on 2026-07-01.
+      A local unpacked Apple Health export at <code>&lt;LOCAL_APPLE_HEALTH_EXPORT_DIR&gt;/</code> was inspected on 2026-07-01.
       Only structural counts and attribute shapes were used; implementation should not bake in personal paths or source names.
     </p>
     <ul>
@@ -182,7 +182,7 @@ <h2>Proposed Design</h2>
     <h2>Workout Field Mapping</h2>
     <ul>
       <li><code>external_id</code>: prefer <code>HKMetadataKeySyncIdentifier</code>, then <code>HKExternalUUID</code>; fall back to a deterministic hash from type/start/end/source/duration/statistics when Apple exports lack IDs.</li>
-      <li><code>name</code>: map <code>workoutActivityType</code> suffix into readable names such as <code>Outdoor Run</code>, <code>Outdoor Cycle</code>, <code>Walking</code>. Unknown values keep a readable suffix rather than failing import.</li>
+      <li><code>name</code>: map <code>workoutActivityType</code> suffix into readable names such as <code>Outdoor Run</code>, <code>Outdoor Cycling</code>, <code>Walking</code>. Unknown values keep a readable suffix rather than failing import.</li>
       <li><code>start_time</code>, <code>end_time</code>: parse Apple Health XML timestamps with the existing HealthKit layout.</li>
       <li><code>duration_sec</code>: use <code>duration</code> plus <code>durationUnit</code> when present; otherwise compute from start/end.</li>
       <li><code>energy_kcal</code>: parse nested <code>WorkoutStatistics type="HKQuantityTypeIdentifierActiveEnergyBurned"</code> using <code>sum</code> and <code>unit</code>.</li>
@@ -309,7 +309,7 @@ <h2>Verification Run</h2>
       <li><code>env GOCACHE=/tmp/health-go-build-cache go test ./internal/applehealth -count=1</code> passed.</li>
       <li><code>env GOCACHE=/tmp/health-go-build-cache go test ./internal/applehealth ./internal/health -count=1</code> passed.</li>
       <li><code>env GOCACHE=/tmp/health-go-build-cache go test ./cmd/import ./internal/handler ./internal/storage ./internal/ui -run "Workout|AppleHealth|ParseXML|Import" -count=1</code> passed.</li>
-      <li><code>env GOCACHE=/tmp/health-go-build-cache go run ./cmd/import --file /Users/dzarlax/Downloads/apple_health_export/export.xml --dry-run --batch 100000 --pause 0</code> parsed <code>6,803,675</code> metric points and <code>862</code> workouts in 24 seconds, with no DB writes.</li>
+      <li><code>env GOCACHE=/tmp/health-go-build-cache go run ./cmd/import --file /path/to/apple_health_export/export.xml --dry-run --batch 100000 --pause 0</code> parsed <code>6,803,618</code> metric points and <code>862</code> workouts in 24 seconds, with no DB writes.</li>
       <li><code>env GOCACHE=/tmp/health-go-build-cache make test-unit</code> passed.</li>
     </ul>
   </section>
@@ -324,6 +324,21 @@ <h2>Known Limitations After Implementation</h2>
     </ul>
   </section>
 
+  <section class="ok">
+    <h2>PR Review Follow-Up</h2>
+    <p>
+      Addressed review feedback on 2026-07-03 after PR creation.
+    </p>
+    <ul>
+      <li>Applied the existing physiological quality gate to mapped XML quantity records, so impossible zero values such as heart rate are dropped while valid zero values such as walking asymmetry are preserved.</li>
+      <li>Mapped XML cycling workouts to canonical <code>Indoor Cycling</code> / <code>Outdoor Cycling</code> names to match existing exact-name filters.</li>
+      <li>Changed Admin import status counter reads to <code>atomic.LoadInt64</code> to match atomic writers.</li>
+      <li>Redacted local machine paths from this plan and kept dry-run examples portable.</li>
+      <li>Added validation/canonical-name tests for <code>parseWorkout</code> and kept duration-minute handling explicit.</li>
+      <li>Cleaned up small review nits: batch flag help text and standard-library <code>math.Abs</code> in unit tests.</li>
+    </ul>
+  </section>
+
   <section class="approval">
     <h2>Approval Gate</h2>
     <p>

internal/applehealth/parse.go

@@ -453,6 +453,8 @@ func workoutDurationSec(raw, unit string, start, end time.Time) float64 {
 			return v
 		case "h", "hr", "hrs", "hour", "hours":
 			return v * 3600
+		case "m", "min", "mins", "minute", "minutes":
+			return v * 60
 		default:
 			return v * 60
 		}
@@ -470,9 +472,9 @@ func workoutName(activityType string, indoor bool) string {
 		return "Outdoor Run"
 	case "Cycling":
 		if indoor {
-			return "Indoor Cycle"
+			return "Indoor Cycling"
 		}
-		return "Outdoor Cycle"
+		return "Outdoor Cycling"
 	case "Walking":
 		return "Walking"
 	case "FunctionalStrengthTraining":
@@ -600,6 +602,9 @@ func parseRecord(a map[string]string) []storage.MetricPoint {
 		if hkFractionToPercent[suffix] && value > 0 && value <= 1.0 {
 			value *= 100
 		}
+		if health.IsImpossible(info[0], value) {
+			return nil
+		}
 		return []storage.MetricPoint{
 			{MetricName: info[0], Units: units, Date: startDate, Qty: value, Source: source},
 		}

internal/applehealth/parse_test.go

@@ -95,6 +95,7 @@ func TestParseXMLFocusedEdgeFixturePinsImportSafety(t *testing.T) {
 	}
 	assertPoint(t, points, "blood_oxygen_saturation", "%", "2026-02-01 06:00:00 +0100", "Synthetic Watch", 98)
 	assertPoint(t, points, "walking_asymmetry", "%", "2026-02-01 06:05:00 +0100", "Synthetic Watch", 0)
+	assertNoPoint(t, points, "heart_rate", "2026-02-01 06:10:00 +0100")
 	assertPoint(t, points, "mindful_minutes", "min", "2026-02-01 09:00:00 +0100", "Synthetic Phone", 15)
 	assertPoint(t, points, "apple_stand_hour", "count", "2026-02-01 10:00:00 +0100", "Synthetic Watch", 1)
 	assertPoint(t, points, "some_new_metric", "count", "2026-02-01 13:00:00 +0100", "Synthetic Device", 12.5)
@@ -103,6 +104,48 @@ func TestParseXMLFocusedEdgeFixturePinsImportSafety(t *testing.T) {
 	assertNoMetric(t, points, "blood_pressure")
 }
 
+func TestParseWorkoutValidationAndCanonicalNames(t *testing.T) {
+	if _, ok := parseWorkout(xmlWorkout{}); ok {
+		t.Fatal("empty workout parsed, want rejected")
+	}
+	if _, ok := parseWorkout(xmlWorkout{
+		ActivityType: "HKWorkoutActivityTypeRunning",
+		StartDate:    "2026-01-02 12:45:00 +0000",
+		EndDate:      "2026-01-02 12:00:00 +0000",
+	}); ok {
+		t.Fatal("workout with end before start parsed, want rejected")
+	}
+
+	indoor, ok := parseWorkout(xmlWorkout{
+		ActivityType: "HKWorkoutActivityTypeCycling",
+		Duration:     "30",
+		DurationUnit: "min",
+		StartDate:    "2026-01-02 12:00:00 +0000",
+		EndDate:      "2026-01-02 12:30:00 +0000",
+		Metadata:     []xmlMetadataEntry{{Key: "HKIndoorWorkout", Value: "1"}},
+	})
+	if !ok {
+		t.Fatal("valid indoor cycling workout rejected")
+	}
+	if indoor.Name != "Indoor Cycling" {
+		t.Fatalf("indoor cycling name = %q, want Indoor Cycling", indoor.Name)
+	}
+
+	outdoor, ok := parseWorkout(xmlWorkout{
+		ActivityType: "HKWorkoutActivityTypeCycling",
+		Duration:     "30",
+		DurationUnit: "min",
+		StartDate:    "2026-01-02 12:00:00 +0000",
+		EndDate:      "2026-01-02 12:30:00 +0000",
+	})
+	if !ok {
+		t.Fatal("valid outdoor cycling workout rejected")
+	}
+	if outdoor.Name != "Outdoor Cycling" {
+		t.Fatalf("outdoor cycling name = %q, want Outdoor Cycling", outdoor.Name)
+	}
+}
+
 func TestParseXMLEmptyAndMalformedInputs(t *testing.T) {
 	var emitted bool
 	if err := parseXMLFixture("testdata/empty_export.xml", func(points []storage.MetricPoint) {

internal/applehealth/testdata/focused_edge_export.xml

@@ -2,6 +2,7 @@
 <HealthData locale="en_US">
   <Record type="HKQuantityTypeIdentifierOxygenSaturation" sourceName="Synthetic Watch" unit="%" creationDate="2026-02-01 06:00:00 +0100" startDate="2026-02-01 06:00:00 +0100" endDate="2026-02-01 06:01:00 +0100" value="98"/>
   <Record type="HKQuantityTypeIdentifierWalkingAsymmetryPercentage" sourceName="Synthetic Watch" unit="%" creationDate="2026-02-01 06:05:00 +0100" startDate="2026-02-01 06:05:00 +0100" endDate="2026-02-01 06:06:00 +0100" value="0"/>
+  <Record type="HKQuantityTypeIdentifierHeartRate" sourceName="Synthetic Watch" unit="count/min" creationDate="2026-02-01 06:10:00 +0100" startDate="2026-02-01 06:10:00 +0100" endDate="2026-02-01 06:11:00 +0100" value="0"/>
   <Record type="HKCategoryTypeIdentifierMindfulSession" sourceName="Synthetic Phone" creationDate="2026-02-01 09:15:00 +0100" startDate="2026-02-01 09:00:00 +0100" endDate="2026-02-01 09:15:00 +0100" value="HKCategoryValueNotApplicable"/>
   <Record type="HKCategoryTypeIdentifierAppleStandHour" sourceName="Synthetic Watch" creationDate="2026-02-01 10:00:00 +0100" startDate="2026-02-01 10:00:00 +0100" endDate="2026-02-01 11:00:00 +0100" value="HKCategoryValueAppleStandHourStood"/>
   <Record type="HKCategoryTypeIdentifierMindfulSession" sourceName="Synthetic Phone" creationDate="2026-02-01 11:00:00 +0100" startDate="2026-02-01 11:00:00 +0100" endDate="2026-02-01 10:59:00 +0100" value="HKCategoryValueNotApplicable"/>

internal/health/workout_units_test.go

@@ -1,6 +1,9 @@
 package health
 
-import "testing"
+import (
+	"math"
+	"testing"
+)
 
 func TestNormalizeDistanceKm(t *testing.T) {
 	tests := []struct {
@@ -17,16 +20,9 @@ func TestNormalizeDistanceKm(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := NormalizeDistanceKm(tt.qty, tt.units); abs(got-tt.want) > 0.000001 {
+			if got := NormalizeDistanceKm(tt.qty, tt.units); math.Abs(got-tt.want) > 0.000001 {
 				t.Fatalf("NormalizeDistanceKm(%v, %q) = %v, want %v", tt.qty, tt.units, got, tt.want)
 			}
 		})
 	}
 }
-
-func abs(v float64) float64 {
-	if v < 0 {
-		return -v
-	}
-	return v
-}

internal/ui/import_handler.go

@@ -41,14 +41,14 @@ func (j *importJob) status() importStatus {
 	s := importStatus{
 		Running:          j.running,
 		Done:             j.done,
-		Parsed:           j.parsed,
-		Inserted:         j.inserted,
-		Skipped:          j.skipped,
-		WorkoutsParsed:   j.workoutsParsed,
-		WorkoutsUpserted: j.workoutsUpserted,
-		WorkoutsFailed:   j.workoutsFailed,
-		BytesRead:        j.bytesRead,
-		TotalBytes:       j.totalBytes,
+		Parsed:           atomic.LoadInt64(&j.parsed),
+		Inserted:         atomic.LoadInt64(&j.inserted),
+		Skipped:          atomic.LoadInt64(&j.skipped),
+		WorkoutsParsed:   atomic.LoadInt64(&j.workoutsParsed),
+		WorkoutsUpserted: atomic.LoadInt64(&j.workoutsUpserted),
+		WorkoutsFailed:   atomic.LoadInt64(&j.workoutsFailed),
+		BytesRead:        atomic.LoadInt64(&j.bytesRead),
+		TotalBytes:       atomic.LoadInt64(&j.totalBytes),
 		StartedAt:        j.startedAt.Format(time.RFC3339),
 		Err:              j.err,
 	}