Skip to content

Make recommended the default skill ranking#2437

Merged
jesse-merhi merged 7 commits into
mainfrom
jesse/default-skill-ranking
Jun 1, 2026
Merged

Make recommended the default skill ranking#2437
jesse-merhi merged 7 commits into
mainfrom
jesse/default-skill-ranking

Conversation

@jesse-merhi
Copy link
Copy Markdown
Member

@jesse-merhi jesse-merhi commented May 29, 2026

Summary

  • makes recommended the canonical public skill sort for the web browse and home feeds
  • keeps old default sort input as a compatibility alias
  • preserves the v1 API no-sort default as updated, while adding explicit sort=recommended / sort=default support
  • ranks recommended results by stars, all-time installs, downloads, then updated time
  • updates search popularity scoring so downloads are only a fallback signal
  • adds digest index/backfill support, docs, OpenAPI coverage, and regression tests for ranking and sort-state behavior

Tests

Passed on deb9eb73:

  • git diff --check
  • bun run format:check
  • bun run lint
  • VITE_CONVEX_URL=https://example.invalid bunx vitest run convex/skills.listPublicPageV4.test.ts convex/skills.publicListCursor.test.ts convex/httpApiV1.handlers.test.ts convex/search.test.ts convex/lib/skillSearchDigest.test.ts convex/functions.test.ts convex/maintenance.test.ts src/__tests__/skills-index.test.tsx src/__tests__/skills-route-default-sort.test.ts src/__tests__/home-route.test.tsx src/__tests__/openapi-contract.test.ts — 11 files, 399 tests
  • bunx tsc -p packages/schema/tsconfig.json --noEmit
  • bunx tsc -p packages/clawhub/tsconfig.json --noEmit
  • bun run --cwd packages/clawhub-mod typecheck

Attempted locally but still blocked by the existing public-corpus dependency issue:

  • bunx tsc --noEmit now only fails because scripts/public-corpus/dummyOwners.ts cannot resolve @faker-js/faker in this local install.

Review

  • Branch review loop: 3 consecutive clean passes on deb9eb73.
  • Independent review passes: 3 clean / mergeable verdicts on deb9eb73.

Prod-data preview

  • Read skillSearchDigest from prod deployment wry-manatee-359 using read-only CLI access.
  • Built a local snapshot preview for the actual /skills page with the PR ranking formula.
  • Verified /skills?prodSnapshot=1 in Playwright; top rows were:
    1. @pskoett/Self-Improving Agent
    2. @ivangdavila/Self-Improving + Proactive Agent
    3. @spclaudehome/Skill Vetter
    4. @steipete/Gog
    5. @halthelobster/Proactive Agent

Notes

  • Convex prod export/import for a full cloned dev deployment is blocked by missing backup/export permission (CreateBackups).
  • convex logs --failure is not supported by the installed Convex CLI.
  • Local UI screenshots were captured for /skills mobile and desktop after confirming Recommended defaults and search switches to Relevance.

Copilot AI review requested due to automatic review settings May 29, 2026 05:41
@jesse-merhi jesse-merhi requested review from a team and Patrick-Erichsen as code owners May 29, 2026 05:41
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 29, 2026

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

Project Deployment Actions Updated (UTC)
clawhub Ready Ready Preview, Comment Jun 1, 2026 8:22pm

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 29, 2026

Codex review: needs maintainer review before merge. Reviewed June 1, 2026, 5:58 PM ET / 21:58 UTC.

Summary
The PR makes Recommended the default web/home skill ranking, adds explicit recommended/default API sort handling, updates digest ranking indexes/backfill/search scoring, and covers the behavior with docs, OpenAPI, and regression tests.

Reproducibility: not applicable. this is a feature/default-ranking PR, not a bug report. Source comparison shows current main still defaults web browse/home to downloads/newest-oriented behavior while the branch adds Recommended.

Review metrics: 3 noteworthy metrics.

  • Diff surface: 25 files changed. The change spans Convex schema/API/search, web routes, docs, OpenAPI, and tests, so rollout review should consider the full discovery surface.
  • Recommended-rank indexes: 2 added. The new active and non-suspicious rank indexes make Convex deploy/backfill timing relevant before merge.
  • Targeted validation reported: 11 test files, 399 tests. The PR body reports focused coverage for ranking, cursors, API aliases, OpenAPI, and route state behavior.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Merging intentionally changes existing no-sort web browse and home-feed behavior from downloads/newest-oriented ordering to Recommended ranking, so maintainers need to accept that user-visible compatibility change.
  • [P2] Recommended ordering depends on Convex schema deployment plus digest upsert/backfill population of rank stats; the fallback reduces missing-stat risk, but production rollout timing still matters.
  • [P1] The PR body reports full repo-wide bunx tsc --noEmit is still blocked by an existing missing @faker-js/faker public-corpus dependency, so repo-wide type coverage is not fully evidenced here.

Maintainer options:

  1. Approve the default-ranking rollout
    Maintainers can accept the user-visible browse/home ordering change after confirming Recommended should replace downloads as the default discovery path.
  2. Gate on deploy and backfill timing
    Maintainers can hold merge until the Convex schema deployment and digest backfill/upsert path are ready to populate rank stats before judging the default order stable.
  3. Keep downloads as the default
    Maintainers can pause or close this PR if the product decision is to keep current browse/home ordering for existing users.

Next step before merge

  • [P2] Human maintainer approval is required because the PR intentionally changes public browse/home defaults and the author is a repo member; there is no narrow automated repair to queue.

Security
Cleared: No concrete security or supply-chain concern was found; the diff does not add dependencies, workflows, secret handling, or downloaded execution paths.

Review details

Best possible solution:

Land only after maintainers explicitly approve Recommended as the public default and the production schema/backfill rollout plan, while preserving the v1 API no-sort updated behavior.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a feature/default-ranking PR, not a bug report. Source comparison shows current main still defaults web browse/home to downloads/newest-oriented behavior while the branch adds Recommended.

Is this the best way to solve the issue?

Unclear as a product decision: the implementation is narrow and well-covered for the chosen direction, but whether this is the best solution depends on maintainers accepting the compatibility and rollout impact.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against cb6ced7906f7.

Label changes

Label justifications:

  • P2: This is a normal-priority product/API improvement with limited blast radius but a visible public discovery default change.
  • merge-risk: 🚨 compatibility: The PR changes existing default browse/home ordering for users who do not specify a sort, which green CI cannot decide as a product-compatible rollout.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-change live-output proof from a production-data /skills snapshot preview plus Playwright verification and captured UI screenshots.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-change live-output proof from a production-data /skills snapshot preview plus Playwright verification and captured UI screenshots.
Evidence reviewed

What I checked:

  • Repository policy read: Read the full target AGENTS.md and applied the Convex query/index, stat migration, proof, and PR validation guidance to this review. (AGENTS.md:1, cb6ced7906f7)
  • Current main web default is downloads: Current main redirects missing /skills browse sort to downloads, so the requested default change is not already implemented on main. (src/routes/skills/index.tsx:61, cb6ced7906f7)
  • Current main browse model uses downloads: Current main resolves browse-without-query to downloads and passes that list sort to listPublicPageV4. (src/routes/skills/-useSkillsBrowseModel.ts:100, cb6ced7906f7)
  • PR changes backend default to recommended: The PR normalizes an omitted or default public-list sort to recommended and forces recommended direction to descending. (convex/skills.ts:5788, deb9eb739e08)
  • PR adds rank indexes: The PR adds active and non-suspicious recommended rank indexes over stars, all-time installs, downloads, and updated time. (convex/schema.ts:916, deb9eb739e08)
  • Stat migration policy followed: Digest extraction reads migrated stats through readCanonicalStat, matching AGENTS.md stat-field migration guidance. (convex/lib/skillSearchDigest.ts:65, deb9eb739e08)

Likely related people:

  • Patrick Erichsen: Blame and log show this person introduced the current listPublicPageV4, web browse default, digest helper, and search-ranking code paths in commit 1f56a71. (role: recent area contributor; confidence: high; commits: 1f56a7143043; files: convex/skills.ts, src/routes/skills/index.tsx, src/routes/skills/-useSkillsBrowseModel.ts)
  • Vyctor H. Brzezowski: Recent public-list and digest hydration work in commit 97023d3 touched the same list/digest output path this PR changes. (role: recent adjacent contributor; confidence: medium; commits: 97023d3123f4; files: convex/skills.ts, convex/lib/skillSearchDigest.ts)
  • jesse-merhi: This person has prior merged history on publisher abuse scoring and skill stat aggregation in commit 8a2c0c0, which is adjacent to the ranking/stat-signal surface changed here. (role: adjacent stats contributor; confidence: medium; commits: 8a2c0c06fd17; files: convex/publisherAbuse.ts, convex/schema.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes “Recommended” (a composite popularity-based rank) the default public skill ranking across the Skills browse UI, the home feed, and the /api/v1/skills HTTP API, while preserving consistent behavior for search (“Relevance”) and for legacy/stale sort URLs.

Changes:

  • Updates Skills browse and home feed to default to Recommended, including sort-option UX and sort-state normalization.
  • Adds backend “default” ranking support via new digest indexes and server-side fallback logic to keep pagination cursors stable during backfill.
  • Adjusts search popularity scoring to weight stars/installs more than downloads, and adds regression tests + docs updates.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/routes/skills/index.tsx Updates sort options and removes the old browse-default redirect to allow Recommended to be the implicit default.
src/routes/skills/-useSkillsBrowseModel.ts Normalizes sort/dir semantics for default vs relevance and omits sort in list requests when using implicit Recommended.
src/routes/skills/-params.ts Introduces default sort key and updates mapping logic so default/relevance don’t map to a list sort parameter.
src/routes/index.tsx Home feed now relies on backend default ranking instead of explicitly forcing downloads.
src/tests/skills-route-default-sort.test.ts Updates route-default behavior expectations now that no redirect is used.
src/tests/skills-index.test.tsx Adds regression coverage for default-sort behavior, stale sort/dir cleanup, and explicit browse/search sort transitions.
src/tests/home-route.test.tsx Updates expectations for home feed queries to use Recommended by default (no explicit sort).
docs/http-api.md Documents default/recommended sort behavior and updated popularity/relevance notes.
convex/skills.ts Adds default-rank index support, default sort handling for list endpoints, highlighted sorting changes, and cursor/backfill compatibility logic.
convex/skills.listPublicPageV4.test.ts Adds tests for default-rank indexes, direction normalization, cursor compatibility, and highlighted default sorting.
convex/search.ts Revises popularity weighting/capping and tie-breaking to downweight downloads and emphasize stars/installs.
convex/search.test.ts Updates and expands search scoring tests to reflect the new popularity model and tie-breaking.
convex/schema.ts Adds new skillSearchDigest indexes for default ranking (active + nonsuspicious).
convex/maintenance.ts Backfill now upserts digests (vs insert-only) to populate new rank stats/index fields.
convex/lib/skillSearchDigest.ts Ensures digest rank stats are populated from canonical stats to support default ranking indexes.
convex/lib/skillSearchDigest.test.ts Adds coverage for filling digest rank stats from legacy nested stats fields.
convex/httpApiV1/skillsV1.ts Makes API v1 list sort default to default/recommended and supports recommended alias.
convex/httpApiV1.handlers.test.ts Adds coverage asserting /api/v1/skills defaults to recommended and that aliases map correctly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 29, 2026
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 29, 2026
@jesse-merhi jesse-merhi force-pushed the jesse/default-skill-ranking branch from 2bf9a52 to 753a27c Compare June 1, 2026 15:12
@clawsweeper clawsweeper Bot added the proof: sufficient Contributor real behavior proof is sufficient. label Jun 1, 2026
@jesse-merhi jesse-merhi force-pushed the jesse/default-skill-ranking branch from 753a27c to 1164829 Compare June 1, 2026 15:18
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 1, 2026
@jesse-merhi jesse-merhi force-pushed the jesse/default-skill-ranking branch from 1164829 to ad0ffec Compare June 1, 2026 15:27
@jesse-merhi
Copy link
Copy Markdown
Member Author

@clawsweeper re-review

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented Jun 1, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot removed the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 1, 2026
@clawsweeper clawsweeper Bot removed the proof: sufficient Contributor real behavior proof is sufficient. label Jun 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient Contributor real behavior proof is sufficient. label Jun 1, 2026
@jesse-merhi jesse-merhi merged commit 01aa28c into main Jun 1, 2026
21 checks passed
@jesse-merhi jesse-merhi deleted the jesse/default-skill-ranking branch June 1, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal backlog priority with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants