AI scribe data layer and requirements#4196
Conversation
AI_ENABLED allows to test AI features only in some wanted environment. AI_API_KEY and AI_API_URL allows to call the LLM while we wait for tmail backend endpoints. It will be removed when tmail backend endpoints are ready.
This part will be highly reworked soon because : - prompts will come from a remote JSON file - menu will be build depending of prompts that will contains translations I think it should also be moved in another module later.
This mixin leverages HTML callbacks that can be set by WebView editor to send text selection event to the composer so the composer will receive text selection event necessary to display buttons related to text selection.
|
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 WalkthroughA new Scribe AI text generation feature module is introduced with complete data, domain, and presentation layers, including multi-language localization support (English, French, Russian, Vietnamese), AI API integration, and text selection event handling. Main app is updated to integrate the new module and localization delegates. Changes
Sequence DiagramsequenceDiagram
participant WebView as WebView (HTML)
participant SelectionListener as Selection Listener
participant TextSelectionMixin as Text Selection Mixin
participant Presenter as AI Presenter
participant Interactor as GenerateAITextInteractor
participant AIPrompts as AIPrompts Utility
participant AIRepository as AIRepository
participant AIApi as AIApi
participant RemoteAPI as Remote AI API
WebView->>SelectionListener: selectionchange event
SelectionListener->>SelectionListener: Compute bounding rect<br/>Extract selectedText
SelectionListener->>TextSelectionMixin: postMessage with selection data
TextSelectionMixin->>TextSelectionMixin: handleSelectionChange()<br/>Parse TextSelectionData
TextSelectionMixin->>Presenter: onSelectionChanged callback<br/>(TextSelectionData)
Presenter->>Interactor: execute(AIAction, selectedText)
Interactor->>AIPrompts: buildPrompt(action, selectedText)
AIPrompts-->>Interactor: Formatted prompt string
Interactor->>AIRepository: generateText(prompt)
AIRepository->>AIApi: chatCompletion(AIAPIRequest)
AIApi->>RemoteAPI: POST /chat/completions<br/>(model, messages, auth headers)
RemoteAPI-->>AIApi: AIApiResponse (choices[0].message.content)
AIApi-->>AIRepository: AIResponse
AIRepository-->>Interactor: AIResponse
alt Success
Interactor->>Presenter: Right(GenerateAITextSuccess)
Presenter->>Presenter: Update UI with response
else Failure
Interactor->>Presenter: Left(GenerateAITextFailure)
Presenter->>Presenter: Show error state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Added a separated ScribeLocalizations to keep it separated from main app.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (17)
scribe/.gitignore (1)
25-28: Add explicitpubspec.lockpattern to match the documented guideline.The comment on line 25 correctly references that libraries should not include
pubspec.lock, yet there's no explicit ignore pattern for it. For consistency and to enforce the guideline, add a pattern to ignore it.Apply this diff to make the .gitignore explicit:
# Flutter/Dart/Pub related # Libraries should not include pubspec.lock, per https://dart.dev/guides/libraries/private-files#pubspeclock. +pubspec.lock **/doc/api/ .dart_tool/ build/scribe/README.md (1)
1-3: Consider expanding README with usage and build notesOnce the module stabilizes, it would be useful to document main entry points (barrel exports, localization delegate) and how to run prebuild/localization generation for scribe.
scribe/lib/scribe.dart (1)
1-10: Revisit which scribe components should be part of the public barrel APIRight now the barrel exports both domain/presentation types and data-layer implementations (
ai_repository_impl,ai_datasource). That’s convenient but also exposes internals as part of the public API surface, which can make future refactors harder.Consider limiting the barrel exports to domain contracts/state and presentation models, keeping data-layer implementations internal and imported only where needed.
scripts/prebuild.sh (1)
29-33: Scribe localization generation block aligns with existing workflowThe dedicated block to extract/generate ARB files for the scribe module mirrors the main app’s intl workflow and keeps scribe’s localizations self-contained. Just keep paths in sync if
scribe_localizations.dartor its l10n directory move in future.scribe/lib/scribe/ai/data/model/ai_api_request.dart (1)
3-10: Consider making the AI model configurable.The model is currently hardcoded to
'gpt-oss-120b'. Consider adding amodelparameter to the constructor with a default value to enable flexibility for testing, different model versions, or multi-model support.Example refactor:
class AIAPIRequest { static const String _defaultModel = 'gpt-oss-120b'; + final String model; final List<AIMessage> messages; const AIAPIRequest({ + this.model = _defaultModel, required this.messages, }); Map<String, dynamic> toJson() { return { - 'model': _defaultModel, + 'model': model, 'messages': messages.map((m) => m.toJson()).toList(), }; } }scribe/lib/scribe/ai/data/repository/ai_repository_impl.dart (1)
10-13: LGTM! Clean delegation pattern.The repository correctly delegates to the data source. The
awaiton line 12 is redundant since you're directly returning the Future—you can simplify toreturn _dataSource.request(prompt);for more idiomatic async code, though this is optional.Optional simplification:
@override Future<AIResponse> generateText(String prompt) async { - return await _dataSource.request(prompt); + return _dataSource.request(prompt); }scribe/lib/scribe/ai/data/network/ai_api.dart (2)
30-37: Consider using custom exception types for better error handling.The method throws generic
Exceptioninstances. Consider defining custom exception types (e.g.,AIApiEmptyResponseException,AIApiStatusCodeException) to enable more precise error handling and recovery strategies in calling code.Example:
// In a separate exceptions file class AIApiException implements Exception { final String message; AIApiException(this.message); } class AIApiEmptyResponseException extends AIApiException { AIApiEmptyResponseException() : super('AI API returned empty response'); } class AIApiStatusCodeException extends AIApiException { final int statusCode; AIApiStatusCodeException(this.statusCode) : super('AI API returned status code: $statusCode'); }
18-28: Consider adding timeout and retry configuration.The API request lacks timeout configuration and retry logic. Consider:
- Adding a timeout to prevent indefinite hanging on stalled connections
- Implementing retry logic with exponential backoff for transient failures
- These can be configured on the Dio instance or per-request basis
Example timeout configuration:
final response = await _dio.post( '$_baseUrl/chat/completions', options: Options( sendTimeout: const Duration(seconds: 30), receiveTimeout: const Duration(seconds: 60), headers: { 'Authorization': 'Bearer $_apiKey', 'Content-Type': 'application/json', }, ), data: request.toJson(), );scribe/lib/scribe/ai/data/config/ai_config.dart (1)
6-6: Consider case-insensitive comparison for boolean environment variable.The current implementation only recognizes lowercase
'true'. Users settingAI_ENABLED=TRUEorAI_ENABLED=Truewill see the feature disabled.Apply this diff for more robust boolean parsing:
- static bool get isAiEnabled => dotenv.get('AI_ENABLED', fallback: 'false') == 'true'; + static bool get isAiEnabled => dotenv.get('AI_ENABLED', fallback: 'false').toLowerCase() == 'true';scribe/lib/scribe/ai/domain/model/ai_response.dart (1)
1-17: Clarify nullability intent for theresultfield.The
resultfield is nullable (String?), marked asrequired, yetfromJsonalways provides a fallback to''(line 8), meaning it never actually returnsnull. This creates an inconsistency between the type signature and runtime behavior.Choose one of the following approaches:
Option 1 (Recommended): Make
resultnon-nullable if empty string is a valid fallback:class AIResponse { - final String? result; + final String result; const AIResponse({required this.result}); factory AIResponse.fromJson(Map<String, dynamic> json) { return AIResponse( result: json['result'] as String? ?? json['text'] as String? ?? '', ); }Option 2: Allow
nulland remove the fallback if absence of data should be explicit:factory AIResponse.fromJson(Map<String, dynamic> json) { return AIResponse( - result: json['result'] as String? ?? json['text'] as String? ?? '', + result: json['result'] as String? ?? json['text'] as String?, ); }scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart (1)
34-38: Consider enriching error messages with more context.The current error handling provides minimal context. Including the prompt, HTTP status, or error type would aid debugging.
Example enhancement:
} on DioException catch (e) { - throw Exception('Failed to generate AI text: ${e.message}'); + throw Exception('Failed to generate AI text: ${e.type} - ${e.message}${e.response?.statusCode != null ? " (HTTP ${e.response!.statusCode})" : ""}'); } catch (e) { - throw Exception('Failed to generate AI text: $e'); + throw Exception('Failed to generate AI text. Prompt length: ${prompt.length}. Error: $e');scribe/lib/scribe/ai/domain/usecases/generate_ai_text_interactor.dart (1)
17-26: LGTM with optional suggestion for error categorization.The broad
catch (e)is acceptable for a use case boundary, converting all exceptions toGenerateAITextFailure. However, distinguishing error types (network, validation, API) could improve error handling downstream.If richer error context would benefit the UI, consider:
try { final prompt = AIPrompts.buildPrompt(action, selectedText); final response = await _repository.generateText(prompt); return Right(GenerateAITextSuccess(response)); + } on NetworkException catch (e) { + return Left(GenerateAITextFailure(e, type: FailureType.network)); + } on ValidationException catch (e) { + return Left(GenerateAITextFailure(e, type: FailureType.validation)); } catch (e) { - return Left(GenerateAITextFailure(e)); + return Left(GenerateAITextFailure(e, type: FailureType.unknown)); }scribe/lib/scribe/ai/domain/state/generate_ai_text_state.dart (1)
6-13: Consider addingconstconstructor for performance.The
GenerateAITextSuccessconstructor could beconstsince all fields are final, enabling compile-time constant optimization and reducing unnecessary rebuilds in Flutter widgets.Apply this diff:
class GenerateAITextSuccess extends UIState { final AIResponse response; - GenerateAITextSuccess(this.response); + const GenerateAITextSuccess(this.response); @override List<Object?> get props => [response]; }scribe/lib/scribe/ai/presentation/model/ai_action.dart (1)
20-29: Consider defensive localization access.The null assertion on line 27 assumes
ScribeLocalizationsis always available. While this should be true given the delegate registration inmain.dart, consider using a fallback for robustness:return ScribeLocalizations.of(context)?.customPromptAction ?? 'Help me write';core/lib/utils/html/html_utils.dart (1)
51-112: Selection-change listener is solid; consider naming consistency and debouncing behavior.The listener correctly normalizes the selection, guards against redundant events, and emits
{ hasSelection, selectedText, coordinates }to bothwindow.parentandwindow.flutter_inappwebview, with reasonable fallbacks tohasSelection: false.A few small refinements to consider:
- The record’s
name(Line 112) is'onSelectionChange'while the constant isregisterSelectionChangeListener, whereas other records (e.g.,registerDropListener) keepnamealigned with the constant name. Ifnameis used as an identifier for script (un)registration, aligning it would avoid confusion:- name: 'onSelectionChange'); + name: 'registerSelectionChangeListener');
The
lastSelectedTextcheck (Lines 53–80) debounces by text only. If a user drags a selection so that the text stays the same but its bounding rect moves, consumers relying on coordinates may not get updated positions. If that interaction matters, consider including rect coordinates in the dedupe key or relaxing the optimization.In the JS
catchblock (Lines 101–105), you currently swallow errors. Adding aconsole.error(e)(or similar) before sendinghasSelection: falsewould make interop issues easier to debug without impacting behavior.lib/features/composer/presentation/widgets/mixins/text_selection_mixin.dart (2)
3-3: Use an explicit function type instead of bareFunction.Using
Functionloses static checking on the callback’s parameter and return type. Prefer a fully-typed function signature here:-typedef OnTextSelectionChanged = Function(TextSelectionData?); +typedef OnTextSelectionChanged = void Function(TextSelectionData? selection);This keeps callsites type-safe and clearer in IDEs.
20-44: Defensive parsing is good; consider logging and supporting partial selections.
TextSelectionData.fromMapis nicely defensive (defaultshasSelectiontofalse, centralizes empty state, and wraps parsing in atry/catch). Two small improvements to consider:
The
catchblock (Lines 41–43) silently returnsTextSelectionData.empty(). If the JS payload shape ever drifts, this will be hard to diagnose. Logging the exception and/or rawdata(at least in debug builds) would make schema issues visible without breaking consumers.When
hasSelectionistruebut eitherselectedTextorcoordinatesis missing (Lines 31–40), you currently collapse toTextSelectionData.empty(). That makes “selection without coordinates” indistinguishable from “no selection”. If there are scenarios where text is still useful without coordinates (e.g., non-visual handling), consider returning a non-empty instance (e.g.,selectedTextwithcoordinates: null) instead of fully discarding the selection.These are behavioral tweaks, not correctness fixes, but they can make the bridge more robust and debuggable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
assets/images/ic_sparkle.svgis excluded by!**/*.svgpubspec.lockis excluded by!**/*.lockscribe/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
core/lib/presentation/resources/image_paths.dart(1 hunks)core/lib/utils/html/html_utils.dart(1 hunks)lib/features/composer/presentation/widgets/mixins/text_selection_mixin.dart(1 hunks)lib/l10n/intl_messages.arb(1 hunks)lib/main.dart(2 hunks)pubspec.yaml(1 hunks)scribe/.gitignore(1 hunks)scribe/.metadata(1 hunks)scribe/README.md(1 hunks)scribe/lib/scribe.dart(1 hunks)scribe/lib/scribe/ai/data/config/ai_config.dart(1 hunks)scribe/lib/scribe/ai/data/datasource/ai_datasource.dart(1 hunks)scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart(1 hunks)scribe/lib/scribe/ai/data/model/ai_api_request.dart(1 hunks)scribe/lib/scribe/ai/data/model/ai_api_response.dart(1 hunks)scribe/lib/scribe/ai/data/model/ai_message.dart(1 hunks)scribe/lib/scribe/ai/data/network/ai_api.dart(1 hunks)scribe/lib/scribe/ai/data/repository/ai_repository_impl.dart(1 hunks)scribe/lib/scribe/ai/domain/constants/ai_prompts.dart(1 hunks)scribe/lib/scribe/ai/domain/model/ai_response.dart(1 hunks)scribe/lib/scribe/ai/domain/repository/ai_scribe_repository.dart(1 hunks)scribe/lib/scribe/ai/domain/state/generate_ai_text_state.dart(1 hunks)scribe/lib/scribe/ai/domain/usecases/generate_ai_text_interactor.dart(1 hunks)scribe/lib/scribe/ai/l10n/intl_en.arb(1 hunks)scribe/lib/scribe/ai/l10n/intl_fr.arb(1 hunks)scribe/lib/scribe/ai/l10n/intl_messages.arb(1 hunks)scribe/lib/scribe/ai/l10n/intl_ru.arb(1 hunks)scribe/lib/scribe/ai/l10n/intl_vi.arb(1 hunks)scribe/lib/scribe/ai/localizations/scribe_localizations.dart(1 hunks)scribe/lib/scribe/ai/presentation/model/ai_action.dart(1 hunks)scribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dart(1 hunks)scribe/pubspec.yaml(1 hunks)scripts/prebuild.sh(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-09T09:36:45.349Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4194
File: lib/features/manage_account/presentation/manage_account_dashboard_controller.dart:174-176
Timestamp: 2025-12-09T09:36:45.349Z
Learning: In Dart/Flutter projects using GetX, do not wrap getBinding<T>() calls in try/catch since they return null when not found. Only wrap Get.find<T>() calls in try/catch because they throw if a dependency is unavailable. When a binding is optional, consider checking Get.isRegistered<T>() or handling the null/exception path gracefully instead of blindly catching, and document the expectation for failure modes where a dependency may not be registered.
Applied to files:
scribe/lib/scribe/ai/data/datasource/ai_datasource.dartcore/lib/utils/html/html_utils.dartcore/lib/presentation/resources/image_paths.dartscribe/lib/scribe/ai/data/model/ai_message.dartscribe/lib/scribe/ai/data/config/ai_config.dartscribe/lib/scribe/ai/domain/constants/ai_prompts.dartscribe/lib/scribe/ai/domain/repository/ai_scribe_repository.dartscribe/lib/scribe/ai/data/repository/ai_repository_impl.dartscribe/lib/scribe/ai/localizations/scribe_localizations.dartscribe/lib/scribe/ai/data/model/ai_api_response.dartscribe/lib/scribe/ai/domain/usecases/generate_ai_text_interactor.dartscribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dartscribe/lib/scribe/ai/data/model/ai_api_request.dartscribe/lib/scribe.dartlib/features/composer/presentation/widgets/mixins/text_selection_mixin.dartscribe/lib/scribe/ai/domain/state/generate_ai_text_state.dartlib/main.dartscribe/lib/scribe/ai/data/network/ai_api.dartscribe/lib/scribe/ai/presentation/model/ai_action.dartscribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dartscribe/lib/scribe/ai/domain/model/ai_response.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). (9)
- GitHub Check: analyze-test (fcm)
- GitHub Check: analyze-test (forward)
- GitHub Check: analyze-test (server_settings)
- GitHub Check: analyze-test (core)
- GitHub Check: analyze-test (default)
- GitHub Check: analyze-test (model)
- GitHub Check: analyze-test (rule_filter)
- GitHub Check: analyze-test (contact)
- GitHub Check: analyze-test (email_recovery)
🔇 Additional comments (32)
core/lib/presentation/resources/image_paths.dart (1)
264-264: NewicSparklegetter looks consistent; verify underlying asset wiringThe getter follows the existing convention and is fine. Please just confirm that
ic_sparkle.svgis present under the images directory used byAssetsPaths.imagesand is declared inpubspec.yaml, otherwise runtime asset resolution will fail.pubspec.yaml (1)
57-59: Localscribepackage dependency wiring looks consistentAdding
scribeas a path dependency alongside other local modules is coherent and should integrate cleanly with the workspace setup.lib/l10n/intl_messages.arb (1)
2-2: ARB metadata-only change is safeOnly
@@last_modifiedwas updated; no translation keys or placeholders changed, so there is no functional impact on localization.scribe/.metadata (1)
1-10: Including.metadatafor the scribe package is appropriateTracking Flutter revision/channel and project type for the nested
scribepackage in VCS is standard and helps tooling behave predictably.scribe/pubspec.yaml (1)
1-83: Scribe package manifest looks coherent with the main appThe SDK constraint, local
coredependency, and minimal runtime/dev setup are consistent with the parent project; nothing stands out as problematic in this manifest.lib/main.dart (2)
12-12: Scribe localizations import path is correctImporting
ScribeLocalizationsfrom the scribe package matches the new package layout and keeps AI-scribe i18n concerns encapsulated in that module.
77-83: Registration ofScribeLocalizations.delegatein MaterialApp is appropriateAdding the scribe delegate to
localizationsDelegatesintegrates the new AI-scribe strings into the app’s localization system without affecting existing resolution logic.scripts/prebuild.sh (1)
8-8: Includingscribein the modules build loop is consistentAdding
scribeto themodulesarray ensures its codegen (build_runner) runs alongside other feature packages, which is important now that it’s a separate package.scribe/lib/scribe/ai/domain/repository/ai_scribe_repository.dart (1)
1-5: LGTM! Clean repository interface.The repository interface follows clean architecture principles with a well-defined contract. The single method signature is clear and appropriately abstract.
scribe/lib/scribe/ai/data/datasource/ai_datasource.dart (1)
1-5: LGTM! Clean datasource interface.The datasource interface provides a clear abstraction at the data layer with a focused contract.
scribe/lib/scribe/ai/l10n/intl_messages.arb (1)
1-129: LGTM! Well-structured localization resource.The ARB file follows proper format with complete metadata blocks and covers a comprehensive set of UI strings for the AI Scribe feature.
scribe/lib/scribe/ai/l10n/intl_fr.arb (1)
1-129: LGTM! French localization complete.The French ARB file is properly structured with complete translations matching the English resource.
scribe/lib/scribe/ai/l10n/intl_ru.arb (1)
1-129: LGTM! Russian localization complete.The Russian ARB file is properly structured with complete translations matching the other language resources.
scribe/lib/scribe/ai/data/network/ai_api.dart (1)
18-38: Good addition of empty response validation.The empty response check on line 31 addresses the previous review feedback appropriately. The method correctly constructs the API request with proper headers and validates the response.
scribe/lib/scribe/ai/data/model/ai_message.dart (1)
1-18: LGTM! Standard json_serializable pattern.The implementation correctly uses
json_annotationwith proper factory and serialization methods. The generated part file (ai_message.g.dart) will be created during the build process.scribe/lib/scribe/ai/data/config/ai_config.dart (1)
6-10: File not found in repository.The file
scribe/lib/scribe/ai/data/config/ai_config.dartdoes not exist in the tmail-flutter repository. The review comment cannot be verified or applied to this codebase. Confirm the correct file path or repository context.scribe/lib/scribe/ai/presentation/model/ai_action.dart (1)
5-18: LGTM: Clean sealed class hierarchy.The sealed class design enables exhaustive pattern matching and the delegation to
action.getFullLabel(context)provides good separation of concerns.scribe/lib/scribe/ai/localizations/scribe_localizations.dart (4)
7-20: LGTM: Standard localization setup.The locale handling and initialization follow Flutter best practices. The nullable return from
of(context)is correct, and locale name resolution properly handles both simple and compound locales.
22-22: LGTM: Correct delegate implementation.The delegate properly restricts support to the four localized languages and correctly returns
falsefromshouldReloadsince it's a const instance.Also applies to: 136-147
24-101: LGTM: Consistent Intl.message implementation.All localization getters follow the correct pattern with properly named message keys. The default English strings are clear and descriptive.
104-133: LGTM: Input and dialog messages properly defined.The separate message keys for
inputPlaceholderandcustomPromptActionallow for localization flexibility even though they share the same English text. This is good practice for maintaining translation independence.scribe/lib/scribe/ai/data/model/ai_api_response.dart (2)
5-18: LGTM: Safe array access in content getter.The
contentgetter properly guards against empty lists withisNotEmptybefore accessingchoices[0]. The nullable return type correctly handles the case when no choices are available.
20-44: LGTM: Standard JSON-serializable models.The
ChoiceandMessageclasses follow the correct pattern for json_serializable with immutable const constructors and proper annotations.scribe/lib/scribe/ai/presentation/model/ai_scribe_menu_action.dart (3)
4-46: LGTM: Exhaustive action labeling.The switch statement properly covers all 16 action cases with appropriate localization mappings. Consider the same defensive localization access pattern suggested in
ai_action.dartif you want to avoid potential null assertion failures.
48-76: LGTM: Smart hierarchical label construction.The
categorygetter correctly groups actions, andgetFullLabelelegantly handles both submenu and direct actions with the "Category > Action" format.
79-129: LGTM: Clean category-to-actions mapping.The category enum properly groups related actions and the
hasSubmenucheck is straightforward. The mapping between categories and their actions is correct.scribe/lib/scribe/ai/domain/constants/ai_prompts.dart (4)
4-41: LGTM: Clean prompt dispatching with future extensibility.The pattern matching on
AIActionis idiomatic Dart 3 code, and the exhaustive switch inbuildPredefinedPromptensures all actions are handled. The TODO comment appropriately flags the plan to load prompts from a remote source.
43-57: LGTM: Consistent and clear improvement prompts.The improvement prompts follow a consistent structure with clear instructions. The "Preserve input formatting" directive is important for maintaining user intent.
59-69: LGTM: Parameterized prompts for flexibility.The parameterized
changeToneToandtranslateTomethods provide good flexibility while maintaining consistency with other prompts. Note that all prompt instructions are in English, which should work with most AI backends.
71-78: LGTM: Proper handling of custom prompts.The nullable text handling is correct, allowing custom prompts to work both with and without existing text. The format is consistent with predefined prompts.
lib/features/composer/presentation/widgets/mixins/text_selection_mixin.dart (2)
47-70: Coordinates mapping looks correct; just confirm bottom-right semantics match the JS side.The
TextSelectionCoordinates.fromMapfactory correctly normalizes numeric values todoublewith safe defaults, andpositionreturningOffset(x, y)is straightforward.On the JS side, the selection listener sends
x: rect.rightandy: rect.bottom, i.e., the bottom-right corner of the rect. That meanspositionhere is also bottom-right. Just ensure any overlay/tooltip logic consumingpositionexpects that anchor point (and not top-left), otherwise you may see small placement offsets.
72-79: Mixin design cleanly encapsulates selection handling.The mixin’s API is minimal and clear:
handleSelectionChangeparses the raw map once intoTextSelectionDataand forwards it through a nullableonSelectionChangedgetter, letting widgets opt into handling without extra boilerplate. This keeps selection plumbing localized and easy to reuse.
To have stronger code
2 participants
The logical part of this PR that started to get too big #4187
Included :
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.