Skip to content

[mcp] Add proper web-vitals metric collection #33109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jorge-cab
Copy link
Contributor

@jorge-cab jorge-cab commented May 2, 2025

Multiple things here:

  • Improve the mean calculation for metrics so we don't report 0 when web-vitals fail to be retrieved
  • improve ui chaos monkey to use puppeteer APIs since only those trigger INP/CLS metrics since we need emulated mouse clicks
  • Add logic to navigate to a temp page after render since some web-vitals metrics are only calculated when the page is backgrounded
  • Some readability improvements

import {StdioServerTransport} from '@modelcontextprotocol/sdk/server/stdio.js';
import {z} from 'zod';
import {compile, type PrintedCompilerPipelineValue} from './compiler';
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
Copy link
Member

Choose a reason for hiding this comment

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

Looks like maybe you have some lint configuration misconfigured

@@ -29,16 +33,16 @@ const server = new McpServer({

server.tool(
'query-react-dev-docs',
'Search/look up official docs from react.dev',
'This tool lets you search for official docs from react.dev. This always has the most up to date information on React. You can look for documentation on APIs such as <ViewTransition>, <Activity>, and hooks like useOptimistic, useSyncExternalStore, useTransition, and more. Whenever you think hard about React, please use this tool to get more information before proceeding.',
Copy link
Member

Choose a reason for hiding this comment

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

I saw that @poteto removed "please"s from the prompts in a previous PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I accidentally got myself into merge hell for trying to do mercurial patterns but shawn saved me lol

Copy link
Member

@poteto poteto left a comment

Choose a reason for hiding this comment

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

Did you try out in claude?


TEST: ${results.webVitals.inp}
Copy link
Member

Choose a reason for hiding this comment

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

what's the TESTfor?

Comment on lines +138 to +141
for (const selector of selectors) {
await page.click(selector);
await delay(500);
}
Copy link
Member

Choose a reason for hiding this comment

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

depending on the number of elements this could be pretty slow. I don't think you need to wait 500ms after every click before clicking again right?

Suggested change
for (const selector of selectors) {
await page.click(selector);
await delay(500);
}
await Promise.all(selectors.map((selector) => page.click(selector)));
await delay(500);

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.

4 participants