Skip to content

WEEK aggregation: groups use the locale week, bounds use the ISO week #1454

Description

@ahmetabdullahgultekin

WEEK aggregation groups measurements with a locale-dependent week definition but computes the period boundaries with a fixed ISO one. When the two disagree, a group's own members fall outside the bounds reported for that group.

The mismatch

Grouping and periodKey use the locale's first-day-of-week:

  • core/usecase/MeasurementAggregationUseCase.kt:86WeekFields.of(Locale.getDefault())
  • core/usecase/MeasurementAggregationUseCase.kt:99date.get(wf.weekOfWeekBasedYear())
  • core/data/Enums.kt:511periodKey, same locale lookup

But periodBounds is hardcoded to ISO:

  • core/data/Enums.kt:488WEEK -> { val mon = date.with(DayOfWeek.MONDAY); mon to mon.plusWeeks(1) }

Demonstration

Run under Locale.US (first day of week = Sunday), with two measurements: Sun 2025-04-06 and Mon 2025-04-07.

US first day of week : SUNDAY
Apr 6 (SUNDAY) week = 15  ┐ grouped together
Apr 7 (MONDAY) week = 15  ┘
periodBounds from minTs (Apr 6) = [2025-03-31, 2025-04-07)
does that range contain its own group member Apr 7? -> false

minTs is the Sunday, and with(DayOfWeek.MONDAY) walks backwards to the ISO week's Monday (Mar 31), so the reported period is the preceding ISO week — excluding the Monday that is in the same group, and covering Mar 31–Apr 5 which are not.

Consumers that filter by these bounds therefore select the wrong rows, e.g. ui/screen/table/TableScreen.kt:278-279 and :374-375.

Under Locale.GERMANY (first day = Monday) the two definitions agree and the bug is invisible — which is likely why it has survived.

Why it is not covered

app/src/test/java/com/health/openscale/core/usecase/MeasurementAggregationUseCaseTest.kt contains no occurrence of "week" at all — the existing tests cover empty input, NONE, and DAY grouping only. So neither the grouping nor the bounds are guarded for WEEK.

Possible directions

Either make periodBounds locale-aware to match the grouping:

WEEK -> {
    val firstDay = WeekFields.of(Locale.getDefault()).firstDayOfWeek
    val start = date.with(TemporalAdjusters.previousOrSame(firstDay))
    start to start.plusWeeks(1)
}

…or make grouping ISO to match the bounds (WeekFields.ISO), which is more predictable across devices but changes what users in Sunday-first locales see. The first keeps current user-visible grouping behaviour; the second makes the data locale-independent. Your call which is intended — I couldn't tell from the code whether locale-sensitive weeks are a deliberate feature.

Happy to send a PR with either direction plus tests covering both locales, if you tell me which you prefer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugIndicates an unexpected problem or unintended behavior

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions