Finish install-based package recommendation behavior#2659
Closed
jesse-merhi wants to merge 17 commits into
Closed
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Completes the remaining “install-based” ranking/recommendation work across the plugin/package/skill catalogs, including safer pagination fallbacks while recommendation-score backfills are incomplete, and aligns install/stat recording + digest synchronization so public list/search views stay consistent.
Changes:
- Extend catalog sorting/plumbing to support
recommended/installs/updatedconsistently across UI routes and HTTP/Convex endpoints, including stable cursor fallback behavior. - Record package install metrics with required viewer identity inputs and improve hard-delete cleanup of install dedupe rows.
- Keep package search digest stats in sync with package stat updates and add a maintenance backfill to patch stale digest stats.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/plugins/index.tsx | Adjusts default sort + sort persistence behavior for plugin browse/search. |
| src/components/SkillHeader.tsx | Uses migrated top-level skill stats fields when present. |
| src/components/SkillHeader.test.tsx | Adds coverage for migrated install stats selection. |
| src/tests/packages-route.test.tsx | Updates/adds tests for plugin route sort defaults and install sorting. |
| docs/http-api.md | Documents unified packages endpoint sort options. |
| docs/api.md | Documents /api/v1/packages sort parameter options. |
| convex/statsMaintenance.ts | Adds internal mutation to backfill package digest stats and a helper comparer. |
| convex/statsMaintenance.test.ts | Adds tests for the new digest-stat backfill mutation. |
| convex/skills.ts | Adds recommended sort support and recommended→updated fallback cursoring for package catalog rows. |
| convex/skills.packageCatalog.test.ts | Tests recommended sort and fallback behavior for package catalog pagination. |
| convex/schema.ts | Adds install-sorted indexes for capability/category package digest tables. |
| convex/packages.ts | Adds install-sorted digest query builders, makes install metric args required, syncs digest stats post-stat-processing, and improves install-dedupe cleanup on hard delete. |
| convex/packages.stats.test.ts | Updates tests for required install metric args and validates digest stat syncing. |
| convex/packages.public.test.ts | Adds tests for install-sorted capability/category listings and install-dedupe cleanup during hard delete. |
| convex/lib/packageSearchDigest.ts | Adds helper to patch digest stats across digest tables for a package. |
| convex/httpApiV1/packagesV1.ts | Adds recommended-fallback cursor state handling and default-sort behavior for plugin endpoints; tightens install metric recording to identity-backed installs only. |
| convex/httpApiV1.handlers.test.ts | Adds handler coverage for merged sorting behavior and recommended fallback semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+133
to
+137
| function hasPluginBrowseFilter( | ||
| args: Pick<PluginsPageDataRequest, "category" | "featured" | "official" | "executesCode">, | ||
| ) { | ||
| return Boolean(args.category || args.featured || args.official || args.executesCode); | ||
| } |
Comment on lines
+145
to
+149
| function hasPersistentPluginBrowseFilter( | ||
| args: Pick<PluginsPageDataRequest, "category" | "official" | "executesCode">, | ||
| ) { | ||
| return Boolean(args.category || args.official || args.executesCode); | ||
| } |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
Contributor
|
Codex review: needs maintainer review before merge. Reviewed June 15, 2026, 8:07 PM ET / 00:07 UTC. Summary Reproducibility: not applicable. as a maintainer-authored feature/plumbing PR; review focused on source, diff, related merged work, and validation evidence. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the branch after normal maintainer review and required checks, keeping install-based catalog behavior on the existing package, plugin, and skill APIs. Do we have a high-confidence way to reproduce the issue? Not applicable as a maintainer-authored feature/plumbing PR; review focused on source, diff, related merged work, and validation evidence. Is this the best way to solve the issue? Yes; completing the install/recommended sort plumbing in the existing catalog endpoints and digest/stat paths is the narrow maintainable path, and no supported duplicate path was found. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ab3c708cc993. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
This was referenced Jun 16, 2026
Member
Author
|
Closing this broad follow-up PR in favor of the smaller split stack:
The split PRs each have proof-pack bodies based on their own net diff. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #2633. That PR merged the first install-ranking slice; this keeps the remaining branch work that was still local afterward.
Verification
VITE_CONVEX_URL=https://example.invalid bunx vitest run convex/httpApiV1.handlers.test.ts convex/skills.packageCatalog.test.ts src/__tests__/packages-route.test.tsx convex/packages.public.test.ts convex/packages.stats.test.tspassed: 5 files, 605 testsbun run ci:unitpassed: 264 files, 3189 testsbunx convex dev --once --typecheck=disablepassedbun run ci:types-buildpassedbun run format:checkpassedbun run lintpassedbun run deadcode:cipassedStatic Gate Note
bun run ci:staticcurrently stops inbun auditbefore format/lint/dead-code becausedompurifyadvisories are reported throughmermaidandmonaco-editor. I ran the remaining static checks directly after that failure.