Skip to content

Conversation

@mlopezFC
Copy link
Member

@mlopezFC mlopezFC commented Oct 27, 2025

Summary by CodeRabbit

  • New Features

    • Added a demo showcasing a generative-thinking chat mode with streaming, per-word updates, realistic delays, and action buttons.
  • Refactor

    • Reworked message rendering to reuse the loader and markdown viewer for smoother, more stable UI updates.
  • Chores

    • Minor cleanup and project/version dependency updates; removed unused internal constant.

@mlopezFC mlopezFC requested a review from javier-godoy October 27, 2025 19:58
@coderabbitai
Copy link

coderabbitai bot commented Oct 27, 2025

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between 628add3 and 928ea7a.

📒 Files selected for processing (2)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1 hunks)
 _________________________________________________
< Energizer Bunny with a computer science degree. >
 -------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

Walkthrough

Adds a new generative demo and registers it; refactors ChatMessage to reuse a MarkdownViewer and persistent loader with consolidated update logic; removes an unused constant from ChatAssistant; bumps project and markdown-editor versions. New public demo class includes a public streamWords method.

Changes

Cohort / File(s) Summary
Demo registration
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java
Registers the new ChatAssistantGenerativeDemo in the demo suite via addDemo.
New generative demo
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
Adds ChatAssistantGenerativeDemo (extends VerticalLayout) that builds a chat UI, provides "Chat" and "Chat With Generative Thinking" actions, and exposes public Stream<String> streamWords(String fullText) which emits words with randomized per-word delays and updates the UI via UI.access.
Chat message rendering & loader
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java
Introduces a reusable MarkdownViewer field and a persistent loader Div; consolidates update logic into updateMessage(message), toggling loader visibility and updating markdown or plain text in place instead of recreating viewers.
Minor cleanup
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
Removes an unused private constant and an extra blank import; no public API changes.
Build config
pom.xml
Bumps project version to 4.0.0-SNAPSHOT and markdown-editor.version from 1.1.0 to 2.0.1.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Review concurrency and correct use of UI.access / thread handling in ChatAssistantGenerativeDemo.streamWords and async tasks.
  • Verify ChatMessage.updateMessage updates markdown and plain text in-place without leaking components or leaving stale DOM nodes.
  • Confirm loader visibility/state transitions and reuse of MarkdownViewer across repeated updates.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • javier-godoy
  • paodb

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title references three distinct changes present in the changeset: the markdown-editor version update to 2.0.1 (build!), the message implementation fix (fix:), and the new generative AI demo addition (feat(demo):). All three components are legitimate aspects of the PR, making the title related to real parts of the changeset. However, the title attempts to cover multiple distinct changes rather than focusing on a single main point, which makes it somewhat overly broad. While concise labeling would be preferable (the title is 124 characters), it does provide specific information about the changes included.

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.

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: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6da555 and 17bd4ad.

📒 Files selected for processing (2)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)
  • SuppressWarnings (28-41)
⏰ 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). (1)
  • GitHub Check: build-vaadin24
🔇 Additional comments (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)

38-38: LGTM!

The registration of the new demo follows the existing pattern and correctly integrates the new ChatAssistantGenerativeDemo into the demo suite.

Copy link
Member

@javier-godoy javier-godoy left a comment

Choose a reason for hiding this comment

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

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: 0

♻️ Duplicate comments (3)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (3)

114-118: Unread count doesn't match actual message count.

Only one initial message is sent (lines 114-116), but the unread count is set to 4 at line 118. This creates an inconsistent state in the demo.

Align the unread count with the actual number of messages:

-    chatAssistant.setUnreadMessages(4);
+    chatAssistant.setUnreadMessages(1);

130-140: Stream doesn't terminate early when interrupted.

When an InterruptedException is caught at line 136, the interrupt flag is re-set, but the stream continues to process all remaining words. This means an interrupted thread will keep blocking and consuming resources.

Add a check to break out of the stream early when interrupted:

     return Arrays.stream(words)
         .map(word -> {
+            if (Thread.currentThread().isInterrupted()) {
+                return null;
+            }
             try {
                 long delay = ThreadLocalRandom.current().nextLong(50, 250);
                 Thread.sleep(delay);
             } catch (InterruptedException e) {
                 Thread.currentThread().interrupt(); 
+                return null;
             }
             
             return word + " ";
-        });
+        })
+        .takeWhile(word -> word != null);

87-100: User input is ignored in favor of hardcoded sample text.

At line 87, the delayed message is initialized with message.getValue() as content, but this is immediately replaced when streamWords(sampleText) executes at line 100. The user's input is completely ignored.

If the intent is to demonstrate streaming with a consistent sample, initialize with empty or placeholder content:

-      CustomMessage delayedMessage = CustomMessage.builder().loading(true).content(message.getValue())
+      CustomMessage delayedMessage = CustomMessage.builder().loading(true).content("")
           .messageTime(LocalDateTime.now())
           .name("Assistant").avatar("chatbot.png").tagline("Generated by assistant").build();

Alternatively, if the intent is to echo the user's input with streaming, use the actual input:

-          streamWords(sampleText)
+          streamWords(message.getValue())
🧹 Nitpick comments (2)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (1)

67-74: Consider deferring MarkdownViewer content initialization.

At line 72, the MarkdownViewer is initialized with message.getContent(), but then setMessage(message) at line 75 immediately calls updateMessage, which sets the content again at line 106. This results in the content being set twice.

Consider initializing the MarkdownViewer with empty or placeholder content and letting updateMessage handle the actual content:

     if (markdownEnabled) {
-      markdownViewer = new MarkdownViewer(message.getContent());
+      markdownViewer = new MarkdownViewer("");
       this.add(markdownViewer);
     }
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)

123-123: Consider making the method private.

The streamWords method is only used internally within this demo class (at line 100). Making it private would better encapsulate the demo implementation and signal that it's not intended as public API.

-  public Stream<String> streamWords(String fullText) {
+  private Stream<String> streamWords(String fullText) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17bd4ad and c49a134.

📒 Files selected for processing (3)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (4 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1 hunks)
💤 Files with no reviewable changes (1)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)
  • SuppressWarnings (28-41)
🔇 Additional comments (3)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (2)

48-48: Good refactor for component reuse.

Adding a dedicated MarkdownViewer field allows the component to be reused rather than recreated on every update, improving performance.


98-112: The review comment is based on an incorrect assumption.

The Message class defines the content field with @Builder.Default private String content = "";, which means getContent() will never return null—it will return at least an empty string. The null checks present elsewhere in the code (lines 86, 88, 92) are for other nullable properties like name, avatar, and messageTime, which do not have default values. The code at lines 98–112 is safe as written.

Likely an incorrect or invalid review comment.

src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)

1-5: LGTM!

Copyright year correctly updated to 2025.

@mlopezFC mlopezFC requested a review from javier-godoy October 28, 2025 22:00
Copy link
Member

@javier-godoy javier-godoy left a comment

Choose a reason for hiding this comment

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

This PR contains multiple non-WIP commit. Please squash each WIP commit into its corresponding non-WIP commit.

Also note that the last sentence in the commit body must end with a period (applies to non-WIP commit b9ef3c1)

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)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (2)

115-115: Remove redundant clear() call.

The message TextArea is already cleared at line 89 with message.setValue(""), making this call redundant.

Apply this diff:

         }
       });
 
-      message.clear();
     });

124-142: Consider making streamWords private.

The streamWords method is only used internally within this demo class. Making it private would better reflect its intended scope.

Apply this diff:

-  public Stream<String> streamWords(String fullText) {
+  private Stream<String> streamWords(String fullText) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c49a134 and daf72c4.

📒 Files selected for processing (5)
  • pom.xml (1 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (4 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1 hunks)
💤 Files with no reviewable changes (1)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-24T13:40:40.471Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-24T13:40:40.471Z
Learning: In ChatAssistant's initializeAvatar method (src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java), direct DOM manipulation using executeJs is intentionally used to move the Avatar component to be a child of a button that is inside a div within the React-controlled web component structure. This approach is necessary because the target elements are created by React, and standard Vaadin component hierarchy methods cannot be used due to ReactAdapterComponent limitations.

Applied to files:

  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java
📚 Learning: 2024-11-11T17:49:24.845Z
Learnt from: paodb
PR: FlowingCode/ChatAssistant#31
File: pom.xml:317-317
Timestamp: 2024-11-11T17:49:24.845Z
Learning: In `src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`, the parameter `position` in the method `scrollToIndex(int position)` is properly documented at line 328.

Applied to files:

  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
📚 Learning: 2025-07-22T19:52:28.019Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-22T19:52:28.019Z
Learning: When using ReactAdapterComponent with custom React components in ChatAssistant, standard Vaadin component hierarchy methods like add() cannot be used because the component doesn't implement HasComponents, and the DOM structure is controlled by React. Direct DOM manipulation with executeJs may be necessary to integrate Vaadin components (like Avatar) into React-generated DOM structures.

Applied to files:

  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java
🧬 Code graph analysis (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)
  • SuppressWarnings (28-41)
⏰ 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). (1)
  • GitHub Check: build-vaadin24
🔇 Additional comments (2)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (1)

48-48: Excellent refactoring to improve performance.

The changes effectively optimize message updates by:

  • Reusing a single MarkdownViewer instance (lines 48, 71-74, 106) instead of recreating it on each update
  • Maintaining a persistent loader and toggling visibility (lines 67-70, 103) instead of adding/removing it
  • Consolidating update logic in updateMessage (lines 98-112) for better maintainability

This reduces unnecessary DOM manipulation and object creation, improving performance for frequent message updates like the streaming demo.

Also applies to: 67-74, 98-112

pom.xml (1)

21-21: Markdown-editor version 2.0.1 update is secure and properly handled.

Version 2.0.1 is a bugfix release containing JavaScript connector improvements and Javadoc fixes, with no new breaking changes in this patch. Breaking changes were introduced in 2.0.0, which your codebase has already addressed per the ChatMessage updates noted in context. No security vulnerabilities detected in Maven ecosystem for this version.

Comment on lines 103 to 108
streamWords(messageToSend)
.forEach(item -> {
currentUI.access(() -> {
delayedMessage.setContent(delayedMessage.getContent() + " " + item);
chatAssistant.updateMessage(delayedMessage);
});
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix double-spacing and inefficient string concatenation.

Line 106 adds a space before each word, but streamWords already appends a trailing space at line 140. This creates double spaces between words. Additionally, repeated string concatenation (getContent() + " " + item) is inefficient.

Apply this diff to fix both issues:

           streamWords(messageToSend)
           .forEach(item -> {
             currentUI.access(() -> {
-              delayedMessage.setContent(delayedMessage.getContent() + " " + item);
+              delayedMessage.setContent(delayedMessage.getContent() + item);
               chatAssistant.updateMessage(delayedMessage); 
             });
           });
🤖 Prompt for AI Agents
In
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
around lines 103 to 109, the loop prepends an extra space before each word and
repeatedly concatenates strings via getContent() + " " + item which is
inefficient; change the code to create a single StringBuilder initialized with
delayedMessage.getContent(), then in the forEach append only item (no added
space since streamWords already includes trailing spaces), and after the stream
completes call delayedMessage.setContent(builder.toString()) and
chatAssistant.updateMessage(delayedMessage) once (inside currentUI.access if
needed) to avoid double-spacing and costly repeated concatenation.

@javier-godoy javier-godoy changed the title feat(demo): add generative ai simulation demo fix:fix update message implementation and build:update markdowneditor to 2.0.1 and feat(demo):add generative ai simulation demo Oct 29, 2025
Fix update message logic avoiding creation and removal of internal
elements depending on the message and just focusing on updating the
contents of the internal state, and moving the creation of the internal
elements to the constructor of the component.

Closes #47
@javier-godoy javier-godoy changed the title fix:fix update message implementation and build:update markdowneditor to 2.0.1 and feat(demo):add generative ai simulation demo build!:update markdowneditor to 2.0.1 fix:fix update message implementation and feat(demo):add generative ai simulation demo Oct 29, 2025
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

♻️ Duplicate comments (2)
pom.xml (1)

21-21: Dependency major version upgrade handled correctly.

The markdown-editor upgrade from 1.1.0 to 2.0.1 is a breaking change, which has been appropriately reflected in the project's major version bump to 4.0.0-SNAPSHOT.

src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)

103-109: Fix double-spacing and inefficient string concatenation.

Two issues in the streaming loop:

  1. Double-spacing: Line 106 prepends a space (" " + item), but streamWords at line 140 already appends a trailing space to each word. This creates double spaces between words.
  2. Inefficient concatenation: Repeatedly calling getContent() + " " + item in a loop is inefficient and allocates many intermediate String objects.

Apply this diff to fix both issues:

           streamWords(messageToSend)
           .forEach(item -> {
             currentUI.access(() -> {
-              delayedMessage.setContent(delayedMessage.getContent() + " " + item);
+              delayedMessage.setContent(delayedMessage.getContent() + item);
               chatAssistant.updateMessage(delayedMessage); 
             });
           });

Alternatively, for better performance with many words, consider using a StringBuilder outside the stream and appending once at the end.

🧹 Nitpick comments (3)
src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (2)

48-48: Good refactor: reusable MarkdownViewer instance.

Introducing a persistent markdownViewer field enables content updates without recreating the component, improving performance and reducing overhead.


98-112: Excellent refactoring of update logic.

Consolidating loading state and content updates into updateMessage improves clarity. Reusing the markdownViewer instance via setContent() is more efficient than recreating it on each update.

src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)

124-142: Consider making streamWords private.

The streamWords method is only used internally within this demo class. Making it private would better reflect its intended scope and prevent unintended external usage.

Apply this diff:

-  public Stream<String> streamWords(String fullText) {
+  private Stream<String> streamWords(String fullText) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between daf72c4 and b624d64.

📒 Files selected for processing (5)
  • pom.xml (2 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java (0 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (4 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1 hunks)
💤 Files with no reviewable changes (1)
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-24T13:40:40.471Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-24T13:40:40.471Z
Learning: In ChatAssistant's initializeAvatar method (src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java), direct DOM manipulation using executeJs is intentionally used to move the Avatar component to be a child of a button that is inside a div within the React-controlled web component structure. This approach is necessary because the target elements are created by React, and standard Vaadin component hierarchy methods cannot be used due to ReactAdapterComponent limitations.

Applied to files:

  • pom.xml
  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
📚 Learning: 2025-07-22T19:52:28.019Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-22T19:52:28.019Z
Learning: When using ReactAdapterComponent with custom React components in ChatAssistant, standard Vaadin component hierarchy methods like add() cannot be used because the component doesn't implement HasComponents, and the DOM structure is controlled by React. Direct DOM manipulation with executeJs may be necessary to integrate Vaadin components (like Avatar) into React-generated DOM structures.

Applied to files:

  • src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java
📚 Learning: 2024-11-11T17:49:24.845Z
Learnt from: paodb
PR: FlowingCode/ChatAssistant#31
File: pom.xml:317-317
Timestamp: 2024-11-11T17:49:24.845Z
Learning: In `src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`, the parameter `position` in the method `scrollToIndex(int position)` is properly documented at line 328.

Applied to files:

  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
🧬 Code graph analysis (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)
  • SuppressWarnings (28-41)
🔇 Additional comments (7)
pom.xml (1)

7-7: Major version bump aligns with dependency upgrade.

The project version bump from 3.0.1 to 4.0.0 is appropriate given the major version upgrade of the markdown-editor dependency from 1.1.0 to 2.0.1, which may introduce breaking changes.

src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatMessage.java (1)

67-74: LGTM: Clean initialization of reusable components.

The loader and markdownViewer are now initialized once in the constructor, with visibility managed dynamically. This is more efficient than the previous approach.

src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)

38-38: LGTM: New demo registered correctly.

The ChatAssistantGenerativeDemo is properly registered following the existing pattern.

src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (4)

1-46: LGTM: Standard demo setup.

The file header, imports, and class structure follow the expected pattern for a Vaadin demo component.


48-76: LGTM: ChatAssistant configuration is correct.

The ChatAssistant setup with avatar provider, typing indicators, custom renderer, and submit listener is implemented properly.


78-85: LGTM: Simple chat button works correctly.

The "Chat" button implementation properly sends the message and clears the input field.


117-122: LGTM: Initial message and layout setup.

The initial assistant message and component layout are configured correctly.

}
});

message.clear();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove redundant clear() call.

Line 115 calls message.clear(), but the TextArea was already cleared at line 89. This is redundant.

Apply this diff:

       });

-      message.clear();
     });
📝 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.

Suggested change
message.clear();
});
});
🤖 Prompt for AI Agents
In
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
around line 115, there is a redundant call to message.clear() — the TextArea is
already cleared earlier at line 89; remove the second message.clear() at line
115 so the duplicate clearing is eliminated and the test/demo code remains
concise.

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: 0

🧹 Nitpick comments (3)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (3)

103-109: Optimize string concatenation for better performance.

Repeated string concatenation via getContent() + item creates a new string object on each iteration, which is inefficient. For a demo this works, but consider using a StringBuilder to accumulate content.

If you'd like to optimize, accumulate words in a StringBuilder and set content once at the end, or less frequently:

StringBuilder accumulated = new StringBuilder();
streamWords(messageToSend)
.forEach(item -> {
  accumulated.append(item);
  currentUI.access(() -> {
    delayedMessage.setContent(accumulated.toString());
    chatAssistant.updateMessage(delayedMessage); 
  });
});

115-115: Remove redundant clear() call.

The TextArea is already cleared at line 89 with message.setValue(""). This second clear() call at line 115 is redundant.

Apply this diff:

      });

-      message.clear();
    });

124-142: Consider making streamWords private.

The streamWords method is only used internally within this demo class (line 103). Making it private would better reflect its intended scope and prevent unintended external use.

Apply this diff:

-  public Stream<String> streamWords(String fullText) {
+  private Stream<String> streamWords(String fullText) {

Note: The blocking nature of Thread.sleep is acceptable for this demo context since it runs in the CompletableFuture's async thread pool.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b624d64 and 628add3.

📒 Files selected for processing (2)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-24T13:40:40.471Z
Learnt from: mlopezFC
PR: FlowingCode/ChatAssistant#35
File: src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java:179-186
Timestamp: 2025-07-24T13:40:40.471Z
Learning: In ChatAssistant's initializeAvatar method (src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java), direct DOM manipulation using executeJs is intentionally used to move the Avatar component to be a child of a button that is inside a div within the React-controlled web component structure. This approach is necessary because the target elements are created by React, and standard Vaadin component hierarchy methods cannot be used due to ReactAdapterComponent limitations.

Applied to files:

  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
📚 Learning: 2024-11-11T17:49:24.845Z
Learnt from: paodb
PR: FlowingCode/ChatAssistant#31
File: pom.xml:317-317
Timestamp: 2024-11-11T17:49:24.845Z
Learning: In `src/main/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistant.java`, the parameter `position` in the method `scrollToIndex(int position)` is properly documented at line 328.

Applied to files:

  • src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java
🧬 Code graph analysis (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (1)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)
  • SuppressWarnings (28-41)
🔇 Additional comments (3)
src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantDemoView.java (1)

38-38: LGTM! New demo properly registered.

The generative demo is correctly added to the demo suite following the established pattern.

src/test/java/com/flowingcode/vaadin/addons/chatassistant/ChatAssistantGenerativeDemo.java (2)

48-76: Well-structured demo setup.

The demo initialization is clear and demonstrates good UX practices with the typing indicator. The custom renderer pattern is correctly implemented.


78-85: LGTM! Simple chat functionality works correctly.

The basic chat button properly sends the message and clears the input field.

@javier-godoy javier-godoy merged commit 58d78a2 into master Oct 29, 2025
2 of 3 checks passed
@javier-godoy javier-godoy deleted the issue-49 branch October 29, 2025 19:59
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.

3 participants