refactor(cli): migrate local-inference.js to TypeScript

[cv]

Summary

• Convert bin/lib/local-inference.js (228 lines) to src/lib/local-inference.ts
• Typed interfaces: ValidationResult, GpuInfo, RunCaptureFn
• 9 pure functions + 4 with DI-injected runCapture
• Co-locate tests (30 tests)

Stacked on #1240. 616 CLI tests pass. Coverage ratchet passes.

Relates to #924 (shell consolidation).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added local inference provider validation and connectivity diagnostics for vllm-local and ollama-local.
  • Implemented Ollama model discovery, warmup/probe commands, and intelligent model selection based on GPU memory.
  • Added health-check and model readiness checks to surface configuration or networking issues earlier.

• Tests

  • Enhanced test coverage for provider validation, model detection, and unknown-provider handling; updated test targets accordingly.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5120efa8-c456-4948-85c5-2c8d3a0bb8e6

📥 Commits

Reviewing files that changed from the base of the PR and between af3eff4 and 6c34a6d.

📒 Files selected for processing (2)

• src/lib/local-inference.test.ts
• src/lib/local-inference.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/local-inference.test.ts

---

📝 Walkthrough

Walkthrough

Replaces the in-repo JavaScript local-inference implementation with a thin shim exporting from the compiled module, adds a new TypeScript src/lib/local-inference.ts implementing provider validation and Ollama helpers, and updates tests to import the compiled exports.

Changes

Cohort / File(s)	Summary
TypeScript Source Module src/lib/local-inference.ts	New TypeScript implementation: provider URL/health-check builders, container reachability checks, validation orchestration (validateLocalProvider, validateOllamaModel), Ollama parsing/discovery/selection helpers, warmup/probe command builders, and exported types/constants.
Build Output Shim bin/lib/local-inference.js	Replaced detailed in-file implementation with module.exports = require("../../dist/lib/local-inference"), delegating all exports to the compiled dist module.
Tests src/lib/local-inference.test.ts	Updated imports to reference ../../dist/lib/local-inference; adjusted formatting of expectations; added test case for validateLocalProvider with an unknown provider (using a stubbed health-check callback).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Validator as validateLocalProvider
participant Shell as runCapture (shell)
participant Provider as Local Provider (ollama / vllm)
participant Container as Container -> Host Gateway
Validator->>Shell: run provider health-check command
Shell->>Provider: execute HTTP/CLI health probe
Shell-->>Validator: health output / error
Validator->>Shell: run container reachability command
Shell->>Container: curl host gateway from container image
Shell-->>Validator: reachability output
Validator-->>Validator: return ValidationResult (ok/message)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 New shim hops out, TypeScript hops in,
Health checks and probes, a neat little spin,
Ollama models parsed, defaults held tight,
Tests point to dist and everything's right,
A tiny rabbit cheers at the compiled win! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor(cli): migrate local-inference.js to TypeScript' accurately describes the main change: converting a JavaScript file to TypeScript with new typed implementations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cv/migrate-local-inference-ts

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (4)

src/lib/local-inference.test.ts (1)

117-125: Test name doesn't match behavior.

The test is named "reports generic container unreachability for unknown providers" but actually verifies that unknown providers return { ok: true } immediately without any health check (since getLocalProviderHealthCheck("custom-provider") returns null). The runCapture stub is never called in this path.

Consider renaming to align with the existing test on line 113 ("treats unknown local providers as already valid") or combining them, since both test the same code path.

Suggested rename

-  it("reports generic container unreachability for unknown providers", () => {
+  it("skips runCapture entirely for unknown providers with no health check", () => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/local-inference.test.ts` around lines 117 - 125, Rename or
consolidate the test so its name reflects the actual behavior: the case
exercises validateLocalProvider("custom-provider", ...) when
getLocalProviderHealthCheck("custom-provider") returns null, so the provider is
treated as already valid and runCapture is not invoked; update the test name to
something like "treats unknown local providers as already valid" (or merge it
with the existing test covering the same path) and keep the callCount stub as-is
to show the health-check is not executed.

src/lib/local-inference.ts (3)

196-208: timeoutSeconds interpolated directly into shell command.

The timeoutSeconds parameter (default 120) is interpolated directly into the curl command at line 207. While TypeScript ensures it's a number at compile time, if this function is called from JavaScript without type checking, a malicious string could be injected.

Given this is an internal function and the codebase uses TypeScript type checking, the risk is low. However, consider adding runtime validation if this becomes part of a public API.

Optional defensive validation

 export function getOllamaProbeCommand(
   model: string,
   timeoutSeconds = 120,
   keepAlive = "15m",
 ): string {
+  const timeout = Math.floor(Number(timeoutSeconds)) || 120;
   const payload = JSON.stringify({
     model,
     prompt: "hello",
     stream: false,
     keep_alive: keepAlive,
   });
-  return `curl -sS --max-time ${timeoutSeconds} http://localhost:11434/api/generate -H 'Content-Type: application/json' -d ${shellQuote(payload)} 2>/dev/null`;
+  return `curl -sS --max-time ${timeout} http://localhost:11434/api/generate -H 'Content-Type: application/json' -d ${shellQuote(payload)} 2>/dev/null`;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/local-inference.ts` around lines 196 - 208, The getOllamaProbeCommand
function interpolates timeoutSeconds directly into a shell command; add a
runtime validation/coercion step for the timeoutSeconds parameter (e.g., use
Number(timeoutSeconds), verify Number.isFinite and Number.isInteger and that
it's >0, or clamp to a safe range) and fall back to the default 120 (or throw)
when validation fails before building the curl string; keep using shellQuote for
payload as-is and reference timeoutSeconds and getOllamaProbeCommand so the
check is applied immediately prior to constructing the return string.

---

18-18: Consider exporting RunCaptureFn type.

The RunCaptureFn type is used by multiple exported functions (validateLocalProvider, getOllamaModelOptions, getDefaultOllamaModel, validateOllamaModel) but is not exported. Consumers implementing or mocking this function would benefit from having access to the type definition.

Suggested change

-type RunCaptureFn = (cmd: string, opts?: { ignoreError?: boolean }) => string;
+export type RunCaptureFn = (cmd: string, opts?: { ignoreError?: boolean }) => string;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/local-inference.ts` at line 18, Export the RunCaptureFn type so
consumers can import and implement/mock it: change the local declaration "type
RunCaptureFn = (cmd: string, opts?: { ignoreError?: boolean }) => string;" to an
exported type (export type RunCaptureFn = ...). Ensure references in functions
validateLocalProvider, getOllamaModelOptions, getDefaultOllamaModel, and
validateOllamaModel continue to use the same symbol and update any external
imports/usages to import { RunCaptureFn } where needed.

---

9-10: Consider moving shellQuote to src/lib/ to eliminate reverse dependency.

The require("../../bin/lib/runner") on line 10 creates a reverse dependency: library code in src/lib/ depends on CLI-layer code in bin/lib/, which violates typical architectural layering. While this works in practice (shellQuote is a pure utility), it prevents using src/lib/local-inference.ts outside the CLI context.

Moving shellQuote to src/lib/ (e.g., src/lib/shell-quote.ts) and updating imports across bin/, scripts/, and src/lib/ would fix this. Alternatively, accept shellQuote as a dependency-injected parameter like runCapture.

This is acceptable for now given migration scope, but address as technical debt.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/local-inference.ts` around lines 9 - 10, The file local-inference.ts
currently requires shellQuote from bin/lib/runner creating a reverse dependency;
move the shellQuote implementation into a new module under src/lib (e.g.,
src/lib/shell-quote.ts) and export it, then update all imports that currently
pull shellQuote from bin/lib/runner (including local-inference.ts and any files
in bin/ and scripts/) to import from the new src/lib/shell-quote module;
alternatively, refactor functions that call shellQuote (e.g., runCapture or the
functions in local-inference.ts) to accept shellQuote as a dependency-injected
parameter and wire callers accordingly so src/lib no longer depends on bin/.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/local-inference.test.ts`:
- Around line 117-125: Rename or consolidate the test so its name reflects the
actual behavior: the case exercises validateLocalProvider("custom-provider",
...) when getLocalProviderHealthCheck("custom-provider") returns null, so the
provider is treated as already valid and runCapture is not invoked; update the
test name to something like "treats unknown local providers as already valid"
(or merge it with the existing test covering the same path) and keep the
callCount stub as-is to show the health-check is not executed.

In `@src/lib/local-inference.ts`:
- Around line 196-208: The getOllamaProbeCommand function interpolates
timeoutSeconds directly into a shell command; add a runtime validation/coercion
step for the timeoutSeconds parameter (e.g., use Number(timeoutSeconds), verify
Number.isFinite and Number.isInteger and that it's >0, or clamp to a safe range)
and fall back to the default 120 (or throw) when validation fails before
building the curl string; keep using shellQuote for payload as-is and reference
timeoutSeconds and getOllamaProbeCommand so the check is applied immediately
prior to constructing the return string.
- Line 18: Export the RunCaptureFn type so consumers can import and
implement/mock it: change the local declaration "type RunCaptureFn = (cmd:
string, opts?: { ignoreError?: boolean }) => string;" to an exported type
(export type RunCaptureFn = ...). Ensure references in functions
validateLocalProvider, getOllamaModelOptions, getDefaultOllamaModel, and
validateOllamaModel continue to use the same symbol and update any external
imports/usages to import { RunCaptureFn } where needed.
- Around line 9-10: The file local-inference.ts currently requires shellQuote
from bin/lib/runner creating a reverse dependency; move the shellQuote
implementation into a new module under src/lib (e.g., src/lib/shell-quote.ts)
and export it, then update all imports that currently pull shellQuote from
bin/lib/runner (including local-inference.ts and any files in bin/ and scripts/)
to import from the new src/lib/shell-quote module; alternatively, refactor
functions that call shellQuote (e.g., runCapture or the functions in
local-inference.ts) to accept shellQuote as a dependency-injected parameter and
wire callers accordingly so src/lib no longer depends on bin/.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5bb8e2f8-6466-435d-a45b-e83a1272a2ee

📥 Commits

Reviewing files that changed from the base of the PR and between 2804b74 and af3eff4.

📒 Files selected for processing (3)

• bin/lib/local-inference.js
• src/lib/local-inference.test.ts
• src/lib/local-inference.ts

[ericksoa]

Clean migration, LGTM.