Skip to content

feat(igpsport): refresh and verify native workouts with secondary targets - #62

Open
KirillSugonyakin wants to merge 8 commits into
jorge-huxley:masterfrom
KirillSugonyakin:codex/upstream-workout-refresh
Open

KirillSugonyakin wants to merge 8 commits into
jorge-huxley:masterfrom
KirillSugonyakin:codex/upstream-workout-refresh

Conversation

@KirillSugonyakin

@KirillSugonyakin KirillSugonyakin commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Keep the existing Intervals.icu event ID → iGPSPORT workout ID mapping and add export fingerprints for automatic in-place updates. Also export native secondary cadence targets, verify accepted workouts by reading them back, and checkpoint accepted IDs before verification.

This revision removes provider/external identity matching, provider/type/date fallback, and cross-event rebinding as requested. A replacement event with a new Intervals ID is a new workout; this PR does not attempt provider-replacement reconciliation or execution-date calendar synchronization.

Changes

  • Keep uploaded_workouts authoritative. Store per-event workout_export_state with the mapped remote ID, export fingerprint, and verification version/status. Metadata cannot confirm a different mapped ID.
  • Same event with changed export → update its mapped workout; verified unchanged export → skip writes. Existing mappings without metadata retain their IDs and gain confirmed metadata after update/read-back.
  • Preserve native secondaryIntensityTarget cadence alongside the explicit power/HR primary target, native units, and supported time/distance/open/repetition semantics. Reject unsupported combinations and malformed repetitions before writes.
  • Read accepted creates/updates through GetWorkOutDetail and compare executable structure, including both targets. CLI and GUI save accepted IDs as pending before read-back; failed verification retains the mapping without a confirmed fingerprint.
  • Keep in-window recreation after manual remote deletion, [skip-igp] suppression even with force, event-description precedence/truncation, complete-list validation, and unstructured-event mapping retention. No automatic remote deletion.
  • CLI and GUI state saves are atomic. New metadata stores no workout content or credentials. Invalid API IDs are not persisted. Activity, Bryton, and legacy profile-sync interfaces remain intact; API requests retain bounded connect/read timeouts.

Verification

  • uv run --frozen pytest -q: 357 passed, Python 3.13.
  • Combined with revised feat(igpsport): sync and verify selected cycling rider settings #63 on upstream 6dfab98: 411 passed, conflict-free merge.
  • Bash syntax, diff checks, and CLI help smoke checks passed.
  • Regression coverage includes event identity, cadence-only edits, unchanged skipping, metadata/remote-ID binding, pending-ID restart, CLI/GUI persistence, forced skip markers, and invalid accepted IDs.
  • Secret scan with network verification disabled: all 24 candidates match existing upstream lines; no new candidate lines. No private configuration, athlete payloads, generated state, or FIT files are included.
  • Independent review completed; GUI interrupted-write/replacement regressions now confirm that the previous readable checkpoint survives save failures.

Live evidence and limitations

Offline tests use synthetic responses; this revision made no live account writes.

Separately, the fork implementation has been exercised with real cycling plans. The account owner reports updating the plans and completing a workout on an iGPSPORT device using its secondary cadence target. A read-only cloud inspection confirmed three linked cadence-bearing plans: their GetWorkOutDetail structures matched expected primary and native secondary cadence targets, without the legacy cadenceTarget field. The day's recorded activity identifies an iGPSPORT device and is paired with the corresponding plan.

The API read-back and owner-reported device use are separate evidence. Mobile-editor rendering has not been independently documented, and this is not a claim of compatibility across all devices or firmware versions.

The separate #63 contribution shares the small HTTP timeout helper so each PR can be merged independently. No release tags, production pin changes, deployment, or live-state migration are included.

@KirillSugonyakin KirillSugonyakin changed the title feat(igpsport): refresh planned workouts with durable identities feat(igpsport): refresh and verify native workouts with secondary targets Oct 15, 2026
@jorge-huxley

Copy link
Copy Markdown
Owner

Thanks for the contribution! Native secondary cadence support looks useful, especially now that iGPSPORT supports secondary targets.

Before merging, I’d like to understand whether the workout identity/reconciliation changes are necessary alongside that addition.

My understanding is that the existing implementation already updates a mapped iGPSPORT workout with forced sync, but normally skips already-uploaded workouts without detecting edits. Could we address automatic updates more simply by retaining the existing Intervals.icu event ID → iGPSPORT workout ID mapping and storing an export fingerprint? Same event, changed fingerprint → update the existing remote workout.

I understand that provider/external identities and the provider/type/date fallback address a different scenario: a provider replacing an event with a new event ID. Could you provide concrete examples of that behavior and explain why it needs to be handled here?

Since iGPSPORT stores these workouts as a list rather than a dated calendar, I see the benefit as avoiding duplicate or obsolete list entries—not synchronizing execution dates. Is that the intended purpose?

My preference would be to keep this contribution focused on automatic fingerprint-based updates, native secondary cadence, and upload verification, leaving provider-replacement reconciliation out for now. Checkpointing accepted IDs also seems useful independently.

Also, the PR notes say testing used synthetic responses without live account writes. Has native secondary cadence been tested separately with a real upload? Before merging, I’d like confirmation that:

  • The live API accepts the payload.
  • The mobile editor displays the correct primary and secondary targets.
  • GetWorkOutDetail returns them correctly and passes verification.
  • Ideally, a compatible device receives and uses both targets correctly.

@KirillSugonyakin

Copy link
Copy Markdown
Contributor Author

Your understanding is correct: automatic updates can use the existing Intervals event ID → iGPSPORT workout ID mapping plus an export fingerprint.

Provider-replacement reconciliation addresses a separate case where RestorTrain recreates a planned session with a new Intervals event ID. Its purpose is to avoid duplicate or obsolete iGPSPORT list entries, not synchronize execution dates. I agree with leaving that reconciliation out and focusing this PR on fingerprint-based updates, native secondary cadence, verification, and checkpointing accepted IDs.

Regarding real-world testing: yes, this has now been used beyond synthetic tests. I updated my cycling plans and completed today’s workout on my iGPSPORT device using the secondary cadence target.

A read-only check of the current cloud state confirmed three linked cadence-bearing plans. For each, GetWorkOutDetail matches the expected executable structure, including primary targets and native secondaryIntensityTarget cadence targets. None contains the legacy cadenceTarget field.

Today’s recorded activity is also present in Intervals.icu, identifies an iGPSPORT device, and is paired with the corresponding plan. That supports the real-device test alongside my firsthand confirmation of using the secondary target.

I’ll clarify the PR description to separate automated tests, live API verification, and device-use confirmation. I haven’t separately documented the mobile-editor display.

@jorge-huxley

Copy link
Copy Markdown
Owner

Thanks for clarifying and for confirming the real-device testing! That sounds good. We’ll wait for the fingerprint-based simplification, removal of the provider-replacement reconciliation, and updated PR description before reviewing again.

@KirillSugonyakin

Copy link
Copy Markdown
Contributor Author

Implemented the event-ID/fingerprint simplification and removed provider-replacement reconciliation. Native secondary cadence, verification, and accepted-ID checkpointing remain. Description updated; all 357 tests pass in CI. Ready for another review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants