Skip to content

fix: auto-switch to relevance sort when search query is entered #778

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.

Sign up for GitHub

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

Closed
oolong-tea-2026 wants to merge 2 commits into from
Closed

fix: auto-switch to relevance sort when search query is entered #778

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4851ba2
fix: auto-switch to relevance sort when search query is entered
oolong-tea-2026 Mar 12, 2026
fdf9654
fix: preserve manual sort override during search
oolong-tea-2026 Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions src/routes/skills/-useSkillsBrowseModel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ export function useSkillsBrowseModel({

const trimmedQuery = useMemo(() => query.trim(), [query])
const hasQuery = trimmedQuery.length > 0
const sort: SortKey =
search.sort === 'relevance' && !hasQuery
? 'downloads'
: (search.sort ?? (hasQuery ? 'relevance' : 'downloads'))
const sort: SortKey = hasQuery
? (search.sort ?? 'relevance')
: (search.sort === 'relevance' ? 'downloads' : (search.sort ?? 'downloads'))
Comment on lines +51 to +53
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 12, 2026

Choose a reason for hiding this comment

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

Manual sort override silently ignored while searching

The new logic unconditionally returns 'relevance' whenever hasQuery is true, meaning the sort dropdown becomes non-functional during an active search. When a user selects 'downloads' or 'stars' from the dropdown while searching, onSortChange writes search.sort to the URL, but the computed sort always evaluates to 'relevance' regardless — the actual sort used never matches the user's selection.

The PR description's "After" column states the manual sort selection is "Respected", but the code contradicts this. The original code correctly threaded search.sort through for any manually-chosen value while a query was active; only the implicit default was broken.

To preserve the ability to manually override the sort during search, the auto-switch to relevance should only apply when search.sort has not been explicitly set by the user (i.e. when it is undefined). The beforeLoad injection of sort=downloads into the URL on cold page loads would still bypass this intent, which is a related but separate root-cause issue worth addressing alongside this fix.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/routes/skills/-useSkillsBrowseModel.ts
Line: 51-53

Comment:
**Manual sort override silently ignored while searching**

The new logic unconditionally returns `'relevance'` whenever `hasQuery` is true, meaning the sort dropdown becomes non-functional during an active search. When a user selects `'downloads'` or `'stars'` from the dropdown while searching, `onSortChange` writes `search.sort` to the URL, but the computed `sort` always evaluates to `'relevance'` regardless — the actual sort used never matches the user's selection.

The PR description's "After" column states the manual sort selection is **"Respected"**, but the code contradicts this. The original code correctly threaded `search.sort` through for any manually-chosen value while a query was active; only the implicit default was broken.

To preserve the ability to manually override the sort during search, the auto-switch to relevance should only apply when `search.sort` has not been explicitly set by the user (i.e. when it is `undefined`). The `beforeLoad` injection of `sort=downloads` into the URL on cold page loads would still bypass this intent, which is a related but separate root-cause issue worth addressing alongside this fix.

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

Copy link
Author

@oolong-tea-2026 oolong-tea-2026 Mar 12, 2026

Choose a reason for hiding this comment

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

Good catch! Fixed in fdf9654. The new approach: onQueryChange clears the URL sort param when entering a search query (so search.sort ?? 'relevance' kicks in as default), but manual dropdown selection writes to search.sort and takes precedence. This preserves user overrides while fixing the original bug where beforeLoad-injected sort=downloads persisted through search.

const listSort = toListSort(sort)
const dir = parseDir(search.dir, sort)
const searchKey = trimmedQuery
Expand Down Expand Up @@ -229,7 +228,13 @@ export function useSkillsBrowseModel({
const trimmed = next.trim()
navigateTimer.current = window.setTimeout(() => {
void navigate({
search: (prev) => ({ ...prev, q: trimmed ? next : undefined }),
search: (prev) => ({
...prev,
q: trimmed ? next : undefined,
// When entering a search query, clear any injected sort so relevance
// becomes the default. Users can still pick a different sort manually.
sort: trimmed ? undefined : prev.sort,
}),
replace: true,
})
}, 220)
Expand Down