Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Sep 15, 2025

PR Type

Enhancement


Description

  • Remove unused is_append property from chat message handling

  • Increase log filter size from 80 to 100 entries


Diagram Walkthrough

flowchart LR
  A["Chat Message Processing"] --> B["Remove is_append Check"]
  C["Log Filters"] --> D["Increase Size 80→100"]
Loading

File Walkthrough

Relevant files
Enhancement
chat-box.svelte
Remove unused is_append check                                                       

src/routes/chat/[agentId]/[conversationId]/chat-box.svelte

  • Remove is_append condition from message replacement logic
+0/-1     
conversationTypes.js
Remove is_append from type definition                                       

src/lib/helpers/types/conversationTypes.js

  • Remove is_append property from ChatResponseModel type definition
+0/-1     
Configuration changes
persist-log.svelte
Increase log filter sizes                                                               

src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte

  • Increase contentLogFilter size from 80 to 100
  • Increase stateLogFilter size from 80 to 100
+2/-2     

@iceljc iceljc marked this pull request as draft September 15, 2025 14:48
@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Removing the !message.is_append guard changes update behavior for the last assistant message; verify that streamed or partial assistant messages are not mistakenly replaced instead of appended, and that multi-chunk responses still render correctly.

if (!message.is_streaming) {
	if (dialogs[dialogs.length - 1]?.message_id === message.message_id
		&& dialogs[dialogs.length - 1]?.sender?.role === UserRole.Assistant
	) {
		dialogs[dialogs.length - 1] = {
			...message,
			is_chat_message: true
Type Sync

After removing is_append from the JSDoc type, ensure all call sites and APIs no longer send or rely on this field to avoid unexpected runtime properties and confusion in downstream consumers.

* @property {RichContent} rich_content - Rich content.
* @property {string} post_action_disclaimer - The message disclaimer.
* @property {string} data - The message data.
* @property {Object} states
* @property {Date} created_at - The message sent time.
* @property {boolean} has_message_files
* @property {boolean} is_chat_message
* @property {boolean} is_streaming
* @property {string} [indication]
*/

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

@iceljc iceljc marked this pull request as ready for review September 17, 2025 00:13
@iceljc iceljc merged commit 3996704 into SciSharp:main Sep 17, 2025
1 of 2 checks passed
@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Behavioral Change

Removing the is_append guard alters how incoming assistant messages replace the last dialog entry; verify this doesn’t regress streaming or multi-chunk message assembly where partials used to rely on is_append.

if (!message.is_streaming) {
	if (dialogs[dialogs.length - 1]?.message_id === message.message_id
		&& dialogs[dialogs.length - 1]?.sender?.role === UserRole.Assistant
	) {
		dialogs[dialogs.length - 1] = {
			...message,
			is_chat_message: true

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Replace magic number with a constant

Replace the magic number 100 with a named constant for the size of
contentLogFilter and stateLogFilter to improve readability and maintainability.

src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte [51-53]

-let contentLogFilter = { size: 100, startTime: utcNow };
+const LOG_FILTER_SIZE = 100;
 /** @type {import('$conversationTypes').ConversationLogFilter} */
-let stateLogFilter = { size: 100, startTime: utcNow };
+let contentLogFilter = { size: LOG_FILTER_SIZE, startTime: utcNow };
+/** @type {import('$conversationTypes').ConversationLogFilter} */
+let stateLogFilter = { size: LOG_FILTER_SIZE, startTime: utcNow };
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies a "magic number" used in multiple places and proposes replacing it with a constant, which is a good practice for improving code readability and maintainability.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant