diff --git a/design/reports.md b/design/reports.md index bae340c6..9b0c106d 100644 --- a/design/reports.md +++ b/design/reports.md @@ -152,7 +152,45 @@ See `src/domain/reports/user_report_loader.ts`. ## Report Registry The `ReportRegistry` is a `Map`-backed registry of report definitions keyed by -name. It provides `register`, `get`, `getAll`, `getByScope`, and `has` methods. +name. Every report type exists in one of two states: + +- **Fully loaded** — the bundle has been imported and the `ReportDefinition` + (including its `execute` function) is available in the internal `reports` + map. `register`, `get`, `getAll`, `getByScope`, and `has` all operate on + fully-loaded entries. +- **Lazy** — the type is known to exist from the extension bundle catalog, + but its bundle has not been imported yet. Lazy entries live in a separate + `lazyTypes` map and are materialized from the on-disk catalog on second + and subsequent process starts without touching the bundle files. + `registerLazy`, `isLazy`, `getAllLazy`, and the `LazyReportEntry` type + describe this state. + +`ensureTypeLoaded(name)` promotes a single lazy entry to fully loaded by +importing its bundle on demand and invoking `promoteFromLazy`. Concurrent +callers for the same type share a single in-flight promise via an internal +`typeLoadPromises` map, so a burst of promotions still triggers at most one +bundle import per type. `ensureTypeLoaded` is a no-op for types that are +already loaded or not registered at all. + +The CLI wires two hooks into the registry at startup (see `src/cli/mod.ts`): + +- `setLoader` — a full eager-load fallback that walks the reports directory + and imports every bundle. Triggered by `ensureLoaded()` and used when no + catalog is available. +- `setTypeLoader` — a per-type loader that imports a single bundle via the + catalog entry for that type. Backs `ensureTypeLoaded` in normal operation. + +**Promotion contract for iteration.** Because `getAll()` returns only +fully-loaded entries, any domain service that iterates the registry (most +notably `executeReports` in `report_execution_service.ts`) must first call +`ensureTypeLoaded` for every candidate report name — typically the union of +`selection.require` and the model type's declared report defaults — before +calling `getAll()` and filtering the result. Skipping this promotion step +causes lazy user-extension reports to be silently filtered out of the +applicable set on the second and subsequent process runs (the catalog is +populated, so the fully-loaded map contains only eagerly-registered builtin +reports). Iteration without promotion is the regression fixed by issue #81 +after the lazy-loading rework in #1089. The global singleton uses `globalThis` so the same registry is shared across module boundaries (important when extensions are loaded outside the bundle): @@ -164,7 +202,7 @@ export const reportRegistry: ReportRegistry = ``` Duplicate name registration throws an error. See -`src/domain/reports/report_registry.ts`. +`src/domain/reports/report_registry.ts` for the full API. ## Three-Level Control Model diff --git a/src/domain/reports/report_execution_service.ts b/src/domain/reports/report_execution_service.ts index bd95bd80..e86a322c 100644 --- a/src/domain/reports/report_execution_service.ts +++ b/src/domain/reports/report_execution_service.ts @@ -332,6 +332,38 @@ export async function executeReports( modelTypeReports?: string[], varySuffix?: string, ): Promise { + // Promote lazy-registered reports for every candidate name before calling + // getAll(). getAll() only returns fully-loaded entries from the registry's + // `reports` Map; lazy entries in `lazyTypes` are excluded. Without this + // step, user extension reports registered lazily from the bundle catalog + // (second+ process run) would be silently filtered out of the applicable + // set — the regression fixed by #81 after the lazy loading rework in + // #1089. + // + // We promote every candidate name unconditionally, including ones whose + // scope will end up mismatched by filterReports() below. We cannot inspect + // `report.scope` until the bundle is imported, and extending + // LazyReportEntry with scope would duplicate the bundle catalog and + // defeat the point of lazy loading for scope-mismatched reports anyway. + // Wasted imports are bounded by |selection.require ∪ modelTypeReports| + // and only happen once per process — ensureTypeLoaded() dedupes concurrent + // callers via typeLoadPromises, and is a no-op for already-loaded and + // unknown names. Errors from ensureTypeLoaded() propagate unchanged: a + // broken bundle for a required report must fail loudly rather than be + // silently skipped. + const candidateNames = new Set(); + for (const ref of selection?.require ?? []) { + candidateNames.add(getReportRefName(ref)); + } + if (modelTypeReports) { + for (const name of modelTypeReports) { + candidateNames.add(name); + } + } + await Promise.all( + Array.from(candidateNames, (name) => registry.ensureTypeLoaded(name)), + ); + const allReports = registry.getAll(); const applicable = filterReports( allReports, diff --git a/src/domain/reports/report_execution_service_test.ts b/src/domain/reports/report_execution_service_test.ts index 821497c3..1fbb185c 100644 --- a/src/domain/reports/report_execution_service_test.ts +++ b/src/domain/reports/report_execution_service_test.ts @@ -1077,3 +1077,215 @@ Deno.test("buildRedactSensitiveArgs: returns args unchanged for workflow scope", assertEquals(capturedResult !== null, true); assertEquals(capturedResult!.apiKey, "sk-secret"); }); + +// --- executeReports lazy-report promotion tests (regression: #81) --- + +/** + * Builds a minimal MethodReportContext for lazy-promotion tests. + */ +function makeMethodContext( + repo: ReturnType["repo"], + modelType: ModelType, +): MethodReportContext { + return { + scope: "method", + repoDir: "/tmp/test", + logger: { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + } as unknown as MethodReportContext["logger"], + // deno-lint-ignore no-explicit-any + dataRepository: repo as any, + // deno-lint-ignore no-explicit-any + definitionRepository: {} as any, + modelType, + modelId: "test-id", + definition: { id: "test-id", name: "test", version: 1, tags: {} }, + globalArgs: {}, + methodArgs: {}, + methodName: "run", + executionStatus: "succeeded", + dataHandles: [], + }; +} + +/** + * Creates a fresh ReportRegistry with a single lazy entry and a type loader + * that promotes it on demand. Returns the registry and a counter for the + * number of times the loader was invoked. + */ +function createRegistryWithLazyReport( + typeName: string, + scope: "method" | "model" | "workflow" = "method", +): { registry: ReportRegistry; loaderCallCount: { value: number } } { + const registry = new ReportRegistry(); + registry.registerLazy({ + type: typeName, + bundlePath: `/tmp/fake-bundles/${typeName}.js`, + sourcePath: `/tmp/fake-sources/${typeName}.ts`, + version: "2026.04.11.1", + }); + + const loaderCallCount = { value: 0 }; + registry.setTypeLoader((type) => { + loaderCallCount.value++; + registry.promoteFromLazy(type, makeReport(scope)); + return Promise.resolve(); + }); + + return { registry, loaderCallCount }; +} + +Deno.test("executeReports: promotes lazy report named in selection.require", async () => { + const { registry, loaderCallCount } = createRegistryWithLazyReport( + "@test/lazy-method", + ); + const { repo, saved } = createInMemoryDataRepo(); + const modelType = ModelType.create("test/model"); + const context = makeMethodContext(repo, modelType); + + const summary = await executeReports( + registry, + context, + modelType, + "test-id", + { require: ["@test/lazy-method"] }, + {}, + undefined, + "run", + undefined, + ); + + assertEquals(loaderCallCount.value, 1); + assertEquals(summary.failures, 0); + assertEquals(summary.results.length, 1); + assertEquals(summary.results[0].name, "@test/lazy-method"); + assertEquals(summary.results[0].success, true); + // Persisted two artifacts (markdown + json) + assertEquals(saved.length, 2); + assertStringIncludes(saved[0].name, "test-lazy-method"); +}); + +Deno.test("executeReports: promotes lazy report from modelTypeReports defaults", async () => { + const { registry, loaderCallCount } = createRegistryWithLazyReport( + "@test/lazy-default", + ); + const { repo } = createInMemoryDataRepo(); + const modelType = ModelType.create("test/model"); + const context = makeMethodContext(repo, modelType); + + const summary = await executeReports( + registry, + context, + modelType, + "test-id", + undefined, + {}, + undefined, + "run", + ["@test/lazy-default"], + ); + + assertEquals(loaderCallCount.value, 1); + assertEquals(summary.failures, 0); + assertEquals(summary.results.length, 1); + assertEquals(summary.results[0].name, "@test/lazy-default"); + assertEquals(summary.results[0].success, true); +}); + +Deno.test("executeReports: handles ReportRef object form in require", async () => { + const { registry, loaderCallCount } = createRegistryWithLazyReport( + "@test/lazy-ref", + ); + const { repo } = createInMemoryDataRepo(); + const modelType = ModelType.create("test/model"); + const context = makeMethodContext(repo, modelType); + + const summary = await executeReports( + registry, + context, + modelType, + "test-id", + { require: [{ name: "@test/lazy-ref", methods: ["run"] }] }, + {}, + undefined, + "run", + undefined, + ); + + assertEquals(loaderCallCount.value, 1); + assertEquals(summary.results.length, 1); + assertEquals(summary.results[0].name, "@test/lazy-ref"); + assertEquals(summary.results[0].success, true); +}); + +Deno.test("executeReports: ensureTypeLoaded failure fails loudly", async () => { + const registry = new ReportRegistry(); + registry.registerLazy({ + type: "@test/lazy-broken", + bundlePath: "/tmp/fake-bundles/broken.js", + sourcePath: "/tmp/fake-sources/broken.ts", + version: "2026.04.11.1", + }); + registry.setTypeLoader(() => { + return Promise.reject(new Error("bundle import failed")); + }); + + const { repo } = createInMemoryDataRepo(); + const modelType = ModelType.create("test/model"); + const context = makeMethodContext(repo, modelType); + + let caught: Error | undefined; + try { + await executeReports( + registry, + context, + modelType, + "test-id", + { require: ["@test/lazy-broken"] }, + {}, + undefined, + "run", + undefined, + ); + } catch (err) { + caught = err as Error; + } + + assertEquals(caught !== undefined, true); + assertStringIncludes(caught!.message, "bundle import failed"); +}); + +Deno.test("executeReports: already-loaded reports are not re-promoted", async () => { + const registry = new ReportRegistry(); + registry.register("@test/already-loaded", makeReport("method")); + + let loaderCalled = false; + registry.setTypeLoader(() => { + loaderCalled = true; + return Promise.reject(new Error("type loader should not be called")); + }); + + const { repo } = createInMemoryDataRepo(); + const modelType = ModelType.create("test/model"); + const context = makeMethodContext(repo, modelType); + + const summary = await executeReports( + registry, + context, + modelType, + "test-id", + { require: ["@test/already-loaded"] }, + {}, + undefined, + "run", + undefined, + ); + + assertEquals(loaderCalled, false); + assertEquals(summary.results.length, 1); + assertEquals(summary.results[0].success, true); +});