Skip to content

fix: keep a reported 4096 context when num_ctx is also set - #695

Open
SashaMIT wants to merge 1 commit into
off-grid-ai:mainfrom
SashaMIT:codered-context-4096
Open

SashaMIT wants to merge 1 commit into
off-grid-ai:mainfrom
SashaMIT:codered-context-4096

Conversation

@SashaMIT

@SashaMIT SashaMIT commented Sep 27, 2026 •

Copy link
Copy Markdown

Summary

  • fetchRemoteModelInfo of an Ollama model whose llama.context_length is 4096, with parameters num_ctx 16384, returned 16384.
  • 4096 was both the fallback and a real context, so a reported 4096 was treated as missing. An empty model_info still uses num_ctx. An 8192 context still stays 8192.

Test plan

  • npx jest __tests__/unit/stores/remoteModelCapabilities.test.ts -t "keeps a reported 4096"
  • The existing empty-model_info fallback still returns 16384.

Summary by CodeRabbit

  • Bug Fixes
    • Remote model context lengths now reflect the value reported by the model when available, even if a different num_ctx value is configured.
    • When no positive context length is reported and no num_ctx value is available, the context length defaults to 4096 tokens.

A llama.context_length of 4096 was treated as missing, so parameters num_ctx replaced it.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: off-grid-ai/OGAM/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: ac130091-bc86-4cd1-8bd4-5d853086db9d

📥 Commits

Reviewing files that changed from the base of the PR and between 321b7e4 and 9ea2c82.

📒 Files selected for processing (2)
  • __tests__/unit/stores/remoteModelCapabilities.test.ts
  • src/stores/remoteModelCapabilities.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The Ollama capability extractor now distinguishes an explicitly reported context length of 4096 from a missing context length. It uses num_ctx only when model_info has no positive value and defaults to 4096 when neither source provides one.

Changes

Ollama context length

Layer / File(s) Summary
Context length resolution
src/stores/remoteModelCapabilities.ts, __tests__/unit/stores/remoteModelCapabilities.test.ts
The extractor uses num_ctx only when model_info has no positive context length, then defaults to 4096 if neither source provides one. The test verifies that a reported 4096 remains unchanged when num_ctx is 16384.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: alichherawalla

Merge Risk: ⚪ Minimal · up to 9ea2c

The change retains an explicitly reported 4096 context instead of replacing it with num_ctx 16384. The supplied review context identifies no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the bug, expected behavior, and focused tests. However, it omits required template sections, including Type of Change, Checklist, Related Issues, and Additional Notes. Use the repository template. Add the Type of Change selection, complete the applicable General and Testing checklist items, and include Related Issues and Additional Notes or remove those sections if the repository permits it.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix: preserving a reported context length of 4096 when num_ctx is also set.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

__tests__/unit/stores/remoteModelCapabilities.test.ts

Oops! Something went wrong! :(

ESLint: 8.57.1

Error: .eslintrc.js » @react-native/eslint-config``#overrides[4]:
Environment key "jest/globals" is unknown

at /.eslint-tmp/node_modules/.pnpm/@eslint+eslintrc@2.1.4_supports-color@8.1.1/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2079:23
at Array.forEach (<anonymous>)
at ConfigValidator.validateEnvironment (/.eslint-tmp/node_modules/.pnpm/@eslint+eslintrc@2.1.4_supports-color@8.1.1/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2073:34)
at ConfigValidator.validateConfigArray (/.eslint-tmp/node_modules/.pnpm/@eslint+eslintrc@2.1.4_supports-color@8.1.1/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2223:18)
at CascadingConfigArrayFactory._finalizeConfigArray (/.eslint-tmp/node_modules/.pnpm/@eslint+eslintrc@2.1.4_supports-color@8.1.1/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3985:23)
at CascadingConfigArrayFactory.getConfigArrayForFile (/.eslint-tmp/node_modules/.pnpm/@eslint+eslintrc@2.1.4_supports-color@8.1.1/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:3791:21)
at FileEnumerator._iterateFilesWithFile (/.eslint-tmp/node_modules/.pnpm/eslint@8.57.1_supports-color@8.1.1/node_modules/eslint/lib/cli-engine/file-enumerator.js:368:43)
at FileEnumerator._iterateFiles (/.eslint-tmp/node_modules/.pnpm/eslint@8.57.1_supports-color@8.1.1/node_modules/eslint/lib/cli-engine/file-enumerator.js:349:25)
at FileEnumerator.iterateFiles (/.eslint-tmp/node_modules/.pnpm/eslint@8.57.1_supports-color@8.1.1/node_modules/eslint/lib/cli-engine/file-enumerator.js:299:59)
at iterateFiles.next (<anonymous>)
src/stores/remoteModelCapabilities.ts

ESLint skipped: the matched ESLint configuration already failed (config-incompatibility).


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.

@sonarqubecloud

Copy link
Copy Markdown

This branch has not been deployed

No deployments
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