From 01331af10c18676a7756487a2ab6f476f845b478 Mon Sep 17 00:00:00 2001 From: ismeth Date: Fri, 20 Feb 2026 14:21:23 +0100 Subject: [PATCH] refactor(athena): consolidate parseModelString to single source of truth --- src/agents/athena/index.ts | 1 - src/agents/athena/model-parser.ts | 23 ---------- .../builtin-agents/council-member-agents.ts | 2 +- src/config/schema/athena.ts | 4 +- .../model-string-parser.test.ts} | 45 ++++++++++++++----- .../delegate-task/model-string-parser.ts | 20 +++++++-- 6 files changed, 54 insertions(+), 41 deletions(-) delete mode 100644 src/agents/athena/model-parser.ts rename src/{agents/athena/model-parser.test.ts => tools/delegate-task/model-string-parser.test.ts} (54%) diff --git a/src/agents/athena/index.ts b/src/agents/athena/index.ts index 1709f7cf..624f553e 100644 --- a/src/agents/athena/index.ts +++ b/src/agents/athena/index.ts @@ -1,5 +1,4 @@ export * from "./types" export * from "./agent" export * from "./council-member-agent" -export * from "./model-parser" diff --git a/src/agents/athena/model-parser.ts b/src/agents/athena/model-parser.ts deleted file mode 100644 index 6a408832..00000000 --- a/src/agents/athena/model-parser.ts +++ /dev/null @@ -1,23 +0,0 @@ -export interface ParsedModel { - providerID: string - modelID: string -} - -export function parseModelString(model: string): ParsedModel | null { - if (!model) { - return null - } - - const slashIndex = model.indexOf("/") - if (slashIndex <= 0) { - return null - } - - const providerID = model.substring(0, slashIndex) - const modelID = model.substring(slashIndex + 1) - if (!modelID) { - return null - } - - return { providerID, modelID } -} diff --git a/src/agents/builtin-agents/council-member-agents.ts b/src/agents/builtin-agents/council-member-agents.ts index d2b83c69..6a364e9c 100644 --- a/src/agents/builtin-agents/council-member-agents.ts +++ b/src/agents/builtin-agents/council-member-agents.ts @@ -1,7 +1,7 @@ import type { AgentConfig } from "@opencode-ai/sdk" import type { CouncilConfig, CouncilMemberConfig } from "../athena/types" import { createCouncilMemberAgent } from "../athena/council-member-agent" -import { parseModelString } from "../athena/model-parser" +import { parseModelString } from "../../tools/delegate-task/model-string-parser" import { log } from "../../shared/logger" /** Prefix used for all dynamically-registered council member agent keys. */ diff --git a/src/config/schema/athena.ts b/src/config/schema/athena.ts index ad7d96d5..21ea2f1d 100644 --- a/src/config/schema/athena.ts +++ b/src/config/schema/athena.ts @@ -1,12 +1,12 @@ import { z } from "zod" -import { parseModelString } from "../../agents/athena/model-parser" +import { parseModelString } from "../../tools/delegate-task/model-string-parser" /** Validates model string format: "provider/model-id" (e.g., "openai/gpt-5.3-codex"). */ const ModelStringSchema = z .string() .min(1) .refine( - (model) => parseModelString(model) !== null, + (model) => parseModelString(model) !== undefined, { message: 'Model must be in "provider/model-id" format (e.g., "openai/gpt-5.3-codex")' } ) diff --git a/src/agents/athena/model-parser.test.ts b/src/tools/delegate-task/model-string-parser.test.ts similarity index 54% rename from src/agents/athena/model-parser.test.ts rename to src/tools/delegate-task/model-string-parser.test.ts index c63bfdef..b13679db 100644 --- a/src/agents/athena/model-parser.test.ts +++ b/src/tools/delegate-task/model-string-parser.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { parseModelString } from "./model-parser" +import { parseModelString } from "./model-string-parser" describe("parseModelString", () => { describe("valid model strings", () => { @@ -52,22 +52,47 @@ describe("parseModelString", () => { describe("invalid model strings", () => { //#given malformed or empty model strings //#when parsing model strings - //#then it returns null + //#then it returns undefined - test("returns null for empty string", () => { - expect(parseModelString("")).toBeNull() + test("returns undefined for empty string", () => { + expect(parseModelString("")).toBeUndefined() }) - test("returns null for model without slash", () => { - expect(parseModelString("no-slash-model")).toBeNull() + test("returns undefined for model without slash", () => { + expect(parseModelString("no-slash-model")).toBeUndefined() }) - test("returns null for empty provider", () => { - expect(parseModelString("/missing-provider")).toBeNull() + test("returns undefined for empty provider", () => { + expect(parseModelString("/missing-provider")).toBeUndefined() }) - test("returns null for empty model", () => { - expect(parseModelString("missing-model/")).toBeNull() + test("returns undefined for empty model", () => { + expect(parseModelString("missing-model/")).toBeUndefined() + }) + }) + + describe("whitespace handling", () => { + //#given model strings with whitespace + //#when parsing + //#then it rejects whitespace-only parts and trims valid parts + + test("returns undefined for whitespace-only string", () => { + expect(parseModelString(" ")).toBeUndefined() + }) + + test("returns undefined for whitespace-only provider", () => { + expect(parseModelString(" /model")).toBeUndefined() + }) + + test("returns undefined for whitespace-only model", () => { + expect(parseModelString("provider/ ")).toBeUndefined() + }) + + test("trims whitespace from provider and model", () => { + expect(parseModelString(" openai / gpt-5.3-codex ")).toEqual({ + providerID: "openai", + modelID: "gpt-5.3-codex", + }) }) }) }) diff --git a/src/tools/delegate-task/model-string-parser.ts b/src/tools/delegate-task/model-string-parser.ts index 97d4f331..544fcdff 100644 --- a/src/tools/delegate-task/model-string-parser.ts +++ b/src/tools/delegate-task/model-string-parser.ts @@ -2,9 +2,21 @@ * Parse a model string in "provider/model" format. */ export function parseModelString(model: string): { providerID: string; modelID: string } | undefined { - const parts = model.split("/") - if (parts.length >= 2) { - return { providerID: parts[0], modelID: parts.slice(1).join("/") } + if (!model || !model.trim()) { + return undefined } - return undefined + + const slashIndex = model.indexOf("/") + if (slashIndex <= 0) { + return undefined + } + + const providerID = model.substring(0, slashIndex).trim() + const modelID = model.substring(slashIndex + 1).trim() + + if (!providerID || !modelID) { + return undefined + } + + return { providerID, modelID } }