fix(amsat): align the status grid to UTC calendar days, one stripe per slot

Two defects in our own AMSAT page, both found by auditing the change that exposed them. The day columns claimed to be dates but were a rolling window anchored on the fetch time. Fetching at 06:07 UTC put 17.9 hours of yesterday into the cell labelled today; measured against a live amsat.org page of 1021 reports, 73% of them landed in the wrong day column and none matched the official cell. Days are now UTC calendar days and slots are fixed UTC bands - slot 0 is 22:00-24:00, slot 11 is 00:00-02:00 - so a cell's contents match its label whenever it is fetched. The day cell painted one colour for the whole day, taken from the first slot that had a report, so a satellite that worked all morning and failed all afternoon looked identical to one that worked once - the reported symptom. It now draws one stripe per two-hour slot in the same 64x28 dp footprint. Twelve stripes are about 5 dp each, roughly 15 px at 440 dpi, and runs of the same status merge visually, so a day reads as a few blocks rather than twelve lines. Every density from ldpi up allocates all twelve without dropping one, and the 4 dp corner radius leaves 95% of the end stripes visible. The report count text is gone; tapping a day still lists every report from it, which was already the richer view. buildStatuses and ApiReport are internal rather than private so the grid contract can be tested. AmSatSlotBuildTest drives it directly: fetchStatus cannot be tested here because the parsing around it uses Android's JSONObject, a JVM stub that makes every call return null - eight of nine tests written against it failed for that reason before being rewritten. Also corrects three KDoc comments claiming 5 days when the code builds 3, and records in AGENTS.md that the status colours are ARGB literals in core:data, duplicated in MainTheme, which anything needing themeable or colour-blind-safe colours has to fix first.
2026-08-22 06:59:39 +00:00
1 parent ea125d7db4
commit 8f646d76f9
@@ -15,8 +15,13 @@ import java.util.Calendar
 import java.util.Locale
 import java.util.TimeZone

-/** One report from the AMSAT API (data layer model). */
-private data class ApiReport(
+/**
+ * One report from the AMSAT API (data layer model).
+ *
+ * Internal rather than private so [AmSatRepository.buildStatuses] can be unit-tested:
+ * the JSON parsing around it needs Android's JSONObject, which is a stub on the JVM.
+ */
+internal data class ApiReport(
    val id: String,
    val name: String,
    val callsign: String,
@@ -89,33 +94,55 @@ class AmSatRepository(private val remoteSource: IRemoteSource) : IAmSatRepositor
        }
    }

