Feat/log detail 3 mode toggle - #16
Conversation
…es, and deduplicate network requests in React Native SDK
…equest header layout
…hints in inspector toolbars
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThis PR adds persisted retention controls and capped display providers across event streams, improves network interception and URL presentation, adds query-parameter tabs and interceptor badges, updates detail rendering, and introduces localized settings text. ChangesRetention and persisted settings
Network observability and inspector presentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a data retention feature with hard caps and view-only display limits across various event and log lists, along with UI enhancements like a dedicated "Params" tab for network requests and support for tracking the interceptor path (fetch vs. XHR). The review feedback highlights a critical issue where passing null to truncateList disables list truncation entirely, causing memory leaks and breaking the data retention hard cap across multiple providers. Additionally, potential FormatException crashes in the URL parser due to invalid percent-encoding should be resolved by extending the try-catch block, and dynamic recreation of the TabController during the build phase should be avoided to prevent state inconsistency.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ConsoleNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { | ||
| _sub = wsMessageHandler.onLog.listen((entry) { | ||
| if (state.length > 10000) { | ||
| state = [...state.skip(1000), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory log list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 10000) if the limit is null (Unlimited).
| ConsoleNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { | |
| _sub = wsMessageHandler.onLog.listen((entry) { | |
| if (state.length > 10000) { | |
| state = [...state.skip(1000), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| }); | |
| } | |
| ConsoleNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { | |
| _sub = wsMessageHandler.onLog.listen((entry) { | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 10000; | |
| state = truncateList([...state, entry], limit); | |
| }); | |
| } |
| updated[index] = _mergeNetworkEntries(state[index], entry); | ||
| state = updated; | ||
| } else { | ||
| if (state.length > 5000) { | ||
| state = [...state.skip(500), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory network entry list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 5000) if the limit is null (Unlimited).
} else {
final limit = _ref.read(retentionLimitProvider).limit ?? 5000;
state = truncateList([...state, entry], limit);
}| ErrorNotifier(WsMessageHandler handler, this._ref) : super([]) { | ||
| _sub = handler.onError.listen((entry) { | ||
| if (state.length > 5000) { | ||
| state = [...state.skip(500), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory error list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 5000) if the limit is null (Unlimited).
| ErrorNotifier(WsMessageHandler handler, this._ref) : super([]) { | |
| _sub = handler.onError.listen((entry) { | |
| if (state.length > 5000) { | |
| state = [...state.skip(500), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| }); | |
| } | |
| ErrorNotifier(WsMessageHandler handler, this._ref) : super([]) { | |
| _sub = handler.onError.listen((entry) { | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 5000; | |
| state = truncateList([...state, entry], limit); | |
| }); | |
| } |
| StateChangesNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { | ||
| _sub = wsMessageHandler.onState.listen((entry) { | ||
| if (state.length > 5000) { | ||
| state = [...state.skip(500), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory state changes list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 5000) if the limit is null (Unlimited).
| StateChangesNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { | |
| _sub = wsMessageHandler.onState.listen((entry) { | |
| if (state.length > 5000) { | |
| state = [...state.skip(500), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| }); | |
| } | |
| StateChangesNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { | |
| _sub = wsMessageHandler.onState.listen((entry) { | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 5000; | |
| state = truncateList([...state, entry], limit); | |
| }); | |
| } |
| void add(DisplayEntry entry) { | ||
| if (state.length >= 5000) { | ||
| state = [...state.sublist(state.length - 4000), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory display entries list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 5000) if the limit is null (Unlimited).
| void add(DisplayEntry entry) { | |
| if (state.length >= 5000) { | |
| state = [...state.sublist(state.length - 4000), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| } | |
| void add(DisplayEntry entry) { | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 5000; | |
| state = truncateList([...state, entry], limit); | |
| } |
| steps: steps, | ||
| ); | ||
|
|
||
| if (state.length > 5000) { | ||
| state = [...state.skip(500), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory benchmark list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 5000) if the limit is null (Unlimited).
| steps: steps, | |
| ); | |
| if (state.length > 5000) { | |
| state = [...state.skip(500), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| }); | |
| } | |
| steps: steps, | |
| ); | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 5000; | |
| state = truncateList([...state, entry], limit); | |
| }); | |
| } |
| PerformanceNotifier(WsMessageHandler handler, this._ref) : super([]) { | ||
| _sub = handler.onPerformance.listen((entry) { | ||
| if (state.length > 10000) { | ||
| state = [...state.skip(1000), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory performance list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 10000) if the limit is null (Unlimited).
| PerformanceNotifier(WsMessageHandler handler, this._ref) : super([]) { | |
| _sub = handler.onPerformance.listen((entry) { | |
| if (state.length > 10000) { | |
| state = [...state.skip(1000), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| }); | |
| } | |
| PerformanceNotifier(WsMessageHandler handler, this._ref) : super([]) { | |
| _sub = handler.onPerformance.listen((entry) { | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 10000; | |
| state = truncateList([...state, entry], limit); | |
| }); | |
| } |
| MemoryLeakNotifier(WsMessageHandler handler, this._ref) : super([]) { | ||
| _sub = handler.onMemoryLeak.listen((entry) { | ||
| if (state.length > 5000) { | ||
| state = [...state.skip(500), entry]; | ||
| } else { | ||
| state = [...state, entry]; | ||
| } | ||
| state = truncateList([...state, entry], null); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Passing null as the limit to truncateList disables list truncation entirely, causing the in-memory memory leak list to grow infinitely and leak memory. This also breaks the 'Data Retention' feature since the hard cap is never applied to the source list. You should read the user-configured limit from _ref and apply it, falling back to a safe maximum (e.g., 5000) if the limit is null (Unlimited).
| MemoryLeakNotifier(WsMessageHandler handler, this._ref) : super([]) { | |
| _sub = handler.onMemoryLeak.listen((entry) { | |
| if (state.length > 5000) { | |
| state = [...state.skip(500), entry]; | |
| } else { | |
| state = [...state, entry]; | |
| } | |
| state = truncateList([...state, entry], null); | |
| }); | |
| } | |
| MemoryLeakNotifier(WsMessageHandler handler, this._ref) : super([]) { | |
| _sub = handler.onMemoryLeak.listen((entry) { | |
| final limit = _ref.read(retentionLimitProvider).limit ?? 5000; | |
| state = truncateList([...state, entry], limit); | |
| }); | |
| } |
| FormattedUrl? parseFormattedUrl(String? url) { | ||
| if (url == null) return null; | ||
| final trimmed = url.trim(); | ||
| if (trimmed.isEmpty || trimmed == '<unknown url>') return null; | ||
|
|
||
| Uri uri; | ||
| try { | ||
| uri = Uri.parse(trimmed); | ||
| } catch (_) { | ||
| return FormattedUrl(host: null, path: trimmed, queryParams: const [], raw: trimmed); | ||
| } | ||
|
|
||
| // `uri.queryParametersAll` keeps insertion order AND preserves repeated | ||
| // keys as lists — the former matters for `?order=` style params, the | ||
| // latter for Supabase-style `?id=in.(1,2,3)` filters. | ||
| final params = <FormattedQueryParam>[]; | ||
| uri.queryParametersAll.forEach((k, values) { | ||
| for (final v in values) { | ||
| params.add(FormattedQueryParam(k, _decode(v))); | ||
| } | ||
| }); | ||
|
|
||
| return FormattedUrl( | ||
| host: uri.host.isEmpty ? null : uri.host, | ||
| path: uri.path.isEmpty ? '/' : uri.path, | ||
| queryParams: params, | ||
| raw: trimmed, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Accessing uri.queryParametersAll can throw a FormatException if the query parameters contain invalid percent-encoding (e.g., ?q=%g1). Since this access is currently outside the try-catch block, it can crash the parser and any widget that calls it. Wrapping the entire parsing and query parameter extraction in the try-catch block will make it completely safe.
FormattedUrl? parseFormattedUrl(String? url) {
if (url == null) return null;
final trimmed = url.trim();
if (trimmed.isEmpty || trimmed == '<unknown url>') return null;
try {
final uri = Uri.parse(trimmed);
// `uri.queryParametersAll` keeps insertion order AND preserves repeated
// keys as lists — the former matters for `?order=` style params, the
// latter for Supabase-style `?id=in.(1,2,3)` filters.
final params = <FormattedQueryParam>[];
uri.queryParametersAll.forEach((k, values) {
for (final v in values) {
params.add(FormattedQueryParam(k, _decode(v)));
}
});
return FormattedUrl(
host: uri.host.isEmpty ? null : uri.host,
path: uri.path.isEmpty ? '/' : uri.path,
queryParams: params,
raw: trimmed,
);
} catch (_) {
return FormattedUrl(host: null, path: trimmed, queryParams: const [], raw: trimmed);
}
}| void _resizeControllerIfNeeded(int newLength) { | ||
| if (_tabController.length == newLength) return; | ||
| final oldIndex = _tabController.index.clamp(0, newLength - 1); | ||
| _tabController.removeListener(_onTabIndexChange); | ||
| _tabController.dispose(); | ||
| _tabController = _makeController(oldIndex, newLength); | ||
| _tabController.addListener(_onTabIndexChange); | ||
| } |
There was a problem hiding this comment.
Disposing and recreating the TabController dynamically during the build phase is a bad practice in Flutter and can lead to state inconsistency, scroll controller errors, or 'setState() called during build' crashes. Since ParamsTab already handles the empty state gracefully with an EmptyState widget, it is safer to keep the tab count static or manage the tab controller recreation in didUpdateWidget instead of during build.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/features/all_events/presentation/pages/all_events_page.dart (1)
409-458: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore
StableListViewfor the event list.This replaces the required stable list implementation with
ListView.custom, discarding the stable delegate behavior during live list updates. UseStableListViewhere instead. As per path instructions, “Use StableListView for list views.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/all_events/presentation/pages/all_events_page.dart` around lines 409 - 458, Replace the ListView.custom in the event list with StableListView, preserving the existing controller, item extent, item builder logic, keys, and visible item count through StableListView’s corresponding API. Ensure the stable delegate behavior remains active during live updates.Source: Path instructions
lib/l10n/app_zh_TW.arb (1)
170-173: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove duplicate JSON keys.
Same issue as
app_zh_CN.arb:noNetworkRequestsandapiCallsAppearHereeach appear twice (lines 170–173) with identical values. Remove the duplicate entries at lines 172–173.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/l10n/app_zh_TW.arb` around lines 170 - 173, Remove the duplicated `noNetworkRequests` and `apiCallsAppearHere` entries from the localization resource, keeping only one occurrence of each key and preserving their existing values.lib/l10n/app_zh_CN.arb (1)
170-173: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove duplicate JSON keys.
noNetworkRequestsandapiCallsAppearHereeach appear twice (lines 170–173) with identical values. Duplicate keys can cause issues with ARB code generation and are invalid JSON practice. Remove the duplicate entries at lines 172–173.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/l10n/app_zh_CN.arb` around lines 170 - 173, Remove the duplicate noNetworkRequests and apiCallsAppearHere entries from the ARB resource, retaining only one definition of each key and its existing value.
🧹 Nitpick comments (8)
lib/features/network_inspector/presentation/shared/params_tab.dart (1)
107-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValue cell uses
SelectableTextrather thanTextComponent.Same "Detail panels must use TextComponent for selectable text" path instruction as flagged in
request_detail_panel.dart— this params tab is rendered inside that same detail panel and the value cell is deliberately selectable (see comment above it).As per path instructions, "Detail panels must use TextComponent for selectable text."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/network_inspector/presentation/shared/params_tab.dart` around lines 107 - 129, The selectable value in the params tab currently uses Flutter’s SelectableText instead of the required TextComponent. In the value-cell widget near widget.keyName and widget.value, replace SelectableText with TextComponent while preserving the existing value, styling, and selectable-text behavior.Source: Path instructions
lib/features/network_inspector/presentation/request/request_detail_panel.dart (4)
774-783: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame decode-with-fallback logic duplicated across files.
_decodedhere duplicates theUri.decodeFull/catch-fallback pattern also inlined inevent_row.dart's_tooltipFor(and presumablynetwork_detail.dart). Sincelib/core/utils/network_url_formatter.dartalready hosts URL-formatting helpers (e.g.formatUrlOneLine), a shareddecodeUrlSafe(String)there would consolidate this and keep the fallback behavior consistent as more call sites are added (the "All Events detail rendering" layer needs the same thing).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/network_inspector/presentation/request/request_detail_panel.dart` around lines 774 - 783, Add a shared decodeUrlSafe(String) helper to network_url_formatter.dart that returns Uri.decodeFull(url) with the original URL as fallback on decoding errors. Replace the local _decoded implementation and duplicated inline decoding in event_row.dart's _tooltipFor and network_detail.dart with this helper, updating imports and preserving existing behavior.
219-270: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate "via" badge rendering vs.
_ViaBadgeinevent_row.dart.This inline
Builderreimplements the same fetch/xhr → color/label mapping that_ViaBadgeinlib/features/all_events/presentation/event_row/event_row.dartalready encapsulates. Extracting a sharedViaBadgewidget (parameterized by size/padding if needed) would avoid the two implementations drifting (e.g. this one derives the label viaentry.via.toUpperCase()while the other hardcodes'FETCH'/'XHR'per case).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/network_inspector/presentation/request/request_detail_panel.dart` around lines 219 - 270, Replace the inline via badge Builder in the request detail panel with a shared ViaBadge widget extracted from _ViaBadge in event_row.dart. Centralize the NetworkVia fetch/xhr color and label mapping there, supporting any required size or padding parameters, and update both call sites to use it so labels and styling cannot diverge.
93-114: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDisposing/recreating
_tabControllerdirectly insidebuild()is fragile — considerdidUpdateWidget.Mutating
_tabController(dispose + reassign) as a side effect ofbuild()works today because the same pass immediately hands the new controller toDetailTabBar/TabBarView, but it's a latent trap: if this widget is ever driven by an ancestor rebuild unrelated toentry(e.g. a theme change) whileTabController-consuming descendants haven't yet re-subscribed, this pattern risks "used after being disposed" errors down the line. The existing_rebuildController()(triggered fromref.listen, withsetState) already gives you the right template — worth using an analogous check indidUpdateWidget(oldWidget)comparingoldWidget.entry.urlvswidget.entry.urlinstead of doing it inline inbuild().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/network_inspector/presentation/request/request_detail_panel.dart` around lines 93 - 114, Move the tab-controller length synchronization out of build and into didUpdateWidget, comparing oldWidget.entry.url with widget.entry.url before rebuilding. Reuse the existing _rebuildController pattern to dispose and recreate _tabController, including clamped index preservation and setState as appropriate; keep build focused on deriving tab labels and rendering descendants without mutating controller state.
201-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
TextComponentfor the selectable URL
TextComponentalready supports selectable text (selectabledefaults totrue), so this detail panel should use it here instead of rawSelectableTextto stay consistent with the panel pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/network_inspector/presentation/request/request_detail_panel.dart` around lines 201 - 217, The selectable URL currently uses raw SelectableText instead of the panel’s standard TextComponent. Replace SelectableText in the URL row with TextComponent, preserving the decoded URL, styling, padding, and max-line behavior while relying on TextComponent’s default selectable support.Source: Path instructions
client_sdks/devconnect-react-native/src/client.ts (1)
547-571: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
init()now resolves before the host is known / connection is attempted — update the docstring.Previously
await DevConnect.init()implied a connection attempt (with the real host) had already started by the time the promise resolved. Now the promise resolves immediately after synchronous patching, while host resolution +connect()happen in a detached.then()/.catch()chain. Functionally fine for the "don't miss early requests" goal, but the public JSDoc example (await DevConnect.init({ appName: 'MyApp' })) no longer implies "connected", which could surprise consumers who gate other logic (e.g.DevConnect.isConnected()) on the awaited call.📝 Suggested docstring clarification
/** * Initialize DevConnect. * + * Note: this resolves as soon as interceptors are patched — host + * detection and the WebSocket connection happen asynchronously in the + * background afterwards, so `DevConnect.isConnected()` may still be + * `false` immediately after `await`. + * * ```typescript🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client_sdks/devconnect-react-native/src/client.ts` around lines 547 - 571, Update the public JSDoc for DevConnect.init to state that awaiting it only completes synchronous initialization and interceptor patching; host detection and connect() occur asynchronously afterward. Clarify that callers must not assume DevConnect.isConnected() is true immediately after await, and reference the init() documentation/example near the changed initialization flow.lib/features/settings/presentation/shared/preset_dropdown.dart (1)
44-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
ColorTokensor theme colors over hardcoded hex values.
0xFF0D9488(active teal),0xFF1E1E2E(dark popup background), and0xFF6B7280(icon grey) are hardcoded, whilesettings_page.dartusesColorTokensfor the same surfaces. If the app's theme palette changes, these values won't adapt. Consider sourcing these fromColorTokensorTheme.of(context).♻️ Proposed refactor using ColorTokens/theme
- color: isDark ? const Color(0xFF1E1E2E) : Colors.white, + color: isDark ? ColorTokens.darkSurface : Colors.white,- if (isActive) - const Icon(LucideIcons.check, - size: 14, color: Color(0xFF0D9488)) + if (isActive) + Icon(LucideIcons.check, + size: 14, color: Theme.of(context).colorScheme.primary)- const Icon(LucideIcons.layers, size: 15, color: Color(0xFF6B7280)), + Icon(LucideIcons.layers, size: 15, + color: isDark ? Colors.grey[400] : Colors.grey[600]),Also applies to: 54-54, 64-64, 76-76, 88-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/settings/presentation/shared/preset_dropdown.dart` at line 44, Replace the hardcoded teal, dark popup background, and grey icon colors in the preset dropdown’s build logic with the corresponding ColorTokens or Theme.of(context) colors, matching the palette usage in settings_page.dart; update all affected color assignments in the dropdown.lib/features/settings/presentation/sections/all_events_display_section.dart (1)
16-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared section widget to eliminate duplication with
DataRetentionSection.
AllEventsDisplaySectionandDataRetentionSectionare structurally identical — same Column layout, same Row withSizedBox(width: 100)label, samePresetDropdownwiring, same helper text padding. Only the icon, provider, and localization keys differ. A single parameterized widget would cut ~50 lines of duplicated code and prevent drift when the layout evolves.♻️ Proposed shared widget
// retention_section.dart class RetentionSection extends ConsumerWidget { final IconData icon; final String Function(AppLocalizations) title; final String Function(AppLocalizations) description; final String Function(AppLocalizations) helper; final AutoDisposeStateNotifierProvider<RetentionLimitNotifier, RetentionPreset> provider; const RetentionSection({ super.key, required this.icon, required this.title, required this.description, required this.helper, required this.provider, }); `@override` Widget build(BuildContext context, WidgetRef ref) { final preset = ref.watch(provider); final l10n = S.of(context); return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ SectionTitle(icon: icon, title: title(l10n)), Text(description(l10n), style: TextStyle(fontSize: 11, color: Colors.grey[500], height: 1.4)), const SizedBox(height: 14), Row(children: [ SizedBox(width: 100, child: Text(l10n.maxItems, style: TextStyle(fontSize: 13, color: Colors.grey[500]))), Expanded(child: PresetDropdown( selected: preset, onSelected: (p) => ref.read(provider.notifier).set(p), )), ]), const SizedBox(height: 6), Padding(padding: const EdgeInsets.only(left: 100), child: Text(helper(l10n), style: TextStyle(fontSize: 10, color: Colors.grey[600], height: 1.4))), ], ); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/settings/presentation/sections/all_events_display_section.dart` around lines 16 - 66, Extract the duplicated layout from AllEventsDisplaySection and DataRetentionSection into a shared parameterized ConsumerWidget, such as RetentionSection. Pass the icon, localized title/description/helper selectors, and the appropriate retention provider; centralize provider watching, PresetDropdown updates, labels, spacing, and styling in the shared widget, then replace both section implementations with configured instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/components/misc/retention_hint.dart`:
- Line 66: Add a localized `showingCountOfTotal` ARB key with count and total
placeholders in every supported locale, then update the retention hint widget to
use the generated localization accessor instead of the hardcoded `Showing $count
of $total` string.
In `@lib/core/providers/retention_provider.dart`:
- Around line 23-30: The RetentionPreset.label getter hardcodes the
non-localized “Unlimited” display text. Remove presentation labels from the enum
metadata or add a context-aware label resolver used by PresetDropdown, and
resolve RetentionPreset.unlimited through S.of(context) while preserving enum
values/names for persistence.
In `@lib/core/providers/tab_visibility_provider.dart`:
- Around line 32-34: Update _load() to preserve an intentionally persisted empty
list from disable as an empty tab set, while still falling back to
TabKey.values.toSet() when a non-empty decoded list contains no recognized tab
names. Track whether the decoded list was empty before filtering, and only apply
the “all enabled” fallback for non-empty invalid data.
In `@lib/core/utils/network_url_formatter.dart`:
- Around line 58-65: In the query parameter construction within the formatter,
remove the redundant _decode call because uri.queryParametersAll already returns
decoded values; pass each value directly to FormattedQueryParam while preserving
repeated keys and insertion order.
- Around line 68-72: Preserve explicit ports when constructing FormattedUrl in
the URL formatter: replace the host-only value with uri.authority, or add
uri.port as a separate field and update downstream detail panels and copy
formatters to use it. Ensure URLs such as localhost:8080 retain their port.
In `@lib/features/all_events/presentation/detail/network_detail.dart`:
- Around line 58-64: Update _rebuildController() to pass the current
tabLabels.length into _makeController(...) instead of relying on the default
length, and clamp the restored tab index to the valid range before creating the
TabController so index 4 cannot be used with fewer tabs.
- Around line 67-73: In _resizeControllerIfNeeded, convert the result of
_tabController.index.clamp(0, newLength - 1) to an int before passing it to
_makeController, using oldIndex.toInt() or an equivalent integer conversion.
In `@lib/features/all_events/presentation/event_row/event_row.dart`:
- Around line 144-151: Only render the 4px spacer when the network event’s via
value is fetch or xhr; update the conditional block around _ViaBadge and
SizedBox so NetworkVia.unknown does not add an empty gap, while preserving the
badge and spacing for valid via values.
In `@lib/features/all_events/presentation/header/header_bar.dart`:
- Around line 128-139: The new “Showing $count of $untrimmed” hint in the header
widget is not localized. Add a generated localization entry with placeholders
for count and untrimmed, then replace the hardcoded string in the header’s Text
widget with the corresponding S.of(context) lookup.
In `@lib/features/all_events/presentation/shared/params_tab.dart`:
- Around line 34-49: Replace the ListView.builder in the detail-panel build
method with the repository’s StableListView equivalent, preserving its padding,
itemCount, and existing itemBuilder logic that renders _ParamCell values. Ensure
the list follows the required stable-list behavior for presentation-layer lists.
- Around line 118-128: Replace the raw SelectableText in the detail-panel value
rendering with the shared TextComponent, preserving the existing text, styling,
and selectable behavior. Add the corresponding text_component.dart import and
configure TextComponent consistently with other inspector detail panels.
- Around line 39-47: The copy action in the parameter row currently uses the
joined display value, flattening repeated keys and losing URI encoding. Preserve
the raw value list from the query parameter entry, and in the copy handler
serialize the key and all values with Uri(queryParameters: ...) before writing
to the clipboard, while keeping the joined string only for display.
In `@lib/features/benchmark/provider/benchmark_providers.dart`:
- Line 114: Bound retained event data at ingestion in the notifier using a fixed
5,000-entry ceiling rather than the unbounded truncateList call. Update the
corresponding state assignments in the event handlers for
benchmark_providers.dart and the analogous providers in console_providers.dart,
state_providers.dart, network_providers.dart, and storage_providers.dart, while
preserving any separate configurable display limit.
In `@lib/features/display/provider/display_providers.dart`:
- Around line 40-42: Update DisplayNotifier.add to read the current value from
retentionLimitProvider and pass it to truncateList instead of null, ensuring
appended DisplayEntry history is capped and the shouldDrop rule runs for stored
entries.
In `@lib/features/error_inspector/presentation/header/toolbar.dart`:
- Around line 88-147: Localize the hardcoded “Showing N of M” text and remove
duplicated trimmed-display logic. Add a shared localization key and use it in
both the toolbar’s trimmed section and RetentionHint, then extract the
`isTrimmed` calculation and “Showing” text rendering into a reusable widget or
helper while preserving the toolbar-specific CountUp, error color, and
PulsingDot behavior.
In `@lib/features/network_inspector/presentation/request/request_card.dart`:
- Around line 258-260: Guard the via badge-building block with both viaLabel and
viaColor non-null checks; update the condition around the via badge in the
request card so viaColor is promoted before calling withValues, including both
badge color and border styling.
- Around line 54-67: Update the URL formatting logic around `path`, `host`, and
`displayUrl`: use `uri.authority` without inventing a default scheme, preserve
ports and other authority details, and fall back to `/` when the parsed path is
empty so host-only URLs retain a meaningful title. Ensure the existing root-path
and service-action handling in `titleText` uses this normalized path.
In `@lib/features/state_inspector/presentation/pages/state_inspector_page.dart`:
- Around line 309-321: Localize the retention counter and trimmed-total hint
currently rendered in the State Inspector page. Add localized formatter methods
for the capped count and “Showing …” text, including the preset’s “Unlimited”
label, then replace the direct interpolated strings in the relevant Text widgets
with these formatters.
---
Outside diff comments:
In `@lib/features/all_events/presentation/pages/all_events_page.dart`:
- Around line 409-458: Replace the ListView.custom in the event list with
StableListView, preserving the existing controller, item extent, item builder
logic, keys, and visible item count through StableListView’s corresponding API.
Ensure the stable delegate behavior remains active during live updates.
In `@lib/l10n/app_zh_CN.arb`:
- Around line 170-173: Remove the duplicate noNetworkRequests and
apiCallsAppearHere entries from the ARB resource, retaining only one definition
of each key and its existing value.
In `@lib/l10n/app_zh_TW.arb`:
- Around line 170-173: Remove the duplicated `noNetworkRequests` and
`apiCallsAppearHere` entries from the localization resource, keeping only one
occurrence of each key and preserving their existing values.
---
Nitpick comments:
In `@client_sdks/devconnect-react-native/src/client.ts`:
- Around line 547-571: Update the public JSDoc for DevConnect.init to state that
awaiting it only completes synchronous initialization and interceptor patching;
host detection and connect() occur asynchronously afterward. Clarify that
callers must not assume DevConnect.isConnected() is true immediately after
await, and reference the init() documentation/example near the changed
initialization flow.
In
`@lib/features/network_inspector/presentation/request/request_detail_panel.dart`:
- Around line 774-783: Add a shared decodeUrlSafe(String) helper to
network_url_formatter.dart that returns Uri.decodeFull(url) with the original
URL as fallback on decoding errors. Replace the local _decoded implementation
and duplicated inline decoding in event_row.dart's _tooltipFor and
network_detail.dart with this helper, updating imports and preserving existing
behavior.
- Around line 219-270: Replace the inline via badge Builder in the request
detail panel with a shared ViaBadge widget extracted from _ViaBadge in
event_row.dart. Centralize the NetworkVia fetch/xhr color and label mapping
there, supporting any required size or padding parameters, and update both call
sites to use it so labels and styling cannot diverge.
- Around line 93-114: Move the tab-controller length synchronization out of
build and into didUpdateWidget, comparing oldWidget.entry.url with
widget.entry.url before rebuilding. Reuse the existing _rebuildController
pattern to dispose and recreate _tabController, including clamped index
preservation and setState as appropriate; keep build focused on deriving tab
labels and rendering descendants without mutating controller state.
- Around line 201-217: The selectable URL currently uses raw SelectableText
instead of the panel’s standard TextComponent. Replace SelectableText in the URL
row with TextComponent, preserving the decoded URL, styling, padding, and
max-line behavior while relying on TextComponent’s default selectable support.
In `@lib/features/network_inspector/presentation/shared/params_tab.dart`:
- Around line 107-129: The selectable value in the params tab currently uses
Flutter’s SelectableText instead of the required TextComponent. In the
value-cell widget near widget.keyName and widget.value, replace SelectableText
with TextComponent while preserving the existing value, styling, and
selectable-text behavior.
In `@lib/features/settings/presentation/sections/all_events_display_section.dart`:
- Around line 16-66: Extract the duplicated layout from AllEventsDisplaySection
and DataRetentionSection into a shared parameterized ConsumerWidget, such as
RetentionSection. Pass the icon, localized title/description/helper selectors,
and the appropriate retention provider; centralize provider watching,
PresetDropdown updates, labels, spacing, and styling in the shared widget, then
replace both section implementations with configured instances.
In `@lib/features/settings/presentation/shared/preset_dropdown.dart`:
- Line 44: Replace the hardcoded teal, dark popup background, and grey icon
colors in the preset dropdown’s build logic with the corresponding ColorTokens
or Theme.of(context) colors, matching the palette usage in settings_page.dart;
update all affected color assignments in the dropdown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a885797c-cec2-49ee-87d1-ac8c027eea2d
⛔ Files ignored due to path filters (3)
macos/Podfileis excluded by none and included by nonemacos/Runner.xcodeproj/project.pbxprojis excluded by none and included by nonetest_manual/test_url.dartis excluded by none and included by none
📒 Files selected for processing (55)
client_sdks/devconnect-react-native/src/client.tslib/components/misc/retention_hint.dartlib/core/providers/retention_provider.dartlib/core/providers/tab_visibility_provider.dartlib/core/theme/theme_provider.dartlib/core/utils/list_retention.dartlib/core/utils/network_url_formatter.dartlib/core/utils/retention_capped.dartlib/features/all_events/presentation/detail/error_detail.dartlib/features/all_events/presentation/detail/event_detail_panel.dartlib/features/all_events/presentation/detail/log_detail.dartlib/features/all_events/presentation/detail/network_detail.dartlib/features/all_events/presentation/event_row/event_row.dartlib/features/all_events/presentation/header/header_bar.dartlib/features/all_events/presentation/pages/all_events_page.dartlib/features/all_events/presentation/shared/params_tab.dartlib/features/all_events/provider/all_events_provider.dartlib/features/benchmark/provider/benchmark_providers.dartlib/features/console/presentation/header/toolbar.dartlib/features/console/presentation/shared/count_pill.dartlib/features/console/provider/console_providers.dartlib/features/display/provider/display_providers.dartlib/features/error_inspector/presentation/header/toolbar.dartlib/features/error_inspector/provider/error_providers.dartlib/features/network_inspector/presentation/pages/network_inspector_page.dartlib/features/network_inspector/presentation/request/request_card.dartlib/features/network_inspector/presentation/request/request_detail_panel.dartlib/features/network_inspector/presentation/shared/params_tab.dartlib/features/network_inspector/presentation/toolbar/toolbar.dartlib/features/network_inspector/provider/network_providers.dartlib/features/performance/provider/performance_providers.dartlib/features/settings/presentation/pages/settings_page.dartlib/features/settings/presentation/sections/all_events_display_section.dartlib/features/settings/presentation/sections/data_retention_section.dartlib/features/settings/presentation/shared/preset_dropdown.dartlib/features/state_inspector/presentation/pages/state_inspector_page.dartlib/features/state_inspector/provider/state_providers.dartlib/features/storage_viewer/presentation/header/toolbar.dartlib/features/storage_viewer/provider/storage_providers.dartlib/l10n/app_en.arblib/l10n/app_fr.arblib/l10n/app_ja.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_ja.dartlib/l10n/app_localizations_vi.dartlib/l10n/app_localizations_zh.dartlib/l10n/app_vi.arblib/l10n/app_zh.arblib/l10n/app_zh_CN.arblib/l10n/app_zh_TW.arblib/l10n/untranslated.txtlib/models/network/network_entry.dartlib/server/ws_message_handler.dart
💤 Files with no reviewable changes (1)
- lib/features/network_inspector/presentation/pages/network_inspector_page.dart
| if (isTrimmed) ...[ | ||
| const SizedBox(height: 2), | ||
| Text( | ||
| 'Showing $count of $total', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the "Showing N of M" string.
The Showing $count of $total text is user-facing but hardcoded in English. This PR adds localized strings for dataRetention, allEventsDisplay, and other retention-related labels across all supported locales, but this hint text is missed. Non-English users will see an English fragment in an otherwise localized UI.
🌐 Proposed fix to localize the hint
Add a new ARB key (e.g. showingCountOfTotal) with a placeholder:
+ "showingCountOfTotal": "Showing {count} of {total}",
+ "`@showingCountOfTotal`": {
+ "placeholders": {
+ "count": { "type": "int" },
+ "total": { "type": "int" }
+ }
+ },Then use it in the widget:
- Text(
- 'Showing $count of $total',
+ Text(
+ S.of(context).showingCountOfTotal(count, total),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'Showing $count of $total', | |
| Text( | |
| S.of(context).showingCountOfTotal(count, total), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/components/misc/retention_hint.dart` at line 66, Add a localized
`showingCountOfTotal` ARB key with count and total placeholders in every
supported locale, then update the retention hint widget to use the generated
localization accessor instead of the hardcoded `Showing $count of $total`
string.
| String get label => switch (this) { | ||
| RetentionPreset.unlimited => 'Unlimited', | ||
| RetentionPreset.p100 => '100', | ||
| RetentionPreset.p500 => '500', | ||
| RetentionPreset.p1k => '1K', | ||
| RetentionPreset.p5k => '5K', | ||
| RetentionPreset.p10k => '10K', | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the Unlimited preset label.
PresetDropdown consumes this metadata, so non-English settings screens render “Unlimited” in English. Resolve display labels through S.of(context) in presentation while retaining enum names for persistence.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/core/providers/retention_provider.dart` around lines 23 - 30, The
RetentionPreset.label getter hardcodes the non-localized “Unlimited” display
text. Remove presentation labels from the enum metadata or add a context-aware
label resolver used by PresetDropdown, and resolve RetentionPreset.unlimited
through S.of(context) while preserving enum values/names for persistence.
| // Defensive: if nothing matched (corrupt data), fall back to | ||
| // "all enabled" rather than "all hidden" — better UX. | ||
| return result.isEmpty ? TabKey.values.toSet() : result; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve an intentionally empty tab selection.
disable can persist [], but _load() converts that valid state into “all enabled” on restart. Distinguish an empty decoded list from a non-empty list containing no recognized tab names.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/core/providers/tab_visibility_provider.dart` around lines 32 - 34, Update
_load() to preserve an intentionally persisted empty list from disable as an
empty tab set, while still falling back to TabKey.values.toSet() when a
non-empty decoded list contains no recognized tab names. Track whether the
decoded list was empty before filtering, and only apply the “all enabled”
fallback for non-empty invalid data.
| // `uri.queryParametersAll` keeps insertion order AND preserves repeated | ||
| // keys as lists — the former matters for `?order=` style params, the | ||
| // latter for Supabase-style `?id=in.(1,2,3)` filters. | ||
| final params = <FormattedQueryParam>[]; | ||
| uri.queryParametersAll.forEach((k, values) { | ||
| for (final v in values) { | ||
| params.add(FormattedQueryParam(k, _decode(v))); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat >/tmp/check_uri.dart <<'DART'
void main() {
final uri = Uri.parse('https://example.test/?q=a%252Fb');
print(uri.queryParametersAll['q']);
}
DART
dart /tmp/check_uri.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file outline ==\n'
ast-grep outline lib/core/utils/network_url_formatter.dart --view expanded || true
printf '\n== relevant source slice ==\n'
nl -ba lib/core/utils/network_url_formatter.dart | sed -n '1,220p'
printf '\n== search for helper usages ==\n'
rg -n "_decode|queryParametersAll|FormattedQueryParam|formatUrlOneLine" lib/core/utils/network_url_formatter.dart lib/core -S || trueRepository: ridelinktechs/devconnect-manage-kit
Length of output: 318
🌐 Web query:
Dart Uri.queryParametersAll decoded values queryParametersAll documentation
💡 Result:
In Dart, the Uri.queryParametersAll property is a getter that returns a Map<String, List> containing the URI's query parameters [1][2]. Key characteristics of this property include: Decoded Values: Each key and value within the resulting map is automatically decoded [1][2]. This process follows the rules specified for HTML form submissions (FORM post) as described in the HTML 4.01 specification, section 17.13.4, which includes handling percent-encoding and converting plus signs (+) into spaces [1][2][3]. Handling Multiple Values: Unlike the queryParameters property—which returns a Map<String, String> and maps each key to a single, arbitrary value if a key appears multiple times—queryParametersAll maps every key to a list of all its corresponding values [1][4]. If a key appears only once, its value is represented as a singleton list [1][2]. Edge Cases: If a key occurs in the query string without an assigned value, it is mapped to an empty string ("") within the list [1][2]. If the URI contains no query, the returned map is empty [1][2]. Unmodifiable: The returned map and the lists contained within it are unmodifiable [1][2]. Example Usage: final uri = Uri.parse('https://example.com/api/fetch?limit=10&limit=20&max=100'); print(uri.queryParametersAll); // {"limit":["10","20"],"max":["100"]}
Citations:
- 1: https://api.dart.dev/dart-core/Uri/queryParametersAll.html
- 2: https://api.dart.dev/stable/3.3.1/dart-core/Uri/queryParametersAll.html
- 3: https://api.dart.dev/dart-core/Uri-class.html
- 4: https://api.dart.dev/stable/2.10.3/dart-core/Uri/queryParameters.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate file ==\n'
git ls-files | rg '^lib/core/utils/network_url_formatter\.dart$|network_url_formatter\.dart$' || true
printf '\n== print file with line numbers if present ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('lib/core/utils/network_url_formatter.dart')
print('exists:', p.exists())
if p.exists():
for i, line in enumerate(p.read_text().splitlines(), 1):
if 1 <= i <= 220:
print(f"{i:4d}: {line}")
PY
printf '\n== search decode helper in repo ==\n'
rg -n "String _decode|_decode\\(" lib/core -S || trueRepository: ridelinktechs/devconnect-manage-kit
Length of output: 7669
Avoid decoding queryParametersAll values twice in lib/core/utils/network_url_formatter.dart:62-64. Uri.queryParametersAll already returns decoded components, so _decode(v) can turn %252F into /. Keep v as-is here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/core/utils/network_url_formatter.dart` around lines 58 - 65, In the query
parameter construction within the formatter, remove the redundant _decode call
because uri.queryParametersAll already returns decoded values; pass each value
directly to FormattedQueryParam while preserving repeated keys and insertion
order.
| return FormattedUrl( | ||
| host: uri.host.isEmpty ? null : uri.host, | ||
| path: uri.path.isEmpty ? '/' : uri.path, | ||
| queryParams: params, | ||
| raw: trimmed, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat >/tmp/check_authority.dart <<'DART'
void main() {
final uri = Uri.parse('http://localhost:8080/api');
print('host=${uri.host}, port=${uri.port}, authority=${uri.authority}');
}
DART
dart /tmp/check_authority.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== network_url_formatter ==\n'
sed -n '1,220p' lib/core/utils/network_url_formatter.dart
printf '\n== find FormattedUrl usages ==\n'
rg -n "FormattedUrl|formattedUrl|authority|uri\.host|uri\.port" lib -S
printf '\n== locate detail panels mentioning url ==\n'
rg -n "detail panel|copy.*url|copy.*URL|FormattedUrl|raw:" lib -SRepository: ridelinktechs/devconnect-manage-kit
Length of output: 17178
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "repo root:"
pwd
echo
echo "files:"
git ls-files | rg '^lib/core/utils/network_url_formatter\.dart$|^lib/.*formatted_url|^lib/.*detail.*panel|^lib/.*url' -n
echo
echo "network_url_formatter:"
sed -n '1,220p' lib/core/utils/network_url_formatter.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 6878
🌐 Web query:
Dart Uri.host Uri.authority port difference documentation
💡 Result:
In Dart, the Uri class distinguishes between authority, host, and port as follows: The authority component represents the hierarchical part of the URI that includes the user information, host, and port [1][2]. It is a single string that aggregates these parts [1]. The host is a component of the authority that specifies the hostname, IPv4 address, or IPv6 address [3][2]. The port is an integer component of the authority that specifies the port number [4]. A key distinction is that the port property always returns an integer [4]. If no port is explicitly provided in the URI string, the port property returns the default port for the scheme (80 for http, 443 for https, or 0 for all other schemes) [4][2]. You can use the hasPort property to determine if the port was explicitly specified in the original URI [5]. In summary, while authority is the concatenated string representation of the authority section, host and port are the specific, parsed components of that section [1][2].
Citations:
- 1: https://api.dart.dev/dart-core/Uri/authority.html
- 2: https://api.dart.dev/dart-core/Uri/Uri.html
- 3: https://api.dart.dev/dart-core/Uri/host.html
- 4: https://api.dart.dev/dart-core/Uri/port.html
- 5: https://api.dart.dev/dart-core/Uri-class.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== request_card ==\n'
sed -n '1,140p' lib/features/network_inspector/presentation/request/request_card.dart
printf '\n== request_detail_panel url/copy area ==\n'
sed -n '740,840p' lib/features/network_inspector/presentation/request/request_detail_panel.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 8589
Preserve the full authority here. uri.host drops explicit ports, so URLs like http://localhost:8080/api get rendered without :8080 in the downstream detail panels and copy formatters. Store uri.authority or keep port alongside host.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/core/utils/network_url_formatter.dart` around lines 68 - 72, Preserve
explicit ports when constructing FormattedUrl in the URL formatter: replace the
host-only value with uri.authority, or add uri.port as a separate field and
update downstream detail panels and copy formatters to use it. Ensure URLs such
as localhost:8080 retain their port.
| final isTrimmed = | ||
| retentionLimit != null && capped.total > count; | ||
| return Column( | ||
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| if (count > 0) | ||
| Padding( | ||
| padding: const EdgeInsets.only(right: 4), | ||
| child: PulsingDot( | ||
| color: ColorTokens.logError, | ||
| size: 7, | ||
| Row( | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| if (count > 0) | ||
| Padding( | ||
| padding: const EdgeInsets.only(right: 4), | ||
| child: PulsingDot( | ||
| color: ColorTokens.logError, | ||
| size: 7, | ||
| ), | ||
| ), | ||
| AnimatedContainer( | ||
| duration: const Duration(milliseconds: 200), | ||
| curve: Curves.easeOutCubic, | ||
| padding: const EdgeInsets.symmetric( | ||
| horizontal: 8, vertical: 2), | ||
| decoration: BoxDecoration( | ||
| color: count > 0 | ||
| ? ColorTokens.logError.withValues(alpha: 0.12) | ||
| : (isDark | ||
| ? Colors.white.withValues(alpha: 0.06) | ||
| : Colors.black.withValues(alpha: 0.04)), | ||
| borderRadius: BorderRadius.circular(10), | ||
| ), | ||
| child: CountUp( | ||
| value: count, | ||
| formatter: (n) => retentionLimit == null | ||
| ? '$n' | ||
| : '$n / $retentionLabel', | ||
| style: TextStyle( | ||
| fontSize: 11, | ||
| fontWeight: FontWeight.w700, | ||
| fontFamily: AppConstants.monoFontFamily, | ||
| color: count > 0 | ||
| ? ColorTokens.logError | ||
| : (isDark | ||
| ? Colors.grey[400] | ||
| : Colors.grey[600]), | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| AnimatedContainer( | ||
| duration: const Duration(milliseconds: 200), | ||
| curve: Curves.easeOutCubic, | ||
| padding: const EdgeInsets.symmetric( | ||
| horizontal: 8, vertical: 2), | ||
| decoration: BoxDecoration( | ||
| color: count > 0 | ||
| ? ColorTokens.logError.withValues(alpha: 0.12) | ||
| : (isDark | ||
| ? Colors.white.withValues(alpha: 0.06) | ||
| : Colors.black.withValues(alpha: 0.04)), | ||
| borderRadius: BorderRadius.circular(10), | ||
| ), | ||
| child: CountUp( | ||
| value: count, | ||
| ], | ||
| ), | ||
| if (isTrimmed) ...[ | ||
| const SizedBox(height: 2), | ||
| Text( | ||
| 'Showing $count of ${capped.total}', | ||
| style: TextStyle( | ||
| fontSize: 11, | ||
| fontWeight: FontWeight.w700, | ||
| fontSize: 9, | ||
| fontFamily: AppConstants.monoFontFamily, | ||
| color: count > 0 | ||
| ? ColorTokens.logError | ||
| : (isDark | ||
| ? Colors.grey[400] | ||
| : Colors.grey[600]), | ||
| color: isDark ? Colors.grey[600] : Colors.grey[500], | ||
| ), | ||
| ), | ||
| ), | ||
| ], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Localize the "Showing N of M" string and consider extracting shared logic.
Line 140 hardcodes 'Showing $count of ${capped.total}' in English. Since this PR adds localization keys for other new retention strings, this user-facing text should also be localized. The same hardcoded pattern exists in RetentionHint, so both would need the new key.
Additionally, the isTrimmed check and "Showing N of M" rendering duplicate RetentionHint's logic. While the error toolbar justifiably needs CountUp animation, ColorTokens.logError, and PulsingDot, extracting the trimmed-text portion into a small shared widget would prevent future drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/error_inspector/presentation/header/toolbar.dart` around lines
88 - 147, Localize the hardcoded “Showing N of M” text and remove duplicated
trimmed-display logic. Add a shared localization key and use it in both the
toolbar’s trimmed section and RetentionHint, then extract the `isTrimmed`
calculation and “Showing” text rendering into a reusable widget or helper while
preserving the toolbar-specific CountUp, error color, and PulsingDot behavior.
| final path = uri?.path ?? entry.url; | ||
| // Prefix scheme so the user can read the request at a glance — | ||
| // matches what curl-style tooling shows. | ||
| final host = uri == null | ||
| ? '' | ||
| : '${uri.scheme.isEmpty ? 'https' : uri.scheme}://${uri.host}'; | ||
| // Card title is the path (compact) — the full scheme+host lives | ||
| // in the badge row below, so we don't repeat it on the title. | ||
| final displayUrl = path; | ||
| final formatted = parseFormattedUrl(entry.url); | ||
| final isRootPath = path == '/' || path.isEmpty; | ||
| final titleText = (entry.serviceAction != null && isRootPath) | ||
| ? entry.serviceAction! | ||
| : displayUrl; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context with line numbers.
file='lib/features/network_inspector/presentation/request/request_card.dart'
wc -l "$file"
sed -n '1,220p' "$file" | cat -nRepository: ridelinktechs/devconnect-manage-kit
Length of output: 10604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='lib/features/network_inspector/presentation/request/request_card.dart'
# Show the rest of the widget around host/path usage and any null-sensitive code.
sed -n '220,373p' "$file" | cat -n
# Probe Dart Uri behavior on representative inputs without modifying repo files.
python3 - <<'PY'
from urllib.parse import urlparse
cases = [
"example.com",
"/foo",
"https://example.com",
"https://example.com:8080/path",
"http://example.com",
"example.com/path",
]
for s in cases:
u = urlparse(s)
print(s, "=>", {"scheme": u.scheme, "netloc": u.netloc, "path": u.path, "authority": u.netloc})
PYRepository: ridelinktechs/devconnect-manage-kit
Length of output: 9502
🌐 Web query:
Dart Uri.parse relative URL host authority path behavior docs
💡 Result:
In Dart, the Uri.parse method is designed to parse URI strings according to RFC 3986 [1][2]. A key behavior of the Uri class is that it is normalizing, meaning it does not preserve semantically equivalent but non-canonical string representations [3]. When you use Uri.parse on a relative URL: 1. Normalization: The Uri class automatically normalizes the path [3]. For instance, leading "./" segments are removed because they are considered equivalent to the path without them [3]. This means that if you parse "./foo", the resulting Uri object will represent it as "foo" [3]. 2. Authority and Host: If a URI string lacks a scheme and authority, Uri.parse interprets it as a relative URI reference [1][2]. If the string does not start with a slash, it is treated as a path-only relative URI [4][2]. 3. Resolving Relative URIs: To resolve a relative URI against a base URI, use the resolve or resolveUri methods [5][2]. The resolution process follows the "Transform Reference" algorithm described in RFC 3986 [4][1]. Specifically, the resolveUri method is updated to handle cases where the base URI is a relative path (i.e., no scheme, no authority, and the path does not start with a slash), combining paths without stripping leading ".." segments [4][2]. 4. Distinction: Because the Uri class is designed for semantic URI handling rather than unstructured string storage, it is not possible to preserve distinguishing features like leading "./" in the Uri object itself [3]. If your application requires preserving these distinctions for system-level lookups, you should manage them as strings before parsing them into Uri objects [3].
Citations:
- 1: https://github.com/dart-lang/sdk/blob/4c8a4f0d7ad055fa7dea5e80862cd2074f4454d3/sdk/lib/core/uri.dart
- 2: https://github.com/dart-lang/sdk/blob/d62c54ebed698399afc8c28efc8cdaacbc056327/sdk/lib/core/uri.dart
- 3: Uri.parse() loses information for leading ./ dart-lang/sdk#32490
- 4: https://api.dart.dev/dart-core/Uri/resolveUri.html
- 5: https://api.dart.dev/dart-core/Uri-class.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='lib/features/network_inspector/presentation/request/request_card.dart'
sed -n '220,330p' "$file" | cat -nRepository: ridelinktechs/devconnect-manage-kit
Length of output: 6972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='lib/features/network_inspector/presentation/request/request_card.dart'
# Find the host rendering and any parseFormattedUrl helpers used in this widget.
rg -n "formatted|host|parseFormattedUrl|serviceAction|queryHint" "$file"Repository: ridelinktechs/devconnect-manage-kit
Length of output: 1004
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='lib/features/network_inspector/presentation/request/request_card.dart'
sed -n '220,330p' "$file" | cat -n
rg -n "host|authority|path|displayUrl|titleText|serviceAction" "$file"Repository: ridelinktechs/devconnect-manage-kit
Length of output: 7764
Don’t invent a scheme or drop the authority here. Uri.parse('example.com') and //example.com:8080/path both end up with a bad https:// host and the port gets lost. Use uri.authority for the host row, and fall back to / when uri.path is empty so host-only URLs still have a title.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/network_inspector/presentation/request/request_card.dart` around
lines 54 - 67, Update the URL formatting logic around `path`, `host`, and
`displayUrl`: use `uri.authority` without inventing a default scheme, preserve
ports and other authority details, and fall back to `/` when the parsed path is
empty so host-only URLs retain a meaningful title. Ensure the existing root-path
and service-action handling in `titleText` uses this normalized path.
| color: viaColor!.withValues(alpha: 0.10), | ||
| border: Border.all( | ||
| color: viaColor.withValues(alpha: 0.22), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
dart analyze lib/features/network_inspector/presentation/request/request_card.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first
ast-grep outline lib/features/network_inspector/presentation/request/request_card.dart --view expanded || true
# Show the relevant area with line numbers
wc -l lib/features/network_inspector/presentation/request/request_card.dart
sed -n '220,290p' lib/features/network_inspector/presentation/request/request_card.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 4416
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the definitions and nearby logic for viaLabel and viaColor
rg -n -C 4 "viaLabel|viaColor" lib/features/network_inspector/presentation/request/request_card.dartRepository: ridelinktechs/devconnect-manage-kit
Length of output: 2468
Guard viaColor before building the via badge. viaLabel != null doesn’t promote viaColor, so viaColor.withValues(...) at line 260 still violates null-safety; gate the block with viaColor != null too.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/network_inspector/presentation/request/request_card.dart` around
lines 258 - 260, Guard the via badge-building block with both viaLabel and
viaColor non-null checks; update the condition around the via badge in the
request card so viaColor is promoted before calling withValues, including both
badge color and border styling.
| Text( | ||
| retentionLimit == null | ||
| ? '$c changes' | ||
| : '$c / $retentionLabel changes', | ||
| style: theme.textTheme.bodySmall, | ||
| ), | ||
| if (isTrimmed) | ||
| Text( | ||
| 'Showing $c of ${capped.total}', | ||
| style: TextStyle( | ||
| fontSize: 9, | ||
| fontFamily: AppConstants.monoFontFamily, | ||
| color: isDark ? Colors.grey[600] : Colors.grey[500], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the new retention counter text.
The newly rendered count and “Showing …” text are English-only, including the Unlimited label supplied by the preset. This leaves the State Inspector partially untranslated in every non-English locale. Add localized formatters for the capped count and trimmed-total hint.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/state_inspector/presentation/pages/state_inspector_page.dart`
around lines 309 - 321, Localize the retention counter and trimmed-total hint
currently rendered in the State Inspector page. Add localized formatter methods
for the capped count and “Showing …” text, including the preset’s “Unlimited”
label, then replace the direct interpolated strings in the relevant Text widgets
with these formatters.
…re providers to accurately reflect dropped items in retention capping
…eys from event detail and state inspector components
…valid URI parsing and tab controller updates
1 participant
Description
Related Issue
Type of Change
Testing
Screenshots (if applicable)
Summary by CodeRabbit