test(api): FsCheck property tests for domain invariants (#299)

[thomasluizon]

Property-based tests (FsCheck.Xunit 3.3.3) over pure domain/application logic. API analog of orbit-ui-mobile#418.

24 [Property] tests (~2,400 generated cases, seed-pinned) covering: overdue/DueDate rules, streak monotonicity (+ two-engine agreement), skip=Value0 semantics, XP/level-ladder bounds, log→unlog idempotency, and value-object JSON round-trips — with bounded generators exercising all 4 frequency units + one-time/flexible + monthly month-end clamp + yearly Feb-29 anchors.

Merges before #298 (its mutation score depends on these tests). No production code touched. Validated: build green; Domain 503 / Application 2615 / Infrastructure 1387 pass; stress-tested at 72k cases with zero generator failures. (Flagged but not fixed: a real-but-unreachable int32 overflow in XpRequiredForLevel at ~level 4634 / ~2.1e9 XP — bounded around, not a live risk.)

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #307 — test(api): FsCheck property tests for domain invariants

Scope: PR #307 in thomasluizon/orbit-api
Recommendation: APPROVE

Summary

Test-only PR (no src/ changes) adding 24 FsCheck property tests across the Domain and Application test projects, covering habit scheduling, streaks, gamification XP/level ladder, log/unlog idempotency, and JSON round-trips. Reviewed the new generators (OrbitArbitraries) and all six new test files against the production logic in HabitScheduleService.cs, LevelDefinitions.cs, Habit.cs, and User.cs. No correctness bugs found; two property tests are tautological or under-exercise the behavior their name claims to cover.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] CompletedAndSkippedWindows_PartitionInWindowLogs is tautological — it can never fail
· dimension: Correctness / test-coverage (rubric #1)
· location: tests/Orbit.Application.Tests/Services/HabitSchedulePropertyTests.cs:76-97
· issue: The test derives start/end via HabitScheduleService.GetWindowStart/GetWindowEnd — the same helpers GetCompletedInWindow/GetSkippedInWindow call internally — then asserts the production count equals a manually-filtered count using the identical predicate (l.Value > 0 / l.Value == 0) over that same window. Both sides reduce to the exact same pure computation.
· risk: A real regression in window-boundary math (e.g. GetWindowStart mis-anchoring a monthly/yearly window) would be reproduced identically on both sides of the assertion, so this test would still pass — it provides no signal despite reading like a partition-invariant check.
· fix: Derive start/end independently of GetWindowStart/GetWindowEnd (e.g. compute the calendar week/month/year boundary directly in the test) so the assertion is a real oracle, not a restatement of the code under test.
· reference: orbit-api CLAUDE.md "Testing" — new tests should verify behavior, not restate it.

[MEDIUM] StreakEngines_AgreeAtAnchor never exercises the lookback-seed behavior it claims to test
· dimension: Correctness / test-coverage (rubric #1)
· location: tests/Orbit.Application.Tests/Services/HabitSchedulePropertyTests.cs:126-140
· issue: BuildStreakSeries is invoked with seedFrom == from on every run, so its walk covers exactly the same date range as ComputeStreakAsOf's, and both share the same private StreakRun.Advance engine — with seedFrom == from the "two-engine agreement" is guaranteed by inspection, not exercised. The one parameter that actually differentiates BuildStreakSeries — a seedFrom before from, per its own XML doc ("a lookback before from so the streak entering the window is correct") — is never set to a different value.
· risk: A regression in the seed-lookback logic (the streak carried into the window from before from) would not be caught by this property, despite the PR description advertising "two-engine agreement" as covered.
· fix: Generate seedFrom as from.AddDays(-lookback) for some positive lookback, and assert agreement on the emitted point at to so the test actually walks the lookback path before comparing.
· reference: orbit-api CLAUDE.md "Testing" — new tests should verify behavior.

Low / Info

None posted (signal gate).

Subagents

Agent	Verdict
security-reviewer	N/A — no src/ changes in this diff (test-only PR)
contract-aligner	N/A — no DTO/Controller/packages/shared surface touched

Validation

Check	Result
Build (dotnet)	N/A — skipped in CI; Build/Unit Tests run as separate required checks
Tests (dotnet)	N/A — skipped in CI; Build/Unit Tests run as separate required checks

What's good

• Generators are bounded and deterministic (seed-pinned Replay), avoiding pathological day-walks while covering all 4 frequency units, one-time, flexible, monthly month-end clamp, and yearly Feb-29 anchors.
• SafeTextGen explicitly avoids lone UTF-16 surrogates that would break System.Text.Json round-trip equality — a real, easy-to-miss edge case for the JSON round-trip properties.
• Streak monotonicity, XP monotonicity, and freeze-award bounding properties are genuine, non-circular oracles (bounds/ordering asserted independently of the implementation), and correctly account for the "unresolved anchor" carry-forward semantics in HasMissedPastOccurrence/IsOverdueOnDate.
• All new comments are /// XML-doc — no bare narration comments, compliant with ORBIT0001.
• PR body transparently flags the pre-existing, unreached int32-overflow edge in XpRequiredForLevel at ~level 4634 rather than silently masking it — the LadderLevel arbitrary correctly caps at 2000 to stay well clear of it.

Recommendation

No blocking issues — this is a clean, well-scoped test-only PR. The two Medium findings are optional follow-ups (strengthen two specific property assertions so they'd actually catch a regression); neither blocks merge.

🤖 Generated with Claude Code via the /pr-review skill