TASK-034 follow-up: repeat tap on a starred row now un-stars instead of no-op

The prior fix for double-counting used an early-return guard that made
a repeat tap on an already-Starred row do nothing -- fixed the count
bug but also made the star impossible to undo from the ALL list.
Replaced it with a real toggle: an already-starred-this-session tap
now calls unstar() and decrements the count back down instead of being
inert.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

app/src/main/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModel.kt

@@ -284,17 +284,23 @@ class RepositoryListViewModel(
     fun toggleFavorite(repository: RepositoryRowUiModel) {
         val content = mutableState.value as? RepositoryListUiState.Content ?: return
         val mode = content.mode
-        // On the ALL list, a repeat tap after a successful star would otherwise re-call star()
-        // (harmless server-side -- PUT is idempotent) and, before this guard, double-increment the
-        // client-displayed count each time. Once this session has confirmed a star succeeded,
-        // further taps here are a no-op; unstarring from the ALL list isn't supported (see the
-        // FavoriteStar doc comment in RepositoryListScreen.kt for why).
-        if (mode == RepositoryListMode.ALL && repository.favoriteAction == FavoriteActionUiState.Starred) return
+        // Once this session has confirmed a star succeeded (favoriteAction == Starred), a repeat
+        // tap on the ALL list now un-stars instead of starring again -- a real toggle, matching
+        // what tapping an already-filled star obviously should do. Before this, a repeat tap either
+        // re-called star() and double-incremented the displayed count, or (briefly) was a dead-end
+        // no-op with no way back to unstarred without switching to the FAVORITES tab.
+        val alreadyStarredThisSession = mode == RepositoryListMode.ALL && repository.favoriteAction == FavoriteActionUiState.Starred
         updateRow(repository.coordinates) { it.copy(favoriteAction = FavoriteActionUiState.InProgress) }
         viewModelScope.launch {
             try {
-                when (mode) {
-                    RepositoryListMode.ALL -> {
+                when {
+                    alreadyStarredThisSession -> {
+                        favoritesApi.unstar(repository.coordinates)
+                        updateRow(repository.coordinates) {
+                            it.copy(favoriteAction = FavoriteActionUiState.Idle, starsCount = (it.starsCount - 1).coerceAtLeast(0))
+                        }
+                    }
+                    mode == RepositoryListMode.ALL -> {
                         favoritesApi.star(repository.coordinates)
                         // starsCount only ever reflects what the last full list load returned
                         // otherwise -- a successful star here needs its own +1, or the row keeps
@@ -303,7 +309,7 @@ class RepositoryListViewModel(
                             it.copy(favoriteAction = FavoriteActionUiState.Starred, starsCount = it.starsCount + 1)
                         }
                     }
-                    RepositoryListMode.FAVORITES -> {
+                    else -> {
                         favoritesApi.unstar(repository.coordinates)
                         removeRow(repository.coordinates)
                     }

app/src/test/java/com/ubisam/ubgitea/feature/repositories/RepositoryListViewModelTest.kt

@@ -354,8 +354,8 @@ class RepositoryListViewModelTest {
     }
 
     @Test
-    fun `a repeat tap after a successful star is a no-op, not a second star call or count increment`() = runTest(dispatcher.scheduler) {
-        val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo"))))
+    fun `a repeat tap on an already-starred row from the all-repositories list un-stars it`() = runTest(dispatcher.scheduler) {
+        val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 4))))
         val viewModel = viewModel(browser)
         advanceUntilIdle()
 
@@ -363,13 +363,35 @@ class RepositoryListViewModelTest {
         viewModel.toggleFavorite(row)
         advanceUntilIdle()
         val afterFirstTap = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
+        assertEquals(FavoriteActionUiState.Starred, afterFirstTap.favoriteAction)
+        assertEquals(5, afterFirstTap.starsCount)
 
         viewModel.toggleFavorite(afterFirstTap)
         advanceUntilIdle()
 
         val afterSecondTap = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
-        assertEquals(1, afterSecondTap.starsCount)
+        assertEquals(FavoriteActionUiState.Idle, afterSecondTap.favoriteAction)
+        assertEquals(4, afterSecondTap.starsCount)
         assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.starredCalls)
+        assertEquals(listOf(RepositoryCoordinates.of("owner", "repo")), browser.unstarredCalls)
+    }
+
+    @Test
+    fun `starring then un-starring from the all-repositories list round-trips the count back to zero`() = runTest(dispatcher.scheduler) {
+        val browser = FakeBrowser(repositories = listOf(page1Of(repo("repo", starsCount = 0))))
+        val viewModel = viewModel(browser)
+        advanceUntilIdle()
+
+        val row = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
+        viewModel.toggleFavorite(row)
+        advanceUntilIdle()
+        val starred = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
+
+        viewModel.toggleFavorite(starred)
+        advanceUntilIdle()
+
+        val unstarred = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
+        assertEquals(0, unstarred.starsCount)
     }
 
     @Test

docs/development-log.md

@@ -580,3 +580,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl
 - Known limitations: still optimistic, not authoritative — if Gitea's real count ever differs from this app's local +1 (e.g. a concurrent star from another client), the display stays off until the next full list reload. The underlying "can't tell if already starred on load" limitation is unchanged; this only stops it from causing a visible count bug.
 - Next recommended task: get live confirmation that the count now increments on tap; otherwise continue per the standing direction on everything else still queued from this session.
 - Next owner: Claude, continuing per the standing direction.
+
+## 2026-08-29 (continued) — TASK-034 follow-up: repeat tap now un-stars instead of doing nothing
+
+- Agent and role: Claude, direct continuation of the same live session — immediately after confirming the count now increments, the human product owner reported "이제 올라가는데 다시 눌렀을때 취소가 안되네" (it goes up now, but tapping again doesn't un-star it).
+- Bug found: the first pass's fix for the double-count bug added an early-return guard that made a repeat tap on an already-`Starred` row a complete no-op — stopped the double-count, but also made the star impossible to undo from the ALL list without switching to the FAVORITES tab, which is not what a filled/tappable star should do.
+- How it was implemented: replaced the no-op guard with a real toggle — `toggleFavorite` now branches on the row's own `favoriteAction`: not-yet-starred-this-session calls `star()` (`+1`, → `Starred`), already-starred-this-session calls `unstar()` (`-1`, coerced at 0, → `Idle`). Tests updated to match: the old "repeat tap is a no-op" test was replaced with one asserting the repeat tap actually un-stars (both `star()` and `unstar()` get called, count and state both revert), plus a new round-trip test (star then unstar lands back at the original count).
+- Files changed: `RepositoryListViewModel.kt`, `RepositoryListViewModelTest.kt`, `tasks/034-favorite-star-count-not-incrementing.md` (updated throughout to describe both rounds of this fix).
+- Checks and results: `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug` BUILD SUCCESSFUL, all unit tests passing, 0 lint errors. Installed on the human product owner's device over wireless `adb`.
+- Also this turn: the `task-015-notifications` worktree had been removed from disk between sessions (branch/commits were intact in the main repo); recreated via `git worktree add`, plus a fresh untracked `local.properties` pointing at the local Android SDK.
+- Review findings and resolutions: None from a separate review pass.
+- Known limitations: still optimistic (not re-fetched from the server) in both directions; a failed *unstar* attempt still collapses to the same flat `Failed` UI state as a failed star attempt, which can render as not-starred even if the repo is still actually starred server-side — accepted as a minor, harmlessly-retryable gap rather than adding more state to track which action was in flight.
+- Next recommended task: get live confirmation that both starring (count up) and un-starring (count back down, tappable again) now work from the ALL list.
+- Next owner: Claude, continuing per the standing direction.

tasks/034-favorite-star-count-not-incrementing.md

@@ -1,4 +1,4 @@
-# TASK-034: Star count doesn't increment after a successful favorite
+# TASK-034: Star count doesn't increment, then repeat tap doesn't un-star
 
 Status: implemented; all required checks pass; live-installed, awaiting the human product owner's confirmation
 Owner: Claude
@@ -6,15 +6,15 @@ Reviewer: Codex
 
 ## Outcome
 
-Following TASK-033's OAuth scope fix, the human product owner confirmed starring itself now works
-("별표 눌러서 즐겨찾기는 되는데") but reported the displayed star count doesn't go up
-("카운팅은 안올라가네"). `toggleFavorite`'s ALL-list success branch updated only `favoriteAction`
-(to `Starred`) and never touched `starsCount`, so a row kept showing whatever count the last full
-list load had returned, even after a star that the server had actually accepted. Fixed by
-incrementing the row's `starsCount` in the same state update that marks it `Starred`. While fixing
-this, found and closed a related gap: a repeat tap on an already-`Starred` row (the ALL list has no
-way to know a repository's already-starred state on initial load, so the star stays tappable) would
-have re-called `star()` and double-incremented the client-displayed count each time — now a no-op.
+Two rounds of live feedback on the same feature, following TASK-033's OAuth scope fix. First: the
+human product owner confirmed starring itself now works ("별표 눌러서 즐겨찾기는 되는데") but the
+displayed star count didn't go up ("카운팅은 안올라가네") — `toggleFavorite`'s ALL-list success
+branch updated only `favoriteAction` (to `Starred`) and never touched `starsCount`. Fixed by
+incrementing `starsCount` in the same state update. Second, immediately after: with a naive
+repeat-tap guard in place, tapping an already-starred row no longer un-starred it either
+("이제 올라가는데 다시 눌렀을때 취소가 안되네") — the first fix's guard made a repeat tap a dead
+no-op instead of a real toggle. Replaced that guard with an actual toggle: a repeat tap on a
+row this session knows is starred now calls `unstar()` and decrements the count back down.
 
 ## Scope
 
@@ -23,32 +23,40 @@ have re-called `star()` and double-incremented the client-displayed count each t
 - Excluded: No change to the FAVORITES-list unstar path (removing the row entirely already made
   its count irrelevant, no bug there). No change to how the initial list load populates
   `starsCount` (that already came from Gitea's own response, unaffected). No attempt to fetch the
-  server's authoritative post-star count instead of incrementing client-side (see Design notes).
+  server's authoritative post-star/unstar count instead of adjusting it client-side (see Design
+  notes).
 
 ## Design notes
 
-- **Optimistic +1, not a re-fetch**: Gitea's star endpoint (`PUT`, 204 No Content) doesn't return
-  the new count, so getting the authoritative number back would mean a second network call
-  (re-fetching the repository) purely to update one number — not worth it for a value the app
-  already knows how to derive from something it just confirmed itself (the star succeeded, so the
-  count is exactly one higher than what was last displayed).
-- **The repeat-tap guard closes a real double-count bug, not just a hypothetical one**: because the
-  ALL list can't determine a repository's already-starred state on load (an existing, accepted
-  limitation from TASK-018/TASK-030), a `Starred` row's star icon stays enabled and tappable —
-  nothing before this fix stopped a second tap from calling `star()` again and incrementing the
-  displayed count a second time for a repository the user only starred once. The guard checks the
-  row's own `favoriteAction` before doing anything, so it also saves a redundant network call, not
-  just a redundant `+1`.
+- **Optimistic ±1, not a re-fetch**: Gitea's star/unstar endpoints (`PUT`/`DELETE`, 204 No Content)
+  don't return the new count, so getting the authoritative number back would mean a second network
+  call purely to update one number — not worth it for a value the app already knows how to derive
+  from something it just confirmed itself (the action succeeded, so the count is exactly one
+  higher or lower than what was last displayed). `starsCount` is floored at 0 via
+  `coerceAtLeast(0)` as a defensive guard, not because normal use can reach it.
+- **A real toggle, not a guard**: the first pass at this fix added an early-return guard that made
+  a repeat tap on an already-`Starred` row do nothing — that stopped the double-count bug but also,
+  as immediately reported, made the star impossible to undo from the ALL list without switching to
+  the FAVORITES tab. The actual fix branches on the row's own `favoriteAction`: not-yet-starred-this-
+  session taps call `star()` (`+1`, → `Starred`), already-starred-this-session taps call
+  `unstar()` (`-1`, → `Idle`) — this both fixes the double-count and restores the obviously-expected
+  behavior of tapping a filled star to unfill it.
+- **The underlying "can't tell if already starred on load" limitation is untouched**: this toggle
+  only tracks what happened *within this session's own interaction* with a row, same as
+  `favoriteAction` always has (TASK-018/TASK-030). A repository already starred before the app
+  session started still shows outlined/untapped until acted on directly.
 
 ## Acceptance criteria
 
 - [x] Starring a repository from the ALL list increments its displayed star count by exactly 1.
-- [x] Tapping an already-`Starred` row again does not call `star()` a second time and does not
-      change the displayed count further.
+- [x] Tapping an already-`Starred` row again un-stars it: calls `unstar()`, decrements the count by
+      1, and returns the row to `Idle` (outlined, tappable to star again).
+- [x] The count never displays below 0.
 - [x] Unstarring from the FAVORITES list is unaffected (row removal, no count to update).
 - [x] Build, unit tests, and lint pass.
-- [ ] Live confirmation from the human product owner that the count now visibly increments on tap —
-      installed on the device but not yet re-tested in this exact session.
+- [ ] Live confirmation from the human product owner that both starring (count up) and un-starring
+      (count back down) now work as expected from the ALL list — installed on the device but not
+      yet re-tested in this exact session.
 
 ## Required checks
 
@@ -60,28 +68,36 @@ have re-called `star()` and double-incremented the client-displayed count each t
 
 ## Evidence
 
-- `RepositoryListViewModel.kt`: `toggleFavorite`'s ALL-list success branch now updates
-  `starsCount = it.starsCount + 1` alongside `favoriteAction = Starred`; new early-return guard
-  when the row's `favoriteAction` is already `Starred`.
-- `RepositoryListViewModelTest.kt`: `repo()` test helper gained a `starsCount` parameter; two new
-  tests — starring increments the displayed count by 1; a repeat tap after a successful star is a
-  no-op (no second `star()` call, count stays at 1).
+- `RepositoryListViewModel.kt`: `toggleFavorite` now branches on whether the row is already
+  `Starred` this session — un-star path (`unstar()`, `starsCount - 1` coerced at 0, → `Idle`) and
+  star path (`star()`, `starsCount + 1`, → `Starred`) are both explicit `when` branches, replacing
+  the first pass's early-return no-op guard.
+- `RepositoryListViewModelTest.kt`: `repo()` test helper gained a `starsCount` parameter; tests
+  cover starring incrementing the count by 1, a repeat tap un-starring (calls both `star()` and
+  `unstar()`, decrements the count, returns to `Idle`), and a full star→unstar round trip landing
+  back at the original count.
 - `./gradlew compileDebugKotlin compileDebugUnitTestKotlin`: BUILD SUCCESSFUL.
-- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing (including the two new
-  ones above).
+- `./gradlew testDebugUnitTest`: BUILD SUCCESSFUL, all unit tests passing.
 - `./gradlew assembleDebug lintDebug`: BUILD SUCCESSFUL, 0 lint errors.
 - Installed on the human product owner's device over wireless `adb` (`adb install -r`, streamed
-  install succeeded); not yet re-tested live in this exact session.
+  install succeeded) twice across this task's two rounds; not yet re-tested live against the final
+  toggle behavior in this exact session.
 
 ## Known limitations
 
-- Still optimistic, not authoritative: if Gitea's real count differs from this app's local
-  increment for any reason (e.g. another client starred the same repo around the same time), the
+- Still optimistic, not authoritative: if Gitea's real count differs from this app's local ±1 for
+  any reason (e.g. another client starred/unstarred the same repo around the same time), the
   displayed number stays off until the next full list reload. Considered an acceptable tradeoff
-  given the alternative (a second network call per star) for a rarely-contended personal count.
+  given the alternative (a second network call per action) for a rarely-contended personal count.
 - The underlying "can't tell if already starred on initial load" limitation (TASK-018/TASK-030)
-  is unchanged — this task only stops it from causing a *visible count* bug, it doesn't resolve the
-  limitation itself.
+  is unchanged — a repository starred before the current app session still shows outlined until
+  acted on directly in this session, same as before this task.
+- A failed *un*star attempt (network error mid-toggle) still collapses to the same flat `Failed`
+  state as a failed star attempt, which renders as outlined/not-starred on the ALL list even though
+  the repository may still actually be starred server-side — `FavoriteActionUiState.Failed` carries
+  no memory of which action was being attempted. Accepted as a minor, retryable rendering gap
+  rather than adding state to track it, since a retry from that visual state is harmless either way
+  (an unwanted second `star()` on an already-starred repo is a no-op server-side).
 
 ## Review findings