Skip to content

fix(providers): raise Nous model discovery response cap - #3939

Closed
Vocllum wants to merge 1 commit into
lidge-jun:devfrom
Vocllum:fix/nous-discovery-response-cap
Closed

fix(providers): raise Nous model discovery response cap#3939
Vocllum wants to merge 1 commit into
lidge-jun:devfrom
Vocllum:fix/nous-discovery-response-cap

Conversation

@Vocllum

@Vocllum Vocllum commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Raise the Nous provider's live model-discovery response cap from 256 KiB to 1 MiB.

Problem

Nous's authenticated /v1/models endpoint currently returns a mixed paid/free catalog whose JSON is larger than the provider-specific 262_144 byte limit. A representative response was HTTP 200 with 390 model rows and 566,054 bytes, but OpenCodex rejected it with:

Provider model discovery for "nous" exceeded the 262144-byte response limit

This prevents live catalog reconciliation and leaves the configured Nous model roster stale or incomplete.

Fix

  • Set the Nous provider-specific cap to 1_048_576 bytes.
  • Keep the existing process-wide 4 MiB safety ceiling.
  • Keep the existing maxModels: 512 row limit.
  • Add a deterministic regression test using a 390-row payload larger than 256 KiB and within the new 1 MiB cap.

Verification

  • The regression test fails on the base configuration with Expected: 1048576 / Received: 262144.
  • The regression test passes after the change.
  • The provider model-discovery contract test file passes.
  • Typecheck, privacy scan, and git diff --check pass.
  • On macOS with OpenCodex 2.46.0, the patched local service successfully discovers the live Nous catalog without the previous response-limit error.

The upstream 429/524 responses observed for some free inference models are separate upstream availability/quota issues and are outside this catalog-discovery fix.

Fixes #3938

Summary by CodeRabbit

  • Bug Fixes
    • Increased the maximum response size for Nous model discovery, allowing larger mixed model catalogs to load successfully.
  • Tests
    • Added coverage to verify that larger Nous discovery responses are accepted within the provider-specific limit.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: cda48eb1-53f5-43b5-9ba1-d360d4946592

📥 Commits

Reviewing files that changed from the base of the PR and between 942c028 and e31f5be.

📒 Files selected for processing (2)
  • src/providers/registry.ts
  • tests/providers/provider-model-discovery-contract.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The Nous provider’s live model discovery limit increases from 256 KiB to 1 MiB. A regression test verifies that a 390-row catalog exceeding 256 KiB succeeds under the provider-specific limit.

Changes

Nous discovery response limit

Layer / File(s) Summary
Provider cap and regression test
src/providers/registry.ts, tests/providers/provider-model-discovery-contract.test.ts
nous.modelDiscovery.maxResponseBytes increases from 262_144 to 1_048_576 at src/providers/registry.ts:1542-1545. The test creates a catalog larger than 256 KiB, asserts the 1 MiB cap, and confirms readBoundedDiscoveryJson returns ok: true at tests/providers/provider-model-discovery-contract.test.ts:431-451.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to e31f5

Nous model discovery now accepts valid catalogs up to 1 MiB, including the reported larger catalog, while retaining bounded response handling and the existing model-row limit. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: increasing the Nous model discovery response cap.
Linked Issues check ✅ Passed The changes satisfy issue #3938. src/providers/registry.ts raises the Nous discovery cap to 1 MiB while retaining the 4 MiB process-wide ceiling and existing model limits. `tests/providers/provider-…
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue. The registry update fixes the Nous response-size rejection, and the regression test verifies the required behavior. No unrelated changes are prese…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 58 / 80

이 PR은 Nous Portal(nous)의 라이브 모델 목록을 받을 때 쓰는 응답 크기 한도를 올립니다. 지금 devsrc/providers/registry.ts에서 nous.modelDiscovery.maxResponseBytes262_144(256 KiB)입니다. 실제 인증된 /v1/models 응답은 약 390개 행·약 566 KiB라서, HTTP 200인데도 OpenCodex가 Provider model discovery for "nous" exceeded the 262144-byte response limit로 거절합니다. 그러면 라이브 카탈로그 동기화가 실패하고, 설정된 시드 목록(tencent/hy3:free 등)만 남거나 불완전해집니다. Free 콤보처럼 nous/...:free 슬롯을 쓰는 경로에서 특히 눈에 띕니다.

고치는 방식은 최소입니다. 프로바이더 전용 한도만 1_048_576(1 MiB)로 올리고, 프로세스 전역 천장(src/providers/model-discovery-limits.tsMODEL_DISCOVERY_MAX_RESPONSE_BYTES = 4 MiB)과 maxModels: 512는 그대로 둡니다. readBoundedDiscoveryJson / resolveProviderModelDiscovery는 이미 프로바이더 값을 전역 천장으로 클램프하므로, 이 숫자는 “Nous만 조금 더 허용하되 4 MiB를 넘기지 않음”이라는 기존 계약 안에 있습니다. 같은 1 MiB 값은 이미 Baseten modelDiscovery에도 쓰이고 있어서, 레포 안 선례와도 맞습니다. types.ts/config.ts 분리 캠페인과는 무관한 registry·테스트 한 줄 수정이라 close-don't-rebase 대상이 아닙니다.

테스트는 tests/providers/provider-model-discovery-contract.test.ts에 390행짜리 합성 JSON을 넣고, 바이트 길이가 256 KiB를 넘는지·해석된 maxResponseBytes가 1 MiB인지·readBoundedDiscoveryJson이 성공하는지를 확인합니다. 이슈 #3938에 적힌 관측값(566 KiB / 390 rows)을 재현하려는 의도이고, 업스트림 429/524(일부 free 추론 모델 가용성)는 이번 범위 밖이라고 본문에서 분명히 나눕니다. OAuth 응답 바인딩(#1603 계열)과도 다른 레이어(카탈로그 discovery)입니다.

라인 registry.ts · nous.modelDiscovery.maxResponseBytes - 값 자체는 타당합니다. 관측 566 KiB를 넘기고 전역 4 MiB 아래입니다. 다만 카탈로그가 앞으로 1 MiB를 넘기면 또 같은 증상으로 돌아올 수 있으니, 장기적으로는 “관측 크기 + 여유”를 짧게 남겨 두는 편이 안전합니다(지금은 여유 충분).

라인 provider-model-discovery-contract.test.ts · 새 테스트 - 바이트 한도와 readBoundedDiscoveryJson 성공만 검증합니다. maxModels: 512 아래에서 390행이 extractProviderModelItems까지 통과하는지는 이 테스트가 보지 않습니다. 지금 설정으로는 문제 없지만, “관측 카탈로그가 행 한도에도 안 걸린다”까지 잠그려면 한 줄 더 있으면 더 단단합니다.

경로/심볼 · CI - hygiene / label / resolve-pr / CodeRabbit이 아직 pending입니다. 머지 전에 초록만 확인하면 됩니다. 코드 범위는 파일 2개·+24/−1로 리뷰 부담이 작습니다.

메인테이너의 판단이 필요한 지점

  • 1 MiB가 Nous에 대한 장기 기본값으로 충분한지, 아니면 DeepInfra(512 KiB)·Baseten(1 MiB)처럼 “큰 카탈로그 프로바이더” 군에 그냥 맞춰도 되는지
  • #3938을 이 PR 머지와 함께 닫을지(본문에 Fixes #3938 있음), 아니면 라이브 재확인 코멘트를 남긴 뒤 닫을지
  • 테스트에 extractProviderModelItems/행 한도 단언을 추가할지, 바이트 회귀만으로 충분한지

너의 추천
CI 초록 확인 후 dev에 머지하세요. 이슈 #3938은 Fixes #3938로 같이 닫히게 두면 됩니다. 테스트 보강(390행이 maxModels: 512 안에서 extract까지 ok)은 있으면 좋지만 머지 블로커는 아닙니다. types/config 분리와 무관하니 rebase·보류 없이 독립 랜딩하면 됩니다.

이 댓글은 grok-bot이 작성했습니다

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

Delivered through #3957 as commit 5cd71ec on dev. The original Nous cap change is preserved, with additional isolated regressions through catalog gathering/cache and the management connection probe. Vocllum is retained in the landed Co-authored-by trailer.

Current candidate CI 34166962088 completed with 19 successful jobs and 2 explicit skips; both new 390-row regressions passed. Local product checks were NOT RUN per owner instruction. After merge, the landed tree matched Git's expected integration tree and the commit was verified in dev. Closing this source PR as carried, not claiming it was merged directly. The existing global and raw-row bounds remain.

@lidge-jun lidge-jun closed this Sep 7, 2026
@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants