Add Deck browsing feature with controller, UI, routing, and format helpers - #48
Add Deck browsing feature with controller, UI, routing, and format helpers#48ercmine wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee14d2a24
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _telemetryRepository().then((repo) async { | ||
| repo.enqueue(state.sessionId, event); | ||
| await repo.flush(state.sessionId); |
There was a problem hiding this comment.
Serialize telemetry flushes for each enqueue
Avoid starting repo.flush in an unawaited .then for every event, because rapid swipes/taps can invoke this method concurrently and race on the same in-memory queue. TelemetryRepository.flush mutates and drains a shared list, so overlapping flushes can send duplicate batches (inflating analytics) or abort mid-drain when one flush removes items another assumed were still present; this should be serialized with a single in-flight flush per session.
Useful? React with 👍 / 👎.
| nextCursor: response.nextCursor, | ||
| hasMore: response.nextCursor != null, | ||
| lastBatchMix: response.mix, | ||
| usedFallback: response.plans.isEmpty || fallbackCount >= (response.plans.length ~/ 2), |
There was a problem hiding this comment.
Use a true 50% threshold for fallback detection
The fallback flag currently uses fallbackCount >= (response.plans.length ~/ 2), which rounds down and misclassifies odd-sized batches: for 1 plan the threshold is 0 (so fallback is always marked active), and for 3 plans it trips at 1 fallback item (33%). This causes incorrect "Fallback suggestions are active" messaging and should use a non-truncating half check.
Useful? React with 👍 / 👎.
| _enqueueTelemetry( | ||
| TelemetryEventInput.cardViewed( | ||
| planId: topPlan.id, | ||
| viewMs: DateTime.now().difference(startedAt).inMilliseconds, |
There was a problem hiding this comment.
Clamp card view duration before telemetry emit
This emits raw elapsed milliseconds with no upper bound, but backend validation only accepts viewMs up to 600000; if a user leaves a card open for more than 10 minutes, the event is rejected and the queued batch for that session keeps failing, dropping subsequent telemetry until the invalid item is removed. Clamp or omit oversized durations before enqueueing.
Useful? React with 👍 / 👎.
Motivation
Description
deck_state.dart), controller (deck_controller.dart) with telemetry, location and repository integration, and a new route at/sessions/:id/deckwired intorouter.dartandsession_page.dart.deck_page.dartand a set of widgets for cards and details (deck_card.dart,deck_card_skeleton.dart,deck_actions_bar.dart,card_details_sheet.dart, and several small badges/pills), and hooked link launching into existinglinkLauncherProviderusage.deckControllerProviderinproviders/app_providers.dart, added format helpers incore/format/formatters.dart, and updatedpubspec.yamlto includeflutter_card_swiper,cached_network_image, andcollectiondependencies.Testing
flutter analyzeto ensure static analysis and imports are correct and it completed successfully.flutter testand unit/widget tests completed successfully (no failing tests).Codex Task