fix(search): keep skills all tab on approved recommended ranking

Stop default recommended browse from falling back to updated ordering when
scores are missing, and exclude pending-review items from public browse/search
while preserving the last approved version for established skills.
This commit is contained in:
vyctorbrzezowski
2026-06-25 11:38:04 -07:00
committed by Patrick Erichsen
parent aa82eea8d3
commit 7fa17e159e
8 changed files with 279 additions and 81 deletions
+106
View File
@@ -0,0 +1,106 @@
/* @vitest-environment node */
import { describe, expect, it } from "vitest";
import {
hasPriorApprovedPublicSkillVersion,
isPubliclyListableSkillVersion,
isSkillPendingPublicReview,
shouldExcludeSkillFromPublicBrowse,
} from "./publicBrowse";
describe("publicBrowse", () => {
it("treats scanner review flags as pending public review", () => {
expect(
isSkillPendingPublicReview({
moderationStatus: "active",
moderationReason: "scanner.llm.review",
moderationFlags: ["flagged.review"],
}),
).toBe(true);
});
it("treats active pending.scan skills as pending public review", () => {
expect(
isSkillPendingPublicReview({
moderationStatus: "active",
moderationReason: "pending.scan",
moderationFlags: undefined,
}),
).toBe(true);
});
it("excludes first-publish pending review skills from public browse", () => {
expect(
shouldExcludeSkillFromPublicBrowse({
softDeletedAt: undefined,
moderationStatus: "active",
moderationReason: "pending.scan",
moderationFlags: undefined,
moderationVerdict: "clean",
moderationSourceVersionId: undefined,
latestVersionId: "skillVersions:1",
githubScanStatus: "clean",
stats: {
versions: 1,
downloads: 0,
stars: 0,
installsCurrent: 0,
installsAllTime: 0,
comments: 0,
},
}),
).toBe(true);
});
it("keeps previously approved skills visible while a newer version is pending review", () => {
expect(
hasPriorApprovedPublicSkillVersion({
stats: {
versions: 2,
downloads: 0,
stars: 0,
installsCurrent: 0,
installsAllTime: 0,
comments: 0,
},
}),
).toBe(true);
expect(
shouldExcludeSkillFromPublicBrowse({
softDeletedAt: undefined,
moderationStatus: "active",
moderationReason: "pending.scan",
moderationFlags: undefined,
moderationVerdict: "clean",
moderationSourceVersionId: "skillVersions:2",
latestVersionId: "skillVersions:2",
githubScanStatus: "clean",
stats: {
versions: 2,
downloads: 0,
stars: 0,
installsCurrent: 0,
installsAllTime: 0,
comments: 0,
},
}),
).toBe(false);
});
it("rejects pending-review skill versions from public listing", () => {
expect(
isPubliclyListableSkillVersion({
_id: "skillVersions:pending",
skillId: "skills:1",
softDeletedAt: undefined,
version: "2.0.0",
createdAt: 1,
changelog: "c",
changelogSource: "user",
parsed: { frontmatter: {}, license: "MIT" },
vtAnalysis: { status: "pending", checkedAt: 1 },
llmAnalysis: undefined,
staticScan: undefined,
}),
).toBe(false);
});
});
+111
View File
@@ -0,0 +1,111 @@
import type { Doc, Id } from "../_generated/dataModel";
import type { MutationCtx, QueryCtx } from "../_generated/server";
import { isPublicSkillDoc } from "./globalStats";
import {
isSecurityScanStatusBlockedFromPublic,
normalizeSecurityScanStatus,
} from "./securityScanPolicy";
import { isSkillReviewFlagged, isSkillSuspicious } from "./skillSafety";
type SkillPublicBrowseFields = Pick<
Doc<"skills">,
| "softDeletedAt"
| "moderationStatus"
| "moderationReason"
| "moderationFlags"
| "moderationVerdict"
| "moderationSourceVersionId"
| "latestVersionId"
| "githubScanStatus"
| "stats"
>;
type SkillVersionPublicBrowseFields = Pick<
Doc<"skillVersions">,
| "_id"
| "skillId"
| "softDeletedAt"
| "version"
| "createdAt"
| "changelog"
| "changelogSource"
| "parsed"
| "vtAnalysis"
| "llmAnalysis"
| "staticScan"
>;
function isPendingSkillModerationReason(reason: string | null | undefined) {
const normalized = reason?.trim().toLowerCase();
return (
normalized === "pending.scan" ||
normalized === "pending.scan.stale" ||
normalized === "scanner.vt.pending" ||
normalized === "scanner.llm.pending"
);
}
export function isSkillPendingPublicReview(
skill: Pick<Doc<"skills">, "moderationStatus" | "moderationReason" | "moderationFlags">,
) {
if (isSkillReviewFlagged(skill)) return true;
return isPendingSkillModerationReason(skill.moderationReason);
}
export function hasPriorApprovedPublicSkillVersion(skill: Pick<Doc<"skills">, "stats">) {
return (skill.stats?.versions ?? 0) > 1;
}
export function shouldExcludeSkillFromPublicBrowse(skill: SkillPublicBrowseFields) {
if (!isPublicSkillDoc(skill)) return true;
if (isSkillSuspicious(skill)) return true;
if (normalizeSecurityScanStatus(skill.githubScanStatus) === "pending") return true;
if (!isSkillPendingPublicReview(skill)) return false;
return !hasPriorApprovedPublicSkillVersion(skill);
}
export function isPubliclyListableSkillVersion(
version: SkillVersionPublicBrowseFields | null | undefined,
) {
if (!version || version.softDeletedAt) return false;
const statuses = [
normalizeSecurityScanStatus(version.vtAnalysis?.status),
normalizeSecurityScanStatus(version.llmAnalysis?.verdict ?? version.llmAnalysis?.status),
normalizeSecurityScanStatus(version.staticScan?.status),
];
if (statuses.some((status) => status === "pending" || status === "not-run")) return false;
return !statuses.some((status) => isSecurityScanStatusBlockedFromPublic(status));
}
export async function resolvePublicBrowseVersionForSkill(
ctx: Pick<QueryCtx | MutationCtx, "db">,
skill: SkillPublicBrowseFields & { _id: Id<"skills"> },
): Promise<Doc<"skillVersions"> | null> {
const latestVersionId = skill.latestVersionId;
if (!latestVersionId) return null;
if (!isSkillPendingPublicReview(skill)) {
const latestVersion = await ctx.db.get(latestVersionId);
return latestVersion &&
latestVersion.skillId === skill._id &&
isPubliclyListableSkillVersion(latestVersion)
? latestVersion
: null;
}
if (!hasPriorApprovedPublicSkillVersion(skill)) return null;
const versions = await ctx.db
.query("skillVersions")
.withIndex("by_skill_active_created", (q) =>
q.eq("skillId", skill._id).eq("softDeletedAt", undefined),
)
.order("desc")
.take(24);
for (const version of versions) {
if (version._id === skill.moderationSourceVersionId) continue;
if (isPubliclyListableSkillVersion(version)) return version;
}
return null;
}
+2 -2
View File
@@ -708,7 +708,7 @@ describe("search helpers", () => {
);
});
it("preserves suspicious lexical fallback results when nonSuspiciousOnly is unset", async () => {
it("excludes suspicious lexical fallback results from public search", async () => {
const clean = makeSkillDoc({ id: "skills:clean", slug: "orf-clean", displayName: "ORF Clean" });
const suspicious = makeSkillDoc({
id: "skills:suspicious",
@@ -727,7 +727,7 @@ describe("search helpers", () => {
limit: 10,
});
expect(result.map((entry) => entry.skill.slug)).toEqual(["orf-clean", "orf-suspicious"]);
expect(result.map((entry) => entry.skill.slug)).toEqual(["orf-clean"]);
expect(ctx.usedIndexes).toEqual(
expect.arrayContaining(["by_active_updated", "by_active_created"]),
);
+13 -5
View File
@@ -16,6 +16,7 @@ import { generateEmbedding } from "./lib/embeddings";
import { hasOfficialPublisherRow, toPublicPublisherWithOfficial } from "./lib/officialPublishers";
import type { HydratableSkill, PublicPublisher } from "./lib/public";
import { toPublicSkill } from "./lib/public";
import { shouldExcludeSkillFromPublicBrowse } from "./lib/publicBrowse";
import { getOwnerPublisher } from "./lib/publishers";
import {
matchesAllTokens,
@@ -270,6 +271,7 @@ function matchesCatalogFilters(
}
function toPublicSearchSkill(skill: HydratableSkill) {
if (shouldExcludeSkillFromPublicBrowse(skill)) return null;
return toPublicSkill({
...skill,
categories: resolveStoredSkillCategories(skill),
@@ -576,10 +578,15 @@ export const directPrefixSkillMatches = internalQuery({
...(digest.categories ?? []),
...(digest.topics ?? []),
]);
const matchesDirectRecallFilters = (digest: Doc<"skillSearchDigest">) =>
(!args.highlightedOnly || isSkillHighlighted(digestToHydratableSkill(digest))) &&
passesAllQueryTokens(digest) &&
matchesCatalogFilters(digest, categorySlug, topic);
const matchesDirectRecallFilters = (digest: Doc<"skillSearchDigest">) => {
const skill = digestToHydratableSkill(digest);
return (
!shouldExcludeSkillFromPublicBrowse(skill) &&
(!args.highlightedOnly || isSkillHighlighted(skill)) &&
passesAllQueryTokens(digest) &&
matchesCatalogFilters(skill, categorySlug, topic)
);
};
const needsExpandedRecall = Boolean(
categorySlug || topic || args.highlightedOnly || queryTokens.length > 1,
);
@@ -950,7 +957,7 @@ export const lexicalFallbackSkills = internalQuery({
.take(MAX_EXACT_SLUG_MATCHES);
for (const exactSlugSkill of exactSlugSkills) {
if (
!exactSlugSkill.softDeletedAt &&
!shouldExcludeSkillFromPublicBrowse(exactSlugSkill) &&
(!args.nonSuspiciousOnly || !isSkillSuspicious(exactSlugSkill)) &&
(!args.excludePendingScan || exactSlugSkill.githubScanStatus !== "pending") &&
matchesCatalogFilters(exactSlugSkill, categorySlug, topic)
@@ -994,6 +1001,7 @@ export const lexicalFallbackSkills = internalQuery({
const matchesFallbackRecallFilters = (digest: Doc<"skillSearchDigest">) => {
const skill = digestToHydratableSkill(digest);
return (
!shouldExcludeSkillFromPublicBrowse(skill) &&
(!args.highlightedOnly || isSkillHighlighted(skill)) &&
(!args.excludePendingScan || skill.githubScanStatus !== "pending") &&
matchesCatalogFilters(skill, categorySlug, topic) &&
+3 -3
View File
@@ -87,7 +87,7 @@ describe("skills.listPublicPageV4", () => {
});
});
it("falls back to updated results while recommendation scores are missing", () => {
it("falls back to the recommended rank index while recommendation scores are missing", () => {
expect(
__test.resolveRecommendedPublicListQuery({
scoreIndexName: "by_active_recommended_score",
@@ -99,8 +99,8 @@ describe("skills.listPublicPageV4", () => {
hasMissingScores: true,
}),
).toEqual({
sort: "updated",
indexName: "by_active_updated",
sort: "recommended",
indexName: "by_active_recommended_rank",
decodedCursor: null,
});
});
+14 -23
View File
@@ -428,7 +428,7 @@ describe("skills package catalog queries", () => {
expect(indexNames).not.toContain("by_active_topic_updated");
});
it("falls topic recommendation sorting back to downloads while scores are missing", async () => {
it("keeps topic recommended sorting on the recommendation score index while scores are missing", async () => {
const indexNames: string[] = [];
const calendarSkill = makeDigest("calendar-skill", { topics: ["calendar"] });
@@ -444,7 +444,7 @@ describe("skills package catalog queries", () => {
},
],
isDone: false,
continueCursor: "downloads-next",
continueCursor: "recommended-next",
},
],
[calendarSkill],
@@ -459,9 +459,9 @@ describe("skills package catalog queries", () => {
);
expect(result.page.map((entry) => entry.name)).toEqual(["calendar-skill"]);
expect(indexNames).toContain("by_active_topic_downloads");
expect(indexNames).not.toContain("by_active_topic_recommended_score");
expect(result.continueCursor).toContain('"recommendedFallback":"downloads"');
expect(indexNames).toContain("by_active_topic_recommended_score");
expect(indexNames).not.toContain("by_active_topic_downloads");
expect(result.continueCursor).not.toContain('"recommendedFallback":"downloads"');
});
it("keeps legacy topic recommendation fallback cursors on the updated index", async () => {
@@ -606,12 +606,7 @@ describe("skills package catalog queries", () => {
},
);
expect(indexNames).toEqual([
"by_active_recommended_score",
"by_active_recommended_score_version",
"by_active_recommended_score_version",
"by_active_recommended_score",
]);
expect(indexNames).toEqual(["by_active_recommended_score"]);
expect(result.page).toEqual([
expect.objectContaining({
name: "recommended-skill",
@@ -620,7 +615,7 @@ describe("skills package catalog queries", () => {
]);
});
it("falls back to downloads sort for recommended package catalog rows while scores backfill", async () => {
it("keeps recommended package catalog rows on the recommendation score index while scores backfill", async () => {
const indexNames: string[] = [];
const result = await listPackageCatalogPageHandler(
makeCtx(
@@ -628,7 +623,7 @@ describe("skills package catalog queries", () => {
{
page: [makeDigest("fallback-skill")],
isDone: false,
continueCursor: "next-updated-page",
continueCursor: "next-recommended-page",
},
],
{
@@ -644,13 +639,9 @@ describe("skills package catalog queries", () => {
},
);
expect(indexNames).toEqual([
"by_active_recommended_score",
"by_active_recommended_score_version",
"by_active_stats_downloads",
]);
expect(indexNames).toEqual(["by_active_recommended_score"]);
expect(result.page).toEqual([expect.objectContaining({ name: "fallback-skill" })]);
expect(result.continueCursor).toContain('"recommendedFallback":"downloads"');
expect(result.continueCursor).not.toContain('"recommendedFallback":"downloads"');
});
it("uses the recommended score index for recommended package catalog rows", async () => {
@@ -681,7 +672,7 @@ describe("skills package catalog queries", () => {
expect(result.page).toEqual([expect.objectContaining({ name: "recommended-skill" })]);
});
it("falls recommended package catalog rows back to downloads when scores are missing", async () => {
it("keeps recommended package catalog rows on the recommendation score index when scores are missing", async () => {
const indexNames: string[] = [];
const result = await listPackageCatalogPageHandler(
makeCtx(
@@ -689,7 +680,7 @@ describe("skills package catalog queries", () => {
{
page: [makeDigest("download-fallback-skill")],
isDone: false,
continueCursor: "updated-next",
continueCursor: "recommended-next",
},
],
{ indexNames, missingRecommendedScores: true },
@@ -700,9 +691,9 @@ describe("skills package catalog queries", () => {
},
);
expect(indexNames).toEqual(["by_active_recommended_score", "by_active_stats_downloads"]);
expect(indexNames).toEqual(["by_active_recommended_score"]);
expect(result.page).toEqual([expect.objectContaining({ name: "download-fallback-skill" })]);
expect(result.continueCursor).toContain('"recommendedFallback":"downloads"');
expect(result.continueCursor).not.toContain('"recommendedFallback":"downloads"');
});
it("keeps recommended package catalog cursors on their original index", async () => {
+7 -7
View File
@@ -382,7 +382,7 @@ describe("public skill list deterministic cursors", () => {
});
});
it("keeps topic recommendation fallback cursors on the updated index", async () => {
it("keeps topic recommended browse on the recommended score index when scores are missing", async () => {
const firstDigest = makeSearchDigest({
skillId: "skills:calendar-one",
slug: "calendar-one",
@@ -452,12 +452,12 @@ describe("public skill list deterministic cursors", () => {
expect(first.nextCursor).not.toBeNull();
expect(second.nextCursor).toBeNull();
expect(getPageMock.mock.calls[0]?.[1]).toMatchObject({
index: "by_active_topic_updated",
index: "by_active_topic_recommended_score",
startIndexKey: [undefined, "calendar"],
startInclusive: true,
});
expect(getPageMock.mock.calls[1]?.[1]).toMatchObject({
index: "by_active_topic_updated",
index: "by_active_topic_recommended_score",
startIndexKey: firstIndexKey,
startInclusive: false,
});
@@ -543,7 +543,7 @@ describe("public skill list deterministic cursors", () => {
});
});
it("falls back to the updated index while recommendation scores are missing", async () => {
it("falls back to the recommended rank index while recommendation scores are missing", async () => {
const { ctx, withIndex } = makeMissingRecommendedScoresCtx();
await listPublicPageV4Handler(ctx, {
@@ -555,14 +555,14 @@ describe("public skill list deterministic cursors", () => {
]);
expect(getPageMock).toHaveBeenCalledTimes(1);
expect(getPageMock.mock.calls[0]?.[1]).toMatchObject({
index: "by_active_updated",
index: "by_active_recommended_rank",
startIndexKey: [undefined],
endIndexKey: [undefined],
startInclusive: true,
});
});
it("falls back to the non-suspicious updated index while recommendation scores are missing", async () => {
it("falls back to the non-suspicious recommended rank index while recommendation scores are missing", async () => {
const { ctx, withIndex } = makeMissingRecommendedScoresCtx();
await listPublicApiPageV1Handler(ctx, {
@@ -576,7 +576,7 @@ describe("public skill list deterministic cursors", () => {
]);
expect(getPageMock).toHaveBeenCalledTimes(1);
expect(getPageMock.mock.calls[0]?.[1]).toMatchObject({
index: "by_nonsuspicious_updated",
index: "by_nonsuspicious_recommended_rank",
startIndexKey: [undefined, false],
endIndexKey: [undefined, false],
startInclusive: true,
+23 -41
View File
@@ -87,6 +87,12 @@ import {
toPublicSkill,
toPublicUser,
} from "./lib/public";
import {
hasPriorApprovedPublicSkillVersion,
isSkillPendingPublicReview,
resolvePublicBrowseVersionForSkill,
shouldExcludeSkillFromPublicBrowse,
} from "./lib/publicBrowse";
import {
assertCanManageOwnedResource,
canAccessPublisherOwnerScope,
@@ -5423,16 +5429,10 @@ export const listPublicPageV4 = query({
}
if (officialFirstCursor && categorySlug) {
const sort =
officialFirstCursor.sort ??
(requestedSort === "recommended" &&
(await hasMissingRecommendedScores(ctx, args.nonSuspiciousOnly ?? false, null))
? "updated"
: requestedSort);
return await listOfficialFirstSkillCategoryPage(ctx, {
state: { ...officialFirstCursor, sort },
sort,
dir: resolvePublicListDir(sort, args.dir),
state: { ...officialFirstCursor, sort: officialFirstCursor.sort ?? requestedSort },
sort: officialFirstCursor.sort ?? requestedSort,
dir: resolvePublicListDir(officialFirstCursor.sort ?? requestedSort, args.dir),
numItems,
topic,
categorySlug,
@@ -5713,20 +5713,9 @@ async function listSkillTopicFilteredPage(
allowLegacyArray: false,
});
const recommendedCursor = opts.sort === "recommended" ? decodeTopicCursor("recommended") : null;
const updatedCursor = opts.sort === "recommended" ? decodeTopicCursor("updated") : null;
const useUpdatedRecommendationFallback =
opts.sort === "recommended" &&
(Boolean(updatedCursor) ||
(!recommendedCursor &&
(await hasMissingRecommendedScores(ctx, opts.nonSuspiciousOnly, null))));
const sort = useUpdatedRecommendationFallback ? "updated" : opts.sort;
const sort = opts.sort;
const indexName = getTopicIndexName(sort);
const decodedCursor =
opts.sort === "recommended"
? sort === "updated"
? updatedCursor
: recommendedCursor
: decodeTopicCursor(sort);
const decodedCursor = opts.sort === "recommended" ? recommendedCursor : decodeTopicCursor(sort);
const items: PublicSkillEntry[] = [];
let scanCursor = decodedCursor ?? eqPrefix;
let scanInclusive = !decodedCursor;
@@ -5986,6 +5975,7 @@ async function buildPublicSkillEntryFromDigest(
digest: Doc<"skillSearchDigest">,
): Promise<PublicSkillEntry | null> {
const hydratable = digestToHydratableSkill(digest);
if (shouldExcludeSkillFromPublicBrowse(hydratable)) return null;
const publicSkill = toPublicSkill(hydratable);
if (!publicSkill) return null;
const ownerInfo = await addOfficialStatusToOwnerInfo(
@@ -6021,6 +6011,13 @@ async function loadPublicLatestVersionForDigest(
if (digest.latestVersionSkillId !== undefined && digest.latestVersionSkillId !== digest.skillId) {
return null;
}
const skill = await ctx.db.get(digest.skillId);
if (skill && isSkillPendingPublicReview(skill) && hasPriorApprovedPublicSkillVersion(skill)) {
const version = await resolvePublicBrowseVersionForSkill(ctx, skill);
return version && isPublicSkillVersionAvailableForSkill(version, digest.skillId)
? version
: null;
}
const version = await ctx.db.get(digest.latestVersionId);
return isPublicSkillVersionAvailableForSkill(version, digest.skillId) ? version : null;
}
@@ -6306,6 +6303,7 @@ function skillCatalogMatchesFilters(
},
) {
if (!isVisibleSkillCatalogDigest(digest)) return false;
if (shouldExcludeSkillFromPublicBrowse(digestToHydratableSkill(digest))) return false;
if (args.channel === "private") return false;
const isOfficial = isSkillCatalogOfficial(digest);
const channel = getSkillCatalogChannel(digest);
@@ -6366,15 +6364,7 @@ async function listSkillPackageCatalogTopicPage(
let done = decodedCursor.done;
let loops = 0;
let remainingScanBudget = MAX_SKILL_CATALOG_SCAN_DOCUMENTS;
const isFreshRecommendedRequest =
args.sort === "recommended" && args.paginationOpts.cursor === null;
const recommendedFallback =
args.sort === "recommended"
? (decodedCursor.recommendedFallback ??
(isFreshRecommendedRequest && (await hasMissingRecommendedScores(ctx, false, null))
? SKILL_CATALOG_RECOMMENDED_FALLBACK_SORT
: undefined))
: undefined;
const recommendedFallback = decodedCursor.recommendedFallback;
const catalogSort = recommendedFallback ?? args.sort;
while (
@@ -6588,15 +6578,7 @@ export const listPackageCatalogPage = query({
let done = decodedCursor.done;
let loops = 0;
let remainingScanBudget = MAX_SKILL_CATALOG_SCAN_DOCUMENTS;
const isFreshRecommendedRequest =
args.sort === "recommended" && args.paginationOpts.cursor === null;
const recommendedFallback =
args.sort === "recommended"
? (decodedCursor.recommendedFallback ??
(isFreshRecommendedRequest && (await hasMissingRecommendedScores(ctx, false, null))
? SKILL_CATALOG_RECOMMENDED_FALLBACK_SORT
: undefined))
: undefined;
const recommendedFallback = decodedCursor.recommendedFallback;
const catalogSort = recommendedFallback ?? args.sort;
while (
@@ -6925,7 +6907,7 @@ function resolveRecommendedPublicListQuery({
return { sort: "updated", indexName: updatedIndexName, decodedCursor: updatedCursor };
}
if (hasMissingScores) {
return { sort: "updated", indexName: updatedIndexName, decodedCursor: null };
return { sort: "recommended", indexName: rankIndexName, decodedCursor: null };
}
return { sort: "recommended", indexName: scoreIndexName, decodedCursor: null };
}