Skip to content

Conversation

@zatteo
Copy link
Member

@zatteo zatteo commented Dec 11, 2025

Rely on the work of the backend team to get the JMAP capability:

  • display AI Scribe features only if it exists
  • send calls to the route from the JMAP capability

Summary by CodeRabbit

Release Notes

  • New Features

    • Added AI-powered text editing assistance in the composer with suggestions for grammar, tone, translation, and more.
    • Integrated AI text generation capabilities with replace and insert actions for suggested edits.
    • New AI assistant modal and overlay UI for accessing AI suggestions while composing.
  • Improvements

    • Enhanced availability detection for AI features based on account capabilities.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 11, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This PR migrates AI Scribe integration from a static configuration model to a capability-based system. It removes the AIConfig singleton class and replaces AIScribeInComposerMixin with AiScribeMixin, introducing extension-based functionality via HandleAiScribeInComposerExtension. The data layer is restructured to use DioClient instead of direct Dio instances, and the scribe package's public API is significantly expanded with new UI components (context menus, modals, buttons, overlays), models (AI capability, text selection, modal layout), and utilities (modal manager, layout calculator). Supporting changes include new image assets, localization entries, and session extensions for AI capability retrieval.

Possibly related PRs

  • linagora/tmail-flutter\AI scribe data layer and requirements #4196 - Modifies the same AI scribe module files (scribe barrel, AI data layer, API/datasource/repository, text selection, and related models)
  • linagora/tmail-flutter\AI scribe UI #4197 - Modifies AI-scribe composer integration with replacement of mixins and AIScribe-related methods

Suggested reviewers

  • chibenwa
  • tddang-linagora

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: shifting from direct LLM calls to using JMAP backend for AI Scribe functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

This PR has been deployed to https://linagora.github.io/tmail-flutter/4203.

@Crash-- Crash-- mentioned this pull request Dec 12, 2025
@chibenwa
Copy link
Member

@coderabbitai review please

@coderabbitai
Copy link

coderabbitai bot commented Dec 12, 2025

@chibenwa: I'll review the changes in this PR focusing on the JMAP backend integration for the AI Scribe feature.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart (1)

828-857: LGTM! Clean implementation of AI scribe binding injection.

The method correctly follows GetX patterns by checking Get.isRegistered<T>() before deleting bindings, and the error handling with try/catch is consistent with injectVacationBindings. Based on learnings, this approach is appropriate since Get.delete<T>() doesn't throw but checking registration first is defensive.

One minor observation: consider extracting the binding types to a list to reduce repetition:

// Optional refactor for reduced repetition:
final bindingTypes = [
  AIDataSourceImpl,
  AIDataSource,
  AIScribeRepositoryImpl,
  AIScribeRepository,
  GenerateAITextInteractor,
];
for (final type in bindingTypes) {
  if (Get.isRegistered(tag: type.toString())) {
    Get.delete(tag: type.toString(), force: true);
  }
}

However, the current explicit approach is also valid and more type-safe.

scribe/lib/scribe/ai/data/network/ai_api.dart (1)

20-22: Consider using a more specific exception type.

Throwing a generic Exception makes it harder for callers to handle this specific error case. Consider defining a custom exception (e.g., AIApiNotAvailableException) or using an existing domain exception for better error handling upstream.

