diff --git a/e2e_tests/tests_omni_light/fixtures/modules.json b/e2e_tests/tests_omni_light/fixtures/modules.json new file mode 100644 index 0000000..b4a7465 --- /dev/null +++ b/e2e_tests/tests_omni_light/fixtures/modules.json @@ -0,0 +1,19 @@ +{ + "version": "1.0.0", + "includes": [{ "path": "modules_browser_only.json" }], + "modules": [ + { + "id": "external-test-hello-world-route", + "type": "Routes", + "path": "@/modules/external-module-test/helloWorldRouteDefinition/create", + "dependencies": {} + }, + { + "id": "routing-main", + "collisionStrategy": "merge", + "dependencies": { + "module:routes": ["external-test-hello-world-route"] + } + } + ] +} diff --git a/e2e_tests/tests_omni_light/scripts/extend-light-modules-json.mjs b/e2e_tests/tests_omni_light/scripts/extend-light-modules-json.mjs deleted file mode 100644 index d251727..0000000 --- a/e2e_tests/tests_omni_light/scripts/extend-light-modules-json.mjs +++ /dev/null @@ -1,53 +0,0 @@ -import { readFile, writeFile } from "node:fs/promises"; -import path from "node:path"; - -const frontendRoot = process.cwd(); -const sourceManifestPath = path.join( - frontendRoot, - "public", - "modules_browser_only.json", -); -const targetManifestPath = path.join(frontendRoot, "public", "modules.json"); - -const content = await readFile(sourceManifestPath, "utf8"); -const manifest = JSON.parse(content); - -if (!Array.isArray(manifest.modules)) { - throw new Error( - "Expected modules_browser_only.json to contain a modules array", - ); -} - -const externalRouteEntry = { - id: "external-test-hello-world-route", - path: "@/modules/external-module-test/helloWorldRouteDefinition/create", - dependencies: {}, -}; - -const hasModuleId = (id) => manifest.modules.some((module) => module.id === id); - -if (!hasModuleId(externalRouteEntry.id)) { - manifest.modules.push(externalRouteEntry); -} - -const routingMain = manifest.modules.find( - (module) => module.id === "routing-main", -); -if (!routingMain) { - throw new Error("routing-main module not found in modules_browser_only.json"); -} - -const routes = routingMain.dependencies?.["module:routes"]; -if (!Array.isArray(routes)) { - throw new Error("routing-main.module:routes must be an array"); -} - -if (!routes.includes(externalRouteEntry.id)) { - routes.push(externalRouteEntry.id); -} - -await writeFile( - targetManifestPath, - `${JSON.stringify(manifest, null, 2)}\n`, - "utf8", -); diff --git a/e2e_tests/tests_omni_light/scripts/run-frontend.sh b/e2e_tests/tests_omni_light/scripts/run-frontend.sh index a83dff4..ae57d68 100755 --- a/e2e_tests/tests_omni_light/scripts/run-frontend.sh +++ b/e2e_tests/tests_omni_light/scripts/run-frontend.sh @@ -1,8 +1,9 @@ #!/usr/bin/env bash # Builds and starts the Vite preview server for e2e light tests. # -# Uses frontend/omni browser-only manifest and extends it at runtime with -# external test module entries. +# Uses frontend/omni browser-only manifest composed with the test fixtures +# modules.json (which includes modules_browser_only.json and adds the external +# test module entries via the composition system). set -euo pipefail @@ -16,8 +17,7 @@ cd "$FRONTEND_DIR" rm -rf src/modules/external-module-test cp -R "$LIGHT_TESTS_DIR/fixtures/external-module-test" src/modules/external-module-test -rm -f public/modules.json -node "$LIGHT_TESTS_DIR/scripts/extend-light-modules-json.mjs" +cp "$LIGHT_TESTS_DIR/fixtures/modules.json" public/modules.json pnpm install pnpm build diff --git a/e2e_tests/tests_omni_light/src/chat.spec.ts b/e2e_tests/tests_omni_light/src/chat.spec.ts index 15d624b..1ebdfbd 100644 --- a/e2e_tests/tests_omni_light/src/chat.spec.ts +++ b/e2e_tests/tests_omni_light/src/chat.spec.ts @@ -48,4 +48,26 @@ test.describe("Chat", () => { await chatPage.sendMessage("world"); await chatPage.assertLastResponse("world"); }); + + test("Each assistant message keeps the model that generated it", async ({ + page, + }) => { + const chatPage = new ChatPage(page); + + await chatPage.navigateTo(); + await chatPage.selectModel("gpt-4o"); + + await chatPage.sendMessage("first message"); + await chatPage.assertLastResponse("first message"); + await chatPage.waitForIdle(); + + await chatPage.selectModel("gpt-4o-mini"); + + await chatPage.sendMessage("second message"); + await chatPage.assertLastResponse("second message"); + await chatPage.waitForIdle(); + + await chatPage.assertAssistantMessageModelName(0, "gpt-4o"); + await chatPage.assertAssistantMessageModelName(1, "gpt-4o-mini"); + }); }); diff --git a/e2e_tests/tests_omni_light/src/pages.ts b/e2e_tests/tests_omni_light/src/pages.ts index dc29df0..c5ddd63 100644 --- a/e2e_tests/tests_omni_light/src/pages.ts +++ b/e2e_tests/tests_omni_light/src/pages.ts @@ -44,7 +44,7 @@ export class ChatPage { hasText: /Select model|gpt-/i, }) .first(); - await expect(modelButton).toBeEnabled({ timeout: 10000 }); + await modelButton.waitFor({ state: "visible", timeout: 10000 }); await modelButton.click(); // Click the first option in the popover/command list const firstOption = this.page.locator('[role="option"]').first(); @@ -52,20 +52,54 @@ export class ChatPage { await firstOption.click(); } + async selectModel(modelName: string): Promise { + const modelButton = this.page + .locator("button") + .filter({ hasText: /Select model|gpt-/i }) + .first(); + await modelButton.waitFor({ state: "visible", timeout: 10000 }); + await modelButton.click(); + const option = this.page + .locator('[role="option"]') + .filter({ hasText: modelName }) + .first(); + await option.waitFor({ state: "visible", timeout: 5000 }); + await option.click(); + } + async sendMessage(message: string): Promise { const input = this.page.getByRole("textbox", { name: "Type a message...", }); - await expect(input).toBeEnabled({ timeout: 5000 }); await input.fill(message); await this.page.getByRole("button", { name: "Send" }).click(); } + async waitForIdle(): Promise { + const input = this.page.getByRole("textbox", { + name: "Type a message...", + }); + await input + .and(this.page.locator(":enabled")) + .waitFor({ timeout: 10000 }); + } + async assertLastResponse(content: string): Promise { await expect( this.page.locator(".bg-muted.rounded-2xl").last(), ).toContainText(content, { timeout: 5000 }); } + + async assertAssistantMessageModelName( + index: number, + modelName: string, + ): Promise { + // Each assistant message is in a flex container with justify-start. + // The model-name label is the first

inside the message content area. + await expect( + this.page.locator("div.flex.justify-start .flex-1 > p").nth(index), + ).toHaveText(modelName, { timeout: 5000 }); + } } export class Sidebar { diff --git a/frontend/omni/docs/architecture/core.md b/frontend/omni/docs/architecture/core.md index 4b8a839..2a83914 100644 --- a/frontend/omni/docs/architecture/core.md +++ b/frontend/omni/docs/architecture/core.md @@ -136,7 +136,7 @@ A manifest file has the following top-level shape: 2. The root manifest's own `modules` are applied last and always win. **`collisionStrategy`** on a module entry controls what happens when that module's `id` already exists from an earlier include: -- `"merge"` *(default)* — deep-merges `config`; incoming wins on shared keys. `dependencies` is shallow-merged; incoming wins on key collision. +- `"merge"` *(default)* — deep-merges `config`; incoming wins on shared keys. `dependencies` is merged per key: string values follow the same "incoming wins" rule; **array values are unioned** (base items first, then new incoming items; duplicates dropped). - `"replace"` — incoming entry fully replaces the existing one. - `"drop"` — removes the existing entry; does not add the incoming one either. diff --git a/frontend/omni/src/core/module-system/manifestJson.test.ts b/frontend/omni/src/core/module-system/manifestJson.test.ts index c75bdf3..758ecd2 100644 --- a/frontend/omni/src/core/module-system/manifestJson.test.ts +++ b/frontend/omni/src/core/module-system/manifestJson.test.ts @@ -157,7 +157,7 @@ describe("resolveManifest — collisionStrategy", () => { expect(svc?.config).toEqual({ nested: { a: 1, b: 2, c: 3 } }); }); - it("merge: merges dependencies records, incoming wins on key collision", async () => { + it("merge: merges dependencies records, incoming wins on string key collision", async () => { mockFetch({ "/modules.json": manifest( [ @@ -190,6 +190,68 @@ describe("resolveManifest — collisionStrategy", () => { }); }); + it("merge: unions array dependencies, base items first then new incoming items", async () => { + mockFetch({ + "/modules.json": manifest( + [ + entry("router", { + dependencies: { + "module:routes": ["extra-route"], + }, + }), + ], + { includes: [{ path: "/base.json" }] }, + ), + "/base.json": manifest([ + entry("router", { + dependencies: { + "module:routes": ["chat-route", "fallback-route"], + }, + }), + ]), + }); + + const result = await resolveManifest("/modules.json"); + const router = result.modules.find((m) => m.id === "router"); + + expect(router?.dependencies?.["module:routes"]).toEqual([ + "chat-route", + "fallback-route", + "extra-route", + ]); + }); + + it("merge: array union deduplicates items already present in the base", async () => { + mockFetch({ + "/modules.json": manifest( + [ + entry("router", { + dependencies: { + "module:routes": ["chat-route", "extra-route"], + }, + }), + ], + { includes: [{ path: "/base.json" }] }, + ), + "/base.json": manifest([ + entry("router", { + dependencies: { + "module:routes": ["chat-route", "fallback-route"], + }, + }), + ]), + }); + + const result = await resolveManifest("/modules.json"); + const router = result.modules.find((m) => m.id === "router"); + + expect(router?.dependencies?.["module:routes"]).toEqual([ + "chat-route", + "fallback-route", + "extra-route", + ]); + }); + it("replace: incoming entry fully replaces the included one", async () => { mockFetch({ "/modules.json": manifest( diff --git a/frontend/omni/src/core/module-system/manifestJson.ts b/frontend/omni/src/core/module-system/manifestJson.ts index 71912fc..507947a 100644 --- a/frontend/omni/src/core/module-system/manifestJson.ts +++ b/frontend/omni/src/core/module-system/manifestJson.ts @@ -22,6 +22,9 @@ export interface ManifestEntry { * includes that overwrite earlier ones). * * - `merge` (default): deep-merge config; incoming wins on shared keys. + * For dependency keys whose values are arrays in both base and incoming, + * the arrays are unioned (base items first, then new incoming items; + * duplicates are dropped). For string dependency values, incoming wins. * - `replace`: incoming entry fully replaces the existing one. * - `drop`: remove the existing entry; do not add this entry either. * @@ -117,6 +120,23 @@ function applyModules( return result; } +function mergeDependencies( + base: Record, + incoming: Record, +): Record { + const result: Record = { ...base }; + for (const [key, val] of Object.entries(incoming)) { + const existing = result[key]; + if (Array.isArray(val) && Array.isArray(existing)) { + const seen = new Set(existing); + result[key] = [...existing, ...val.filter((v) => !seen.has(v))]; + } else { + result[key] = val; + } + } + return result; +} + function deepMergeEntries( base: ManifestEntry, incoming: ManifestEntry, @@ -127,10 +147,10 @@ function deepMergeEntries( base.dependencies !== undefined || incoming.dependencies !== undefined ) { - merged.dependencies = { - ...(base.dependencies ?? {}), - ...(incoming.dependencies ?? {}), - }; + merged.dependencies = mergeDependencies( + base.dependencies ?? {}, + incoming.dependencies ?? {}, + ); } if (base.config !== undefined || incoming.config !== undefined) { diff --git a/frontend/omni/src/modules/chat/ChatComponent.svelte b/frontend/omni/src/modules/chat/ChatComponent.svelte index b9f0796..18b0c54 100644 --- a/frontend/omni/src/modules/chat/ChatComponent.svelte +++ b/frontend/omni/src/modules/chat/ChatComponent.svelte @@ -80,7 +80,7 @@ const providerGroups = $derived( })), ); -let messages = $state([]); +let messages = $state[]>([]); let chatStatus = $state<"ready" | "submitted" | "streaming">("ready"); const isIdle = $derived( @@ -106,6 +106,7 @@ async function handleSend(text: string) { id: assistantMessageId, role: "assistant", parts: [{ type: "text", text: "" }], + metadata: { modelName: selectedModelData.modelName }, }, ]; chatStatus = "submitted"; @@ -172,7 +173,6 @@ function makeMessageId(): string { status={chatStatus} {modelsLoading} hasModels={availableModels.length > 0} - selectedModelName={selectedModelData?.modelName} /> {#if messages.length === 0 && availableModels.length > 0} diff --git a/frontend/omni/src/modules/chat/ChatConversationArea.svelte b/frontend/omni/src/modules/chat/ChatConversationArea.svelte index 101eb71..3f0032b 100644 --- a/frontend/omni/src/modules/chat/ChatConversationArea.svelte +++ b/frontend/omni/src/modules/chat/ChatConversationArea.svelte @@ -13,13 +13,11 @@ let { status, modelsLoading, hasModels, - selectedModelName, }: { - messages: UIMessage[]; + messages: UIMessage<{ modelName?: string }>[]; status: string; modelsLoading: boolean; hasModels: boolean; - selectedModelName: string | undefined; } = $props(); let conversationEl = $state(null); @@ -72,7 +70,7 @@ $effect(() => { {/if} {#each messages as message (message.id)} - + {/each} diff --git a/frontend/omni/src/modules/chat/ChatMessageItem.svelte b/frontend/omni/src/modules/chat/ChatMessageItem.svelte index d54250f..a1c7756 100644 --- a/frontend/omni/src/modules/chat/ChatMessageItem.svelte +++ b/frontend/omni/src/modules/chat/ChatMessageItem.svelte @@ -9,10 +9,8 @@ import * as Collapsible from "$lib/shadcnui/components/ui/collapsible/index.js"; let { message, - selectedModelName, }: { - message: UIMessage; - selectedModelName: string | undefined; + message: UIMessage<{ modelName?: string }>; } = $props(); function renderMarkdown(text: string): string { @@ -51,7 +49,7 @@ function renderMarkdown(text: string): string { > {message.role === "user" ? "You" - : (selectedModelName ?? "Assistant")} + : (message.metadata?.modelName ?? "Assistant")}