Add "View Autofixes" feature for variant analysis results #4065

jcogs33 · 2025-06-29T02:12:01Z

Description

Adds a "View Autofixes" feature for variant analysis results. This is a canary-only feature.
Generates autofixes for selected variant analysis results and opens the resulting autofix outputs in a markdown file.
Activated via the "View Autofixes" button in the variant analysis webview or via right-clicking the variant analysis query history item.

At a high-level, the implementation:

Checks for a local checkout of autofix
Sets up to run autofix by:
- Overriding the relevant query help
- Getting the relevant SARIF(s)
- Downloading source root(s)
Runs autofix locally
Outputs autofix results in a markdown file

Commit-by-commit review recommended. Let me know if you want me to split anything onto a separate PR.

Consideration

See in-line comments
Change from my original implementation: Instead of downloading databases and then extracting the source root from the database, I directly download the source root since it seemed a bit faster. Let me know if you think the database approach is better.
I will make a follow-up PR to convert this to support Go autofix instead of cocofix, and I'll update the queryHelpOverrideDirectory to the new go/pkg/autofix/prompt/qhelps directory in that PR.

Testing

I've manually tested this feature but have not added unit/view/integration tests since it seems another canary feature did not add tests. Let me know if I should add tests after all.
Note: There is a known issue with the E2E CI test failing. It is not caused by changes in this PR.

…ix limit

This function will increment file numbers when running autofix in a loop

extensions/ql-vscode/src/common/interface-types.ts

extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts

jcogs33 · 2025-07-02T01:51:53Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+  filterSort: RepositoriesFilterSortStateWithIds = defaultFilterSortState,
+  variantAnalyses: Map<number, VariantAnalysis>,
+  credentials: Credentials,
+  logger: NotificationLogger,


In retrospect, I'm not sure if I should be using NotificationLogger. 🤔 Let me know if I should use extLogger instead of or in addition to NotificationLogger?
Also let me know if I should add more logging in general?

If you aren't showing notifications, I think you can use the BaseLogger type here.

jcogs33 · 2025-07-02T01:54:42Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+ * @throws Error if the AUTOFIX_PATH environment variable is not set or the path does not exist.
+ */
+function findLocalAutofix(): string {
+  const localAutofixPath = process.env.AUTOFIX_PATH;


I'm currently requiring that users set this environment variable (see corresponding documentation). Let me know if there is something better I should do (config file, etc.) instead of using this environment variable.

I think using a config setting would make sense (something like codeQL.autofix.path). I'm not sure how VS Code picks up environment variables, but it's a lot easier to set config settings than environment variables.

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

koesie10

Thanks! I've not tested this locally, but the demo you shared looks good!

extensions/ql-vscode/src/common/interface-types.ts

extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx

koesie10 · 2025-07-02T08:04:34Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+  filterSort: RepositoriesFilterSortStateWithIds = defaultFilterSortState,
+  variantAnalyses: Map<number, VariantAnalysis>,
+  credentials: Credentials,
+  logger: NotificationLogger,


If you aren't showing notifications, I think you can use the BaseLogger type here.

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

koesie10 · 2025-07-02T08:33:48Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+    "--source-root",
+    srcRootPath,
+    "--model",
+    "capi-dev-4o", // may fail with older versions of cocofix


Should this be configurable (something like codeQL.autofix.model)?

Yes. gpt 4o is going away soon. And we are trying various models to replace it.

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

koesie10 · 2025-07-02T08:35:09Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+      env: {
+        CAPI_DEV_KEY: process.env.CAPI_DEV_KEY,
+        PATH: process.env.PATH,
+      },


Would it make sense to allow specifying the environment variables in the config (in something like codeQL.autofix.env)?

koesie10 · 2025-07-02T08:35:45Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+      if (showCommand) {
+        void logger.log(`Spawning '${bin} ${args.join(" ")}' in ${cwd}`);
+      }
+      if (args.some((a) => a === undefined || a === "")) {


I don't think we need this check, it's fine for the command to fail if this is the case.

koesie10 · 2025-07-02T08:38:36Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+          `Invalid empty or undefined arguments: ${args.join(" ")}`,
+        );
+      }
+      const p = spawn(bin, args, { stdio: [0, 1, 2], ...options });


I think stdio: [0, 1, 2] will ensure that the autofix CLI logs to the stdout/stderr of VS Code, but I don't think you can view those. I think it would make sense to log those to the extension logs. A good example of this can be found in runCodeQlCliInNewProcess, specifically in handleProcessOutput.

aeisenberg

Looks good. I haven't tried this locally.

extensions/ql-vscode/src/compare/compare-view.ts

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

aeisenberg · 2025-07-02T17:50:16Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+    localAutofixPath,
+    "prompt-templates",


Does localAutofixPath need to be a checkout of the autofix repo? Or does it point to a binary?

The new location for the qhelp overrides is here: go/pkg/autofix/prompt/qhelps. But, this is only available in a full checkout of the repo. It's not available with the go binary. The cocofix prompt-templates will go away.

Yes, it needs to be a checkout of the autofix repo. Will that be a problem for any reason?
I've included instructions about checking out the repo in the documentation for this feature (see documentation links beneath my original commits). Note that DCA also expects a checkout of the autofix repo when running autofix locally with DCA, and I mimicked that expectation.

