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

[cv]

Summary

• Convert bin/lib/inference-config.js (143 lines) to src/lib/inference-config.ts
• Typed interfaces: ProviderSelectionConfig, GatewayInference
• All 3 exports are pure — straightforward full conversion
• Co-locate tests: test/inference-config.test.js → src/lib/inference-config.test.ts

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

Relates to #924 (shell consolidation).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Inference configuration and provider/model selection behavior consolidated into a single, consistent implementation.

• Tests

  • Test suite updated to align with the consolidated inference behavior and to use clearer, more focused assertions.

• Chores

  • Project setup now runs a CLI build step during installation when available, streamlining installation/prep.

[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: 1e539270-042d-43f5-8c2e-8b4e64d41a8c

📥 Commits

Reviewing files that changed from the base of the PR and between 3e241bc and 6dc2ed1.

📒 Files selected for processing (2)

• package.json
• src/lib/inference-config.test.ts

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

• package.json
• src/lib/inference-config.test.ts

---

📝 Walkthrough

Walkthrough

Replaced the binary-level inline inference config with a re-export to dist/lib/inference-config, added a new TypeScript implementation src/lib/inference-config.ts that centralizes provider/model selection and parsing, updated tests to import from the compiled module, and adjusted package.json prepare script to run build:cli.

Changes

Cohort / File(s)	Summary
New TypeScript source src/lib/inference-config.ts	Adds centralized inference configuration: constants (route URL, defaults, options, managed provider id), interfaces (ProviderSelectionConfig, GatewayInference), and helpers (getProviderSelectionConfig, getOpenClawPrimaryModel, parseGatewayInference).
Binary re-export bin/lib/inference-config.js	Removed inline constants and logic; now re-exports the implementation from ../../dist/lib/inference-config.
Tests updated src/lib/inference-config.test.ts	Switched imports to the compiled dist module, replaced node:assert/strict checks with Vitest expect assertions, simplified several assertions and test cases, and adjusted minor typings.
Prepare script change package.json	prepare script now conditionally runs npm run build:cli (attempts to build CLI when TypeScript is available) before the existing git-hook setup logic.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐇 I nudged the bits from bin to dist with care,

Tuned tests and routes and gave configs new air.
A hop, a patch, a tidy little feat—
Now models line up neatly, ready and sweet.

🚥 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 clearly describes the main change: migrating inference-config.js from JavaScript to TypeScript, which aligns with the substantial refactoring shown in the changeset.

_{✏️ 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-inference-config-ts

---

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

[coderabbitai]

🧹 Nitpick comments (2)

src/lib/inference-config.ts (1)

118-122: Unreachable null return path.

resolvedModel will always be a truthy string because the ternary on line 120 always returns either model (when provided and truthy), DEFAULT_OLLAMA_MODEL, or DEFAULT_CLOUD_MODEL — all of which are non-empty strings. The conditional resolvedModel ? ... : null on line 121 will never evaluate to null.

This isn't a bug (the function still works correctly), but the dead code path and the | null return type are misleading. Consider simplifying if the intent is to always return a qualified model string.

♻️ Suggested simplification

-export function getOpenClawPrimaryModel(provider: string, model?: string): string | null {
-  const resolvedModel =
-    model || (provider === "ollama-local" ? DEFAULT_OLLAMA_MODEL : DEFAULT_CLOUD_MODEL);
-  return resolvedModel ? `${MANAGED_PROVIDER_ID}/${resolvedModel}` : null;
+export function getOpenClawPrimaryModel(provider: string, model?: string): string {
+  const resolvedModel =
+    model || (provider === "ollama-local" ? DEFAULT_OLLAMA_MODEL : DEFAULT_CLOUD_MODEL);
+  return `${MANAGED_PROVIDER_ID}/${resolvedModel}`;
 }

🤖 Prompt for AI Agents

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

In `@src/lib/inference-config.ts` around lines 118 - 122, The function
getOpenClawPrimaryModel has an unreachable null path because resolvedModel is
always a non-empty string; update the function to always return a qualified
model string by changing the return type from string | null to string and return
`${MANAGED_PROVIDER_ID}/${resolvedModel}` directly (remove the ternary that
returns null), keeping the resolution logic using model, DEFAULT_OLLAMA_MODEL
and DEFAULT_CLOUD_MODEL unchanged.

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

6-17: Tests import from compiled dist/ — build is configured but worth clarifying locally.

Importing from ../../dist/lib/inference-config requires the dist/ directory to exist. CI handles this correctly via prek pre-push hooks which run npm run build:cli before npx vitest run. The workflow is documented in CONTRIBUTING.md: use npx prek run --all-files for a complete local check, or run npm run build:cli && npm test explicitly if testing in isolation.

The comment "for correct coverage attribution" explains the pattern but could be clearer for contributors unfamiliar with the build structure. Consider expanding the comment to note that npm run build:cli must precede test execution.

🤖 Prompt for AI Agents

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

In `@src/lib/inference-config.test.ts` around lines 6 - 17, The test currently
imports from "../../dist/lib/inference-config" which requires the compiled dist/
to exist; update the top comment in src/lib/inference-config.test.ts (the block
above the import list referencing CLOUD_MODEL_OPTIONS, DEFAULT_OLLAMA_MODEL,
DEFAULT_ROUTE_CREDENTIAL_ENV, DEFAULT_ROUTE_PROFILE, INFERENCE_ROUTE_URL,
MANAGED_PROVIDER_ID, getOpenClawPrimaryModel, getProviderSelectionConfig,
parseGatewayInference) to explicitly state that contributors must run the build
before running tests locally—e.g., "run npm run build:cli before npm test" or
use "npx prek run --all-files"—so it's clear how to produce dist/ for correct
coverage attribution.

🤖 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/inference-config.test.ts`:
- Around line 6-17: The test currently imports from
"../../dist/lib/inference-config" which requires the compiled dist/ to exist;
update the top comment in src/lib/inference-config.test.ts (the block above the
import list referencing CLOUD_MODEL_OPTIONS, DEFAULT_OLLAMA_MODEL,
DEFAULT_ROUTE_CREDENTIAL_ENV, DEFAULT_ROUTE_PROFILE, INFERENCE_ROUTE_URL,
MANAGED_PROVIDER_ID, getOpenClawPrimaryModel, getProviderSelectionConfig,
parseGatewayInference) to explicitly state that contributors must run the build
before running tests locally—e.g., "run npm run build:cli before npm test" or
use "npx prek run --all-files"—so it's clear how to produce dist/ for correct
coverage attribution.

In `@src/lib/inference-config.ts`:
- Around line 118-122: The function getOpenClawPrimaryModel has an unreachable
null path because resolvedModel is always a non-empty string; update the
function to always return a qualified model string by changing the return type
from string | null to string and return
`${MANAGED_PROVIDER_ID}/${resolvedModel}` directly (remove the ternary that
returns null), keeping the resolution logic using model, DEFAULT_OLLAMA_MODEL
and DEFAULT_CLOUD_MODEL unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 63f96e66-33f7-4795-87cd-7efb117d8004

📥 Commits

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

📒 Files selected for processing (3)

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/lib/inference-config.ts (2)

9-10: Add type annotation to avoid any leakage.

The require call returns an untyped object, so DEFAULT_OLLAMA_MODEL is implicitly any. This allows type errors to slip through where it's used (lines 109, 120).

♻️ Proposed fix to add explicit typing

 // eslint-disable-next-line `@typescript-eslint/no-require-imports`
-const { DEFAULT_OLLAMA_MODEL } = require("../../bin/lib/local-inference");
+const { DEFAULT_OLLAMA_MODEL } = require("../../bin/lib/local-inference") as {
+  DEFAULT_OLLAMA_MODEL: string;
+};

🤖 Prompt for AI Agents

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

In `@src/lib/inference-config.ts` around lines 9 - 10, The require call for
DEFAULT_OLLAMA_MODEL is untyped and yields an implicit any; replace it with a
typed import or add an explicit type annotation so DEFAULT_OLLAMA_MODEL is
strongly typed (e.g. declare its type as string or the appropriate
union/interface) to prevent any leakage where it's used (references:
DEFAULT_OLLAMA_MODEL in this file and usages around lines where it's read).
Concretely: change the require to a typed ES import or annotate the destructured
constant like "const { DEFAULT_OLLAMA_MODEL }: { DEFAULT_OLLAMA_MODEL: string }
= require(...)" (or the correct type) and update any downstream typings if
needed so callers at the earlier mentioned uses (DEFAULT_OLLAMA_MODEL at its
usage sites) no longer see any.

---

47-53: Nit: as const is unnecessary here.

The as const assertion on "custom" doesn't affect the final type since the value is spread into a larger object where endpointType is inferred as string anyway. It's harmless but could be removed for clarity.

🤖 Prompt for AI Agents

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

In `@src/lib/inference-config.ts` around lines 47 - 53, Remove the unnecessary
TypeScript const assertion on the string literal in the base object: change the
`endpointType: "custom" as const` entry inside the `base` object to simply
`endpointType: "custom"`, leaving the other properties (`endpointUrl:
INFERENCE_ROUTE_URL`, `ncpPartner`, `profile: DEFAULT_ROUTE_PROFILE`,
`provider`) unchanged so the `base` variable continues to behave the same
without the redundant `as const`.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@package.json`:
- Line 18: The prepare script currently silences failures for "npm run
build:cli"; update the package.json "prepare" script so the npm run build:cli
invocation does not include "2>/dev/null || true" (i.e., remove the stderr
redirection and the "|| true" that masks errors) so the build will fail fast if
build:cli fails; keep the rest of the prepare logic (the npm install step and
prek handling) intact and ensure the change targets the "prepare" script string
containing "npm run build:cli".

---

Nitpick comments:
In `@src/lib/inference-config.ts`:
- Around line 9-10: The require call for DEFAULT_OLLAMA_MODEL is untyped and
yields an implicit any; replace it with a typed import or add an explicit type
annotation so DEFAULT_OLLAMA_MODEL is strongly typed (e.g. declare its type as
string or the appropriate union/interface) to prevent any leakage where it's
used (references: DEFAULT_OLLAMA_MODEL in this file and usages around lines
where it's read). Concretely: change the require to a typed ES import or
annotate the destructured constant like "const { DEFAULT_OLLAMA_MODEL }: {
DEFAULT_OLLAMA_MODEL: string } = require(...)" (or the correct type) and update
any downstream typings if needed so callers at the earlier mentioned uses
(DEFAULT_OLLAMA_MODEL at its usage sites) no longer see any.
- Around line 47-53: Remove the unnecessary TypeScript const assertion on the
string literal in the base object: change the `endpointType: "custom" as const`
entry inside the `base` object to simply `endpointType: "custom"`, leaving the
other properties (`endpointUrl: INFERENCE_ROUTE_URL`, `ncpPartner`, `profile:
DEFAULT_ROUTE_PROFILE`, `provider`) unchanged so the `base` variable continues
to behave the same without the redundant `as const`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6aedc712-bcd4-4875-a1fe-0f18230eb1a7

📥 Commits

Reviewing files that changed from the base of the PR and between 733b530 and 3e241bc.

📒 Files selected for processing (2)

• package.json
• src/lib/inference-config.ts

[ericksoa]

Two things to address before merge:

1. prepare script silences build:cli failures (package.json)

CodeRabbit flagged this and I agree — 2>/dev/null || true on build:cli means if the build fails (e.g., devDeps stripped by --omit=dev), the shim at bin/lib/inference-config.js will hit MODULE_NOT_FOUND at runtime with no earlier signal. Can you either remove the error suppression or add a guard in the shim (e.g., try/catch with a helpful error message)?

2. Test coverage was weakened, not just migrated (inference-config.test.ts)

The switch from full-object assertions to expect.objectContaining with only model + providerLabel dropped coverage on the structural fields (endpointType, endpointUrl, ncpPartner, profile, credentialEnv). Those are the ones most likely to break if someone edits the base object. Could you keep at least one full-object assertion per provider category (one hosted, one local) alongside the shorter ones?

[cv]

Thanks @ericksoa — both addressed in 6dc2ed1:

1. prepare hook: Replaced 2>/dev/null || true with a guard that only runs build:cli when tsc is available, but lets it fail loudly if the build breaks:

if command -v tsc >/dev/null 2>&1 || [ -x node_modules/.bin/tsc ]; then npm run build:cli; fi

This tolerates fresh clones before devDeps are installed (no tsc → skip), but surfaces real build failures (tsc present + compile error → hard fail).

2. Test assertions: Restored full-object toEqual for one hosted (openai-api) and one local (vllm-local) provider so structural fields (endpointType, endpointUrl, ncpPartner, profile, credentialEnv) are covered. The remaining providers still use objectContaining to keep the test concise.

[ericksoa]

Both items addressed. LGTM.