Skip to content

feat(embeddings): control request-body dimensions via GITNEXUS_EMBEDDING_REQUEST_DIMS#2574

Merged
abhigyanpatwari merged 4 commits into
abhigyanpatwari:mainfrom
ChamHerry:feat/embedding-request-dims-omit
Jul 21, 2026
Merged

feat(embeddings): control request-body dimensions via GITNEXUS_EMBEDDING_REQUEST_DIMS#2574
abhigyanpatwari merged 4 commits into
abhigyanpatwari:mainfrom
ChamHerry:feat/embedding-request-dims-omit

Conversation

@ChamHerry

@ChamHerry ChamHerry commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

wangxc
feat(embeddings): support GITNEXUS_EMBEDDING_REQUEST_DIMS=omit
6f43825 
What: Honor GITNEXUS_EMBEDDING_REQUEST_DIMS=omit by suppressing the request-body
`dimensions` field sent to HTTP embedding backends.

Why: Strict OpenAI-compatible backends return vectors in the model's native size
but reject an unfamiliar `dimensions` field, breaking `analyze --embeddings`
against them. The var was parsed but never propagated, so `omit` was a no-op.

How: Add `requestDimensions` to HttpConfig, return it from readConfig, and forward
`config.requestDimensions` (not the validation-only `config.dimensions`) to
`httpEmbedBatch`. Local dimension checks still use `config.dimensions`.

Details: Coexists with the retry/pacing fields introduced upstream; both feature
sets are preserved. Default behavior unchanged when REQUEST_DIMS is unset.

Impact: gitnexus/src/core/embeddings/http-client.ts; README; unit tests.
@ChamHerry
ChamHerry requested a review from azizur100389 as a code owner July 20, 2026 02:11
@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

CI Report

✅ All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
15148 15086 0 62 16s

✅ All 15086 tests passed

62 test(s) skipped — expand for details

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 81.42% 57599/70739 81.39% 📈 +0.0 🟢 ████████████████░░░░
Branches 68.99% 35815/51912 68.96% 📈 +0.0 🟢 █████████████░░░░░░░
Functions 87.81% 6811/7756 87.77% 📈 +0.0 🟢 █████████████████░░░
Lines 84.91% 51389/60518 84.88% 📈 +0.0 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@magyargergo

magyargergo commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

@GitNexus review

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

REQUEST CHANGES. This PR adds a GITNEXUS_EMBEDDING_REQUEST_DIMS env var to gitnexus/src/core/embeddings/http-client.ts so operators can keep GITNEXUS_EMBEDDING_DIMS set (used to validate the returned embedding vector's length) while omitting — or independently overriding — the dimensions field sent in the outgoing /v1/embeddings request body, for strict backends that reject unrecognized request fields. It's a small, self-contained change (one source file, one doc file, one test file) with no route, schema, or persistence surface touched. It carries one confirmed, low-consequence correctness bug and a real coverage gap on the feature's actual selling point (the decoupled numeric override), both cheap to fix.

Findings

  • MEDIUM — malformed GITNEXUS_EMBEDDING_REQUEST_DIMS throws an error naming the wrong environment variable. In readConfig, the malformed-value branch throws `${EMBEDDING_DIMS_ENV_ERROR_LEAD}, got "${rawRequestDims}"`, reusing the constant defined at line 116 whose literal text is "GITNEXUS_EMBEDDING_DIMS must be a positive integer". Setting GITNEXUS_EMBEDDING_REQUEST_DIMS=garbage therefore prints "GITNEXUS_EMBEDDING_DIMS must be a positive integer, got "garbage"", misdirecting the operator to edit the wrong variable. The lead constant's own doc comment (immediately above, at line 110-116) documents it as specifically the GITNEXUS_EMBEDDING_DIMS error lead, and isHttpEmbeddingDimsError at line 122-123 is exported specifically for the CLI to recognize that config mistake. Functional classification (config error vs. stack dump) still works because both messages share the same substring, so this doesn't crash anything — it only misleads a human debugging their config. Remediation: throw with a distinct lead string naming GITNEXUS_EMBEDDING_REQUEST_DIMS, or parameterize the existing lead by variable name.

  • LOW — the PR's actual new capability (decoupled numeric request-dims) is untested. All coverage the PR adds — http-embedder.test.ts:170-192 and :216-237 — only exercises GITNEXUS_EMBEDDING_REQUEST_DIMS=omit, which drives requestDimensions to undefined (a single boolean branch in readConfig line 154-155). No test sets GITNEXUS_EMBEDDING_DIMS and GITNEXUS_EMBEDDING_REQUEST_DIMS to two different numeric values and asserts the request body carries the request-dims value while response validation still uses the DIMS value — the actual decoupling this PR exists to provide (parsed at line 157-164). A regression that collapses the two back together would pass CI. Remediation: add a test with GITNEXUS_EMBEDDING_DIMS=1024 / GITNEXUS_EMBEDDING_REQUEST_DIMS=512, asserting body.dimensions === 512 while a 1024-length mock response still validates.

  • LOW — omit-aliases and malformed-value path have no test coverage. The regex /^(omit|none|off|false|0)$/i at line 154 accepts five case-insensitive forms; only 'omit' is tested. Likewise no test sets GITNEXUS_EMBEDDING_REQUEST_DIMS to a non-numeric string and asserts on the thrown error (which would also have caught the MEDIUM finding above). Remediation: parametrize the alias list and add one malformed-value assertion.

  • LOW — README under-documents accepted values. README.md:287 and the added paragraph at lines 295-296 document only omit. The code also accepts none/off/false/0 and any positive integer as an explicit override — none of that is documented, so a reader has no way to discover the numeric-override capability from the docs alone.

Change summary and blast radius

Target: PR #2574, base/merge-base 94a528f, head 6f43825. Three files changed, all pure-modify (no adds/deletes/renames): gitnexus/README.md, gitnexus/src/core/embeddings/http-client.ts, gitnexus/test/unit/http-embedder.test.ts.

Core change: HttpConfig gains an optional requestDimensions field (line 32), computed in readConfig and substituted for config.dimensions at the two httpEmbedBatch call sites — httpEmbed line 456 and httpEmbedQuery line 513. I directly verified via GitNexus context on readConfig (Function:gitnexus/src/core/embeddings/http-client.ts:readConfig, found exact, incoming callers getHttpDimensions, httpEmbed, httpEmbedQuery) that response-size validation (config.dimensions ?? DEFAULT_DIMS, used in both httpEmbed and httpEmbedQuery's dimension-mismatch checks) and getHttpDimensions (exported at line 204) still read config.dimensions, not the new requestDimensions — the new field is scoped correctly to the request-body path only, with no fan-out into the ~13 other isHttpMode/dimension-consuming call sites the module's own comments describe. When GITNEXUS_EMBEDDING_REQUEST_DIMS is unset, requestDimensions is initialized directly from dimensions (line 152) before the conditional block, so default behavior is structurally unchanged, not just incidentally preserved.

Coverage and residual risk

Tests present: two new tests cover the omit path on both the batch (embedText) and single-query (embedQuery) code paths, confirming 'dimensions' in body === false while GITNEXUS_EMBEDDING_DIMS still gates the expected response length. ENV_KEYS was updated (test/unit/http-embedder.test.ts:12) so the new var is cleaned up between tests. Tests missing: numeric-override decoupling, non-omit aliases, and the malformed-REQUEST_DIMS error path (see LOW findings above).

Review limitations: I made a direct, verified GitNexus context call against the head index (readConfig, Function kind, file gitnexus/src/core/embeddings/http-client.ts, status: "found") and read the full head source of http-client.ts, the test file, and the README diff hunks myself. I dispatched five parallel expert-lens subagents (correctness, security, blast-radius, coverage, adversarial) per the swarm-lanes workflow; only the coverage lane returned before this review was finalized, and its findings (which independently corroborate the misleading-error-message bug and the numeric-override coverage gap, though at higher severity labels than I've calibrated here) are folded in above. The correctness, security, blast-radius, and adversarial lanes, and the critic gate, did not complete in time and are not reflected — treat this as a partial-coverage review on those four dimensions, though the change's small, single-module surface and the direct verification I performed limit how much a full pass would likely add. No --pdg taint pass was run; this change touches config parsing of a local, operator-controlled env var flowing into an outbound JSON request body, not an external/remote input boundary, so taint analysis is unlikely to be load-bearing here, but that is inference, not verified coverage.

Analyzed base: 94a528f577a21bd90fac5128c16852c08fbee7b7
Analyzed head: 6f43825b8eec9932e2760ff7bdd116a3764b403f

@azizur100389

azizur100389 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the focused fix. I tested this against the current review branch (origin/azizur/origin-blocked-analyze-guidance-2556 at 6f2f9cd0) by merging the PR head cleanly and running:

cd gitnexus && npm test -- --run test/unit/http-embedder.test.ts

Result: 57 tests passed.

I still think this needs a small follow-up before approval:

  1. GITNEXUS_EMBEDDING_REQUEST_DIMS=garbage currently throws an error that says GITNEXUS_EMBEDDING_DIMS must be a positive integer, which points the operator at the wrong env var. Please use a distinct error lead for GITNEXUS_EMBEDDING_REQUEST_DIMS or parameterize the message.

  2. The new numeric override path is not covered. The tests cover REQUEST_DIMS=omit, but not the case where response validation and request payload dimensions intentionally differ, e.g. GITNEXUS_EMBEDDING_DIMS=1024 with GITNEXUS_EMBEDDING_REQUEST_DIMS=512. Please add a test asserting the request body sends 512 while a 1024-length mock response still validates.

  3. The README documents omit, but not the accepted aliases or positive-integer override behavior. Please document the supported values so users can discover the full feature without reading the source.

Also, the PR title validation check is currently failing, so that needs to be fixed before merge.

@magyargergo magyargergo changed the title feat/embedding request dims omit feat(embedding): expose LLM dimensions in the request Jul 20, 2026
magyargergo and others added 2 commits July 20, 2026 20:59
@magyargergo
Merge branch 'main' into feat/embedding-request-dims-omit
d3c34a4
@magyargergo @claude
fix(embeddings): name GITNEXUS_EMBEDDING_REQUEST_DIMS in its own conf…
2bcea63 
…ig error

Address the review findings on abhigyanpatwari#2574.

What:
- A malformed GITNEXUS_EMBEDDING_REQUEST_DIMS now throws an error naming
  GITNEXUS_EMBEDDING_REQUEST_DIMS, not the sibling GITNEXUS_EMBEDDING_DIMS.
- isHttpEmbeddingDimsError recognizes both leads, so the CLI still classifies
  the REQUEST_DIMS config mistake as a clean config error, not a stack dump.
- Tests: numeric-override decoupling (DIMS=1024 validates the response while
  REQUEST_DIMS=512 is sent in the body), the omit aliases (none/off/false/0),
  and the malformed-value error path (which also pins the naming fix).
- README documents the full accepted values: omit-aliases and integer override.

Why: readConfig reused the DIMS error lead for the REQUEST_DIMS branch, so
REQUEST_DIMS=garbage misdirected the operator to edit the wrong variable. The
feature's actual decoupling and its non-omit inputs had no test coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@magyargergo magyargergo changed the title feat(embedding): expose LLM dimensions in the request feat(embeddings): control request-body dimensions via GITNEXUS_EMBEDDING_REQUEST_DIMS Jul 20, 2026
@abhigyanpatwari
abhigyanpatwari merged commit 7534f53 into abhigyanpatwari:main Jul 21, 2026
39 of 40 checks passed
@magyargergo magyargergo mentioned this pull request Jul 21, 2026
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azizur100389 azizur100389 Awaiting requested review from azizur100389 azizur100389 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ChamHerry @magyargergo @azizur100389 @abhigyanpatwari