Skip to content

Conversation

@knudtty
Copy link
Contributor

@knudtty knudtty commented Dec 18, 2025

Still seems somewhat present but noticeably better. Profiler shows a great reduction

@knudtty knudtty requested review from a team and pulpdrew and removed request for a team December 18, 2025 23:04
@changeset-bot
Copy link

changeset-bot bot commented Dec 18, 2025

⚠️ No Changeset found

Latest commit: 53329e7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Dec 18, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Dec 19, 2025 5:46pm

@claude
Copy link

claude bot commented Dec 18, 2025

PR Review: Fix lagginess on search page

Performance Optimizations ✅

Good approach using useWatch instead of watch to reduce re-renders. The component extraction and memoization strategy is sound.

Issues Found

  • ⚠️ Inconsistent memoization in DBSearchPage.tsx:548 → SaveSearchModal is memoized but its props are not stable (callbacks like onClose recreated every render). Either remove memo or wrap callbacks in useCallback

  • ⚠️ Unnecessary memo on simple components → SearchSubmitButton, ResumeLiveTailButton, SourceEditMenu, SourceEditModal, NewSourceModal do not have expensive computations or child trees. Remove memo unless profiler shows benefit

  • Breaking change in SQLInlineEditor.tsx:415 → onClick now captured in closure without dependencies. If ref changes, the callback still references old ref. Add ref to dependency array or remove useCallback (inline is fine for onClick)

Minor

  • SetStateAction import not needed if using stable callback pattern
  • Consider consolidating the new callbacks in DBSearchPage - many could be avoided with proper stable references

Recommendation

Fix the SQLInlineEditor onClick closure bug before merge. Consider removing memo from simple components - they add overhead without benefit.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 18, 2025

E2E Test Results

All tests passed • 47 passed • 3 skipped • 692s

Status Count
✅ Passed 47
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

Copy link
Contributor

@pulpdrew pulpdrew left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit ee9f43b into main Dec 19, 2025
11 of 12 checks passed
@kodiakhq kodiakhq bot deleted the aaron/fix-lag branch December 19, 2025 17:46
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