Skip to content

🧩 refactor: Move the Message Rows Into @librechat/chat - #17838

Open
berry-13 wants to merge 4 commits into
devfrom
berry-13/chat-messages-rows
Open

berry-13 wants to merge 4 commits into
devfrom
berry-13/chat-messages-rows

Conversation

@berry-13

@berry-13 berry-13 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Summary

Next link of the message-list extraction, following #17802. The rows that draw one message of the thread (MessageRender, Message, MessageContent, ContentRender, MessageParts) and the thread's ThreadMessage switch lived in the client, so the package's message list could only render rows by asking the app to render them through the renderMessageRow slot.

They now live in @librechat/chat and are exported from @librechat/chat/components, and the thread renders them directly. MessagesHost.renderMessageRow is retired. The pieces of a row that stay in the app reach it through new host members the app fills in MessagesHostContext: useMaximizeChatSpace, useAgentsMap, renderMessageBody, renderWakeup, renderPrivateText, renderToolCallLimitNotice and renderAuthorHeader. PrivateText and its owner-text provider stay in the app. Rendering and behavior are unchanged.

Type of change

  • Refactor

Testing

Automated tests:

  • packages/chat: typecheck clean (src and test); jest for messages, react/messages and the consumer spec: 35 suites, 441 tests pass. ContentRender and HoverActions.streaming specs moved into the package against a fake host with their assertions unchanged.
  • client: tsc --noEmit clean; jest for Chat/Messages, Messages, Share, hooks/Files, routes and Providers: 135 suites, 2,466 tests pass.
  • static-checks --against origin/dev passes. ESLint with suppressions off on the 24 touched files exits 0, and packages/chat has no suppression entries.

Screenshots / recordings

No user-facing change.

Risk / compatibility

None: markup and memo comparators are unchanged. The one behavior-adjacent change is that the thread now mounts the row components directly instead of calling a host function, so row identity and memoization are the same as before.

Checklist

  • I reviewed my own changes
  • Relevant tests have been added or updated
  • Existing relevant tests pass
  • The change does not introduce new warnings or errors

