Skip to content

fix(rerank): apply per-connection proxy and strip top_n for Voyage AI - #2

Closed
kamenkadmitry wants to merge 1 commit into
mainfrom
fix/rerank-proxy-and-voyage-topn
Closed

kamenkadmitry wants to merge 1 commit into
mainfrom
fix/rerank-proxy-and-voyage-topn

Conversation

@kamenkadmitry

Copy link
Copy Markdown
Collaborator

Summary

  • Rerank handler ignored per-connection proxy pinning. handleRerank used plain fetch() without runWithProxyContext, so rerank requests always egressed directly — bypassing the SOCKS5 proxy configured on the connection. This caused failures for providers that block certain IP ranges (e.g. Voyage AI returns 403 from Russian IPs, making all rerank model tests fail while embedding tests on the same provider passed).

  • Voyage AI rejects top_n with HTTP 400. The default Cohere-compatible request adapter in transformRequestForProvider passed top_n through unconditionally. Voyage AI's rerank API does not accept this parameter and returns: "Argument 'top_n' is not supported by our API". A new "voyage" format entry strips top_n while preserving all other Cohere-compatible fields.

  • modelTestRunner did not pass connectionId to the embeddings handler during test-all, so per-connection proxy resolution was not triggered for embedding model tests either (the embeddings service resolves proxy from credentials.connectionId internally, but the options.connectionId parameter enables the same path when credentials don't carry it).

Changes

File Change
open-sse/config/rerankRegistry.ts Add format: "voyage" to voyage-ai provider entry
open-sse/handlers/rerank.ts Import resolveProxyForConnection + runWithProxyContext; add connectionId param to handleRerank; wrap fetch in runWithProxyContext when connectionId is set; add "voyage" format to transformRequestForProvider that strips top_n
src/app/api/v1/rerank/route.ts Pass credentials.connectionId to handleRerank
src/lib/api/modelTestRunner.ts Pass connectionId to handleValidatedEmbeddingRequestBody
tests/unit/rerank-providers-5332.test.ts 4 new tests: voyage-ai strips top_n, preserves body without top_n, registered with format=voyage, alias resolves correctly

Root cause analysis

The rerank handler was the only handler that didn't use the proxy infrastructure — chatCore uses runWithProxyContext via chatHelpers.ts, embeddings uses it via src/lib/embeddings/service.ts, images and count_tokens use it directly in their routes. Rerank was missed when proxy support was added across handlers.

Test plan

  • node --import tsx/esm --test tests/unit/rerank-providers-5332.test.ts — 11/11 pass (4 new)
  • node --import tsx/esm --test tests/unit/rerank.test.ts — 12/12 pass
  • node --import tsx/esm --test tests/unit/rerank-openrouter-6574.test.ts — pass
  • node --import tsx/esm --test tests/unit/embedding-rerank-provider-registry.test.ts — pass
  • npm run typecheck:core — pass
  • npx eslint on all changed files — pass
  • Pre-commit hooks (prettier, eslint, docs-sync, any-budget check, tracked-artifacts) — pass
  • Manual: verify Voyage AI rerank models pass test-all with per-connection SOCKS5 proxy

Generated with Devin

The rerank handler used plain fetch() without runWithProxyContext, so
per-connection proxy pinning was silently ignored — rerank requests
egressed directly instead of through the configured SOCKS5 proxy.
This caused failures for providers that block certain IP ranges
(e.g. Voyage AI returns 403 from Russian IPs).

Additionally, Voyage AI's rerank API rejects the top_n parameter with
HTTP 400 ("Argument 'top_n' is not supported by our API"). The default
Cohere-compatible request adapter passed top_n through unconditionally.
A new "voyage" format strips top_n while preserving all other fields.

Also fixes modelTestRunner to pass connectionId to the embeddings
handler during test-all, matching the pattern already used by the
embeddings route for proxy resolution.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@kamenkadmitry

Copy link
Copy Markdown
Collaborator Author

Closing in favor of upstream PR: cheaper-inference#7420

kamenkadmitry pushed a commit that referenced this pull request Jul 19, 2026
…aper-inference#6318)

* feat(cli): add CLI tools for pi, omp, letta, codewhale and jcode

* fix(build): resolve CI build and lint errors

* fix(cli): resolve merge conflicts, add tests, align error handling for cli-additions

Resolve duplicate codewhale key from base merge, add unit/integration
tests for omp/letta settings routes and the omp DB module, and align
omp-settings/letta-settings error handling with sanitizeErrorMessage()
+ the pattern used by sibling jcode/pi/codewhale routes in this PR.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* chore(quality): correct cliRuntime.ts file-size baseline to actual post-merge line count

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* fix(changelog): re-restore cheaper-inference#6318 bullet after release sync

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* fix(merge): restore cheaper-inference#6126 clinepass files reverted by release auto-resolve + baseline re-merge

The release sync's auto-resolve reverted sibling PR cheaper-inference#6126's clinepass work
(registry, catalog, oauth constants, clineAuth.ts, token-refresh case, tests)
and the file-size baseline — all outside this PR's scope. Restored to the
release versions, re-applied only this PR's own baseline entries, restored the
cheaper-inference#6126 CHANGELOG bullet (re-inserting only this PR's own).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* fix(db): re-export db/omp from localDb (check:db-rules #2)

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* fix(db): keep localDb.ts at the 800-line cap after the omp re-export

Folded the MemoryVecMeta type re-export into the memoryVec named-export block
(inline 'type' specifier) so adding the db/omp line stays within the new-file
cap.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* fix(cli): reduce cheaper-inference#6318 scope to omp + letta (pi/codewhale/jcode already shipped)

pi, codewhale, and jcode landed via a separate PR before this one was
reconciled — re-adding parallel versions of their catalog entries, routes,
dashboard card, and i18n strings would have been a straight regression
(duplicate "pi" key silently shadowing the release's own entry, orphaned
JcodeToolCard/BaseUrlSelect/ApiKeySelect/cliEndpointMatch UI files with no
release-side wiring, and unrelated formatting/refactor drift in
codewhale-settings/pi-settings/config-generator/routeGuard picked up along
the way).

This PR now ships only the two tools that are genuinely new: omp (Oh My Pi)
and letta. Both settings routes shell out to `which omp`/`which letta` to
detect the local install, so they're loopback-gated in
LOCAL_ONLY_API_PREFIXES (Hard Rules cheaper-inference#15/cheaper-inference#17) in addition to the shared
requireCliToolsAuth() guard every cli-tools route requires
(tests/unit/cli-tools-auth-hardening.test.ts) — neither route had the guard
wired in yet. cli-catalog-counts.test.ts is updated to the real cardinality
(8 agent entries / 32 total, since omp+letta are both category "agent";
pi/codewhale/jcode were always category "code" and are unaffected). The
integration tests for omp/letta now pass a Request object to GET/DELETE and
assert the 401-when-auth-required path, matching the pattern already used by
the codewhale/jcode sibling routes. complexity-baseline.json is back to the
release's 2053 (the cheaper-inference#6318 rebaseline note is gone — dropping the duplicate
JcodeToolCard.tsx/BaseUrlSelect.tsx removed the violations it was covering);
file-size-baseline.json's cliTools.ts entry shrank 955->915 to match the
smaller real file. CHANGELOG bullet rewritten to describe only omp+letta,
with a note on why pi/codewhale/jcode aren't part of this PR; also restores
the Kiro External IdP bullet that a prior merge auto-resolve had dropped
from the living section.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* test(cli-tools): align cli-tools-schema registry count with omp+letta (30→32)

Second exact-count guard missed in the scope-reduction pass; same legitimate
alignment as cli-catalog-counts.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* fix(cli-tools): omp entry needs docsUrl (CliCatalogEntrySchema requires it)

https://github.com/can1357/oh-my-pi — verified official repo.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* chore(quality): cliTools.ts frozen 915→916 (+1 omp docsUrl line, own growth)

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* chore(changelog): restore base + re-insert cheaper-inference#6318 bullet after release sync

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

* chore(sync): merge release tip + restore own CHANGELOG bullet

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>

---------

Co-authored-by: hamsa0x7 <hamsa0x7@users.noreply.github.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@gmail.com>
Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
kamenkadmitry pushed a commit that referenced this pull request Jul 19, 2026
…805) + cheaper-inference#6807 (translator test 1195)