I'll update the prompt-template directory when I do a follow-up PR to update from cocofix to Go.

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

aeisenberg · 2025-07-02T17:56:23Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+
+    // Extract the tarball
+    await new Promise<void>((resolve, reject) => {
+      const process = spawn("tar", ["-xzf", tarballPath], { cwd: downloadDir });


We're using tar-stream elsewhere for extracting tarballs. It would be better to use something like that instead of spawning a new process.

Just be aware that tar-stream is currently a dev dependency and would need to be moved to a dependency.

aeisenberg · 2025-07-02T17:57:22Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+  } = await getRepoStoragePaths(autofixOutputStoragePath, nwo);
+
+  // Get autofix binary.
+  // Switch to Go binary in the future and have user pass full path


aeisenberg · 2025-07-02T17:58:16Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+    "--source-root",
+    srcRootPath,
+    "--model",
+    "capi-dev-4o", // may fail with older versions of cocofix


Yes. gpt 4o is going away soon. And we are trying various models to replace it.

aeisenberg · 2025-07-02T18:02:40Z

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

+      "```\n\n</details>\n\n ### Notes\n - notes placeholder\n\n";
+
+    // Format the content with Markdown
+    const formattedContent = `## ${header}\n\n${frontFormatting}${content}${backFormatting}`;


If content itself has three backticks, then this formatting will be broken. I don't know how likely that is. In this case, probably just doing a find/replace of three with \``.

…alysis view

Co-authored-by: Andrew Eisenberg <[email protected]>

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

jcogs33 · 2025-07-03T00:48:48Z

Thanks for the reviews @koesie10 and @aeisenberg! 🙇
I've addressed some of your comments. I'll re-request your reviews once I'm finished with them all.

Jami Cogswell added 22 commits June 27, 2025 19:12

Add 'View Autofixes' button to variant analysis view

8dfcf8b

Add 'View Autofixes' query history command for variant analysis items

1103437

Add 'view-autofixes.ts' file for main implementation

5f5a20f

Check for local autofix

fb39455

Override query help

27bdc31

Get full names (owner/repo) of the selected repositories

9a8bd79

Get storage paths for autofix output

66d9526

Add function to process the selected repositories

9c4d7a7

Get SARIF for variant analysis repo

2afd2ee

Read 'repo_task.json' file

2bcf42e

Download source root

b0b8b62

Add function for running autofix on a given repo

05c398f

Get storage paths for repository-specific autofix output

4d5437f

Set up to run autofix on repo results

5dfaf3f

Add function for setting variable autofix args

03585af

Add function for executing autofix

19400a9

Add function for running autofix on a given SARIF

3c7d9ca

Run autofix for case when the repo result count does not exceed the f…

74e8277

…ix limit

Add function that appends a suffix to a given file path

e7076f9

This function will increment file numbers when running autofix in a loop

Run autofix for case when the repo result count exceeds the fix limit

e04eb62

Add markdown formatting to autofix output

2c12d71

Combine and output final autofix results

6b47932

jcogs33 force-pushed the jcogs33/view-autofixes branch 3 times, most recently from 8095989 to c5f0e5e Compare June 30, 2025 21:29

Add canary

3ff5d64

jcogs33 force-pushed the jcogs33/view-autofixes branch from c5f0e5e to 3ff5d64 Compare July 2, 2025 00:44

jcogs33 commented Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/common/interface-types.ts Outdated Show resolved Hide resolved

jcogs33 commented Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts Outdated Show resolved Hide resolved

jcogs33 commented Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts Outdated Show resolved Hide resolved

jcogs33 commented Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts Show resolved Hide resolved

jcogs33 commented Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts Show resolved Hide resolved

jcogs33 commented Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts Show resolved Hide resolved

jcogs33 marked this pull request as ready for review July 2, 2025 02:21

jcogs33 requested review from a team as code owners July 2, 2025 02:21

koesie10 reviewed Jul 2, 2025

View reviewed changes

Remove config listener for canary setting

4e089e7

aeisenberg reviewed Jul 2, 2025

View reviewed changes

Jami Cogswell and others added 6 commits July 2, 2025 20:27

canary: use separate message for the user settings for the variant an…

04a858a

…alysis view

Remove progress updates that don't display

7d3e403

Fix missing await

161c5de

Use parse.dir instead of dirname

e23699f

Remove createVarAutofixArgs function

22d6429

Update 'autofix not found' error message

3030f6c

Co-authored-by: Andrew Eisenberg <[email protected]>

github-advanced-security bot found potential problems Jul 2, 2025

View reviewed changes

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts Fixed Show fixed Hide fixed

Fix prettier formatting

ab5ef91

Add "View Autofixes" feature for variant analysis results #4065

Are you sure you want to change the base?

Add "View Autofixes" feature for variant analysis results #4065

Conversation

jcogs33 commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Consideration

Testing

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

koesie10 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeisenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jcogs33 commented Jul 3, 2025

Uh oh!

Uh oh!

jcogs33 commented Jun 29, 2025 •

edited

Loading