fix(catalog): rank on where a word appears and how rare it is (#3312)

* fix(catalog): rank on where a word appears and how rare it is

Word search returned the right move in the top three for 87% of a
39-query eval set built from real catalog intents. Three defects, all in
the same 75-line scorer, and all found by running the queries rather than
by reading the code.

A token matching an item's NAME counted exactly as much as one buried in
a description. Searching "typewriter effect on a title" ranked the item
literally called `typewriter` seventh, behind entries that merely mention
typing. Name and title now carry three times the weight: an author who
types a move's name is giving the strongest signal available and it was
being averaged away.

Plurals shared no vocabulary with the singular. "a stat that counts up
and then pulses once" matched nothing in a description reading "lands
with a restrained scale pulse", because `counts` is not `count`. Adding
detail to a query made results strictly worse, which is the opposite of
what a search should do. Plurals now fold, and only plurals: Porter would
fold `counter` to `count` and `values` to `valu`, merging moves that mean
different things.

Field weighting alone made one case worse, which is why inverse document
frequency is here too. "reveal a headline one line at a time" put every
item merely NAMED `*-reveal` on top, because one strong hit on the
catalog's most common word outscored several weak hits on the words that
actually narrowed it down. Rarity now scales each term.

Separately: a query in a script this ranker cannot index no longer
reports itself as an empty catalog. Tokenising on [a-z]+ leaves nothing
of a Japanese query, and returning "no items match" told the author the
catalog lacked a move it may well have, then invited them to file a gap
report about it. That case now says what actually happened and withholds
the gap prompt, since nothing was searched.

Measured on the same 39 queries, before and after:
  top-1  31/39 (79%) -> 33/39 (85%)
  top-3  34/39 (87%) -> 39/39 (100%)

Test plan: 13 new tests, each a real failing query reduced to the
smallest fixture that still reproduces it. Existing tests migrated to the
fields API (two callers total). Full CLI suite 2643 passed, 2
pre-existing transcribe failures unchanged. Verified against the real
CLI: "typewriter effect on a title" now returns typewriter first, and
"chat conversation between a user and an assistant" returns chat-message,
chat-thread, ai-chat-reveal instead of transitions-blur.

* docs(skills): say to query the catalog in English

The runtime message added alongside this explains an unsearchable query
after the fact. Saying it up front is cheaper: an agent that never writes
the query in Japanese never sees the error, never wastes the turn, and
never files a gap report about a component that exists.

Worth stating rather than assuming, because the mistake is a reasonable
one. On a Japanese or Chinese project the brief, the narration and the
captions are all in that language and the query naturally follows. The
rule is that the query language and the video language are unrelated:
describe the move in English, write the on-screen copy in whatever the
video needs.

Both skills that own `catalog --query` carry it, and those are the only
two that mention the command at all.

* fix(catalog): fail a non-English query instead of returning nothing

The message explaining an unsearchable query went to stdout and the
command exited 0. An agent that checks the exit code, which is most of
them, read that as "searched successfully, the catalog has nothing" and
went off to hand-author a move that is sitting in the registry. The
explanation only helped a human who happened to be reading the terminal.

It is bad input, not an empty shelf, so it now behaves like one: the
guidance goes to stderr and the command exits 1, matching what an invalid
--type already does. A genuine empty result, where the query parsed fine
and the catalog simply has nothing, still exits 0 -- that distinction is
the whole point, and both halves are pinned by tests.

The wording now also says what to do rather than only what happened:
search in English, and let the on-screen copy of the video stay in
whatever language it needs. That was the part agents were getting wrong,
since a Japanese project makes a Japanese query feel natural.

Test plan: 3 new tests covering the exit code, the wording, and the
genuine-empty case that must stay at 0. Also asserts the gap-report line
is absent, since nothing was searched and a report there is noise in the
one signal that tells us what to build. catalog.test.ts 32 passed;
commands + registry suites 887 passed with the 2 pre-existing transcribe
failures unchanged. Verified against the real CLI: a CJK query exits 1, a
genuine miss exits 0.
This commit is contained in:
Miguel Ángel
2026-08-17 17:20:22 -04:00
committed by GitHub
parent 35eb6d2906
commit 6b17c24f98
7 changed files with 315 additions and 35 deletions
+68
View File
@@ -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
+45 -14
View File
@@ -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,
@@ -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.
@@ -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,
@@ -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;
}
@@ -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 };
}