Skip to content

refactor: extract tryMiddlePowerFallback helper in model-resolver#3755

Merged
lpcox merged 1 commit into
mainfrom
fix/model-resolver-dedup-fallback
May 25, 2026
Merged

refactor: extract tryMiddlePowerFallback helper in model-resolver#3755
lpcox merged 1 commit into
mainfrom
fix/model-resolver-dedup-fallback

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented May 25, 2026

Summary

Extract the duplicated selectMiddlePowerFallback call + log + return pattern into a shared tryMiddlePowerFallback() helper function.

The same 6-line pattern (call, check result, log message, return) appeared in two locations within resolveModel(). Now both call sites use the shared helper.

Verification

  • All 61 model-resolver tests pass

Closes #3613

The same selectMiddlePowerFallback call + log + return pattern was
duplicated in two places within resolveModel(). Extract into a shared
tryMiddlePowerFallback() helper that both call sites now use.

Closes #3613

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 25, 2026 14:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the API proxy’s model resolution flow by extracting the repeated “middle-power fallback” selection + log + return logic into a single helper, reducing duplication inside resolveModel().

Changes:

  • Added tryMiddlePowerFallback() helper to encapsulate the middle-power fallback pattern.
  • Replaced two duplicated fallback blocks in resolveModel() with calls to the new helper.
Show a summary per file
File Description
containers/api-proxy/model-resolver.js Introduces tryMiddlePowerFallback() and updates both resolveModel() call sites to use it, removing duplicated logic.

Copilot's findings

Tip

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

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

@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.36% 96.41% 📈 +0.05%
Statements 96.21% 96.25% 📈 +0.04%
Functions 97.98% 97.98% ➡️ +0.00%
Branches 90.50% 90.54% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.9% → 85.5% (+1.61%) 83.9% → 85.5% (+1.61%)

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

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

GitHub MCP: ✅ Retrieved PR #3704 - "Reduce Documentation Maintainer workflow prompt/tool overhead"
GitHub.com connectivity: ⚠️ No HTTP code provided
File write/read: ❌ File not found at expected path

Author: @lpcox
Assignees: @lpcox, @Copilot

Overall: FAIL (file test failed)

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine Validation ✅

Results:

  • ✅ GitHub API: 2 PR entries verified
  • ✅ GitHub check: playwright_check=PASS
  • ✅ File verify: smoke-test-claude-26405503272.txt exists

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

BYOK Smoke Test Results

MCP GitHub API - PR #3755: refactor: extract tryMiddlePowerFallback helper in model-resolver
File Write/Read - /tmp/smoke-test-file.txt not found
BYOK Inference - Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com

Status: PARTIAL - BYOK path working, file test failed (may be cross-container)
PR author: @lpcox

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test Codex: FAIL
PRs reviewed: Reduce Documentation Maintainer workflow prompt/tool overhead; refactor(api-proxy): extract nested callbacks from proxyRequest into focused module-level functions
✅ GitHub PR review/read
❌ safeinputs-gh unavailable
✅ Playwright GitHub title
❌ Tavily unavailable; ❌ discussion query unavailable
✅ File IO/bash; ✅ npm ci && npm run build
Overall status: FAIL

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
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 All passed ✅ PASS
Go env All passed ✅ PASS
Go uuid All passed ✅ PASS
Java gson 0/0 ❌ FAIL
Java caffeine 0/0 ❌ FAIL
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 7/8 ecosystems passed — FAIL

❌ Java Failures

Both Java projects (gson and caffeine) failed with Maven repository permission errors:

[ERROR] Could not create local repository at /home/runner/.m2/repository

Root Cause: Maven cannot create the local repository directory due to filesystem permission restrictions in the AWF agent container environment. The /home/runner/.m2 directory exists but Maven lacks permissions to create the repository subdirectory.

Impact: All Java/Maven builds will fail in the current AWF configuration until this permission issue is resolved.

✅ Other Ecosystems

All other ecosystems (Bun, C++, Deno, .NET, Go, Node.js, Rust) passed successfully:

  • All repositories cloned successfully
  • All builds/installs completed successfully
  • All tests passed

Generated by Build Test Suite for issue #3755 · ● 3.8M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: 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

Services Connectivity: ❌ FAIL

  • ❌ Redis (host.docker.internal:6379): timeout/no response
  • ❌ PostgreSQL readiness (host.docker.internal:5432): no response
  • ❌ PostgreSQL query: timeout/no response

Overall: FAIL — AWF sandbox cannot reach service containers via host.docker.internal

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Version Comparison Test Results

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

Overall Result:FAILED - Not all versions match

Analysis

  • Python: Minor version mismatch (3.12.13 vs 3.12.3)
  • Node.js: Major version mismatch (v24 vs v22)
  • Go: ✅ Versions match exactly

The chroot environment is not using the exact same runtime versions as the host. This could indicate that the selective bind mounts are working as intended (using containerized versions) or that the host runtimes are not being properly exposed.

Tested by Smoke Chroot

@lpcox lpcox merged commit 5d8169d into main May 25, 2026
66 of 69 checks passed
@lpcox lpcox deleted the fix/model-resolver-dedup-fallback branch May 25, 2026 14:47
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.

[Duplicate Code] Model resolver middle-power fallback logic duplicated twice

2 participants