Owner-approved /merge-prs tail freeze. localDb.ts is re-export-only (Hard Rule #2);
translator test grew from cheaper-inference#6807's regression suite. Both frozen (shrink-only).
kamenkadmitry pushed a commit that referenced this pull request Jul 19, 2026
…heaper-inference#7610) (cheaper-inference#7715)

GrokCliExecutor.execute() dispatches via raw https.request (nativePost)
instead of the shared fetch path, so it never inherited (nor delegated to)
BaseExecutor.execute()'s proactive-refresh gate the way codex.ts does via
super.execute(). The only refresh that ever fired was the reactive one on a
401/403 from upstream — the rotating xAI refresh_token idled until real
expiry, matching the "unusable within minutes, must delete/re-add" report.

Wires in the same needsRefresh()/refreshCredentials() gate, using
runWithOnPersist + isUnrecoverableRefreshError to keep the [refresh +
persist] atomic under the same per-connection mutex Codex/Claude rely on
for rotating refresh tokens (base.ts:592-644).

Also fixes the smaller, separate bug #2 from the same report: grok-cli was
absent from OAUTH_TEST_CONFIG in the connection-test route, so "Test
Connection" always reported "Provider test not supported" regardless of
token health. Added a checkExpiry entry (same pattern as qwen/cline/
kilocode — Grok Build's proxy doesn't expose a lightweight probe endpoint
with the cli-specific headers this shared prober sends). Extracted
OAUTH_TEST_CONFIG into its own module (oauthTestConfig.ts) so the new entry
doesn't grow the frozen route.ts past its file-size cap.

Bug cheaper-inference#3 (no browser/device-code login for Grok Build) and bug cheaper-inference#4 (quota
display) from the same issue are feature gaps, not regressions — left as
follow-ups per the triage plan-file.

Refs cheaper-inference#7610
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant