Conversation
reduce API requests sent on user input. This was previously managed via `bloc_concurrency` debounce transformer, which was changed to `restartable` after incorrect state updates were reported in testing
continuation of migration away from portfolio balance in previous PR
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntegrates SparklineRepository into app bootstrap and DI, updating AppBootstrapper.ensureInitialized to accept it. Adds warm-up of SparklineRepository. Refactors header total balance to derive from CoinsState. Adds debounced fiat input updates. Updates sdk submodule pointer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Main as main.dart
participant Bootstrap as AppBootstrapper
participant Repo as SparklineRepository
User->>Main: App start
Main->>Repo: defaultInstance()
Main->>Bootstrap: ensureInitialized(komodoDefiSdk, mm2Api, repo)
activate Bootstrap
Bootstrap->>Repo: init()
Repo-->>Bootstrap: init complete
Bootstrap-->>Main: initialization complete
sequenceDiagram
autonumber
actor User
participant Widget as FiatInputs
participant Deb as Debouncer
participant Callback as onFiatAmountUpdate
User->>Widget: Type fiat amount
Widget->>Widget: _hasUserInput = true
Widget->>Deb: run(300ms, callback)
Note over Deb: Resets on rapid input
Deb-->>Callback: invoke (if mounted)
Callback-->>Widget: amount propagated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements performance optimization for the fiat onramp by adding debouncing to fiat amount input changes to reduce excessive API calls. The changes also include some code cleanup and refactoring of the header actions component.
- Adds debouncing mechanism to fiat input changes with 300ms delay
- Implements user input tracking to prevent race conditions between user input and bloc state updates
- Refactors header actions to simplify balance calculation and remove unused portfolio growth dependency
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk | Updates submodule commit reference |
| lib/views/fiat/fiat_inputs.dart | Adds debouncer and user input tracking to prevent excessive API calls |
| lib/views/common/header/actions/header_actions.dart | Simplifies balance calculation and removes portfolio growth dependency |
| lib/services/initializer/app_bootstrapper.dart | Refactors to accept sparkline repository parameter |
| lib/main.dart | Updates bootstrapper call to pass sparkline repository and removes TODO comment |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Visit the preview URL for this PR (updated for commit 170fdeb): https://walletrc--pull-3125-merge-1jjbp2zk.web.app (expires Wed, 17 Sep 2025 12:53:33 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/initializer/app_bootstrapper.dart (1)
83-86: Fix return type mismatch in _initHive (non-changed but critical).
Hive.init(path)returns void; returning it fromFuture<void>is incorrect. Return after calling it.- return Hive.init(path); + Hive.init(path); + return;
🧹 Nitpick comments (5)
lib/views/fiat/fiat_inputs.dart (1)
112-119: Optionally release the user-input guard after the debounced flush.This lets subsequent state updates reflect in the field without waiting for a rebuild caused by currency/asset changes.
void fiatAmountChanged(String? newValue) { // track if user has made inputs to avoid overwriting them // with stale bloc state updates (e.g. race condition) _hasUserInput = true; _debouncer.run(() { - if (mounted) { - widget.onFiatAmountUpdate(newValue); - } + if (!mounted) return; + widget.onFiatAmountUpdate(newValue); + // Allow prop-driven updates after we've committed the user's input + _hasUserInput = false; }); }lib/views/common/header/actions/header_actions.dart (2)
21-22: Top-level Widget creation: consider lazy building.Creating
SvgPicture.asset(...)Widgets at module load time can be suboptimal. Prefer storing asset paths and constructing Widgets in build-time to avoid premature asset resolution.
36-41: Reduce rebuild scope using BlocSelector or buildWhen.Header re-renders on any CoinsState change. Limit rebuilds to
walletCoinschanges.- child: BlocBuilder<CoinsBloc, CoinsState>( - builder: (context, state) { - final totalBalance = _getTotalBalance( - state.walletCoins.values, - context, - ); + child: BlocSelector<CoinsBloc, CoinsState, Iterable<Coin>>( + selector: (state) => state.walletCoins.values, + builder: (context, coins) { + final totalBalance = _getTotalBalance(coins, context); return ActionTextButton( text: LocaleKeys.balance.tr(), secondaryText: '\$${formatAmt(totalBalance)}', onTap: null, ); }, ),lib/services/initializer/app_bootstrapper.dart (1)
29-33: Warm-up awaitAll: consider error isolation/logging.If any initializer throws, the whole await fails. Optionally wrap each future with its own try/catch to log and proceed, if that's desired UX.
lib/main.dart (1)
87-87: DI registration looks good.SparklineRepository is exposed via RepositoryProvider; this aligns with the added bootstrap parameter.
Optional: since you already have an instance, RepositoryProvider.value(value: sparklineRepository) clarifies ownership (no implicit creation).
- RepositoryProvider(create: (_) => sparklineRepository), + RepositoryProvider.value(value: sparklineRepository),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
lib/main.dart(1 hunks)lib/services/initializer/app_bootstrapper.dart(3 hunks)lib/views/common/header/actions/header_actions.dart(3 hunks)lib/views/fiat/fiat_inputs.dart(3 hunks)sdk(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
Applied to files:
lib/views/common/header/actions/header_actions.dart
📚 Learning: 2025-04-01T15:51:37.060Z
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The `excludedAssetList` from `app_config.dart` is used in the `_filterExcludedAssets` method in `coins_state.dart`. Since `coins_state.dart` is a part file of `coins_bloc.dart`, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Applied to files:
lib/views/common/header/actions/header_actions.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Test web-app-linux-profile
- GitHub Check: Test web-app-macos
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Build Mobile (Android)
- GitHub Check: unit_tests
- GitHub Check: Build Desktop (macos)
- GitHub Check: build_and_preview
- GitHub Check: validate_code_guidelines
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (windows)
🔇 Additional comments (7)
sdk (1)
1-1: Submodule bump (3f503d2…): verify commit contents and CI checkout
- Confirm SDK commit 3f503d2 (Francois, 2025-09-02) includes SparklineRepository.defaultInstance, debounced fiat helpers and no breaking changes.
- CI workflows use
actions/checkout@v4but omitwith: submodules: recursive—add explicit submodule checkout to ensure tests run against the updated SDK.- Pin to a tagged SDK release or include a changelog/release link in this PR for reproducibility.
lib/views/fiat/fiat_inputs.dart (1)
67-75: Init/Dispose lifecycle looks good.
_debounceris created ininitStateand disposed indispose. Nice.lib/views/common/header/actions/header_actions.dart (2)
51-51: LGTM.Const-wrapping the Padding is fine here.
57-60: LGTM.Clean fold; readable and efficient enough for header use.
lib/services/initializer/app_bootstrapper.dart (1)
12-16: No further action needed: The onlyAppBootstrapper.ensureInitializedcall in lib/main.dart (now at line 65) correctly passeskomodoDefiSdk, mm2Api, sparklineRepository; all otherensureInitializedinvocations are either Flutter bindings or distinct static methods and aren’t affected.lib/main.dart (2)
64-65: Verify existence and correct import for SparklineRepository
No definition or export ofSparklineRepositorywas found viarg; please confirm its class location or re-export and add the appropriate import path.
62-66: Verify Hive init before SparklineRepository instantiation
EnsureSparklineRepository.defaultInstance()doesn’t perform Hive I/O before Hive is initialized. If it does, defer its creation until afterAppBootstrapperruns Hive.init (e.g., move instantiation insideensureInitializedor pass a factory() => SparklineRepository.defaultInstance()).
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 8. To receive Bugbot reviews on all of your PRs, please upgrade to Bugbot Pro by visiting the Cursor dashboard. Your first 14 days will be free to try! |
smk762
left a comment
smk762
left a comment
There was a problem hiding this comment.
Though the reduction in calls was not as dramatically reduced here as nted in yje PR's opening comment, there was still a significant reduction of around 30% compared to a window running dev branch build performing the same actions.
LGTM, thanks!
…into perf/fiat-amount-debounce
Changes:
bloc_concurrencydebounce transformer, which was later changed torestartableafter out-of-order state updates were observed in testing.Before (141 requests):
Screen.Recording.2025-09-02.at.16.43.44.mov
After (30 requests):
Screen.Recording.2025-09-02.at.16.49.47.mov
Summary by CodeRabbit
New Features
Improvements
Chores