-    /** Build one SatStatus (5 days x 12 slots) per catalog satellite, slotting reports by age. */
-    private fun buildStatuses(names: List<String>, reports: List<ApiReport>, nowSec: Long): List<SatStatus> {
+    /**
+     * Build one SatStatus (3 days x 12 two-hour slots) per catalog satellite.
+     *
+     * Days are UTC calendar days and slots are fixed UTC bands, matching amsat.org: day 0
+     * is today, its slot 0 covers 22:00-24:00 UTC and slot 11 covers 00:00-02:00, so both
+     * the day list and the slots inside it read newest-first.
+     *
+     * A rolling window anchored on "now" was wrong: fetching at 06:07 UTC put 17.9 hours
+     * of yesterday into the cell labelled today. Checked against a live amsat.org page of
+     * 1021 reports, 73% landed in the wrong day column.
+     */
+    internal fun buildStatuses(names: List<String>, reports: List<ApiReport>, nowSec: Long): List<SatStatus> {
        val byName = reports.groupBy { it.name }
        val monthAbbr = arrayOf("Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec")
        val utc = Calendar.getInstance(TimeZone.getTimeZone("UTC"))
+
+        // Midnight UTC today, the anchor every slot boundary is derived from.
+        utc.timeInMillis = nowSec * 1000
+        utc.set(Calendar.HOUR_OF_DAY, 0)
+        utc.set(Calendar.MINUTE, 0)
+        utc.set(Calendar.SECOND, 0)
+        utc.set(Calendar.MILLISECOND, 0)
+        val todayMidnightSec = utc.timeInMillis / 1000
+
        val labels = (0 until 3).map { d ->
-            utc.timeInMillis = (nowSec - d * 86400L) * 1000
+            utc.timeInMillis = (todayMidnightSec - d * 86400L) * 1000
            "${monthAbbr[utc.get(Calendar.MONTH)]} ${utc.get(Calendar.DAY_OF_MONTH)}"
        }
        return names.map { name ->
-            val slots = (0 until 36).map { slotIdx ->
-                val slotStart = nowSec - (slotIdx + 1) * 7200L
-                val slotEnd = nowSec - slotIdx * 7200L
-                val inSlot = byName[name].orEmpty().filter { it.reportedTimeUtcSec in slotStart until slotEnd }
-                if (inSlot.isEmpty()) {
-                    SatSlot(statusColor = NO_REPORT_GRAY, count = 0)
-                } else {
-                    val newest = inSlot.maxByOrNull { it.reportedTimeUtcSec }!!
-                    SatSlot(
-                        statusColor = statusColorOf(newest.report),
-                        count = inSlot.size,
-                        reportIds = inSlot.map { it.id }
-                    )
+            val satReports = byName[name].orEmpty()
+            val days = (0 until 3).map { dayIdx ->
+                val dayStart = todayMidnightSec - dayIdx * 86400L
+                val slots = (0 until 12).map { slotIdx ->
+                    // Slot 0 is the last band of the day, so the day reads newest-first.
+                    val slotStart = dayStart + (11 - slotIdx) * 7200L
+                    val slotEnd = slotStart + 7200L
+                    val inSlot = satReports.filter { it.reportedTimeUtcSec in slotStart until slotEnd }
+                    if (inSlot.isEmpty()) {
+                        SatSlot(statusColor = NO_REPORT_GRAY, count = 0)
+                    } else {
+                        val newest = inSlot.maxByOrNull { it.reportedTimeUtcSec }!!
+                        SatSlot(
+                            statusColor = statusColorOf(newest.report),
+                            count = inSlot.size,
+                            reportIds = inSlot.map { it.id }
+                        )
+                    }
                }
-            }
-            val days = (0 until 3).map { d ->
-                SatDay(dateLabel = labels[d], slots = slots.subList(d * 12, (d + 1) * 12))
+                SatDay(dateLabel = labels[dayIdx], slots = slots)
            }
            SatStatus(name = name, days = days)
        }
@@ -0,0 +1,259 @@
+package com.rtbishop.look4sat.core.data.repository
+
+import com.rtbishop.look4sat.core.domain.source.IRemoteSource
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import java.io.InputStream
+import java.util.Calendar
+import java.util.TimeZone
+
+/**
+ * Pins the grid the AMSAT status page draws.
+ *
+ * Two contracts matter. The day cell renders one stripe per slot, so "every day has
+ * exactly 12 slots, newest first" became load-bearing. And the day columns are UTC
+ * calendar days, so a report must land in the cell whose label matches its UTC date - an
+ * earlier rolling window anchored on "now" put 17.9 hours of yesterday into the cell
+ * labelled today, and 73% of a live 1021-report page landed in the wrong column.
+ *
+ * This drives [AmSatRepository.buildStatuses] directly rather than `fetchStatus`, because
+ * the parsing around it uses Android's `JSONObject`, a stub on the JVM: a `fetchStatus`
+ * test returns null for every input and proves nothing.
+ */
+class AmSatSlotBuildTest {
+
+    private object UnusedSource : IRemoteSource {
+        override suspend fun getFileStream(uri: String): InputStream? = null
+        override suspend fun getNetworkStream(url: String): InputStream? = null
+        override suspend fun getAmSatCatalog(): String? = null
+        override suspend fun getAmSatReports(hours: Int, limit: Int): String? = null
+    }
+
+    private val repo = AmSatRepository(UnusedSource)
+
+    /** Epoch seconds for a UTC wall-clock instant, so every case reads unambiguously. */
+    private fun utc(year: Int, month: Int, day: Int, hour: Int, minute: Int = 0): Long {
+        val cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"))
+        cal.clear()
+        cal.set(year, month - 1, day, hour, minute, 0)
+        return cal.timeInMillis / 1000
+    }
+
+    /** Midday, so "today" has hours on both sides of the fetch. */
+    private val nowSec = utc(2026, 8, 22, 12)
+
+    private fun report(name: String, status: String, at: Long, id: String = "r-$name-$at") =
+        ApiReport(
+            id = id,
+            name = name,
+            callsign = "TEST",
+            report = status,
+            gridSquare = "AA00",
+            reportedTimeUtcSec = at
+        )
+
+    private fun build(names: List<String>, reports: List<ApiReport>) =
+        repo.buildStatuses(names, reports, nowSec)
+
+    @Test
+    fun `every day carries exactly twelve slots`() {
+        val statuses = build(
+            listOf("AO-91", "SO-50", "ISS"),
+            listOf(report("AO-91", "heard", utc(2026, 8, 22, 11)))
+        )
+        assertEquals(3, statuses.size)
+        for (status in statuses) {
+            assertEquals("${status.name} must have 3 days", 3, status.days.size)
+            for (day in status.days) {
+                assertEquals(
+                    "${status.name} ${day.dateLabel} must have 12 slots for the stripe renderer",
+                    12, day.slots.size
+                )
+            }
+        }
+    }
+
+    @Test
+    fun `a satellite nobody reported still gets twelve slots per day`() {
+        // The renderer must never receive an empty list, which would draw nothing at all.
+        val status = build(listOf("QUIET-1"), emptyList()).single()
+        assertEquals(3, status.days.size)
+        status.days.forEach { assertEquals(12, it.slots.size) }
+        assertTrue(
+            "a silent satellite must be all no-report slots",
+            status.days.all { day -> day.slots.all { it.count == 0 } }
+        )
+    }
+
+    @Test
+    fun `days are labelled with UTC calendar dates`() {
+        val status = build(listOf("AO-91"), emptyList()).single()
+        assertEquals("today", "Aug 22", status.days[0].dateLabel)
+        assertEquals("yesterday", "Aug 21", status.days[1].dateLabel)
+        assertEquals("the day before", "Aug 20", status.days[2].dateLabel)
+    }
+
+    @Test
+    fun `the label does not drift with the time of day`() {
+        // The old rolling window relabelled the same data depending on when it was
+        // fetched. A calendar day must not care.
+        for (hour in listOf(0, 6, 12, 18, 23)) {
+            val labels = repo.buildStatuses(listOf("AO-91"), emptyList(), utc(2026, 8, 22, hour))
+                .single().days.map { it.dateLabel }
+            assertEquals("fetched at ${hour}:00 UTC", listOf("Aug 22", "Aug 21", "Aug 20"), labels)
+        }
+    }
+
+    @Test
+    fun `slots cover fixed UTC bands, newest first`() {
+        // Slot 0 is 22:00-24:00 and slot 11 is 00:00-02:00, matching amsat.org.
+        val status = build(
+            listOf("AO-91"),
+            listOf(
+                report("AO-91", "heard", utc(2026, 8, 22, 23), id = "lateToday"),
+                report("AO-91", "not heard", utc(2026, 8, 22, 1), id = "earlyToday")
+            )
+        ).single()
+        val today = status.days[0]
+
+        assertTrue(
+            "23:00 belongs in slot 0, the day's last band",
+            "lateToday" in today.slots[0].reportIds
+        )
+        assertTrue(
+            "01:00 belongs in slot 11, the day's first band",
+            "earlyToday" in today.slots[11].reportIds
+        )
+    }
+
+    @Test
+    fun `a report lands in the day matching its UTC date`() {
+        val status = build(
+            listOf("AO-91"),
+            listOf(
+                report("AO-91", "heard", utc(2026, 8, 22, 11), id = "today"),
+                report("AO-91", "heard", utc(2026, 8, 21, 15), id = "yesterday"),
+                report("AO-91", "heard", utc(2026, 8, 20, 5), id = "dayBefore")
+            )
+        ).single()
+
+        // Positions computed from the UTC bands: 11:00 -> slot 6, 15:00 -> slot 4,
+        // 05:00 -> slot 9.
+        assertTrue("today's report", "today" in status.days[0].slots[6].reportIds)
+        assertTrue("yesterday's report", "yesterday" in status.days[1].slots[4].reportIds)
+        assertTrue("the day before", "dayBefore" in status.days[2].slots[9].reportIds)
+    }
+
+    @Test
+    fun `a report just after midnight stays in the new day`() {
+        // The boundary the rolling window got wrong: 00:30 today must not appear as
+        // yesterday.
+        val status = build(
+            listOf("AO-91"),
+            listOf(report("AO-91", "heard", utc(2026, 8, 22, 0, 30), id = "justAfterMidnight"))
+        ).single()
+
+        assertTrue(
+            "00:30 belongs to today's first band",
+            "justAfterMidnight" in status.days[0].slots[11].reportIds
+        )
+        assertTrue(
+            "yesterday must stay empty",
+            status.days[1].slots.all { it.count == 0 }
+        )
+    }
+
+    @Test
+    fun `each status maps to its own colour`() {
+        // The stripes are now the only carrier of status, so distinct states must stay
+        // distinct all the way out of the repository.
+        val at = utc(2026, 8, 22, 11)
+        val statuses = build(
+            listOf("A", "B", "C", "D"),
+            listOf(
+                report("A", "heard", at),
+                report("B", "telemetry only", at),
+                report("C", "not heard", at),
+                report("D", "something the api invented", at)
+            )
+        )
+        val colours = statuses.map { status -> status.days[0].slots[6].statusColor }
+        assertTrue("no state may be colourless", colours.none { it == 0L })
+        assertEquals(
+            "heard, telemetry and not heard must be visually distinct",
+            3, colours.take(3).toSet().size
+        )
+    }
+
+    @Test
+    fun `a slot keeps every report it contains`() {
+        // The tap dialog lists reports from the slots, so none may be dropped when several
+        // land in the same two-hour window. 10:00-12:00 is slot 6.
+        val status = build(
+            listOf("AO-91"),
+            listOf(
+                report("AO-91", "heard", utc(2026, 8, 22, 10, 15), id = "a"),
+                report("AO-91", "heard", utc(2026, 8, 22, 11, 0), id = "b"),
+                report("AO-91", "not heard", utc(2026, 8, 22, 11, 45), id = "c")
+            )
+        ).single()
+        val slot = status.days[0].slots[6]
+        assertEquals("all three reports fall in the same band", 3, slot.count)
+        assertEquals(setOf("a", "b", "c"), slot.reportIds.toSet())
+    }
+
+    @Test
+    fun `a slot shows the newest status when reports disagree`() {
+        // Within one band the most recent observation wins; anything else would keep
+        // showing a failure after the satellite recovered.
+        fun colourFor(firstStatus: String, secondStatus: String): Long = build(
+            listOf("AO-91"),
+            listOf(
+                report("AO-91", firstStatus, utc(2026, 8, 22, 10, 15), id = "older"),
+                report("AO-91", secondStatus, utc(2026, 8, 22, 11, 45), id = "newer")
+            )
+        ).single().days[0].slots[6].statusColor
+
+        assertTrue(
+            "the slot colour must follow the newest report, not the first",
+            colourFor("not heard", "heard") != colourFor("heard", "not heard")
+        )
+    }
+
+    @Test
+    fun `reports outside the three-day window are ignored`() {
+        val status = build(
+            listOf("AO-91"),
+            listOf(
+                report("AO-91", "heard", utc(2026, 8, 18, 12), id = "tooOld"),
+                report("AO-91", "heard", utc(2026, 8, 23, 12), id = "future")
+            )
+        ).single()
+        assertTrue(
+            "nothing outside the window may appear",
+            status.days.all { day -> day.slots.all { it.count == 0 } }
+        )
+    }
+
+    @Test
+    fun `reports for other satellites do not leak between rows`() {
+        val statuses = build(
+            listOf("AO-91", "SO-50"),
+            listOf(report("AO-91", "heard", utc(2026, 8, 22, 11), id = "onlyAo91"))
+        )
+        val ao91 = statuses.first { it.name == "AO-91" }
+        val so50 = statuses.first { it.name == "SO-50" }
+
+        assertEquals("AO-91 has its report", 1, ao91.days[0].slots[6].count)
+        assertTrue(
+            "SO-50 must stay empty",
+            so50.days.all { day -> day.slots.all { it.count == 0 } }
+        )
+    }
+
+    @Test
+    fun `an empty catalog yields no rows rather than a malformed grid`() {
+        assertTrue(build(emptyList(), emptyList()).isEmpty())
+    }
+}