Skip to content

refactor(model-resolver): decompose resolveModel into focused sub-functions; move version utils tests#4938

Merged
lpcox merged 2 commits into
mainfrom
copilot/refactor-resolve-model-function
Jun 14, 2026
Merged

refactor(model-resolver): decompose resolveModel into focused sub-functions; move version utils tests#4938
lpcox merged 2 commits into
mainfrom
copilot/refactor-resolve-model-function

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

resolveModel in model-resolver.js was a 156-line monolith handling 7 distinct logical paths inline, making individual paths difficult to test or modify in isolation. The version utilities (globMatch, extractVersionNumbers, compareByVersion) were already extracted to model-utils.js; this PR completes the refactoring.

Changes

model-resolver.jsresolveModel decomposed into three functions

  • _resolveDirectMatch(key, requestedModel, currentProvider, availableModels, fallbackConfig, log) — no-alias path: direct match → GPT-5 family version fallback → middle-power fallback
  • _resolveAliasPatterns(aliasKey, aliasDefinition, requestedModel, aliases, availableModels, currentProvider, newChain, fallbackConfig, log) — pattern expansion: iterates patterns, recurses into alias references, glob-matches against provider models, deduplicates, version-sorts, picks best candidate
  • resolveModel (~36 lines) — orchestrator only: loop detection → alias lookup (with gpt-5 family alias fallback) → dispatch to one of the above

The log array is passed by reference through all sub-functions to preserve the existing log-accumulation contract. The module.exports surface is unchanged (re-exports of version utils kept for caller compatibility).

// Before: resolveModel was a 156-line function with 7 inline paths
// After:
function resolveModel(requestedModel, aliases, availableModels, currentProvider, chain = [], modelFallbackConfig = DEFAULT_MODEL_FALLBACK) {
  // loop detection, alias lookup, gpt-5 family alias fallback (~20 lines)
  if (!aliasEntry) {
    return _resolveDirectMatch(key, requestedModel, currentProvider, availableModels, fallbackConfig, log);
  }
  return _resolveAliasPatterns(aliasKey, aliasDefinition, requestedModel, aliases, availableModels, currentProvider, newChain, fallbackConfig, log);
}

model-utils.test.js (new)

Dedicated test file for globMatch, extractVersionNumbers, compareByVersion imported directly from ./model-utils.

model-resolver.test.js

Removed the duplicate version-utility test sections (now in model-utils.test.js); updated imports accordingly. All 1044 tests across 48 suites continue to pass.

Copilot AI changed the title [WIP] Refactor resolveModel function and extract version utilities refactor(model-resolver): decompose resolveModel into focused sub-functions; move version utils tests Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:02
Copilot AI requested a review from lpcox June 14, 2026 17:02
@lpcox lpcox marked this pull request as ready for review June 14, 2026 17:07
Copilot AI review requested due to automatic review settings June 14, 2026 17:07
@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.60% 96.64% 📈 +0.04%
Statements 96.47% 96.51% 📈 +0.04%
Functions 98.80% 98.80% ➡️ +0.00%
Branches 91.18% 91.21% 📈 +0.03%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/workdir-setup.ts 92.6% → 94.4% (+1.85%) 92.6% → 94.4% (+1.85%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the API proxy’s model alias resolution logic by decomposing the previously monolithic resolveModel implementation into focused helper functions, and relocates the pure “version/glob” utility tests into a dedicated test file for clearer separation of concerns.

Changes:

  • Split resolveModel into _resolveDirectMatch (no-alias path) and _resolveAliasPatterns (alias/pattern expansion path), leaving resolveModel as an orchestrator with loop detection + alias lookup.
  • Added model-utils.test.js to directly test globMatch, extractVersionNumbers, and compareByVersion.
  • Removed duplicated version-utility tests from model-resolver.test.js and updated imports/comments accordingly.
Show a summary per file
File Description
containers/api-proxy/model-resolver.js Decomposes resolveModel into smaller helpers while preserving recursive alias resolution, logging, and fallback behavior.
containers/api-proxy/model-utils.test.js Introduces dedicated unit coverage for pure glob/version utility helpers.
containers/api-proxy/model-resolver.test.js Removes duplicated utility tests and keeps resolver tests focused on alias/fallback behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot BYOK (Direct) Mode

Status: PASS — Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

🔥 Smoke Test Results — PASS

Test Result
GitHub MCP (list_pull_requests) ✅ PASS
GitHub.com connectivity (HTTP) ✅ 200 OK
File write/read ⚠️ SKIP (pre-step vars not resolved)

PR: refactor(model-resolver): decompose resolveModel into focused sub-functions; move version utils tests
Author: @Copilot | Assignees: @lpcox @Copilot

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

🔐 Smoke Test: Copilot PAT Auth — PASS

Test Result
GitHub MCP connectivity
GitHub.com HTTP (200)
File write/read

Auth mode: PAT (COPILOT_GITHUB_TOKEN)
PR author: @Copilot | Assignees: @lpcox, @Copilot

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

✅ Add interactive workflow designer agent
✅ fix: normalize model name separators in AWF CLI validation
✅ GitHub title check
✅ File write/read check
✅ Discussion lookup
✅ Build check
Overall status: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions github-actions Bot mentioned this pull request Jun 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔥 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Notes
1. Module Loading otel.js loads; isEnabled()true; 14 exports (public API + test helpers)
2. Test Suite 59 tests passed, 0 failed (otel.test.js: 39, otel-fanout.test.js: 20)
3. Env Var Forwarding api-proxy-service-config.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
4. Token Tracker Integration onUsage callback present in token-tracker-http.js (line 256); calls setTokenAttributes via hook
5. OTEL Diagnostics No OTLP endpoint configured → graceful fallback to FileSpanExporter (/var/log/api-proxy/otel.jsonl); no unexpected errors

All 5 scenarios passed. OTEL tracing integration is working correctly on this branch.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions

Copy link
Copy Markdown
Contributor

🔬 Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.16.0 v22.22.3 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Overall: ❌ FAILED — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox

Smoke Test Results:

Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra

Overall: PASS

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ Timeout (no response)
PostgreSQL pg_isready ❌ No response on port 5432
PostgreSQL SELECT 1 ❌ Not attempted (pg_isready failed)

Overall: FAILhost.docker.internal is not reachable from this runner environment. Services containers are likely not running or the hostname is not resolvable in this context.

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color passed ✅ PASS
Go env passed ✅ PASS
Go uuid passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #4938 ·

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results: Gemini

Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox Smoke Test Results:

  • GitHub MCP Testing: ✅
  • GitHub.com Connectivity: ✅
  • File Write/Read Test: ✅
  • BYOK Inference Test: ✅

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)

Overall: PASS

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants