Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 68 additions & 0 deletions packages/cli/src/commands/catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,74 @@ describe("catalog --json meaning search", () => {
});
});

describe("a query with no searchable words", () => {
// Runs the command capturing stderr, and reports the exit code the CLI would
// have used. finishCommand throws a signal rather than calling process.exit.
async function runForExit(
args: Record<string, unknown>,
): Promise<{ exitCode: number; err: string }> {
const command = (await import("./catalog.js")).default as unknown as {
run: (context: { args: Record<string, unknown> }) => Promise<void>;
};
const lines: string[] = [];
const capture = (...parts: unknown[]): void => {
lines.push(parts.map(String).join(" "));
};
const log = vi.spyOn(console, "log").mockImplementation(capture);
const error = vi.spyOn(console, "error").mockImplementation(capture);
let exitCode = 0;
try {
await command.run({ args });
} catch (thrown) {
const signal = thrown as { result?: { exitCode?: number }; exitCode?: number };
exitCode = signal.result?.exitCode ?? signal.exitCode ?? -1;
} finally {
log.mockRestore();
error.mockRestore();
}
// eslint-disable-next-line no-control-regex
const esc = String.fromCharCode(27);
return { exitCode, err: lines.join("\n").split(`${esc}[`).join("").replace(/\d+m/g, "") };
}

it("exits non-zero, because it is bad input rather than an empty shelf", async () => {
// The flag is word-tier only, so the stubbed on-device ranker has to be off
// or it answers with hits and the branch never runs.
state.modelStatus = "declined";
state.ranking = null;

const { exitCode } = await runForExit({ query: "実写写真のみ 9:16 生活ハック" });

// An agent that only reads the exit code would otherwise take "success, no
// results" at face value and hand-author a move already in the registry.
expect(exitCode).toBe(1);
});

it("says to search in English and does not blame the catalog", async () => {
state.modelStatus = "declined";
state.ranking = null;

const { err } = await runForExit({ query: "実写写真のみ 9:16 生活ハック" });

expect(err).toContain("No searchable words in query");
expect(err).toContain("Search in English");
expect(err).toContain("not a gap in");
// The gap channel must not be offered: nothing was searched, so a report
// here is noise in the one signal that tells us what to build.
expect(err).not.toContain("--search-miss");
});

it("leaves a genuine empty result exiting zero", async () => {
state.ranking = [];
state.modelStatus = "declined";

const { exitCode, err } = await runForExit({ query: "quantum entanglement reactor" });

expect(exitCode).toBe(0);
expect(err).toContain("No items match");
});
});

describe("searchMissCommand", () => {
it("keeps a non-ASCII query intact", () => {
// Half of the gap reports received so far were CJK. A query mangled on the
Expand Down
59 changes: 45 additions & 14 deletions packages/cli/src/commands/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { loadProjectConfig, DEFAULT_PROJECT_CONFIG } from "../utils/projectConfi
import { resolve } from "node:path";
import { finishCommand } from "../utils/commandResult.js";
import { runAdd } from "./add.js";
import { searchByWords } from "../registry/localSearch.js";
import { hasNoSearchableTokens, searchByWords } from "../registry/localSearch.js";
import {
downloadOfferMessage,
ensureLocalModel,
Expand Down Expand Up @@ -243,6 +243,13 @@ export default defineCommand({
const matching = searched ? searched.items : tagged;

if (matching.length === 0) {
// "We could not read your query" is a different answer from "the catalog
// has nothing like this", and only one of them is worth reporting as a
// gap. Word matching indexes English, so a query in another script parses
// to no tokens at all and used to return the same empty list as a genuine
// miss -- sending an author off to report a move that may well exist.
const unsearchable =
Boolean(query) && searched?.localMode === "words" && hasNoSearchableTokens(query as string);
// An empty result is exactly when the tier matters most: nothing found on
// the weakest tier means something different from nothing found on the
// best one.
Expand All @@ -259,7 +266,13 @@ export default defineCommand({
shown: 0,
total: tagged.length,
...(warnings.length ? { warnings } : {}),
report_gap: searchMissCommand(query, tierToken(searched)),
...(unsearchable ? { unsearchable_query: true } : {}),
// Withheld when the query never parsed: the catalog has not been
// shown to be missing anything, so inviting a gap report here
// would file noise against a search that never ran.
...(unsearchable
? {}
: { report_gap: searchMissCommand(query, tierToken(searched)) }),
results: [],
},
null,
Expand All @@ -274,16 +287,34 @@ export default defineCommand({
query ? `query "${query}"` : null,
args.tag ? `tag "${args.tag}"` : null,
].filter(Boolean);
console.log(`No items match ${criteria.join(" and ")}.`);
// Zero results is the unambiguous case: no tier judgement to make and
// nothing to install, so name the gap channel outright.
if (query) {
console.log("");
console.log(c.dim(" Nothing in the catalog does this? Report the gap:"));
console.log(c.dim(` ${searchMissCommand(query, tierToken(searched))}`));
if (unsearchable) {
console.error(c.error(`No searchable words in query "${query}".`));
console.error("");
console.error(
c.warn(
" Word matching indexes the catalog in English, so a query written in another\n" +
" script produces no terms to match and returns nothing. This is not a gap in\n" +
" the catalog. Search in English; the on-screen copy of your video can stay\n" +
" in any language.",
),
);
} else {
console.log(`No items match ${criteria.join(" and ")}.`);
// Zero results is the unambiguous case: no tier judgement to make and
// nothing to install, so name the gap channel outright.
if (query) {
console.log("");
console.log(c.dim(" Nothing in the catalog does this? Report the gap:"));
console.log(c.dim(` ${searchMissCommand(query, tierToken(searched))}`));
}
}
}
if (query) await offerLocalModel(0, json, config.registry, artifactRevision);
// A query with no searchable words is bad input, not an empty shelf, so it
// exits non-zero like an invalid --type does. An agent that only checks the
// exit code would otherwise read "searched successfully, catalog has
// nothing" and go hand-author a move that is sitting in the registry.
if (unsearchable) finishCommand(1);
return;
}

Expand Down Expand Up @@ -561,11 +592,11 @@ async function applySearch<
// Shared vocabulary, not substring presence. A user asking to "make the
// pace feel faster" shares no literal substring with any description, so
// the old test returned nothing at all for exactly the phrasing people use.
const words = searchByWords(
query,
items,
(item) => `${item.name} ${item.title} ${item.description} ${(item.tags ?? []).join(" ")}`,
);
const words = searchByWords(query, items, (item) => ({
// Name and title are what an author types when they already know the move.
strong: `${item.name} ${item.title}`,
weak: `${item.description} ${(item.tags ?? []).join(" ")}`,
}));
// Word matching ranks the items in hand, so nothing can go missing.
return { items: words, localMode: "words", warnings, missing: 0, unindexed: 0, topScore: null };
}
Expand Down
94 changes: 91 additions & 3 deletions packages/cli/src/registry/localSearch.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from "vitest";

import { rankByWords, searchByWords, tokenize } from "./localSearch.js";
import { hasNoSearchableTokens, rankByWords, searchByWords, tokenize } from "./localSearch.js";

interface Item {
name: string;
Expand All @@ -13,7 +13,7 @@ const ITEMS: Item[] = [
{ name: "text-reveal", text: "Type reveals line by line beneath a mask." },
];

const textOf = (item: Item) => item.text;
const textOf = (item: Item) => ({ strong: "", weak: item.text });

describe("tokenize", () => {
it("drops stop words", () => {
Expand All @@ -25,7 +25,7 @@ describe("tokenize", () => {
});

it("lowercases and strips punctuation and digits", () => {
expect(tokenize("Camera, pushes 42 times!")).toEqual(["camera", "pushes", "times"]);
expect(tokenize("Camera, pushes 42 times!")).toEqual(["camera", "push", "time"]);
});

it("returns nothing for a query made only of stop words", () => {
Expand Down Expand Up @@ -95,3 +95,91 @@ describe("searchByWords", () => {
expect(searchByWords("quantum entanglement", ITEMS, textOf)).toEqual([]);
});
});

// ── The three ranking defects this scorer exists to fix ─────────────────────
// Each case is a query that failed against the real catalog, reduced to the
// smallest fixture that still reproduces it.

const named = (name: string, title: string, body: string) => ({ name, title, body });
const fieldsOf = (i: { name: string; title: string; body: string }) => ({
strong: `${i.name} ${i.title}`,
weak: i.body,
});

describe("name and title outrank description", () => {
const items = [
named("typewriter", "Typewriter", "Character-by-character text reveal with a blinking cursor."),
named("mk-emphasis-type", "Emphasis Type", "A phrase types on with emphasis on the key word."),
named("variable-axis-type", "Variable Axis Type", "Type that shifts along a variable axis."),
];

it("puts the item actually called typewriter first", () => {
// Ranked seventh against the real catalog before field weighting: an author
// typing a move's name got entries that merely mention typing.
expect(rankByWords("typewriter effect on a title", items, fieldsOf)[0]?.item.name).toBe(
"typewriter",
);
});
});

describe("plurals fold to the singular", () => {
const items = [
named(
"count-up",
"Count Up",
"A stat counter that eases between values and lands with a pulse.",
),
named("skeleton-reveal", "Skeleton Reveal", "Placeholder blocks resolve into content once."),
];

it("matches a pluralised query against a singular description", () => {
// "counts" is not "count" and "pulses" is not "pulse", so adding detail to
// a query used to make the result strictly worse.
expect(
rankByWords("a stat that counts up and then pulses once", items, fieldsOf)[0]?.item.name,
).toBe("count-up");
});

it("leaves a word that merely ends in s alone", () => {
expect(tokenize("press glass")).toEqual(["press", "glass"]);
});
});

describe("common words count for less than distinctive ones", () => {
const items = [
named("line-by-line-slide", "Line By Line Slide", "A headline arrives one line at a time."),
named("ui-3d-reveal", "UI 3D Reveal", "A panel reveals in three dimensions."),
named("panel-reveal", "Panel Reveal", "A panel reveals itself."),
named("pull-back-reveal", "Pull Back Reveal", "The camera pulls back to reveal the scene."),
];

it("does not let one hit on a catalog-wide word beat several precise ones", () => {
// Field weighting alone made this worse: every item merely NAMED *-reveal
// outranked the one that does the thing, because "reveal" is the catalog's
// most common word. Inverse document frequency is what settles it.
expect(rankByWords("reveal a headline one line at a time", items, fieldsOf)[0]?.item.name).toBe(
"line-by-line-slide",
);
});
});

describe("hasNoSearchableTokens", () => {
it("flags a query in a script this ranker cannot index", () => {
// The catalog is written in English and tokenising on [a-z]+ leaves nothing
// of a Japanese query. Reporting that as "no items match" told the author
// the catalog lacked a move it may well have.
expect(hasNoSearchableTokens("実写写真のみ 9:16 生活ハック")).toBe(true);
});

it("does not flag an ordinary query that simply matches nothing", () => {
expect(hasNoSearchableTokens("quantum entanglement")).toBe(false);
});

it("does not flag an empty query, which is a listing rather than a search", () => {
expect(hasNoSearchableTokens(" ")).toBe(false);
});

it("flags a query that is only stop words and punctuation", () => {
expect(hasNoSearchableTokens("the and of !!!")).toBe(true);
});
});
Loading
Loading