Skip to content

feat(playground): reworking to incorporate graph and rewrite placehol…#1637

Merged
neelneelneel merged 1 commit intodevfrom
1617-rewrite-and-show-output-as-siblings
Aug 1, 2025
Merged

feat(playground): reworking to incorporate graph and rewrite placehol…#1637
neelneelneel merged 1 commit intodevfrom
1617-rewrite-and-show-output-as-siblings

Conversation

@neelneelneel
Copy link
Copy Markdown
Contributor

…ders

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes

@neelneelneel neelneelneel requested a review from a team as a code owner August 1, 2025 02:39
@neelneelneel neelneelneel linked an issue Aug 1, 2025 that may be closed by this pull request
1 task
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /describe

@neelneelneel neelneelneel merged commit c829baf into dev Aug 1, 2025
4 checks passed
@neelneelneel neelneelneel deleted the 1617-rewrite-and-show-output-as-siblings branch August 1, 2025 02:39
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /update_changelog

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(playground): reworking to incorporate graph and rewrite placehol…


User description

…ders

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes


PR Type

Enhancement


Description

  • Add sibling navigation in message hover UI

  • Introduce "Rewrite" button placeholder action

  • Refactor ChatMessage into parent/children tree

  • Update ChatRoom history via message tree


Diagram Walkthrough

flowchart LR
  PM["PixelMessage"] --> CC["createChatMessage"]
  CC --> CM["ChatMessage (node)"]
  CM --> RT["Root message"]
  RT --> CH["Child messages"]
  CH --> HT["History traversal"]
Loading

File Walkthrough

Relevant files
Enhancement
RoomMessage.tsx
Add rewrite button and sibling navigation                               

packages/playground/src/components/room/RoomMessage.tsx

  • Added ChevronLeft and ChevronRight icons
  • Introduced rewriteMessage handler and button
  • Implemented sibling navigation UI controls
  • Updated sidebar toggling to use message.id
+70/-4   
chat.message.ts
Refactor ChatMessage into tree structure                                 

packages/playground/src/stores/chat/chat.message.ts

  • Renamed messageId to id
  • Added parent, children, position fields
  • Implemented connectParent, addChild, activateMessage
  • Converted update methods to arrow functions
+126/-15
chat.room.ts
Use root-based tree for chat history                                         

packages/playground/src/stores/chat/chat.room.ts

  • Introduced ROOT_MESSAGE_ID and TEMP_MESSAGE_ID
  • Replaced history array with root-based tree
  • Updated history getter to traverse active children
  • Refactored message loading to use createChatMessage and addChild
+87/-75 
types.d.ts
Add parentMessageId to PixelMessage                                           

packages/playground/src/types.d.ts

  • Added optional parentMessageId to PixelMessage
+1/-0     

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /review

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 1, 2025

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Null Parent

The parent property is typed as ChatMessage but initialized to null, leading to potential null dereference when accessing parent.children in the siblings getter.

export class ChatMessage {
	private _store: ChatMessageInterface = {
		id: "",
		parent: null,
		position: -1,
		children: [],
Incomplete Message Mapping

The createChatMessage method only handles INPUT_TEXT and never updates content for response messages, so agent replies will render with empty content.

/**
 * Create a ChatMessage from a pixelMessage
 * @param pixelMessage - message from backend that needs to be converted
 */
private createChatMessage = (pixelMessage: PixelMessage): ChatMessage => {
	// create a message
	const message = new ChatMessage(pixelMessage.messageId);
downloadHistory Formatting

In downloadHistory, html is built via map but never joined into a single string, resulting in an array rather than valid HTML.

this.setIsLoading(true);

// convert the content to html
const html = this.history
	.map((message) => {
		if (message.content.type === "TEXT") {

@QodoAI-Agent
Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Guard parent null access

Add a null‐check for parent to avoid accessing children on a null parent and causing
a runtime error. Return an empty array when parent is not set (e.g., for the root
message).

packages/playground/src/stores/chat/chat.message.ts [123-125]

 get siblings() {
-  return this._store.parent.children;
+  return this._store.parent ? this._store.parent.children : [];
 }
Suggestion importance[1-10]: 7

__

Why: Prevents a runtime error when accessing parent.children on the root message; correct null‐check improves robustness.

Medium
General
Enable rewrite button

Change rewriteMessage to accept the text to rewrite and invoke it with
message.content.text, and remove the hardcoded disabled prop so the button is
clickable when rewriting is supported.

packages/playground/src/components/room/RoomMessage.tsx [242-250]

 <Button
   variant="text"
   size="small"
   color="inherit"
-  disabled={true}
-  onClick={() => rewriteMessage()}
+  onClick={() => rewriteMessage(message.content.text)}
 >
   Rewrite
 </Button>
Suggestion importance[1-10]: 4

__

Why: Enabling the rewrite button improves UX but overlooks updating rewriteMessage to accept an argument, making the change incomplete.

Low
Simplify tools mapping

Remove the unnecessary second argument passed to map, as it is treated as a thisArg
and does not serve as an accumulator. Simplifying this call makes the intent
clearer.

packages/playground/src/stores/chat/chat.room.ts [406-409]

-const tools: string[] = this._store.options.tools.map(
-  (t) => t.id,
-  [],
-);
+const tools: string[] = this._store.options.tools.map((t) => t.id);
Suggestion importance[1-10]: 4

__

Why: Removing the unused thisArg makes the map call clearer and more concise without affecting functionality.

Low

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.

Rewrite and Show Output as Siblings

2 participants