fix(build): resolve ESLint compliance and TypeScript compilation errors - #130
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRefactors evaluation APIs, analytics/token usage types, and provider implementations; introduces MCP-first generation/streaming in NeuroLink with new health and tool metrics; tightens CLI typing and error handling; overhauls SageMaker error/usage shapes; expands Ollama provider and readiness checks; restructures model/config factories; removes several report docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as CLI/SDK Caller
participant Eval as Evaluation API
participant LLM as Provider (eval model)
User->>App: evaluateResponse(context)
App->>Eval: generateEvaluation(context)
Eval->>LLM: Build prompt + request
LLM-->>Eval: Raw evaluation output
Eval->>Eval: parseEvaluationResult + schema validate
Eval-->>App: EvaluationResult {scores, context, reasoning}
App-->>User: Display relevance/accuracy/completeness/overall
note over Eval: New single-arg API and enriched result fields
sequenceDiagram
autonumber
actor User
participant CLI as CLICommandFactory
participant Exec as Command Executor
participant Err as handleError
User->>CLI: neurolink generate|stream (args)
CLI->>Exec: executeGenerate/executeStream(specified args)
Exec->>Exec: Run provider call\ncollect analytics (TokenUsage, requestDuration)
alt success
Exec-->>User: Output (json/table/yaml)
else failure
Exec->>Err: handleError(error, context)
Err-->>User: Standardized error message
end
note over Exec,Err: Centralized error handling and YAML support
sequenceDiagram
autonumber
actor Dev
participant NL as NeuroLink
participant MCP as External MCP
participant Prov as Direct Provider
Dev->>NL: generateText(...)
NL->>NL: attemptMCPGenerationWithRetries
NL->>MCP: Execute tools / servers
alt MCP yields content
MCP-->>NL: Content + tool metrics
NL->>NL: Store in memory
else fallback
NL->>Prov: Direct generation
Prov-->>NL: Content
end
NL-->>Dev: Result + analytics + evaluation
note over NL: MCP-first, then provider fallback
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 38
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (46)
src/lib/middleware/builtin/guardrails.ts (1)
49-55: Escape bad-word patterns; current RegExp construction is unsafe and can misfire.If any term includes regex metacharacters (e.g., ".", "[]", "(", "|"), it will alter the pattern or throw. Escape terms before building the regex.
- for (const term of config.badWords.list) { - const regex = new RegExp(term, "gi"); - filteredText = filteredText?.replace(regex, "*".repeat(term.length)); - } + for (const term of config.badWords.list) { + const regex = new RegExp(escapeRegExp(term), "gi"); + filteredText = filteredText?.replace(regex, "*".repeat(term.length)); + }Add this helper (top-level in this file):
function escapeRegExp(s: string) { return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); }Optional perf upgrade: precompile once at middleware creation time into
{re, mask}pairs and reuse in both generate and stream paths.src/lib/middleware/registry.ts (1)
205-212: Remove unused merged config to satisfy zero-warning goal
_mergedConfigis computed but never used; this will trip no-unused-vars in strict ESLint setups. Either thread it through to calls or remove it. Minimal fix below removes it now; reintroduce when you actually use it.- // Merge configurations: global < middleware config < runtime config - const _mergedConfig = { - ...globalConfig, - ...config?.config, - }; + // Merge configurations would be applied here if/when middleware methods accept config.src/lib/utils/timeout.ts (2)
407-433: Streaming timeout is ineffective; race occurs after awaiting the item. Race generator.next() instead.The current code does for await (const item ...) then races Promise.resolve(item) vs timeout, which resolves immediately. Time spent waiting for the next chunk isn’t timed out. Fix by racing generator.next() against a single stream-wide timeout. Also prefer ReturnType and unref the timer.
- let timeoutId: NodeJS.Timeout | undefined; + let timeoutId: ReturnType<typeof setTimeout> | undefined; const timeoutPromise = new Promise<never>((_, reject) => { timeoutId = setTimeout(() => { reject( new TimeoutError( `${provider} streaming operation timed out after ${timeoutMs}ms`, timeoutMs, provider, "stream", ), ); }, timeoutMs); + if (typeof (timeoutId as any)?.unref === "function") { + (timeoutId as any).unref(); + } }); try { - for await (const item of generator) { - const raceResult = await Promise.race([ - Promise.resolve(item), - timeoutPromise, - ]); - yield raceResult; - } + while (true) { + const next = (await Promise.race([generator.next(), timeoutPromise])) as IteratorResult<T>; + if (next.done) break; + yield next.value; + } } finally { if (timeoutId) { clearTimeout(timeoutId); } }
303-305: Replace deprecated substr with slice to satisfy ESLint (prefer-string-slice).This can trip “prefer-string-slice” and similar rules; slice is the modern, standard approach.
- return `${operation}-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + return `${operation}-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;src/lib/core/dynamicModels.ts (2)
216-217: Fix localhost health URL construction.
a || balways takes the left non-empty string; you may send HEAD to the root instead of/health.- const healthUrl = url.replace(/\/api\/.*$/, "/health") || `${url}/health`; + const hasApiSegment = /\/api\/[^/]+/.test(url); + const base = hasApiSegment ? url.replace(/\/api\/.*$/, "") : url.replace(/\/$/, ""); + const healthUrl = `${base}/health`;
176-205: File-read timeout is ineffective (sync I/O blocks timers). Use fs.promises and guard double-settle.With
readFileSync, the timeout can’t fire while the event loop is blocked.- return new Promise((resolve, reject) => { - const timeoutId = setTimeout(() => { - reject(new Error(`File read timeout after ${timeoutMs}ms`)); - }, timeoutMs); - - (async () => { - try { - const fs = await import("fs"); - const path = await import("path"); - - const fullPath = path.resolve(source); - - // Check if file exists first - if (!fs.existsSync(fullPath)) { - throw new Error(`File not found: ${fullPath}`); - } - - const content = fs.readFileSync(fullPath, "utf8"); - const data = JSON.parse(content); - - clearTimeout(timeoutId); - resolve(data); - } catch (error) { - clearTimeout(timeoutId); - reject(error); - } - })(); - }); + return new Promise((resolve, reject) => { + let settled = false; + const timeoutId = setTimeout(() => { + if (settled) return; + settled = true; + reject(new Error(`File read timeout after ${timeoutMs}ms`)); + }, timeoutMs); + + (async () => { + try { + const fsp = await import("fs/promises"); + const path = await import("path"); + const fullPath = path.resolve(source); + + await fsp.access(fullPath).catch(() => { + throw new Error(`File not found: ${fullPath}`); + }); + + const content = await fsp.readFile(fullPath, "utf8"); + const data = JSON.parse(content); + if (settled) return; + settled = true; + clearTimeout(timeoutId); + resolve(data); + } catch (error) { + if (settled) return; + settled = true; + clearTimeout(timeoutId); + reject(error); + } + })(); + });src/lib/utils/performance.ts (3)
86-88: Fix falsy duration check (0ms currently reported as “No metrics available”).Duration can be 0; the truthiness check suppresses valid results.
Apply this diff:
- if (!metric || !metric.duration) { + if (!metric || metric.duration === undefined) { return `${operationName}: No metrics available`; }
271-276: Also fix CLI delta sign formatting.Same signage issue appears in CLI logs.
Apply this diff:
- logger.debug( - ` Delta: +${endMemory.heapUsed - startMemory.heapUsed}MB`, - ); + const deltaMB = endMemory.heapUsed - startMemory.heapUsed; + logger.debug(` Delta: ${deltaMB >= 0 ? "+" : "−"}${Math.abs(deltaMB)}MB`);
121-127: Type-safe access to global.gc to satisfy TS/ESLint.
global.gcisn’t in Node typings and may trip TS/ESLint. UseglobalThiswith a narrowed shape.Apply this diff:
static forceGC(): boolean { - if (typeof global !== "undefined" && global.gc) { - global.gc(); - return true; - } - return false; + const g = globalThis as unknown as { gc?: () => void }; + if (g.gc) { + g.gc(); + return true; + } + return false; }src/lib/providers/ollama.ts (4)
168-176: Guard against undefineddata.responseto avoid NaN token counts.
estimateTokensis called with possibly undefineddata.response, which can throw or yield incorrect usage metrics.Apply this diff:
- return { - text: data.response, + const responseText = typeof data.response === "string" ? data.response : ""; + return { + text: responseText, usage: { - promptTokens: data.prompt_eval_count || this.estimateTokens(prompt), - completionTokens: data.eval_count || this.estimateTokens(data.response), - totalTokens: - (data.prompt_eval_count || this.estimateTokens(prompt)) + - (data.eval_count || this.estimateTokens(data.response)), + promptTokens: data.prompt_eval_count ?? this.estimateTokens(prompt), + completionTokens: data.eval_count ?? this.estimateTokens(responseText), + totalTokens: + (data.prompt_eval_count ?? this.estimateTokens(prompt)) + + (data.eval_count ?? this.estimateTokens(responseText)), },
779-789: Map AbortError to TimeoutError and use the class timeout, notdefaultTimeout.Fetch timeouts raise AbortError; current check misses them, and
defaultTimeoutmay be undefined here.Apply this diff:
- if ((error as Error).name === "TimeoutError") { - return new TimeoutError( - `Ollama request timed out. The model might be loading or the request is too complex.`, - this.defaultTimeout, - ); - } + const name = (error as Error).name || ""; + const message = (error as Error).message || ""; + if ( + name === "TimeoutError" || + name === "AbortError" || + message.includes("The operation was aborted") + ) { + return new TimeoutError( + `Ollama request timed out. The model might be loading or the request is too complex.`, + this.timeout, + ); + } @@ - if ( - (error as Error).message?.includes("ECONNREFUSED") || - (error as Error).message?.includes("fetch failed") - ) { + if (message.includes("ECONNREFUSED") || message.includes("fetch failed")) {
387-420: Fix undefinedmodelNameaccess and honor env-configured tool-capable models.Prevents crash when
modelNameis unset and aligns with the documented env variable support.Apply this diff:
- supportsTools(): boolean { - const modelName = this.modelName.toLowerCase(); + supportsTools(): boolean { + const modelName = (this.modelName ?? this.getDefaultModel()).toLowerCase(); @@ - const ollamaConfig = modelConfig.getProviderConfig("ollama"); - const toolCapableModels = - (ollamaConfig?.modelBehavior?.toolCapableModels as string[]) || []; + const ollamaConfig = modelConfig.getProviderConfig("ollama"); + const envVar = process.env.OLLAMA_TOOL_CAPABLE_MODELS; + const envModels = envVar + ? envVar.split(",").map((s) => s.trim().toLowerCase()).filter(Boolean) + : []; + const configModels = + (ollamaConfig?.modelBehavior?.toolCapableModels as string[] | undefined) ?? + []; + const toolCapableModels = [ + ...new Set([...envModels, ...configModels.map((m) => m.toLowerCase())]), + ]; + const defaultToolModels = [ + "llama3.1:8b-instruct", + "mistral:7b-instruct", + "hermes3:8b", + ]; @@ - const isToolCapable = toolCapableModels.some((capableModel) => - modelName.includes(capableModel), - ); + const candidates = + toolCapableModels.length > 0 ? toolCapableModels : defaultToolModels; + const isToolCapable = candidates.some((capableModel) => + modelName.includes(capableModel), + ); @@ - availableToolModels: toolCapableModels.slice(0, 3), // Show first 3 for brevity + availableToolModels: candidates.slice(0, 3), // Show first 3 for brevity
192-199: Add “DOM” to tsconfig.cli.json’s lib or import ReadableStream from “stream/web”tsconfig.cli.json currently has no
libentry, so references to the DOM’sReadableStreamtype will cause TS errors. Either
- add
to tsconfig.cli.json"lib": ["DOM", "DOM.Iterable", "ES2022"]
or- import
ReadableStreamfrom"stream/web"in src/lib/providers/ollama.tssrc/lib/utils/providerUtils.ts (2)
139-141: Honor OLLAMA_MODEL env or any available model; current hard-coded tag can cause false negatives.
isProviderAvailable("ollama")only checks forllama3.2:latest, ignoringprocess.env.OLLAMA_MODELand other installed tags. This can incorrectly report Ollama as unavailable.Apply this diff:
- const { models } = await response.json(); - const defaultOllamaModel = "llama3.2:latest"; - return models.some((m: UnknownRecord) => m.name === defaultOllamaModel); + const { models } = await response.json(); + const configured = process.env.OLLAMA_MODEL || "llama3.2:latest"; + if (!Array.isArray(models)) return false; + const names = models + .map((m: UnknownRecord) => m?.name) + .filter((n: unknown): n is string => typeof n === "string"); + // available if configured model is present OR at least one model exists + return names.includes(configured) || names.length > 0;
217-221: Case label never matches due to.toLowerCase(); fix "azureOpenai" to "azureopenai".
switch (provider.toLowerCase())makes"azureOpenai"unreachable in both validators; Azure env checks can be skipped silently.Apply this diff in both switches:
- case "azureOpenai": + case "azureopenai":Also applies to: 491-493
src/lib/mcp/servers/utilities/utilityServer.ts (1)
94-99: Display string can misreport timezone for ISO/UTC or be misleading.
displayStringalways useseffectiveTimezone(often defaulting to "Asia/Kolkata"), even whenformat === "UTC"orformat === "ISO". This yields incorrect messaging.Apply this diff to make the display consistent with the requested format:
- const displayDateString = now.toLocaleString("en-US", { - timeZone: effectiveTimezone, - }); - resultData.displayString = `The current time is ${displayDateString} in ${effectiveTimezone}`; + let displayDateString: string; + let displayTzLabel: string; + if (format === "UTC") { + displayDateString = now.toLocaleString("en-US", { timeZone: "UTC" }); + displayTzLabel = "UTC"; + } else if (format === "local") { + displayDateString = now.toLocaleString("en-US", { + timeZone: effectiveTimezone, + }); + displayTzLabel = String(effectiveTimezone); + } else { + // ISO: present the ISO string and label accordingly + displayDateString = now.toISOString(); + displayTzLabel = "ISO (UTC)"; + } + resultData.displayString = `The current time is ${displayDateString} in ${displayTzLabel}`;src/lib/providers/huggingFace.ts (1)
162-177: Enhanced system prompt is computed but not applied to messages.
prepareStreamOptionsreturns an enhancedsystem, butbuildMessagesArray(options)uses the originaloptions.systemPrompt. Tool-calling guidance is dropped.Apply this diff to pass the enhanced system prompt to message building:
- const streamOptions = this.prepareStreamOptions(options, analysisSchema); - - // Build message array from options - const messages = buildMessagesArray(options); + const streamOptions = this.prepareStreamOptions(options, analysisSchema); + // Build message array with enhanced system prompt + const messages = buildMessagesArray({ + ...options, + systemPrompt: (streamOptions as { system?: string }).system ?? options.systemPrompt, + });src/lib/providers/googleAiStudio.ts (1)
68-94: Unify error handling with other providers: return, don’t throw, inside handleProviderError.Elsewhere (e.g., Mistral)
handleProviderErrorreturns an Error and the caller doesthrow this.handleProviderError(error). Throwing inside this method is inconsistent and makes control flow harder to reason about.- protected handleProviderError(error: unknown): Error { - if (error instanceof TimeoutError) { - throw new NetworkError(error.message, this.providerName); - } + protected handleProviderError(error: unknown): Error { + if (error instanceof TimeoutError) { + return new NetworkError(error.message, this.providerName); + } ... - if (message.includes("API_KEY_INVALID")) { - throw new AuthenticationError( + if (message.includes("API_KEY_INVALID")) { + return new AuthenticationError( "Invalid Google AI API key. Please check your GOOGLE_AI_API_KEY environment variable.", this.providerName, ); } ... - if (message.includes("RATE_LIMIT_EXCEEDED")) { - throw new RateLimitError( + if (message.includes("RATE_LIMIT_EXCEEDED")) { + return new RateLimitError( "Google AI rate limit exceeded. Please try again later.", this.providerName, ); } - - throw new ProviderError(`Google AI error: ${message}`, this.providerName); + return new ProviderError(`Google AI error: ${message}`, this.providerName); }src/lib/core/evaluationProviders.ts (1)
97-125: sortProvidersByPreference mutates input and lacks deterministic tiebreaker
- Sorting in-place can surprise callers.
- Add a stable tiebreaker (provider name) to avoid non-deterministic ordering when scores tie.
- Prefer nullish coalescing for clarity.
-export function sortProvidersByPreference( +export function sortProvidersByPreference( providers: ProviderModelConfig[], preferCheap: boolean = true, ): ProviderModelConfig[] { - return providers.sort((a, b) => { + return [...providers].sort((a, b) => { const aPerf = a.performance || { cost: 0, speed: 0, quality: 0 }; const bPerf = b.performance || { cost: 0, speed: 0, quality: 0 }; if (preferCheap) { // Cost > Speed > Quality for cheap preference - if ((aPerf.cost || 0) !== (bPerf.cost || 0)) { - return (bPerf.cost || 0) - (aPerf.cost || 0); + if ((aPerf.cost ?? 0) !== (bPerf.cost ?? 0)) { + return (bPerf.cost ?? 0) - (aPerf.cost ?? 0); } - if ((aPerf.speed || 0) !== (bPerf.speed || 0)) { - return (bPerf.speed || 0) - (aPerf.speed || 0); + if ((aPerf.speed ?? 0) !== (bPerf.speed ?? 0)) { + return (bPerf.speed ?? 0) - (aPerf.speed ?? 0); } - return (bPerf.quality || 0) - (aPerf.quality || 0); + const delta = (bPerf.quality ?? 0) - (aPerf.quality ?? 0); + return delta !== 0 ? delta : a.provider.localeCompare(b.provider); } else { // Quality > Speed > Cost for quality preference - if ((aPerf.quality || 0) !== (bPerf.quality || 0)) { - return (bPerf.quality || 0) - (aPerf.quality || 0); + if ((aPerf.quality ?? 0) !== (bPerf.quality ?? 0)) { + return (bPerf.quality ?? 0) - (aPerf.quality ?? 0); } - if ((aPerf.speed || 0) !== (bPerf.speed || 0)) { - return (bPerf.speed || 0) - (aPerf.speed || 0); + if ((aPerf.speed ?? 0) !== (bPerf.speed ?? 0)) { + return (bPerf.speed ?? 0) - (aPerf.speed ?? 0); } - return (bPerf.cost || 0) - (aPerf.cost || 0); + const delta = (bPerf.cost ?? 0) - (aPerf.cost ?? 0); + return delta !== 0 ? delta : a.provider.localeCompare(b.provider); } }); }src/lib/providers/sagemaker/types.ts (2)
110-118: Align usage field naming: total vs totalTokensSageMakerUsage now uses total. Later in this file, SageMakerGenerateResult.usage still exposes totalTokens, causing an inconsistent public surface.
export interface SageMakerUsage { /** Number of prompt tokens */ promptTokens: number; /** Number of completion tokens */ completionTokens: number; /** Total tokens used */ - total: number; + total: number;(Keep as total here; see next diff to align the generate result.)
471-475: RenametotalTokens→totalacross the codebase- usage: { - promptTokens: number; - completionTokens: number; - totalTokens?: number; - }; + usage: { + promptTokens: number; + completionTokens: number; + total?: number; + };This change impacts ~40 occurrences of
totalTokens(in tests, utils, providers, CLI output, core/baseProvider, examples, etc.). Update all references or provide a migration alias to avoid downstream type errors.src/lib/providers/azureOpenai.ts (1)
78-92: 401 detection via substring is brittle; check status fields insteadRely on status/response.status where available and fall back to message.
- protected handleProviderError(error: unknown): Error { - const errorObj = error as UnknownRecord; - if ( - errorObj?.message && - typeof errorObj.message === "string" && - errorObj.message.includes("401") - ) { - return new Error("Invalid Azure OpenAI API key or endpoint."); - } - const message = - errorObj?.message && typeof errorObj.message === "string" - ? errorObj.message - : "Unknown error"; - return new Error(`Azure OpenAI error: ${message}`); - } + protected handleProviderError(error: unknown): Error { + const err = error as { status?: number; response?: { status?: number }; message?: string }; + const status = err?.response?.status ?? err?.status; + if (status === 401) { + return new Error("Invalid Azure OpenAI API key or endpoint."); + } + const message = typeof err?.message === "string" ? err.message : "Unknown error"; + return new Error(`Azure OpenAI error: ${message}`); + }src/lib/mcp/servers/aiProviders/aiAnalysisTools.ts (2)
164-169: Guard object spread: result.usage can be undefined (will throw at runtime).Spreading
undefinedin an object literal throws a TypeError. Provide a safe default.Apply this diff:
- return { + const baseUsage = result.usage ?? { input: 0, output: 0, total: 0 }; + return { success: true, data: { ...parsedData, generatedAt: new Date().toISOString(), sessionId: context.sessionId, }, - usage: { - ...result.usage, + usage: { + ...baseUsage, executionTime, provider: providerName, model: "analysis-engine", },
101-109: Prefer schema.parse over type assertion to enforce defaults and validation.Rely on Zod to apply defaults and validate input for each tool.
Apply this diff pattern to each tool:
- const typedParams = params as AnalyzeUsageParams; + const typedParams = AnalyzeUsageSchema.parse(params);- const typedParams = params as BenchmarkParams; + const typedParams = BenchmarkSchema.parse(params);- const typedParams = params as OptimizeParametersParams; + const typedParams = OptimizeParametersSchema.parse(params);Also applies to: 201-209, 311-319
src/lib/providers/sagemaker/client.ts (1)
364-392: Node stream handling is broken;pipe-style Readable doesn’t have getReader().
convertReadableStreamearly-returns for Node streams, dropping data. Support Node.js AsyncIterable Readable.Apply:
- private async *convertReadableStream( - streamObj: Record<string, unknown>, - ): AsyncIterable<Uint8Array> { - const reader = - typeof streamObj.getReader === "function" - ? streamObj.getReader() - : undefined; - - if (!reader) { - return; // No valid reader available - } - - try { - while (true) { - const { done, value } = await reader.read(); - if (done) { - break; - } - - if (value instanceof Uint8Array) { - yield value; - } else if (typeof value === "string") { - yield new TextEncoder().encode(value); - } - } - } finally { - reader.releaseLock?.(); - } - } + private async *convertReadableStream( + streamObj: Record<string, unknown>, + ): AsyncIterable<Uint8Array> { + // Web ReadableStream with getReader() + if (typeof (streamObj as any).getReader === "function") { + const reader = (streamObj as any).getReader(); + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + if (value instanceof Uint8Array) yield value; + else if (typeof value === "string") yield new TextEncoder().encode(value); + else yield new TextEncoder().encode(String(value)); + } + } finally { + reader.releaseLock?.(); + } + return; + } + + // Node.js Readable (AsyncIterable) + if (streamObj && typeof (streamObj as any)[Symbol.asyncIterator] === "function") { + for await (const chunk of streamObj as any) { + if (chunk instanceof Uint8Array) yield chunk; + else if (typeof chunk === "string") yield new TextEncoder().encode(chunk); + else if (typeof (globalThis as any).Buffer !== "undefined" && (globalThis as any).Buffer.isBuffer?.(chunk)) { + yield new Uint8Array(chunk); + } else { + yield new TextEncoder().encode(String(chunk)); + } + } + return; + } + // Unsupported + return; + }src/lib/mcp/servers/aiProviders/aiWorkflowTools.ts (3)
117-123: Unused interface_TestCase(ESLint no-unused).Either remove or use it to type
testCases.Apply:
-interface _TestCase { +interface _TestCase { name: string; type: string; code: string; description: string; assertions: number; }And use it in the generator (see next comment).
231-238: Harden AI JSON parsing; models often wrap JSON in code fences. Also typetestCases.Raw
JSON.parse(result.content)is brittle.Apply:
- const aiResponse = JSON.parse(result.content); - const testCases = aiResponse.testCases || []; + const aiResponse = safeParseJSON(result.content); + const testCases: _TestCase[] = Array.isArray(aiResponse.testCases) + ? aiResponse.testCases + : [];Add helper outside this hunk:
// Robustly parse JSON that may be wrapped in code fences/backticks function safeParseJSON(input: string): any { const trimmed = input.trim(); const fence = /^```(?:json)?\s*([\s\S]*?)\s*```$/i; const match = trimmed.match(fence); const raw = match ? match[1] : trimmed; try { return JSON.parse(raw); } catch { // Fallback: try to extract first {...} block const start = raw.indexOf("{"); const end = raw.lastIndexOf("}"); if (start >= 0 && end > start) { return JSON.parse(raw.slice(start, end + 1)); } throw new Error("Failed to parse AI JSON response"); } }
367-370: Preserve zero values; use nullish coalescing instead of||.
complexityReductionof 0 currently becomes 15;readabilityScore0 becomes 85.Apply:
- linesReduced: aiResponse.metrics?.linesReduced || 0, - complexityReduction: aiResponse.metrics?.complexityReduction || 15, - readabilityScore: aiResponse.metrics?.readabilityScore || 85, + linesReduced: aiResponse.metrics?.linesReduced ?? 0, + complexityReduction: aiResponse.metrics?.complexityReduction ?? 15, + readabilityScore: aiResponse.metrics?.readabilityScore ?? 85,src/lib/providers/sagemaker/parsers.ts (1)
735-738: Operator precedence bug: finishReason computation is wrong.
a || b ? 'stop' : undefinedparses asa || (b ? 'stop' : undefined). You likely want “stop” when either condition is truthy.Apply:
- finishReason: - data.finish_reason || data.status === "complete" ? "stop" : undefined, + finishReason: + (data.finish_reason || data.status === "complete") ? "stop" : undefined,Optional: if you need to map
finish_reasonvalues (e.g., "length"), perform explicit mapping instead of forcing "stop".src/lib/core/types.ts (2)
146-151: Remove unused, conflictingStreamingOptionsinterface.You already import
StreamOptionselsewhere; this duplicate type is unused and confusing.Apply:
-/** - * Options for AI requests with unified provider configuration - */ -export interface StreamingOptions { - providers: ProviderConfig[]; - temperature?: number; - maxTokens?: number; - systemPrompt?: string; -}
61-65: Update provider model identifiers
- AWS Bedrock Anthropic Claude 3.7 Sonnet constant is already correct.
- Vertex AI Anthropic models: replace with
- claude-3-7-sonnet@20250219
- claude-opus-4@20250514
- claude-sonnet-4@20250514
- Gemini 2.5 on Vertex/AI Studio: use
- gemini-2.5-pro
- gemini-2.5-flash
- gemini-2.5-flash-lite
Apply fixes in src/lib/core/types.ts (lines 61–65, 83–99, 112–124, 269–282).
src/lib/core/modelConfiguration.ts (2)
833-841: Tie YAML branch to the real parser (or block it).Make sure the loader uses the fixed
parseYAMLContent.No code change needed if you apply the previous diff; otherwise, explicitly throw for
.yaml/.ymlto avoid silent misloads.
1114-1117: Require all env vars, not just one, to mark a provider “available”.Using
.somemeans a provider is “available” if any single required env var is set.Apply:
- return config.requiredEnvVars.some((envVar) => - Boolean(process.env[envVar]), - ); + return config.requiredEnvVars.every((envVar) => Boolean(process.env[envVar]));src/lib/providers/sagemaker/language-model.ts (1)
168-172: Usage shape inconsistency: usetotal, nottotalTokens.Other paths (batch/parallel) and analytics standardize on
total. Keep this file consistent to avoid type drift and downstream adapters.Apply:
- usage: { - promptTokens: number; - completionTokens: number; - totalTokens?: number; - }; + usage: { + promptTokens: number; + completionTokens: number; + total?: number; + }; @@ - usage: { - promptTokens: usage.promptTokens, - completionTokens: usage.completionTokens, - totalTokens: usage.total, - }, + usage: { + promptTokens: usage.promptTokens, + completionTokens: usage.completionTokens, + total: usage.total, + },Also applies to: 267-271, 287-291
src/lib/core/analytics.ts (1)
72-113: Support legacytotalTokensto avoid undercounting.Some providers may still surface only
totalTokens. Add a fallback before returning zero usage.// Try OpenAI/Mistral format (promptTokens/completionTokens) @@ const total = typeof usage.total === "number" ? usage.total : input + output; return { input, output, total }; } + // Legacy totalTokens-only case + if (typeof (usage as Record<string, unknown>).totalTokens === "number") { + const total = (usage as Record<string, number>).totalTokens; + return { input: 0, output: 0, total }; + } + // Handle total-only case if (typeof usage.total === "number") { return { input: 0, output: 0, total: usage.total }; }src/cli/commands/models.ts (2)
469-471: “FREE” should require both input and output to be zero.
Right now, a zero input price but non-zero output shows as FREE.- const cost = - model.pricing.inputCostPer1K === 0 - ? chalk.green("FREE") - : `$${(model.pricing.inputCostPer1K + model.pricing.outputCostPer1K).toFixed(6)}/1K`; + const isFree = + model.pricing.inputCostPer1K === 0 && + model.pricing.outputCostPer1K === 0; + const cost = isFree + ? chalk.green("FREE") + : `$${(model.pricing.inputCostPer1K + model.pricing.outputCostPer1K).toFixed(6)}/1K`;
809-860: Don’t caststatsObj.pricingtoModelPricing; it’s a summary, not per-model pricing
getModelStatistics()returns{ average: number; min: number; max: number; free: number }not a
ModelPricing(which hasinputCostPer1K/outputCostPer1K). Define a matchingPricingSummaryand update the cast and howfreeis printed:- const pricing = statsObj.pricing as ModelPricing; + type PricingSummary = { average: number; min: number; max: number; free: number }; + const pricing = statsObj.pricing as PricingSummary; logger.always(chalk.bold("Pricing Overview:")); logger.always( @@ - logger.always(` Free models: ${pricing.free || false}`); + logger.always(` Free models: ${pricing.free}`);src/lib/providers/googleVertex.ts (3)
139-152: Temp credentials file is never cleaned up.
This risks leaking credentials on disk. Register an exit hook and best-effort unlink on errors.fs.writeFileSync( credentialsFilePath, JSON.stringify(serviceAccountCredentials, null, 2), ); // Set the environment variable to point to our runtime-created file process.env.GOOGLE_APPLICATION_CREDENTIALS = credentialsFilePath; + // Ensure cleanup on exit + const cleanup = () => { + try { fs.unlinkSync(credentialsFilePath); } catch {} + }; + process.once("exit", cleanup); + process.once("SIGINT", () => { cleanup(); process.exit(1); });
1450-1463: Timeout controller cleaned up too early; streaming loses timeout protection.
You callcleanup()immediately afterstreamText(), so long-lived streams won’t be aborted on timeout.- const result = streamText(streamOptions); - - timeoutController?.cleanup(); + const result = streamText(streamOptions); // Transform string stream to content object stream using BaseProvider method const transformedStream = this.createTextStream(result);And ensure cleanup happens on finish/error:
onError: (event: { error: unknown }) => { const error = event.error; const errorMessage = error instanceof Error ? error.message : String(error); logger.error(`${functionTag}: Stream error`, { provider: this.providerName, modelName: this.modelName, error: errorMessage, chunkCount, }); + timeoutController?.cleanup(); }, onFinish: (event: { finishReason: string; usage: Record<string, unknown>; text?: string; }) => { logger.debug(`${functionTag}: Stream finished`, { finishReason: event.finishReason, totalChunks: chunkCount, }); + timeoutController?.cleanup(); },Also applies to: 1405-1415, 1417-1426
353-356: Override signature should match BaseProvider.
Base expectsPromise<LanguageModelV1>. Align types to avoid covariance issues.- protected async getAISDKModel(): Promise<LanguageModel> { + protected async getAISDKModel(): Promise<LanguageModelV1> { const model = await this.getModel(); - return model as LanguageModel; + return model as LanguageModelV1; }src/lib/core/baseProvider.ts (1)
271-286: Apply middleware wrapper when acquiring the model.
getAISDKModelWithMiddleware()exists to attach middleware; use it instead of the raw model.- const model = await this.getAISDKModel(); // This method is now REQUIRED + const model = await this.getAISDKModelWithMiddleware(options);src/lib/core/evaluation.ts (1)
12-31: Type duplication with src/lib/types/providers.ts.
EvaluationResult/EvaluationContextare re-declared here and in providers.ts. Single-source them in providers.ts and re-export to avoid divergence.-// Enhanced evaluation result interface -export interface EvaluationResult extends EvaluationData { … } -// Enhanced evaluation context -export interface EvaluationContext { … } +// Prefer importing shared types +import type { EvaluationResult, EvaluationContext } from "../types/providers.js";Also applies to: 34-53
src/cli/factories/commandFactory.ts (3)
85-90: Add yaml to --format choices to match types and handlers.Users can pass --format yaml (types allow it), but yargs rejects it. Add yaml to choices.
- format: { - choices: ["text", "json", "table"], + format: { + choices: ["text", "json", "table", "yaml"], default: "text",Also update completion to suggest yaml (see below).
278-315: Implement YAML serialization when format=yaml.Add yaml dependency and handle serialization.
-import fs from "fs"; +import fs from "fs"; +import { stringify as yamlStringify } from "yaml"; ... - if (options.format === "json") { + if (options.format === "json") { output = JSON.stringify(result, null, 2); + } else if (options.format === "yaml") { + try { + output = yamlStringify(result as unknown); + } catch (e) { + output = JSON.stringify(result, null, 2); + }Ensure package.json has "yaml" or rely on existing deps.
888-907: Dry-run analytics uses tokens instead of tokenUsage.This prevents token stats from printing.
- tokens: { input: 10, output: 15, total: 25 }, + tokenUsage: { input: 10, output: 15, total: 25 },src/lib/neurolink.ts (1)
3292-3309: require() in ESM will always throw; use createRequire.Today this forces the fallback path every time. Use node:module createRequire.
+import { createRequire } from "node:module"; +const __require = createRequire(import.meta.url); ... - const toolRegistrationModule = require("./sdk/toolRegistration.js"); + const toolRegistrationModule = __require("./sdk/toolRegistration.js");No behavior change except enabling validation in ESM builds.
♻️ Duplicate comments (1)
src/lib/neurolink.ts (1)
3051-3077: Apply the same evaluation defaulting in fallback stream.See previous diff; mirror it here to satisfy EvaluationData.
2012b26 to
b92dcea
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
b3de32d to
5e71bec
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
- Achieve 100% ESLint compliance (0 errors, 0 warnings) - Fix TypeScript compilation issues introduced by ESLint cleanup - Update SageMakerError constructor calls to use options object pattern - Fix property initialization and variable declaration issues - Add missing imports and resolve type casting problems - Remove obsolete memory-bank reports - Ensure build process works end-to-end 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
5e71bec to
bce6cc3
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
1 participant
Pull Request
Description
Type of Change
Related Issues
Changes Made
AI Provider Impact
Component Impact
Testing
Test Environment
Performance Impact
Breaking Changes
Screenshots/Demo
Checklist
Additional Notes
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
Chores