refactor: remove AI slop from refactored files
Behavior-preserving cleanup of AI-generated code smells in 5 files authored/moved by this PR: - src/hooks/model-fallback/fallback-state-controller.ts (-47/+47 net reorganization, redundant defensiveness removed) - src/shared/model-string-parser.ts (-4 LOC obvious-comment cleanup) - src/shared/ripgrep-cli.ts (-13 LOC obvious comments + redundant defensive checks) - src/tools/delegate-task/tool-description.ts (-6 LOC) - src/tools/look-at/look-at-input-preparer.ts (-6 LOC) Targets: obvious comments that restate code, over-defensive null checks on guaranteed values, redundant existence checks. No public API signatures changed, no type hints removed, no new abstractions introduced. Full test suite still passes.
This commit is contained in:
@@ -53,9 +53,7 @@ export function createModelFallbackStateController(input: {
|
|||||||
): boolean {
|
): boolean {
|
||||||
const agentKey = getAgentConfigKey(agentName)
|
const agentKey = getAgentConfigKey(agentName)
|
||||||
const requirements = AGENT_MODEL_REQUIREMENTS[agentKey]
|
const requirements = AGENT_MODEL_REQUIREMENTS[agentKey]
|
||||||
const fallbackChain = sessionFallbackChains.has(sessionID)
|
const fallbackChain = sessionFallbackChains.get(sessionID) ?? requirements?.fallbackChain
|
||||||
? sessionFallbackChains.get(sessionID)
|
|
||||||
: requirements?.fallbackChain
|
|
||||||
|
|
||||||
if (!fallbackChain?.length) {
|
if (!fallbackChain?.length) {
|
||||||
log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")")
|
log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")")
|
||||||
@@ -63,30 +61,31 @@ export function createModelFallbackStateController(input: {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const existing = pendingModelFallbacks.get(sessionID)
|
const existing = pendingModelFallbacks.get(sessionID)
|
||||||
if (existing) {
|
if (!existing) {
|
||||||
if (existing.pending) {
|
pendingModelFallbacks.set(sessionID, {
|
||||||
log("[model-fallback] Pending fallback already armed for session: " + sessionID)
|
providerID: currentProviderID,
|
||||||
return false
|
modelID: currentModelID,
|
||||||
}
|
fallbackChain,
|
||||||
existing.providerID = currentProviderID
|
attemptCount: 0,
|
||||||
existing.modelID = currentModelID
|
pending: true,
|
||||||
existing.pending = true
|
})
|
||||||
if (existing.attemptCount >= existing.fallbackChain.length) {
|
log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName)
|
||||||
log("[model-fallback] Fallback chain exhausted for session: " + sessionID)
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
log("[model-fallback] Re-armed pending fallback for session: " + sessionID)
|
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
pendingModelFallbacks.set(sessionID, {
|
if (existing.pending) {
|
||||||
providerID: currentProviderID,
|
log("[model-fallback] Pending fallback already armed for session: " + sessionID)
|
||||||
modelID: currentModelID,
|
return false
|
||||||
fallbackChain,
|
}
|
||||||
attemptCount: 0,
|
|
||||||
pending: true,
|
existing.providerID = currentProviderID
|
||||||
})
|
existing.modelID = currentModelID
|
||||||
log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName)
|
existing.pending = true
|
||||||
|
if (existing.attemptCount >= existing.fallbackChain.length) {
|
||||||
|
log("[model-fallback] Fallback chain exhausted for session: " + sessionID)
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
log("[model-fallback] Re-armed pending fallback for session: " + sessionID)
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -41,13 +41,13 @@ export function parseModelString(
|
|||||||
const trimmedModel = model.trim()
|
const trimmedModel = model.trim()
|
||||||
if (!trimmedModel) return undefined
|
if (!trimmedModel) return undefined
|
||||||
|
|
||||||
const parts = trimmedModel.split("/")
|
const separatorIndex = trimmedModel.indexOf("/")
|
||||||
if (parts.length < 2) {
|
if (separatorIndex === -1) {
|
||||||
return undefined
|
return undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
const providerID = parts[0]?.trim()
|
const providerID = trimmedModel.slice(0, separatorIndex).trim()
|
||||||
const rawModelID = parts.slice(1).join("/").trim()
|
const rawModelID = trimmedModel.slice(separatorIndex + 1).trim()
|
||||||
if (!providerID || !rawModelID) {
|
if (!providerID || !rawModelID) {
|
||||||
return undefined
|
return undefined
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,7 +28,7 @@ function findExecutable(name: string): string | null {
|
|||||||
return result.stdout.trim().split("\n")[0]
|
return result.stdout.trim().split("\n")[0]
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
// Command execution failed
|
return null
|
||||||
}
|
}
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
@@ -62,21 +62,9 @@ export function resolveGrepCli(): ResolvedCli {
|
|||||||
return cachedCli
|
return cachedCli
|
||||||
}
|
}
|
||||||
|
|
||||||
const bundledRg = getOpenCodeBundledRg()
|
const rgPath = getOpenCodeBundledRg() ?? findExecutable("rg") ?? getInstalledRipgrepPath()
|
||||||
if (bundledRg) {
|
if (rgPath) {
|
||||||
cachedCli = { path: bundledRg, backend: "rg" }
|
cachedCli = { path: rgPath, backend: "rg" }
|
||||||
return cachedCli
|
|
||||||
}
|
|
||||||
|
|
||||||
const systemRg = findExecutable("rg")
|
|
||||||
if (systemRg) {
|
|
||||||
cachedCli = { path: systemRg, backend: "rg" }
|
|
||||||
return cachedCli
|
|
||||||
}
|
|
||||||
|
|
||||||
const installedRg = getInstalledRipgrepPath()
|
|
||||||
if (installedRg) {
|
|
||||||
cachedCli = { path: installedRg, backend: "rg" }
|
|
||||||
return cachedCli
|
return cachedCli
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -108,14 +96,16 @@ export async function resolveGrepCliWithAutoInstall(): Promise<ResolvedCli> {
|
|||||||
cachedCli = { path: rgPath, backend: "rg" }
|
cachedCli = { path: rgPath, backend: "rg" }
|
||||||
return cachedCli
|
return cachedCli
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
const message = error instanceof Error ? error.message : String(error)
|
||||||
|
|
||||||
if (current.backend === "grep") {
|
if (current.backend === "grep") {
|
||||||
log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep. Falling back to GNU grep.`, {
|
log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep. Falling back to GNU grep.`, {
|
||||||
error: error instanceof Error ? error.message : String(error),
|
error: message,
|
||||||
grep_path: current.path,
|
grep_path: current.path,
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep and GNU grep was not found.`, {
|
log(`[${PUBLISHED_PACKAGE_NAME}] Failed to auto-install ripgrep and GNU grep was not found.`, {
|
||||||
error: error instanceof Error ? error.message : String(error),
|
error: message,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -13,28 +13,26 @@ export interface DelegateTaskPresentation {
|
|||||||
export function createDelegateTaskPresentation(options: DelegateTaskToolOptions): DelegateTaskPresentation {
|
export function createDelegateTaskPresentation(options: DelegateTaskToolOptions): DelegateTaskPresentation {
|
||||||
const { userCategories } = options
|
const { userCategories } = options
|
||||||
const allCategories = mergeCategories(userCategories)
|
const allCategories = mergeCategories(userCategories)
|
||||||
const categoryNames = Object.keys(allCategories)
|
const categoryEntries = Object.entries(allCategories).map(([name, categoryConfig]) => ({
|
||||||
|
name,
|
||||||
|
categoryConfig,
|
||||||
|
description: userCategories?.[name]?.description || CATEGORY_DESCRIPTIONS[name],
|
||||||
|
}))
|
||||||
|
const categoryNames = categoryEntries.map(({ name }) => name)
|
||||||
const categoryExamples = categoryNames.join(", ")
|
const categoryExamples = categoryNames.join(", ")
|
||||||
|
|
||||||
const availableCategories: AvailableCategory[] = options.availableCategories
|
const availableCategories: AvailableCategory[] = options.availableCategories
|
||||||
?? Object.entries(allCategories).map(([name, categoryConfig]) => {
|
?? categoryEntries.map(({ name, categoryConfig, description }) => {
|
||||||
const userDescription = userCategories?.[name]?.description
|
|
||||||
const builtinDescription = CATEGORY_DESCRIPTIONS[name]
|
|
||||||
const description = userDescription || builtinDescription || "General tasks"
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
name,
|
name,
|
||||||
description,
|
description: description || "General tasks",
|
||||||
model: categoryConfig.model,
|
model: categoryConfig.model,
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
const availableSkills: AvailableSkill[] = options.availableSkills ?? []
|
const availableSkills: AvailableSkill[] = options.availableSkills ?? []
|
||||||
|
|
||||||
const categoryList = categoryNames.map(name => {
|
const categoryList = categoryEntries.map(({ name, description }) => {
|
||||||
const userDescription = userCategories?.[name]?.description
|
|
||||||
const builtinDescription = CATEGORY_DESCRIPTIONS[name]
|
|
||||||
const description = userDescription || builtinDescription
|
|
||||||
return description ? ` - ${name}: ${description}` : ` - ${name}`
|
return description ? ` - ${name}: ${description}` : ` - ${name}`
|
||||||
}).join("\n")
|
}).join("\n")
|
||||||
|
|
||||||
|
|||||||
@@ -101,17 +101,16 @@ export function prepareLookAtInput(args: LookAtArgs): PrepareLookAtInputResult {
|
|||||||
if (filePath) {
|
if (filePath) {
|
||||||
let mimeType = inferMimeTypeFromFilePath(filePath)
|
let mimeType = inferMimeTypeFromFilePath(filePath)
|
||||||
let actualFilePath = filePath
|
let actualFilePath = filePath
|
||||||
let tempFilePath: string | null = null
|
|
||||||
let tempConversionPath: string | null = null
|
let tempConversionPath: string | null = null
|
||||||
|
|
||||||
if (needsConversion(mimeType)) {
|
if (needsConversion(mimeType)) {
|
||||||
log(`[look_at] Detected unsupported format: ${mimeType}, converting to JPEG...`)
|
log(`[look_at] Detected unsupported format: ${mimeType}, converting to JPEG...`)
|
||||||
try {
|
try {
|
||||||
tempFilePath = convertImageToJpeg(filePath, mimeType)
|
const convertedFilePath = convertImageToJpeg(filePath, mimeType)
|
||||||
tempConversionPath = tempFilePath
|
tempConversionPath = convertedFilePath
|
||||||
actualFilePath = tempFilePath
|
actualFilePath = convertedFilePath
|
||||||
mimeType = "image/jpeg"
|
mimeType = "image/jpeg"
|
||||||
log(`[look_at] Conversion successful: ${tempFilePath}`)
|
log(`[look_at] Conversion successful: ${convertedFilePath}`)
|
||||||
} catch (conversionError) {
|
} catch (conversionError) {
|
||||||
const failedConversionPath = getTemporaryConversionPath(conversionError)
|
const failedConversionPath = getTemporaryConversionPath(conversionError)
|
||||||
if (failedConversionPath) {
|
if (failedConversionPath) {
|
||||||
@@ -139,8 +138,6 @@ export function prepareLookAtInput(args: LookAtArgs): PrepareLookAtInputResult {
|
|||||||
cleanup() {
|
cleanup() {
|
||||||
if (tempConversionPath) {
|
if (tempConversionPath) {
|
||||||
cleanupConvertedImage(tempConversionPath)
|
cleanupConvertedImage(tempConversionPath)
|
||||||
} else if (tempFilePath) {
|
|
||||||
cleanupConvertedImage(tempFilePath)
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user