fix: default skills search to relevance

[Wangnov]

Summary

• clear the implicit browse downloads sort when a user starts typing a query on /skills
• keep explicit user-selected sorts intact so only the inherited default is reset
• add a regression test for the browse-to-search transition

Show the bug

bug_report_clawhub_01_compressed.mp4

Testing

• bun run test src/tests/skills-index.test.tsx src/tests/skills-route-default-sort.test.ts
• bun run lint:oxlint
• git diff --check

UI

• No visual layout change; this fixes result ordering when the Skills page search box inherits the browse default sort.

[vercel]

Someone is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

This PR fixes a result-ordering bug on the /skills page where the browse-mode default sort (sort=downloads, injected by the beforeLoad redirect) would persist into search mode instead of resetting to relevance when a user started typing.

The fix is clean and well-reasoned. Key observations:

• Correct discrimination between implicit default and explicit user choice: the beforeLoad hook always redirects to ?sort=downloads without a dir param. onSortChange conversely always writes an explicit dir via parseDir. The new usesImplicitBrowseDefault predicate (prev.sort === 'downloads' && prev.dir === undefined) exploits this invariant correctly — no existing user-selected sort can satisfy it because the sort picker always emits a dir.
• Correct entry-guard with enteringSearch: the reset is constrained to the first keystroke from an empty query, so users who are already searching and adjust their query do not have their sort unexpectedly cleared.
• null spread is valid: ...(condition ? { ... } : null) is safe JS/TS and results in a no-op spread when the condition is false.
• Round-trip correctness: after the fix clears sort in the URL, clearing the query causes beforeLoad to restore sort=downloads — the full browse-to-search-to-browse cycle is consistent.
• Minor test coverage gap: the regression test covers the positive path (sort is reset) but not the negative path (explicit dir prevents the reset). Suggested improvement in an inline comment.

Confidence Score: 4/5

• Safe to merge; the fix is logically correct and the only gap is a missing negative-path test case.
• The core logic is sound: it correctly uses the absence of an explicit dir param to distinguish the system-injected browse default from any user-selected sort, and gates the reset on the first entry into search mode. No null-dereference risks, no data loss paths, no security concerns. Score is 4 rather than 5 only because the usesImplicitBrowseDefault guard condition (dir === undefined) has no test covering the case where the condition is false — a regression there would go undetected.
• No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/__tests__/skills-index.test.tsx
Line: 181-204

Comment:
**Consider adding a negative-path test for the guard condition**

The new test only exercises the case where the sort _is_ reset (implicit browse default). The key guard — `prev.dir === undefined` — that prevents resetting an explicitly-chosen `downloads` sort has no corresponding test. If that condition were accidentally removed, existing tests would still pass.

Adding a sibling test like the following would fully validate the invariant:

```ts
it('preserves explicitly user-set downloads sort when entering search', async () => {
  searchMock = { sort: 'downloads', dir: 'desc' }
  vi.useFakeTimers()

  render(<SkillsIndex />)

  const input = screen.getByPlaceholderText('Filter by name, slug, or summary…')
  await act(async () => {
    fireEvent.change(input, { target: { value: 'cli-design-framework' } })
    await vi.runAllTimersAsync()
  })

  expect(navigateMock).toHaveBeenCalled()
  const lastCall = navigateMock.mock.calls.at(-1)?.[0] as {
    replace?: boolean
    search: (prev: Record<string, unknown>) => Record<string, unknown>
  }
  // dir is explicitly set, so sort must not be cleared
  expect(lastCall.search({ sort: 'downloads', dir: 'desc' })).toEqual({
    q: 'cli-design-framework',
    sort: 'downloads',
    dir: 'desc',
  })
})
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 8cbca18}

[Wangnov]

Just adding some context relative to #778 in case it helps with triage:

I took a look at the follow-up there, and it does address the earlier concern about manual sort overrides during search.

The remaining difference in this PR is mostly about scope:

• it only clears the inherited browse default when first entering search from an empty query
• it preserves explicitly chosen sorts across later query edits
• it includes regression coverage for both the reset path and the preserve path

If maintainers would prefer to consolidate on #778, I’d be very happy for this narrower guard behavior and the accompanying test coverage to be folded into that PR instead.

[oolong-tea-2026]

Thanks for the context! We intentionally chose to reset sort on every query change rather than only on the first entry — our reasoning is that when a user is actively typing/editing a search query, their primary intent is finding the most relevant result, so relevance sort should be the consistent default. Users can always re-select a different sort after their query settles.

That said, your test coverage is great and would be a nice addition either way. Happy to coordinate with maintainers on the best path forward.