+class AIApiNotAvailableException implements Exception {
+  final String message;
+  AIApiNotAvailableException([this.message = 'AI API is not available']);
+  @override
+  String toString() => message;
+}

 if (url == null) {
-  throw Exception('AI API is not available');
+  throw AIApiNotAvailableException();
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f385d39 and 2f2ac2e.

📒 Files selected for processing (13)
  • lib/features/composer/presentation/composer_controller.dart (1 hunks)
  • lib/features/composer/presentation/composer_view.dart (2 hunks)
  • lib/features/composer/presentation/composer_view_web.dart (3 hunks)
  • lib/features/composer/presentation/mixin/ai_scribe_in_composer_mixin.dart (2 hunks)
  • lib/features/home/domain/extensions/session_extensions.dart (3 hunks)
  • lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart (3 hunks)
  • model/lib/ai/ai_capability.dart (1 hunks)
  • model/lib/ai/capability_ai.dart (1 hunks)
  • scribe/lib/scribe/ai/data/config/ai_config.dart (0 hunks)
  • scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart (1 hunks)
  • scribe/lib/scribe/ai/data/model/ai_api_request.dart (0 hunks)
  • scribe/lib/scribe/ai/data/network/ai_api.dart (1 hunks)
  • scribe/lib/scribe/ai/presentation/bindings/ai_scribe_bindings.dart (2 hunks)
💤 Files with no reviewable changes (2)
  • scribe/lib/scribe/ai/data/model/ai_api_request.dart
  • scribe/lib/scribe/ai/data/config/ai_config.dart
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-12T04:54:05.432Z
Learnt from: dab246
Repo: linagora/tmail-flutter PR: 4191
File: lib/features/email/presentation/extensions/handle_email_action_extension.dart:37-80
Timestamp: 2025-12-12T04:54:05.432Z
Learning: In lib/features/email/presentation/extensions/handle_email_action_extension.dart, the mailboxDashBoardController.selectedEmail should only be synchronized when isMobileThreadDisabled is true. This is intentional behavior and should not be changed to update selectedEmail in non-mobile or thread-enabled contexts.

Applied to files:

  • lib/features/composer/presentation/mixin/ai_scribe_in_composer_mixin.dart
  • lib/features/composer/presentation/composer_controller.dart
  • lib/features/composer/presentation/composer_view_web.dart
  • lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
📚 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:

  • lib/features/composer/presentation/mixin/ai_scribe_in_composer_mixin.dart
  • lib/features/composer/presentation/composer_controller.dart
  • lib/features/home/domain/extensions/session_extensions.dart
  • model/lib/ai/ai_capability.dart
  • lib/features/composer/presentation/composer_view_web.dart
  • scribe/lib/scribe/ai/data/network/ai_api.dart
  • model/lib/ai/capability_ai.dart
  • lib/features/composer/presentation/composer_view.dart
  • scribe/lib/scribe/ai/presentation/bindings/ai_scribe_bindings.dart
  • lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
  • scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart
📚 Learning: 2025-12-09T12:47:45.861Z
Learnt from: zatteo
Repo: linagora/tmail-flutter PR: 4196
File: scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart:34-35
Timestamp: 2025-12-09T12:47:45.861Z
Learning: In the tmail-flutter repository, avoid suggesting replacing DioError with DioException unless there is a codebase-wide migration. This pattern applies to all Dart files; when reviewing, only propose a DioError-safe change (e.g., compatible error handling or conversion) unless a global migration is in place. Ensure consistency of DioError usage across modules and flag any deviations for a repo-wide decision.

Applied to files:

  • lib/features/composer/presentation/mixin/ai_scribe_in_composer_mixin.dart
  • lib/features/composer/presentation/composer_controller.dart
  • lib/features/home/domain/extensions/session_extensions.dart
  • model/lib/ai/ai_capability.dart
  • lib/features/composer/presentation/composer_view_web.dart
  • scribe/lib/scribe/ai/data/network/ai_api.dart
  • model/lib/ai/capability_ai.dart
  • lib/features/composer/presentation/composer_view.dart
  • scribe/lib/scribe/ai/presentation/bindings/ai_scribe_bindings.dart
  • lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart
  • scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart
🔇 Additional comments (13)
model/lib/ai/capability_ai.dart (1)

1-3: LGTM! Clean capability identifier definition.

The capability identifier follows JMAP conventions and integrates cleanly with the capability infrastructure.

model/lib/ai/ai_capability.dart (1)

1-20: LGTM! Well-structured capability model.

The AICapability class follows established patterns for capability properties with proper JSON serialization and equality support.

lib/features/home/domain/extensions/session_extensions.dart (1)

15-16: LGTM! Consistent capability integration.

The AI capability follows the established pattern of other capability getters with appropriate error handling and logging.

Also applies to: 38-38, 172-184

lib/features/composer/presentation/mixin/ai_scribe_in_composer_mixin.dart (1)

25-35: Verify the dual-check logic for AI availability.

The isAIScribeAvailable checks both the global AIConfig.isAiEnabled flag (line 26) and the session capability (line 33). This means AI Scribe will only be available if both conditions are met.

Is this intentional? The PR description states "Display AI Scribe features only if the JMAP capability exists," which suggests the capability check alone should be sufficient. The global config check might be:

  • Intentional for a phased rollout or feature flag
  • A defensive measure during migration
  • An oversight

Please confirm whether both checks are needed or if the global config check can be removed in favor of relying solely on the JMAP capability.

lib/features/composer/presentation/composer_controller.dart (1)

271-275: LGTM! Clean implementation of session/accountId exposure.

The getters appropriately delegate to the mailbox dashboard controller and correctly implement the mixin's abstract getters.

lib/features/composer/presentation/composer_view.dart (1)

472-473: LGTM! Proper UI gating based on capability availability.

The UI correctly gates AIScribe features using controller.isAIScribeAvailable, hiding the UI when the capability is not present.

Also applies to: 558-560

scribe/lib/scribe/ai/presentation/bindings/ai_scribe_bindings.dart (1)

10-12: Verify behavior when endpoint is null.

The scribeEndpoint parameter is optional (line 10), and it's passed directly to AIDataSourceImpl without validation (line 26).

Please verify:

  1. What happens when endpoint is null in AIDataSourceImpl and subsequently in AIApi?
  2. Is there a default endpoint that will be used?
  3. Should there be validation to ensure the endpoint is provided when AI features are enabled?

Without clear fallback behavior, initializing AIScribe bindings without an endpoint could lead to runtime errors when attempting to make AI requests.

Consider adding validation or documentation about the expected behavior when scribeEndpoint is null.

Also applies to: 24-27

scribe/lib/scribe/ai/data/datasource_impl/ai_datasource_impl.dart (1)

11-17: Verify AIApi behavior with null endpoint.

The endpoint parameter is optional and passed directly to AIApi (line 16). This change removes the previous hardcoded AIConfig.aiApiKey and AIConfig.aiApiUrl usage in favor of a configurable endpoint.

Please ensure that AIApi handles a null endpoint appropriately:

  1. Does AIApi have a default endpoint when none is provided?
  2. Will it throw a clear error if endpoint is null and a request is attempted?
  3. Should there be a runtime check here to fail fast if endpoint is null?

This relates to the same concern raised in ai_scribe_bindings.dart about null endpoint handling.

lib/features/mailbox_dashboard/presentation/controller/mailbox_dashboard_controller.dart (1)

898-898: Verify binding injection order.

The injectAIScribeBindings call is placed after injectPreferencesBindings and before the mobile FCM bindings. Ensure this ordering doesn't create any dependency issues if AI scribe bindings rely on preferences or other injected dependencies.

lib/features/composer/presentation/composer_view_web.dart (3)

562-563: LGTM! Consistent capability-based AI scribe gating.

The conditional logic correctly disables the AI scribe button when the capability is unavailable by passing null for both the callback and the key. This is a clean pattern for feature gating.


838-839: Tablet view follows the same pattern - LGTM.

Consistent with the desktop implementation.


959-962: Selection button guard is correct.

Early return when isAIScribeAvailable is false prevents unnecessary widget building and Obx subscription.

scribe/lib/scribe/ai/data/network/ai_api.dart (1)

5-13: LGTM on the endpoint-based API restructure.

The simplification from separate API key and base URL to a single endpoint aligns with the JMAP capability-driven approach where the endpoint is provided by the session capability.

@hoangdat
Copy link
Member

  • @dab246 will follow up on this PR to combine UI + JMAP capability for Scribe, so no need to fix up more @zatteo

@hoangdat hoangdat changed the base branch from ai-scribe-ui to features/ai December 18, 2025 02:15
zatteo and others added 11 commits December 18, 2025 09:23
Check JMAP capability AND if AI is enabled in env.file and use this method everywhere  where we checked env.file
No more API key and url needed because everything is managed by the backend.

Also backend does not accept a model parameter for the moment.
It was used at the beginning for configuration purpose but now we
have the JMAP capability, no need anymore.
@hoangdat
Copy link
Member

hoangdat commented Dec 18, 2025

  • should add Enter to send prompt directly. Shift + Enter to add new line
image

@dab246
Copy link
Member

dab246 commented Dec 18, 2025

  • should add Enter to send prompt directly. Shift + Enter to add new line

Fixed

@dab246
Copy link
Member

dab246 commented Dec 18, 2025

  • Auto hide Replace action when perform prompt in new email without content
Screenshot 2025-12-18 at 16 42 19

@tddang-linagora
Copy link
Collaborator

When select 3 lines and prompt, and the response also contains multiple lines text, click "Replace", only the last response's line is insert

Also cannot reproduce withReplace option, all text from this selection also be replaced, not only selected text with 4212 PR

@dab246
Copy link
Member

dab246 commented Dec 18, 2025

When using multiple composer, we get multiple overlay icons

Fixed

Screen.Recording.2025-12-18.at.17.01.52.mov

@hoangdat
Copy link
Member

hoangdat commented Dec 18, 2025

  • Alignment of prompt when AI button at very right of composer
image
  • alignment of prompt when response have long result but at the bottom of composer
image

@hoangdat
Copy link
Member

hoangdat commented Dec 18, 2025

  • click on response text of AI, dialog to be disappeared
Screen.Recording.2025-12-18.at.5.39.49.PM.mov

@hoangdat
Copy link
Member

@zatteo I built image: v0.22.2-ai-scribe-01, please help me to deploy into cozy platform to test and feedback team.

Cc @chibenwa @poupotte @paultranvan

@zatteo
Copy link
Member Author

zatteo commented Dec 18, 2025

Summary of feedbacks from everyone who tested:

  • AI scribe icon in bottom bar is pixelated

=> #4216

  • If you scroll with a selection, the AI selection button does not follow the selection
Screen.Recording.2025-12-18.at.17.12.04.mov
  • After insert or replacing, it should hide the AI selection button (deselecting the text is a good solution 👍)

I checked this one and it is fixed by #4212 or #4214

  • With a lot of content and selecting all content, we do not see the AI selection button
Screen.Recording.2025-12-18.at.17.17.16.mov
  • It would be nice to be able to select the text from the LLM result (to copy past part of the LLM result)

=> #4215

@dab246
Copy link
Member

dab246 commented Dec 19, 2025

  • click on response text of AI, dialog to be disappeared

Fixed

… of composer or response have long result but at the bottom of composer
@dab246
Copy link
Member

dab246 commented Dec 19, 2025

  • Alignment of prompt when AI button at very right of composer

  • alignment of prompt when response have long result but at the bottom of composer

Fixed

Screen.Recording.2025-12-19.at.11.36.18.mov

@hoangdat
Copy link
Member

If you scroll with a selection, the AI selection button does not follow the selection

as other web AI do. Fx: chatgpt

Screen.Recording.2025-12-19.at.12.00.50.PM.mov

@hoangdat hoangdat merged commit c8bf127 into features/ai Dec 19, 2025
20 checks passed
This was referenced Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants