mirror of
https://github.com/kuhyx/screen-locker.git
synced 2026-11-13 21:50:54 +01:00
Stop the phone counting deleted and empty manual workouts
Some checks are pending
md-naming / markdown naming convention (push) Waiting to run
pre-commit / pre-commit (push) Waiting to run
Tests / test (3.10) (push) Waiting to run
Tests / test (3.11) (push) Waiting to run
Tests / test (3.12) (push) Waiting to run
Tests / lock-popup-safety (push) Waiting to run
release-apk / release (APK) (push) Waiting to run
Some checks are pending
md-naming / markdown naming convention (push) Waiting to run
pre-commit / pre-commit (push) Waiting to run
Tests / test (3.10) (push) Waiting to run
Tests / test (3.11) (push) Waiting to run
Tests / test (3.12) (push) Waiting to run
Tests / lock-popup-safety (push) Waiting to run
release-apk / release (APK) (push) Waiting to run
The app said "manual budget exhausted for this window" (2/2w, 4/10m) while
the PC read 1/2w, 2/10m for the same records. The PC was right; two
independent Dart defects each inflated a window by one.
`_readMergedPayloads` never looked at `Record.deleted`, so the tombstoned
duplicate `manual:2026-08-21T21:00` -- the second of two logged on 08-21,
already deleted -- kept consuming a weekly slot. Suppression is collected
across the whole device union and applied after the merge, not per file:
one device tombstoning a record while another still holds it live must
delete it, and a per-file skip would just let the live copy win. This
mirrors the PC's `_tombstoned_ids`.
`countManualBudget` also counted `manual_workout:2026-08-15`, a bare
`{type, kind, date}` stub with no evidence fields. The PC refuses to ingest
those ("no workout data to ingest; skipping permanently"), so the phone was
charging budget for a workout neither device ever accepted. The filter lives
in the counter rather than the shared reader because `runnerup_verified` and
`pc_workout_verified` payloads legitimately carry no `start_time`, and the
history view must keep showing them.
Covered by a shared literal fixture both languages read, rather than two
independent round-trips: each side's own tests passed while the pair
disagreed, which is exactly how this shipped. Verified by reverting each
guard in turn and watching the twin fail.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AWq9MRemX9ZWvNmnhNWUP2
This commit is contained in:
parent
e2d08f33a7
commit
81ebccb51a
64
screen_locker/tests/fixtures/manual_budget_merge.json
vendored
Normal file
64
screen_locker/tests/fixtures/manual_budget_merge.json
vendored
Normal file
@ -0,0 +1,64 @@
|
||||
{
|
||||
"_comment": [
|
||||
"Shared cross-language fixture for manual-workout budget counting.",
|
||||
"Derived from a real divergence on 2026-08-27: the phone showed",
|
||||
"'Manual: 2/2w 4/10m -- budget exhausted' while the PC read 1/2w 2/10m.",
|
||||
"Two Dart-only defects caused it, both represented below:",
|
||||
" 1. the phone's merge ignored tombstones, so the deleted duplicate",
|
||||
" manual:2026-08-21T21:00 still consumed a weekly slot;",
|
||||
" 2. the phone counted the evidence-less stub manual_workout:2026-08-15,",
|
||||
" which the PC refuses to ingest, costing an extra monthly slot.",
|
||||
"Each side's own round-trip test passed while the pair disagreed, so this",
|
||||
"literal -- read by both -- is the only thing that catches the drift."
|
||||
],
|
||||
"today": "2026-08-27",
|
||||
"records": [
|
||||
{
|
||||
"id": "manual:2026-08-11T18:00",
|
||||
"deleted": false,
|
||||
"payload": {
|
||||
"kind": "manual_workout",
|
||||
"type": "manual_workout",
|
||||
"date": "2026-08-11",
|
||||
"start_time": "18:00",
|
||||
"end_time": "19:00"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "manual_workout:2026-08-15",
|
||||
"deleted": false,
|
||||
"payload": {
|
||||
"kind": "manual_workout",
|
||||
"type": "manual_workout",
|
||||
"date": "2026-08-15"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "manual:2026-08-21T20:00",
|
||||
"deleted": false,
|
||||
"payload": {
|
||||
"kind": "manual_workout",
|
||||
"type": "manual_workout",
|
||||
"date": "2026-08-21",
|
||||
"start_time": "20:00",
|
||||
"end_time": "21:00"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "manual:2026-08-21T21:00",
|
||||
"deleted": true,
|
||||
"payload": {
|
||||
"kind": "manual_workout",
|
||||
"type": "manual_workout",
|
||||
"date": "2026-08-21",
|
||||
"start_time": "21:00",
|
||||
"end_time": "22:00"
|
||||
}
|
||||
}
|
||||
],
|
||||
"expected": {
|
||||
"week": 1,
|
||||
"month": 2,
|
||||
"exhausted": false
|
||||
}
|
||||
}
|
||||
96
screen_locker/tests/test_manual_budget_merge_fixture.py
Normal file
96
screen_locker/tests/test_manual_budget_merge_fixture.py
Normal file
@ -0,0 +1,96 @@
|
||||
"""Cross-language fixture test for manual-workout budget counting.
|
||||
|
||||
The Dart app mirrors this accounting in ``lib/models/manual_workout_budget.dart``
|
||||
and ``lib/services/workout_sync_service_read.dart``. Each language's own
|
||||
round-trip test passed happily while the two sides disagreed -- on 2026-08-27
|
||||
the phone read ``2/2w 4/10m (exhausted)`` for the very records this side read
|
||||
as ``1/2w 2/10m`` -- so the only thing that catches the drift is a shared
|
||||
literal both read: ``fixtures/manual_budget_merge.json``. The Dart twin of
|
||||
this file is ``test/models/manual_budget_merge_fixture_test.dart``; they must
|
||||
stay in step.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from screen_locker._log_mixin import write_signed_entry
|
||||
from screen_locker._manual_workout import (
|
||||
MANUAL_WORKOUT_TYPE,
|
||||
count_in_window,
|
||||
is_budget_exhausted,
|
||||
)
|
||||
|
||||
FIXTURE = Path(__file__).parent / "fixtures" / "manual_budget_merge.json"
|
||||
|
||||
|
||||
def _fixture() -> dict[str, Any]:
|
||||
"""Load the shared cross-language budget fixture."""
|
||||
loaded: dict[str, Any] = json.loads(FIXTURE.read_text(encoding="utf-8"))
|
||||
return loaded
|
||||
|
||||
|
||||
def _live_payloads(records: list[dict[str, Any]]) -> list[dict[str, Any]]:
|
||||
"""Drop tombstoned records, as the sync reader does across the union."""
|
||||
return [r["payload"] for r in records if not r["deleted"]]
|
||||
|
||||
|
||||
def _log_from(payloads: list[dict[str, Any]], log_file: Path) -> Path:
|
||||
"""Write the ingestable payloads into a real signed log file.
|
||||
|
||||
Stubs without ``start_time`` are skipped exactly as ``_manual_sync``
|
||||
refuses them ("no workout data to ingest"), so the fixture exercises the
|
||||
same admission rule the live ingest applies.
|
||||
"""
|
||||
for payload in payloads:
|
||||
if not isinstance(payload.get("start_time"), str):
|
||||
continue
|
||||
workout_data = {k: v for k, v in payload.items() if k != "kind"}
|
||||
workout_data["type"] = MANUAL_WORKOUT_TYPE
|
||||
workout_data["workout_id"] = f"manual:{payload['date']}T{payload['start_time']}"
|
||||
write_signed_entry(log_file, str(payload["date"]), workout_data)
|
||||
return log_file
|
||||
|
||||
|
||||
def test_budget_matches_the_phone_for_the_2026_08_27_divergence(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""The window counts must equal the numbers the Dart twin asserts."""
|
||||
fixture = _fixture()
|
||||
expected = fixture["expected"]
|
||||
today = str(fixture["today"])
|
||||
log_file = _log_from(_live_payloads(fixture["records"]), tmp_path / "log.json")
|
||||
|
||||
assert count_in_window(log_file, 7, today=today) == expected["week"]
|
||||
assert count_in_window(log_file, 30, today=today) == expected["month"]
|
||||
assert is_budget_exhausted(log_file, today=today) is expected["exhausted"]
|
||||
|
||||
|
||||
def test_a_tombstoned_duplicate_frees_its_weekly_slot(tmp_path: Path) -> None:
|
||||
"""Resurrecting the deleted duplicate must exhaust the week.
|
||||
|
||||
This is the pre-fix behaviour the phone exhibited; asserting it proves the
|
||||
tombstone is load-bearing rather than incidentally outside the window.
|
||||
"""
|
||||
fixture = _fixture()
|
||||
today = str(fixture["today"])
|
||||
resurrected = [r["payload"] for r in fixture["records"]]
|
||||
log_file = _log_from(resurrected, tmp_path / "log.json")
|
||||
|
||||
assert count_in_window(log_file, 7, today=today) == 2
|
||||
assert is_budget_exhausted(log_file, today=today) is True
|
||||
|
||||
|
||||
def test_an_evidence_less_stub_never_consumes_budget(tmp_path: Path) -> None:
|
||||
"""The `{type, kind, date}` stub must cost nothing in either window."""
|
||||
fixture = _fixture()
|
||||
today = str(fixture["today"])
|
||||
stub = next(
|
||||
r["payload"] for r in fixture["records"] if "start_time" not in r["payload"]
|
||||
)
|
||||
log_file = _log_from([stub], tmp_path / "log.json")
|
||||
|
||||
assert count_in_window(log_file, 7, today=today) == 0
|
||||
assert count_in_window(log_file, 30, today=today) == 0
|
||||
@ -24,6 +24,12 @@ ManualBudget countManualBudget(
|
||||
var week = 0;
|
||||
var month = 0;
|
||||
for (final payload in payloads) {
|
||||
// Skip evidence-less `{type, kind, date}` stubs. The PC refuses to ingest
|
||||
// these ("no workout data to ingest; skipping permanently"), so counting
|
||||
// them made the phone's budget stricter than the PC's for a workout that
|
||||
// neither device ever accepted. `start_time` is the discriminator: every
|
||||
// real self-report has one, and no stub does.
|
||||
if (payload['start_time'] is! String) continue;
|
||||
final dateStr = payload['date'];
|
||||
if (dateStr is! String) continue;
|
||||
final date = DateTime.tryParse(dateStr);
|
||||
|
||||
@ -64,13 +64,21 @@ extension WorkoutSyncServiceRead on WorkoutSyncService {
|
||||
: MirrorStore(primary: firebase, mirror: github);
|
||||
try {
|
||||
final merged = <String, Record>{};
|
||||
final tombstoned = <String>{};
|
||||
for (final device in await client.listDirectory(_pathPrefix)) {
|
||||
final text = await client.getFileText(
|
||||
'$_pathPrefix/$device/$_logFilename',
|
||||
);
|
||||
if (text == null) continue;
|
||||
_mergeRecords(_decode(text), merged, kind: kind);
|
||||
final log = _decode(text);
|
||||
_collectTombstones(log, tombstoned);
|
||||
_mergeRecords(log, merged, kind: kind);
|
||||
}
|
||||
// Suppression is applied across the whole device union, never per file:
|
||||
// one device tombstoning a record while another still holds it live must
|
||||
// delete it, and a per-file skip would just let the live copy win the
|
||||
// merge. Mirrors the PC's `_tombstoned_ids` (screen_locker/_sync_records.py).
|
||||
tombstoned.forEach(merged.remove);
|
||||
return merged.values
|
||||
.map((r) => (r.fields['payload']!.$1! as Map).cast<String, dynamic>())
|
||||
.toList();
|
||||
@ -80,6 +88,17 @@ extension WorkoutSyncServiceRead on WorkoutSyncService {
|
||||
}
|
||||
}
|
||||
|
||||
/// Collects the ids this device log marks deleted, into [into].
|
||||
///
|
||||
/// Gathered before the kind filter runs, because a tombstone carries no
|
||||
/// payload to match on: filtering first would drop the deletion and let the
|
||||
/// live copy from another device survive the merge.
|
||||
static void _collectTombstones(Log log, Set<String> into) {
|
||||
for (final entry in log.entries) {
|
||||
if (entry.value.deleted) into.add(entry.key);
|
||||
}
|
||||
}
|
||||
|
||||
/// Merges records into [into], keeping the highest-HLC copy of each id.
|
||||
///
|
||||
/// [kind] filters on the payload's `kind` discriminator; pass null to keep
|
||||
|
||||
@ -0,0 +1,113 @@
|
||||
@TestOn('vm')
|
||||
library;
|
||||
|
||||
import 'dart:convert';
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:crdt_sync/crdt_sync.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:workout_app/models/manual_workout.dart';
|
||||
|
||||
/// Cross-language fixture test for manual-workout budget counting.
|
||||
///
|
||||
/// The locker mirrors this accounting in `screen_locker/_manual_workout.py`
|
||||
/// and `screen_locker/_sync_records.py`. Each language's own round-trip test
|
||||
/// passed happily while the two sides disagreed -- on 2026-08-27 this phone
|
||||
/// read `2/2w 4/10m (exhausted)` for the very records the PC read as
|
||||
/// `1/2w 2/10m` -- so the only thing that catches the drift is a shared
|
||||
/// literal both read. The Python twin is
|
||||
/// `screen_locker/tests/test_manual_budget_merge_fixture.py`.
|
||||
///
|
||||
/// The fixture lives in the locker's test tree because that is the side that
|
||||
/// owns the budget; this app reads it up four directories.
|
||||
final File _fixtureFile = File(
|
||||
'../../screen_locker/tests/fixtures/manual_budget_merge.json',
|
||||
);
|
||||
|
||||
Map<String, dynamic> _fixture() =>
|
||||
jsonDecode(_fixtureFile.readAsStringSync()) as Map<String, dynamic>;
|
||||
|
||||
/// Rebuilds the fixture as a device log, exactly as a sync backend serves it.
|
||||
///
|
||||
/// Going through `Record`/`Log` rather than handing the payloads straight to
|
||||
/// the counter is the point: the tombstone that caused the incident only
|
||||
/// exists at the record layer, so a test that skipped it could not fail.
|
||||
String _deviceLogJson(List<dynamic> records) {
|
||||
final hlc = Hlc.fromStr('2026-08-27T00:00:00.000Z-0000-fixture');
|
||||
final log = <String, Record>{
|
||||
for (final entry in records.cast<Map<String, dynamic>>())
|
||||
entry['id'] as String: Record(
|
||||
id: entry['id'] as String,
|
||||
fields: {'payload': (entry['payload'], hlc)},
|
||||
deleted: entry['deleted'] as bool,
|
||||
deletedHlc: (entry['deleted'] as bool) ? hlc : null,
|
||||
),
|
||||
};
|
||||
return jsonEncode(log.map((id, r) => MapEntry(id, r.toJson())));
|
||||
}
|
||||
|
||||
/// Applies the same union-wide suppression the reader does.
|
||||
List<Map<String, dynamic>> _livePayloads(String logJson) {
|
||||
final raw = jsonDecode(logJson) as Map<String, dynamic>;
|
||||
final live = <Map<String, dynamic>>[];
|
||||
for (final data in raw.values) {
|
||||
final record = Record.fromJson(data as Map<String, dynamic>);
|
||||
if (record.deleted) continue;
|
||||
final payload = record.fields['payload']?.$1;
|
||||
if (payload is Map) live.add(payload.cast<String, dynamic>());
|
||||
}
|
||||
return live;
|
||||
}
|
||||
|
||||
void main() {
|
||||
test('budget matches the locker for the 2026-08-27 divergence', () {
|
||||
final fixture = _fixture();
|
||||
final expected = fixture['expected'] as Map<String, dynamic>;
|
||||
final today = DateTime.parse(fixture['today'] as String);
|
||||
|
||||
final payloads = _livePayloads(
|
||||
_deviceLogJson(fixture['records'] as List<dynamic>),
|
||||
);
|
||||
final budget = countManualBudget(payloads, today);
|
||||
|
||||
expect(budget.week, expected['week']);
|
||||
expect(budget.month, expected['month']);
|
||||
expect(budget.exhausted, expected['exhausted']);
|
||||
});
|
||||
|
||||
test('a tombstoned duplicate frees its weekly slot', () {
|
||||
final fixture = _fixture();
|
||||
final records = fixture['records'] as List<dynamic>;
|
||||
final today = DateTime.parse(fixture['today'] as String);
|
||||
|
||||
// Resurrecting the deleted duplicate is precisely the pre-fix behaviour:
|
||||
// it must push the week to the cap, proving the tombstone is load-bearing
|
||||
// rather than incidentally absent from the window.
|
||||
final resurrected = [
|
||||
for (final entry in records.cast<Map<String, dynamic>>())
|
||||
{...entry, 'deleted': false},
|
||||
];
|
||||
final budget = countManualBudget(
|
||||
_livePayloads(_deviceLogJson(resurrected)),
|
||||
today,
|
||||
);
|
||||
|
||||
expect(budget.week, 2);
|
||||
expect(budget.exhausted, isTrue);
|
||||
});
|
||||
|
||||
test('an evidence-less stub never consumes budget', () {
|
||||
final fixture = _fixture();
|
||||
final today = DateTime.parse(fixture['today'] as String);
|
||||
final stub = (fixture['records'] as List<dynamic>)
|
||||
.cast<Map<String, dynamic>>()
|
||||
.firstWhere((e) => !(e['payload'] as Map).containsKey('start_time'));
|
||||
|
||||
final budget = countManualBudget([
|
||||
(stub['payload'] as Map).cast<String, dynamic>(),
|
||||
], today);
|
||||
|
||||
expect(budget.week, 0);
|
||||
expect(budget.month, 0);
|
||||
});
|
||||
}
|
||||
@ -67,9 +67,22 @@ void main() {
|
||||
});
|
||||
|
||||
testWidgets('hides the form when the budget is exhausted', (tester) async {
|
||||
// Real self-reports, not bare {kind, date} stubs: the counter ignores
|
||||
// payloads with no `start_time`, because the PC refuses to ingest those
|
||||
// and a budget must not be spent on a workout neither device accepted.
|
||||
final payloads = [
|
||||
{'kind': 'manual_workout', 'date': '2026-07-12'},
|
||||
{'kind': 'manual_workout', 'date': '2026-07-13'},
|
||||
{
|
||||
'kind': 'manual_workout',
|
||||
'date': '2026-07-12',
|
||||
'start_time': '18:00',
|
||||
'end_time': '19:00',
|
||||
},
|
||||
{
|
||||
'kind': 'manual_workout',
|
||||
'date': '2026-07-13',
|
||||
'start_time': '18:00',
|
||||
'end_time': '19:00',
|
||||
},
|
||||
];
|
||||
await _pump(
|
||||
tester,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user