Performance optimizations for /skills page#54
Performance optimizations for /skills page#54thewilloftheshadow merged 5 commits intoopenclaw:mainfrom
Conversation
|
@jamwt is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
|
This doesn't fix the /http/v1/skills endpoint -- but that is called much less frequently. |
There was a problem hiding this comment.
Additional Suggestion:
Test expectation doesn't match implementation: expects second search call limit of 100 but code increments by 25 each time.
View Details
📝 Patch Details
diff --git a/src/__tests__/skills-index.test.tsx b/src/__tests__/skills-index.test.tsx
index 07f96e8..8e363da 100644
--- a/src/__tests__/skills-index.test.tsx
+++ b/src/__tests__/skills-index.test.tsx
@@ -52,7 +52,7 @@ describe('SkillsIndex', () => {
expect(usePaginatedQueryMock).toHaveBeenCalledWith(
expect.anything(),
{},
- { initialNumItems: 50 },
+ { initialNumItems: 25 },
)
})
@@ -71,7 +71,7 @@ describe('SkillsIndex', () => {
// usePaginatedQuery should be called with 'skip' when there's a search query
expect(usePaginatedQueryMock).toHaveBeenCalledWith(expect.anything(), 'skip', {
- initialNumItems: 50,
+ initialNumItems: 25,
})
await act(async () => {
await vi.runAllTimersAsync()
@@ -79,7 +79,7 @@ describe('SkillsIndex', () => {
expect(actionFn).toHaveBeenCalledWith({
query: 'remind',
highlightedOnly: false,
- limit: 50,
+ limit: 25,
})
})
@@ -88,8 +88,8 @@ describe('SkillsIndex', () => {
vi.stubGlobal('IntersectionObserver', undefined)
const actionFn = vi
.fn()
+ .mockResolvedValueOnce(makeSearchResults(25))
.mockResolvedValueOnce(makeSearchResults(50))
- .mockResolvedValueOnce(makeSearchResults(100))
useActionMock.mockReturnValue(actionFn)
vi.useFakeTimers()
@@ -107,7 +107,7 @@ describe('SkillsIndex', () => {
expect(actionFn).toHaveBeenLastCalledWith({
query: 'remind',
highlightedOnly: false,
- limit: 100,
+ limit: 50,
})
})
})
Analysis
Test expectations don't match pageSize implementation in skills search pagination
What fails: The test expectations in src/__tests__/skills-index.test.tsx were inconsistent with the implementation after commit 2288c4b changed pageSize from 50 to 25.
How to reproduce:
npm test -- src/__tests__/skills-index.test.tsxBefore the fix, the test "loads more results when search pagination is requested" would fail because:
- Code initializes
searchLimit = pageSize(25) - After load more:
setSearchLimit((value) => value + pageSize)increments by 25 (to 50) - But test expected second call with
limit: 100instead of 50 - Mock returned 50 and 100 items but limits were only 25 and 100
- Button detection logic failed because
searchResults.length !== searchLimit
What was happening: Tests expected pagination values from when pageSize = 50:
- Initial pagination:
initialNumItems: 50 - First search call:
limit: 50 - Second search call:
limit: 100
Expected behavior: With pageSize = 25 (per commit message: "update app to page only 25"):
- Initial pagination:
initialNumItems: 25 - First search call:
limit: 25 - Second search call:
limit: 50(25 + 25)
Fix: Updated test expectations and mocks in src/__tests__/skills-index.test.tsx:
- Line 54:
initialNumItems: 50→initialNumItems: 25 - Line 74:
initialNumItems: 50→initialNumItems: 25 - Line 82:
limit: 50→limit: 25 - Line 92-93:
makeSearchResults(50)andmakeSearchResults(100)→makeSearchResults(25)andmakeSearchResults(50) - Line 110:
limit: 100→limit: 50
All 286 tests now pass.
Thanks vercel bot!
|
Thanks Jamie! |
/skills page is lagging hard. Mostly because every request is ending up being a cache miss right now.
This PR makes a number of changes to optimize /skills:
this should have a dramatic impact on the server-side load and the overall performance of clawdhub