Bridge remaining period gap and fix data-loss in merge (follow-up to #166)#165
Merged
Conversation
Logging consecutive days used to fragment into disjoint period entries once a period's end date became fixed (e.g. via the "no end date set" confirmation): the next day no longer fell within any existing range, so tapping it silently started a brand-new, disconnected period with the in-between day unloggable in either one. PeriodRepository.resolvePeriodForLogging now checks, before creating a new entry, whether the logged date is adjacent to an existing period's start or end and extends it, or bridges a one-day gap between two periods and merges them. HomeScreen routes calendar taps and the Log Period FAB through this resolver instead of the old "covers today"-only check. The History tab also gains a manual "Merge with previous period" action per card (via PeriodRepository.mergePeriods, now reusable outside the one-time migration) so users can fix up any already-fragmented history that predates this fix or has a gap too large to auto-detect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WUWr5zTAXGDw1EooCpHxfy
…-corruption-smyc69 # Conflicts: # app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryScreen.kt # app/src/main/java/com/mapgie/goflo/ui/screens/history/HistoryViewModel.kt # app/src/main/java/com/mapgie/goflo/ui/screens/home/HomeScreen.kt # app/src/test/java/com/mapgie/goflo/data/repository/PeriodRepositoryTest.kt
Builds on #166's fix (periodForDate day-after-end adjacency, deferred extension, one-time migration). Two gaps remained against the exact reported scenario (a period ending day N, an unloggable day N+1, and a new period starting day N+2): - periodForDate's day-after-end match only reopens the earlier period for a deferred extension; if a second period already starts the following day, extending the first just leaves them touching, requiring a further manual History merge. PeriodRepository.mergeGapAt detects this and merges both immediately when the tapped date bridges them, wired through HomeScreen/HomeViewModel. - mergeAdjacentPeriods (the one-time repair migration) only merged already-touching periods (zero-day gap), not periods separated by the single unlogged day this bug actually produces. Extended its gap check from "touching only" to "touching or one day apart". - Both the manual History merge and the migration were calling deleteLogsForPeriod on the absorbed period — a helper written for actual period deletion — silently discarding its flow/symptom tracking-log history instead of preserving it. Removed those calls; the data is still valid, dated history and isn't orphaned by merging. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WUWr5zTAXGDw1EooCpHxfy
mapgie
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This branch originally duplicated work that landed independently in #166 (merged), which fixed the same underlying period-logging fragmentation bug via a different, and in several respects better, approach (deferred-until-Save persistence, a one-time repair migration, merge-with-either-neighbor in History). Rather than carry two competing implementations, this PR is now rebased on top of #166 and only adds what it was still missing, found by comparing the two approaches against the exact bug report (logging day 28 → 29 → 30 fragmented into a 1-day period on the 28th, an unloggable 29th, and a new ongoing period on the 30th).
What was still missing after #166
periodForDate's "day after explicit end" match reopens the earlier period for a deferred extension, but if a second, separate period already starts the following day (exactly the reported scenario), extending the first one just leaves the two touching — still two rows, still needing a manual History merge.PeriodRepository.mergeGapAtnow detects a tapped date bridging a one-day gap between two distinct periods and merges them immediately, wired throughHomeScreen/HomeViewModel.mergeAdjacentPeriodsonly merged periods that were already touching (zero-day gap); it never merged periods separated by the single unlogged day this bug actually produces. Extended the gap check from "touching only" to "touching or one day apart".deleteLogsForPeriod(a helper written for actual period deletion) on the absorbed period, silently discarding its flow/symptom tracking-log history instead of preserving it. Removed those calls — that data is still valid, dated history, not orphaned by merging two period rows into one.Changes
PeriodRepository: newmergeGapAt;mergeAdjacentPeriodsgap check widened to 1-2 days.HomeViewModel/HomeScreen: route the gap-bridging check through the existing navigation flow.HistoryViewModel/GoFloApplication: removed the destructivedeleteLogsForPeriodcalls from both merge paths.PeriodRepositoryTest: new fake in-memoryPeriodDao/SymptomDaoand tests formergeGapAtand the widenedmergeAdjacentPeriodsgap detection.LESSONS.md: consolidated the two (now-overlapping) lessons left by the parallel work into one.changelog/unreleased/period-logging-gap-merge-fix.json.Test plan
PeriodRepositoryTestpass in CI (can't run Gradle in this environment).python3 a11y_check.pypasses (verified locally, clean).https://claude.ai/code/session_01WUWr5zTAXGDw1EooCpHxfy