diff --git a/packages/test/src/test/util/ExportBarrelParity.test.ts b/packages/test/src/test/util/ExportBarrelParity.test.ts index fcdd7e797..0835a45bb 100644 --- a/packages/test/src/test/util/ExportBarrelParity.test.ts +++ b/packages/test/src/test/util/ExportBarrelParity.test.ts @@ -10,9 +10,10 @@ import { fileURLToPath } from "node:url"; import { describe, expect, it } from "vitest"; /** - * A provider whose `src/ai/index.ts` and `src/ai/index.browser.ts` are two - * hand-maintained barrels over ONE set of modules must export the same surface - * from both, except where the difference is deliberate and stated here. + * Any `.browser.ts` under `providers//src/` and the `.ts` + * beside it are two hand-maintained views of ONE set of modules, and must + * export the same surface, except where the difference is deliberate and + * stated here. * * `ExportTypesPairing.test.ts` proves the manifest routes a browser consumer to * the browser declarations. That is what turns a barrel omission from invisible @@ -23,8 +24,14 @@ import { describe, expect, it } from "vitest"; * compiled into the browser bundle through the runtime entry, missing only its * `export *` line. * - * This guard is deliberately source-only: it reads the two barrels as text, so - * it runs under `use-source` and needs no `dist`. + * The scan is recursive but stays scoped to `providers/`, deliberately. Adding + * `packages/` pulls in `packages/tasks/src/task/image/imageTextRender.browser.ts`, + * a genuine implementation split whose browser file exports one factory against + * the node module's fifteen names — an exemption of fifteen entries buying + * nothing, since the provider tree is where the two-barrel convention lives. + * + * This guard is deliberately source-only: it reads the files as text, so it + * runs under `use-source` and needs no `dist`. */ /** The exported names of one top-level statement, as a stable comparable key. */ @@ -106,6 +113,24 @@ function memberNames(clause: string): string[] { }); } +/** + * A specifier, with a trailing `.browser` removed. + * + * The browser half of a pair names the browser half of every module it + * re-exports, so without this every runtime pair reports as pure drift + * (`* from "./common/Ollama_Client"` against + * `* from "./common/Ollama_Client.browser"`) and the guard says nothing. + * + * The cost is that a star-export comparison across a `.browser` sibling becomes + * NOMINAL — it asserts the two barrels name the same module NAME, not the same + * names. The recursive scan is what closes that: `Ollama_Client.browser.ts` and + * `Ollama_Client.ts` are themselves a compared pair now, so the surface behind + * the specifier is checked one level down. + */ +function normalizeSpecifier(specifier: string): string { + return specifier.replace(/\.browser$/, ""); +} + /** * The surface one barrel exports, as one entry per importable name. * @@ -120,12 +145,12 @@ function parseBarrel(text: string): ParsedBarrel { for (const statement of topLevelExportStatements(text)) { const star = /^export\s+(?:type\s+)?\*\s+from\s+"([^"]+)"/.exec(statement); if (star !== null) { - surface.add(`* from "${star[1]}"`); + surface.add(`* from "${normalizeSpecifier(star[1] as string)}"`); continue; } const named = /^export\s+(?:type\s+)?\{([\s\S]*?)\}\s*(?:from\s+"([^"]+)")?/.exec(statement); if (named !== null) { - const from = named[2] === undefined ? "" : ` from "${named[2]}"`; + const from = named[2] === undefined ? "" : ` from "${normalizeSpecifier(named[2])}"`; for (const name of memberNames(named[1] as string)) surface.add(`${name}${from}`); continue; } @@ -155,13 +180,17 @@ function parseBarrel(text: string): ParsedBarrel { * Anything else appearing here is a real asymmetry: the modules behind these * barrels carry no `node:` imports, and each omitted module was already in the * browser bundle via the runtime entry — only its `export *` was missing. + * + * Keyed by the BROWSER file path, not the package: a package contributes as + * many pairs as it has `.browser.ts` siblings, and an exemption belongs to the + * one file that earns it. */ const INTENTIONAL_NODE_ONLY: ReadonlyMap = new Map([ - ["providers/deepseek", ["_testOnly"]], - ["providers/ollama", ["_testOnly"]], - ["providers/openai", ["_testOnly"]], - ["providers/openrouter", ["_testOnly"]], - ["providers/xai", ["_testOnly"]], + ["providers/deepseek/src/ai/index.browser.ts", ["_testOnly"]], + ["providers/ollama/src/ai/index.browser.ts", ["_testOnly"]], + ["providers/openai/src/ai/index.browser.ts", ["_testOnly"]], + ["providers/openrouter/src/ai/index.browser.ts", ["_testOnly"]], + ["providers/xai/src/ai/index.browser.ts", ["_testOnly"]], ]); interface BarrelPair { @@ -185,22 +214,55 @@ function repoRoot(): string { throw new Error("could not locate the workspace root from " + import.meta.url); } +/** Every `.browser.ts?` under `dir`, recursively, as paths relative to `root`. */ +function browserFilesUnder(root: string, dir: string, out: string[]): void { + for (const entry of readdirSync(join(root, dir), { withFileTypes: true })) { + if (entry.name === "node_modules" || entry.name === "dist") continue; + if (entry.isDirectory()) { + browserFilesUnder(root, `${dir}/${entry.name}`, out); + continue; + } + if (/\.browser\.tsx?$/.test(entry.name)) out.push(`${dir}/${entry.name}`); + } +} + +/** + * Whether the browser file is nothing but a re-export of its node sibling. + * + * That shape is in parity by construction — it IS the node surface — and it is + * the shape this convention wants where a package has no platform-specific + * source (`providers/llamacpp-server`, `providers/stable-diffusion-server`). + * Comparing it would report the node file's every other statement as drift. + */ +function isSiblingReExport(browser: ParsedBarrel, nodeStem: string): boolean { + return browser.surface.size === 1 && browser.surface.has(`* from "./${nodeStem}"`); +} + function barrelPairs(root: string): BarrelPair[] { const pairs: BarrelPair[] = []; const providersDir = join(root, "providers"); for (const entry of readdirSync(providersDir, { withFileTypes: true })) { if (!entry.isDirectory()) continue; const packageDir = `providers/${entry.name}`; - const nodePath = `${packageDir}/src/ai/index.ts`; - const browserPath = `${packageDir}/src/ai/index.browser.ts`; - if (!existsSync(join(root, nodePath)) || !existsSync(join(root, browserPath))) continue; - pairs.push({ - packageDir, - nodePath, - browserPath, - node: parseBarrel(readFileSync(join(root, nodePath), "utf8")), - browser: parseBarrel(readFileSync(join(root, browserPath), "utf8")), - }); + if (!existsSync(join(root, packageDir, "src"))) continue; + const browserFiles: string[] = []; + browserFilesUnder(root, `${packageDir}/src`, browserFiles); + for (const browserPath of browserFiles) { + const stem = /^(.+)\.browser\.(tsx?)$/.exec(browserPath); + if (stem === null) continue; + const nodePath = `${stem[1]}.${stem[2]}`; + if (!existsSync(join(root, nodePath))) continue; + const browser = parseBarrel(readFileSync(join(root, browserPath), "utf8")); + const nodeStem = (nodePath.split("/").pop() as string).replace(/\.tsx?$/, ""); + if (isSiblingReExport(browser, nodeStem)) continue; + pairs.push({ + packageDir, + nodePath, + browserPath, + node: parseBarrel(readFileSync(join(root, nodePath), "utf8")), + browser, + }); + } } return pairs; } @@ -215,9 +277,12 @@ describe("provider ai barrels", () => { const pairs = barrelPairs(root); it("finds the barrel pairs to check", () => { - // Vacuous-pass guard: the scan keying on a path that no longer exists would - // otherwise turn this whole file into a no-op. - expect(pairs.map((pair) => pair.packageDir).sort()).toEqual([...INTENTIONAL_NODE_ONLY.keys()]); + // Vacuous-pass guard, in two parts: the scan reached the provider tree, and + // it recursed into it. Deliberately NOT an equality against the pinned + // keys — a pin is an exemption, so requiring one per pair would mean a + // correct new provider fails until somebody registers it as needing none. + expect(pairs.length).toBeGreaterThan(40); + expect(pairs.some((pair) => pair.browserPath.includes("/src/ai/"))).toBe(true); }); it("parses every top-level export in both barrels", () => { @@ -231,14 +296,11 @@ describe("provider ai barrels", () => { it("exports the same surface from both barrels apart from the pinned node-only names", () => { const drift: string[] = []; for (const pair of pairs) { - const expected = [...(INTENTIONAL_NODE_ONLY.get(pair.packageDir) ?? [])].sort(); + const expected = [...(INTENTIONAL_NODE_ONLY.get(pair.browserPath) ?? [])].sort(); const actual = nodeOnly(pair); const extra = actual.filter((name) => !expected.includes(name)); if (extra.length === 0) continue; - drift.push( - `${pair.packageDir}: ${pair.nodePath} exports ${extra.join(", ")} but ` + - `${pair.browserPath} does not` - ); + drift.push(`${pair.nodePath} exports ${extra.join(", ")} but ${pair.browserPath} does not`); } expect(drift).toEqual([]); }); @@ -249,14 +311,14 @@ describe("provider ai barrels", () => { // asked for, and hides the next omission behind an entry that reads as // deliberate. const stale: string[] = []; - for (const [packageDir, names] of INTENTIONAL_NODE_ONLY) { - const pair = pairs.find((candidate) => candidate.packageDir === packageDir); - expect(pair, `${packageDir} is pinned but has no barrel pair`).toBeDefined(); + for (const [browserPath, names] of INTENTIONAL_NODE_ONLY) { + const pair = pairs.find((candidate) => candidate.browserPath === browserPath); + expect(pair, `${browserPath} is pinned but has no barrel pair`).toBeDefined(); if (pair === undefined) continue; const actual = nodeOnly(pair); for (const name of names) { if (actual.includes(name)) continue; - stale.push(`${packageDir}: "${name}" is pinned node-only but both barrels export it`); + stale.push(`${browserPath}: "${name}" is pinned node-only but both barrels export it`); } } expect(stale).toEqual([]); @@ -313,4 +375,41 @@ describe("barrel parsing", () => { ); expect([...surface]).toEqual(['* from "./common/Here"']); }); + + it("keys a `.browser` sibling under the same specifier as its node peer", () => { + // Without this the two halves of every runtime pair agree on nothing and + // the guard reports each one as total drift. + const node = parseBarrel('export * from "./common/Ollama_Client";\n'); + const browser = parseBarrel('export * from "./common/Ollama_Client.browser";\n'); + expect([...browser.surface]).toEqual([...node.surface]); + }); + + it("normalizes a `.browser` specifier on a named re-export too", () => { + const { surface } = parseBarrel('export { createClient } from "./common/Client.browser";\n'); + expect([...surface]).toEqual(['createClient from "./common/Client"']); + }); + + it("does not strip `.browser` from the middle of a specifier", () => { + const { surface } = parseBarrel('export * from "./common/Client.browser.impl";\n'); + expect([...surface]).toEqual(['* from "./common/Client.browser.impl"']); + }); +}); + +describe("sibling re-export detection", () => { + it("recognizes a browser file that is only a re-export of its node peer", () => { + expect(isSiblingReExport(parseBarrel('export * from "./ai-runtime";\n'), "ai-runtime")).toBe( + true + ); + }); + + it("does not treat a barrel that also re-exports other modules as one", () => { + const browser = parseBarrel( + ['export * from "./ai";', 'export * from "./common/Extra";'].join("\n") + ); + expect(isSiblingReExport(browser, "ai")).toBe(false); + }); + + it("does not treat a re-export of a different module as one", () => { + expect(isSiblingReExport(parseBarrel('export * from "./ai/index";\n'), "ai")).toBe(false); + }); }); diff --git a/packages/test/src/test/util/ExportTypesPairing.test.ts b/packages/test/src/test/util/ExportTypesPairing.test.ts index 630bc7852..61b513c1e 100644 --- a/packages/test/src/test/util/ExportTypesPairing.test.ts +++ b/packages/test/src/test/util/ExportTypesPairing.test.ts @@ -369,7 +369,8 @@ function browserSplitViolations( } /** - * One `src/.browser.ts` and the `src/.ts` beside it, as text. + * One `.browser.ts` and the `.ts` beside it, as text — anywhere + * under `src`, not only at the entry layer. * * Injected rather than read inside the rule so the fixtures below can state * both halves, the same way {@link browserSplitViolations} takes its probe. @@ -402,11 +403,16 @@ function specifiersIn(text: string): string[] { } /** - * Browser entries that are a copy of the node entry beside them AND name only + * Browser modules that are a copy of the node module beside them AND name only * relative specifiers. * - * **The relative/bare distinction is the entire rule.** Two identical entry - * files mean opposite things depending on what they import: + * The rule reads every `.browser.ts` under `src`, not just the entry + * layer the manifests name: a nominal split is the same defect one directory + * down (`providers/openrouter/src/ai/runtime.browser.ts` was a byte copy of + * `runtime.ts`), and the entry-only scan could not see it. + * + * **The relative/bare distinction is the entire rule.** Two identical files + * mean opposite things depending on what they import: * * - A **relative** specifier (`"./ai/index"`) is resolved once, by the file's * own path, and nothing in this toolchain substitutes `X.browser.ts` for @@ -806,32 +812,47 @@ describe("workspace exports maps", () => { } }); - it("re-exports rather than duplicates a browser entry that has no split", () => { - // Collected from `src/*.browser.ts` — the ENTRY layer, which is what the - // manifests above name. A `.browser.ts` with no `.ts` beside it (e.g. - // `packages/tasks/src/codec.browser.ts`) is not an entry pair and has - // nothing to be a duplicate of. + it("re-exports rather than duplicates a browser module that has no split", () => { + // Collected recursively from `src`, not just the entry layer the manifests + // name: a nominal split reads the same one directory down, and the flat + // scan could not see it. A `.browser.ts` with no `.ts` beside it (e.g. + // `packages/tasks/src/codec.browser.ts`) is not a pair and has nothing to + // be a duplicate of. const pairs: EntryPair[] = []; - for (const manifest of manifests) { - const srcDir = join(root.dir, dirname(manifest.relative), "src"); - if (!existsSync(srcDir)) continue; - for (const file of readdirSync(srcDir)) { - const stem = /^(.+)\.browser\.(tsx?)$/.exec(file); + const collect = (packageDir: string, relative: string): void => { + for (const entry of readdirSync(join(root.dir, packageDir, relative), { + withFileTypes: true, + })) { + if (entry.name === "node_modules" || entry.name === "dist") continue; + if (entry.isDirectory()) { + collect(packageDir, `${relative}/${entry.name}`); + continue; + } + const stem = /^(.+)\.browser\.(tsx?)$/.exec(entry.name); if (stem === null) continue; const nodeFile = `${stem[1]}.${stem[2]}`; - if (!existsSync(join(srcDir, nodeFile))) continue; + const dir = join(root.dir, packageDir, relative); + if (!existsSync(join(dir, nodeFile))) continue; pairs.push({ - browserPath: `${dirname(manifest.relative)}/src/${file}`, - nodePath: `${dirname(manifest.relative)}/src/${nodeFile}`, - browserText: readFileSync(join(srcDir, file), "utf8"), - nodeText: readFileSync(join(srcDir, nodeFile), "utf8"), + browserPath: `${packageDir}/${relative}/${entry.name}`, + nodePath: `${packageDir}/${relative}/${nodeFile}`, + browserText: readFileSync(join(dir, entry.name), "utf8"), + nodeText: readFileSync(join(dir, nodeFile), "utf8"), }); } + }; + for (const manifest of manifests) { + const packageDir = dirname(manifest.relative); + if (!existsSync(join(root.dir, packageDir, "src"))) continue; + collect(packageDir, "src"); } // The nine `packages/workglow` shims are pairs, and correctly identical — // they are the negative case the rule has to keep passing, so a scan that // found nothing would prove nothing. expect(pairs.length).toBeGreaterThan(9); + // And the scan recursed: a regression to the flat `src` listing loses every + // pair below the entry layer, silently. + expect(pairs.some((pair) => pair.browserPath.includes("/src/ai/"))).toBe(true); expect(duplicateBrowserEntryViolations(pairs)).toEqual([]); }); diff --git a/providers/ollama/src/ai/runtime.browser.ts b/providers/ollama/src/ai/runtime.browser.ts index 59877518a..6ef9f763a 100644 --- a/providers/ollama/src/ai/runtime.browser.ts +++ b/providers/ollama/src/ai/runtime.browser.ts @@ -14,5 +14,7 @@ // organize-imports-ignore export * from "./common/Ollama_Client.browser"; +export * from "./common/Ollama_TextGeneration"; +export * from "./common/Ollama_StructuredGeneration"; export * from "./registerOllamaInline.browser"; export * from "./registerOllamaWorker.browser"; diff --git a/providers/openrouter/src/ai/runtime.browser.ts b/providers/openrouter/src/ai/runtime.browser.ts index 59cd395db..6209b4d49 100644 --- a/providers/openrouter/src/ai/runtime.browser.ts +++ b/providers/openrouter/src/ai/runtime.browser.ts @@ -6,7 +6,10 @@ // organize-imports-ignore -export * from "./common/OpenRouter_Client"; -export * from "./common/OpenRouter_EffortPolicy"; -export * from "./registerOpenRouterInline"; -export * from "./registerOpenRouterWorker"; +// There is no platform-specific runtime source here: the browser bundle is the +// same graph compiled `--target=browser`. Re-exporting the node barrel is what +// stops the two declarations drifting apart. Safe because the specifier is +// RELATIVE — resolved once, identically under either condition, so a +// hand-maintained copy of this file could only ever drift from the module it +// already resolves to. +export * from "./runtime";