Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Carry over requestOptions from config to completion modles #4813

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

splatch
Copy link
Contributor

@splatch splatch commented Mar 25, 2025

Description

The configuration handling of models and completion models is fairly similar, however they differ in carrying over request options. This PR attempts to align both parts to have common logic which accepts config level request options to be taken into account.

Checklist

  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

Screenshots

No UI changes.

Testing instructions

With this PR request options set at the config level should be considered also at tabCompletionModel level.

…at models.

Signed-off-by: Łukasz Dywicki <luke@code-house.org>
@splatch splatch requested a review from a team as a code owner March 25, 2025 17:58
@splatch splatch requested review from sestinj and removed request for a team March 25, 2025 17:58
Copy link

netlify bot commented Mar 25, 2025

Deploy Preview for continuedev ready!

Name Link
🔨 Latest commit d0b8c45
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/67e2eecb2b0e190008edd70a
😎 Deploy Preview https://deploy-preview-4813--continuedev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sestinj
Copy link
Contributor

sestinj commented Apr 1, 2025

@splatch before looking deeper into this PR, it would be helpful to understand how you're using requestOptions—right now we don't support top-level requestOptions in config.yaml (https://docs.continue.dev/reference) and we don't currently have plans to. If that continues to be the case, I don't think we would want to merge this, given we are eventually going to move toward YAML format

@splatch
Copy link
Contributor Author

splatch commented Apr 1, 2025

Hello @sestinj, reason for above changes was inability to get requestOptions for completion models. If you will have a look again on the change set there is one extra line: applyRequestOptionsToModels(tabAutocompleteModels, config);.
Without it you can't pass any options to completion models. So even if that part of configuration relies on same ModelDescription as models, it doesn't work.

I was stuck with 1.0.2 release of jetbrains plugin which worked with config.json, thus I started my patchwork from that place.

@sestinj
Copy link
Contributor

sestinj commented Apr 9, 2025

sure, this makes sense and the PR is good to go once these tests re-run.

what I'm wondering is more of just which of the request options you find helpful (a bit of user research into how people are using them : ))

@splatch
Copy link
Contributor Author

splatch commented Apr 9, 2025

Hah, nothing special. Got a ollama instance behind nginx with self signed ssl certificate in front. Intellij have had hard times reaching it without requestOptions set.

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.

2 participants