Skip to content

feat(cli): align find with AI Picks ranking - #15

Merged
cyhhao merged 2 commits into
mainfrom
feature/find-ai-picks-pagination
May 30, 2026
Merged

feat(cli): align find with AI Picks ranking#15
cyhhao merged 2 commits into
mainfrom
feature/find-ai-picks-pagination

Conversation

@cyhhao

@cyhhao cyhhao commented May 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make askill find request Registry AI Picks ordering by default via llm_score desc.
  • Return the actual sort contract in find --json and document the default ordering.
  • Preserve the latest --tag/pagination behavior while adding --page=/--limit= support and Prev/Next hints.

Validation

  • npm run build
  • npm test (27 passed)
  • JSON schema parse
  • targeted Docker E2E: test_find_json_output test_dashboard_json_contracts test_search (10 passed)
  • full Docker E2E (238 passed, 0 failed, 1 skipped)

Notes

  • Ported only the useful delta from the dirty control worktree; did not carry over stale schema/docs changes that would revert PR13/PR14 contracts.

@cyhhao

cyhhao commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a74d3d1bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/cli.ts Outdated
Comment on lines +1823 to +1827
if (query) {
parts.push(formatFindQueryArg(query));
}
if (tag) {
parts.push(`--tag ${tag}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Escape pagination command arguments

When the original query or tag contains shell metacharacters, the Prev/Next hint is not safe to copy: formatFindQueryArg leaves single-word queries unquoted, and double quotes still allow expansions such as $() in bash. For example, a search invoked with a quoted query like foo;bar or foo $(cmd) can produce a pagination command that either runs a different command or executes command substitution when pasted, so the new navigation hint should shell-escape every user-derived argument.

Useful? React with 👍 / 👎.

Comment thread src/cli.ts
tag,
page,
limit,
sort: SEARCH_DEFAULT_SORT,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Don't request an unsupported sort key

For registries that implement the documented /api/v1/skills contract, this makes every askill find request use an unsupported sort value: the checked API spec still lists sort as only stars, updated, or name (docs/api-spec.md:47). Unless the server contract is updated everywhere before this CLI ships, these searches can be rejected or silently fall back to a different order despite the JSON claiming llm_score desc, so either update the API contract/server support or avoid sending this key.

Useful? React with 👍 / 👎.

@cyhhao

cyhhao commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Pushed follow-up commit 71c02d0 addressing the two Codex P2 comments.

Fixes:

  • Shell-escape user-derived query/tag values in Prev/Next pagination hints using POSIX single-quote escaping.
  • Updated docs/api-spec.md so /api/v1/skills documents llm_score and popular_score sort keys alongside stars, updated, and name.
  • Added E2E coverage for shell-escaped tag and query pagination hints, including a $(...) query case.

Validation:

  • npm run build
  • npm test (27 passed)
  • JSON schema parse
  • targeted Docker E2E: 14 passed
  • full Docker E2E: 242 passed, 0 failed, 1 skipped

@cyhhao

cyhhao commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@cyhhao
cyhhao merged commit b1b30d9 into main May 30, 2026
1 check passed
@cyhhao
cyhhao deleted the feature/find-ai-picks-pagination branch May 30, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant