From f24134b68cad75f9b067fbc88452e0f42a11bfda Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 17 Aug 2026 18:12:40 -0400 Subject: [PATCH 1/5] fix(add): apply --vars to components, and explain a failed download Customising an item on the catalog page, copying the printed command and running it did nothing for a component. `--vars` was accepted, documented and then dropped: buildSnippet put the values on a block's mount element and returned a bare "paste from ..." comment for a component, so 221 of the 375 catalog items silently ignored every value the page produced. A component has no mount element to hang values on. It is markup pasted into a host, and it resolves values through __hyperframes.getVariables(), which merges the declared defaults of every [data-composition-variables] element in the document with render-time overrides. So the component's own declaration is the only place a chosen value can live and still be there after the paste. `add --vars` now rewrites those defaults. Blocks keep the mount attribute. Per-mount values are strictly better where a mount exists: the file on disk stays byte-identical to the registry's, so a later reinstall can still tell an edit from an update, and two mounts of the same block can differ. A value the item cannot accept is now refused rather than written. An out-of-range number or an unlisted enum value falls back at runtime and warns, so writing one would produce a file that renders exactly as if the value had been ignored -- the failure this change exists to remove. Ids the item never declared are reported too, instead of vanishing. Only the requested item is rewritten; a dependency dragged in behind it never declared these variables. Separately, `Install failed: fetch failed` is now a sentence. Item FILES are not cached (only manifests are), so a network blip surfaces as node's bare message with no URL and no cause, immediately after the user copied a command off a web page -- which reads as "the command was wrong" rather than "the network was". It now names what failed, says it is usually connectivity or a proxy rather than a bad command, and mentions HTTPS_PROXY. Also fixes the two transcribe tests that were failing before this branch. They assert the whisper soft-skip path but never pinned the engine, and `auto` picks Parakeet whenever parakeet-mlx is installed -- so on those machines the test shelled out to a real ASR binary, failed with "Parakeet did not produce output", and landed in the generic failure branch it claims is never taken. Pinned to `engine: "whisper"`, plus an assertion that the mocked transcribe actually ran, which is what stops the test passing on a machine without Parakeet while testing nothing on one with it. The file now runs in 18ms rather than 3.7s, because it no longer launches a subprocess. Test plan: 10 new tests for the rewrite (enum and range refusal, the numeric-string coercion the catalog URL depends on since every query value is a string, delimiter escaping, unparseable declarations) and 3 for the failure message. Full CLI suite: 2661 passed, ZERO failures. Verified as a user, not just in unit tests: installed blur-in with the exact reported command, confirmed the declaration carried 76 / accent / center, pasted it into a composition and ran `check` -- which reported canvas_overflow at 76px, which only happens if the baked size is really in effect. Bad values warn and are refused; blocks still emit data-variable-values. --- packages/cli/src/commands/add.test.ts | 36 ++++- packages/cli/src/commands/add.ts | 90 +++++++++-- packages/cli/src/commands/transcribe.test.ts | 18 ++- packages/cli/src/registry/index.ts | 25 +-- packages/cli/src/registry/installer.ts | 121 +++++++++++---- .../cli/src/registry/variableDefaults.test.ts | 108 +++++++++++++ packages/cli/src/registry/variableDefaults.ts | 143 ++++++++++++++++++ 7 files changed, 475 insertions(+), 66 deletions(-) create mode 100644 packages/cli/src/registry/variableDefaults.test.ts create mode 100644 packages/cli/src/registry/variableDefaults.ts diff --git a/packages/cli/src/commands/add.test.ts b/packages/cli/src/commands/add.test.ts index e11e639740..4c575b6015 100644 --- a/packages/cli/src/commands/add.test.ts +++ b/packages/cli/src/commands/add.test.ts @@ -3,7 +3,14 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "no import { tmpdir } from "node:os"; import { join } from "node:path"; import type { RegistryItem, RegistryManifest } from "@hyperframes/core"; -import { AddError, buildSnippet, parseVariableValues, remapTarget, runAdd } from "./add.js"; +import { + AddError, + buildSnippet, + describeInstallFailure, + parseVariableValues, + remapTarget, + runAdd, +} from "./add.js"; import { trackRegistryItemAdded } from "../telemetry/events.js"; // Assert the emitted payload rather than the transport: `shouldTrack()` is @@ -462,3 +469,30 @@ describe("variable values in the snippet", () => { expect(parseVariableValues(undefined)).toBeNull(); }); }); + +describe("describeInstallFailure", () => { + it("explains a bare transport failure instead of echoing it", () => { + // What the user actually sees after copying a command off the catalog page. + // Item FILES are not cached, so a blip surfaces as node's `fetch failed` + // with no URL and no cause, and reads like the command was wrong. + const message = describeInstallFailure(new Error("fetch failed")); + + expect(message).toContain("could not download the item's files"); + expect(message).toContain("rather than a bad command"); + expect(message).toContain("HTTPS_PROXY"); + }); + + it("surfaces the underlying cause when node attached one", () => { + const err = new Error("fetch failed", { cause: new Error("getaddrinfo ENOTFOUND") }); + + expect(describeInstallFailure(err)).toContain("getaddrinfo ENOTFOUND"); + }); + + it("leaves a non-transport failure exactly as it was", () => { + // An unsafe target or a malformed item is the caller's problem to read; a + // connectivity lecture there would send them to fix the wrong thing. + const message = describeInstallFailure(new Error('Unsafe target "../x"')); + + expect(message).toBe('Install failed: Unsafe target "../x"'); + }); +}); diff --git a/packages/cli/src/commands/add.ts b/packages/cli/src/commands/add.ts index e892065f65..813237a5a2 100644 --- a/packages/cli/src/commands/add.ts +++ b/packages/cli/src/commands/add.ts @@ -140,6 +140,8 @@ export interface RunAddResult { installed: string[]; snippet: string; clipboardCopied: boolean; + /** Variable ids whose default was baked into an installed component. */ + variablesApplied: string[]; warnings: string[]; } @@ -181,22 +183,64 @@ async function installAll( destDir: string, baseUrl: string | undefined, force: boolean, -): Promise<{ written: string[]; preserved: string[] }> { + requestedName: string, + variableValues: Record | null, +): Promise<{ + written: string[]; + preserved: string[]; + variablesApplied: string[]; + variablesUnknown: string[]; + variablesInvalid: { id: string; reason: string }[]; +}> { const written: string[] = []; const preserved: string[] = []; + let variablesApplied: string[] = []; + let variablesUnknown: string[] = []; + let variablesInvalid: { id: string; reason: string }[] = []; try { for (const planItem of installPlan) { - const result = await installItem(planItem, { destDir, baseUrl, force }); + const result = await installItem(planItem, { + destDir, + baseUrl, + force, + // Only the item the user named. A dependency dragged in behind it never + // declared these variables and must not be rewritten by them. + variableValues: planItem.name === requestedName ? variableValues : null, + }); written.push(...result.written); preserved.push(...result.preserved); + if (planItem.name === requestedName) { + variablesApplied = result.variablesApplied; + variablesUnknown = result.variablesUnknown; + variablesInvalid = result.variablesInvalid; + } } } catch (err) { - throw new AddError( - `Install failed: ${err instanceof Error ? err.message : String(err)}`, - "install-failed", - ); + throw new AddError(describeInstallFailure(err), "install-failed"); } - return { written, preserved }; + return { written, preserved, variablesApplied, variablesUnknown, variablesInvalid }; +} + +/** + * Turn a transport failure into something a reader can act on. + * + * Item FILES are not cached (only manifests are), so a network blip surfaces + * here as node's bare `fetch failed` with no URL, no cause and no suggestion. + * That is what a user sees after copying a command off the catalog page, and + * it reads like the command was wrong rather than the network. + */ +export function describeInstallFailure(err: unknown): string { + const message = err instanceof Error ? err.message : String(err); + const cause = err instanceof Error && err.cause instanceof Error ? err.cause.message : ""; + const transport = + /fetch failed|ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|EAI_AGAIN|socket hang up|aborted/i; + if (!transport.test(`${message} ${cause}`)) return `Install failed: ${message}`; + return ( + `Install failed: could not download the item's files${cause ? ` (${cause})` : ""}. ` + + "The registry is served over the network and item files are not cached, so this is " + + "usually connectivity, a proxy, or an offline machine rather than a bad command. " + + "Check your connection and retry; if you are behind a proxy, set HTTPS_PROXY." + ); } export async function runAdd(opts: RunAddArgs): Promise { @@ -244,12 +288,16 @@ export async function runAdd(opts: RunAddArgs): Promise { })); // 5. Install — dependencies first, requested item last. - const { written, preserved } = await installAll( - installPlan, - projectDir, - config.registry, - opts.force ?? false, - ); + const variableValues = parseVariableValues(opts.vars); + const { written, preserved, variablesApplied, variablesUnknown, variablesInvalid } = + await installAll( + installPlan, + projectDir, + config.registry, + opts.force ?? false, + item.name, + variableValues, + ); // Report what landed, not what was asked for: a failed install throws above, // and the bulk `add ` path re-enters here per item, so this one place @@ -269,9 +317,16 @@ export async function runAdd(opts: RunAddArgs): Promise { itemForInstall.files.find((f) => f.type === "hyperframes:composition") ?? itemForInstall.files[0]; const snippetTargetRel = primaryFile?.target ?? ""; - const snippet = buildSnippet(item, snippetTargetRel, parseVariableValues(opts.vars)); + const snippet = buildSnippet(item, snippetTargetRel, variableValues); const clipboardCopied = !opts.skipClipboard && snippet ? copyToClipboard(snippet) : false; + for (const { id, reason } of variablesInvalid) { + warnings.push(`--vars ${id} ignored: ${reason}`); + } + if (variablesUnknown.length > 0) { + warnings.push(`--vars ignored (not declared by ${item.name}): ${variablesUnknown.join(", ")}`); + } + return { ok: true, name: item.name, @@ -282,6 +337,7 @@ export async function runAdd(opts: RunAddArgs): Promise { installed: installPlan.map((planItem) => planItem.name), snippet, clipboardCopied, + variablesApplied, warnings, }; } @@ -374,6 +430,12 @@ export default defineCommand({ for (const file of result.written) { console.log(` ${c.dim(relative(projectDir, file))}`); } + if (result.variablesApplied.length > 0) { + // Say it out loud. A component's values are baked into the file rather + // than shown in the snippet, so without this the command looks + // identical whether --vars worked or was thrown away. + console.log(` ${c.dim(`variables applied: ${result.variablesApplied.join(", ")}`)}`); + } if (result.snippet) { console.log(""); console.log(c.dim("Include snippet:")); diff --git a/packages/cli/src/commands/transcribe.test.ts b/packages/cli/src/commands/transcribe.test.ts index 621210c900..5c4bd521d5 100644 --- a/packages/cli/src/commands/transcribe.test.ts +++ b/packages/cli/src/commands/transcribe.test.ts @@ -48,8 +48,16 @@ describe("transcribe command", () => { it("explicit run exits non-zero and is NOT reported as a command failure", async () => { const { dir, input } = dummyAudio(); dirs.push(dir); - await transcribeCmd.run!({ args: { input, json: true, optional: false } } as never); + // Pin the engine. `auto` picks Parakeet whenever parakeet-mlx happens to be + // installed, and only the whisper path is mocked here -- so on those + // machines this test used to shell out to a real ASR binary, fail with + // "Parakeet did not produce output", and land in the generic failure branch + // instead of the soft-skip it is asserting. + await transcribeCmd.run!({ + args: { input, json: true, optional: false, engine: "whisper" }, + } as never); + expect(transcribeMock).toHaveBeenCalled(); expect(consumeCommandResult().exitCode).toBe(1); expect(trackTranscribeUnavailable).toHaveBeenCalledWith({ optional: false }); expect(trackCommandFailure).not.toHaveBeenCalled(); @@ -58,8 +66,14 @@ describe("transcribe command", () => { it("--optional skips cleanly with exit 0", async () => { const { dir, input } = dummyAudio(); dirs.push(dir); - await transcribeCmd.run!({ args: { input, json: true, optional: true } } as never); + await transcribeCmd.run!({ + args: { input, json: true, optional: true, engine: "whisper" }, + } as never); + // Asserting the mock ran is what keeps this honest: without it the test + // passes on a machine with no Parakeet and silently tests nothing on one + // that has it. + expect(transcribeMock).toHaveBeenCalled(); expect(consumeCommandResult().exitCode).toBe(0); expect(trackTranscribeUnavailable).toHaveBeenCalledWith({ optional: true }); expect(trackCommandFailure).not.toHaveBeenCalled(); diff --git a/packages/cli/src/registry/index.ts b/packages/cli/src/registry/index.ts index c7ca5f984e..b81f5f12a5 100644 --- a/packages/cli/src/registry/index.ts +++ b/packages/cli/src/registry/index.ts @@ -1,21 +1,8 @@ -export { - DEFAULT_REGISTRY_URL, - fetchRegistryManifest, - fetchItemManifest, - fetchItemFile, -} from "./remote.js"; +// Only what other modules actually import. Everything else in this folder is +// reached through its own module, so re-exporting it here just creates surface +// that has to be kept working without anyone depending on it. +export { DEFAULT_REGISTRY_URL } from "./remote.js"; -export { - listRegistryItems, - loadAllItems, - resolveItem, - resolveItemsByTag, - type ResolveOptions, -} from "./resolver.js"; +export { listRegistryItems, loadAllItems, resolveItemsByTag } from "./resolver.js"; -export { - installItem, - assertSafeTarget, - type InstallOptions, - type InstallResult, -} from "./installer.js"; +export { installItem } from "./installer.js"; diff --git a/packages/cli/src/registry/installer.ts b/packages/cli/src/registry/installer.ts index eb3d71f615..266439aaeb 100644 --- a/packages/cli/src/registry/installer.ts +++ b/packages/cli/src/registry/installer.ts @@ -11,6 +11,7 @@ import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { resolve, relative, isAbsolute } from "node:path"; import type { FileTarget, RegistryItem } from "@hyperframes/core"; import { fetchItemFile, DEFAULT_REGISTRY_URL } from "./remote.js"; +import { applyVariableDefaults, type ApplyResult } from "./variableDefaults.js"; export interface InstallOptions { /** Project root where files land. Every target resolves relative to this. */ @@ -19,6 +20,12 @@ export interface InstallOptions { baseUrl?: string; /** Overwrite files the project has changed since they were installed. */ force?: boolean; + /** + * `--vars` values to bake into a COMPONENT's declared defaults. A block + * carries its values on the mount element instead, so this is ignored there: + * per-mount values are strictly better when a mount exists. + */ + variableValues?: Record | null; } export interface InstallResult { @@ -26,6 +33,11 @@ export interface InstallResult { written: string[]; /** Absolute paths left alone because the project had changed them. */ preserved: string[]; + /** Variable ids whose default was rewritten in an installed component. */ + variablesApplied: string[]; + /** Ids the item does not declare, and ids it declares but cannot accept. */ + variablesUnknown: string[]; + variablesInvalid: { id: string; reason: string }[]; } /** @@ -91,7 +103,7 @@ export function hasLocalEdits( * install time so a registry that bypasses schema validation still can't write * outside the project. */ -export function assertSafeTarget(destDir: string, target: string): void { +function assertSafeTarget(destDir: string, target: string): void { if (isAbsolute(target)) { throw new Error(`Unsafe target "${target}": absolute paths are not allowed.`); } @@ -108,6 +120,11 @@ export function assertSafeTarget(destDir: string, target: string): void { } } +/** A component's pasteable markup: the file whose declared defaults `--vars` edits. */ +function isInstalledComponentSnippet(item: RegistryItem, file: FileTarget): boolean { + return item.type === "hyperframes:component" && file.target.toLowerCase().endsWith(".html"); +} + function isInstalledRegistryBlockComposition(item: RegistryItem, file: FileTarget): boolean { return ( item.type === "hyperframes:block" && @@ -124,6 +141,61 @@ function addRegistryItemMarker(source: string, item: RegistryItem): string { return `\n${source}`; } +interface FileOutcome { + destPath: string; + target: string; + preserved: boolean; + hash: string | null; + vars: ApplyResult | null; +} + +/** Fetch, write and post-process one file. Extracted so installItem stays readable. */ +async function installOneFile( + item: RegistryItem, + file: FileTarget, + destDir: string, + baseUrl: string, + record: InstallRecord, + options: InstallOptions, +): Promise { + const destPath = resolve(destDir, file.target); + + // Decided before fetching rather than after: a file we are going to keep + // should never be overwritten and then put back, because a crash in + // between would lose it for real. + if ( + !options.force && + existsSync(destPath) && + hasLocalEdits(record, file.target, readFileSync(destPath)) + ) { + return { destPath, target: file.target, preserved: true, hash: null, vars: null }; + } + + await fetchItemFile(item, file, destPath, baseUrl); + if (isInstalledRegistryBlockComposition(item, file)) { + const source = readFileSync(destPath, "utf-8"); + writeFileSync(destPath, addRegistryItemMarker(source, item), "utf-8"); + } + // A component has no mount element to hang values on, so the chosen + // values go into its own declaration or they go nowhere. See + // variableDefaults.ts for why that is the only surviving home. + let vars: ApplyResult | null = null; + if (options.variableValues && isInstalledComponentSnippet(item, file)) { + const source = readFileSync(destPath, "utf-8"); + vars = applyVariableDefaults(source, options.variableValues); + if (vars.applied.length > 0) writeFileSync(destPath, vars.html, "utf-8"); + } + // Hash what actually landed, marker and baked defaults included, or the + // next install reads its own output as the project's edit. + return { + destPath, + target: file.target, + preserved: false, + hash: digest(readFileSync(destPath)), + vars, + }; +} + /** * Install a resolved `RegistryItem` into `destDir` by fetching each file in * parallel and writing it to its validated target path. @@ -143,34 +215,9 @@ export async function installItem( const record = readInstallRecord(destDir); const outcomes = await Promise.all( - item.files.map(async (file: FileTarget) => { - const destPath = resolve(destDir, file.target); - - // Decided before fetching rather than after: a file we are going to keep - // should never be overwritten and then put back, because a crash in - // between would lose it for real. - if ( - !options.force && - existsSync(destPath) && - hasLocalEdits(record, file.target, readFileSync(destPath)) - ) { - return { destPath, target: file.target, preserved: true, hash: null }; - } - - await fetchItemFile(item, file, destPath, baseUrl); - if (isInstalledRegistryBlockComposition(item, file)) { - const source = readFileSync(destPath, "utf-8"); - writeFileSync(destPath, addRegistryItemMarker(source, item), "utf-8"); - } - // Hash what actually landed, marker included, or the next install reads - // its own marker as the project's edit. - return { - destPath, - target: file.target, - preserved: false, - hash: digest(readFileSync(destPath)), - }; - }), + item.files.map((file: FileTarget) => + installOneFile(item, file, destDir, baseUrl, record, options), + ), ); const written = outcomes.filter((o) => !o.preserved).map((o) => o.destPath); @@ -183,5 +230,19 @@ export async function installItem( writeInstallRecord(destDir, record); } - return { written, preserved }; + const vars = outcomes.map((o) => o.vars).filter((v): v is ApplyResult => v !== null); + return { + written, + preserved, + variablesApplied: vars.flatMap((v) => v.applied), + // An id nothing declared is only genuinely unknown once every file has had + // a chance at it, so intersect rather than union. + variablesUnknown: vars.length + ? vars.reduce( + (acc, v) => acc.filter((id) => v.unknown.includes(id)), + vars[0]!.unknown, + ) + : [], + variablesInvalid: vars.flatMap((v) => v.invalid), + }; } diff --git a/packages/cli/src/registry/variableDefaults.test.ts b/packages/cli/src/registry/variableDefaults.test.ts new file mode 100644 index 0000000000..2ea19df874 --- /dev/null +++ b/packages/cli/src/registry/variableDefaults.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from "vitest"; + +import { applyVariableDefaults } from "./variableDefaults.js"; + +/** The shape the registry actually ships: single-quoted attribute, JSON inside. */ +const COMPONENT = `
+ Design +
`; + +function defaultOf(html: string, id: string): unknown { + const raw = html.match(/data-composition-variables='([\s\S]*?)'/)![1]!; + const decl = JSON.parse(raw.replace(/'/g, "'")) as { id: string; default: unknown }[]; + return decl.find((d) => d.id === id)!.default; +} + +describe("applyVariableDefaults", () => { + it("rewrites the declared default so a pasted component carries the chosen value", () => { + // The whole point: a component has no mount element, so this is the only + // place a value picked on the catalog page can survive being pasted. + const r = applyVariableDefaults(COMPONENT, { size: 76, tone: "accent" }); + + expect(r.applied.sort()).toEqual(["size", "tone"]); + expect(defaultOf(r.html, "size")).toBe(76); + expect(defaultOf(r.html, "tone")).toBe("accent"); + }); + + it("leaves untouched variables at their shipped defaults", () => { + const r = applyVariableDefaults(COMPONENT, { size: 76 }); + + expect(defaultOf(r.html, "tone")).toBe("strong"); + }); + + it("reports an id the item does not declare instead of silently dropping it", () => { + const r = applyVariableDefaults(COMPONENT, { nope: 1 }); + + expect(r.unknown).toEqual(["nope"]); + expect(r.applied).toEqual([]); + expect(r.html).toBe(COMPONENT); + }); + + it("refuses an enum value outside the declared options", () => { + // Writing it would produce a file that renders as if the value were + // ignored, because the composition's own guard falls back to the default. + const r = applyVariableDefaults(COMPONENT, { tone: "chartreuse" }); + + expect(r.invalid).toEqual([{ id: "tone", reason: "not one of strong, muted, accent" }]); + expect(defaultOf(r.html, "tone")).toBe("strong"); + }); + + it("refuses a number outside its declared range", () => { + expect(applyVariableDefaults(COMPONENT, { size: 9999 }).invalid).toEqual([ + { id: "size", reason: "above max 120" }, + ]); + expect(applyVariableDefaults(COMPONENT, { size: 1 }).invalid).toEqual([ + { id: "size", reason: "below min 24" }, + ]); + }); + + it("coerces a numeric string, because a URL and a form both produce one", () => { + // The catalog page puts values in the query string, where every value is a + // string. Writing "76" where the composition expects a number would make + // the guard fall back and look like the value was ignored. + const r = applyVariableDefaults(COMPONENT, { size: "76" }); + + expect(r.applied).toEqual(["size"]); + expect(defaultOf(r.html, "size")).toBe(76); + }); + + it("escapes a value containing the attribute's own delimiter", () => { + const withText = COMPONENT.replace( + '{ "id": "size", "type": "number", "role": "style", "default": 52, "min": 24, "max": 120 }', + '{ "id": "label", "type": "string", "default": "hi" }', + ); + const r = applyVariableDefaults(withText, { label: "it's fine" }); + + expect(r.applied).toEqual(["label"]); + // A raw apostrophe would close the attribute early and break the markup. + expect(r.html).not.toMatch(/data-composition-variables='[^']*it's/); + expect(defaultOf(r.html, "label")).toBe("it's fine"); + }); + + it("does nothing when the item declares no variables at all", () => { + const plain = "
no declaration here
"; + + expect(applyVariableDefaults(plain, { size: 1 })).toEqual({ + html: plain, + applied: [], + unknown: ["size"], + invalid: [], + }); + }); + + it("refuses to rewrite a declaration it cannot parse", () => { + const broken = `
x
`; + + expect(applyVariableDefaults(broken, { id: 1 }).html).toBe(broken); + }); + + it("is a no-op for an empty value set", () => { + expect(applyVariableDefaults(COMPONENT, {}).html).toBe(COMPONENT); + }); +}); diff --git a/packages/cli/src/registry/variableDefaults.ts b/packages/cli/src/registry/variableDefaults.ts new file mode 100644 index 0000000000..1d1e781c94 --- /dev/null +++ b/packages/cli/src/registry/variableDefaults.ts @@ -0,0 +1,143 @@ +/** + * Bake chosen variable values into an installed item's declared defaults. + * + * A block is mounted by a `
`, so `add --vars` can put + * the values on that mount as `data-variable-values` and two mounts of the same + * block can differ. A component has no mount element: it is markup you paste + * into a host composition, and it reads its values through + * `__hyperframes.getVariables()`, which merges the declared defaults of every + * `[data-composition-variables]` element in the document with render-time + * overrides. + * + * So for a component the only place a chosen value can live and survive being + * pasted is the component's own declaration. Rewriting the defaults there is + * what makes "customise it on the catalog page, copy the command, run it" end + * with the look you picked. Before this, `--vars` was accepted, documented, and + * silently discarded for every component in the catalog. + */ + +interface VariableDeclaration { + id?: unknown; + type?: unknown; + default?: unknown; + options?: unknown; + min?: unknown; + max?: unknown; +} + +export interface ApplyResult { + /** The source with defaults rewritten. Unchanged when nothing applied. */ + html: string; + /** Variable ids whose default was replaced. */ + applied: string[]; + /** Ids the item does not declare. */ + unknown: string[]; + /** Ids declared but given a value the declaration does not allow. */ + invalid: { id: string; reason: string }[]; +} + +const ATTR = "data-composition-variables"; + +/** Locate the attribute's quoted value, tolerating either delimiter. */ +function findDeclaration(source: string): { start: number; end: number; raw: string } | null { + const at = source.indexOf(`${ATTR}=`); + if (at === -1) return null; + const quote = source[at + ATTR.length + 1]; + if (quote !== "'" && quote !== '"') return null; + const start = at + ATTR.length + 2; + const end = source.indexOf(quote, start); + if (end === -1) return null; + return { start, end, raw: source.slice(start, end) }; +} + +function decode(raw: string): string { + return raw.replace(/'/g, "'").replace(/"/g, '"'); +} + +/** Mirrors the escaping the block-mount path uses, so either delimiter is safe. */ +function encode(json: string, quote: string): string { + return quote === "'" ? json.replace(/'/g, "'") : json.replace(/"/g, """); +} + +function optionValues(decl: VariableDeclaration): string[] | null { + if (!Array.isArray(decl.options)) return null; + return decl.options.map((o) => + o && typeof o === "object" && "value" in o + ? String((o as { value: unknown }).value) + : String(o), + ); +} + +/** + * Reject a value the declaration cannot represent, rather than writing it. + * + * A bad enum falls back to the default at runtime and warns, so writing one + * here would produce a file that renders as if the value had been ignored -- + * which is the exact failure this function exists to remove. + */ +function rejectEnum(decl: VariableDeclaration, value: unknown): string | null { + const opts = optionValues(decl); + if (!opts) return null; + return opts.includes(String(value)) ? null : `not one of ${opts.join(", ")}`; +} + +function rejectNumber(decl: VariableDeclaration, value: unknown): string | null { + if (decl.type !== "number") return null; + const n = typeof value === "number" ? value : Number(value); + if (!Number.isFinite(n)) return "not a number"; + if (typeof decl.min === "number" && n < decl.min) return `below min ${decl.min}`; + if (typeof decl.max === "number" && n > decl.max) return `above max ${decl.max}`; + return null; +} + +function reject(decl: VariableDeclaration, value: unknown): string | null { + return rejectEnum(decl, value) ?? rejectNumber(decl, value); +} + +export function applyVariableDefaults( + source: string, + values: Record, +): ApplyResult { + const ids = Object.keys(values); + if (ids.length === 0) return { html: source, applied: [], unknown: [], invalid: [] }; + + const found = findDeclaration(source); + if (!found) return { html: source, applied: [], unknown: ids, invalid: [] }; + + let declared: VariableDeclaration[]; + try { + const parsed: unknown = JSON.parse(decode(found.raw)); + if (!Array.isArray(parsed)) return { html: source, applied: [], unknown: ids, invalid: [] }; + declared = parsed as VariableDeclaration[]; + } catch { + // A declaration we cannot parse is one we must not rewrite. + return { html: source, applied: [], unknown: ids, invalid: [] }; + } + + const applied: string[] = []; + const invalid: { id: string; reason: string }[] = []; + const byId = new Map(declared.map((d) => [String(d.id), d])); + + for (const [id, value] of Object.entries(values)) { + const decl = byId.get(id); + if (!decl) continue; + const reason = reject(decl, value); + if (reason) { + invalid.push({ id, reason }); + continue; + } + decl.default = decl.type === "number" ? Number(value) : value; + applied.push(id); + } + + const unknown = ids.filter((id) => !byId.has(id)); + if (applied.length === 0) return { html: source, applied, unknown, invalid }; + + // One declaration per line, matching how the registry authors these files, so + // a re-install produces a readable diff rather than one enormous line. + const quote = source[found.start - 1]!; + const body = declared.map((d) => ` ${JSON.stringify(d)}`).join(",\n"); + const rewritten = encode(`[\n${body}\n ]`, quote); + const html = source.slice(0, found.start) + rewritten + source.slice(found.end); + return { html, applied, unknown, invalid }; +} From d0b7ce03315c4296152983faf9fca535aad6edfb Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 17 Aug 2026 18:21:57 -0400 Subject: [PATCH 2/5] fix(player): load the runtime before the body, not after Customising a component on a catalog page did nothing to the preview. badge-pop with count 10 and a green accent rendered 3, in red. The probe injects the runtime by appending a script to an already loaded document, and only once it has a reason to: a nested composition, or five polls with a timeline present. A component has neither. It is markup pasted into a composition, and it reads its values in an inline IIFE that runs while the body is parsing: var vars = window.__hyperframes && window.__hyperframes.getVariables ? window.__hyperframes.getVariables() : {}; With the runtime arriving afterwards that guard always took the empty branch, so the component used the defaults hardcoded in its own script and every chosen value was dropped. The values were never the problem: the preview sets window.__hfVariables correctly, and nothing was there to read it. prepareSrcdocForElement now puts the same runtime URL in the document's head before the srcdoc is set. A classic external script in head is parser-blocking, so it runs before body scripts without changing what gets loaded or adding a dependency the player did not already have. A CLI render never had this bug because the engine already orders it this way. Skipped when the page carries the runtime already, so a CLI-rendered page (which inlines it) does not get a second copy re-initialising the runtime underneath a live composition. The probe's late injection stays for the src= path, where there is no srcdoc to prepare. The runtime URL moved to its own module so the two injection points cannot drift apart. Test plan: 8 new tests for the injection (ordering against the reading script, head placement, both no-op guards, missing head/body, attributes on the head tag). Three srcdoc tests asserted byte-identical forwarding and now assert what they were actually protecting -- that the composition arrives intact -- plus the new runtime guarantee. player 338 passed, studio 4249 passed. Verified end to end against the real runtime and a real registry component, asking for size 96 / accent / right: before 52px, rgb(243,243,243), flex-start, runtime absent after 96px, rgb(60,230,172), flex-end, runtime present rgb(60,230,172) is #3ce6ac, the accent green. That is the reported bug before, and the chosen values after. --- packages/player/src/composition-probe.ts | 14 +--- .../player/src/hyperframes-player.test.ts | 13 +++- packages/player/src/runtime-in-srcdoc.test.ts | 71 +++++++++++++++++++ packages/player/src/runtime-in-srcdoc.ts | 54 ++++++++++++++ packages/player/src/runtime-url.ts | 19 +++++ packages/player/src/shader-options.ts | 17 +++-- 6 files changed, 169 insertions(+), 19 deletions(-) create mode 100644 packages/player/src/runtime-in-srcdoc.test.ts create mode 100644 packages/player/src/runtime-in-srcdoc.ts create mode 100644 packages/player/src/runtime-url.ts diff --git a/packages/player/src/composition-probe.ts b/packages/player/src/composition-probe.ts index b7fdfc0fdd..b3c500b104 100644 --- a/packages/player/src/composition-probe.ts +++ b/packages/player/src/composition-probe.ts @@ -19,19 +19,9 @@ import { isRuntimeDurationAdapter, } from "./timeline-adapters.js"; -declare const __HYPERFRAMES_RUNTIME_CDN_URL__: string; +import { RUNTIME_CDN_URL, runtimeCdnUrlForVersion } from "./runtime-url.js"; -export function runtimeCdnUrlForVersion(version: string): string { - if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(version)) { - throw new Error(`Invalid HyperFrames runtime version: ${version}`); - } - return `https://cdn.jsdelivr.net/npm/@hyperframes/core@${version}/dist/hyperframe.runtime.iife.js`; -} - -const RUNTIME_CDN_URL = - typeof __HYPERFRAMES_RUNTIME_CDN_URL__ === "string" - ? __HYPERFRAMES_RUNTIME_CDN_URL__ - : runtimeCdnUrlForVersion("0.0.0-dev"); +export { runtimeCdnUrlForVersion }; export interface ProbeResult { duration: number; diff --git a/packages/player/src/hyperframes-player.test.ts b/packages/player/src/hyperframes-player.test.ts index f065f09786..ae853eb8e5 100644 --- a/packages/player/src/hyperframes-player.test.ts +++ b/packages/player/src/hyperframes-player.test.ts @@ -1473,7 +1473,11 @@ describe("HyperframesPlayer srcdoc attribute", () => { player.setAttribute("srcdoc", html); document.body.appendChild(player); - expect(player.iframe.getAttribute("srcdoc")).toBe(html); + // Not byte-identical: srcdoc now also carries the runtime, injected ahead + // of body scripts so a pasted component can read its variables during + // parse. The composition itself must still arrive intact. + expect(player.iframe.getAttribute("srcdoc")).toContain("hello"); + expect(player.iframe.getAttribute("srcdoc")).toContain("hyperframe.runtime.iife.js"); player.remove(); }); @@ -1487,7 +1491,8 @@ describe("HyperframesPlayer srcdoc attribute", () => { const html = "after connect"; player.setAttribute("srcdoc", html); - expect(player.iframe.getAttribute("srcdoc")).toBe(html); + expect(player.iframe.getAttribute("srcdoc")).toContain("after connect"); + expect(player.iframe.getAttribute("srcdoc")).toContain("hyperframe.runtime.iife.js"); player.remove(); }); @@ -1548,7 +1553,9 @@ describe("HyperframesPlayer srcdoc attribute", () => { document.body.appendChild(player); expect(player.iframe.getAttribute("src")).toBe("/api/projects/foo/preview"); - expect(player.iframe.getAttribute("srcdoc")).toBe(""); + // srcdoc carries the runtime now; what matters here is that both + // attributes are present so the browser can arbitrate. + expect(player.iframe.getAttribute("srcdoc")).toContain(""); player.remove(); }); diff --git a/packages/player/src/runtime-in-srcdoc.test.ts b/packages/player/src/runtime-in-srcdoc.test.ts new file mode 100644 index 0000000000..ca923d949c --- /dev/null +++ b/packages/player/src/runtime-in-srcdoc.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; + +import { ensureRuntimeBeforeBodyScripts } from "./runtime-in-srcdoc.js"; + +const URL = "https://cdn.example/hyperframe.runtime.iife.js"; + +/** What a pasted component looks like: it reads its variables during parse. */ +const COMPONENT_PAGE = `t +
+ +`; + +describe("ensureRuntimeBeforeBodyScripts", () => { + it("puts the runtime ahead of the script that reads variables", () => { + // The whole bug: the probe appended the runtime after load, so this guard + // always took the empty branch and the component used its hardcoded + // defaults. badge-pop with count 10 rendered 3. + const out = ensureRuntimeBeforeBodyScripts(COMPONENT_PAGE, URL); + + expect(out).toContain(``); + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("getVariables")); + }); + + it("puts it inside head, where an external script blocks the parser", () => { + const out = ensureRuntimeBeforeBodyScripts(COMPONENT_PAGE, URL); + + // Landing after would not block body parsing in the same way. + expect(out.indexOf(URL)).toBeGreaterThan(out.indexOf("")); + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("")); + }); + + it("does not add a second copy when the page already links it", () => { + const already = ``; + + expect(ensureRuntimeBeforeBodyScripts(already, URL)).toBe(already); + }); + + it("leaves a CLI-rendered page alone, which inlines the runtime already", () => { + // The engine inlines it and defines the global on the way in. A second + // copy would re-initialise the runtime underneath a live composition. + const rendered = ``; + + expect(ensureRuntimeBeforeBodyScripts(rendered, URL)).toBe(rendered); + }); + + it("falls back to before when there is no head", () => { + const out = ensureRuntimeBeforeBodyScripts("", URL); + + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("")); + }); + + it("handles a bare fragment by going first", () => { + const out = ensureRuntimeBeforeBodyScripts("
hi
", URL); + + expect(out.startsWith(``)).toBe(true); + }); + + it("is a no-op on empty input", () => { + expect(ensureRuntimeBeforeBodyScripts("", URL)).toBe(""); + }); + + it("survives a head tag carrying attributes", () => { + const out = ensureRuntimeBeforeBodyScripts( + ``, + URL, + ); + + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("read()")); + }); +}); diff --git a/packages/player/src/runtime-in-srcdoc.ts b/packages/player/src/runtime-in-srcdoc.ts new file mode 100644 index 0000000000..5027c8b6c5 --- /dev/null +++ b/packages/player/src/runtime-in-srcdoc.ts @@ -0,0 +1,54 @@ +/** + * Put the runtime in the document's head, before anything in the body runs. + * + * The probe injects the runtime by appending a ``; + const head = /]*>/i.exec(html); + if (head) { + const at = head.index + head[0].length; + return html.slice(0, at) + tag + html.slice(at); + } + // No head: get in before so body scripts still see the runtime. A + // fragment with neither lands at the front, which is the same guarantee. + const body = /]*>/i.exec(html); + if (body) return html.slice(0, body.index) + tag + html.slice(body.index); + const htmlTag = /]*>/i.exec(html); + if (htmlTag) { + const at = htmlTag.index + htmlTag[0].length; + return html.slice(0, at) + tag + html.slice(at); + } + return tag + html; +} diff --git a/packages/player/src/runtime-url.ts b/packages/player/src/runtime-url.ts new file mode 100644 index 0000000000..7191647e1f --- /dev/null +++ b/packages/player/src/runtime-url.ts @@ -0,0 +1,19 @@ +/** + * Where the runtime comes from. + * + * Split out of composition-probe so the probe's late injection and the srcdoc's + * parse-time injection cannot drift onto different URLs. + */ +declare const __HYPERFRAMES_RUNTIME_CDN_URL__: string; + +export function runtimeCdnUrlForVersion(version: string): string { + if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(version)) { + throw new Error(`Invalid HyperFrames runtime version: ${version}`); + } + return `https://cdn.jsdelivr.net/npm/@hyperframes/core@${version}/dist/hyperframe.runtime.iife.js`; +} + +export const RUNTIME_CDN_URL = + typeof __HYPERFRAMES_RUNTIME_CDN_URL__ === "string" + ? __HYPERFRAMES_RUNTIME_CDN_URL__ + : runtimeCdnUrlForVersion("0.0.0-dev"); diff --git a/packages/player/src/shader-options.ts b/packages/player/src/shader-options.ts index 1b59bc786a..791f7a6da0 100644 --- a/packages/player/src/shader-options.ts +++ b/packages/player/src/shader-options.ts @@ -4,6 +4,9 @@ * URLs and srcdoc HTML. */ +import { ensureRuntimeBeforeBodyScripts } from "./runtime-in-srcdoc.js"; +import { RUNTIME_CDN_URL } from "./runtime-url.js"; + export const SHADER_CAPTURE_SCALE_ATTR = "shader-capture-scale"; export const SHADER_LOADING_ATTR = "shader-loading"; const SHADER_CAPTURE_SCALE_PARAM = "__hf_shader_capture_scale"; @@ -141,9 +144,15 @@ export function prepareSrcForElement(el: Element, src: string): string { } export function prepareSrcdocForElement(el: Element, srcdoc: string): string { - return injectShaderOptionsIntoSrcdoc( - srcdoc, - normalizeShaderCaptureScale(el.getAttribute(SHADER_CAPTURE_SCALE_ATTR)), - getShaderModeFromElement(el), + // Runtime first, and in the head: a component's inline script reads its + // variables while the body is parsing, long before the probe's own injection + // could land. See runtime-in-srcdoc.ts. + return ensureRuntimeBeforeBodyScripts( + injectShaderOptionsIntoSrcdoc( + srcdoc, + normalizeShaderCaptureScale(el.getAttribute(SHADER_CAPTURE_SCALE_ATTR)), + getShaderModeFromElement(el), + ), + RUNTIME_CDN_URL, ); } From 12e3c3bfd127603945bf3b2efd250c219d6bce28 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 17 Aug 2026 18:44:05 -0400 Subject: [PATCH 3/5] fix(add): name the registry and the real reason an install failed, and retry `Install failed: fetch failed` was two words that describe every network problem equally badly. Three things were missing, and each of them was the whole answer in a different case. The URL. undici throws with no URL attached, so a project that points `registry` at a private host in hyperframes.json got a message that looked like the public registry had failed. Naming the URL is the entire diagnosis there. The cause. undici buries the real reason one or two levels down in `cause`, and it was being dropped. The reported failure turned out to be `self-signed certificate in certificate chain`: a private registry whose certificate node refuses and curl accepts, which is why the host looked healthy from a terminal. That sentence tells the reader which knob to turn; `fetch failed` sends them to check a connection that is working. The retry. Item files are the one uncached path -- manifests fall back to a stale copy, but every install downloads its files fresh -- so a single blip killed the whole command. Now two extra attempts with short backoff, and deliberately NOT for TLS failures: a self-signed certificate fails identically every time, so retrying it only makes the user wait three times as long for the same message. Also retypes the declaration reader. It modelled variables as a local interface of six `unknown` fields and re-checked each one at every use. Core already owns this shape as a discriminated union and exports `isCompositionVariable`, the same predicate `parseCompositionVariables` filters with, so the union is used directly and the duplicate type is gone. A declaration the schema rejects now leaves the file untouched rather than being partially rewritten from guesses. Test plan: 4 retry and URL tests, 5 cause-chain tests, and the add-side tests now cover the custom-registry hint and its absence on the default registry. The variableDefaults fixtures gained the `label` the schema actually requires; without it they were not valid declarations, which the stricter reader caught. CLI suite 2671 passed, zero failures. Verified with the BUILT dist rather than the source, in the reporter's own project directory. The failure now reads: File fetch failed: https:///registry/components/blur-in/blur-in.html - fetch failed (self-signed certificate in certificate chain [SELF_SIGNED_CERT_IN_CHAIN]) and once the project points back at the public registry the original command succeeds with `variables applied: size, tone, align`. --- packages/cli/src/commands/add.test.ts | 22 +++- packages/cli/src/commands/add.ts | 25 ++-- packages/cli/src/registry/remote.test.ts | 109 +++++++++++++++++- packages/cli/src/registry/remote.ts | 78 ++++++++++++- .../cli/src/registry/variableDefaults.test.ts | 8 +- packages/cli/src/registry/variableDefaults.ts | 70 ++++++----- 6 files changed, 264 insertions(+), 48 deletions(-) diff --git a/packages/cli/src/commands/add.test.ts b/packages/cli/src/commands/add.test.ts index 4c575b6015..2f3d7ce5e1 100644 --- a/packages/cli/src/commands/add.test.ts +++ b/packages/cli/src/commands/add.test.ts @@ -482,10 +482,26 @@ describe("describeInstallFailure", () => { expect(message).toContain("HTTPS_PROXY"); }); - it("surfaces the underlying cause when node attached one", () => { - const err = new Error("fetch failed", { cause: new Error("getaddrinfo ENOTFOUND") }); + it("names the project's own registry when it is not the public one", () => { + // The reported failure: hyperframes.json pointed at a private host with a + // self-signed certificate. Telling that reader to check their connection + // sends them to debug the one thing that was working. + const message = describeInstallFailure( + new Error("fetch failed"), + "https://private.example/registry", + ); + + expect(message).toContain("https://private.example/registry"); + expect(message).toContain("not the public registry"); + }); + + it("stays quiet about the registry when it is the default one", () => { + const message = describeInstallFailure( + new Error("fetch failed"), + "https://raw.githubusercontent.com/heygen-com/hyperframes/main/registry", + ); - expect(describeInstallFailure(err)).toContain("getaddrinfo ENOTFOUND"); + expect(message).not.toContain("not the public registry"); }); it("leaves a non-transport failure exactly as it was", () => { diff --git a/packages/cli/src/commands/add.ts b/packages/cli/src/commands/add.ts index 813237a5a2..fd6ed2aba8 100644 --- a/packages/cli/src/commands/add.ts +++ b/packages/cli/src/commands/add.ts @@ -15,7 +15,7 @@ import { existsSync } from "node:fs"; import { resolve, relative } from "node:path"; import { ITEM_TYPE_DIRS, type RegistryItem } from "@hyperframes/core"; import { c } from "../ui/colors.js"; -import { installItem, resolveItemsByTag } from "../registry/index.js"; +import { DEFAULT_REGISTRY_URL, installItem, resolveItemsByTag } from "../registry/index.js"; import { resolveItemWithDependencies } from "../registry/resolver.js"; import { gateRegistryItemsCompatibility, @@ -216,7 +216,7 @@ async function installAll( } } } catch (err) { - throw new AddError(describeInstallFailure(err), "install-failed"); + throw new AddError(describeInstallFailure(err, baseUrl), "install-failed"); } return { written, preserved, variablesApplied, variablesUnknown, variablesInvalid }; } @@ -229,17 +229,28 @@ async function installAll( * That is what a user sees after copying a command off the catalog page, and * it reads like the command was wrong rather than the network. */ -export function describeInstallFailure(err: unknown): string { +export function describeInstallFailure(err: unknown, registry?: string): string { const message = err instanceof Error ? err.message : String(err); const cause = err instanceof Error && err.cause instanceof Error ? err.cause.message : ""; const transport = /fetch failed|ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|EAI_AGAIN|socket hang up|aborted/i; if (!transport.test(`${message} ${cause}`)) return `Install failed: ${message}`; + + // Name the registry first. A project that set `registry` in hyperframes.json + // points at a private host, and when that host is down the failure has + // nothing to do with the user's connection -- telling them to check their + // network sends them to debug the one thing that is working. + const custom = + registry && !registry.startsWith(DEFAULT_REGISTRY_URL) + ? `\n This project's hyperframes.json sets registry to ${registry}, so that is the host ` + + "being contacted, not the public registry. If it is down or private, that is the failure." + : ""; return ( - `Install failed: could not download the item's files${cause ? ` (${cause})` : ""}. ` + - "The registry is served over the network and item files are not cached, so this is " + - "usually connectivity, a proxy, or an offline machine rather than a bad command. " + - "Check your connection and retry; if you are behind a proxy, set HTTPS_PROXY." + `Install failed: could not download the item's files.\n ${message}` + + "\n Item files are not cached, so every install fetches them. This is usually the " + + "registry host or the network rather than a bad command." + + custom + + "\n Retry, or set HTTPS_PROXY if you are behind a proxy." ); } diff --git a/packages/cli/src/registry/remote.test.ts b/packages/cli/src/registry/remote.test.ts index a6b046caf1..1fe299f23d 100644 --- a/packages/cli/src/registry/remote.test.ts +++ b/packages/cli/src/registry/remote.test.ts @@ -12,8 +12,13 @@ vi.mock("node:os", async (importOriginal) => ({ homedir: () => scratchHome, })); -const { fetchItemManifest, fetchRegistryManifest, DEFAULT_REGISTRY_URL } = - await import("./remote.js"); +const { + describeCauseChain, + fetchItemFile, + fetchItemManifest, + fetchRegistryManifest, + DEFAULT_REGISTRY_URL, +} = await import("./remote.js"); const MANIFEST = { name: "hyperframes", items: [{ name: "count-up" }] }; const ITEM = { name: "count-up", type: "hyperframes:component", files: [] }; @@ -160,3 +165,103 @@ describe("fetchItemManifest", () => { ).rejects.toThrow("HTTP 404"); }); }); + +describe("describeCauseChain", () => { + it("surfaces the reason undici hides under cause", () => { + // The reported failure. `fetch failed` alone describes every network + // problem equally badly; the sentence that tells you what to do is one + // level down, and it was being dropped. + const err = new Error("fetch failed", { + cause: new Error("self-signed certificate in certificate chain"), + }); + + expect(describeCauseChain(err)).toBe( + "fetch failed (self-signed certificate in certificate chain)", + ); + }); + + it("includes an errno code when the message does not already carry it", () => { + const inner = Object.assign(new Error("getaddrinfo ENOTFOUND example.invalid"), { + code: "ENOTFOUND", + }); + + // The code is already in the text, so repeating it would be noise. + expect(describeCauseChain(new Error("fetch failed", { cause: inner }))).toBe( + "fetch failed (getaddrinfo ENOTFOUND example.invalid)", + ); + }); + + it("walks more than one level", () => { + const deep = new Error("a", { cause: new Error("b", { cause: new Error("c") }) }); + + expect(describeCauseChain(deep)).toBe("a (b; c)"); + }); + + it("survives a cause cycle rather than hanging", () => { + const a = new Error("a"); + const b = new Error("b", { cause: a }); + (a as { cause?: unknown }).cause = b; + + expect(describeCauseChain(a)).toBe("a (b)"); + }); + + it("returns the plain message when there is no cause", () => { + expect(describeCauseChain(new Error("HTTP 404"))).toBe("HTTP 404"); + }); +}); + +describe("fetchItemFile retries", () => { + const item = { name: "blur-in", type: "hyperframes:component" } as never; + const file = { path: "blur-in.html", target: "compositions/components/blur-in.html" } as never; + const dest = () => join(scratchHome, `dl-${Math.random().toString(36).slice(2)}.html`); + + it("recovers from a transient blip instead of failing the whole install", async () => { + // Item files are the one uncached path, so a single blip used to kill the + // command outright. Two cheap retries is a better trade than that. + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValueOnce(new Error("fetch failed", { cause: new Error("ECONNRESET") })) + .mockResolvedValueOnce({ + ok: true, + status: 200, + arrayBuffer: async () => new TextEncoder().encode("
ok
").buffer, + } as unknown as Response); + + await expect(fetchItemFile(item, file, dest(), DEFAULT_REGISTRY_URL)).resolves.toBeUndefined(); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it("does not retry a certificate failure, which fails identically every time", async () => { + // The reported case: a private registry with a self-signed certificate. + // Retrying only makes the user wait three times as long for one answer. + const fetchSpy = vi.spyOn(globalThis, "fetch").mockRejectedValue( + new Error("fetch failed", { + cause: new Error("self-signed certificate in certificate chain"), + }), + ); + + await expect(fetchItemFile(item, file, dest(), DEFAULT_REGISTRY_URL)).rejects.toThrow( + /self-signed certificate/, + ); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it("names the URL it could not reach", async () => { + vi.spyOn(globalThis, "fetch").mockRejectedValue( + new Error("fetch failed", { cause: new Error("self-signed certificate in chain") }), + ); + + await expect( + fetchItemFile(item, file, dest(), "https://private.example/registry"), + ).rejects.toThrow(/https:\/\/private\.example\/registry\/components\/blur-in\/blur-in\.html/); + }); + + it("gives up after a bounded number of attempts", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValue(new Error("fetch failed", { cause: new Error("ECONNRESET") })); + + await expect(fetchItemFile(item, file, dest(), DEFAULT_REGISTRY_URL)).rejects.toThrow(); + expect(fetchSpy).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/cli/src/registry/remote.ts b/packages/cli/src/registry/remote.ts index 6bc0f3b1c4..76a7604794 100644 --- a/packages/cli/src/registry/remote.ts +++ b/packages/cli/src/registry/remote.ts @@ -150,6 +150,72 @@ export async function fetchItemManifest( } } +/** + * Flatten an error and its `cause` chain into one readable line. + * + * `fetch failed` on its own is useless. `fetch failed (self-signed certificate + * in certificate chain)` tells the reader exactly which knob to turn, and that + * string only exists one or two levels down the chain. + */ +export function describeCauseChain(err: unknown): string { + const parts: string[] = []; + let current: unknown = err; + const seen = new Set(); + while (current instanceof Error && !seen.has(current)) { + seen.add(current); + const code = (current as { code?: unknown }).code; + const text = + code && !current.message.includes(String(code)) + ? `${current.message} [${String(code)}]` + : current.message; + if (text && !parts.includes(text)) parts.push(text); + current = current.cause; + } + if (parts.length === 0) return String(err); + const [head, ...rest] = parts; + return rest.length ? `${head} (${rest.join("; ")})` : head!; +} + +/** + * A transient failure worth trying again, as opposed to a settled answer. + * + * A refused connection, a reset socket or a DNS hiccup usually clears on the + * next attempt. A TLS failure does not: a self-signed certificate on a private + * registry fails identically every time, and retrying it only makes the user + * wait three times as long for the same message. + */ +function isRetryableTransport(err: unknown): boolean { + const text = describeCauseChain(err).toLowerCase(); + if (/certificate|self-signed|self signed|unable to verify|altname|ssl|tls/.test(text)) { + return false; + } + return /fetch failed|econnreset|econnrefused|etimedout|eai_again|socket hang up|timeouterror|aborted|network/.test( + text, + ); +} + +/** + * Item files are the one uncached path: manifests fall back to a stale copy, + * but every install downloads its files fresh. That made a single blip fatal to + * the whole command, which is a bad trade for two extra attempts costing under + * a second when the network is healthy. + */ +async function fetchWithRetry(url: string, attempts = 3): Promise { + let lastErr: unknown; + for (let attempt = 1; attempt <= attempts; attempt++) { + try { + return await fetch(url, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); + } catch (err) { + lastErr = err; + if (attempt === attempts || !isRetryableTransport(err)) break; + // Short, bounded backoff. Long enough to clear a blip, short enough that + // a genuinely offline machine still fails promptly. + await new Promise((resolve) => setTimeout(resolve, 150 * attempt)); + } + } + throw lastErr; +} + /** * Download a single file referenced by an item to a local destination. * Caller is responsible for target-path validation (see installer.ts). @@ -165,7 +231,17 @@ export async function fetchItemFile( throw new Error(`Unsafe file.path "${file.path}": path segments may not contain "..".`); } const url = `${baseUrl}/${ITEM_TYPE_DIRS[item.type]}/${item.name}/${file.path}`; - const res = await fetch(url, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); + let res: Response; + try { + res = await fetchWithRetry(url); + } catch (err) { + // undici throws a bare "fetch failed" and buries the real reason in + // `cause`, sometimes a level deeper again. That is where the diagnosis + // lives: a self-signed certificate on a private registry, a DNS failure, a + // refused connection. Without it the message is two words that describe + // every possible network problem equally badly. + throw new Error(`File fetch failed: ${url} — ${describeCauseChain(err)}`, { cause: err }); + } if (!res.ok) { throw new Error(`File fetch failed: ${url} — HTTP ${res.status}`); } diff --git a/packages/cli/src/registry/variableDefaults.test.ts b/packages/cli/src/registry/variableDefaults.test.ts index 2ea19df874..04b07b1553 100644 --- a/packages/cli/src/registry/variableDefaults.test.ts +++ b/packages/cli/src/registry/variableDefaults.test.ts @@ -6,8 +6,8 @@ import { applyVariableDefaults } from "./variableDefaults.js"; const COMPONENT = `
Design @@ -74,8 +74,8 @@ describe("applyVariableDefaults", () => { it("escapes a value containing the attribute's own delimiter", () => { const withText = COMPONENT.replace( - '{ "id": "size", "type": "number", "role": "style", "default": 52, "min": 24, "max": 120 }', - '{ "id": "label", "type": "string", "default": "hi" }', + '{ "id": "size", "type": "number", "role": "style", "label": "Size", "default": 52, "min": 24, "max": 120 }', + '{ "id": "label", "type": "string", "label": "Label", "default": "hi" }', ); const r = applyVariableDefaults(withText, { label: "it's fine" }); diff --git a/packages/cli/src/registry/variableDefaults.ts b/packages/cli/src/registry/variableDefaults.ts index 1d1e781c94..6b1b631afd 100644 --- a/packages/cli/src/registry/variableDefaults.ts +++ b/packages/cli/src/registry/variableDefaults.ts @@ -16,14 +16,7 @@ * silently discarded for every component in the catalog. */ -interface VariableDeclaration { - id?: unknown; - type?: unknown; - default?: unknown; - options?: unknown; - min?: unknown; - max?: unknown; -} +import { isCompositionVariable, type CompositionVariable } from "@hyperframes/core/variables"; export interface ApplyResult { /** The source with defaults rewritten. Unchanged when nothing applied. */ @@ -59,13 +52,8 @@ function encode(json: string, quote: string): string { return quote === "'" ? json.replace(/'/g, "'") : json.replace(/"/g, """); } -function optionValues(decl: VariableDeclaration): string[] | null { - if (!Array.isArray(decl.options)) return null; - return decl.options.map((o) => - o && typeof o === "object" && "value" in o - ? String((o as { value: unknown }).value) - : String(o), - ); +function optionValues(decl: CompositionVariable): string[] | null { + return decl.type === "enum" ? decl.options.map((option) => option.value) : null; } /** @@ -75,22 +63,22 @@ function optionValues(decl: VariableDeclaration): string[] | null { * here would produce a file that renders as if the value had been ignored -- * which is the exact failure this function exists to remove. */ -function rejectEnum(decl: VariableDeclaration, value: unknown): string | null { - const opts = optionValues(decl); - if (!opts) return null; - return opts.includes(String(value)) ? null : `not one of ${opts.join(", ")}`; +function rejectEnum(decl: CompositionVariable, value: unknown): string | null { + const options = optionValues(decl); + if (!options) return null; + return options.includes(String(value)) ? null : `not one of ${options.join(", ")}`; } -function rejectNumber(decl: VariableDeclaration, value: unknown): string | null { +function rejectNumber(decl: CompositionVariable, value: unknown): string | null { if (decl.type !== "number") return null; const n = typeof value === "number" ? value : Number(value); if (!Number.isFinite(n)) return "not a number"; - if (typeof decl.min === "number" && n < decl.min) return `below min ${decl.min}`; - if (typeof decl.max === "number" && n > decl.max) return `above max ${decl.max}`; + if (decl.min !== undefined && n < decl.min) return `below min ${decl.min}`; + if (decl.max !== undefined && n > decl.max) return `above max ${decl.max}`; return null; } -function reject(decl: VariableDeclaration, value: unknown): string | null { +function reject(decl: CompositionVariable, value: unknown): string | null { return rejectEnum(decl, value) ?? rejectNumber(decl, value); } @@ -104,19 +92,31 @@ export function applyVariableDefaults( const found = findDeclaration(source); if (!found) return { html: source, applied: [], unknown: ids, invalid: [] }; - let declared: VariableDeclaration[]; + // isCompositionVariable is the predicate parseCompositionVariables filters + // with -- the schema's own definition of a well-formed declaration. Using it + // here means everything below works on a real discriminated union instead of + // a bag of `unknown` re-checked at each use, and a declaration the schema + // rejects is one we must not rewrite, because we would be guessing at its + // shape. parseCompositionVariables itself takes a DOM Element, which the CLI + // has no business constructing to read a string. + let parsed: unknown; try { - const parsed: unknown = JSON.parse(decode(found.raw)); - if (!Array.isArray(parsed)) return { html: source, applied: [], unknown: ids, invalid: [] }; - declared = parsed as VariableDeclaration[]; + parsed = JSON.parse(decode(found.raw)); } catch { - // A declaration we cannot parse is one we must not rewrite. + return { html: source, applied: [], unknown: ids, invalid: [] }; + } + if (!Array.isArray(parsed)) return { html: source, applied: [], unknown: ids, invalid: [] }; + const declared: CompositionVariable[] = parsed.filter(isCompositionVariable); + if (declared.length !== parsed.length) { + // Rewriting a partially understood declaration would drop the entries we + // could not model, so leave the file exactly as the registry shipped it. return { html: source, applied: [], unknown: ids, invalid: [] }; } const applied: string[] = []; const invalid: { id: string; reason: string }[] = []; - const byId = new Map(declared.map((d) => [String(d.id), d])); + const byId = new Map(declared.map((decl) => [decl.id, decl])); + const updated = new Map(); for (const [id, value] of Object.entries(values)) { const decl = byId.get(id); @@ -126,7 +126,10 @@ export function applyVariableDefaults( invalid.push({ id, reason }); continue; } - decl.default = decl.type === "number" ? Number(value) : value; + // The declaration's own type decides how the value is stored. A number + // written as the string "76" would trip the composition's guard and fall + // back, which looks exactly like the value being ignored. + updated.set(id, decl.type === "number" ? Number(value) : String(value)); applied.push(id); } @@ -136,7 +139,12 @@ export function applyVariableDefaults( // One declaration per line, matching how the registry authors these files, so // a re-install produces a readable diff rather than one enormous line. const quote = source[found.start - 1]!; - const body = declared.map((d) => ` ${JSON.stringify(d)}`).join(",\n"); + const body = declared + .map((decl) => { + const next = updated.has(decl.id) ? { ...decl, default: updated.get(decl.id)! } : decl; + return ` ${JSON.stringify(next)}`; + }) + .join(",\n"); const rewritten = encode(`[\n${body}\n ]`, quote); const html = source.slice(0, found.start) + rewritten + source.slice(found.end); return { html, applied, unknown, invalid }; From a7f0cd64381473c4979bfb064d48bdc82e570a86 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 17 Aug 2026 18:50:21 -0400 Subject: [PATCH 4/5] fix(registry): name the registry on the not-found path too The item-file failure now names the host it could not reach, but the sibling path did not. A project whose registry is unreachable at the MANIFEST stage got `Item "blur-in" not found - registry unreachable or empty`, which reads as the public catalog having lost the item and sends the reader to search a registry that never saw the request. Same fix, same reason, applied where the other three call sites live so one of them cannot stay behind: the message names the host and says it came from this project's hyperframes.json, and only when it is not the public registry, so the common case stays short. Test plan: 3 tests covering the private-registry hint and its absence on the default registry and on no registry at all. CLI suite 2674 passed, zero failures. Verified with the built dist against a host with a bad certificate: Item "blur-in" not found - registry unreachable or empty. Contacted https://self-signed.badssl.com/registry, set by this project's hyperframes.json, not the public registry. --- packages/cli/src/registry/resolver.test.ts | 28 ++++++++++++++++++++++ packages/cli/src/registry/resolver.ts | 20 +++++++++++++--- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/registry/resolver.test.ts b/packages/cli/src/registry/resolver.test.ts index e9b7d46a76..d8f6b2611e 100644 --- a/packages/cli/src/registry/resolver.test.ts +++ b/packages/cli/src/registry/resolver.test.ts @@ -5,6 +5,7 @@ import { loadAllItems, resolveItem, resolveItemWithDependencies, + unreachableRegistryMessage, } from "./resolver.js"; const MANIFEST: RegistryManifest = { @@ -205,3 +206,30 @@ describe("registry resolver", () => { }); }); }); + +describe("unreachableRegistryMessage", () => { + it("names a private registry, so the reader looks at the right host", () => { + // Same dead end the item-file failure used to be: without the host, a + // project that set `registry` in hyperframes.json reads this as the public + // catalog having lost the item. + const message = unreachableRegistryMessage("blur-in", "https://private.example/registry"); + + expect(message).toContain("https://private.example/registry"); + expect(message).toContain("hyperframes.json"); + }); + + it("stays quiet when the registry is the public one", () => { + const message = unreachableRegistryMessage( + "blur-in", + "https://raw.githubusercontent.com/heygen-com/hyperframes/main/registry", + ); + + expect(message).toBe('Item "blur-in" not found \u2014 registry unreachable or empty.'); + }); + + it("stays quiet when no registry was supplied at all", () => { + expect(unreachableRegistryMessage("blur-in")).toBe( + 'Item "blur-in" not found \u2014 registry unreachable or empty.', + ); + }); +}); diff --git a/packages/cli/src/registry/resolver.ts b/packages/cli/src/registry/resolver.ts index 8ba98749a8..a41502198c 100644 --- a/packages/cli/src/registry/resolver.ts +++ b/packages/cli/src/registry/resolver.ts @@ -91,11 +91,25 @@ export async function resolveItem( } const item = items[items.length - 1]; if (!item) { - throw new Error(`Item "${name}" not found — registry unreachable or empty.`); + throw new Error(unreachableRegistryMessage(name, options.baseUrl)); } return item; } +/** + * "registry unreachable or empty" without saying WHICH registry is the same + * dead end the item-file failure used to be: a project that sets `registry` in + * hyperframes.json reads it as the public catalog having lost the item, and + * goes looking in the wrong place. Naming the host is the diagnosis. + */ +export function unreachableRegistryMessage(name: string, baseUrl?: string): string { + const where = + baseUrl && !baseUrl.startsWith(DEFAULT_REGISTRY_URL) + ? ` Contacted ${baseUrl}, set by this project's hyperframes.json, not the public registry.` + : ""; + return `Item "${name}" not found — registry unreachable or empty.${where}`; +} + /** * Resolve an item and all of its transitive `registryDependencies` in * topological order — dependencies first, the requested item last — so callers @@ -122,7 +136,7 @@ export async function resolveItemWithDependencies( throw new Error( available.length > 0 ? `Item "${name}" not found in registry. Available: ${available}` - : `Item "${name}" not found — registry unreachable or empty.`, + : unreachableRegistryMessage(name, options.baseUrl), ); } @@ -146,7 +160,7 @@ export async function resolveItemWithDependencies( throw new Error( available.length > 0 ? `Dependency "${itemName}" not found in registry. Available: ${available}` - : `Dependency "${itemName}" not found — registry unreachable or empty.`, + : unreachableRegistryMessage(itemName, options.baseUrl), ); } From c9194f763f53d6ce78d69e4396b6d1029dd0ad57 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 17 Aug 2026 18:59:39 -0400 Subject: [PATCH 5/5] fix(catalog): reconcile the two spellings of a compound word `countdown` returned exactly one item, the only thing tagged with that spelling. `count down timer` returned sixteen, and that one was in none of them. The tokenizer splits on word boundaries, so the two spellings of a single idea produced disjoint sets, and whichever phrasing an author happened to type decided which half of the answer they saw. Neither half was the whole answer: the one-word spelling hid count-up and decline-chart, which are the two things you would actually build with. Both directions now, each gated on the catalog's own vocabulary so this can only add signal. A query token is split when both halves are words the catalog uses, and adjacent tokens are joined when the compound is. A word in neither form, like `timer` which appears in no item, is left alone: this widens phrasing, it does not invent matches. Everything inferred this way carries a fraction of a real token's weight. That is the part worth keeping honest, because the first version relied on the halves being statistically common in a 375-item catalog, which is not the same as making them count for less. In a small corpus that version let `type` matching the name of `type-match-cut` outrank `typewriter` matching the name of `typewriter`: searching a word returned something that merely contained half of it. Two tests written against that real failure caught it. All spellings now return the same 17 items, and each still ranks its own exact match first: `countdown` leads with yt-circle-pointer, `count down` leads with the two-word items, and count-up and decline-chart appear in both. Test plan: 6 new tests covering both directions, the identical-set property that was the actual defect, exact-match precedence, an unknown word left alone, and the typewriter case. Eval set unchanged at 33/39 top-1 and 39/39 top-3, so no query regressed. CLI suite 2680 passed. --- packages/cli/src/registry/localSearch.test.ts | 50 ++++++++++++ packages/cli/src/registry/localSearch.ts | 80 ++++++++++++++++++- 2 files changed, 126 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/registry/localSearch.test.ts b/packages/cli/src/registry/localSearch.test.ts index 9cd94f2458..8d31029c01 100644 --- a/packages/cli/src/registry/localSearch.test.ts +++ b/packages/cli/src/registry/localSearch.test.ts @@ -183,3 +183,53 @@ describe("hasNoSearchableTokens", () => { expect(hasNoSearchableTokens("the and of !!!")).toBe(true); }); }); + +describe("the two spellings of a compound word find the same items", () => { + // Reduced from the real failure: `countdown` returned exactly one item (the + // only thing tagged with that spelling) while `count down timer` returned + // sixteen that did not include it. Whichever phrasing an author happened to + // type decided which half of the answer they saw. + const items = [ + named("yt-circle-pointer", "Circle Pointer", "An annotation overlay with a countdown chip."), + named("count-up", "Count Up", "A stat counter that eases between two values."), + named("decline-chart", "Decline Chart", "A line that counts down as its value falls."), + named("aurora-drift", "Aurora Drift", "A slow gradient background."), + ]; + const namesFor = (q: string) => searchByWords(q, items, fieldsOf).map((i) => i.name); + + it("finds the one-word item from the two-word query", () => { + expect(namesFor("count down timer")).toContain("yt-circle-pointer"); + }); + + it("finds the two-word items from the one-word query", () => { + // The half of the answer the compound spelling used to hide. + expect(namesFor("countdown")).toEqual(expect.arrayContaining(["count-up", "decline-chart"])); + }); + + it("returns the same set either way, which is the actual defect", () => { + expect(namesFor("countdown").sort()).toEqual(namesFor("count down").sort()); + }); + + it("still ranks the exact compound match first", () => { + // Splitting must not cost the item that spells it the way you asked. The + // compound is kept and is rare, so its weight survives the added halves. + expect(namesFor("countdown")[0]).toBe("yt-circle-pointer"); + }); + + it("leaves a word the catalog never uses alone", () => { + // `timer` appears in none of these items, and inventing a match for it + // would be widening the query into fiction rather than into phrasing. + expect(namesFor("timer")).toEqual([]); + }); + + it("does not let a split dislodge the item named for the whole word", () => { + // `typewriter` splits into `type` + `writer` if both are known. The item + // literally called typewriter must still win. + const typing = [ + named("typewriter", "Typewriter", "Character-by-character reveal."), + named("type-match-cut", "Type Match Cut", "A cut matched on a writer's type."), + ]; + + expect(searchByWords("typewriter", typing, fieldsOf)[0]?.name).toBe("typewriter"); + }); +}); diff --git a/packages/cli/src/registry/localSearch.ts b/packages/cli/src/registry/localSearch.ts index 7c94b9e7fc..7e1f3148ab 100644 --- a/packages/cli/src/registry/localSearch.ts +++ b/packages/cli/src/registry/localSearch.ts @@ -100,6 +100,71 @@ export interface ItemText { weak: string; } +/** + * Reconcile the two spellings of one compound word. + * + * The tokenizer splits on word boundaries, so `countdown` is one token and + * `count down` is two, and neither can ever match the other. That made the two + * spellings of a single idea return disjoint result sets: `countdown` returned + * only the one item tagged with that exact word, while `count down timer` + * returned sixteen that did not include it. Whichever phrasing an author + * happened to type decided which half of the answer they saw, and neither half + * was the whole answer. + * + * Both directions, and both gated on the catalog's own vocabulary so this can + * only ever add signal: + * + * - A query token is split when both halves are words the catalog actually + * uses. The compound is always kept, so nothing is lost: `countdown` is rare + * and keeps its high inverse-document-frequency weight, while the common + * halves it adds bring in the items written the other way and carry almost + * no weight of their own. That is why splitting `typewriter` into `type` and + * `writer` cannot dislodge the item literally called typewriter. + * - Adjacent query tokens are joined when the compound is a word the catalog + * actually uses, so `count down` also reaches items written `countdown`. + * + * A word in neither form, like `timer` (which appears in none of the catalog's + * items), is left exactly as it was: this widens phrasing, it does not invent + * matches. + * + * Everything added here is INFERRED rather than asked for, so it carries a + * fraction of a real token's weight. Without that the inference can outvote the + * question: `type` matching the name of `type-match-cut` at full strength beats + * `typewriter` matching the name of `typewriter`, and searching a word returns + * something that merely contains half of it. Relying on the halves being + * statistically common in a large catalog is not the same as making them + * count for less, and only one of the two holds when the corpus is small. + */ +const INFERRED_TOKEN_WEIGHT = 0.35; + +function expandCompounds( + want: Map, + order: string[], + vocabulary: Set, +): void { + const infer = (token: string): void => { + if (!want.has(token)) want.set(token, INFERRED_TOKEN_WEIGHT); + }; + + for (const token of order) { + if (token.length < 6) continue; + // Shortest useful part is 3 characters, matching the tokenizer's own floor. + for (let cut = 3; cut <= token.length - 3; cut++) { + const head = token.slice(0, cut); + const tail = token.slice(cut); + if (vocabulary.has(head) && vocabulary.has(tail)) { + infer(head); + infer(tail); + break; + } + } + } + for (let i = 0; i < order.length - 1; i++) { + const joined = `${order[i]}${order[i + 1]}`; + if (vocabulary.has(joined)) infer(joined); + } +} + /** * Rank every item by shared vocabulary, best first. * @@ -111,7 +176,10 @@ export function rankByWords( items: T[], textOf: (item: T) => ItemText, ): Scored[] { - const want = new Set(tokenize(query)); + const asked = tokenize(query); + // Token -> how much a match on it is worth. Asked-for words count fully; + // words inferred from a compound count for a fraction. + const want = new Map(asked.map((token) => [token, 1])); if (want.size === 0) return items.map((item) => ({ item, score: 0 })); const parsed = items.map((item) => { @@ -120,6 +188,10 @@ export function rankByWords( return { item, strongTokens, allTokens: new Set([...strongTokens, ...tokenize(weak)]) }; }); + const vocabulary = new Set(); + for (const entry of parsed) for (const token of entry.allTokens) vocabulary.add(token); + expandCompounds(want, asked, vocabulary); + // How rare each queried word is across the catalog. Without this a common // word carries the same weight as a distinctive one, and field weighting // makes that worse rather than better: searching "reveal a headline one line @@ -127,7 +199,7 @@ export function rankByWords( // strong hit on the catalog's most common word outscored several weak hits // on the words that actually narrowed it down. const idf = new Map(); - for (const token of want) { + for (const token of want.keys()) { const df = parsed.reduce((count, p) => count + (p.allTokens.has(token) ? 1 : 0), 0); // +1 inside the log keeps a token present in every item at a small // positive weight rather than exactly zero: still nearly worthless, but @@ -138,8 +210,8 @@ export function rankByWords( return parsed .map(({ item, strongTokens, allTokens }) => { let shared = 0; - for (const token of want) { - const weight = idf.get(token) ?? 1; + for (const [token, asking] of want) { + const weight = (idf.get(token) ?? 1) * asking; if (strongTokens.has(token)) shared += STRONG_FIELD_WEIGHT * weight; else if (allTokens.has(token)) shared += weight; }