Skip to content

Add initial implementation of auto-modeling#2448

Merged
koesie10 merged 8 commits intomainfrom
koesie10/auto-model
May 30, 2023
Merged

Add initial implementation of auto-modeling#2448
koesie10 merged 8 commits intomainfrom
koesie10/auto-model

Conversation

@koesie10
Copy link
Copy Markdown
Member

This adds an initial implementation of auto-modeling using the /repos/:owner/:name/code-scanning/codeql/auto-model endpoint. The button is hidden behind the codeQL.dataExtensions.llmGeneration setting.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@koesie10 koesie10 force-pushed the koesie10/auto-model branch from 95b2373 to a88e683 Compare May 25, 2023 11:28
@koesie10 koesie10 marked this pull request as ready for review May 25, 2023 12:09
@koesie10 koesie10 requested review from a team as code owners May 25, 2023 12:09
@koesie10 koesie10 requested review from charisk and starcke May 25, 2023 12:09
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

Thanks for taking this over the line! I've not tested it locally yet but we can tweak separately.

Comment thread extensions/ql-vscode/src/data-extensions-editor/auto-model.ts Outdated
Comment thread extensions/ql-vscode/src/data-extensions-editor/auto-model.ts
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

Thanks for taking this over the line! I've not tested it locally yet but we can tweak separately.

Copy link
Copy Markdown
Contributor

@starcke starcke left a comment

Choose a reason for hiding this comment

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

Comments from reviewing createAutoModelRequest.

Comment thread extensions/ql-vscode/src/data-extensions-editor/auto-model.ts Outdated
Comment thread extensions/ql-vscode/src/data-extensions-editor/auto-model.ts
Copy link
Copy Markdown
Contributor

@starcke starcke left a comment

Choose a reason for hiding this comment

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

Looks good, just some minor comments/questions.

Comment thread extensions/ql-vscode/src/data-extensions-editor/auto-model.ts
const predictedBySignature: Record<string, Method[]> = {};
for (const method of predicted) {
if (!method.classification) {
continue;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we log or warn if some samples come back unclassified?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think so, I think we should log that on the Turbomodel side, the user can't do anything with this information.

// Order the sinks by the input alphabetically. This will ensure that the first argument is always
// first in the list of sinks, the second argument is always second, etc.
// If we get back "Argument[1]" and "Argument[3]", "Argument[1]" should always be first
sinks.sort((a, b) => (a.input ?? "").localeCompare(b.input ?? ""));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesnt matter I think but is Argument[10] also going to be sorted properly?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, it currently doesn't sort properly. I'll change it so it properly sorts them.

Comment thread extensions/ql-vscode/test/unit-tests/data-extensions-editor/auto-model.test.ts Outdated
Comment thread extensions/ql-vscode/test/unit-tests/data-extensions-editor/auto-model.test.ts Outdated
Copy link
Copy Markdown
Contributor

@charisk charisk left a comment

Choose a reason for hiding this comment

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

LGTM!

@koesie10 koesie10 merged commit 5a66d6f into main May 30, 2023
@koesie10 koesie10 deleted the koesie10/auto-model branch May 30, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants