Skip to content

Commit 03c64ef

Browse files
henrymercerhenrymercer
committed
Add more documentation for ML-powered JS queries status report
Also be more explicit about which version strings are reportable in the code.
1 parent cc622a0 commit 03c64ef

File tree

9 files changed

+84
-72
lines changed

9 files changed

+84
-72
lines changed

‎lib/config-utils.js‎

Lines changed: 2 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/config-utils.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/util.js‎

Lines changed: 30 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/util.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/util.test.js‎

Lines changed: 8 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/util.test.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/config-utils.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { RepositoryNwo } from "./repository";
1818
import {
1919
codeQlVersionAbove,
2020
GitHubVersion,
21-
ML_POWERED_JS_QUERIES_PACK_NAME,
21+
ML_POWERED_JS_QUERIES_PACK,
2222
} from "./util";
2323

2424
// Property names from the user-supplied config file.
@@ -296,18 +296,15 @@ async function addBuiltinSuiteQueries(
296296
languages.includes("javascript") &&
297297
(found === "security-extended" || found === "security-and-quality") &&
298298
!packs.javascript?.some(
299-
(pack) => pack.packName === ML_POWERED_JS_QUERIES_PACK_NAME
299+
(pack) => pack.packName === ML_POWERED_JS_QUERIES_PACK.packName
300300
) &&
301301
(await featureFlags.getValue(FeatureFlag.MlPoweredQueriesEnabled)) &&
302302
(await codeQlVersionAbove(codeQL, CODEQL_VERSION_ML_POWERED_QUERIES))
303303
) {
304304
if (!packs.javascript) {
305305
packs.javascript = [];
306306
}
307-
packs.javascript.push({
308-
packName: ML_POWERED_JS_QUERIES_PACK_NAME,
309-
version: "~0.0.2",
310-
});
307+
packs.javascript.push(ML_POWERED_JS_QUERIES_PACK);
311308
}
312309

313310
const suites = languages.map((l) => `${l}-${suiteName}.qls`);

‎src/util.test.ts‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -297,32 +297,26 @@ const ML_POWERED_JS_STATUS_TESTS: Array<[PackWithVersion[], string]> = [
297297
[[], "false"],
298298
[[{ packName: "someOtherPack" }], "false"],
299299
[
300-
[
301-
{ packName: "someOtherPack" },
302-
{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.0.2" },
303-
],
304-
"~0.0.2",
305-
],
306-
[
307-
[{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.0.2" }],
308-
"~0.0.2",
300+
[{ packName: "someOtherPack" }, util.ML_POWERED_JS_QUERIES_PACK],
301+
util.ML_POWERED_JS_QUERIES_PACK.version!,
309302
],
310-
[[{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME }], "other"],
303+
[[util.ML_POWERED_JS_QUERIES_PACK], util.ML_POWERED_JS_QUERIES_PACK.version!],
304+
[[{ packName: util.ML_POWERED_JS_QUERIES_PACK.packName }], "other"],
311305
[
312-
[{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "~0.0.1" }],
306+
[{ packName: util.ML_POWERED_JS_QUERIES_PACK.packName, version: "~0.0.1" }],
313307
"other",
314308
],
315309
[
316310
[
317-
{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "0.0.1" },
318-
{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME, version: "0.0.2" },
311+
{ packName: util.ML_POWERED_JS_QUERIES_PACK.packName, version: "0.0.1" },
312+
{ packName: util.ML_POWERED_JS_QUERIES_PACK.packName, version: "0.0.2" },
319313
],
320314
"other",
321315
],
322316
[
323317
[
324318
{ packName: "someOtherPack" },
325-
{ packName: util.ML_POWERED_JS_QUERIES_PACK_NAME },
319+
{ packName: util.ML_POWERED_JS_QUERIES_PACK.packName },
326320
],
327321
"other",
328322
],

‎src/util.ts‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as semver from "semver";
1010
import { getApiClient, GitHubApiDetails } from "./api-client";
1111
import * as apiCompatibility from "./api-compatibility.json";
1212
import { CodeQL, CODEQL_VERSION_NEW_TRACING } from "./codeql";
13-
import { Config } from "./config-utils";
13+
import { Config, PackWithVersion } from "./config-utils";
1414
import { Language } from "./languages";
1515
import { Logger } from "./logging";
1616

@@ -628,46 +628,60 @@ export function checkNotWindows11() {
628628
}
629629
}
630630

631-
export const ML_POWERED_JS_QUERIES_PACK_NAME =
632-
"codeql/javascript-experimental-atm-queries";
633-
634631
/**
635-
* Set containing version strings of the ML-powered JS query pack that are reportable.
636-
*
637-
* We restrict the set of version strings we report to limit the cardinality of the ML-powered JS
638-
* queries status report field, since some platforms that ingest this status report charge based on
639-
* the cardinality of the fields.
632+
* The ML-powered JS query pack to add to the analysis if a repo is opted into the ML-powered
633+
* queries beta.
640634
*/
641-
export const ML_POWERED_JS_QUERIES_REPORTABLE_VERSIONS = new Set(["~0.0.2"]);
635+
export const ML_POWERED_JS_QUERIES_PACK: PackWithVersion = {
636+
packName: "codeql/javascript-experimental-atm-queries",
637+
version: "~0.0.2",
638+
};
642639

643640
/**
644641
* Get information about ML-powered JS queries to populate status reports with.
645642
*
646643
* This will be:
647644
*
648-
* - The version string if the analysis will use a specific version of the pack and that version
649-
* string is within {@link ML_POWERED_JS_QUERIES_REPORTABLE_VERSIONS}.
645+
* - The version string if the analysis is using the ML-powered query pack that will be added to the
646+
* analysis if the repo is opted into the ML-powered queries beta, i.e.
647+
* {@link ML_POWERED_JS_QUERIES_PACK.version}. If the version string
648+
* {@link ML_POWERED_JS_QUERIES_PACK.version} is undefined, then the status report string will be
649+
* "latest", however this shouldn't occur in practice (see comment below).
650650
* - "false" if the analysis won't run any ML-powered JS queries.
651651
* - "other" in all other cases.
652652
*
653+
* Our goal of the status report here is to allow us to compare the occurrence of timeouts and other
654+
* errors with ML-powered queries turned on and off. We also want to be able to compare minor
655+
* version bumps caused by us bumping the version range of `ML_POWERED_JS_QUERIES_PACK` in a new
656+
* version of the CodeQL Action. For instance, we might want to compare the `~0.1.0` and `~0.0.2`
657+
* version strings.
658+
*
659+
* We restrict the set of strings we report here by excluding other version strings and combinations
660+
* of version strings. We do this to limit the cardinality of the ML-powered JS queries status
661+
* report field, since some platforms that ingest this status report bill based on the cardinality
662+
* of its fields.
663+
*
653664
* This function lives here rather than in `init-action.ts` so it's easier to test, since tests for
654665
* `init-action.ts` would each need to live in their own file. See `analyze-action-env.ts` for an
655666
* explanation as to why this is.
656667
*/
657668
export function getMlPoweredJsQueriesStatus(config: Config): string {
658669
const mlPoweredJsQueryPacks = (config.packs.javascript || []).filter(
659-
(pack) => pack.packName === ML_POWERED_JS_QUERIES_PACK_NAME
670+
(pack) => pack.packName === ML_POWERED_JS_QUERIES_PACK.packName
660671
);
661672
if (mlPoweredJsQueryPacks.length === 0) {
662673
return "false";
663674
}
664675
const firstVersionString = mlPoweredJsQueryPacks[0].version;
665676
if (
666677
mlPoweredJsQueryPacks.length === 1 &&
667-
firstVersionString &&
668-
ML_POWERED_JS_QUERIES_REPORTABLE_VERSIONS.has(firstVersionString)
678+
ML_POWERED_JS_QUERIES_PACK.version === firstVersionString
669679
) {
670-
return firstVersionString;
680+
// We should always specify an explicit version string in `ML_POWERED_JS_QUERIES_PACK`,
681+
// otherwise we won't be able to make changes to the pack unless those changes are compatible
682+
// with each version of the CodeQL Action. Therefore in practice, we should never hit the
683+
// `latest` case here.
684+
return ML_POWERED_JS_QUERIES_PACK.version || "latest";
671685
}
672686
return "other";
673687
}

0 commit comments

Comments
 (0)