Skip to content

Refuse a typed BPM that is not a number, instead of dropping it #2224

Closed

Description

@jonyardley

What you would notice

Type "12a" or "9000" in an item's BPM field and save: the item saves with no tempo, and no refusal is shown, because the screen parses the text and drops what it cannot read before the core sees it.

Why it matters

Found by the Swift shell rules review on #2223. The Android app (#2220) would otherwise reimplement this rule, and the two copies would drift. Severity for the port: High.

What to do

The core takes the raw BPM text and parses it in validation.rs (a parse_bpm), refusing bad text with the usual message.

Done looks like

  • Saving "12a" as a BPM shows a refusal and keeps the form open.
  • A table test in the core covers empty, spaces, zero, the top of the range and text.

Where it lives

ios/Intrada/Views/Screens/ItemFormModel.swift:290-297, 352, 372; crates/intrada-core/src/validation.rs.

Activity

  1. added
    architectureArchitecture, technical debt, infrastructure
    iosiOS-specific work (native SwiftUI shell on Crux — see specs/native-ios.md)
    on Oct 2, 2026
  2. jonyardley commented on Oct 2, 2026

    @jonyardley
    OwnerAuthor

    Claimed: branch bpm-parse-2224. Epic: #2220. Decision: false match: #1028, #980 and #120 are Dependabot bumps citing actions/checkout#2224, not this issue

  3. added
    in-flightClaimed: a branch or PR is actively working this
    on Oct 2, 2026
  4. jonyardley commented on Oct 2, 2026

    @jonyardley
    OwnerAuthor

    Plan

    Done looks like: typing "12a" or "70000" in BPM on the Add or Edit form, or in a new exercise row on the Add form, and tapping Save shows "BPM must be a whole number between 1 and 400" under the tempo fields and keeps the form open. A blank BPM still saves with no tempo.

    Time budget: one session for both PRs.

    Decisions taken:

    • A new input-only TempoInput { marking, bpm }, both optional text, on CreateItem.tempo and UpdateItem.tempo. The stored Tempo stays as it is, so saved items, the database codec and Android are untouched. Rejected: changing Tempo itself (reaches storage) and a second text field beside the number (two sources, the Native iOS: editing items silently fails — UpdateItem double_option breaks bincode #846 shape).
    • UpdateItem.tempo becomes Option<TempoInput>: absent leaves the tempo alone, both parts blank clears it. Rejected: keeping three states, since "clear" and "blank text" would mean the same thing.
    • parse_bpm in validation.rs trims, reads blank as no BPM, and refuses anything else that is not a whole number from 1 to 400, on the existing tempo field, which already marks both tempo boxes on iOS.
    • Two PRs. CI regenerates the bindings and builds iOS on every PR, so the core PR carries the two Swift edits that keep it compiling (the form and the new exercise row send the typed text). The screens PR is the check on the simulator and a UI test.

    Files: domain/types.rs, validation.rs, domain/item/create.rs, edit.rs; ItemFormModel.swift; Swift and Kotlin bridge round-trip tests.

    Tests: written first. A table over "", " ", "0", "1", "400", "401", "12a", " 96 ", "96.5", "+96", "-5", "96 bpm", "99999999999999999999"; one refusal each through Add, Update and adding a piece with exercises; the new type in the bincode round-trip on both shells. Deleting the range check breaks "0" and "401"; deleting the parse breaks "12a".

    Rabbit holes: test fixtures that build CreateItem (about 40); a From<Tempo> for tests keeps that mechanical.

    Out of scope: the photo-read fill (#2229), the click tempo band (#2225), what a new exercise row shows as its subtitle for unread text (formatting, stays in Swift).

    Routing: epic #2220. Bridge shape, so a sensitive surface: Opus 5.5 high, reviewer on the core diff before the screens PR. This session, no parallel stream.

  5. jonyardley commented on Oct 2, 2026

    @jonyardley
    OwnerAuthor

    Claimed: branch bpm-screens-2224 (the screens PR; the core PR #2274 on bpm-parse-2224 is merged).

  6. added
    in-flightClaimed: a branch or PR is actively working this
    and removed
    in-flightClaimed: a branch or PR is actively working this
    on Oct 2, 2026
  7. removed
    in-flightClaimed: a branch or PR is actively working this
    on Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    architectureArchitecture, technical debt, infrastructurehorizon:nextNext — 4–12 weeksiosiOS-specific work (native SwiftUI shell on Crux — see specs/native-ios.md)

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions