Add utilities

[283375]

No description provided.

[283375]

/opencode /review Focus on the new components rather than the utilities only in this conversation.

[opencode-agent]

Here's my review focusing on the new components:

---

Review: New Components (Focusing on issues)

DecimalStepperTextField.kt

1. Misleading default minValue (lines 58, 169)
BigDecimal.fromDouble(Double.MIN_VALUE) produces ~4.9E-324 (the smallest positive double, not negative infinity). This means the default minimum is effectively zero. Stepping down from 0 with step=1.0 would produce -1.0, which gets silently clamped to ~4.9E-324 — a bizarre in-between state. Use a sentinel approach (e.g., null to mean unbounded) or explicitly use BigDecimal.fromDouble(-Double.MAX_VALUE).

2. commitValue on focus loss defaults to zero (line 275)
state.commitValue(state.value ?: BigDecimal.ZERO) — if the field is empty or contains only -/. (intermediate states), focus loss forces it to zero, erasing the user's partial input. Consider preserving the last valid value instead.

3. runCatching swallows all exceptions silently (line 73)
runCatching { ... }.getOrNull() in the value property swallows not just parse failures but also runtime exceptions from the bignum library. Be explicit about expected failures (e.g., toBigDecimalOrNull() or catch only NumberFormatException).

4. Unnecessary !! after null checks in test (line 255)
Minor: state.value?.doubleValue() ?: 0.0 is fine on 255, but comment about flaky clock behavior on lines 253-254 should be fixed — Compose test clock is deterministic.

---

ArcaeaScoreTextField.kt

5. No @Preview composable
All other new component files have previews; this one doesn't. Trivial omission.

6. No dedicated unit test
DecimalStepperTextField has 292 lines of tests but ArcaeaScoreTextField (177 lines, with complex cursor-restoration logic in transformInput) has none. The cursor position tracking through countDigitsUpTo/findPositionForDigits deserves test coverage, especially for edge cases like pasting in the middle of existing text.

7. abs(value) silently inverts negatives (line 28)
createInitialText uses abs(value). If a negative value is ever passed, it's silently converted to positive. Consider adding a precondition check or clamping.

---

PlayRatingCalculator.kt

8. Unnecessary !! (line 63)
calculatePlayRating(scoreValue!!, constantValue!!) — after the ?: return@derivedStateOf null guards on lines 59-60, Kotlin smart-casts both properties to non-null types. The !! operators are dead code. Remove them for idiomatic Kotlin.

9. BigDecimal * Int → intValue() precision (line 52-53)
it * 10 then .intValue(). The bignum library's intValue() truncates (like Java's BigDecimal.intValue()). For the Arcaea constant domain (0.0–99.0 in 0.1 steps) this is correct, but if the stepper were ever reused with different constraints, truncation instead of rounding could produce off-by-one constants. Consider using .roundToLong() or .intValue() with appropriate RoundingMode.

---

UtilitiesCalculatorScreen.kt

10. No empty/null state handling
When charts is empty (no song selected), enabledRatingClasses and ratingDetails are empty, but the ArcaeaRatingClassSelector and PlayRatingCalculator still render. The rating class selector with no enabled classes renders an empty row, and the calculator shows ? for potential — both are graceful, but there's no explicit "select a song first" guidance.

---

DecimalStepperTextFieldTest.kt

11. Flaky long-press test (lines 230–271)
The test advances clock manually then asserts >= 6.0 and fuzzy bounds. The Compose test clock is deterministic — the test should calculate the exact expected value (initial 1.0 + first click 1.0 = 2.0, then delays of 500ms initial + 525ms = 6 repeats at 80ms each, so 2.0 + 7.0 = ~9.0) and assert precisely. The fuzzy bounds weaken test reliability.

---

Summary

Severity	#	File	Issue
Medium	1	DecimalStepperTextField.kt:58	Double.MIN_VALUE as default min is incorrect
Low	2	DecimalStepperTextField.kt:275	Focus loss defaults to zero, discarding partial input
Low	3	DecimalStepperTextField.kt:73	Overly broad runCatching
Low	6	ArcaeaScoreTextField.kt	Missing test coverage for cursor logic
Style	8	PlayRatingCalculator.kt:63	Dead !! operators
Low	11	DecimalStepperTextFieldTest.kt:230	Fuzzy test assertions with deterministic clock

Overall the components are well-structured with good separation of concerns, proper Compose patterns (derivedStateOf, remember, LaunchedEffect), and solid test coverage for the stepper. The main concerns are the default minValue semantics and the missing test coverage for the score text field.

opencode session  |  github run

[283375]

/opencode /review pr

[opencode-agent]

Here is my review of the PR:

---

Review: Add utilities (42 files, +1997/-248)

Notable issues

DecimalStepperTextField.kt:174,201 — Double.MIN_VALUE as default min is incorrect
The class constructor (line 58) correctly defaults to -Double.MAX_VALUE, but both remember composables default to Double.MIN_VALUE (~4.9E-324, the smallest positive double). Stepping down from 0.0 produces -1.0, clamped to ~4.9E-324, rendered as "0.00". Use -Double.MAX_VALUE consistently.

ArcaeaPlayResultValidatorWarnings.kt:77 — Dead file path

import xyz.sevive.arcaeaoffline.core.calculators.calculateScoreRange

The import references calculateScoreRange, but the file is in app/ and uses app/src/main/java/... while the function was moved to core/. Also, lines 77, 99, 125, 182 use !! after null checks — the smart-cast doesn't propagate through the lambda, but they could use safe chaining or .let {}.

UtilitiesChartRecommendScreen.kt:184 — Mismatched tolerance between UI and function default
calculateInvertScoreRange is called with tolerance = 1e-6 in ResultsListItemState, but the function signature defaults to 1e-3. This means scores shown in the list and scores from the calculator dialog could disagree for borderline play ratings. Either document the tighter tolerance or use the same value consistently.

SelectDialog.kt:39 — BasicAlertDialogSurface padding unused
The lambda receives contentPadding but it's ignored. The surface adds 24dp padding on all sides, while items have their own padding(horizontal = 16.dp, vertical = 8.dp). This creates double padding that may look unintentional.

ArcaeaScoreTextField.kt:28 — abs(value) silently inverts negative scores
If a caller passes a negative value to createInitialText, it's silently converted to positive. Either add a precondition check or document this behavior.

---

Code style / minor

PlayRatingCalculator.kt:74 — Redundant !! operators after null guard
Lines 71-72 already return@derivedStateOf null if either value is null, so scoreValue!! and constantValue!! on line 74 can drop the !!. (Kotlin smart-casts after null check in a derivedStateOf block.)

PlayRatingCalculator.kt:51 — constantTextFieldState.commitValue(constant / 10.0)
This is an Int / Double expression that could be written as constant.toDouble() / 10.0 or constant / 10.0 (Kotlin converts). It's fine functionally but inconsistent with the score branch which uses no conversion.

ArcaeaFormatters.kt:23 — Dead branch for scores >= 99,999,999
Arcaea scores top out at ~10,002,221 (10M + max notes). The >= 99_999_999 guard is unreachable. The code path would work correctly even without it, but it's dead code.

ArcaeaScoreTextFieldState.updateValue() — No cursor preservation
Calls replace(0, length, newText) which resets cursor to end. Compare to ArcaeaScoreInputTransformation.transformInput which carefully preserves cursor position through digit-count mapping. If updateValue is called programmatically while the user is editing, the cursor will jump.

---

Missing coverage

ArcaeaScoreTextField has no dedicated tests
The cursor-restoration logic (countDigitsUpTo, findPositionForDigits, digit trimming on paste/deletion at midpoint) is nontrivial (82 lines of transform logic) but has zero test coverage. The DecimalStepperTextField has 286 lines of tests; the score text field deserves at least basic coverage for:

• Pasting 9+ digits at cursor position
• Deleting at midpoint (zero-padding behavior)
• Cursor position after digits insertion/removal at various positions

calculateInvertScoreRange — missing edge case test
The CommonCalculatorsTest tests three valid cases and one null return, but doesn't test boundary conditions like:

• targetPlayRating = 0.0
• constant = 0
• Tolerance interaction (does 1e-6 produce different results than 1e-3?)

---

Structural notes (non-blocking)

• The rememberLastNotNullValue composable in PlayResultEditorFields.kt:36 doesn't appear to be used — check if it's accessed from other files.
• The ArcaeaFormatters.score() function (new) duplicates the formatting logic from ArcaeaScoreInputTransformation.formatWithThousandsSeparator — both format 8-digit strings with ' separators but use different implementations. Consider consolidating.
• The rename from "potential" to "play rating" across 8+ files is clean and consistent.

---

Summary

Priority	File	Issue
High	DecimalStepperTextField.kt:174	Default minValue is Double.MIN_VALUE instead of -Double.MAX_VALUE
Medium	UtilitiesChartRecommendScreen.kt:184	Tolerance mismatch between UI (1e-6) and default (1e-3)
Medium	ArcaeaScoreTextField	No tests for cursor-restoration logic
Low	PlayRatingCalculator.kt:74	Redundant !! operators
Low	SelectDialog.kt:39	Unused contentPadding from BasicAlertDialogSurface
Low	ArcaeaScoreTextField.kt:28	abs() silently inverts negative values

Overall: Well-structured components with good separation of concerns. The utilities screens are clean and the reactive patterns (derivedStateOf, flow operators) are sound. The main bugs are the minValue default and the tolerance discrepancy.

opencode session  |  github run