From de48cb5a7c93b234923f20ba1e8b841e2aa8753f Mon Sep 17 00:00:00 2001 From: Allen7702 Date: Thu, 24 Sep 2026 08:41:25 +0300 Subject: [PATCH] refactor: remove models module and refactor prediction trigger context handling --- src/services/api/index.ts | 2 - src/services/api/modules/models/index.ts | 21 --- src/services/api/modules/models/service.ts | 100 ------------ src/services/worker/constants/handlers.ts | 2 + .../predictionTrigger/schemas/config.ts | 29 ++-- .../handlers/predictionTrigger/utils/data.ts | 11 +- .../utils/resolveEffectiveHandlerConfig.ts | 15 +- src/shared/clients/climate.ts | 6 - src/shared/handlers/catalog.ts | 5 + src/tests/unit/handler-context.test.ts | 18 ++- src/tests/unit/predictionTrigger.test.ts | 4 +- .../resolveEffectiveHandlerConfig.test.ts | 42 ++++- src/tests/unit/routes/models.test.ts | 148 ------------------ 13 files changed, 89 insertions(+), 314 deletions(-) delete mode 100644 src/services/api/modules/models/index.ts delete mode 100644 src/services/api/modules/models/service.ts delete mode 100644 src/shared/clients/climate.ts delete mode 100644 src/tests/unit/routes/models.test.ts diff --git a/src/services/api/index.ts b/src/services/api/index.ts index 8ca21ff..acc736d 100644 --- a/src/services/api/index.ts +++ b/src/services/api/index.ts @@ -15,7 +15,6 @@ import { schedulesModule } from "@/services/api/modules/schedules/index.ts"; import { climateModule } from "@/services/api/modules/climate/index.ts"; import { monitoringModule } from "@/services/api/modules/monitoring/index.ts"; import { handlersModule } from "@/services/api/modules/handlers/index.ts"; -import { modelsModule } from "@/services/api/modules/models/index.ts"; import { evaluationsModule } from "@/services/api/modules/evaluations/index.ts"; import { systemModule } from "@/services/api/modules/system/index.ts"; @@ -52,7 +51,6 @@ const app = new Elysia() .use(climateModule) .use(monitoringModule) .use(handlersModule) - .use(modelsModule) .use(evaluationsModule) .use(systemModule); diff --git a/src/services/api/modules/models/index.ts b/src/services/api/modules/models/index.ts deleted file mode 100644 index 77be586..0000000 --- a/src/services/api/modules/models/index.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { Elysia } from "elysia"; -import { getModels } from "@/services/api/modules/models/service.ts"; - -export const modelsModule = new Elysia({ prefix: "/models", tags: ["Models"] }).get( - "/", - () => getModels(), - { - detail: { - summary: "List configured CHAP models", - description: - "Proxies GET /v1/crud/configured-models from CHAP. Always returns HTTP 200; error/code fields present when CHAP is unreachable.", - operationId: "listModels", - responses: { - "200": { - description: - "List of configured models (may be empty with error/code fields if CHAP is unavailable)", - }, - }, - }, - } -); diff --git a/src/services/api/modules/models/service.ts b/src/services/api/modules/models/service.ts deleted file mode 100644 index c006072..0000000 --- a/src/services/api/modules/models/service.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { AxiosError } from "axios"; -import { chapClient } from "@/shared/clients/chap.ts"; -import { ApiErrorCode } from "@/shared/api/errorCodes.ts"; - -export type ModelCovariate = { - name: string; - displayName: string; -}; - -export type ConfiguredModel = { - id: string; - name: string; - displayName: string; - covariates: ModelCovariate[]; - /** The feature the model predicts (e.g. `disease_cases`). */ - target?: ModelCovariate; - supportedPeriodType?: string; -}; - -export type ModelsResponse = { - models: ConfiguredModel[]; - error?: string; - code?: string; -}; - -function mapFeature(value: unknown): ModelCovariate | null { - if (typeof value !== "object" || value === null) { - return null; - } - const record = value as Record; - const name = record["name"]; - if (typeof name !== "string" || name.length === 0) { - return null; - } - const displayName = - typeof record["displayName"] === "string" && record["displayName"].length > 0 - ? record["displayName"] - : name; - return { name, displayName }; -} - -function mapCovariates(value: unknown): ModelCovariate[] { - if (!Array.isArray(value)) { - return []; - } - return value.map(mapFeature).filter((feature): feature is ModelCovariate => feature !== null); -} - -function mapConfiguredModel(item: unknown): ConfiguredModel | null { - if (typeof item !== "object" || item === null) { - return null; - } - const record = item as Record; - if (typeof record["name"] !== "string" || record["name"].length === 0) { - return null; - } - const id = record["id"]; - if (typeof id !== "string" && typeof id !== "number") { - return null; - } - const displayName = - typeof record["displayName"] === "string" && record["displayName"].length > 0 - ? record["displayName"] - : record["name"]; - const supportedPeriodType = record["supportedPeriodType"]; - const target = mapFeature(record["target"]); - return { - id: String(id), - name: record["name"], - displayName, - covariates: mapCovariates(record["covariates"]), - ...(target ? { target } : {}), - ...(typeof supportedPeriodType === "string" ? { supportedPeriodType } : {}), - }; -} - -function mapConfiguredModels(data: unknown): ConfiguredModel[] { - if (!Array.isArray(data)) { - return []; - } - const models: ConfiguredModel[] = []; - for (const item of data) { - const mapped = mapConfiguredModel(item); - if (mapped) { - models.push(mapped); - } - } - return models; -} - -/** Always resolves with HTTP 200 semantics; `error`/`code` present when CHAP is unreachable. */ -export async function getModels(): Promise { - try { - const response = await chapClient.get("/v1/crud/configured-models"); - return { models: mapConfiguredModels(response.data) }; - } catch (error) { - const message = error instanceof AxiosError ? error.message : "CHAP is currently unreachable"; - return { models: [], error: message, code: ApiErrorCode.CHAP_UNAVAILABLE }; - } -} diff --git a/src/services/worker/constants/handlers.ts b/src/services/worker/constants/handlers.ts index 6fd309d..d1ce8fa 100644 --- a/src/services/worker/constants/handlers.ts +++ b/src/services/worker/constants/handlers.ts @@ -27,6 +27,7 @@ export type HandlerRegistryEntry = { config?: ZodType; context?: ZodType; }; + contextReplaces?: string[]; }; export enum Handlers { @@ -92,6 +93,7 @@ export const HANDLERS: Map = new Map< config: predictionTriggerConfigSchema, context: predictionTriggerContextSchema, }, + contextReplaces: ["period"], }, ], [ diff --git a/src/services/worker/services/handlers/predictionTrigger/schemas/config.ts b/src/services/worker/services/handlers/predictionTrigger/schemas/config.ts index 6c07937..5f8f4a7 100644 --- a/src/services/worker/services/handlers/predictionTrigger/schemas/config.ts +++ b/src/services/worker/services/handlers/predictionTrigger/schemas/config.ts @@ -1,29 +1,24 @@ import { z } from "zod"; +const trainingPeriodSchema = z + .object({ + endPeriod: z.string().min(1).optional(), + periodOffset: z.number().int().min(0).optional(), + numberOfPeriodsToGenerate: z.number().int().positive().default(3), + }) + .refine(({ endPeriod, periodOffset }) => endPeriod !== undefined || periodOffset !== undefined, { + message: "Either endPeriod or periodOffset must be set for the training period", + }); + export const predictionTriggerConfigSchema = z.object({ backtestId: z.number().int().positive(), predictionSetupId: z.number().int().positive(), name: z.string().min(1), - period: z - .object({ - endPeriod: z.string().min(1).optional(), - periodOffset: z.number().int().min(0).optional(), - numberOfPeriodsToGenerate: z.number().int().positive().default(3), - }) - .refine( - ({ endPeriod, periodOffset }) => endPeriod !== undefined || periodOffset !== undefined, - { - message: "Either endPeriod or periodOffset must be set for the training period", - } - ), + period: trainingPeriodSchema, }); export type PredictionTriggerConfig = z.infer; -/** Per-run overrides: a schedule ends the training window relative to each run. */ export const predictionTriggerContextSchema = z.object({ - period: z.object({ - periodOffset: z.number().default(0), - numberOfPeriodsToGenerate: z.number().default(3), - }), + period: trainingPeriodSchema, }); diff --git a/src/services/worker/services/handlers/predictionTrigger/utils/data.ts b/src/services/worker/services/handlers/predictionTrigger/utils/data.ts index f3c346e..5f06fe0 100644 --- a/src/services/worker/services/handlers/predictionTrigger/utils/data.ts +++ b/src/services/worker/services/handlers/predictionTrigger/utils/data.ts @@ -145,11 +145,6 @@ function parsePeriodId(periodId: string, type: PeriodTypeEnum): DateTime { return parsed.startOf("week"); } -/** - * Builds the training window: every period from the setup's `startPeriod` up to - * `periodOffset` periods back from today. Offset 0 ends on the current (incomplete) - * period, 1 ends on the previous one. - */ export function getTrainingPeriods({ startPeriod, periodType, @@ -172,9 +167,9 @@ export function getTrainingPeriods({ } const end = - periodOffset !== undefined - ? now.minus({ [durationLabel]: periodOffset }).startOf(unit) - : parsePeriodId(endPeriod as string, periodType); + endPeriod !== undefined + ? parsePeriodId(endPeriod, periodType) + : now.minus({ [durationLabel]: periodOffset as number }).startOf(unit); if (end < start) { throw new Error( diff --git a/src/services/worker/utils/resolveEffectiveHandlerConfig.ts b/src/services/worker/utils/resolveEffectiveHandlerConfig.ts index 358fb02..d898e9a 100644 --- a/src/services/worker/utils/resolveEffectiveHandlerConfig.ts +++ b/src/services/worker/utils/resolveEffectiveHandlerConfig.ts @@ -2,6 +2,7 @@ import { merge } from "lodash-es"; import { formatHandlerConfigIssues, formatHandlerContextIssues, + getHandlerContextReplaceKeys, validateHandlerConfig, validateHandlerContext, } from "@/shared/handlers/catalog.ts"; @@ -33,7 +34,8 @@ export class EffectiveHandlerConfigError extends Error { /** * Merges pipeline execution context step overrides onto static handlerConfig. - * Context wins on conflicts (deep merge). Validates override and merged config. + * Context wins on conflicts (deep merge), except for the handler's `contextReplaces` keys, + * which the context replaces whole. Validates override and merged config. */ export function resolveEffectiveHandlerConfig(args: { handlerKey: string; @@ -52,8 +54,15 @@ export function resolveEffectiveHandlerConfig(args: { } } - const merged = - override !== undefined && Object.keys(override).length > 0 ? merge({}, base, override) : base; + let merged = base; + if (override !== undefined && Object.keys(override).length > 0) { + merged = merge({}, base, override); + for (const key of getHandlerContextReplaceKeys(args.handlerKey)) { + if (key in override) { + merged[key] = override[key]; + } + } + } const configResult = validateHandlerConfig(args.handlerKey, merged); if (!configResult.success) { diff --git a/src/shared/clients/climate.ts b/src/shared/clients/climate.ts deleted file mode 100644 index b3e0d45..0000000 --- a/src/shared/clients/climate.ts +++ /dev/null @@ -1,6 +0,0 @@ -import axios from "axios"; -import { env } from "@/shared/utils"; - -export const climateDataClient = axios.create({ - baseURL: env.CLIMATE_DATA_BASE_URL, -}); diff --git a/src/shared/handlers/catalog.ts b/src/shared/handlers/catalog.ts index 78c9dc3..b92a5d3 100644 --- a/src/shared/handlers/catalog.ts +++ b/src/shared/handlers/catalog.ts @@ -33,6 +33,11 @@ function getRegistryContextSchema(key: string): ZodType | undefined { return entry?.schemas.context; } +/** Context keys that replace the handler config value whole rather than merging into it. */ +export function getHandlerContextReplaceKeys(handlerKey: string): string[] { + return HANDLERS.get(handlerKey as Handlers)?.contextReplaces ?? []; +} + export function hasHandlerContextSchema(handlerKey: string): boolean { return getRegistryContextSchema(handlerKey) !== undefined; } diff --git a/src/tests/unit/handler-context.test.ts b/src/tests/unit/handler-context.test.ts index 4ac708a..08f199f 100644 --- a/src/tests/unit/handler-context.test.ts +++ b/src/tests/unit/handler-context.test.ts @@ -8,15 +8,29 @@ describe("validateHandlerContext", () => { }); it("accepts valid prediction-trigger context slice", () => { + const result = validateHandlerContext("prediction-trigger", { + period: { endPeriod: "202608", numberOfPeriodsToGenerate: 3 }, + }); + expect(result.success).toBe(true); + }); + + it("accepts a schedule's period offset on prediction-trigger", () => { const result = validateHandlerContext("prediction-trigger", { period: { periodOffset: 1, numberOfPeriodsToGenerate: 3 }, }); expect(result.success).toBe(true); }); - it("rejects a non-numeric period offset on prediction-trigger", () => { + it("rejects a negative period offset on prediction-trigger", () => { + const result = validateHandlerContext("prediction-trigger", { + period: { periodOffset: -1, numberOfPeriodsToGenerate: 3 }, + }); + expect(result.success).toBe(false); + }); + + it("rejects a prediction-trigger context with neither an end period nor an offset", () => { const result = validateHandlerContext("prediction-trigger", { - period: { periodOffset: "last", numberOfPeriodsToGenerate: 3 }, + period: { numberOfPeriodsToGenerate: 3 }, }); expect(result.success).toBe(false); }); diff --git a/src/tests/unit/predictionTrigger.test.ts b/src/tests/unit/predictionTrigger.test.ts index 9b70355..3592de4 100644 --- a/src/tests/unit/predictionTrigger.test.ts +++ b/src/tests/unit/predictionTrigger.test.ts @@ -92,7 +92,7 @@ describe("getTrainingPeriods", () => { expect(periods.at(-1)).toBe("202604"); }); - it("lets a schedule's offset win over the period picked on the step", () => { + it("lets an end period picked for a run win over an offset left on the step", () => { const periods = getTrainingPeriods({ startPeriod: "202207", periodType: PeriodTypeEnum.MONTHLY, @@ -101,7 +101,7 @@ describe("getTrainingPeriods", () => { now: september2026, }); - expect(periods.at(-1)).toBe("202608"); + expect(periods.at(-1)).toBe("202604"); }); it("refuses a training period with neither an offset nor an end period", () => { diff --git a/src/tests/unit/resolveEffectiveHandlerConfig.test.ts b/src/tests/unit/resolveEffectiveHandlerConfig.test.ts index ded30a2..5d10cf8 100644 --- a/src/tests/unit/resolveEffectiveHandlerConfig.test.ts +++ b/src/tests/unit/resolveEffectiveHandlerConfig.test.ts @@ -36,27 +36,59 @@ describe("resolveEffectiveHandlerConfig", () => { expect(merged).toEqual(baseConfig); }); - it("deep-merges context override onto handlerConfig", () => { + it("replaces the step's period with the context's", () => { const merged = resolveEffectiveHandlerConfig({ handlerKey: "prediction-trigger", handlerConfig: baseConfig, pipelineContext: { steps: { "step-1": { - period: { periodOffset: 1, numberOfPeriodsToGenerate: 6 }, + period: { endPeriod: "202610", numberOfPeriodsToGenerate: 6 }, }, }, }, stepId: "step-1", }); expect(merged.period).toEqual({ - endPeriod: "202608", - periodOffset: 1, + endPeriod: "202610", numberOfPeriodsToGenerate: 6, }); expect(merged.predictionSetupId).toBe(3); }); + it("lets a schedule's offset replace the step's fixed end period", () => { + const merged = resolveEffectiveHandlerConfig({ + handlerKey: "prediction-trigger", + handlerConfig: baseConfig, + pipelineContext: { + steps: { + "step-1": { + period: { periodOffset: 1, numberOfPeriodsToGenerate: 3 }, + }, + }, + }, + stepId: "step-1", + }); + expect(merged.period).toEqual({ periodOffset: 1, numberOfPeriodsToGenerate: 3 }); + }); + + it("deep-merges keys the handler does not replace", () => { + const merged = resolveEffectiveHandlerConfig({ + handlerKey: "prediction-trigger", + handlerConfig: baseConfig, + pipelineContext: { + steps: { + "step-1": { + period: { endPeriod: "202610", numberOfPeriodsToGenerate: 6 }, + }, + }, + }, + stepId: "step-1", + }); + expect(merged.name).toBe("run-1"); + expect(merged.backtestId).toBe(2); + }); + it("throws when context override is invalid", () => { expect(() => resolveEffectiveHandlerConfig({ @@ -65,7 +97,7 @@ describe("resolveEffectiveHandlerConfig", () => { pipelineContext: { steps: { "step-1": { - period: { periodOffset: "last" }, + period: { endPeriod: "" }, }, }, }, diff --git a/src/tests/unit/routes/models.test.ts b/src/tests/unit/routes/models.test.ts deleted file mode 100644 index 41bd55a..0000000 --- a/src/tests/unit/routes/models.test.ts +++ /dev/null @@ -1,148 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { AxiosError } from "axios"; - -vi.mock("@/shared/clients/chap.ts", () => ({ - chapClient: { - get: vi.fn(), - }, -})); - -// Import after mock is set up -import { chapClient } from "@/shared/clients/chap.ts"; -import { getModels } from "@/services/api/modules/models/service.ts"; - -const mockedChapGet = vi.mocked(chapClient.get); - -describe("getModels", () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("returns mapped models on success", async () => { - mockedChapGet.mockResolvedValueOnce({ - data: [ - { - id: 42, - name: "seasonal_model", - displayName: "Seasonal Model", - supportedPeriodType: "month", - covariates: [ - { name: "rainfall", displayName: "Rainfall", description: "Rainfall" }, - { name: "mean_temperature", displayName: "Mean temperature" }, - ], - target: { name: "disease_cases", displayName: "Disease cases", description: "Cases" }, - extra: "ignored", - }, - ], - status: 200, - statusText: "OK", - headers: {}, - config: {} as never, - }); - - const body = await getModels(); - - expect(body).toEqual({ - models: [ - { - id: "42", - name: "seasonal_model", - displayName: "Seasonal Model", - supportedPeriodType: "month", - covariates: [ - { name: "rainfall", displayName: "Rainfall" }, - { name: "mean_temperature", displayName: "Mean temperature" }, - ], - target: { name: "disease_cases", displayName: "Disease cases" }, - }, - ], - }); - }); - - it("falls back to name when displayName is missing", async () => { - mockedChapGet.mockResolvedValueOnce({ - data: [{ id: 7, name: "auto_arima" }], - status: 200, - statusText: "OK", - headers: {}, - config: {} as never, - }); - - const body = await getModels(); - - expect(body.models).toEqual([ - { id: "7", name: "auto_arima", displayName: "auto_arima", covariates: [] }, - ]); - }); - - it("defaults covariates to an empty array when CHAP omits or malforms them", async () => { - mockedChapGet.mockResolvedValueOnce({ - data: [ - { id: 1, name: "no_covariates" }, - { id: 2, name: "bad_covariates", covariates: "rainfall" }, - { id: 3, name: "partial_covariates", covariates: [{ displayName: "Nameless" }, null] }, - ], - status: 200, - statusText: "OK", - headers: {}, - config: {} as never, - }); - - const body = await getModels(); - - expect(body.models.map((model) => model.covariates)).toEqual([[], [], []]); - }); - - it("omits target when CHAP omits or malforms it", async () => { - mockedChapGet.mockResolvedValueOnce({ - data: [ - { id: 1, name: "no_target" }, - { id: 2, name: "bad_target", target: "disease_cases" }, - { id: 3, name: "nameless_target", target: { displayName: "Nameless" } }, - ], - status: 200, - statusText: "OK", - headers: {}, - config: {} as never, - }); - - const body = await getModels(); - - expect(body.models.map((model) => "target" in model)).toEqual([false, false, false]); - }); - - it("returns empty models with error/code on CHAP failure", async () => { - const axiosError = new AxiosError("Network Error"); - mockedChapGet.mockRejectedValueOnce(axiosError); - - const body = await getModels(); - - expect(body.models).toEqual([]); - expect(body.error).toBe("Network Error"); - expect(body.code).toBe("chap_unavailable"); - }); - - it("returns empty models with generic error message on non-Axios failure", async () => { - mockedChapGet.mockRejectedValueOnce(new Error("unexpected")); - - const body = await getModels(); - - expect(body.models).toEqual([]); - expect(body.error).toBe("CHAP is currently unreachable"); - expect(body.code).toBe("chap_unavailable"); - }); - - it("returns empty models array when CHAP response data is not an array", async () => { - mockedChapGet.mockResolvedValueOnce({ - data: null, - status: 200, - statusText: "OK", - headers: {}, - config: {} as never, - }); - - const body = await getModels(); - - expect(body.models).toEqual([]); - }); -});