Skip to content

fix(smart-apply): custom model now checks for token limit #7940

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ichim-david
Copy link
Collaborator

This pr makes a couple of changes to the smart apply behavior of the new custom model such as:

  • Added logic to validate token counts against context window limits in CustomModelSelectionProvider. If the token count exceeds the limit, an error is thrown.
  • Added the trimLLMNewlines function to preserve or remove newlines based on the original text, ensuring consistent formatting in transformed responses.
    I think that if the original file didn't have a starting and ending newline our smart apply task shouldn't insert them either,
    as we should stay as close as possible to what you see is what you get.
  • Updated responseTransformer to skip processing for in-progress messages and insert mode tasks.
    If we just insert the text we don't need it to be cleaned up, it's basically a copy paste job

Test plan

  • Try to call smart apply on a file that has over 12000 tokens
  • Try to delete file content and hit apply and notice that it inserts the content as seen in the chat window

…smart apply

The new Qwen based smart apply model performs an entire-file replace, so we can simply remove the leading newlines from the response. This simplifies the response transformation logic for this model since we no longer need todo extra formatting given
the fact that we replace the entire file and it should be correctly
formatted already
The new Qwen based smart apply model performs an entire-file replace, but we should only trim the final newline if the original file ends with a newline.

This commit updates the response transformation logic for the new smart apply model to conditionally trim trailing whitespace based on whether the original file ends with a newline character. This ensures that files without a final newline are not incorrectly modified.
…odels

This commit enhances the response transformation logic for the new Qwen-based smart apply model to provide more accurate and consistent results.

Key changes:

- Adds a `trimLLMNewlines` function to preserve or remove leading/trailing newlines based on the original file content. This ensures that files without a final newline are not incorrectly modified, and files with a newline retain it.
- Skips processing for in-progress messages from smart apply custom models.
- Simplifies the response transformation logic by consolidating newline handling and extraction into dedicated functions.
- Improves test coverage to validate the newline preservation logic.
…t newline test

This commit addresses two issues related to the smart apply feature:

- Fixes an issue where in-progress messages were not being skipped correctly when using insert mode with smart apply custom models. The condition to skip processing for in-progress messages is updated to include the insert mode.
- Removes a redundant test case in `response-transformer.test.ts` that was asserting the addition of a newline character for smart apply without an empty selection range. This test is no longer necessary due to changes in how newlines are handled.
…imit validation

This commit improves the logic for selecting the entire file in the `CustomModelSelectionProvider` and adds validation to prevent exceeding the context window's input token limit.

- Modifies the `getSelectedText` method to prioritize `shouldAlwaysUseEntireFile` and refactors the token limit check to occur before the full file rewrite check.
- Adds a test case to verify that an error is thrown when the token count exceeds the context window input limit.
- Mocks the `vscode.Range` to avoid errors during testing.
…ts for custom models

This commit introduces several enhancements to the smart apply custom model feature, focusing on token handling and test coverage.

- Adds tests to validate the behavior of `CustomModelSelectionProvider` under different token count scenarios, including exceeding the context window input limit, staying within limits but below a threshold, and always returning `ENTIRE_FILE` when `shouldAlwaysUseEntireFile` is true.
- Updates the `trimLLMNewlines` function to handle both `\n` and `\r\n` newline characters, ensuring correct newline preservation across different operating systems.
- Introduces a `SMART_APPLY_MODEL_SET` for efficient checking of smart apply custom models.
@ichim-david ichim-david requested a review from umpox May 19, 2025 12:34
@ichim-david ichim-david requested a review from julialeex June 16, 2025 16:37
Copy link

‼️ Hey @sourcegraph/cody-security, please review this PR carefully as it introduces the usage of an unsafe_ function or abuses PromptString.

@ichim-david ichim-david force-pushed the ichimdav/smart_apply_cody_5727 branch from bf1b2c8 to 27c22b9 Compare June 22, 2025 09:06
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