refactor(shared,config): remove redundant null checks and AI slop from code comments

This commit is contained in:
YeonGyu-Kim
2026-04-03 19:39:55 +09:00
parent d0f795dd8f
commit e0feb16dab
9 changed files with 10 additions and 48 deletions
-1
View File
@@ -5,7 +5,6 @@ export const ExperimentalConfigSchema = z.object({
aggressive_truncation: z.boolean().optional(), aggressive_truncation: z.boolean().optional(),
auto_resume: z.boolean().optional(), auto_resume: z.boolean().optional(),
preemptive_compaction: z.boolean().optional(), preemptive_compaction: z.boolean().optional(),
/** Truncate all tool outputs, not just whitelisted tools (default: false). Tool output truncator is enabled by default - disable via disabled_hooks. */
truncate_all_tool_outputs: z.boolean().optional(), truncate_all_tool_outputs: z.boolean().optional(),
/** Dynamic context pruning configuration */ /** Dynamic context pruning configuration */
dynamic_context_pruning: DynamicContextPruningConfigSchema.optional(), dynamic_context_pruning: DynamicContextPruningConfigSchema.optional(),
@@ -41,7 +41,6 @@ describe("resolveActualContextLimit", () => {
modelContextLimitsCache, modelContextLimitsCache,
}) })
// then — models.dev reports 1M for GA models, resolver should respect it
expect(actualLimit).toBe(1_000_000) expect(actualLimit).toBe(1_000_000)
}) })
@@ -89,7 +88,6 @@ describe("resolveActualContextLimit", () => {
modelContextLimitsCache, modelContextLimitsCache,
}) })
// then — explicit 1M flag overrides cached 200K
expect(actualLimit).toBe(1_000_000) expect(actualLimit).toBe(1_000_000)
}) })
-2
View File
@@ -69,8 +69,6 @@ describe("checkForLegacyPluginEntry", () => {
}) })
it("returns no warning data when config is missing", () => { it("returns no warning data when config is missing", () => {
// given — empty dir, no config files
// when // when
const result = checkForLegacyPluginEntry(testConfigDir) const result = checkForLegacyPluginEntry(testConfigDir)
+1 -1
View File
@@ -118,7 +118,7 @@ export function migrateConfigFile(
fs.copyFileSync(configPath, backupPath) fs.copyFileSync(configPath, backupPath)
backupSucceeded = true backupSucceeded = true
} catch { } catch {
// Original file may not exist yet — skip backup backupSucceeded = false
} }
let writeSucceeded = false let writeSucceeded = false
+1 -2
View File
@@ -92,7 +92,7 @@ export function flattenToFallbackModelStrings(
// invalid strings like "provider/model high(low)". // invalid strings like "provider/model high(low)".
const model = entry.model const model = entry.model
.replace(/\([^()]+\)\s*$/, "") .replace(/\([^()]+\)\s*$/, "")
.replace(/\s+([a-z][a-z0-9_-]*)\s*$/i, (match, suffix) => { .replace(/\s+([a-z][a-z0-9_-]*)\s*$/i, (match: string, suffix: string) => {
const normalized = String(suffix).toLowerCase() const normalized = String(suffix).toLowerCase()
return KNOWN_VARIANTS.has(normalized) return KNOWN_VARIANTS.has(normalized)
? "" ? ""
@@ -101,7 +101,6 @@ export function flattenToFallbackModelStrings(
.trim() .trim()
return `${model}(${variant})` return `${model}(${variant})`
} }
// No explicit variant — preserve model string as-is (including any inline variant)
return entry.model return entry.model
}) })
} }
@@ -244,10 +244,6 @@ describe("resolveCompatibleModelSettings", () => {
expect(result.changes).toEqual([]) expect(result.changes).toEqual([])
}) })
// -----------------------------------------------------------------------
// Registry coverage — every model family from FAMILY_CAPABILITIES
// -----------------------------------------------------------------------
describe("model family registry coverage", () => { describe("model family registry coverage", () => {
const familyCases: Array<{ const familyCases: Array<{
name: string name: string
@@ -309,7 +305,6 @@ describe("resolveCompatibleModelSettings", () => {
} }
}) })
// GPT-5 specific: supports xhigh variant and xhigh reasoningEffort
test("GPT-5 keeps xhigh variant and reasoningEffort", () => { test("GPT-5 keeps xhigh variant and reasoningEffort", () => {
const result = resolveCompatibleModelSettings({ const result = resolveCompatibleModelSettings({
providerID: "openai", providerID: "openai",
@@ -345,7 +340,6 @@ describe("resolveCompatibleModelSettings", () => {
}) })
}) })
// Reasoning effort: "none" and "minimal" are valid per Vercel AI SDK
test("GPT-5 keeps none reasoningEffort", () => { test("GPT-5 keeps none reasoningEffort", () => {
const result = resolveCompatibleModelSettings({ const result = resolveCompatibleModelSettings({
providerID: "openai", providerID: "openai",
@@ -388,7 +382,6 @@ describe("resolveCompatibleModelSettings", () => {
}) })
}) })
// Reasoning effort downgrade within families that support it
test("o-series downgrades xhigh reasoningEffort to high", () => { test("o-series downgrades xhigh reasoningEffort to high", () => {
const result = resolveCompatibleModelSettings({ const result = resolveCompatibleModelSettings({
providerID: "openai", providerID: "openai",
@@ -408,9 +401,6 @@ describe("resolveCompatibleModelSettings", () => {
}) })
test("GPT-5 keeps xhigh but would downgrade a hypothetical beyond-max level", () => { test("GPT-5 keeps xhigh but would downgrade a hypothetical beyond-max level", () => {
// GPT-5 supports up to "xhigh" — verify the ladder works by requesting
// a value that IS in the ladder but NOT in the family's allowed list.
// Since "xhigh" is the max for GPT-5 reasoningEffort, we verify it stays.
const result = resolveCompatibleModelSettings({ const result = resolveCompatibleModelSettings({
providerID: "openai", providerID: "openai",
modelID: "gpt-5.4", modelID: "gpt-5.4",
+1 -13
View File
@@ -51,10 +51,6 @@ export type ModelSettingsCompatibilityResult = {
const VARIANT_LADDER = ["low", "medium", "high", "xhigh", "max"] const VARIANT_LADDER = ["low", "medium", "high", "xhigh", "max"]
const REASONING_LADDER = ["none", "minimal", "low", "medium", "high", "xhigh"] const REASONING_LADDER = ["none", "minimal", "low", "medium", "high", "xhigh"]
// ---------------------------------------------------------------------------
// Generic resolution — one function for both fields
// ---------------------------------------------------------------------------
function downgradeWithinLadder(value: string, allowed: string[], ladder: string[]): string | undefined { function downgradeWithinLadder(value: string, allowed: string[], ladder: string[]): string | undefined {
const requestedIndex = ladder.indexOf(value) const requestedIndex = ladder.indexOf(value)
if (requestedIndex === -1) return undefined if (requestedIndex === -1) return undefined
@@ -91,7 +87,6 @@ function resolveField(
familyKnown: boolean, familyKnown: boolean,
metadataOverride?: string[], metadataOverride?: string[],
): FieldResolution { ): FieldResolution {
// Priority 1: runtime metadata from provider
if (metadataOverride) { if (metadataOverride) {
if (metadataOverride.includes(normalized)) return { value: normalized } if (metadataOverride.includes(normalized)) return { value: normalized }
return { return {
@@ -100,7 +95,6 @@ function resolveField(
} }
} }
// Priority 2: family heuristic from registry
if (familyCaps) { if (familyCaps) {
if (familyCaps.includes(normalized)) return { value: normalized } if (familyCaps.includes(normalized)) return { value: normalized }
return { return {
@@ -109,24 +103,18 @@ function resolveField(
} }
} }
// Known family but field not in registry (e.g. Claude + reasoningEffort)
if (familyKnown) { if (familyKnown) {
return { value: undefined, reason: "unsupported-by-model-family" } return { value: undefined, reason: "unsupported-by-model-family" }
} }
// Unknown family — drop the value
return { value: undefined, reason: "unknown-model-family" } return { value: undefined, reason: "unknown-model-family" }
} }
// ---------------------------------------------------------------------------
// Public API
// ---------------------------------------------------------------------------
export function resolveCompatibleModelSettings( export function resolveCompatibleModelSettings(
input: ModelSettingsCompatibilityInput, input: ModelSettingsCompatibilityInput,
): ModelSettingsCompatibilityResult { ): ModelSettingsCompatibilityResult {
const family = detectHeuristicModelFamily(input.modelID) const family = detectHeuristicModelFamily(input.modelID)
const familyKnown = family !== undefined const familyKnown = Boolean(family)
const changes: ModelSettingsCompatibilityChange[] = [] const changes: ModelSettingsCompatibilityChange[] = []
const metadataVariants = normalizeCapabilitiesVariants(input.capabilities) const metadataVariants = normalizeCapabilitiesVariants(input.capabilities)
const metadataReasoningEfforts = normalizeCapabilitiesReasoningEfforts(input.capabilities) const metadataReasoningEfforts = normalizeCapabilitiesReasoningEfforts(input.capabilities)
-10
View File
@@ -93,7 +93,6 @@ export async function promptWithModelSuggestionRetry(
): Promise<void> { ): Promise<void> {
const timeoutMs = options.timeoutMs ?? PROMPT_TIMEOUT_MS const timeoutMs = options.timeoutMs ?? PROMPT_TIMEOUT_MS
const timeoutContext = createPromptTimeoutContext(args, timeoutMs) const timeoutContext = createPromptTimeoutContext(args, timeoutMs)
// NOTE: Model suggestion retry removed — promptAsync returns 204 immediately,
// model errors happen asynchronously server-side and cannot be caught here // model errors happen asynchronously server-side and cannot be caught here
const promptPromise = client.session.promptAsync({ const promptPromise = client.session.promptAsync({
...args, ...args,
@@ -115,15 +114,6 @@ export async function promptWithModelSuggestionRetry(
} }
} }
/**
* Synchronous variant of promptWithModelSuggestionRetry.
*
* Uses `session.prompt` (blocking HTTP call that waits for the LLM response)
* instead of `promptAsync` (fire-and-forget HTTP 204).
*
* Required by callers that need the response to be available immediately after
* the call returns — e.g. look_at, which reads session messages right away.
*/
export async function promptSyncWithModelSuggestionRetry( export async function promptSyncWithModelSuggestionRetry(
client: Client, client: Client,
args: PromptArgs, args: PromptArgs,
@@ -108,21 +108,21 @@ describe("isSqliteBackend", () => {
//#given //#given
versionReturnValue = true versionReturnValue = true
//#when: first call DB does not exist //#when: first call, DB does not exist
const first = isSqliteBackend() const first = isSqliteBackend()
//#then //#then
expect(first).toBe(false) expect(first).toBe(false)
expect(versionCheckCalls.length).toBe(1) expect(versionCheckCalls.length).toBe(1)
//#when: second call DB still does not exist (retry) //#when: second call, DB still does not exist (retry)
const second = isSqliteBackend() const second = isSqliteBackend()
//#then: retried once //#then: retried once
expect(second).toBe(false) expect(second).toBe(false)
expect(versionCheckCalls.length).toBe(2) expect(versionCheckCalls.length).toBe(2)
//#when: third call no more retries //#when: third call, no more retries
const third = isSqliteBackend() const third = isSqliteBackend()
//#then: no further checks //#then: no further checks
@@ -134,7 +134,7 @@ describe("isSqliteBackend", () => {
//#given //#given
versionReturnValue = true versionReturnValue = true
//#when: first call DB does not exist //#when: first call, DB does not exist
const first = isSqliteBackend() const first = isSqliteBackend()
//#then //#then
@@ -144,14 +144,14 @@ describe("isSqliteBackend", () => {
mkdirSync(join(TEST_DATA_DIR, "opencode"), { recursive: true }) mkdirSync(join(TEST_DATA_DIR, "opencode"), { recursive: true })
writeFileSync(DB_PATH, "") writeFileSync(DB_PATH, "")
//#when: second call retry finds DB //#when: second call, retry finds DB
const second = isSqliteBackend() const second = isSqliteBackend()
//#then: recovers to true and caches permanently //#then: recovers to true and caches permanently
expect(second).toBe(true) expect(second).toBe(true)
expect(versionCheckCalls.length).toBe(2) expect(versionCheckCalls.length).toBe(2)
//#when: third call cached true //#when: third call, cached true
const third = isSqliteBackend() const third = isSqliteBackend()
//#then: no further checks //#then: no further checks