@berry-13 berry-13 added the 🔨 refactor Code cleanup without new features label Nov 5, 2026
Copilot AI balanced review requested due to automatic review settings November 5, 2026 16:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T17:35:54.638698Z aa52b17 New commits
🔒 Security Review ✅ Completed 2026-11-05T16:42:56.057491Z 7c7fe58 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c7fe582db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +72 to +75
jest.mock('../Message', () => ({
__esModule: true,
default: (props: ThreadMessageProps) => mockThreadRow(props),
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Exercise the real thread message component in thread tests

The thread tests now replace the newly internal ThreadMessage component with a fabricated row, so sibling navigation and editing assertions exercise the stub's buttons rather than the message components users interact with. This bypasses the integration boundary changed by this refactor and violates AGENTS.md's explicit restriction to mocking external HTTP APIs or uncontrollable services. Render the real message component with the host providers and spy on callbacks instead; the same newly introduced mock in MultiMessage.spec.tsx should be removed as well.

AGENTS.md reference: AGENTS.md:L211-L214

Useful? React with 👍 / 👎.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures.

│ 23      │ 'http://localhost:3080/api/mcp/servers'                                                                         │ 2891.3330000000133 │ 4161.763000000006  │ 200    │
│ 24      │ 'http://localhost:3080/api/permissions/mcpServer/effective/all'                                                 │ 2894.295000000013  │ 3659.6230000000214 │ 200    │
│ 25      │ 'http://localhost:3080/api/prompts/groups?limit=10'                                                             │ 2899.921000000031  │ 4264.340000000026  │ 200    │
│ 26      │ 'http://localhost:3080/api/keys?name=openAI'                                                                    │ 3419.6080000000366 │ 3925.012000000017  │ 200    │
│ 27      │ 'http://localhost:3080/api/presets'                                                                             │ 3440.3340000000317 │ 3951.994000000035  │ 200    │
│ 28      │ 'http://localhost:3080/api/tags'                                                                                │ 3443.130999999994  │ 3954.546000000002  │ 200    │
│ 29      │ 'http://localhost:3080/api/share/link/16390000-0000-4000-8000-000000000001'                                     │ 3443.3830000000307 │ 4166.597000000009  │ 200    │
│ 30      │ 'http://localhost:3080/api/messages/16390000-0000-4000-8000-000000000001?toolPreviews=1'                        │ 3443.5720000000147 │ 4431.793000000005  │ 200    │
│ 31      │ 'http://localhost:3080/api/user/settings/favorites/tools'                                                       │ 3446.5590000000084 │ 4457.68200000003   │ 200    │
│ 32      │ 'http://localhost:3080/api/endpoints/token-config'                                                              │ 3446.82600000003   │ 4209.453999999998  │ 200    │
│ 33      │ 'http://localhost:3080/api/user/settings/skills/active'                                                         │ 3447.042000000016  │ 4670.058000000019  │ 200    │
│ 34      │ 'http://localhost:3080/api/agents/tools/web_search/auth'                                                        │ 3447.975000000035  │ 7187.926000000007  │ 200    │
│ 35      │ 'http://localhost:3080/api/agents/tools/calls?conversationId=16390000-0000-4000-8000-000000000001'              │ 3448.19200000001   │ 4715.991000000009  │ 200    │
│ 36      │ 'http://localhost:3080/api/agents/chat/status/16390000-0000-4000-8000-000000000001?generationProtocolVersion=2' │ 4572.790999999997  │ 4827.5320000000065 │ 200    │
└─────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────┴────────────────────┴────────────────────┴────────┘

Inspect .lighthouse HTML/JSON and e2e/lighthouse/README.md. Reuse loaded user/config data; overlap independent reads without bypassing authorization.

┌─────────┬────────────────────────────┬─────────────────────┬───────┐
│ (index) │ audit                      │ median              │ limit │
├─────────┼────────────────────────────┼─────────────────────┼───────┤
│ 0       │ 'largest-contentful-paint' │ 4570.685            │ 4500  │
│ 1       │ 'cumulative-layout-shift'  │ 0.01790354288883051 │ 0.1   │
│ 2       │ 'total-blocking-time'      │ 353.6890000000003   │ 500   │
└─────────┴────────────────────────────┴─────────────────────┴───────┘

  1) [chromium] › e2e/lighthouse/load.spec.ts:10:5 › serial database latency stays within web-vitals budgets 

    Error: Median largest-contentful-paint must stay within 4500

    expect(received).toBeLessThanOrEqual(expected)

    Expected: <= 4500
    Received:    4570.685

       at audit.ts:127

      125 |   console.table(measured);
      126 |   for (const { audit, median, limit } of measured) {
    > 127 |     expect(median, `Median ${audit} must stay within ${limit}`).toBeLessThanOrEqual(limit);
          |                                                                 ^
      128 |   }
      129 |   return results;
      130 | }
        at auditPage (/home/runner/work/LibreChat/LibreChat/e2e/lighthouse/audit.ts:127:65)
        at /home/runner/work/LibreChat/LibreChat/e2e/lighthouse/load.spec.ts:33:19

    attachment #1: screenshot (image/png) ──────────────────────────────────────────────────────────
    e2e/lighthouse/.test-results/load-serial-database-latency-stays-within-web-vitals-budgets-chromium/test-failed-1.png
    ────────────────────────────────────────────────────────────────────────────────────────────────

    Error Context: e2e/lighthouse/.test-results/load-serial-database-latency-stays-within-web-vitals-budgets-chromium/error-context.md

    attachment #3: trace (application/zip) ─────────────────────────────────────────────────────────
    e2e/lighthouse/.test-results/load-serial-database-latency-stays-within-web-vitals-budgets-chromium/trace.zip
    Usage:

        npx playwright show-trace e2e/lighthouse/.test-results/load-serial-database-latency-stays-within-web-vitals-budgets-chromium/trace.zip

    ────────────────────────────────────────────────────────────────────────────────────────────────


🤖: global teardown has been started
2026-11-05 16:44:02 �[32minfo�[39m: �[32mMongo Connection options�[39m
2026-11-05 16:44:02 �[32minfo�[39m: �[32m{�[39m
�[32m  "bufferCommands": false�[39m
�[32m}�[39m
🤖:  ✅  Connected to Database
🤖:  ✅  Found user in Database
🤖:  ✅  Deleted 1 convos & 2 messages
🤖:  ✅  Deleted user from Database
🤖: global teardown has been started
2026-11-05 16:44:03 �[32minfo�[39m: �[32mMongo Connection options�[39m
2026-11-05 16:44:03 �[32minfo�[39m: �[32m{�[39m
�[32m  "bufferCommands": false�[39m
�[32m}�[39m
🤖:  ✅  Connected to Database
🤖:  ⚠️  User not found in Database
  1 failed
    [chromium] › e2e/lighthouse/load.spec.ts:10:5 › serial database latency stays within web-vitals budgets 

Open the full run

@berry-13
berry-13 force-pushed the berry-13/chat-messages-rows branch from b36fdb1 to 500fc55 Compare November 6, 2026 16:48
MessageRender, Message, MessageContent, ContentRender, MessageParts and the thread's ThreadMessage now live in the package, and the thread renders them directly, retiring MessagesHost.renderMessageRow. Pieces owned elsewhere reach the rows through new host slots.
The thread specs replaced ThreadMessage with a fabricated row, so sibling paging and edit assertions drove the stub. They now render the real row chain with fake hosts and page and edit through its real controls.
The thread specs now render real rows, so their large-chain case was cut to 60 messages. The 12,000-message chain, which guards the iterative branch walk and row reuse on a long conversation, moves to the resolver spec where it does not mount every row.
… Mock

The child-thread spec relied on the test host's renderMessageRow slot to draw nothing. The thread now mounts the package rows directly, so the spec's agents map and client package mocks have to cover them.
@berry-13
berry-13 force-pushed the berry-13/chat-messages-rows branch from 500fc55 to aa52b17 Compare November 6, 2026 17:34

This branch has not been deployed

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

Labels

🔨 refactor Code cleanup without new features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants