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

fix(amazonq): validationException if empty supplemental context chunk is sent to the service #5920

Merged
merged 5 commits into from
Nov 8, 2024

Conversation

Will-ShaoHua
Copy link
Contributor

@Will-ShaoHua Will-ShaoHua commented Nov 4, 2024

Problem

  • LSP might return chunk with empty string, which will cause validation exception if such supplemental chunk is sent to the service
  • pretty logging
    updated log string example
2024-11-04 11:36:38.767 [debug] CodeWhispererSupplementalContext:
    isUtg: false,
    isProcessTimeout: false,
    contentsLength: 5208,
    latency: 50.849000096321106
    strategy: OpenTabs_BM25
    Chunk 0:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/service/keyStrokeHandler.ts
        Length: 1777
        Score: 75.30026364918442
    Chunk 1:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/service/referenceInlineProvider.ts
        Length: 1437
        Score: 63.365755359719486
    Chunk 2:
        Path: /Volumes/workplace/ide/aws-toolkit-vscode-staging/packages/core/src/codewhisperer/service/inlineCompletionItemProvider.ts
        Length: 1994
        Score: 47.207284002878936

Solution


License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

github-actions bot commented Nov 4, 2024

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@Will-ShaoHua Will-ShaoHua changed the title Fix empty chunk fix(amazonq): validationException if empty supplemental context chunk is sent to the service Nov 4, 2024
@@ -152,7 +152,7 @@ export async function fetchSupplementalContextForSrcV1(
// DO NOT send code chunk with empty content
getLogger().debug(`CodeWhisperer finished fetching crossfile context out of ${relevantCrossFilePaths.length} files`)
return {
supplementalContextItems: supplementalContexts.filter((item) => item.content.trim().length !== 0),
Copy link
Contributor Author

@Will-ShaoHua Will-ShaoHua Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to upstream caller supplementalContextUtil.ts

@Will-ShaoHua
Copy link
Contributor Author

Will-ShaoHua commented Nov 4, 2024

should probably refactor the entire structure of supplementalContextUtil.ts, crossfileContextUtil.ts, utgUtil.ts, it's hard to write unit test cases if everything is 1st class functions instead of classes and methods.

@Will-ShaoHua Will-ShaoHua marked this pull request as ready for review November 4, 2024 21:16
@Will-ShaoHua Will-ShaoHua requested review from a team as code owners November 4, 2024 21:16
strategy: ${supplementalContext.strategy}`,
4,
true
).trimStart()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for thinking about the log format, that helps with troubleshooting for both users and devs.

@Will-ShaoHua Will-ShaoHua merged commit 10b3f1c into aws:master Nov 8, 2024
23 of 25 checks passed
@Will-ShaoHua Will-ShaoHua deleted the fix-empty-chunk branch November 8, 2024 22:16
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.

3 participants